* [PATCH gmem 1/5] KVM: selftests: Rework fallocate() helper to work across multiple memslots
2023-10-23 19:15 [PATCH gmem 0/5] KVM: selftests: Fix multiple memslots in conv test Sean Christopherson
@ 2023-10-23 19:15 ` Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 2/5] KVM: selftests: Handle memslot splits in private mem conversions test Sean Christopherson
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2023-10-23 19:15 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Michael Roth
Rework vm_guest_mem_fallocate() to play nice with ranges that cover more
than one memslot. Converting a range that covers multiple memslots is
most definitely an interesting testcase, and there's no reason to force
the caller to manually shatter the range, especially since the size of
the region might not be known by the caller.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/lib/kvm_util.c | 34 ++++++++++++----------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 8fc70c021c1c..33bc2c6be970 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1206,30 +1206,32 @@ void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot)
__vm_mem_region_delete(vm, memslot2region(vm, slot), true);
}
-void vm_guest_mem_fallocate(struct kvm_vm *vm, uint64_t gpa, uint64_t size,
+void vm_guest_mem_fallocate(struct kvm_vm *vm, uint64_t base, uint64_t size,
bool punch_hole)
{
+ const int mode = FALLOC_FL_KEEP_SIZE | (punch_hole ? FALLOC_FL_PUNCH_HOLE : 0);
struct userspace_mem_region *region;
- uint64_t end = gpa + size - 1;
+ uint64_t end = base + size;
+ uint64_t gpa, len;
off_t fd_offset;
- int mode, ret;
+ int ret;
- region = userspace_mem_region_find(vm, gpa, gpa);
- TEST_ASSERT(region && region->region.flags & KVM_MEM_PRIVATE,
- "Private memory region not found for GPA 0x%lx", gpa);
+ for (gpa = base; gpa < end; gpa += len) {
+ uint64_t offset;
- TEST_ASSERT(region == userspace_mem_region_find(vm, end, end),
- "fallocate() for guest_memfd must act on a single memslot");
+ region = userspace_mem_region_find(vm, gpa, gpa);
+ TEST_ASSERT(region && region->region.flags & KVM_MEM_PRIVATE,
+ "Private memory region not found for GPA 0x%lx", gpa);
- fd_offset = region->region.gmem_offset +
- (gpa - region->region.guest_phys_addr);
+ offset = (gpa - region->region.guest_phys_addr);
+ fd_offset = region->region.gmem_offset + offset;
+ len = min_t(uint64_t, end - gpa, region->region.memory_size - offset);
- mode = FALLOC_FL_KEEP_SIZE | (punch_hole ? FALLOC_FL_PUNCH_HOLE : 0);
-
- ret = fallocate(region->region.gmem_fd, mode, fd_offset, size);
- TEST_ASSERT(!ret, "fallocate() failed to %s at %lx[%lu], fd = %d, mode = %x, offset = %lx\n",
- punch_hole ? "punch hole" : "allocate", gpa, size,
- region->region.gmem_fd, mode, fd_offset);
+ ret = fallocate(region->region.gmem_fd, mode, fd_offset, len);
+ TEST_ASSERT(!ret, "fallocate() failed to %s at %lx (len = %lu), fd = %d, mode = %x, offset = %lx\n",
+ punch_hole ? "punch hole" : "allocate", gpa, len,
+ region->region.gmem_fd, mode, fd_offset);
+ }
}
/* Returns the size of a vCPU's kvm_run structure. */
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH gmem 2/5] KVM: selftests: Handle memslot splits in private mem conversions test
2023-10-23 19:15 [PATCH gmem 0/5] KVM: selftests: Fix multiple memslots in conv test Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 1/5] KVM: selftests: Rework fallocate() helper to work across multiple memslots Sean Christopherson
@ 2023-10-23 19:15 ` Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 3/5] KVM: selftests: Let user specify nr of memslots in private mem conversion Sean Christopherson
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2023-10-23 19:15 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Michael Roth
Rework the SYNC_{SHARED,PRIVATE} handling in the private memory conversion
test to play nice with ranges that are covered by multiple memslots. This
will allow letting the user specify the number of memslots, and more
importantly allow testing KVM_SET_MEMORY_ATTRIBUTES calls that also split
across memslots.
Opportunistically update the error messages to clarify whether the assert
fired in the guest vs. host.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../kvm/x86_64/private_mem_conversions_test.c | 44 +++++++++++--------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
index c04e7d61a585..c3992a295b5a 100644
--- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
+++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
@@ -28,25 +28,25 @@
#define PER_CPU_DATA_SIZE ((uint64_t)(SZ_2M + PAGE_SIZE))
/* Horrific macro so that the line info is captured accurately :-( */
-#define memcmp_g(gpa, pattern, size) \
-do { \
- uint8_t *mem = (uint8_t *)gpa; \
- size_t i; \
- \
- for (i = 0; i < size; i++) \
- __GUEST_ASSERT(mem[i] == pattern, \
- "Expected 0x%x at offset %lu (gpa 0x%llx), got 0x%x", \
- pattern, i, gpa + i, mem[i]); \
+#define memcmp_g(gpa, pattern, size) \
+do { \
+ uint8_t *mem = (uint8_t *)gpa; \
+ size_t i; \
+ \
+ for (i = 0; i < size; i++) \
+ __GUEST_ASSERT(mem[i] == pattern, \
+ "Guest expected 0x%x at offset %lu (gpa 0x%llx), got 0x%x", \
+ pattern, i, gpa + i, mem[i]); \
} while (0)
-static void memcmp_h(uint8_t *mem, uint8_t pattern, size_t size)
+static void memcmp_h(uint8_t *mem, uint64_t gpa, uint8_t pattern, size_t size)
{
size_t i;
for (i = 0; i < size; i++)
TEST_ASSERT(mem[i] == pattern,
- "Expected 0x%x at offset %lu, got 0x%x",
- pattern, i, mem[i]);
+ "Host expected 0x%x at gpa 0x%lx, got 0x%x",
+ pattern, gpa + i, mem[i]);
}
/*
@@ -335,19 +335,25 @@ static void *__test_mem_conversions(void *__vcpu)
case UCALL_ABORT:
REPORT_GUEST_ASSERT(uc);
case UCALL_SYNC: {
- uint8_t *hva = addr_gpa2hva(vm, uc.args[1]);
- uint64_t size = uc.args[2];
+ uint64_t gpa = uc.args[1];
+ size_t size = uc.args[2];
+ size_t i;
TEST_ASSERT(uc.args[0] == SYNC_SHARED ||
uc.args[0] == SYNC_PRIVATE,
"Unknown sync command '%ld'", uc.args[0]);
- /* In all cases, the host should observe the shared data. */
- memcmp_h(hva, uc.args[3], size);
+ for (i = 0; i < size; i += vm->page_size) {
+ size_t nr_bytes = min_t(size_t, vm->page_size, size - i);
+ uint8_t *hva = addr_gpa2hva(vm, gpa + i);
- /* For shared, write the new pattern to guest memory. */
- if (uc.args[0] == SYNC_SHARED)
- memset(hva, uc.args[4], size);
+ /* In all cases, the host should observe the shared data. */
+ memcmp_h(hva, gpa + i, uc.args[3], nr_bytes);
+
+ /* For shared, write the new pattern to guest memory. */
+ if (uc.args[0] == SYNC_SHARED)
+ memset(hva, uc.args[4], nr_bytes);
+ }
break;
}
case UCALL_DONE:
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH gmem 3/5] KVM: selftests: Let user specify nr of memslots in private mem conversion
2023-10-23 19:15 [PATCH gmem 0/5] KVM: selftests: Fix multiple memslots in conv test Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 1/5] KVM: selftests: Rework fallocate() helper to work across multiple memslots Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 2/5] KVM: selftests: Handle memslot splits in private mem conversions test Sean Christopherson
@ 2023-10-23 19:15 ` Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 4/5] KVM: selftests: Use dedicated pattern for testing that mem is shared by default Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 5/5] KVM: selftests: Verify default pattern was written in private mem conversion Sean Christopherson
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2023-10-23 19:15 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Michael Roth
Let the user specify the number of memslots in the private mem conversion
test, i.e. don't require the number of memslots to be '1' or "nr_vcpus".
Creating more memslots than vCPUs is particularly interesting, e.g. it can
result in a single KVM_SET_MEMORY_ATTRIBUTES spanning multiple memslots.
This also fixes an issue where the test will crash when running with
multiple vCPUs but only a single memslot.
To keep the math reasonable, align each vCPU's chunk to at least 2MiB (the
size is 2MiB+4KiB), and require the total size to be cleanly divisible by
the number of memslots. The goal is to be able to validate that KVM plays
nice with multiple memslots, being able to create a truly arbitrary number
of memslots doesn't add meaningful value, i.e. isn't worth the cost.
Reported-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../kvm/x86_64/private_mem_conversions_test.c | 36 +++++++++++--------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
index c3992a295b5a..3f6d8d4dbc53 100644
--- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
+++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
@@ -371,8 +371,10 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t
* Allocate enough memory so that each vCPU's chunk of memory can be
* naturally aligned with respect to the size of the backing store.
*/
- const size_t size = align_up(PER_CPU_DATA_SIZE, get_backing_src_pagesz(src_type));
- const size_t memfd_size = size * nr_vcpus;
+ const size_t alignment = max_t(size_t, SZ_2M, get_backing_src_pagesz(src_type));
+ const size_t per_cpu_size = align_up(PER_CPU_DATA_SIZE, alignment);
+ const size_t memfd_size = per_cpu_size * nr_vcpus;
+ const size_t slot_size = memfd_size / nr_memslots;
struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
pthread_t threads[KVM_MAX_VCPUS];
uint64_t gmem_flags;
@@ -384,6 +386,9 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t
.type = KVM_X86_SW_PROTECTED_VM,
};
+ TEST_ASSERT(slot_size * nr_memslots == memfd_size,
+ "The memfd size (0x%lx) needs to be cleanly divisible by the number of memslots (%u)",
+ memfd_size, nr_memslots);
vm = __vm_create_with_vcpus(shape, nr_vcpus, 0, guest_code, vcpus);
vm_enable_cap(vm, KVM_CAP_EXIT_HYPERCALL, (1 << KVM_HC_MAP_GPA_RANGE));
@@ -395,16 +400,20 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t
memfd = vm_create_guest_memfd(vm, memfd_size, gmem_flags);
for (i = 0; i < nr_memslots; i++)
- vm_mem_add(vm, src_type, BASE_DATA_GPA + size * i,
- BASE_DATA_SLOT + i, size / vm->page_size,
- KVM_MEM_PRIVATE, memfd, size * i);
+ vm_mem_add(vm, src_type, BASE_DATA_GPA + slot_size * i,
+ BASE_DATA_SLOT + i, slot_size / vm->page_size,
+ KVM_MEM_PRIVATE, memfd, slot_size * i);
for (i = 0; i < nr_vcpus; i++) {
- uint64_t gpa = BASE_DATA_GPA + i * size;
+ uint64_t gpa = BASE_DATA_GPA + i * per_cpu_size;
vcpu_args_set(vcpus[i], 1, gpa);
- virt_map(vm, gpa, gpa, size / vm->page_size);
+ /*
+ * Map only what is needed so that an out-of-bounds access
+ * results #PF => SHUTDOWN instead of data corruption.
+ */
+ virt_map(vm, gpa, gpa, PER_CPU_DATA_SIZE / vm->page_size);
pthread_create(&threads[i], NULL, __test_mem_conversions, vcpus[i]);
}
@@ -432,29 +441,28 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t
static void usage(const char *cmd)
{
puts("");
- printf("usage: %s [-h] [-m] [-s mem_type] [-n nr_vcpus]\n", cmd);
+ printf("usage: %s [-h] [-m nr_memslots] [-s mem_type] [-n nr_vcpus]\n", cmd);
puts("");
backing_src_help("-s");
puts("");
puts(" -n: specify the number of vcpus (default: 1)");
puts("");
- puts(" -m: use multiple memslots (default: 1)");
+ puts(" -m: specify the number of memslots (default: 1)");
puts("");
}
int main(int argc, char *argv[])
{
enum vm_mem_backing_src_type src_type = DEFAULT_VM_MEM_SRC;
- bool use_multiple_memslots = false;
+ uint32_t nr_memslots = 1;
uint32_t nr_vcpus = 1;
- uint32_t nr_memslots;
int opt;
TEST_REQUIRE(kvm_has_cap(KVM_CAP_GUEST_MEMFD));
TEST_REQUIRE(kvm_has_cap(KVM_CAP_EXIT_HYPERCALL));
TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM));
- while ((opt = getopt(argc, argv, "hms:n:")) != -1) {
+ while ((opt = getopt(argc, argv, "hm:s:n:")) != -1) {
switch (opt) {
case 's':
src_type = parse_backing_src_type(optarg);
@@ -463,7 +471,7 @@ int main(int argc, char *argv[])
nr_vcpus = atoi_positive("nr_vcpus", optarg);
break;
case 'm':
- use_multiple_memslots = true;
+ nr_memslots = atoi_positive("nr_memslots", optarg);
break;
case 'h':
default:
@@ -472,8 +480,6 @@ int main(int argc, char *argv[])
}
}
- nr_memslots = use_multiple_memslots ? nr_vcpus : 1;
-
test_mem_conversions(src_type, nr_vcpus, nr_memslots);
return 0;
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH gmem 4/5] KVM: selftests: Use dedicated pattern for testing that mem is shared by default
2023-10-23 19:15 [PATCH gmem 0/5] KVM: selftests: Fix multiple memslots in conv test Sean Christopherson
` (2 preceding siblings ...)
2023-10-23 19:15 ` [PATCH gmem 3/5] KVM: selftests: Let user specify nr of memslots in private mem conversion Sean Christopherson
@ 2023-10-23 19:15 ` Sean Christopherson
2023-10-23 19:15 ` [PATCH gmem 5/5] KVM: selftests: Verify default pattern was written in private mem conversion Sean Christopherson
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2023-10-23 19:15 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Michael Roth
Somewhat arbitrarily use 0xaa as the data pattern when testing that memory
is shared by default. Inverting the bits of the init pattern (0xcc)
yields 0x33, which just so happens to be the same as pattern #3, i.e. makes
triaging failures on the default check unnecessarily difficult.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../selftests/kvm/x86_64/private_mem_conversions_test.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
index 3f6d8d4dbc53..8a2f924fd031 100644
--- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
+++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
@@ -122,13 +122,15 @@ struct {
static void guest_test_explicit_conversion(uint64_t base_gpa, bool do_fallocate)
{
+ const uint8_t def_p = 0xaa;
const uint8_t init_p = 0xcc;
uint64_t j;
int i;
/* Memory should be shared by default. */
- memset((void *)base_gpa, ~init_p, PER_CPU_DATA_SIZE);
- guest_sync_shared(base_gpa, PER_CPU_DATA_SIZE, (uint8_t)~init_p, init_p);
+ memset((void *)base_gpa, def_p, PER_CPU_DATA_SIZE);
+ guest_sync_shared(base_gpa, PER_CPU_DATA_SIZE, def_p, init_p);
+
memcmp_g(base_gpa, init_p, PER_CPU_DATA_SIZE);
for (i = 0; i < ARRAY_SIZE(test_ranges); i++) {
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH gmem 5/5] KVM: selftests: Verify default pattern was written in private mem conversion
2023-10-23 19:15 [PATCH gmem 0/5] KVM: selftests: Fix multiple memslots in conv test Sean Christopherson
` (3 preceding siblings ...)
2023-10-23 19:15 ` [PATCH gmem 4/5] KVM: selftests: Use dedicated pattern for testing that mem is shared by default Sean Christopherson
@ 2023-10-23 19:15 ` Sean Christopherson
4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2023-10-23 19:15 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Michael Roth
Add a super paranoid sanity check in the private mem conversion test to
verify that the initial "default" pattern was actually written. The check
doesn't add meaningful test coverage, the goal is mostly to aid in debug
if something goes sideways, e.g. to give the user confidence that the
writes didn't somehow get dropped entirely.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../testing/selftests/kvm/x86_64/private_mem_conversions_test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
index 8a2f924fd031..91f343609e18 100644
--- a/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
+++ b/tools/testing/selftests/kvm/x86_64/private_mem_conversions_test.c
@@ -129,6 +129,7 @@ static void guest_test_explicit_conversion(uint64_t base_gpa, bool do_fallocate)
/* Memory should be shared by default. */
memset((void *)base_gpa, def_p, PER_CPU_DATA_SIZE);
+ memcmp_g(base_gpa, def_p, PER_CPU_DATA_SIZE);
guest_sync_shared(base_gpa, PER_CPU_DATA_SIZE, def_p, init_p);
memcmp_g(base_gpa, init_p, PER_CPU_DATA_SIZE);
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply [flat|nested] 6+ messages in thread