Message ID | 20220829171021.701198-5-pgonda@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: selftests: Add simple SEV test | expand |
On Mon, Aug 29, 2022, Peter Gonda wrote: > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > index 53b9a509c1d5..de13be62d52d 100644 > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > @@ -1388,6 +1388,58 @@ void virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, > } > } > > +/* > + * Mask off any special bits from raw GPA > + * > + * Input Args: > + * vm - Virtual Machine > + * gpa_raw - Raw VM physical address > + * > + * Output Args: None > + * > + * Return: > + * GPA with special bits (e.g. shared/encrypted) masked off. > + */ > +vm_paddr_t addr_raw2gpa(struct kvm_vm *vm, vm_paddr_t gpa_raw) > +{ > + if (!vm->memcrypt.has_enc_bit) > + return gpa_raw; > + > + return gpa_raw & ~(1ULL << vm->memcrypt.enc_bit); 1. The notion of stealing GPA bits to tag the page should not be tied to memory encryption. 2. Assuming that the shared vs. private bit is active-high is wrong, e.g. TDX has active-low behavior. 3. "raw" is not super untuitive. 4. addr_gpa2raw() should not exist. It assumes the "raw" address always wants the encryption bit set. 5. enc_by_default is an SEV-centric flag that should not exist outside of x86. I think the easiest and most familiar solution for #1 will be to borrow the kernel's tag/untag terminology for handling virtual addresses that can be tagged for ASAN, e.g. ARM's top-byte-ignore and x86's linear address masking (LAM). So instead of addr_raw2gpa(), just do: static inline vm_paddr_t vm_untag_gpa(struct kvm_vm *vm, vm_vaddr_t gpa) { return gpa & ~vm->gpa_tag_mask; } That way zero-allocating the VM will Just Work. > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > index 2e6e61bbe81b..b2df259ce706 100644 > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > @@ -118,7 +118,7 @@ void virt_arch_pgd_alloc(struct kvm_vm *vm) > > /* If needed, create page map l4 table. */ > if (!vm->pgd_created) { > - vm->pgd = vm_alloc_page_table(vm); > + vm->pgd = addr_gpa2raw(vm, vm_alloc_page_table(vm)); Rather than add "struct vm_memcrypt", I think it makes sense to introduce "struct kvm_vm_arch" and then the x86 implementation can add pte_me_mask, similar to what KVM does for SPTEs. THen this code because vm->pgd = vm_alloc_page_table(vm) | vm->arch.pte_me_mask; That will Just Work for TDX, because its pte_me_mask will be '0', even though it effectively has an encryption bit (active-low). > vm->pgd_created = true; > } > } > @@ -140,13 +140,15 @@ static uint64_t *virt_create_upper_pte(struct kvm_vm *vm, > int target_level) > { > uint64_t *pte = virt_get_pte(vm, pt_pfn, vaddr, current_level); > + uint64_t paddr_raw = addr_gpa2raw(vm, paddr); > > if (!(*pte & PTE_PRESENT_MASK)) { > *pte = PTE_PRESENT_MASK | PTE_WRITABLE_MASK; > if (current_level == target_level) > - *pte |= PTE_LARGE_MASK | (paddr & PHYSICAL_PAGE_MASK); > + *pte |= PTE_LARGE_MASK | (paddr_raw & PHYSICAL_PAGE_MASK); > else > - *pte |= vm_alloc_page_table(vm) & PHYSICAL_PAGE_MASK; > + *pte |= addr_gpa2raw(vm, vm_alloc_page_table(vm)) & PHYSICAL_PAGE_MASK; Prefer to write this as: *pte |= vm_alloc_page_table(vm) & PHYSICAL_PAGE_MASK; *pte |= vm->arch.pte_me_mask; so that selftests don't assume the encryption bit is stolen from the GPA. > + > } else { > /* > * Entry already present. Assert that the caller doesn't want > @@ -184,6 +186,8 @@ void __virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, int level) > "Physical address beyond maximum supported,\n" > " paddr: 0x%lx vm->max_gfn: 0x%lx vm->page_size: 0x%x", > paddr, vm->max_gfn, vm->page_size); > + TEST_ASSERT(addr_raw2gpa(vm, paddr) == paddr, > + "Unexpected bits in paddr: %lx", paddr); > > /* > * Allocate upper level page tables, if not already present. Return > @@ -206,7 +210,8 @@ void __virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, int level) > pte = virt_get_pte(vm, PTE_GET_PFN(*pde), vaddr, PG_LEVEL_4K); > TEST_ASSERT(!(*pte & PTE_PRESENT_MASK), > "PTE already present for 4k page at vaddr: 0x%lx\n", vaddr); > - *pte = PTE_PRESENT_MASK | PTE_WRITABLE_MASK | (paddr & PHYSICAL_PAGE_MASK); > + *pte = PTE_PRESENT_MASK | PTE_WRITABLE_MASK | > + (addr_gpa2raw(vm, paddr) & PHYSICAL_PAGE_MASK); And with the above suggestions, this can become something like: if (vm_is_gpa_encrypted(vm, paddr)) *pte |= vm->arch.c_bit; else *pte |= vm->arch.s_bit; where SEV sets c_bit and TDX sets s_bit. > void virt_arch_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr) > @@ -515,7 +520,7 @@ vm_paddr_t addr_arch_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva) > if (!(pte[index[0]] & PTE_PRESENT_MASK)) > goto unmapped_gva; > > - return (PTE_GET_PFN(pte[index[0]]) * vm->page_size) + (gva & ~PAGE_MASK); > + return addr_raw2gpa(vm, PTE_GET_PFN(pte[index[0]]) * vm->page_size) + (gva & ~PAGE_MASK); Aha! A use for my rework[*] of this mess! I don't think you need to take a dependency on that work, at a glance the conflicts should be minor, i.e. doesn't matter that much which lands first. [*] https://lore.kernel.org/all/20221006004512.666529-1-seanjc@google.com
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h index 5ecde5ad4c2f..dda8467d1434 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_base.h +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h @@ -401,6 +401,8 @@ void *addr_gpa2hva(struct kvm_vm *vm, vm_paddr_t gpa); void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva); vm_paddr_t addr_hva2gpa(struct kvm_vm *vm, void *hva); void *addr_gpa2alias(struct kvm_vm *vm, vm_paddr_t gpa); +vm_paddr_t addr_raw2gpa(struct kvm_vm *vm, vm_vaddr_t gpa_raw); +vm_paddr_t addr_gpa2raw(struct kvm_vm *vm, vm_vaddr_t gpa); void vcpu_run(struct kvm_vcpu *vcpu); int _vcpu_run(struct kvm_vcpu *vcpu); diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index 53b9a509c1d5..de13be62d52d 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -1388,6 +1388,58 @@ void virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, } } +/* + * Mask off any special bits from raw GPA + * + * Input Args: + * vm - Virtual Machine + * gpa_raw - Raw VM physical address + * + * Output Args: None + * + * Return: + * GPA with special bits (e.g. shared/encrypted) masked off. + */ +vm_paddr_t addr_raw2gpa(struct kvm_vm *vm, vm_paddr_t gpa_raw) +{ + if (!vm->memcrypt.has_enc_bit) + return gpa_raw; + + return gpa_raw & ~(1ULL << vm->memcrypt.enc_bit); +} + +/* + * Add special/encryption bits to a GPA based on encryption bitmap. + * + * Input Args: + * vm - Virtual Machine + * gpa - VM physical address + * + * Output Args: None + * + * Return: + * GPA with special bits (e.g. shared/encrypted) added in. + */ +vm_paddr_t addr_gpa2raw(struct kvm_vm *vm, vm_paddr_t gpa) +{ + struct userspace_mem_region *region; + sparsebit_idx_t pg; + vm_paddr_t gpa_raw = gpa; + + TEST_ASSERT(addr_raw2gpa(vm, gpa) == gpa, "Unexpected bits in GPA: %lx", + gpa); + + if (!vm->memcrypt.has_enc_bit) + return gpa; + + region = userspace_mem_region_find(vm, gpa, gpa); + pg = gpa >> vm->page_shift; + if (sparsebit_is_set(region->encrypted_phy_pages, pg)) + gpa_raw |= (1ULL << vm->memcrypt.enc_bit); + + return gpa_raw; +} + /* * Address VM Physical to Host Virtual * @@ -1405,9 +1457,10 @@ void virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, * address providing the memory to the vm physical address is returned. * A TEST_ASSERT failure occurs if no region containing gpa exists. */ -void *addr_gpa2hva(struct kvm_vm *vm, vm_paddr_t gpa) +void *addr_gpa2hva(struct kvm_vm *vm, vm_paddr_t gpa_raw) { struct userspace_mem_region *region; + vm_paddr_t gpa = addr_raw2gpa(vm, gpa_raw); region = userspace_mem_region_find(vm, gpa, gpa); if (!region) { diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index 2e6e61bbe81b..b2df259ce706 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -118,7 +118,7 @@ void virt_arch_pgd_alloc(struct kvm_vm *vm) /* If needed, create page map l4 table. */ if (!vm->pgd_created) { - vm->pgd = vm_alloc_page_table(vm); + vm->pgd = addr_gpa2raw(vm, vm_alloc_page_table(vm)); vm->pgd_created = true; } } @@ -140,13 +140,15 @@ static uint64_t *virt_create_upper_pte(struct kvm_vm *vm, int target_level) { uint64_t *pte = virt_get_pte(vm, pt_pfn, vaddr, current_level); + uint64_t paddr_raw = addr_gpa2raw(vm, paddr); if (!(*pte & PTE_PRESENT_MASK)) { *pte = PTE_PRESENT_MASK | PTE_WRITABLE_MASK; if (current_level == target_level) - *pte |= PTE_LARGE_MASK | (paddr & PHYSICAL_PAGE_MASK); + *pte |= PTE_LARGE_MASK | (paddr_raw & PHYSICAL_PAGE_MASK); else - *pte |= vm_alloc_page_table(vm) & PHYSICAL_PAGE_MASK; + *pte |= addr_gpa2raw(vm, vm_alloc_page_table(vm)) & PHYSICAL_PAGE_MASK; + } else { /* * Entry already present. Assert that the caller doesn't want @@ -184,6 +186,8 @@ void __virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, int level) "Physical address beyond maximum supported,\n" " paddr: 0x%lx vm->max_gfn: 0x%lx vm->page_size: 0x%x", paddr, vm->max_gfn, vm->page_size); + TEST_ASSERT(addr_raw2gpa(vm, paddr) == paddr, + "Unexpected bits in paddr: %lx", paddr); /* * Allocate upper level page tables, if not already present. Return @@ -206,7 +210,8 @@ void __virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, int level) pte = virt_get_pte(vm, PTE_GET_PFN(*pde), vaddr, PG_LEVEL_4K); TEST_ASSERT(!(*pte & PTE_PRESENT_MASK), "PTE already present for 4k page at vaddr: 0x%lx\n", vaddr); - *pte = PTE_PRESENT_MASK | PTE_WRITABLE_MASK | (paddr & PHYSICAL_PAGE_MASK); + *pte = PTE_PRESENT_MASK | PTE_WRITABLE_MASK | + (addr_gpa2raw(vm, paddr) & PHYSICAL_PAGE_MASK); } void virt_arch_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr) @@ -515,7 +520,7 @@ vm_paddr_t addr_arch_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva) if (!(pte[index[0]] & PTE_PRESENT_MASK)) goto unmapped_gva; - return (PTE_GET_PFN(pte[index[0]]) * vm->page_size) + (gva & ~PAGE_MASK); + return addr_raw2gpa(vm, PTE_GET_PFN(pte[index[0]]) * vm->page_size) + (gva & ~PAGE_MASK); unmapped_gva: TEST_FAIL("No mapping for vm virtual address, gva: 0x%lx", gva);