linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory
@ 2022-11-11  1:42 Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 1/6] KVM: x86: Add support for testing " Vishal Annapurve
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-11  1:42 UTC (permalink / raw)
  To: x86, kvm, linux-kernel, linux-kselftest
  Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	marcorr, erdemaktas, pgonda, nikunj, seanjc, diviness, maz,
	dmatlack, axelrasmussen, maciej.szmigiero, mizhang, bgardon,
	ackerleytng, Vishal Annapurve

This series implements selftests targeting the feature floated by Chao
via:
https://lore.kernel.org/linux-mm/20221109041358.GA118963@chaop.bj.intel.com/T/

Below changes aim to test the fd based approach for guest private memory
in context of normal (non-confidential) VMs executing on non-confidential
platforms.

private_mem_test.c file adds selftest to access private memory from the
guest via private/shared accesses and checking if the contents can be
leaked to/accessed by vmm via shared memory view before/after conversions.

Updates in V1 (Compared to RFC v3 patches):
1) Incorporated suggestions from Sean around simplifying KVM changes
2) Addressed comments from Sean
3) Added private mem test with shared memory backed by 2MB hugepages.

RFC v3 series:
https://lore.kernel.org/lkml/20220819174659.2427983-1-vannapurve@google.com/t/

This series has dependency on following patches:
1) V9 series patches from Chao mentioned above.

Github link for the patches posted as part of this series:
https://github.com/vishals4gh/linux/commits/priv_memfd_selftests-v1

Vishal Annapurve (6):
  KVM: x86: Add support for testing private memory
  KVM: Selftests: Add support for private memory
  KVM: selftests: x86: Add IS_ALIGNED/IS_PAGE_ALIGNED helpers
  KVM: selftests: x86: Execute VMs with private memory
  KVM: selftests: Add get_free_huge_2m_pages
  KVM: selftests: x86: Add selftest for private memory

 arch/x86/kvm/mmu/mmu.c                        |   4 +
 arch/x86/kvm/mmu/mmu_internal.h               |   4 +-
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   2 +
 .../selftests/kvm/include/kvm_util_base.h     |  15 +-
 .../testing/selftests/kvm/include/test_util.h |   5 +
 .../kvm/include/x86_64/private_mem.h          |  37 +++
 .../selftests/kvm/include/x86_64/processor.h  |   1 +
 tools/testing/selftests/kvm/lib/kvm_util.c    |  58 ++++-
 tools/testing/selftests/kvm/lib/test_util.c   |  30 +++
 .../selftests/kvm/lib/x86_64/private_mem.c    | 211 ++++++++++++++++++
 .../selftests/kvm/x86_64/private_mem_test.c   | 190 ++++++++++++++++
 virt/kvm/Kconfig                              |   4 +
 virt/kvm/kvm_main.c                           |   2 +-
 14 files changed, 555 insertions(+), 9 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/x86_64/private_mem.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86_64/private_mem.c
 create mode 100644 tools/testing/selftests/kvm/x86_64/private_mem_test.c

-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [V1 PATCH 1/6] KVM: x86: Add support for testing private memory
  2022-11-11  1:42 [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory Vishal Annapurve
@ 2022-11-11  1:42 ` Vishal Annapurve
  2022-11-22 10:07   ` Chao Peng
  2022-11-11  1:42 ` [V1 PATCH 2/6] KVM: Selftests: Add support for " Vishal Annapurve
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-11  1:42 UTC (permalink / raw)
  To: x86, kvm, linux-kernel, linux-kselftest
  Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	marcorr, erdemaktas, pgonda, nikunj, seanjc, diviness, maz,
	dmatlack, axelrasmussen, maciej.szmigiero, mizhang, bgardon,
	ackerleytng, Vishal Annapurve

Introduce HAVE_KVM_PRIVATE_MEM_TESTING config to be able to test fd based
approach to support private memory with non-confidential selftest VMs.
To support this testing few important aspects need to be considered from
the perspective of selftests -
* KVM needs to know whether the access from guest VM is private or shared.
Confidential VMs (SNP/TDX) carry a dedicated bit in gpa that can be used by
KVM to deduce the nature of the access.
Non-confidential VMs don't have mechanism to carry/convey such an
information to KVM. So KVM just relies on what attributes are set by
userspace VMM keeping the userspace VMM in the TCB for the testing
purposes.
* arch_private_mem_supported is updated to allow private memory logic to
work with non-confidential vm selftests.

Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
 arch/x86/kvm/mmu/mmu.c          | 4 ++++
 arch/x86/kvm/mmu/mmu_internal.h | 4 +++-
 virt/kvm/Kconfig                | 4 ++++
 virt/kvm/kvm_main.c             | 2 +-
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 10017a9f26ee..b3118d00b284 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4280,6 +4280,10 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 
 	fault->gfn = fault->addr >> PAGE_SHIFT;
 	fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
+#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
+	fault->is_private = kvm_slot_can_be_private(fault->slot) &&
+			kvm_mem_is_private(vcpu->kvm, fault->gfn);
+#endif
 
 	if (page_fault_handle_page_track(vcpu, fault))
 		return RET_PF_EMULATE;
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 5cdff5ca546c..2e759f39c2c5 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -188,7 +188,6 @@ struct kvm_page_fault {
 
 	/* Derived from mmu and global state.  */
 	const bool is_tdp;
-	const bool is_private;
 	const bool nx_huge_page_workaround_enabled;
 
 	/*
@@ -221,6 +220,9 @@ struct kvm_page_fault {
 	/* The memslot containing gfn. May be NULL. */
 	struct kvm_memory_slot *slot;
 
+	/* Derived from encryption bits of the faulting GPA for CVMs. */
+	bool is_private;
+
 	/* Outputs of kvm_faultin_pfn.  */
 	kvm_pfn_t pfn;
 	hva_t hva;
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 69ca59e82149..300876afb0ca 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -93,3 +93,7 @@ config HAVE_KVM_RESTRICTED_MEM
 config KVM_GENERIC_PRIVATE_MEM
        bool
        depends on HAVE_KVM_RESTRICTED_MEM
+
+config HAVE_KVM_PRIVATE_MEM_TESTING
+       bool
+       depends on KVM_GENERIC_PRIVATE_MEM
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index dae6a2c196ad..54e57b7f1c15 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1750,7 +1750,7 @@ static void kvm_replace_memslot(struct kvm *kvm,
 
 bool __weak kvm_arch_has_private_mem(struct kvm *kvm)
 {
-	return false;
+	return IS_ENABLED(CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING);
 }
 
 static int check_memory_region_flags(struct kvm *kvm,
-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [V1 PATCH 2/6] KVM: Selftests: Add support for private memory
  2022-11-11  1:42 [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 1/6] KVM: x86: Add support for testing " Vishal Annapurve
@ 2022-11-11  1:42 ` Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 3/6] KVM: selftests: x86: Add IS_ALIGNED/IS_PAGE_ALIGNED helpers Vishal Annapurve
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-11  1:42 UTC (permalink / raw)
  To: x86, kvm, linux-kernel, linux-kselftest
  Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	marcorr, erdemaktas, pgonda, nikunj, seanjc, diviness, maz,
	dmatlack, axelrasmussen, maciej.szmigiero, mizhang, bgardon,
	ackerleytng, Vishal Annapurve

Add support for registering private memory with kvm using
KVM_SET_USER_MEMORY_REGION ioctl.

Helper function to query extended userspace mem region is introduced to
allow memory conversion.

vm_mem_backing_src types is extended to contain additional guest memory
source types to cover the cases where guest memory can be backed by both
anonymous memory and restricted memfd.

Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
 .../selftests/kvm/include/kvm_util_base.h     | 12 +++-
 .../testing/selftests/kvm/include/test_util.h |  4 ++
 tools/testing/selftests/kvm/lib/kvm_util.c    | 58 +++++++++++++++++--
 tools/testing/selftests/kvm/lib/test_util.c   | 12 ++++
 4 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index e42a09cd24a0..5e30f5b461bf 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -30,7 +30,10 @@ typedef uint64_t vm_paddr_t; /* Virtual Machine (Guest) physical address */
 typedef uint64_t vm_vaddr_t; /* Virtual Machine (Guest) virtual address */
 
 struct userspace_mem_region {
-	struct kvm_userspace_memory_region region;
+	union {
+		struct kvm_userspace_memory_region region;
+		struct kvm_userspace_memory_region_ext region_ext;
+	};
 	struct sparsebit *unused_phy_pages;
 	int fd;
 	off_t offset;
@@ -194,7 +197,7 @@ static inline bool kvm_has_cap(long cap)
 
 #define kvm_do_ioctl(fd, cmd, arg)						\
 ({										\
-	static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == _IOC_SIZE(cmd), "");	\
+	static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) >= _IOC_SIZE(cmd), "");	\
 	ioctl(fd, cmd, arg);							\
 })
 
@@ -382,6 +385,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t flags);
 void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa);
 void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot);
+
 struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id);
 vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
 vm_vaddr_t vm_vaddr_alloc_pages(struct kvm_vm *vm, int nr_pages);
@@ -708,6 +712,10 @@ struct kvm_userspace_memory_region *
 kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start,
 				 uint64_t end);
 
+struct kvm_userspace_memory_region_ext *
+kvm_userspace_memory_region_ext_find(struct kvm_vm *vm, uint64_t start,
+				 uint64_t end);
+
 #define sync_global_to_guest(vm, g) ({				\
 	typeof(g) *_p = addr_gva2hva(vm, (vm_vaddr_t)&(g));	\
 	memcpy(_p, &(g), sizeof(g));				\
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index befc754ce9b3..140a61f68fe5 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -96,6 +96,8 @@ enum vm_mem_backing_src_type {
 	VM_MEM_SRC_ANONYMOUS_HUGETLB_16GB,
 	VM_MEM_SRC_SHMEM,
 	VM_MEM_SRC_SHARED_HUGETLB,
+	VM_MEM_SRC_ANONYMOUS_AND_RESTRICTED_MEMFD,
+	VM_MEM_SRC_ANON_HTLB2M_AND_RESTRICTED_MEMFD,
 	NUM_SRC_TYPES,
 };
 
@@ -103,7 +105,9 @@ enum vm_mem_backing_src_type {
 
 struct vm_mem_backing_src_alias {
 	const char *name;
+	/* Flags applicable for normal host accessible guest memory */
 	uint32_t flag;
+	uint32_t need_restricted_memfd;
 };
 
 #define MIN_RUN_DELAY_NS	200000UL
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index f1cb1627161f..5990250ec40b 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -31,6 +31,11 @@ int open_path_or_exit(const char *path, int flags)
 	return fd;
 }
 
+static int memfd_restricted(unsigned int flags)
+{
+	return syscall(__NR_memfd_restricted, flags);
+}
+
 /*
  * Open KVM_DEV_PATH if available, otherwise exit the entire program.
  *
@@ -519,6 +524,35 @@ __weak void vcpu_arch_free(struct kvm_vcpu *vcpu)
 
 }
 
+/*
+ * KVM Userspace Memory Region Ext Find
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   start - Starting VM physical address
+ *   end - Ending VM physical address, inclusive.
+ *
+ * Output Args: None
+ *
+ * Return:
+ *   Pointer to overlapping ext region, NULL if no such region.
+ *
+ * Public interface to userspace_mem_region_find. Allows tests to look up
+ * the memslot datastructure for a given range of guest physical memory.
+ */
+struct kvm_userspace_memory_region_ext *
+kvm_userspace_memory_region_ext_find(struct kvm_vm *vm, uint64_t start,
+				 uint64_t end)
+{
+	struct userspace_mem_region *region;
+
+	region = userspace_mem_region_find(vm, start, end);
+	if (!region)
+		return NULL;
+
+	return &region->region_ext;
+}
+
 /*
  * VM VCPU Remove
  *
@@ -818,6 +852,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 	struct userspace_mem_region *region;
 	size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
 	size_t alignment;
+	int restricted_memfd = -1;
 
 	TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
 		"Number of guest pages is not compatible with the host. "
@@ -915,14 +950,24 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 
 	/* As needed perform madvise */
 	if ((src_type == VM_MEM_SRC_ANONYMOUS ||
-	     src_type == VM_MEM_SRC_ANONYMOUS_THP) && thp_configured()) {
+		src_type == VM_MEM_SRC_ANONYMOUS_THP ||
+		src_type == VM_MEM_SRC_ANONYMOUS_AND_RESTRICTED_MEMFD) &&
+		thp_configured()) {
 		ret = madvise(region->host_mem, npages * vm->page_size,
-			      src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
+			(src_type == VM_MEM_SRC_ANONYMOUS_THP) ?
+				MADV_HUGEPAGE : MADV_NOHUGEPAGE);
 		TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %s",
 			    region->host_mem, npages * vm->page_size,
 			    vm_mem_backing_src_alias(src_type)->name);
 	}
 
+	if (vm_mem_backing_src_alias(src_type)->need_restricted_memfd) {
+		restricted_memfd = memfd_restricted(0);
+		TEST_ASSERT(restricted_memfd != -1,
+			"Failed to create restricted memfd");
+		flags |= KVM_MEM_PRIVATE;
+	}
+
 	region->unused_phy_pages = sparsebit_alloc();
 	sparsebit_set_num(region->unused_phy_pages,
 		guest_paddr >> vm->page_shift, npages);
@@ -931,13 +976,16 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 	region->region.guest_phys_addr = guest_paddr;
 	region->region.memory_size = npages * vm->page_size;
 	region->region.userspace_addr = (uintptr_t) region->host_mem;
-	ret = __vm_ioctl(vm, KVM_SET_USER_MEMORY_REGION, &region->region);
+	region->region_ext.restricted_fd = restricted_memfd;
+	region->region_ext.restricted_offset = 0;
+	ret = ioctl(vm->fd, KVM_SET_USER_MEMORY_REGION, &region->region_ext);
 	TEST_ASSERT(ret == 0, "KVM_SET_USER_MEMORY_REGION IOCTL failed,\n"
 		"  rc: %i errno: %i\n"
 		"  slot: %u flags: 0x%x\n"
-		"  guest_phys_addr: 0x%lx size: 0x%lx",
+		"  guest_phys_addr: 0x%lx size: 0x%lx restricted fd: %d\n",
 		ret, errno, slot, flags,
-		guest_paddr, (uint64_t) region->region.memory_size);
+		guest_paddr, (uint64_t) region->region.memory_size,
+		restricted_memfd);
 
 	/* Add to quick lookup data structures */
 	vm_userspace_mem_region_gpa_insert(&vm->regions.gpa_tree, region);
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 6d23878bbfe1..ebbac8246016 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -254,6 +254,16 @@ const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i)
 			 */
 			.flag = MAP_SHARED,
 		},
+		[VM_MEM_SRC_ANONYMOUS_AND_RESTRICTED_MEMFD] = {
+			.name = "anonymous_and_restricted_memfd",
+			.flag = ANON_FLAGS,
+			.need_restricted_memfd = 1,
+		},
+		[VM_MEM_SRC_ANON_HTLB2M_AND_RESTRICTED_MEMFD] = {
+			.name = "anonymous_hugetlb_2mb_and_restricted_memfd",
+			.flag = ANON_HUGE_FLAGS | MAP_HUGE_2MB,
+			.need_restricted_memfd = 1,
+		},
 	};
 	_Static_assert(ARRAY_SIZE(aliases) == NUM_SRC_TYPES,
 		       "Missing new backing src types?");
@@ -272,11 +282,13 @@ size_t get_backing_src_pagesz(uint32_t i)
 	switch (i) {
 	case VM_MEM_SRC_ANONYMOUS:
 	case VM_MEM_SRC_SHMEM:
+	case VM_MEM_SRC_ANONYMOUS_AND_RESTRICTED_MEMFD:
 		return getpagesize();
 	case VM_MEM_SRC_ANONYMOUS_THP:
 		return get_trans_hugepagesz();
 	case VM_MEM_SRC_ANONYMOUS_HUGETLB:
 	case VM_MEM_SRC_SHARED_HUGETLB:
+	case VM_MEM_SRC_ANON_HTLB2M_AND_RESTRICTED_MEMFD:
 		return get_def_hugetlb_pagesz();
 	default:
 		return MAP_HUGE_PAGE_SIZE(flag);
-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [V1 PATCH 3/6] KVM: selftests: x86: Add IS_ALIGNED/IS_PAGE_ALIGNED helpers
  2022-11-11  1:42 [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 1/6] KVM: x86: Add support for testing " Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 2/6] KVM: Selftests: Add support for " Vishal Annapurve
@ 2022-11-11  1:42 ` Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory Vishal Annapurve
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-11  1:42 UTC (permalink / raw)
  To: x86, kvm, linux-kernel, linux-kselftest
  Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	marcorr, erdemaktas, pgonda, nikunj, seanjc, diviness, maz,
	dmatlack, axelrasmussen, maciej.szmigiero, mizhang, bgardon,
	ackerleytng, Vishal Annapurve

Add IS_ALIGNED/IS_PAGE_ALIGNED helpers for selftests.

Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
 tools/testing/selftests/kvm/include/kvm_util_base.h    | 3 +++
 tools/testing/selftests/kvm/include/x86_64/processor.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 5e30f5b461bf..4eecc847d9f9 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -168,6 +168,9 @@ extern enum vm_guest_mode vm_mode_default;
 #define MIN_PAGE_SIZE		(1U << MIN_PAGE_SHIFT)
 #define PTES_PER_MIN_PAGE	ptes_per_page(MIN_PAGE_SIZE)
 
+/* @a is a power of 2 value */
+#define IS_ALIGNED(x, a)		(((x) & ((typeof(x))(a) - 1)) == 0)
+
 struct vm_guest_mode_params {
 	unsigned int pa_bits;
 	unsigned int va_bits;
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index e8ca0d8a6a7e..62677f266583 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -175,6 +175,7 @@ struct kvm_x86_cpu_feature {
 #define PAGE_SHIFT		12
 #define PAGE_SIZE		(1ULL << PAGE_SHIFT)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
+#define IS_PAGE_ALIGNED(x)	IS_ALIGNED(x, PAGE_SIZE)
 
 #define PHYSICAL_PAGE_MASK      GENMASK_ULL(51, 12)
 #define PTE_GET_PFN(pte)        (((pte) & PHYSICAL_PAGE_MASK) >> PAGE_SHIFT)
-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory
  2022-11-11  1:42 [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory Vishal Annapurve
                   ` (2 preceding siblings ...)
  2022-11-11  1:42 ` [V1 PATCH 3/6] KVM: selftests: x86: Add IS_ALIGNED/IS_PAGE_ALIGNED helpers Vishal Annapurve
@ 2022-11-11  1:42 ` Vishal Annapurve
  2022-11-14 19:37   ` Peter Gonda
  2022-11-11  1:42 ` [V1 PATCH 5/6] KVM: selftests: Add get_free_huge_2m_pages Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 6/6] KVM: selftests: x86: Add selftest for private memory Vishal Annapurve
  5 siblings, 1 reply; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-11  1:42 UTC (permalink / raw)
  To: x86, kvm, linux-kernel, linux-kselftest
  Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	marcorr, erdemaktas, pgonda, nikunj, seanjc, diviness, maz,
	dmatlack, axelrasmussen, maciej.szmigiero, mizhang, bgardon,
	ackerleytng, Vishal Annapurve

Introduce a set of APIs to execute VM with private memslots.

Host userspace APIs for:
1) Setting up and executing VM having private memslots
2) Backing/unbacking guest private memory

Guest APIs for:
1) Changing memory mapping type

Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../kvm/include/x86_64/private_mem.h          |  37 +++
 .../selftests/kvm/lib/x86_64/private_mem.c    | 211 ++++++++++++++++++
 3 files changed, 249 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/x86_64/private_mem.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86_64/private_mem.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 0172eb6cb6ee..57385ad58527 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -53,6 +53,7 @@ LIBKVM_STRING += lib/string_override.c
 LIBKVM_x86_64 += lib/x86_64/apic.c
 LIBKVM_x86_64 += lib/x86_64/handlers.S
 LIBKVM_x86_64 += lib/x86_64/perf_test_util.c
+LIBKVM_x86_64 += lib/x86_64/private_mem.c
 LIBKVM_x86_64 += lib/x86_64/processor.c
 LIBKVM_x86_64 += lib/x86_64/svm.c
 LIBKVM_x86_64 += lib/x86_64/ucall.c
