From: Sean Christopherson <seanjc@google.com>
To: Nicholas Piggin <npiggin@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Shuah Khan <shuah@kernel.org>,
Michael Ellerman <mpe@ellerman.id.au>,
kvm@vger.kernel.org, linux-kselftest@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 2/4] KVM: selftests: Add aligned guest physical page allocator
Date: Wed, 28 Jun 2023 11:24:02 -0700 [thread overview]
Message-ID: <ZJx6wqgS4TOi/D4j@google.com> (raw)
In-Reply-To: <20230408040020.868929-3-npiggin@gmail.com>
On Sat, Apr 08, 2023, Nicholas Piggin wrote:
> powerpc will require this to allocate MMU tables in guest memory that
> are aligned and larger than the base page size.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> .../selftests/kvm/include/kvm_util_base.h | 2 +
> tools/testing/selftests/kvm/lib/kvm_util.c | 44 ++++++++++++-------
> 2 files changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 16425da16861..8a27bd4111ff 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -679,6 +679,8 @@ const char *exit_reason_str(unsigned int exit_reason);
>
> vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
> uint32_t memslot);
> +vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
> + vm_paddr_t paddr_min, uint32_t memslot);
> vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
> vm_paddr_t paddr_min, uint32_t memslot);
> vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 8ec20ac33de0..4f15bbbb8f5e 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1898,6 +1898,7 @@ const char *exit_reason_str(unsigned int exit_reason)
> * Input Args:
> * vm - Virtual Machine
> * num - number of pages
> + * align - pages alignment
> * paddr_min - Physical address minimum
> * memslot - Memory region to allocate page from
> *
> @@ -1911,7 +1912,7 @@ const char *exit_reason_str(unsigned int exit_reason)
> * and their base address is returned. A TEST_ASSERT failure occurs if
> * not enough pages are available at or above paddr_min.
> */
> -vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
> +vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
I'd prefer to use double underscores, even though they are imperfect, because
appending a single specifier always seems to result in the name becoming stale
sooner or later, e.g. when another param with a default is added.
And IIUC, PPC requires the page tables to be naturally aligned, so rather than
expose the inner helper and copy+paste the rather odd KVM_GUEST_PAGE_TABLE_MIN_PADDR
and vm->memslots[MEM_REGION_PT] stuff, what if we instead have vm_alloc_page_table()
deal with the alignment? And provide a PPC-specific wrapper so that other
architectures don't need to manually specify '1' page?
E.g.
---
.../selftests/kvm/include/kvm_util_base.h | 18 +++++++++++++++---
tools/testing/selftests/kvm/lib/kvm_util.c | 14 ++++++++------
.../selftests/kvm/lib/powerpc/processor.c | 8 ++------
3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index f14a059f58fb..e52405c9fa8b 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -700,11 +700,23 @@ const char *exit_reason_str(unsigned int exit_reason);
vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
uint32_t memslot);
-vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
- vm_paddr_t paddr_min, uint32_t memslot);
vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
vm_paddr_t paddr_min, uint32_t memslot);
-vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm);
+
+vm_paddr_t __vm_alloc_page_table(struct kvm_vm *vm, size_t nr_pages);
+
+/*
+ * PowerPC conditionally needs to allocate multiple pages for each page table,
+ * all other architectures consume exactly one page per table.
+ */
+#if defined(__powerpc64__
+#define vm_alloc_page_table __vm_alloc_page_table
+#else
+static inline vm_alloc_page_table(struct kvm_vm *vm)
+{
+ return __vm_alloc_page_table(vm, 1)
+}
+#endif
/*
* ____vm_create() does KVM_CREATE_VM and little else. __vm_create() also
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 70f792ba444c..ffd18afe9725 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1946,8 +1946,9 @@ const char *exit_reason_str(unsigned int exit_reason)
* and their base address is returned. A TEST_ASSERT failure occurs if
* not enough pages are available at or above paddr_min.
*/
-vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
- vm_paddr_t paddr_min, uint32_t memslot)
+static vm_paddr_t __vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
+ size_t align, vm_paddr_t paddr_min,
+ uint32_t memslot)
{
struct userspace_mem_region *region;
sparsebit_idx_t pg, base;
@@ -1992,7 +1993,7 @@ vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
vm_paddr_t paddr_min, uint32_t memslot)
{
- return vm_phy_pages_alloc_align(vm, num, 1, paddr_min, memslot);
+ return __vm_phy_pages_alloc(vm, num, 1, paddr_min, memslot);
}
vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
@@ -2001,10 +2002,11 @@ vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
return vm_phy_pages_alloc(vm, 1, paddr_min, memslot);
}
-vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm)
+vm_paddr_t __vm_alloc_page_table(struct kvm_vm *vm)
{
- return vm_phy_page_alloc(vm, KVM_GUEST_PAGE_TABLE_MIN_PADDR,
- vm->memslots[MEM_REGION_PT]);
+ return __vm_phy_pages_alloc(vm, KVM_GUEST_PAGE_TABLE_MIN_PADDR,
+ nr_pages, nr_pages,
+ vm->memslots[MEM_REGION_PT]);
}
/*
diff --git a/tools/testing/selftests/kvm/lib/powerpc/processor.c b/tools/testing/selftests/kvm/lib/powerpc/processor.c
index 7052ce9b5029..57d64d281467 100644
--- a/tools/testing/selftests/kvm/lib/powerpc/processor.c
+++ b/tools/testing/selftests/kvm/lib/powerpc/processor.c
@@ -44,9 +44,7 @@ void virt_arch_pgd_alloc(struct kvm_vm *vm)
pgd_pages = (1UL << (RADIX_PGD_INDEX_SIZE + 3)) >> vm->page_shift;
if (!pgd_pages)
pgd_pages = 1;
- pgtb = vm_phy_pages_alloc_align(vm, pgd_pages, pgd_pages,
- KVM_GUEST_PAGE_TABLE_MIN_PADDR,
- vm->memslots[MEM_REGION_PT]);
+ pt = vm_alloc_page_table(vm, pgd_pages);
vm->pgd = pgtb;
/* Set the base page directory in the proc table */
@@ -168,9 +166,7 @@ void virt_arch_pg_map(struct kvm_vm *vm, uint64_t gva, uint64_t gpa)
pt_pages = (1ULL << (nls + 3)) >> vm->page_shift;
if (!pt_pages)
pt_pages = 1;
- pt = vm_phy_pages_alloc_align(vm, pt_pages, pt_pages,
- KVM_GUEST_PAGE_TABLE_MIN_PADDR,
- vm->memslots[MEM_REGION_PT]);
+ pt = vm_alloc_page_table(vm, pt_pages);
pde = PDE_VALID | nls | pt;
*pdep = cpu_to_be64(pde);
}
base-commit: 15a281f5c83f34d4d1808e5f790403b0770c5e78
--
WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Nicholas Piggin <npiggin@gmail.com>
Cc: kvm@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
linux-kselftest@vger.kernel.org,
Paolo Bonzini <pbonzini@redhat.com>,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 2/4] KVM: selftests: Add aligned guest physical page allocator
Date: Wed, 28 Jun 2023 11:24:02 -0700 [thread overview]
Message-ID: <ZJx6wqgS4TOi/D4j@google.com> (raw)
In-Reply-To: <20230408040020.868929-3-npiggin@gmail.com>
On Sat, Apr 08, 2023, Nicholas Piggin wrote:
> powerpc will require this to allocate MMU tables in guest memory that
> are aligned and larger than the base page size.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> .../selftests/kvm/include/kvm_util_base.h | 2 +
> tools/testing/selftests/kvm/lib/kvm_util.c | 44 ++++++++++++-------
> 2 files changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 16425da16861..8a27bd4111ff 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -679,6 +679,8 @@ const char *exit_reason_str(unsigned int exit_reason);
>
> vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
> uint32_t memslot);
> +vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
> + vm_paddr_t paddr_min, uint32_t memslot);
> vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
> vm_paddr_t paddr_min, uint32_t memslot);
> vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 8ec20ac33de0..4f15bbbb8f5e 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1898,6 +1898,7 @@ const char *exit_reason_str(unsigned int exit_reason)
> * Input Args:
> * vm - Virtual Machine
> * num - number of pages
> + * align - pages alignment
> * paddr_min - Physical address minimum
> * memslot - Memory region to allocate page from
> *
> @@ -1911,7 +1912,7 @@ const char *exit_reason_str(unsigned int exit_reason)
> * and their base address is returned. A TEST_ASSERT failure occurs if
> * not enough pages are available at or above paddr_min.
> */
> -vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
> +vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
I'd prefer to use double underscores, even though they are imperfect, because
appending a single specifier always seems to result in the name becoming stale
sooner or later, e.g. when another param with a default is added.
And IIUC, PPC requires the page tables to be naturally aligned, so rather than
expose the inner helper and copy+paste the rather odd KVM_GUEST_PAGE_TABLE_MIN_PADDR
and vm->memslots[MEM_REGION_PT] stuff, what if we instead have vm_alloc_page_table()
deal with the alignment? And provide a PPC-specific wrapper so that other
architectures don't need to manually specify '1' page?
E.g.
---
.../selftests/kvm/include/kvm_util_base.h | 18 +++++++++++++++---
tools/testing/selftests/kvm/lib/kvm_util.c | 14 ++++++++------
.../selftests/kvm/lib/powerpc/processor.c | 8 ++------
3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index f14a059f58fb..e52405c9fa8b 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -700,11 +700,23 @@ const char *exit_reason_str(unsigned int exit_reason);
vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
uint32_t memslot);
-vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
- vm_paddr_t paddr_min, uint32_t memslot);
vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
vm_paddr_t paddr_min, uint32_t memslot);
-vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm);
+
+vm_paddr_t __vm_alloc_page_table(struct kvm_vm *vm, size_t nr_pages);
+
+/*
+ * PowerPC conditionally needs to allocate multiple pages for each page table,
+ * all other architectures consume exactly one page per table.
+ */
+#if defined(__powerpc64__
+#define vm_alloc_page_table __vm_alloc_page_table
+#else
+static inline vm_alloc_page_table(struct kvm_vm *vm)
+{
+ return __vm_alloc_page_table(vm, 1)
+}
+#endif
/*
* ____vm_create() does KVM_CREATE_VM and little else. __vm_create() also
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 70f792ba444c..ffd18afe9725 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1946,8 +1946,9 @@ const char *exit_reason_str(unsigned int exit_reason)
* and their base address is returned. A TEST_ASSERT failure occurs if
* not enough pages are available at or above paddr_min.
*/
-vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
- vm_paddr_t paddr_min, uint32_t memslot)
+static vm_paddr_t __vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
+ size_t align, vm_paddr_t paddr_min,
+ uint32_t memslot)
{
struct userspace_mem_region *region;
sparsebit_idx_t pg, base;
@@ -1992,7 +1993,7 @@ vm_paddr_t vm_phy_pages_alloc_align(struct kvm_vm *vm, size_t num, size_t align,
vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
vm_paddr_t paddr_min, uint32_t memslot)
{
- return vm_phy_pages_alloc_align(vm, num, 1, paddr_min, memslot);
+ return __vm_phy_pages_alloc(vm, num, 1, paddr_min, memslot);
}
vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
@@ -2001,10 +2002,11 @@ vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
return vm_phy_pages_alloc(vm, 1, paddr_min, memslot);
}
-vm_paddr_t vm_alloc_page_table(struct kvm_vm *vm)
+vm_paddr_t __vm_alloc_page_table(struct kvm_vm *vm)
{
- return vm_phy_page_alloc(vm, KVM_GUEST_PAGE_TABLE_MIN_PADDR,
- vm->memslots[MEM_REGION_PT]);
+ return __vm_phy_pages_alloc(vm, KVM_GUEST_PAGE_TABLE_MIN_PADDR,
+ nr_pages, nr_pages,
+ vm->memslots[MEM_REGION_PT]);
}
/*
diff --git a/tools/testing/selftests/kvm/lib/powerpc/processor.c b/tools/testing/selftests/kvm/lib/powerpc/processor.c
index 7052ce9b5029..57d64d281467 100644
--- a/tools/testing/selftests/kvm/lib/powerpc/processor.c
+++ b/tools/testing/selftests/kvm/lib/powerpc/processor.c
@@ -44,9 +44,7 @@ void virt_arch_pgd_alloc(struct kvm_vm *vm)
pgd_pages = (1UL << (RADIX_PGD_INDEX_SIZE + 3)) >> vm->page_shift;
if (!pgd_pages)
pgd_pages = 1;
- pgtb = vm_phy_pages_alloc_align(vm, pgd_pages, pgd_pages,
- KVM_GUEST_PAGE_TABLE_MIN_PADDR,
- vm->memslots[MEM_REGION_PT]);
+ pt = vm_alloc_page_table(vm, pgd_pages);
vm->pgd = pgtb;
/* Set the base page directory in the proc table */
@@ -168,9 +166,7 @@ void virt_arch_pg_map(struct kvm_vm *vm, uint64_t gva, uint64_t gpa)
pt_pages = (1ULL << (nls + 3)) >> vm->page_shift;
if (!pt_pages)
pt_pages = 1;
- pt = vm_phy_pages_alloc_align(vm, pt_pages, pt_pages,
- KVM_GUEST_PAGE_TABLE_MIN_PADDR,
- vm->memslots[MEM_REGION_PT]);
+ pt = vm_alloc_page_table(vm, pt_pages);
pde = PDE_VALID | nls | pt;
*pdep = cpu_to_be64(pde);
}
base-commit: 15a281f5c83f34d4d1808e5f790403b0770c5e78
--
next prev parent reply other threads:[~2023-06-28 18:24 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-08 4:00 [PATCH v2 0/4] KVM: selftests: add powerpc support Nicholas Piggin
2023-04-08 4:00 ` Nicholas Piggin
2023-04-08 4:00 ` [PATCH v2 1/4] KVM: selftests: Move pgd_created check into virt_pgd_alloc Nicholas Piggin
2023-04-08 4:00 ` Nicholas Piggin
2023-06-28 18:30 ` Sean Christopherson
2023-06-28 18:30 ` Sean Christopherson
2023-04-08 4:00 ` [PATCH v2 2/4] KVM: selftests: Add aligned guest physical page allocator Nicholas Piggin
2023-04-08 4:00 ` Nicholas Piggin
2023-06-28 18:24 ` Sean Christopherson [this message]
2023-06-28 18:24 ` Sean Christopherson
2023-04-08 4:00 ` [PATCH v2 3/4] KVM: PPC: selftests: add support for powerpc Nicholas Piggin
2023-04-08 4:00 ` Nicholas Piggin
2023-04-08 4:00 ` [PATCH v2 4/4] KVM: PPC: selftests: add selftests sanity tests Nicholas Piggin
2023-04-08 4:00 ` Nicholas Piggin
2023-04-11 5:57 ` [PATCH v2 0/4] KVM: selftests: add powerpc support Joel Stanley
2023-04-11 5:57 ` Joel Stanley
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=ZJx6wqgS4TOi/D4j@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
--cc=pbonzini@redhat.com \
--cc=shuah@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.