[PATCH 1/2] KVM: PPC: BOOK4S HV: Fix memslot rollback in H_SVM_INIT_START

[PATCH 1/2] KVM: PPC: BOOK4S HV: Fix memslot rollback in H_SVM_INIT_START

[Ritesh Harjani (IBM)]

If registering a memslot with the ultravisor fails,
kvmppc_h_svm_init_start() is supposed to undo the memslots it has
already registered. The loop walks them with "m", however it wrongly
passes "memslot" instead (which failed to get registered).

Fix the loop by passing "m" instead, the memslot which was registered
successfully, for rollback from the beginning.

Fixes: 2027a24a75ce ("KVM: PPC: Book3S HV: Disable page merging in H_SVM_INIT_START")
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 463aef870c4e..190f64d992d8 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -497,7 +497,7 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
 		kvm_for_each_memslot(m, bkt, slots) {
 			if (m == memslot)
 				break;
-			__kvmppc_uvmem_memslot_delete(kvm, memslot);
+			__kvmppc_uvmem_memslot_delete(kvm, m);
 		}
 	}

--
2.39.5

[PATCH 2/2] KVM: PPC: BOOK3S HV: Use ksm_disable() for secure guests

[Ritesh Harjani (IBM)]

Pages of a secure guest are moved to device private memory with
migrate_vma_*(), which can't be done for a KSM page. So, when a guest
wants to switch to secure mode, kvmppc_memslot_page_merge(), walks the
VMAs backing each memslot and calls for ksm_madvise(MADV_UNMERGEABLE) on
them. It does the reverse if the H_SVM_INIT_START fails or when the
memslot is getting deleted.

Instead of using ksm_madvise(), one can use ksm_disable() on the mm.
IIUC, ksm_disable() was introduced for this purpose itself in commit
  2c281f54f556e ("mm/ksm: move disabling KSM from s390/gmap code to KSM code").

Also, it seems there are other existing issues in current code.
For e.g. The current code can turn KSM on for the memory the VMM never
marked mergeable. This can happen say for a guest whose H_SVM_INIT_START
fails and therefore the cleanup routine ends up setting VM_MERGEABLE on
it's memory.

Note: This was spotted and reported in one of the series which was
converting legacy vm_flags_t to vma_flags_t, showing concern around why
powerpc has to call into an mm internal functions. Besides, this is the
only remaining user of ksm_madvise(), outside of mm.

Let's fix the existing issues and also address the reported concern.

Reported-by: David Hildenbrand (Arm) <david@kernel.org>
Closes: https://lore.kernel.org/linuxppc-dev/1c0b1368-a381-4b25-bba9-7a4f8e92b764@kernel.org/
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/include/asm/kvm_book3s_uvmem.h |  1 +
 arch/powerpc/kvm/book3s_hv_builtin.c        | 26 ++++++++
 arch/powerpc/kvm/book3s_hv_uvmem.c          | 69 ++++-----------------
 mm/ksm.c                                    |  1 -
 4 files changed, 40 insertions(+), 57 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s_uvmem.h b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
index 0a6319448cb6..c8cc11325639 100644
--- a/arch/powerpc/include/asm/kvm_book3s_uvmem.h
+++ b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
@@ -27,6 +27,7 @@ int kvmppc_uvmem_memslot_create(struct kvm *kvm,
 		const struct kvm_memory_slot *new);
 void kvmppc_uvmem_memslot_delete(struct kvm *kvm,
 		const struct kvm_memory_slot *old);
+int kvmppc_uvmem_disable_ksm(struct mm_struct *mm);
 #else
 static inline int kvmppc_uvmem_init(void)
 {
diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
index fa0e3a22cac0..e39fc28a27a7 100644
--- a/arch/powerpc/kvm/book3s_hv_builtin.c
+++ b/arch/powerpc/kvm/book3s_hv_builtin.c
@@ -14,11 +14,14 @@
 #include <linux/sizes.h>
 #include <linux/cma.h>
 #include <linux/bitops.h>
+#include <linux/ksm.h>
+#include <linux/mmap_lock.h>

 #include <asm/cputable.h>
 #include <asm/interrupt.h>
 #include <asm/kvm_ppc.h>
 #include <asm/kvm_book3s.h>
+#include <asm/kvm_book3s_uvmem.h>
 #include <asm/machdep.h>
 #include <asm/xics.h>
 #include <asm/xive.h>
@@ -627,3 +630,26 @@ void kvmppc_check_need_tlb_flush(struct kvm *kvm, int pcpu)
 	}
 }
 EXPORT_SYMBOL_GPL(kvmppc_check_need_tlb_flush);
+
+#ifdef CONFIG_PPC_UV
+/*
+ * Pages of a secure guest are moved to device private memory one by one with
+ * migrate_vma_*(). This cannot be done to a KSM page, so the VM is not allowed
+ * to have any and neither get new ones. This functions stops KSM for the
+ * whole mm.
+ *
+ * This lives here and not with the rest of the secure guest code, because
+ * that can be built as a module and ksm_disable() is not exported.
+ */
+int kvmppc_uvmem_disable_ksm(struct mm_struct *mm)
+{
+	int ret;
+
+	mmap_write_lock(mm);
+	ret = ksm_disable(mm);
+	mmap_write_unlock(mm);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kvmppc_uvmem_disable_ksm);
+#endif
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 190f64d992d8..fa50c103a9ee 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -47,7 +47,7 @@
  * Locking order
  *
  * 1. kvm->srcu - Protects KVM memslots