diff --git a/tools/testing/selftests/kvm/include/x86_64/private_mem.h b/tools/testing/selftests/kvm/include/x86_64/private_mem.h
new file mode 100644
index 000000000000..e556ded971fd
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86_64/private_mem.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2022, Google LLC.
+ */
+
+#ifndef SELFTEST_KVM_PRIVATE_MEM_H
+#define SELFTEST_KVM_PRIVATE_MEM_H
+
+#include <stdint.h>
+#include <kvm_util.h>
+
+void kvm_hypercall_map_shared(uint64_t gpa, uint64_t size);
+void kvm_hypercall_map_private(uint64_t gpa, uint64_t size);
+
+void vm_unback_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size);
+
+void vm_allocate_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size);
+
+typedef void (*guest_code_fn)(void);
+typedef void (*io_exit_handler)(struct kvm_vm *vm, uint32_t uc_arg1);
+
+struct test_setup_info {
+	uint64_t test_area_gpa;
+	uint64_t test_area_size;
+	uint32_t test_area_slot;
+};
+
+struct vm_setup_info {
+	enum vm_mem_backing_src_type test_mem_src;
+	struct test_setup_info test_info;
+	guest_code_fn guest_fn;
+	io_exit_handler ioexit_cb;
+};
+
+void execute_vm_with_private_test_mem(struct vm_setup_info *info);
+
+#endif /* SELFTEST_KVM_PRIVATE_MEM_H */
diff --git a/tools/testing/selftests/kvm/lib/x86_64/private_mem.c b/tools/testing/selftests/kvm/lib/x86_64/private_mem.c
new file mode 100644
index 000000000000..3076cae81804
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86_64/private_mem.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * tools/testing/selftests/kvm/lib/kvm_util.c
+ *
+ * Copyright (C) 2022, Google LLC.
+ */
+#define _GNU_SOURCE /* for program_invocation_name */
+#include <fcntl.h>
+#include <limits.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+#include <linux/kvm_para.h>
+
+#include <test_util.h>
+#include <kvm_util.h>
+#include <private_mem.h>
+#include <processor.h>
+
+static inline uint64_t __kvm_hypercall_map_gpa_range(uint64_t gpa, uint64_t size,
+	uint64_t flags)
+{
+	return kvm_hypercall(KVM_HC_MAP_GPA_RANGE, gpa, size >> PAGE_SHIFT, flags, 0);
+}
+
+static inline void kvm_hypercall_map_gpa_range(uint64_t gpa, uint64_t size,
+	uint64_t flags)
+{
+	uint64_t ret;
+
+	GUEST_ASSERT_2(IS_PAGE_ALIGNED(gpa) && IS_PAGE_ALIGNED(size), gpa, size);
+
+	ret = __kvm_hypercall_map_gpa_range(gpa, size, flags);
+	GUEST_ASSERT_1(!ret, ret);
+}
+
+void kvm_hypercall_map_shared(uint64_t gpa, uint64_t size)
+{
+	kvm_hypercall_map_gpa_range(gpa, size, KVM_MAP_GPA_RANGE_DECRYPTED);
+}
+
+void kvm_hypercall_map_private(uint64_t gpa, uint64_t size)
+{
+	kvm_hypercall_map_gpa_range(gpa, size, KVM_MAP_GPA_RANGE_ENCRYPTED);
+}
+
+static void vm_update_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size,
+	bool unback_mem)
+{
+	int restricted_fd;
+	uint64_t restricted_fd_offset, guest_phys_base, fd_offset;
+	struct kvm_enc_region enc_region;
+	struct kvm_userspace_memory_region_ext *region_ext;
+	struct kvm_userspace_memory_region *region;
+	int fallocate_mode = 0;
+	int ret;
+
+	region_ext = kvm_userspace_memory_region_ext_find(vm, gpa, gpa + size);
+	TEST_ASSERT(region_ext != NULL, "Region not found");
+	region = &region_ext->region;
+	TEST_ASSERT(region->flags & KVM_MEM_PRIVATE,
+		"Can not update private memfd for non-private memslot\n");
+	restricted_fd = region_ext->restricted_fd;
+	restricted_fd_offset = region_ext->restricted_offset;
+	guest_phys_base = region->guest_phys_addr;
+	fd_offset = restricted_fd_offset + (gpa - guest_phys_base);
+
+	if (unback_mem)
+		fallocate_mode = (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE);
+
+	printf("restricted_fd %d fallocate_mode 0x%x for offset 0x%lx size 0x%lx\n",
+		restricted_fd, fallocate_mode, fd_offset, size);
+	ret = fallocate(restricted_fd, fallocate_mode, fd_offset, size);
+	TEST_ASSERT(ret == 0, "fallocate failed\n");
+	enc_region.addr = gpa;
+	enc_region.size = size;
+	if (unback_mem) {
+		printf("undoing encryption for gpa 0x%lx size 0x%lx\n", gpa, size);
+		vm_ioctl(vm, KVM_MEMORY_ENCRYPT_UNREG_REGION, &enc_region);
+	} else {
+		printf("doing encryption for gpa 0x%lx size 0x%lx\n", gpa, size);
+		vm_ioctl(vm, KVM_MEMORY_ENCRYPT_REG_REGION, &enc_region);
+	}
+}
+
+void vm_unback_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size)
+{
+	vm_update_private_mem(vm, gpa, size, true);
+}
+
+void vm_allocate_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size)
+{
+	vm_update_private_mem(vm, gpa, size, false);
+}
+
+static void handle_vm_exit_map_gpa_hypercall(struct kvm_vm *vm,
+				struct kvm_vcpu *vcpu)
+{
+	uint64_t gpa, npages, attrs, size;
+
+	TEST_ASSERT(vcpu->run->hypercall.nr == KVM_HC_MAP_GPA_RANGE,
+		"Unhandled Hypercall %lld\n", vcpu->run->hypercall.nr);
+	gpa = vcpu->run->hypercall.args[0];
+	npages = vcpu->run->hypercall.args[1];
+	size = npages << MIN_PAGE_SHIFT;
+	attrs = vcpu->run->hypercall.args[2];
+	pr_info("Explicit conversion off 0x%lx size 0x%lx to %s\n", gpa, size,
+		(attrs & KVM_MAP_GPA_RANGE_ENCRYPTED) ? "private" : "shared");
+
+	if (attrs & KVM_MAP_GPA_RANGE_ENCRYPTED)
+		vm_allocate_private_mem(vm, gpa, size);
+	else
+		vm_unback_private_mem(vm, gpa, size);
+
+	vcpu->run->hypercall.ret = 0;
+}
+
+static void vcpu_work(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
+	struct vm_setup_info *info)
+{
+	struct ucall uc;
+	uint64_t cmd;
+
+	/*
+	 * Loop until the guest is done.
+	 */
+
+	while (true) {
+		vcpu_run(vcpu);
+
+		if (vcpu->run->exit_reason == KVM_EXIT_IO) {
+			cmd = get_ucall(vcpu, &uc);
+			if (cmd != UCALL_SYNC)
+				break;
+
+			TEST_ASSERT(info->ioexit_cb, "ioexit cb not present");
+			info->ioexit_cb(vm, uc.args[1]);
+			continue;
+		}
+
+		if (vcpu->run->exit_reason == KVM_EXIT_HYPERCALL) {
+			handle_vm_exit_map_gpa_hypercall(vm, vcpu);
+			continue;
+		}
+
+		TEST_FAIL("Unhandled VCPU exit reason %d\n",
+			vcpu->run->exit_reason);
+		break;
+	}
+
+	if (vcpu->run->exit_reason == KVM_EXIT_IO && cmd == UCALL_ABORT)
+		TEST_FAIL("%s at %s:%ld, val = %lu", (const char *)uc.args[0],
+			  __FILE__, uc.args[1], uc.args[2]);
+}
+
+/*
+ * Execute guest vm with private memory memslots.
+ *
+ * Input Args:
+ *   info - pointer to a structure containing information about setting up a VM
+ *     with private memslots
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Function called by host userspace logic in selftests to execute guest vm
+ * logic. It will install test_mem_slot : containing the region of memory that
+ * would be used to test private/shared memory accesses to a memory backed by
+ * private memslots
+ */
+void execute_vm_with_private_test_mem(struct vm_setup_info *info)
+{
+	struct kvm_vm *vm;
+	struct kvm_enable_cap cap;
+	struct kvm_vcpu *vcpu;
+	uint64_t test_area_gpa, test_area_size;
+	struct test_setup_info *test_info = &info->test_info;
+
+	TEST_ASSERT(info->guest_fn, "guest_fn not present");
+	vm = vm_create_with_one_vcpu(&vcpu, info->guest_fn);
+
+	vm_check_cap(vm, KVM_CAP_EXIT_HYPERCALL);
+	cap.cap = KVM_CAP_EXIT_HYPERCALL;
+	cap.flags = 0;
+	cap.args[0] = (1 << KVM_HC_MAP_GPA_RANGE);
+	vm_ioctl(vm, KVM_ENABLE_CAP, &cap);
+
+	TEST_ASSERT(test_info->test_area_size, "Test mem size not present");
+
+	test_area_size = test_info->test_area_size;
+	test_area_gpa = test_info->test_area_gpa;
+	vm_userspace_mem_region_add(vm, info->test_mem_src, test_area_gpa,
+		test_info->test_area_slot, test_area_size / vm->page_size,
+		KVM_MEM_PRIVATE);
+	vm_allocate_private_mem(vm, test_area_gpa, test_area_size);
+
+	pr_info("Mapping test memory pages 0x%zx page_size 0x%x\n",
+		test_area_size/vm->page_size, vm->page_size);
+	virt_map(vm, test_area_gpa, test_area_gpa, test_area_size/vm->page_size);
+
+	vcpu_work(vm, vcpu, info);
+
+	kvm_vm_free(vm);
+}
-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [V1 PATCH 5/6] KVM: selftests: Add get_free_huge_2m_pages
  2022-11-11  1:42 [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory Vishal Annapurve
                   ` (3 preceding siblings ...)
  2022-11-11  1:42 ` [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory Vishal Annapurve
@ 2022-11-11  1:42 ` Vishal Annapurve
  2022-11-11  1:42 ` [V1 PATCH 6/6] KVM: selftests: x86: Add selftest for private memory Vishal Annapurve
  5 siblings, 0 replies; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-11  1:42 UTC (permalink / raw)
  To: x86, kvm, linux-kernel, linux-kselftest
  Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	marcorr, erdemaktas, pgonda, nikunj, seanjc, diviness, maz,
	dmatlack, axelrasmussen, maciej.szmigiero, mizhang, bgardon,
	ackerleytng, Vishal Annapurve

Add an API to query free 2MB hugepages in the system.

Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
 .../testing/selftests/kvm/include/test_util.h  |  1 +
 tools/testing/selftests/kvm/lib/test_util.c    | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index 140a61f68fe5..f4df49c8b5ba 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -115,6 +115,7 @@ struct vm_mem_backing_src_alias {
 bool thp_configured(void);
 size_t get_trans_hugepagesz(void);
 size_t get_def_hugetlb_pagesz(void);
+size_t get_free_huge_2mb_pages(void);
 const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i);
 size_t get_backing_src_pagesz(uint32_t i);
 bool is_backing_src_hugetlb(uint32_t i);
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index ebbac8246016..4f28ae73f150 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -145,6 +145,24 @@ size_t get_trans_hugepagesz(void)
 	return size;
 }
 
+size_t get_free_huge_2mb_pages(void)
+{
+	size_t free_pages;
+	FILE *f;
+	int ret;
+
+	f = fopen("/sys/kernel/mm/hugepages/hugepages-2048kB/free_hugepages", "r");
+	TEST_ASSERT(f != NULL, "Error in opening hugepages-2048kB/free_hugepages");
+
+	do {
+		ret = fscanf(f, "%ld", &free_pages);
+	} while (errno == EINTR);
+	TEST_ASSERT(ret < 1, "Error reading hugepages-2048kB/free_hugepages");
+	fclose(f);
+
+	return free_pages;
+}
+
 size_t get_def_hugetlb_pagesz(void)
 {
 	char buf[64];
-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [V1 PATCH 6/6] KVM: selftests: x86: Add selftest for private memory
  2022-11-11  1:42 [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory Vishal Annapurve
                   ` (4 preceding siblings ...)
  2022-11-11  1:42 ` [V1 PATCH 5/6] KVM: selftests: Add get_free_huge_2m_pages Vishal Annapurve
@ 2022-11-11  1:42 ` Vishal Annapurve
  5 siblings, 0 replies; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-11  1:42 UTC (permalink / raw)
  To: x86, kvm, linux-kernel, linux-kselftest
  Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	marcorr, erdemaktas, pgonda, nikunj, seanjc, diviness, maz,
	dmatlack, axelrasmussen, maciej.szmigiero, mizhang, bgardon,
	ackerleytng, Vishal Annapurve

Add a selftest to exercise implicit/explicit conversion functionality
within KVM and verify:
1) Shared memory is visible to host userspace after conversion
2) Private memory is not visible to host userspace before/after conversion
3) Host userspace and guest can communicate over shared memory

Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/x86_64/private_mem_test.c   | 190 ++++++++++++++++++
 3 files changed, 192 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86_64/private_mem_test.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 2f0d705db9db..77b79b740424 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -32,6 +32,7 @@
 /x86_64/nested_exceptions_test
 /x86_64/nx_huge_pages_test
 /x86_64/platform_info_test
+/x86_64/private_mem_test
 /x86_64/pmu_event_filter_test
 /x86_64/set_boot_cpu_id
 /x86_64/set_sregs_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 57385ad58527..d1fa27a58f8f 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -95,6 +95,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/monitor_mwait_test
 TEST_GEN_PROGS_x86_64 += x86_64/nested_exceptions_test
 TEST_GEN_PROGS_x86_64 += x86_64/platform_info_test
 TEST_GEN_PROGS_x86_64 += x86_64/pmu_event_filter_test
+TEST_GEN_PROGS_x86_64 += x86_64/private_mem_test
 TEST_GEN_PROGS_x86_64 += x86_64/set_boot_cpu_id
 TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test
 TEST_GEN_PROGS_x86_64 += x86_64/smm_test
diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_test.c
new file mode 100644
index 000000000000..a93f9e5d15a8
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/private_mem_test.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * tools/testing/selftests/kvm/lib/kvm_util.c
+ *
+ * Copyright (C) 2022, Google LLC.
+ */
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include <fcntl.h>
+#include <limits.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+#include <linux/kvm_para.h>
+#include <linux/memfd.h>
+
+#include <test_util.h>
+#include <kvm_util.h>
+#include <private_mem.h>
+#include <processor.h>
+
+#define TEST_AREA_SLOT		10
+#define TEST_AREA_GPA		0xC0000000
+#define TEST_AREA_SIZE		(2 * 1024 * 1024)
+#define GUEST_TEST_MEM_OFFSET	(1 * 1024 * 1024)
+#define GUEST_TEST_MEM_SIZE	(10 * 4096)
+
+#define VM_STAGE_PROCESSED(x)	pr_info("Processed stage %s\n", #x)
+
+#define TEST_MEM_DATA_PATTERN1	0x66
+#define TEST_MEM_DATA_PATTERN2	0x99
+#define TEST_MEM_DATA_PATTERN3	0x33
+#define TEST_MEM_DATA_PATTERN4	0xaa
+#define TEST_MEM_DATA_PATTERN5	0x12
+
+static bool verify_mem_contents(void *mem, uint32_t size, uint8_t pattern)
+{
+	uint8_t *buf = (uint8_t *)mem;
+
+	for (uint32_t i = 0; i < size; i++) {
+		if (buf[i] != pattern)
+			return false;
+	}
+
+	return true;
+}
+
+static void populate_test_area(void *test_area_base, uint64_t pattern)
+{
+	memset(test_area_base, pattern, TEST_AREA_SIZE);
+}
+
+static void populate_guest_test_mem(void *guest_test_mem, uint64_t pattern)
+{
+	memset(guest_test_mem, pattern, GUEST_TEST_MEM_SIZE);
+}
+
+static bool verify_test_area(void *test_area_base, uint64_t area_pattern,
+	uint64_t guest_pattern)
+{
+	void *guest_test_mem = test_area_base + GUEST_TEST_MEM_OFFSET;
+	void *test_area2_base = guest_test_mem + GUEST_TEST_MEM_SIZE;
+	uint64_t test_area2_size = (TEST_AREA_SIZE - (GUEST_TEST_MEM_OFFSET +
+			GUEST_TEST_MEM_SIZE));
+
+	return (verify_mem_contents(test_area_base, GUEST_TEST_MEM_OFFSET, area_pattern) &&
+		verify_mem_contents(guest_test_mem, GUEST_TEST_MEM_SIZE, guest_pattern) &&
+		verify_mem_contents(test_area2_base, test_area2_size, area_pattern));
+}
+
+#define GUEST_STARTED			0
+#define GUEST_PRIVATE_MEM_POPULATED	1
+#define GUEST_SHARED_MEM_POPULATED	2
+#define GUEST_PRIVATE_MEM_POPULATED2	3
+
+/*
+ * Run memory conversion tests with explicit conversion:
+ * Execute KVM hypercall to map/unmap gpa range which will cause userspace exit
+ * to back/unback private memory. Subsequent accesses by guest to the gpa range
+ * will not cause exit to userspace.
+ *
+ * Test memory conversion scenarios with following steps:
+ * 1) Access private memory using private access and verify that memory contents
+ *   are not visible to userspace.
+ * 2) Convert memory to shared using explicit conversions and ensure that
+ *   userspace is able to access the shared regions.
+ * 3) Convert memory back to private using explicit conversions and ensure that
+ *   userspace is again not able to access converted private regions.
+ */
+static void guest_conv_test_fn(void)
+{
+	void *test_area_base = (void *)TEST_AREA_GPA;
+	void *guest_test_mem = (void *)(TEST_AREA_GPA + GUEST_TEST_MEM_OFFSET);
+	uint64_t guest_test_size = GUEST_TEST_MEM_SIZE;
+
+	GUEST_SYNC(GUEST_STARTED);
+
+	populate_test_area(test_area_base, TEST_MEM_DATA_PATTERN1);
+	GUEST_SYNC(GUEST_PRIVATE_MEM_POPULATED);
+	GUEST_ASSERT(verify_test_area(test_area_base, TEST_MEM_DATA_PATTERN1,
+		TEST_MEM_DATA_PATTERN1));
+
+	kvm_hypercall_map_shared((uint64_t)guest_test_mem, guest_test_size);
+
+	populate_guest_test_mem(guest_test_mem, TEST_MEM_DATA_PATTERN2);
+
+	GUEST_SYNC(GUEST_SHARED_MEM_POPULATED);
+	GUEST_ASSERT(verify_test_area(test_area_base, TEST_MEM_DATA_PATTERN1,
+		TEST_MEM_DATA_PATTERN5));
+
+	kvm_hypercall_map_private((uint64_t)guest_test_mem, guest_test_size);
+
+	populate_guest_test_mem(guest_test_mem, TEST_MEM_DATA_PATTERN3);
+	GUEST_SYNC(GUEST_PRIVATE_MEM_POPULATED2);
+
+	GUEST_ASSERT(verify_test_area(test_area_base, TEST_MEM_DATA_PATTERN1,
+		TEST_MEM_DATA_PATTERN3));
+	GUEST_DONE();
+}
+
+static void conv_test_ioexit_fn(struct kvm_vm *vm, uint32_t uc_arg1)
+{
+	void *test_area_hva = addr_gpa2hva(vm, TEST_AREA_GPA);
+	void *guest_test_mem_hva = (test_area_hva + GUEST_TEST_MEM_OFFSET);
+	uint64_t guest_mem_gpa = (TEST_AREA_GPA + GUEST_TEST_MEM_OFFSET);
+	uint64_t guest_test_size = GUEST_TEST_MEM_SIZE;
+
+	switch (uc_arg1) {
+	case GUEST_STARTED:
+		populate_test_area(test_area_hva, TEST_MEM_DATA_PATTERN4);
+		VM_STAGE_PROCESSED(GUEST_STARTED);
+		break;
+	case GUEST_PRIVATE_MEM_POPULATED:
+		TEST_ASSERT(verify_test_area(test_area_hva, TEST_MEM_DATA_PATTERN4,
+				TEST_MEM_DATA_PATTERN4), "failed");
+		VM_STAGE_PROCESSED(GUEST_PRIVATE_MEM_POPULATED);
+		break;
+	case GUEST_SHARED_MEM_POPULATED:
+		TEST_ASSERT(verify_test_area(test_area_hva, TEST_MEM_DATA_PATTERN4,
+				TEST_MEM_DATA_PATTERN2), "failed");
+		populate_guest_test_mem(guest_test_mem_hva, TEST_MEM_DATA_PATTERN5);
+		VM_STAGE_PROCESSED(GUEST_SHARED_MEM_POPULATED);
+		break;
+	case GUEST_PRIVATE_MEM_POPULATED2:
+		TEST_ASSERT(verify_test_area(test_area_hva, TEST_MEM_DATA_PATTERN4,
+				TEST_MEM_DATA_PATTERN5), "failed");
+		VM_STAGE_PROCESSED(GUEST_PRIVATE_MEM_POPULATED2);
+		break;
+	default:
+		TEST_FAIL("Unknown stage %d\n", uc_arg1);
+		break;
+	}
+}
+
+static void execute_memory_conversion_test(enum vm_mem_backing_src_type test_mem_src)
+{
+	struct vm_setup_info info;
+	struct test_setup_info *test_info = &info.test_info;
+
+	info.test_mem_src = test_mem_src;
+	test_info->test_area_gpa = TEST_AREA_GPA;
+	test_info->test_area_size = TEST_AREA_SIZE;
+	test_info->test_area_slot = TEST_AREA_SLOT;
+	info.ioexit_cb = conv_test_ioexit_fn;
+
+	info.guest_fn = guest_conv_test_fn;
+	execute_vm_with_private_test_mem(&info);
+}
+
+int main(int argc, char *argv[])
+{
+	/* Tell stdout not to buffer its content */
+	setbuf(stdout, NULL);
+
+	execute_memory_conversion_test(VM_MEM_SRC_ANONYMOUS_AND_RESTRICTED_MEMFD);
+
+	/* Needs 2MB Hugepages */
+	if (get_free_huge_2mb_pages() >= 1) {
+		printf("Running private mem test with 2M pages\n");
+		execute_memory_conversion_test(VM_MEM_SRC_ANON_HTLB2M_AND_RESTRICTED_MEMFD);
+	} else
+		printf("Skipping private mem test with 2M pages\n");
+
+	return 0;
+}
-- 
2.38.1.431.g37b22c650d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory
  2022-11-11  1:42 ` [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory Vishal Annapurve
@ 2022-11-14 19:37   ` Peter Gonda
  2022-11-15  1:53     ` Vishal Annapurve
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Gonda @ 2022-11-14 19:37 UTC (permalink / raw)
  To: Vishal Annapurve
  Cc: x86, kvm, linux-kernel, linux-kselftest, pbonzini, vkuznets,
	wanpengli, jmattson, joro, tglx, mingo, bp, dave.hansen, hpa,
	shuah, yang.zhong, ricarkol, aaronlewis, wei.w.wang,
	kirill.shutemov, corbet, hughd, jlayton, bfields, akpm,
	chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen, michael.roth,
	qperret, steven.price, ak, david, luto, vbabka, marcorr,
	erdemaktas, nikunj, seanjc, diviness, maz, dmatlack,
	axelrasmussen, maciej.szmigiero, mizhang, bgardon, ackerleytng

On Thu, Nov 10, 2022 at 6:43 PM Vishal Annapurve <vannapurve@google.com> wrote:
>
> Introduce a set of APIs to execute VM with private memslots.
>
> Host userspace APIs for:
> 1) Setting up and executing VM having private memslots
> 2) Backing/unbacking guest private memory
>
> Guest APIs for:
> 1) Changing memory mapping type
>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> ---
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../kvm/include/x86_64/private_mem.h          |  37 +++
>  .../selftests/kvm/lib/x86_64/private_mem.c    | 211 ++++++++++++++++++
>  3 files changed, 249 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/include/x86_64/private_mem.h
>  create mode 100644 tools/testing/selftests/kvm/lib/x86_64/private_mem.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 0172eb6cb6ee..57385ad58527 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -53,6 +53,7 @@ LIBKVM_STRING += lib/string_override.c
>  LIBKVM_x86_64 += lib/x86_64/apic.c
>  LIBKVM_x86_64 += lib/x86_64/handlers.S
>  LIBKVM_x86_64 += lib/x86_64/perf_test_util.c
> +LIBKVM_x86_64 += lib/x86_64/private_mem.c
>  LIBKVM_x86_64 += lib/x86_64/processor.c
>  LIBKVM_x86_64 += lib/x86_64/svm.c
>  LIBKVM_x86_64 += lib/x86_64/ucall.c
> diff --git a/tools/testing/selftests/kvm/include/x86_64/private_mem.h b/tools/testing/selftests/kvm/include/x86_64/private_mem.h
> new file mode 100644
> index 000000000000..e556ded971fd
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86_64/private_mem.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2022, Google LLC.
> + */
> +
> +#ifndef SELFTEST_KVM_PRIVATE_MEM_H
> +#define SELFTEST_KVM_PRIVATE_MEM_H
> +
> +#include <stdint.h>
> +#include <kvm_util.h>
> +
> +void kvm_hypercall_map_shared(uint64_t gpa, uint64_t size);
> +void kvm_hypercall_map_private(uint64_t gpa, uint64_t size);
> +
> +void vm_unback_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size);
> +
> +void vm_allocate_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size);
> +
> +typedef void (*guest_code_fn)(void);
> +typedef void (*io_exit_handler)(struct kvm_vm *vm, uint32_t uc_arg1);
> +
> +struct test_setup_info {
> +       uint64_t test_area_gpa;
> +       uint64_t test_area_size;
> +       uint32_t test_area_slot;
> +};
> +
> +struct vm_setup_info {
> +       enum vm_mem_backing_src_type test_mem_src;
> +       struct test_setup_info test_info;
> +       guest_code_fn guest_fn;
> +       io_exit_handler ioexit_cb;
> +};
> +
> +void execute_vm_with_private_test_mem(struct vm_setup_info *info);
> +
> +#endif /* SELFTEST_KVM_PRIVATE_MEM_H */
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/private_mem.c b/tools/testing/selftests/kvm/lib/x86_64/private_mem.c
> new file mode 100644
> index 000000000000..3076cae81804
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/x86_64/private_mem.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * tools/testing/selftests/kvm/lib/kvm_util.c
> + *
> + * Copyright (C) 2022, Google LLC.
> + */
> +#define _GNU_SOURCE /* for program_invocation_name */
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <sched.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +
> +#include <linux/compiler.h>
> +#include <linux/kernel.h>
> +#include <linux/kvm_para.h>
> +
> +#include <test_util.h>
> +#include <kvm_util.h>
> +#include <private_mem.h>
> +#include <processor.h>
> +
> +static inline uint64_t __kvm_hypercall_map_gpa_range(uint64_t gpa, uint64_t size,
> +       uint64_t flags)
> +{
> +       return kvm_hypercall(KVM_HC_MAP_GPA_RANGE, gpa, size >> PAGE_SHIFT, flags, 0);
> +}
> +
> +static inline void kvm_hypercall_map_gpa_range(uint64_t gpa, uint64_t size,
> +       uint64_t flags)
> +{
> +       uint64_t ret;
> +
> +       GUEST_ASSERT_2(IS_PAGE_ALIGNED(gpa) && IS_PAGE_ALIGNED(size), gpa, size);
> +
> +       ret = __kvm_hypercall_map_gpa_range(gpa, size, flags);
> +       GUEST_ASSERT_1(!ret, ret);
> +}
> +
> +void kvm_hypercall_map_shared(uint64_t gpa, uint64_t size)
> +{
> +       kvm_hypercall_map_gpa_range(gpa, size, KVM_MAP_GPA_RANGE_DECRYPTED);
> +}
> +
> +void kvm_hypercall_map_private(uint64_t gpa, uint64_t size)
> +{
> +       kvm_hypercall_map_gpa_range(gpa, size, KVM_MAP_GPA_RANGE_ENCRYPTED);
> +}
> +
> +static void vm_update_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size,
> +       bool unback_mem)
> +{
> +       int restricted_fd;
> +       uint64_t restricted_fd_offset, guest_phys_base, fd_offset;
> +       struct kvm_enc_region enc_region;
> +       struct kvm_userspace_memory_region_ext *region_ext;
> +       struct kvm_userspace_memory_region *region;
> +       int fallocate_mode = 0;
> +       int ret;
> +
> +       region_ext = kvm_userspace_memory_region_ext_find(vm, gpa, gpa + size);
> +       TEST_ASSERT(region_ext != NULL, "Region not found");
> +       region = &region_ext->region;
> +       TEST_ASSERT(region->flags & KVM_MEM_PRIVATE,
> +               "Can not update private memfd for non-private memslot\n");
> +       restricted_fd = region_ext->restricted_fd;
> +       restricted_fd_offset = region_ext->restricted_offset;
> +       guest_phys_base = region->guest_phys_addr;
> +       fd_offset = restricted_fd_offset + (gpa - guest_phys_base);
> +
> +       if (unback_mem)
> +               fallocate_mode = (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE);
> +
> +       printf("restricted_fd %d fallocate_mode 0x%x for offset 0x%lx size 0x%lx\n",
> +               restricted_fd, fallocate_mode, fd_offset, size);
> +       ret = fallocate(restricted_fd, fallocate_mode, fd_offset, size);
> +       TEST_ASSERT(ret == 0, "fallocate failed\n");
> +       enc_region.addr = gpa;
> +       enc_region.size = size;
> +       if (unback_mem) {
> +               printf("undoing encryption for gpa 0x%lx size 0x%lx\n", gpa, size);
> +               vm_ioctl(vm, KVM_MEMORY_ENCRYPT_UNREG_REGION, &enc_region);
> +       } else {
> +               printf("doing encryption for gpa 0x%lx size 0x%lx\n", gpa, size);
> +               vm_ioctl(vm, KVM_MEMORY_ENCRYPT_REG_REGION, &enc_region);
> +       }
> +}
> +
> +void vm_unback_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size)
> +{
> +       vm_update_private_mem(vm, gpa, size, true);
> +}
> +
> +void vm_allocate_private_mem(struct kvm_vm *vm, uint64_t gpa, uint64_t size)
> +{
> +       vm_update_private_mem(vm, gpa, size, false);
> +}
> +
> +static void handle_vm_exit_map_gpa_hypercall(struct kvm_vm *vm,
> +                               struct kvm_vcpu *vcpu)
> +{
> +       uint64_t gpa, npages, attrs, size;
> +
> +       TEST_ASSERT(vcpu->run->hypercall.nr == KVM_HC_MAP_GPA_RANGE,
> +               "Unhandled Hypercall %lld\n", vcpu->run->hypercall.nr);
> +       gpa = vcpu->run->hypercall.args[0];
> +       npages = vcpu->run->hypercall.args[1];
> +       size = npages << MIN_PAGE_SHIFT;
> +       attrs = vcpu->run->hypercall.args[2];
> +       pr_info("Explicit conversion off 0x%lx size 0x%lx to %s\n", gpa, size,
> +               (attrs & KVM_MAP_GPA_RANGE_ENCRYPTED) ? "private" : "shared");
> +
> +       if (attrs & KVM_MAP_GPA_RANGE_ENCRYPTED)
> +               vm_allocate_private_mem(vm, gpa, size);
> +       else
> +               vm_unback_private_mem(vm, gpa, size);
> +
> +       vcpu->run->hypercall.ret = 0;
> +}
> +
> +static void vcpu_work(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> +       struct vm_setup_info *info)
> +{
> +       struct ucall uc;
> +       uint64_t cmd;
> +
> +       /*
> +        * Loop until the guest is done.
> +        */
> +
> +       while (true) {
> +               vcpu_run(vcpu);
> +
> +               if (vcpu->run->exit_reason == KVM_EXIT_IO) {
> +                       cmd = get_ucall(vcpu, &uc);
> +                       if (cmd != UCALL_SYNC)
> +                               break;
> +
> +                       TEST_ASSERT(info->ioexit_cb, "ioexit cb not present");
> +                       info->ioexit_cb(vm, uc.args[1]);
> +                       continue;
> +               }

Should this be integrated into the ucall library directly somehow?
That way users of VMs with private memory do not need special
handling?

After Sean's series:
https://lore.kernel.org/linux-arm-kernel/20220825232522.3997340-3-seanjc@google.com/
we have a common get_ucall() that this check could be integrated into?

> +
> +               if (vcpu->run->exit_reason == KVM_EXIT_HYPERCALL) {
> +                       handle_vm_exit_map_gpa_hypercall(vm, vcpu);
> +                       continue;
> +               }
> +
> +               TEST_FAIL("Unhandled VCPU exit reason %d\n",
> +                       vcpu->run->exit_reason);
> +               break;
> +       }
> +
> +       if (vcpu->run->exit_reason == KVM_EXIT_IO && cmd == UCALL_ABORT)
> +               TEST_FAIL("%s at %s:%ld, val = %lu", (const char *)uc.args[0],
> +                         __FILE__, uc.args[1], uc.args[2]);
> +}
> +
> +/*
> + * Execute guest vm with private memory memslots.
> + *
> + * Input Args:
> + *   info - pointer to a structure containing information about setting up a VM
> + *     with private memslots
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Function called by host userspace logic in selftests to execute guest vm
> + * logic. It will install test_mem_slot : containing the region of memory that
> + * would be used to test private/shared memory accesses to a memory backed by
> + * private memslots
> + */
> +void execute_vm_with_private_test_mem(struct vm_setup_info *info)
> +{
> +       struct kvm_vm *vm;
> +       struct kvm_enable_cap cap;
> +       struct kvm_vcpu *vcpu;
> +       uint64_t test_area_gpa, test_area_size;
> +       struct test_setup_info *test_info = &info->test_info;
> +
> +       TEST_ASSERT(info->guest_fn, "guest_fn not present");
> +       vm = vm_create_with_one_vcpu(&vcpu, info->guest_fn);

I am a little confused with how this library is going to work for SEV
VMs that want to have UPM private memory eventually.

Why should users of UPM be forced to use this very specific VM
creation and vCPU run loop. In the patch
https://lore.kernel.org/lkml/20220829171021.701198-1-pgonda@google.com/T/#m033ebc32df47a172bc6c46d4398b6c4387b7934d
SEV VMs need to be created specially vm_sev_create_with_one_vcpu() but
then callers can run the VM's vCPUs like other selftests.

How do you see this working with SEV VMs?



> +
> +       vm_check_cap(vm, KVM_CAP_EXIT_HYPERCALL);
> +       cap.cap = KVM_CAP_EXIT_HYPERCALL;
> +       cap.flags = 0;
> +       cap.args[0] = (1 << KVM_HC_MAP_GPA_RANGE);
> +       vm_ioctl(vm, KVM_ENABLE_CAP, &cap);
> +
> +       TEST_ASSERT(test_info->test_area_size, "Test mem size not present");
> +
> +       test_area_size = test_info->test_area_size;
> +       test_area_gpa = test_info->test_area_gpa;
> +       vm_userspace_mem_region_add(vm, info->test_mem_src, test_area_gpa,
> +               test_info->test_area_slot, test_area_size / vm->page_size,
> +               KVM_MEM_PRIVATE);
> +       vm_allocate_private_mem(vm, test_area_gpa, test_area_size);
> +
> +       pr_info("Mapping test memory pages 0x%zx page_size 0x%x\n",
> +               test_area_size/vm->page_size, vm->page_size);
> +       virt_map(vm, test_area_gpa, test_area_gpa, test_area_size/vm->page_size);
> +
> +       vcpu_work(vm, vcpu, info);
> +
> +       kvm_vm_free(vm);
> +}
> --
> 2.38.1.431.g37b22c650d-goog
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory
  2022-11-14 19:37   ` Peter Gonda
@ 2022-11-15  1:53     ` Vishal Annapurve
  2022-12-08 21:56       ` Vishal Annapurve
  0 siblings, 1 reply; 16+ messages in thread
From: Vishal Annapurve @ 2022-11-15  1:53 UTC (permalink / raw)
  To: Peter Gonda
  Cc: x86, kvm, linux-kernel, linux-kselftest, pbonzini, vkuznets,
	wanpengli, jmattson, joro, tglx, mingo, bp, dave.hansen, hpa,
	shuah, yang.zhong, ricarkol, aaronlewis, wei.w.wang,
	kirill.shutemov, corbet, hughd, jlayton, bfields, akpm,
	chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen, michael.roth,
	qperret, steven.price, ak, david, luto, vbabka, marcorr,
	erdemaktas, nikunj, seanjc, diviness, maz, dmatlack,
	axelrasmussen, maciej.szmigiero, mizhang, bgardon, ackerleytng

On Mon, Nov 14, 2022 at 11:37 AM Peter Gonda <pgonda@google.com> wrote:
>...
> > +static void handle_vm_exit_map_gpa_hypercall(struct kvm_vm *vm,
> > +                               struct kvm_vcpu *vcpu)
> > +{
> > +       uint64_t gpa, npages, attrs, size;
> > +
> > +       TEST_ASSERT(vcpu->run->hypercall.nr == KVM_HC_MAP_GPA_RANGE,
> > +               "Unhandled Hypercall %lld\n", vcpu->run->hypercall.nr);
> > +       gpa = vcpu->run->hypercall.args[0];
> > +       npages = vcpu->run->hypercall.args[1];
> > +       size = npages << MIN_PAGE_SHIFT;
> > +       attrs = vcpu->run->hypercall.args[2];
> > +       pr_info("Explicit conversion off 0x%lx size 0x%lx to %s\n", gpa, size,
> > +               (attrs & KVM_MAP_GPA_RANGE_ENCRYPTED) ? "private" : "shared");
> > +
> > +       if (attrs & KVM_MAP_GPA_RANGE_ENCRYPTED)
> > +               vm_allocate_private_mem(vm, gpa, size);
> > +       else
> > +               vm_unback_private_mem(vm, gpa, size);
> > +
> > +       vcpu->run->hypercall.ret = 0;
> > +}
> > +
> > +static void vcpu_work(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> > +       struct vm_setup_info *info)
> > +{
> > +       struct ucall uc;
> > +       uint64_t cmd;
> > +
> > +       /*
> > +        * Loop until the guest is done.
> > +        */
> > +
> > +       while (true) {
> > +               vcpu_run(vcpu);
> > +
> > +               if (vcpu->run->exit_reason == KVM_EXIT_IO) {
> > +                       cmd = get_ucall(vcpu, &uc);
> > +                       if (cmd != UCALL_SYNC)
> > +                               break;
> > +
> > +                       TEST_ASSERT(info->ioexit_cb, "ioexit cb not present");
> > +                       info->ioexit_cb(vm, uc.args[1]);
> > +                       continue;
> > +               }
>
> Should this be integrated into the ucall library directly somehow?
> That way users of VMs with private memory do not need special
> handling?
>
> After Sean's series:
> https://lore.kernel.org/linux-arm-kernel/20220825232522.3997340-3-seanjc@google.com/
> we have a common get_ucall() that this check could be integrated into?
>
> > +
> > +               if (vcpu->run->exit_reason == KVM_EXIT_HYPERCALL) {
> > +                       handle_vm_exit_map_gpa_hypercall(vm, vcpu);
> > +                       continue;
> > +               }
> > +
> > +               TEST_FAIL("Unhandled VCPU exit reason %d\n",
> > +                       vcpu->run->exit_reason);
> > +               break;
> > +       }
> > +
> > +       if (vcpu->run->exit_reason == KVM_EXIT_IO && cmd == UCALL_ABORT)
> > +               TEST_FAIL("%s at %s:%ld, val = %lu", (const char *)uc.args[0],
> > +                         __FILE__, uc.args[1], uc.args[2]);
> > +}
> > +
> > +/*
> > + * Execute guest vm with private memory memslots.
> > + *
> > + * Input Args:
> > + *   info - pointer to a structure containing information about setting up a VM
> > + *     with private memslots
> > + *
> > + * Output Args: None
> > + *
> > + * Return: None
> > + *
> > + * Function called by host userspace logic in selftests to execute guest vm
> > + * logic. It will install test_mem_slot : containing the region of memory that
> > + * would be used to test private/shared memory accesses to a memory backed by
> > + * private memslots
> > + */
> > +void execute_vm_with_private_test_mem(struct vm_setup_info *info)
> > +{
> > +       struct kvm_vm *vm;
> > +       struct kvm_enable_cap cap;
> > +       struct kvm_vcpu *vcpu;
> > +       uint64_t test_area_gpa, test_area_size;
> > +       struct test_setup_info *test_info = &info->test_info;
> > +
> > +       TEST_ASSERT(info->guest_fn, "guest_fn not present");
> > +       vm = vm_create_with_one_vcpu(&vcpu, info->guest_fn);
>
> I am a little confused with how this library is going to work for SEV
> VMs that want to have UPM private memory eventually.
>
> Why should users of UPM be forced to use this very specific VM
> creation and vCPU run loop. In the patch
> https://lore.kernel.org/lkml/20220829171021.701198-1-pgonda@google.com/T/#m033ebc32df47a172bc6c46d4398b6c4387b7934d
> SEV VMs need to be created specially vm_sev_create_with_one_vcpu() but
> then callers can run the VM's vCPUs like other selftests.
>
> How do you see this working with SEV VMs?
>

This VM creation method can be useful to run the VMs whose execution
might call mapgpa to change the memory attributes. New VM creation
method specific to Sev VMs can be introduced.

I tried to reuse this framework earlier for Sev VM selftests via:
1) https://lore.kernel.org/lkml/20220830224259.412342-8-vannapurve@google.com/T/#m8164d3111c9a17ebab77f01635df8930207cc65d
2) https://lore.kernel.org/lkml/20220830224259.412342-8-vannapurve@google.com/T/#m8164d3111c9a17ebab77f01635df8930207cc65d

Though these changes need to be refreshed after this updated series.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 1/6] KVM: x86: Add support for testing private memory
  2022-11-11  1:42 ` [V1 PATCH 1/6] KVM: x86: Add support for testing " Vishal Annapurve
