On Mon, May 16, 2022 at 1:32 PM Peter Xu <peterx@xxxxxxxxxx> wrote:
>
> On Fri, Apr 29, 2022 at 06:39:28PM +0000, David Matlack wrote:
> > The current EPT mapping code in the selftests only supports mapping 4K
> > pages. This commit extends that support with an option to map at 2M or
> > 1G. This will be used in a future commit to create large page mappings
> > to test eager page splitting.
> >
> > No functional change intended.
> >
> > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> > ---
> > tools/testing/selftests/kvm/lib/x86_64/vmx.c | 105 ++++++++++---------
> > 1 file changed, 57 insertions(+), 48 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/x86_64/vmx.c b/tools/testing/selftests/kvm/lib/x86_64/vmx.c
> > index d089d8b850b5..1fa2d1059ade 100644
> > --- a/tools/testing/selftests/kvm/lib/x86_64/vmx.c
> > +++ b/tools/testing/selftests/kvm/lib/x86_64/vmx.c
> > @@ -392,27 +392,60 @@ void nested_vmx_check_supported(void)
> > }
> > }
> >
> > -void nested_pg_map(struct vmx_pages *vmx, struct kvm_vm *vm,
> > - uint64_t nested_paddr, uint64_t paddr)
> > +static void nested_create_upper_pte(struct kvm_vm *vm,
> > + struct eptPageTableEntry *pte,
> > + uint64_t nested_paddr,
> > + uint64_t paddr,
> > + int current_level,
> > + int target_level)
> > +{
> > + if (!pte->readable) {
> > + pte->writable = true;
> > + pte->readable = true;
> > + pte->executable = true;
> > + pte->page_size = (current_level == target_level);
> > + if (pte->page_size)
> > + pte->address = paddr >> vm->page_shift;
> > + else
> > + 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.
> > + */
> > + TEST_ASSERT(current_level != target_level,
> > + "Cannot create hugepage at level: %u, nested_paddr: 0x%lx\n",
> > + current_level, nested_paddr);
> > + TEST_ASSERT(!pte->page_size,
> > + "Cannot create page table at level: %u, nested_paddr: 0x%lx\n",
> > + current_level, nested_paddr);
> > + }
> > +}
> > +
> > +
> > +void __nested_pg_map(struct vmx_pages *vmx, struct kvm_vm *vm,
> > + uint64_t nested_paddr, uint64_t paddr, int target_level)
> > {
> > + const uint64_t page_size = PG_LEVEL_SIZE(target_level);
> > + struct eptPageTableEntry *pt;
> > uint16_t index[4];
> > - struct eptPageTableEntry *pml4e;
> >
> > TEST_ASSERT(vm->mode == VM_MODE_PXXV48_4K, "Attempt to use "
> > "unknown or unsupported guest mode, mode: 0x%x", vm->mode);
> >
> > - TEST_ASSERT((nested_paddr % vm->page_size) == 0,
> > + TEST_ASSERT((nested_paddr % page_size) == 0,
> > "Nested physical address not on page boundary,\n"
> > - " nested_paddr: 0x%lx vm->page_size: 0x%x",
> > - nested_paddr, vm->page_size);
> > + " nested_paddr: 0x%lx page_size: 0x%lx",
> > + nested_paddr, page_size);
> > TEST_ASSERT((nested_paddr >> vm->page_shift) <= vm->max_gfn,
> > "Physical address beyond beyond maximum supported,\n"
> > " nested_paddr: 0x%lx vm->max_gfn: 0x%lx vm->page_size: 0x%x",
> > paddr, vm->max_gfn, vm->page_size);
> > - TEST_ASSERT((paddr % vm->page_size) == 0,
> > + TEST_ASSERT((paddr % page_size) == 0,
> > "Physical address not on page boundary,\n"
> > - " paddr: 0x%lx vm->page_size: 0x%x",
> > - paddr, vm->page_size);
> > + " paddr: 0x%lx page_size: 0x%lx",
> > + paddr, page_size);
> > TEST_ASSERT((paddr >> vm->page_shift) <= vm->max_gfn,
> > "Physical address beyond beyond maximum supported,\n"
> > " paddr: 0x%lx vm->max_gfn: 0x%lx vm->page_size: 0x%x",
> > @@ -423,49 +456,25 @@ void nested_pg_map(struct vmx_pages *vmx, struct kvm_vm *vm,
> > index[2] = (nested_paddr >> 30) & 0x1ffu;
> > index[3] = (nested_paddr >> 39) & 0x1ffu;
> >
> > - /* Allocate page directory pointer table if not present. */
> > - pml4e = vmx->eptp_hva;
> > - if (!pml4e[index[3]].readable) {
> > - pml4e[index[3]].address = vm_alloc_page_table(vm) >> vm->page_shift;
> > - pml4e[index[3]].writable = true;
> > - pml4e[index[3]].readable = true;
> > - pml4e[index[3]].executable = true;
> > - }
> > + pt = vmx->eptp_hva;
> >
> > - /* Allocate page directory table if not present. */
> > - struct eptPageTableEntry *pdpe;
> > - pdpe = addr_gpa2hva(vm, pml4e[index[3]].address * vm->page_size);
> > - if (!pdpe[index[2]].readable) {
> > - pdpe[index[2]].address = vm_alloc_page_table(vm) >> vm->page_shift;
> > - pdpe[index[2]].writable = true;
> > - pdpe[index[2]].readable = true;
> > - pdpe[index[2]].executable = true;
> > - }
> > + for (int current_level = 3; current_level >= 0; current_level--) {
> > + struct eptPageTableEntry *pte = &pt[index[current_level]];
> >
> > - /* Allocate page table if not present. */
> > - struct eptPageTableEntry *pde;
> > - pde = addr_gpa2hva(vm, pdpe[index[2]].address * vm->page_size);
> > - if (!pde[index[1]].readable) {
> > - pde[index[1]].address = vm_alloc_page_table(vm) >> vm->page_shift;
> > - pde[index[1]].writable = true;
> > - pde[index[1]].readable = true;
> > - pde[index[1]].executable = true;
> > - }
> > + nested_create_upper_pte(vm, pte, nested_paddr, paddr,
> > + current_level, target_level);
>
> This is going to run for the last level pte too, so maybe remove the
> "upper" prefix in the helper?
Good idea. Will do.
>
> >
> > - /* Fill in page table entry. */
> > - struct eptPageTableEntry *pte;
> > - pte = addr_gpa2hva(vm, pde[index[1]].address * vm->page_size);
> > - pte[index[0]].address = paddr >> vm->page_shift;
> > - pte[index[0]].writable = true;
> > - pte[index[0]].readable = true;
> > - pte[index[0]].executable = true;
> > + if (pte->page_size)
> > + break;
> >
> > - /*
> > - * For now mark these as accessed and dirty because the only
> > - * testcase we have needs that. Can be reconsidered later.
> > - */
> > - pte[index[0]].accessed = true;
> > - pte[index[0]].dirty = true;
>
> Is it intended to to drop the access/dirty bits here?
This was not intentional. Thanks for catching it!
>
> > + pt = addr_gpa2hva(vm, pte->address * vm->page_size);
> > + }
> > +}
> > +
> > +void nested_pg_map(struct vmx_pages *vmx, struct kvm_vm *vm,
> > + uint64_t nested_paddr, uint64_t paddr)
> > +{
> > + __nested_pg_map(vmx, vm, nested_paddr, paddr, PG_LEVEL_4K);
> > }
> >
> > /*
> > --
> > 2.36.0.464.gb9c8b46e94-goog
> >
>
> --
> Peter Xu
>
