* [PATCH v2 0/2] selftests: verify page stats in kvm x86/mmu @ 2021-08-29 18:26 Mingwei Zhang 2021-08-29 18:26 ` [PATCH v2 1/2] selftests: KVM: align guest physical memory base address to 1GB Mingwei Zhang 2021-08-29 18:26 ` [PATCH v2 2/2] selftests: KVM: use dirty logging to check if page stats work correctly Mingwei Zhang 0 siblings, 2 replies; 7+ messages in thread From: Mingwei Zhang @ 2021-08-29 18:26 UTC (permalink / raw) To: Paolo Bonzini Cc: kvm, linux-kernel, Sean Christopherson, David Matlack, Jing Zhang, Peter Xu, Ben Gardon, Mingwei Zhang This patch set leverages the existing dirty logging performance selftest to verify whether the page stats work correctly in KVM x86/mmu especially with the consequence of dirty logging. v1 -> v2: - split the page alignment fix into a different commit [bgardon] - update the TEST_ASSERT conditions [bgardon] Mingwei Zhang (2): selftests: KVM: align guest physical memory base address to 1GB selftests: KVM: use dirty logging to check if page stats work correctly .../selftests/kvm/dirty_log_perf_test.c | 44 +++++++++++++++++++ .../testing/selftests/kvm/include/test_util.h | 1 + .../selftests/kvm/include/x86_64/processor.h | 7 +++ .../selftests/kvm/lib/perf_test_util.c | 8 ++-- tools/testing/selftests/kvm/lib/test_util.c | 29 ++++++++++++ 5 files changed, 85 insertions(+), 4 deletions(-) -- 2.33.0.259.gc128427fd7-goog ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] selftests: KVM: align guest physical memory base address to 1GB 2021-08-29 18:26 [PATCH v2 0/2] selftests: verify page stats in kvm x86/mmu Mingwei Zhang @ 2021-08-29 18:26 ` Mingwei Zhang 2021-08-31 16:54 ` Sean Christopherson 2021-08-29 18:26 ` [PATCH v2 2/2] selftests: KVM: use dirty logging to check if page stats work correctly Mingwei Zhang 1 sibling, 1 reply; 7+ messages in thread From: Mingwei Zhang @ 2021-08-29 18:26 UTC (permalink / raw) To: Paolo Bonzini Cc: kvm, linux-kernel, Sean Christopherson, David Matlack, Jing Zhang, Peter Xu, Ben Gardon, Mingwei Zhang Existing selftest library function always allocates GPA range that aligns to the end of GPA address space, ie., the allocated GPA range guarantees to end at the last available GPA. This ends up with the fact that selftest programs cannot control the alignment of the base GPA. Depending on the size of the allocation, the base GPA may align only on a 4K based bounday. The alignment of base GPA sometimes creates problems for dirty logging selftest where a 2MB-aligned or 1GB-aligned base GPA is needed to create NPT/EPT mappings for hugepages. So, fix this issue and ensure all GPA allocation starts from a 1GB bounary in all architectures. Cc: Sean Christopherson <seanjc@google.com> Cc: David Matlack <dmatlack@google.com> Cc: Jing Zhang <jingzhangos@google.com> Cc: Peter Xu <peterx@redhat.com> Suggested-by: Ben Gardon <bgardon@google.com> Signed-off-by: Mingwei Zhang <mizhang@google.com> --- tools/testing/selftests/kvm/lib/perf_test_util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c index 0ef80dbdc116..96c30b8d6593 100644 --- a/tools/testing/selftests/kvm/lib/perf_test_util.c +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c @@ -93,10 +93,10 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus, guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) * perf_test_args.guest_page_size; guest_test_phys_mem &= ~(perf_test_args.host_page_size - 1); -#ifdef __s390x__ - /* Align to 1M (segment size) */ - guest_test_phys_mem &= ~((1 << 20) - 1); -#endif + + /* Align to 1G for all architectures */ + guest_test_phys_mem &= ~((1 << 30) - 1); + pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem); /* Add extra memory slots for testing */ -- 2.33.0.259.gc128427fd7-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] selftests: KVM: align guest physical memory base address to 1GB 2021-08-29 18:26 ` [PATCH v2 1/2] selftests: KVM: align guest physical memory base address to 1GB Mingwei Zhang @ 2021-08-31 16:54 ` Sean Christopherson 2021-09-06 19:56 ` Mingwei Zhang 0 siblings, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2021-08-31 16:54 UTC (permalink / raw) To: Mingwei Zhang Cc: Paolo Bonzini, kvm, linux-kernel, David Matlack, Jing Zhang, Peter Xu, Ben Gardon On Sun, Aug 29, 2021, Mingwei Zhang wrote: > Existing selftest library function always allocates GPA range that aligns > to the end of GPA address space, ie., the allocated GPA range guarantees to > end at the last available GPA. This ends up with the fact that selftest > programs cannot control the alignment of the base GPA. Depending on the > size of the allocation, the base GPA may align only on a 4K based > bounday. > > The alignment of base GPA sometimes creates problems for dirty logging > selftest where a 2MB-aligned or 1GB-aligned base GPA is needed to > create NPT/EPT mappings for hugepages. > > So, fix this issue and ensure all GPA allocation starts from a 1GB bounary > in all architectures. > > Cc: Sean Christopherson <seanjc@google.com> > Cc: David Matlack <dmatlack@google.com> > Cc: Jing Zhang <jingzhangos@google.com> > Cc: Peter Xu <peterx@redhat.com> > > Suggested-by: Ben Gardon <bgardon@google.com> > Signed-off-by: Mingwei Zhang <mizhang@google.com> > --- > tools/testing/selftests/kvm/lib/perf_test_util.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c > index 0ef80dbdc116..96c30b8d6593 100644 > --- a/tools/testing/selftests/kvm/lib/perf_test_util.c > +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c > @@ -93,10 +93,10 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus, > guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) * > perf_test_args.guest_page_size; > guest_test_phys_mem &= ~(perf_test_args.host_page_size - 1); > -#ifdef __s390x__ > - /* Align to 1M (segment size) */ > - guest_test_phys_mem &= ~((1 << 20) - 1); > -#endif > + > + /* Align to 1G for all architectures */ > + guest_test_phys_mem &= ~((1 << 30) - 1); 1gb may not be appropriate for all architectures and we don't want to _just_ test 1gb aligned memslots. The alignment should be tied to the backing store, even if the test is hardcoded to use THP, that way the alignment logic works without modification if the backing store is changed. I had a patch[1] that did this, let me go resurrect that series. My series got put on the backburner in favor of Yanan's series[2] which did a much better job of identifying/handling the host virtual address alignment, but IIRC my approach for handling GPA was correct. [1] https://lore.kernel.org/kvm/20210210230625.550939-6-seanjc@google.com/ [2] https://lkml.kernel.org/r/20210330080856.14940-1-wangyanan55@huawei.com > + > pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem); > > /* Add extra memory slots for testing */ > -- > 2.33.0.259.gc128427fd7-goog > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] selftests: KVM: align guest physical memory base address to 1GB 2021-08-31 16:54 ` Sean Christopherson @ 2021-09-06 19:56 ` Mingwei Zhang 0 siblings, 0 replies; 7+ messages in thread From: Mingwei Zhang @ 2021-09-06 19:56 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, kvm, LKML, David Matlack, Jing Zhang, Peter Xu, Ben Gardon > 1gb may not be appropriate for all architectures and we don't want to _just_ > test 1gb aligned memslots. The alignment should be tied to the backing store, > even if the test is hardcoded to use THP, that way the alignment logic works > without modification if the backing store is changed. Agree on that. > > I had a patch[1] that did this, let me go resurrect that series. My series got > put on the backburner in favor of Yanan's series[2] which did a much better > job of identifying/handling the host virtual address alignment, but IIRC my > approach for handling GPA was correct. > > [1] https://lore.kernel.org/kvm/20210210230625.550939-6-seanjc@google.com/ > [2] https://lkml.kernel.org/r/20210330080856.14940-1-wangyanan55@huawei.com > Thanks for the info. I will use patch [1] instead of mine in the next version. Regards. -Mingwei ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] selftests: KVM: use dirty logging to check if page stats work correctly 2021-08-29 18:26 [PATCH v2 0/2] selftests: verify page stats in kvm x86/mmu Mingwei Zhang 2021-08-29 18:26 ` [PATCH v2 1/2] selftests: KVM: align guest physical memory base address to 1GB Mingwei Zhang @ 2021-08-29 18:26 ` Mingwei Zhang 2021-08-30 4:02 ` kernel test robot 1 sibling, 1 reply; 7+ messages in thread From: Mingwei Zhang @ 2021-08-29 18:26 UTC (permalink / raw) To: Paolo Bonzini Cc: kvm, linux-kernel, Sean Christopherson, David Matlack, Jing Zhang, Peter Xu, Ben Gardon, Mingwei Zhang When dirty logging is enabled, KVM splits the hugepage mapping in NPT/EPT into the smallest 4K size after guest VM writes to it. This property could be used to check if the page stats metrics work properly in KVM x86/mmu. At the same time, this logic might be used the other way around: using page stats to verify if dirty logging really splits all huge pages after guest VM writes to all memory. So add page stats checking in dirty logging performance selftest. In particular, add checks in three locations: - just after vm is created; - after populating memory into vm without turning on dirty logging; - after guest vm writing to all memory again with dirty logging turned on. Tested using commands: - ./dirty_log_perf_test -s anonymous_hugetlb_1gb - ./dirty_log_perf_test -s anonymous_hugetlb_2mb - ./dirty_log_perf_test -s anonymous_thp Cc: Sean Christopherson <seanjc@google.com> Cc: David Matlack <dmatlack@google.com> Cc: Jing Zhang <jingzhangos@google.com> Cc: Peter Xu <peterx@redhat.com> Suggested-by: Ben Gardon <bgardon@google.com> Signed-off-by: Mingwei Zhang <mizhang@google.com> --- .../selftests/kvm/dirty_log_perf_test.c | 44 +++++++++++++++++++ .../testing/selftests/kvm/include/test_util.h | 1 + .../selftests/kvm/include/x86_64/processor.h | 7 +++ tools/testing/selftests/kvm/lib/test_util.c | 29 ++++++++++++ 4 files changed, 81 insertions(+) diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c index 3c30d0045d8d..1fc63ad55cf3 100644 --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c @@ -19,6 +19,10 @@ #include "perf_test_util.h" #include "guest_modes.h" +#ifdef __x86_64__ +#include "processor.h" +#endif + /* How many host loops to run by default (one KVM_GET_DIRTY_LOG for each loop)*/ #define TEST_HOST_LOOP_N 2UL @@ -166,6 +170,18 @@ static void run_test(enum vm_guest_mode mode, void *arg) vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size, p->slots, p->backing_src); +#ifdef __x86_64__ + /* + * No vCPUs have been started yet, so KVM should not have created any + * mapping at this moment. + */ + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_4K) == 0, + "4K page is non zero"); + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_2M) == 0, + "2M page is non zero"); + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_1G) == 0, + "1G page is non zero"); +#endif perf_test_args.wr_fract = p->wr_fract; guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm_get_page_shift(vm); @@ -211,6 +227,22 @@ static void run_test(enum vm_guest_mode mode, void *arg) pr_info("Populate memory time: %ld.%.9lds\n", ts_diff.tv_sec, ts_diff.tv_nsec); +#ifdef __x86_64__ + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_4K) != 0, + "4K page is zero"); + /* Ensure THP page stats is non-zero to minimize the flakiness. */ + if (p->backing_src == VM_MEM_SRC_ANONYMOUS_THP) + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_2M) > 0 + "2M page number is zero"); + else if (p->backing_src == VM_MEM_SRC_ANONYMOUS_HUGETLB_2MB) + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_2M) == + (guest_percpu_mem_size * nr_vcpus) >> X86_PAGE_2M_SHIFT, + "2M page number does not match"); + else if (p->backing_src == VM_MEM_SRC_ANONYMOUS_HUGETLB_1GB) + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_1G) == + (guest_percpu_mem_size * nr_vcpus) >> X86_PAGE_1G_SHIFT, + "1G page number does not match"); +#endif /* Enable dirty logging */ clock_gettime(CLOCK_MONOTONIC, &start); enable_dirty_logging(vm, p->slots); @@ -256,6 +288,18 @@ static void run_test(enum vm_guest_mode mode, void *arg) iteration, ts_diff.tv_sec, ts_diff.tv_nsec); } } +#ifdef __x86_64__ + /* + * When vCPUs writes to all memory again with dirty logging enabled, we + * should see only 4K page mappings exist in KVM mmu. + */ + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_4K) != 0, + "4K page is zero after dirtying memory"); + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_2M) == 0, + "2M page is non-zero after dirtying memory"); + TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_1G) == 0, + "1G page is non-zero after dirtying memory"); +#endif /* Disable dirty logging */ clock_gettime(CLOCK_MONOTONIC, &start); diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h index d79be15dd3d2..dca5fcf7aa87 100644 --- a/tools/testing/selftests/kvm/include/test_util.h +++ b/tools/testing/selftests/kvm/include/test_util.h @@ -102,6 +102,7 @@ const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i); size_t get_backing_src_pagesz(uint32_t i); void backing_src_help(void); enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name); +size_t get_page_stats(uint32_t page_level); /* * Whether or not the given source type is shared memory (as opposed to diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 242ae8e09a65..9749319821a3 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -39,6 +39,13 @@ #define X86_CR4_SMAP (1ul << 21) #define X86_CR4_PKE (1ul << 22) +#define X86_PAGE_4K_SHIFT 12 +#define X86_PAGE_4K (1ul << X86_PAGE_4K_SHIFT) +#define X86_PAGE_2M_SHIFT 21 +#define X86_PAGE_2M (1ul << X86_PAGE_2M_SHIFT) +#define X86_PAGE_1G_SHIFT 30 +#define X86_PAGE_1G (1ul << X86_PAGE_1G_SHIFT) + /* CPUID.1.ECX */ #define CPUID_VMX (1ul << 5) #define CPUID_SMX (1ul << 6) diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c index af1031fed97f..07eb6b5c125e 100644 --- a/tools/testing/selftests/kvm/lib/test_util.c +++ b/tools/testing/selftests/kvm/lib/test_util.c @@ -15,6 +15,13 @@ #include "linux/kernel.h" #include "test_util.h" +#include "processor.h" + +static const char * const pagestat_filepaths[] = { + "/sys/kernel/debug/kvm/pages_4k", + "/sys/kernel/debug/kvm/pages_2m", + "/sys/kernel/debug/kvm/pages_1g", +}; /* * Parses "[0-9]+[kmgt]?". @@ -141,6 +148,28 @@ size_t get_trans_hugepagesz(void) return size; } +#ifdef __x86_64__ +size_t get_stats_from_file(const char *path) +{ + size_t value; + FILE *f; + + f = fopen(path, "r"); + TEST_ASSERT(f != NULL, "Error in opening file: %s\n", path); + + fscanf(f, "%ld", &value); + fclose(f); + + return value; +} + +size_t get_page_stats(uint32_t page_level) +{ + TEST_ASSERT(page_level <= X86_PAGE_SIZE_1G, "page type error."); + return get_stats_from_file(pagestat_filepaths[page_level]); +} +#endif + size_t get_def_hugetlb_pagesz(void) { char buf[64]; -- 2.33.0.259.gc128427fd7-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] selftests: KVM: use dirty logging to check if page stats work correctly 2021-08-29 18:26 ` [PATCH v2 2/2] selftests: KVM: use dirty logging to check if page stats work correctly Mingwei Zhang @ 2021-08-30 4:02 ` kernel test robot 2021-08-30 4:42 ` Mingwei Zhang 0 siblings, 1 reply; 7+ messages in thread From: kernel test robot @ 2021-08-30 4:02 UTC (permalink / raw) To: Mingwei Zhang, Paolo Bonzini Cc: kbuild-all, kvm, linux-kernel, Sean Christopherson, David Matlack, Jing Zhang, Peter Xu, Ben Gardon, Mingwei Zhang [-- Attachment #1: Type: text/plain, Size: 2217 bytes --] Hi Mingwei, Thank you for the patch! Yet something to improve: [auto build test ERROR on kvm/queue] [also build test ERROR on vhost/linux-next linux/master linus/master v5.14 next-20210827] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Mingwei-Zhang/selftests-verify-page-stats-in-kvm-x86-mmu/20210830-022817 base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue config: x86_64-rhel-8.3-kselftests (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/0fafdf6b378c081631b13e89a550992c2ddcfd4a git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Mingwei-Zhang/selftests-verify-page-stats-in-kvm-x86-mmu/20210830-022817 git checkout 0fafdf6b378c081631b13e89a550992c2ddcfd4a # save the attached .config to linux build tree mkdir build_dir make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash -C tools/testing/selftests install If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): dirty_log_perf_test.c: In function 'run_test': >> dirty_log_perf_test.c:236:28: error: macro "TEST_ASSERT" requires 3 arguments, but only 1 given 236 | "2M page number is zero"); | ^ In file included from include/kvm_util.h:10, from dirty_log_perf_test.c:17: include/test_util.h:46: note: macro "TEST_ASSERT" defined here 46 | #define TEST_ASSERT(e, fmt, ...) \ | >> dirty_log_perf_test.c:235:3: error: 'TEST_ASSERT' undeclared (first use in this function) 235 | TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_2M) > 0 | ^~~~~~~~~~~ dirty_log_perf_test.c:235:3: note: each undeclared identifier is reported only once for each function it appears in --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 42149 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] selftests: KVM: use dirty logging to check if page stats work correctly 2021-08-30 4:02 ` kernel test robot @ 2021-08-30 4:42 ` Mingwei Zhang 0 siblings, 0 replies; 7+ messages in thread From: Mingwei Zhang @ 2021-08-30 4:42 UTC (permalink / raw) To: kernel test robot Cc: Paolo Bonzini, kbuild-all, kvm, LKML, Sean Christopherson, David Matlack, Jing Zhang, Peter Xu, Ben Gardon > dirty_log_perf_test.c: In function 'run_test': > >> dirty_log_perf_test.c:236:28: error: macro "TEST_ASSERT" requires 3 arguments, but only 1 given > 236 | "2M page number is zero"); > | ^ > In file included from include/kvm_util.h:10, > from dirty_log_perf_test.c:17: > include/test_util.h:46: note: macro "TEST_ASSERT" defined here > 46 | #define TEST_ASSERT(e, fmt, ...) \ > | > >> dirty_log_perf_test.c:235:3: error: 'TEST_ASSERT' undeclared (first use in this function) > 235 | TEST_ASSERT(get_page_stats(X86_PAGE_SIZE_2M) > 0 > | ^~~~~~~~~~~ > dirty_log_perf_test.c:235:3: note: each undeclared identifier is reported only once for each function it appears in > Sorry, there is one fix not checked in before I sent it out. Will fix it in the next version. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-06 19:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-29 18:26 [PATCH v2 0/2] selftests: verify page stats in kvm x86/mmu Mingwei Zhang 2021-08-29 18:26 ` [PATCH v2 1/2] selftests: KVM: align guest physical memory base address to 1GB Mingwei Zhang 2021-08-31 16:54 ` Sean Christopherson 2021-09-06 19:56 ` Mingwei Zhang 2021-08-29 18:26 ` [PATCH v2 2/2] selftests: KVM: use dirty logging to check if page stats work correctly Mingwei Zhang 2021-08-30 4:02 ` kernel test robot 2021-08-30 4:42 ` Mingwei Zhang
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).