public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: David Matlack <dmatlack@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Jim Mattson <jmattson@google.com>, Peter Xu <peterx@redhat.com>,
	Yang Zhong <yang.zhong@intel.com>,
	Wei Wang <wei.w.wang@intel.com>, Ben Gardon <bgardon@google.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: selftests: Fix nx_huge_pages_test on TDP-disabled hosts
Date: Mon, 26 Sep 2022 22:20:54 +0000	[thread overview]
Message-ID: <YzIlxmMOeSZHsnOu@google.com> (raw)
In-Reply-To: <20220926175219.605113-1-dmatlack@google.com>

On Mon, Sep 26, 2022, David Matlack wrote:
> +void virt_map_2m(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
> +		 uint64_t nr_2m_pages)

Gah, the selftest APIs are so frustrating.  Taking the number of pages in virt_map()
and vm_userspace_mem_region_add() is asinine.  Tests either assume a specific page
size, e.g. x86 tests, or manually calculate the number of pages from the number of
bytes, only for those calculations to be reversed.  E.g. set_memory_region_test's
usage of getpagesize().

As a baby step toward providing sane APIs and being able to create huge mappings
in common tests, what about refactoring virt_map() to take the size in bytes (see
below), and then assert in arch code that the page size matches vm->page_size,
except for x86, which translates back to its page "levels".

Then max_guest_memory_test can use virt_map() and __virt_map(), and we could even
delete vm_calc_num_guest_pages().  aarch64's usage in steal_time_init() can be
hardcoded to a single page, the size of the allocation is hardcoded to 64 bytes,
I see no reason to dance around that and pretend that page sizes can be smaller
than 64 bytes.

Then for proper support, we can figure out how to enumerate the allowed page sizes
in the guest for use in common tests.

And in parallel, we can cajole someone into refactoring vm_userspace_mem_region_add()
to take the size in bytes.

E.g.

void __virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
		uint64_t nr_bytes, size_t page_size)
{
	uint64_t nr_pages = DIV_ROUND_UP(nr_bytes, page_size);

	TEST_ASSERT(vaddr + size > vaddr, "Vaddr overflow");
	TEST_ASSERT(paddr + size > paddr, "Paddr overflow");

	while (npages--) {
		virt_pg_map(vm, vaddr, paddr);
		vaddr += page_size;
		paddr += page_size;
	}
}

void virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
	      uint64_t nr_bytes)
{
	__virt_map(vm, vaddr, paddr, nr_bytes, vm->page_size);
}

and in max_guest_memory_test:

#ifdef __x86_64__
		/* TODO: use huge pages for other architectures. */
		__virt_map(vm, gpa, gpa, slot_size, PG_SIZE_1G);
#else
		virt_map(vm, gpa, gpa, slot_size);
#endif

> +{
> +	int i;
> +
> +	for (i = 0; i < nr_2m_pages; i++) {
> +		__virt_pg_map(vm, vaddr, paddr, PG_LEVEL_2M);
> +
> +		vaddr += PG_SIZE_2M;
> +		paddr += PG_SIZE_2M;
> +	}
> +}
> +
>  static uint64_t *_vm_get_page_table_entry(struct kvm_vm *vm,
>  					  struct kvm_vcpu *vcpu,
>  					  uint64_t vaddr)
> diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> index cc6421716400..a850769692b7 100644
> --- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> @@ -22,7 +22,8 @@
>  #define HPAGE_GPA		(4UL << 30) /* 4G prevents collision w/ slot 0 */
>  #define HPAGE_GVA		HPAGE_GPA /* GVA is arbitrary, so use GPA. */
>  #define PAGES_PER_2MB_HUGE_PAGE 512
> -#define HPAGE_SLOT_NPAGES	(3 * PAGES_PER_2MB_HUGE_PAGE)
> +#define HPAGE_SLOT_2MB_PAGES	3
> +#define HPAGE_SLOT_NPAGES	(HPAGE_SLOT_2MB_PAGES * PAGES_PER_2MB_HUGE_PAGE)
>  
>  /*
>   * Passed by nx_huge_pages_test.sh to provide an easy warning if this test is
> @@ -141,7 +142,11 @@ void run_test(int reclaim_period_ms, bool disable_nx_huge_pages,
>  				    HPAGE_GPA, HPAGE_SLOT,
>  				    HPAGE_SLOT_NPAGES, 0);
>  
> -	virt_map(vm, HPAGE_GVA, HPAGE_GPA, HPAGE_SLOT_NPAGES);
> +	/*
> +	 * Use 2MiB virtual mappings so that KVM can map the region with huge
> +	 * pages even if TDP is disabled.
> +	 */
> +	virt_map_2m(vm, HPAGE_GVA, HPAGE_GPA, HPAGE_SLOT_2MB_PAGES);

Hmm, what about probing TDP support and deliberately using 4KiB pages when TDP is
enabled?  That would give a bit of bonus coverage by verifying that KVM creates
huge pages irrespective of guest mapping level.

  reply	other threads:[~2022-09-26 22:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-26 17:52 [PATCH] KVM: selftests: Fix nx_huge_pages_test on TDP-disabled hosts David Matlack
2022-09-26 22:20 ` Sean Christopherson [this message]
2022-09-27 12:13   ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YzIlxmMOeSZHsnOu@google.com \
    --to=seanjc@google.com \
    --cc=bgardon@google.com \
    --cc=dmatlack@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=wei.w.wang@intel.com \
    --cc=yang.zhong@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox