[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aO1yJHcKC85mo0PQ@google.com>
Date: Mon, 13 Oct 2025 14:41:56 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
Yosry Ahmed <yosryahmed@...gle.com>
Subject: Re: [PATCH 08/12] KVM: selftests: Use 'leaf' instead of hugepage to
describe EPT entries
On Wed, Oct 01, 2025, Yosry Ahmed wrote:
> From: Yosry Ahmed <yosryahmed@...gle.com>
>
> The assertions use 'hugepage' to describe a terminal EPT entry, but
> 'leaf' is more accruate as a PG_LEVEL_4K EPT entry is a leaf but not a
> hugepage.
Yes, it's more accurate, but also less precise. I'm guessing the assert message
and comment talked about hugepages because that's the type of mappings that
caused problems at the time.
Ah, actually, I bet the code was copy+pasted from virt_create_upper_pte(), in
which case the assumptions about wanting to create a hupage are both accurate
and precise.
> The distincion will be useful in coming changes that will pass
> the value around and 'leaf' is clearer than hugepage or page_size.
What value?
> Leave the EPT bit named page_size to keep it conforming to the manual.
>
> Signed-off-by: Yosry Ahmed <yosry.ahmed@...ux.dev>
> ---
> tools/testing/selftests/kvm/lib/x86/vmx.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/x86/vmx.c b/tools/testing/selftests/kvm/lib/x86/vmx.c
> index 04c4b97bcd1e7..673756b27e903 100644
> --- a/tools/testing/selftests/kvm/lib/x86/vmx.c
> +++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
> @@ -380,15 +380,15 @@ static void nested_create_pte(struct kvm_vm *vm,
> pte->address = vm_alloc_page_table(vm) >> vm->page_shift;
> } else {
> /*
> - * Entry already present. Assert that the caller doesn't want
> - * a hugepage at this level, and that there isn't a hugepage at
> - * this level.
> + * Entry already present. Assert that the caller doesn't want a
> + * leaf entry at this level, and that there isn't a leaf entry
> + * at this level.
> */
> TEST_ASSERT(current_level != target_level,
> - "Cannot create hugepage at level: %u, nested_paddr: 0x%lx",
> + "Cannot create leaf entry at level: %u, nested_paddr: 0x%lx",
> current_level, nested_paddr);
> TEST_ASSERT(!pte->page_size,
> - "Cannot create page table at level: %u, nested_paddr: 0x%lx",
> + "Leaf entry already exists at level: %u, nested_paddr: 0x%lx",
This change is flat out wrong. The existing PRESENT PTE _might_ be a 4KiB leaf
entry, but it might also be an existing non-leaf page table.
Instead of hacking on the nested code, can we instead tweak __virt_pg_map() to
work with nested TDP? At a glance, it's already quite close, e.g. "just" needs
to be taught about EPT RWX bits and allow the call to pass in the root pointer.
> current_level, nested_paddr);
> }
> }
> --
> 2.51.0.618.g983fd99d29-goog
>
Powered by blists - more mailing lists