@ 2022-11-22 10:07   ` Chao Peng
  2022-11-22 20:06     ` Sean Christopherson
  0 siblings, 1 reply; 16+ messages in thread
From: Chao Peng @ 2022-11-22 10:07 UTC (permalink / raw)
  To: Vishal Annapurve
  Cc: x86, kvm, linux-kernel, linux-kselftest, pbonzini, vkuznets,
	wanpengli, jmattson, joro, tglx, mingo, bp, dave.hansen, hpa,
	shuah, yang.zhong, ricarkol, aaronlewis, wei.w.wang,
	kirill.shutemov, corbet, hughd, jlayton, bfields, akpm,
	yu.c.zhang, jun.nakajima, dave.hansen, michael.roth, qperret,
	steven.price, ak, david, luto, vbabka, marcorr, erdemaktas,
	pgonda, nikunj, seanjc, diviness, maz, dmatlack, axelrasmussen,
	maciej.szmigiero, mizhang, bgardon, ackerleytng

On Fri, Nov 11, 2022 at 01:42:39AM +0000, Vishal Annapurve wrote:
> Introduce HAVE_KVM_PRIVATE_MEM_TESTING config to be able to test fd based
> approach to support private memory with non-confidential selftest VMs.
> To support this testing few important aspects need to be considered from
> the perspective of selftests -
> * KVM needs to know whether the access from guest VM is private or shared.
> Confidential VMs (SNP/TDX) carry a dedicated bit in gpa that can be used by
> KVM to deduce the nature of the access.
> Non-confidential VMs don't have mechanism to carry/convey such an
> information to KVM. So KVM just relies on what attributes are set by
> userspace VMM keeping the userspace VMM in the TCB for the testing
> purposes.
> * arch_private_mem_supported is updated to allow private memory logic to
> work with non-confidential vm selftests.
> 
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> ---
>  arch/x86/kvm/mmu/mmu.c          | 4 ++++
>  arch/x86/kvm/mmu/mmu_internal.h | 4 +++-
>  virt/kvm/Kconfig                | 4 ++++
>  virt/kvm/kvm_main.c             | 2 +-
>  4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 10017a9f26ee..b3118d00b284 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4280,6 +4280,10 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
>  
>  	fault->gfn = fault->addr >> PAGE_SHIFT;
>  	fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
> +#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
> +	fault->is_private = kvm_slot_can_be_private(fault->slot) &&
> +			kvm_mem_is_private(vcpu->kvm, fault->gfn);
> +#endif
>  
>  	if (page_fault_handle_page_track(vcpu, fault))
>  		return RET_PF_EMULATE;
> diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> index 5cdff5ca546c..2e759f39c2c5 100644
> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -188,7 +188,6 @@ struct kvm_page_fault {
>  
>  	/* Derived from mmu and global state.  */
>  	const bool is_tdp;
> -	const bool is_private;
>  	const bool nx_huge_page_workaround_enabled;
>  
>  	/*
> @@ -221,6 +220,9 @@ struct kvm_page_fault {
>  	/* The memslot containing gfn. May be NULL. */
>  	struct kvm_memory_slot *slot;
>  
> +	/* Derived from encryption bits of the faulting GPA for CVMs. */
> +	bool is_private;

Either we can wrap it with the CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING or if
it looks ugly I can remove the "const" in my code.

Chao
> +
>  	/* Outputs of kvm_faultin_pfn.  */
>  	kvm_pfn_t pfn;
>  	hva_t hva;
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index 69ca59e82149..300876afb0ca 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -93,3 +93,7 @@ config HAVE_KVM_RESTRICTED_MEM
>  config KVM_GENERIC_PRIVATE_MEM
>         bool
>         depends on HAVE_KVM_RESTRICTED_MEM
> +
> +config HAVE_KVM_PRIVATE_MEM_TESTING
> +       bool
> +       depends on KVM_GENERIC_PRIVATE_MEM
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dae6a2c196ad..54e57b7f1c15 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1750,7 +1750,7 @@ static void kvm_replace_memslot(struct kvm *kvm,
>  
>  bool __weak kvm_arch_has_private_mem(struct kvm *kvm)
>  {
> -	return false;
> +	return IS_ENABLED(CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING);
>  }
>  
>  static int check_memory_region_flags(struct kvm *kvm,
> -- 
> 2.38.1.431.g37b22c650d-goog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 1/6] KVM: x86: Add support for testing private memory
  2022-11-22 10:07   ` Chao Peng
@ 2022-11-22 20:06     ` Sean Christopherson
  2022-11-24  1:49       ` Marc Orr
                         ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Sean Christopherson @ 2022-11-22 20:06 UTC (permalink / raw)
  To: Chao Peng
  Cc: Vishal Annapurve, x86, kvm, linux-kernel, linux-kselftest,
	pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, yu.c.zhang, jun.nakajima, dave.hansen, michael.roth,
	qperret, steven.price, ak, david, luto, vbabka, marcorr,
	erdemaktas, pgonda, nikunj, diviness, maz, dmatlack,
	axelrasmussen, maciej.szmigiero, mizhang, bgardon, ackerleytng

On Tue, Nov 22, 2022, Chao Peng wrote:
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 10017a9f26ee..b3118d00b284 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -4280,6 +4280,10 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> >  
> >  	fault->gfn = fault->addr >> PAGE_SHIFT;
> >  	fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
> > +#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
> > +	fault->is_private = kvm_slot_can_be_private(fault->slot) &&
> > +			kvm_mem_is_private(vcpu->kvm, fault->gfn);
> > +#endif
> >  
> >  	if (page_fault_handle_page_track(vcpu, fault))
> >  		return RET_PF_EMULATE;
> > diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> > index 5cdff5ca546c..2e759f39c2c5 100644
> > --- a/arch/x86/kvm/mmu/mmu_internal.h
> > +++ b/arch/x86/kvm/mmu/mmu_internal.h
> > @@ -188,7 +188,6 @@ struct kvm_page_fault {
> >  
> >  	/* Derived from mmu and global state.  */
> >  	const bool is_tdp;
> > -	const bool is_private;
> >  	const bool nx_huge_page_workaround_enabled;
> >  
> >  	/*
> > @@ -221,6 +220,9 @@ struct kvm_page_fault {
> >  	/* The memslot containing gfn. May be NULL. */
> >  	struct kvm_memory_slot *slot;
> >  
> > +	/* Derived from encryption bits of the faulting GPA for CVMs. */
> > +	bool is_private;
> 
> Either we can wrap it with the CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING or if
> it looks ugly I can remove the "const" in my code.

Hmm, I think we can keep the const.  Similar to the bug in kvm_faultin_pfn()[*],
the kvm_slot_can_be_private() is bogus.  A fault should be considered private if
it's marked as private, whether or not userspace has configured the slot to be
private is irrelevant.  I.e. the xarray is the single source of truth, memslots
are just plumbing.

Then kvm_mmu_do_page_fault() can do something like:

diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index dbaf6755c5a7..456a9daa36e5 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -260,6 +260,8 @@ enum {
 static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
                                        u32 err, bool prefetch)
 {
+       bool is_tdp = likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault);
+
        struct kvm_page_fault fault = {
                .addr = cr2_or_gpa,
                .error_code = err,
@@ -269,13 +271,15 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
                .rsvd = err & PFERR_RSVD_MASK,
                .user = err & PFERR_USER_MASK,
                .prefetch = prefetch,
-               .is_tdp = likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault),
+               .is_tdp = is_tdp,
                .nx_huge_page_workaround_enabled =
                        is_nx_huge_page_enabled(vcpu->kvm),
 
                .max_level = KVM_MAX_HUGEPAGE_LEVEL,
                .req_level = PG_LEVEL_4K,
                .goal_level = PG_LEVEL_4K,
+               .private = IS_ENABLED(CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING) && is_tdp &&
+                          kvm_mem_is_private(vcpu->kvm, cr2_or_gpa >> PAGE_SHIFT),
        };
        int r;

[*] https://lore.kernel.org/all/Y3Vgc5KrNRA8r6vh@google.com

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 1/6] KVM: x86: Add support for testing private memory
  2022-11-22 20:06     ` Sean Christopherson
@ 2022-11-24  1:49       ` Marc Orr
  2022-11-28 16:21         ` Sean Christopherson
  2022-11-24 13:17       ` Chao Peng
  2022-12-02  0:26       ` Michael Roth
  2 siblings, 1 reply; 16+ messages in thread
From: Marc Orr @ 2022-11-24  1:49 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Chao Peng, Vishal Annapurve, x86, kvm, linux-kernel,
	linux-kselftest, pbonzini, vkuznets, wanpengli, jmattson, joro,
	tglx, mingo, bp, dave.hansen, hpa, shuah, yang.zhong, ricarkol,
	aaronlewis, wei.w.wang, kirill.shutemov, corbet, hughd, jlayton,
	bfields, akpm, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	erdemaktas, pgonda, nikunj, diviness, maz, dmatlack,
	axelrasmussen, maciej.szmigiero, mizhang, bgardon, ackerleytng

On Tue, Nov 22, 2022 at 12:06 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Nov 22, 2022, Chao Peng wrote:
> > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > index 10017a9f26ee..b3118d00b284 100644
> > > --- a/arch/x86/kvm/mmu/mmu.c
> > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > @@ -4280,6 +4280,10 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> > >
> > >     fault->gfn = fault->addr >> PAGE_SHIFT;
> > >     fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
> > > +#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
> > > +   fault->is_private = kvm_slot_can_be_private(fault->slot) &&
> > > +                   kvm_mem_is_private(vcpu->kvm, fault->gfn);
> > > +#endif
> > >
> > >     if (page_fault_handle_page_track(vcpu, fault))
> > >             return RET_PF_EMULATE;
> > > diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> > > index 5cdff5ca546c..2e759f39c2c5 100644
> > > --- a/arch/x86/kvm/mmu/mmu_internal.h
> > > +++ b/arch/x86/kvm/mmu/mmu_internal.h
> > > @@ -188,7 +188,6 @@ struct kvm_page_fault {
> > >
> > >     /* Derived from mmu and global state.  */
> > >     const bool is_tdp;
> > > -   const bool is_private;
> > >     const bool nx_huge_page_workaround_enabled;
> > >
> > >     /*
> > > @@ -221,6 +220,9 @@ struct kvm_page_fault {
> > >     /* The memslot containing gfn. May be NULL. */
> > >     struct kvm_memory_slot *slot;
> > >
> > > +   /* Derived from encryption bits of the faulting GPA for CVMs. */
> > > +   bool is_private;
> >
> > Either we can wrap it with the CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING or if
> > it looks ugly I can remove the "const" in my code.
>
> Hmm, I think we can keep the const.  Similar to the bug in kvm_faultin_pfn()[*],
> the kvm_slot_can_be_private() is bogus.  A fault should be considered private if
> it's marked as private, whether or not userspace has configured the slot to be
> private is irrelevant.  I.e. the xarray is the single source of truth, memslots
> are just plumbing.

If we incorporate Sean's suggestion and use xarray as the single
source of truth, then can we get rid of the
HAVE_KVM_PRIVATE_MEM_TESTING config?

Specifically, the self test can call the KVM_MEMORY_ENCRYPT_REG_REGION
ioctl which will set the bits for the private FD within KVM's xarray.

(Maybe this was part of the point that Sean was making; but his
feedback seemed focused on the discussion about keeping `is_private`
const, whereas I've been staring at this trying to figure out if we
can run the UPM selftests on a non-TDX/SNP VM WITHOUT a special
test-only config. And Sean's idea seems to eliminate the need for the
awkward CONFIG.)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 1/6] KVM: x86: Add support for testing private memory
  2022-11-22 20:06     ` Sean Christopherson
  2022-11-24  1:49       ` Marc Orr
@ 2022-11-24 13:17       ` Chao Peng
  2022-12-02  0:26       ` Michael Roth
  2 siblings, 0 replies; 16+ messages in thread
From: Chao Peng @ 2022-11-24 13:17 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vishal Annapurve, x86, kvm, linux-kernel, linux-kselftest,
	pbonzini, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, hpa, shuah, yang.zhong, ricarkol, aaronlewis,
	wei.w.wang, kirill.shutemov, corbet, hughd, jlayton, bfields,
	akpm, yu.c.zhang, jun.nakajima, dave.hansen, michael.roth,
	qperret, steven.price, ak, david, luto, vbabka, marcorr,
	erdemaktas, pgonda, nikunj, diviness, maz, dmatlack,
	axelrasmussen, maciej.szmigiero, mizhang, bgardon, ackerleytng

On Tue, Nov 22, 2022 at 08:06:01PM +0000, Sean Christopherson wrote:
> On Tue, Nov 22, 2022, Chao Peng wrote:
> > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > index 10017a9f26ee..b3118d00b284 100644
> > > --- a/arch/x86/kvm/mmu/mmu.c
> > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > @@ -4280,6 +4280,10 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> > >  
> > >  	fault->gfn = fault->addr >> PAGE_SHIFT;
> > >  	fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
> > > +#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
> > > +	fault->is_private = kvm_slot_can_be_private(fault->slot) &&
> > > +			kvm_mem_is_private(vcpu->kvm, fault->gfn);
> > > +#endif
> > >  
> > >  	if (page_fault_handle_page_track(vcpu, fault))
> > >  		return RET_PF_EMULATE;
> > > diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> > > index 5cdff5ca546c..2e759f39c2c5 100644
> > > --- a/arch/x86/kvm/mmu/mmu_internal.h
> > > +++ b/arch/x86/kvm/mmu/mmu_internal.h
> > > @@ -188,7 +188,6 @@ struct kvm_page_fault {
> > >  
> > >  	/* Derived from mmu and global state.  */
> > >  	const bool is_tdp;
> > > -	const bool is_private;
> > >  	const bool nx_huge_page_workaround_enabled;
> > >  
> > >  	/*
> > > @@ -221,6 +220,9 @@ struct kvm_page_fault {
> > >  	/* The memslot containing gfn. May be NULL. */
> > >  	struct kvm_memory_slot *slot;
> > >  
> > > +	/* Derived from encryption bits of the faulting GPA for CVMs. */
> > > +	bool is_private;
> > 
> > Either we can wrap it with the CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING or if
> > it looks ugly I can remove the "const" in my code.
> 
> Hmm, I think we can keep the const.  Similar to the bug in kvm_faultin_pfn()[*],
> the kvm_slot_can_be_private() is bogus.  A fault should be considered private if
> it's marked as private, whether or not userspace has configured the slot to be
> private is irrelevant.  I.e. the xarray is the single source of truth, memslots
> are just plumbing.

That makes sense to me. Thanks.

> 
> Then kvm_mmu_do_page_fault() can do something like:
> 
> diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> index dbaf6755c5a7..456a9daa36e5 100644
> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -260,6 +260,8 @@ enum {
>  static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>                                         u32 err, bool prefetch)
>  {
> +       bool is_tdp = likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault);
> +
>         struct kvm_page_fault fault = {
>                 .addr = cr2_or_gpa,
>                 .error_code = err,
> @@ -269,13 +271,15 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>                 .rsvd = err & PFERR_RSVD_MASK,
>                 .user = err & PFERR_USER_MASK,
>                 .prefetch = prefetch,
> -               .is_tdp = likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault),
> +               .is_tdp = is_tdp,
>                 .nx_huge_page_workaround_enabled =
>                         is_nx_huge_page_enabled(vcpu->kvm),
>  
>                 .max_level = KVM_MAX_HUGEPAGE_LEVEL,
>                 .req_level = PG_LEVEL_4K,
>                 .goal_level = PG_LEVEL_4K,
> +               .private = IS_ENABLED(CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING) && is_tdp &&
> +                          kvm_mem_is_private(vcpu->kvm, cr2_or_gpa >> PAGE_SHIFT),
>         };
>         int r;
> 
> [*] https://lore.kernel.org/all/Y3Vgc5KrNRA8r6vh@google.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 1/6] KVM: x86: Add support for testing private memory
  2022-11-24  1:49       ` Marc Orr
@ 2022-11-28 16:21         ` Sean Christopherson
  0 siblings, 0 replies; 16+ messages in thread
From: Sean Christopherson @ 2022-11-28 16:21 UTC (permalink / raw)
  To: Marc Orr
  Cc: Chao Peng, Vishal Annapurve, x86, kvm, linux-kernel,
	linux-kselftest, pbonzini, vkuznets, wanpengli, jmattson, joro,
	tglx, mingo, bp, dave.hansen, hpa, shuah, yang.zhong, ricarkol,
	aaronlewis, wei.w.wang, kirill.shutemov, corbet, hughd, jlayton,
	bfields, akpm, yu.c.zhang, jun.nakajima, dave.hansen,
	michael.roth, qperret, steven.price, ak, david, luto, vbabka,
	erdemaktas, pgonda, nikunj, diviness, maz, dmatlack,
	axelrasmussen, maciej.szmigiero, mizhang, bgardon, ackerleytng

On Wed, Nov 23, 2022, Marc Orr wrote:
> On Tue, Nov 22, 2022 at 12:06 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > @@ -221,6 +220,9 @@ struct kvm_page_fault {
> > > >     /* The memslot containing gfn. May be NULL. */
> > > >     struct kvm_memory_slot *slot;
> > > >
> > > > +   /* Derived from encryption bits of the faulting GPA for CVMs. */
> > > > +   bool is_private;
> > >
> > > Either we can wrap it with the CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING or if
> > > it looks ugly I can remove the "const" in my code.
> >
> > Hmm, I think we can keep the const.  Similar to the bug in kvm_faultin_pfn()[*],
> > the kvm_slot_can_be_private() is bogus.  A fault should be considered private if
> > it's marked as private, whether or not userspace has configured the slot to be
> > private is irrelevant.  I.e. the xarray is the single source of truth, memslots
> > are just plumbing.
> 
> If we incorporate Sean's suggestion and use xarray as the single
> source of truth, then can we get rid of the
> HAVE_KVM_PRIVATE_MEM_TESTING config?

No, we still want the opt-in config.  

> Specifically, the self test can call the KVM_MEMORY_ENCRYPT_REG_REGION
> ioctl which will set the bits for the private FD within KVM's xarray.

Yes, but that should be disallowed for regular VMs without HAVE_KVM_PRIVATE_MEM_TESTING=y.

> (Maybe this was part of the point that Sean was making; but his
> feedback seemed focused on the discussion about keeping `is_private`
> const, whereas I've been staring at this trying to figure out if we
> can run the UPM selftests on a non-TDX/SNP VM WITHOUT a special
> test-only config. And Sean's idea seems to eliminate the need for the
> awkward CONFIG.)

"need" was always relative.  It's obviously possible to enable any code without a
Kconfig, the question is whether or not it's a good idea to do so.  In this case,
the answer is "no", because allowing private memory opens up a number a of code
paths and thus potential bugs.  And we need something for kvm_arch_has_private_mem()
because returning "true" unconditionally is not correct for regular VMs.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 1/6] KVM: x86: Add support for testing private memory
  2022-11-22 20:06     ` Sean Christopherson
  2022-11-24  1:49       ` Marc Orr
  2022-11-24 13:17       ` Chao Peng
@ 2022-12-02  0:26       ` Michael Roth
  2 siblings, 0 replies; 16+ messages in thread
From: Michael Roth @ 2022-12-02  0:26 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Chao Peng, Vishal Annapurve, x86, kvm, linux-kernel,
	linux-kselftest, pbonzini, vkuznets, wanpengli, jmattson, joro,
	tglx, mingo, bp, dave.hansen, hpa, shuah, yang.zhong, ricarkol,
	aaronlewis, wei.w.wang, kirill.shutemov, corbet, hughd, jlayton,
	bfields, akpm, yu.c.zhang, jun.nakajima, dave.hansen, qperret,
	steven.price, ak, david, luto, vbabka, marcorr, erdemaktas,
	pgonda, nikunj, diviness, maz, dmatlack, axelrasmussen,
	maciej.szmigiero, mizhang, bgardon, ackerleytng

On Tue, Nov 22, 2022 at 08:06:01PM +0000, Sean Christopherson wrote:
> On Tue, Nov 22, 2022, Chao Peng wrote:
> > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > index 10017a9f26ee..b3118d00b284 100644
> > > --- a/arch/x86/kvm/mmu/mmu.c
> > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > @@ -4280,6 +4280,10 @@ static int direct_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> > >  
> > >  	fault->gfn = fault->addr >> PAGE_SHIFT;
> > >  	fault->slot = kvm_vcpu_gfn_to_memslot(vcpu, fault->gfn);
> > > +#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
> > > +	fault->is_private = kvm_slot_can_be_private(fault->slot) &&
> > > +			kvm_mem_is_private(vcpu->kvm, fault->gfn);
> > > +#endif
> > >  
> > >  	if (page_fault_handle_page_track(vcpu, fault))
> > >  		return RET_PF_EMULATE;
> > > diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> > > index 5cdff5ca546c..2e759f39c2c5 100644
> > > --- a/arch/x86/kvm/mmu/mmu_internal.h
> > > +++ b/arch/x86/kvm/mmu/mmu_internal.h
> > > @@ -188,7 +188,6 @@ struct kvm_page_fault {
> > >  
> > >  	/* Derived from mmu and global state.  */
> > >  	const bool is_tdp;
> > > -	const bool is_private;
> > >  	const bool nx_huge_page_workaround_enabled;
> > >  
> > >  	/*
> > > @@ -221,6 +220,9 @@ struct kvm_page_fault {
> > >  	/* The memslot containing gfn. May be NULL. */
> > >  	struct kvm_memory_slot *slot;
> > >  
> > > +	/* Derived from encryption bits of the faulting GPA for CVMs. */
> > > +	bool is_private;
> > 
> > Either we can wrap it with the CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING or if
> > it looks ugly I can remove the "const" in my code.
> 
> Hmm, I think we can keep the const.  Similar to the bug in kvm_faultin_pfn()[*],
> the kvm_slot_can_be_private() is bogus.  A fault should be considered private if
> it's marked as private, whether or not userspace has configured the slot to be
> private is irrelevant.  I.e. the xarray is the single source of truth, memslots
> are just plumbing.

I've been looking at pulling this series into our SNP+UPM patchset (and
replacing the UPM selftests that were including with UPMv9). We ended up
with something similar to what you've suggested, but instead of calling
kvm_mem_is_private() directly we added a wrapper in mmu_internal.h that's
called via:

kvm_mmu_do_page_fault():
  struct kvm_page_fault fault = {
    ...
    .is_private = kvm_mmu_fault_is_private()

where kvm_mmu_fault_is_private() is defined something like:

static bool kvm_mmu_fault_is_private(struct kvm *kvm, gpa_t gpa, u64 err)
{
        struct kvm_memory_slot *slot;
        gfn_t gfn = gpa_to_gfn(gpa);
        bool private_fault = false;

        slot = gfn_to_memslot(kvm, gpa_to_gfn(gpa));
        if (!slot)
                goto out;

        if (!kvm_slot_can_be_private(slot))
                goto out;

		/* If platform hook returns 1 then use it's determination of private_fault */
        if (static_call(kvm_x86_fault_is_private)(kvm, gpa, err, &private_fault) == 1)
                goto out;

        /*
         * Handling below is for guests that rely on the VMM to control when a fault
         * should be treated as private or not via KVM_MEM_ENCRYPT_{REG,UNREG}_REGION.
         * This is mainly for the KVM self-tests for restricted memory.
         */
#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING
        private_fault = kvm_mem_is_private(vcpu->kvm, cr2_or_gpa);
#endif

out:
        return private_fault;
}

I tried removing kvm_slot_can_be_private() based on your comments, but
we ended up hitting a crash in restrictedmem_get_page(). I think this is
because the xarray currently defaults to 'private', so when KVM MMU relies
only on xarray it can hit cases where it thinks a GPA should be backed
by a restricted page, but when it calls kvm_restrictedmem_get_pfn() a
null slot->restricted_file gets passed to restricted_get_page() and it
blows up.

I know Chao mentioned they were considering switching to 'shared' as the
default xarray value, which might fix this issue, but until then we've
left these checks in place.

Just figured I'd mention this in case Vishal hits similar issues.

-Mike

> 
> Then kvm_mmu_do_page_fault() can do something like:
> 
> diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> index dbaf6755c5a7..456a9daa36e5 100644
> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -260,6 +260,8 @@ enum {
>  static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>                                         u32 err, bool prefetch)
>  {
> +       bool is_tdp = likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault);
> +
>         struct kvm_page_fault fault = {
>                 .addr = cr2_or_gpa,
>                 .error_code = err,
> @@ -269,13 +271,15 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>                 .rsvd = err & PFERR_RSVD_MASK,
>                 .user = err & PFERR_USER_MASK,
>                 .prefetch = prefetch,
> -               .is_tdp = likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault),
> +               .is_tdp = is_tdp,
>                 .nx_huge_page_workaround_enabled =
>                         is_nx_huge_page_enabled(vcpu->kvm),
>  
>                 .max_level = KVM_MAX_HUGEPAGE_LEVEL,
>                 .req_level = PG_LEVEL_4K,
>                 .goal_level = PG_LEVEL_4K,
> +               .private = IS_ENABLED(CONFIG_HAVE_KVM_PRIVATE_MEM_TESTING) && is_tdp &&
> +                          kvm_mem_is_private(vcpu->kvm, cr2_or_gpa >> PAGE_SHIFT),
>         };
>         int r;
> 
> [*] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2FY3Vgc5KrNRA8r6vh%40google.com&amp;data=05%7C01%7CMichael.Roth%40amd.com%7Cc65b2b9b200e41f189ff08daccc4ffdc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638047443786540517%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Oajn46ulTFXBh0nIx61YmbbMAqW64EqKRniZJwLfXLs%3D&amp;reserved=0

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory
  2022-11-15  1:53     ` Vishal Annapurve
@ 2022-12-08 21:56       ` Vishal Annapurve
  0 siblings, 0 replies; 16+ messages in thread
From: Vishal Annapurve @ 2022-12-08 21:56 UTC (permalink / raw)
  To: Peter Gonda
  Cc: x86, kvm, linux-kernel, linux-kselftest, pbonzini, vkuznets,
	wanpengli, jmattson, joro, tglx, mingo, bp, dave.hansen, hpa,
	shuah, yang.zhong, ricarkol, aaronlewis, wei.w.wang,
	kirill.shutemov, corbet, hughd, jlayton, bfields, akpm,
	chao.p.peng, yu.c.zhang, jun.nakajima, dave.hansen, michael.roth,
	qperret, steven.price, ak, david, luto, vbabka, marcorr,
	erdemaktas, nikunj, seanjc, diviness, maz, dmatlack,
	axelrasmussen, maciej.szmigiero, mizhang, bgardon, ackerleytng

On Mon, Nov 14, 2022 at 5:53 PM Vishal Annapurve <vannapurve@google.com> wrote:
>
> On Mon, Nov 14, 2022 at 11:37 AM Peter Gonda <pgonda@google.com> wrote:
> >...
> > > +static void handle_vm_exit_map_gpa_hypercall(struct kvm_vm *vm,
> > > +                               struct kvm_vcpu *vcpu)
> > > +{
> > > +       uint64_t gpa, npages, attrs, size;
> > > +
> > > +       TEST_ASSERT(vcpu->run->hypercall.nr == KVM_HC_MAP_GPA_RANGE,
> > > +               "Unhandled Hypercall %lld\n", vcpu->run->hypercall.nr);
> > > +       gpa = vcpu->run->hypercall.args[0];
> > > +       npages = vcpu->run->hypercall.args[1];
> > > +       size = npages << MIN_PAGE_SHIFT;
> > > +       attrs = vcpu->run->hypercall.args[2];
> > > +       pr_info("Explicit conversion off 0x%lx size 0x%lx to %s\n", gpa, size,
> > > +               (attrs & KVM_MAP_GPA_RANGE_ENCRYPTED) ? "private" : "shared");
> > > +
> > > +       if (attrs & KVM_MAP_GPA_RANGE_ENCRYPTED)
> > > +               vm_allocate_private_mem(vm, gpa, size);
> > > +       else
> > > +               vm_unback_private_mem(vm, gpa, size);
> > > +
> > > +       vcpu->run->hypercall.ret = 0;
> > > +}
> > > +
> > > +static void vcpu_work(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> > > +       struct vm_setup_info *info)
> > > +{
> > > +       struct ucall uc;
> > > +       uint64_t cmd;
> > > +
> > > +       /*
> > > +        * Loop until the guest is done.
> > > +        */
> > > +
> > > +       while (true) {
> > > +               vcpu_run(vcpu);
> > > +
> > > +               if (vcpu->run->exit_reason == KVM_EXIT_IO) {
> > > +                       cmd = get_ucall(vcpu, &uc);
> > > +                       if (cmd != UCALL_SYNC)
> > > +                               break;
> > > +
> > > +                       TEST_ASSERT(info->ioexit_cb, "ioexit cb not present");
> > > +                       info->ioexit_cb(vm, uc.args[1]);
> > > +                       continue;
> > > +               }
> >
> > Should this be integrated into the ucall library directly somehow?
> > That way users of VMs with private memory do not need special
> > handling?
> >
> > After Sean's series:
> > https://lore.kernel.org/linux-arm-kernel/20220825232522.3997340-3-seanjc@google.com/
> > we have a common get_ucall() that this check could be integrated into?
> >

New patchset posted via [1] modifies the APIs to give more control in
the actual selftest implementation.

[1] https://lore.kernel.org/lkml/20221205232341.4131240-5-vannapurve@google.com/T/

> > > +
> > > +               if (vcpu->run->exit_reason == KVM_EXIT_HYPERCALL) {
> > > +                       handle_vm_exit_map_gpa_hypercall(vm, vcpu);
> > > +                       continue;
> > > +               }
> > > +
> > > +               TEST_FAIL("Unhandled VCPU exit reason %d\n",
> > > +                       vcpu->run->exit_reason);
> > > +               break;
> > > +       }
> > > +
> > > +       if (vcpu->run->exit_reason == KVM_EXIT_IO && cmd == UCALL_ABORT)
> > > +               TEST_FAIL("%s at %s:%ld, val = %lu", (const char *)uc.args[0],
> > > +                         __FILE__, uc.args[1], uc.args[2]);
> > > +}
> > > +
> > > +/*
> > > + * Execute guest vm with private memory memslots.
> > > + *
> > > + * Input Args:
> > > + *   info - pointer to a structure containing information about setting up a VM
> > > + *     with private memslots
> > > + *
> > > + * Output Args: None
> > > + *
> > > + * Return: None
> > > + *
> > > + * Function called by host userspace logic in selftests to execute guest vm
> > > + * logic. It will install test_mem_slot : containing the region of memory that
> > > + * would be used to test private/shared memory accesses to a memory backed by
> > > + * private memslots
> > > + */
> > > +void execute_vm_with_private_test_mem(struct vm_setup_info *info)
> > > +{
> > > +       struct kvm_vm *vm;
> > > +       struct kvm_enable_cap cap;
> > > +       struct kvm_vcpu *vcpu;
> > > +       uint64_t test_area_gpa, test_area_size;
> > > +       struct test_setup_info *test_info = &info->test_info;
> > > +
> > > +       TEST_ASSERT(info->guest_fn, "guest_fn not present");
> > > +       vm = vm_create_with_one_vcpu(&vcpu, info->guest_fn);
> >
> > I am a little confused with how this library is going to work for SEV
> > VMs that want to have UPM private memory eventually.
> >
> > Why should users of UPM be forced to use this very specific VM
> > creation and vCPU run loop. In the patch
> > https://lore.kernel.org/lkml/20220829171021.701198-1-pgonda@google.com/T/#m033ebc32df47a172bc6c46d4398b6c4387b7934d
> > SEV VMs need to be created specially vm_sev_create_with_one_vcpu() but
> > then callers can run the VM's vCPUs like other selftests.
> >
> > How do you see this working with SEV VMs?
> >
>
> This VM creation method can be useful to run the VMs whose execution
> might call mapgpa to change the memory attributes. New VM creation
> method specific to Sev VMs can be introduced.
>
> I tried to reuse this framework earlier for Sev VM selftests via:
> 1) https://lore.kernel.org/lkml/20220830224259.412342-8-vannapurve@google.com/T/#m8164d3111c9a17ebab77f01635df8930207cc65d
> 2) https://lore.kernel.org/lkml/20220830224259.412342-8-vannapurve@google.com/T/#m8164d3111c9a17ebab77f01635df8930207cc65d
>
> Though these changes need to be refreshed after this updated series.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-12-08 21:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-11  1:42 [V1 PATCH 0/6] selftests: KVM: selftests for fd-based private memory Vishal Annapurve
2022-11-11  1:42 ` [V1 PATCH 1/6] KVM: x86: Add support for testing " Vishal Annapurve
2022-11-22 10:07   ` Chao Peng
2022-11-22 20:06     ` Sean Christopherson
2022-11-24  1:49       ` Marc Orr
2022-11-28 16:21         ` Sean Christopherson
2022-11-24 13:17       ` Chao Peng
2022-12-02  0:26       ` Michael Roth
2022-11-11  1:42 ` [V1 PATCH 2/6] KVM: Selftests: Add support for " Vishal Annapurve
2022-11-11  1:42 ` [V1 PATCH 3/6] KVM: selftests: x86: Add IS_ALIGNED/IS_PAGE_ALIGNED helpers Vishal Annapurve
2022-11-11  1:42 ` [V1 PATCH 4/6] KVM: selftests: x86: Execute VMs with private memory Vishal Annapurve
2022-11-14 19:37   ` Peter Gonda
2022-11-15  1:53     ` Vishal Annapurve
2022-12-08 21:56       ` Vishal Annapurve
2022-11-11  1:42 ` [V1 PATCH 5/6] KVM: selftests: Add get_free_huge_2m_pages Vishal Annapurve
2022-11-11  1:42 ` [V1 PATCH 6/6] KVM: selftests: x86: Add selftest for private memory Vishal Annapurve

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).