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?
>
> - /* 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?
> + 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