- * 2. kvm->mm->mmap_lock - find_vma, migrate_vma_pages and helpers, ksm_madvise
+ * 2. kvm->mm->mmap_lock - find_vma, migrate_vma_pages and helpers, ksm_disable
  * 3. kvm->arch.uvmem_lock - protects read/writes to uvmem slots thus acting
  *			     as sync-points for page-in/out
  */
@@ -89,11 +89,9 @@
 #include <linux/pagemap.h>
 #include <linux/migrate.h>
 #include <linux/kvm_host.h>
-#include <linux/ksm.h>
 #include <linux/of.h>
 #include <linux/memremap.h>
 #include <asm/ultravisor.h>
-#include <asm/mman.h>
 #include <asm/kvm_ppc.h>
 #include <asm/kvm_book3s_uvmem.h>

@@ -388,78 +386,30 @@ static bool kvmppc_next_nontransitioned_gfn(const struct kvm_memory_slot *memslo
 	return ret;
 }

-static int kvmppc_memslot_page_merge(struct kvm *kvm,
-		const struct kvm_memory_slot *memslot, bool merge)
-{
-	unsigned long gfn = memslot->base_gfn;
-	unsigned long end, start = gfn_to_hva(kvm, gfn);
-	vm_flags_t vm_flags;
-	int ret = 0;
-	struct vm_area_struct *vma;
-	int merge_flag = (merge) ? MADV_MERGEABLE : MADV_UNMERGEABLE;
-
-	if (kvm_is_error_hva(start))
-		return H_STATE;
-
-	end = start + (memslot->npages << PAGE_SHIFT);
-
-	mmap_write_lock(kvm->mm);
-	do {
-		vma = find_vma_intersection(kvm->mm, start, end);
-		if (!vma) {
-			ret = H_STATE;
-			break;
-		}
-		vma_start_write(vma);
-		/* Copy vm_flags to avoid partial modifications in ksm_madvise */
-		vm_flags = vma->vm_flags;
-		ret = ksm_madvise(vma, vma->vm_start, vma->vm_end,
-			  merge_flag, &vm_flags);
-		if (ret) {
-			ret = H_STATE;
-			break;
-		}
-		vm_flags_reset(vma, vm_flags);
-		start = vma->vm_end;
-	} while (end > vma->vm_end);
-
-	mmap_write_unlock(kvm->mm);
-	return ret;
-}
-
 static void __kvmppc_uvmem_memslot_delete(struct kvm *kvm,
 		const struct kvm_memory_slot *memslot)
 {
 	uv_unregister_mem_slot(kvm->arch.lpid, memslot->id);
 	kvmppc_uvmem_slot_free(kvm, memslot);
-	kvmppc_memslot_page_merge(kvm, memslot, true);
 }

 static int __kvmppc_uvmem_memslot_create(struct kvm *kvm,
 		const struct kvm_memory_slot *memslot)
 {
-	int ret = H_PARAMETER;
-
-	if (kvmppc_memslot_page_merge(kvm, memslot, false))
-		return ret;
+	int ret;

 	if (kvmppc_uvmem_slot_init(kvm, memslot))
-		goto out1;
+		return H_PARAMETER;

 	ret = uv_register_mem_slot(kvm->arch.lpid,
 				   memslot->base_gfn << PAGE_SHIFT,
 				   memslot->npages * PAGE_SIZE,
 				   0, memslot->id);
 	if (ret < 0) {
-		ret = H_PARAMETER;
-		goto out;
+		kvmppc_uvmem_slot_free(kvm, memslot);
+		return H_PARAMETER;
 	}
 	return 0;
-out:
-	kvmppc_uvmem_slot_free(kvm, memslot);
-out1:
-	kvmppc_memslot_page_merge(kvm, memslot, true);
-	return ret;
 }

 unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
@@ -482,6 +432,9 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
 	if (!kvm->arch.svm_enabled)
 		return H_AUTHORITY;

+	if (kvmppc_uvmem_disable_ksm(kvm->mm))
+		return H_STATE;
+
 	srcu_idx = srcu_read_lock(&kvm->srcu);

 	/* register the memslot */
@@ -1107,8 +1060,12 @@ int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn)

 int kvmppc_uvmem_memslot_create(struct kvm *kvm, const struct kvm_memory_slot *new)
 {
-	int ret = __kvmppc_uvmem_memslot_create(kvm, new);
+	int ret;
+
+	if (kvmppc_uvmem_disable_ksm(kvm->mm))
+		return H_STATE;

+	ret = __kvmppc_uvmem_memslot_create(kvm, new);
 	if (!ret)
 		ret = kvmppc_uv_migrate_mem_slot(kvm, new);

diff --git a/mm/ksm.c b/mm/ksm.c
index d24b129932bf..9d3db831ac87 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -3058,7 +3058,6 @@ int ksm_madvise(struct vm_area_struct *vma, unsigned long start,

 	return 0;
 }
-EXPORT_SYMBOL_GPL(ksm_madvise);

 int __ksm_enter(struct mm_struct *mm)
 {
--
2.39.5