diff mbox series

[v1,2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48

Message ID 20230228170756.769461-3-ryan.roberts@arm.com
State Superseded
Headers show
Series KVM: selftests: Fixes for broken tests | expand

Commit Message

Ryan Roberts Feb. 28, 2023, 5:07 p.m. UTC
The high bits [51:48] of a physical address should appear at [15:12] in
a 64K pte, not at [51:48] as was previously being programmed. Fix this
with new helper functions that do the conversion correctly. This also
sets us up nicely for adding LPA2 encodings in future.

Fixes: 7a6629ef746d ("kvm: selftests: add virt mem support for aarch64")

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 .../selftests/kvm/lib/aarch64/processor.c     | 32 ++++++++++++++-----
 1 file changed, 24 insertions(+), 8 deletions(-)

Comments

Oliver Upton March 7, 2023, 5:44 p.m. UTC | #1
Hi Ryan,

Thanks for fixing this. Couple of nits:

On Tue, Feb 28, 2023 at 05:07:56PM +0000, Ryan Roberts wrote:
> The high bits [51:48] of a physical address should appear at [15:12] in
> a 64K pte, not at [51:48] as was previously being programmed. Fix this
> with new helper functions that do the conversion correctly. This also
> sets us up nicely for adding LPA2 encodings in future.
> 
> Fixes: 7a6629ef746d ("kvm: selftests: add virt mem support for aarch64")
> 
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>

nit: no whitespace between footers.

> ---
>  .../selftests/kvm/lib/aarch64/processor.c     | 32 ++++++++++++++-----
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 5972a23b2765..13f28d96521c 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -58,10 +58,27 @@ static uint64_t pte_index(struct kvm_vm *vm, vm_vaddr_t gva)
>  	return (gva >> vm->page_shift) & mask;
>  }
>  
> -static uint64_t pte_addr(struct kvm_vm *vm, uint64_t entry)
> +static uint64_t addr_pte(struct kvm_vm *vm, uint64_t pa, uint64_t attrs)
>  {
> -	uint64_t mask = ((1UL << (vm->va_bits - vm->page_shift)) - 1) << vm->page_shift;
> -	return entry & mask;
> +	uint64_t pte;
> +
> +	pte = pa & GENMASK(47, vm->page_shift);
> +	if (vm->page_shift == 16)
> +		pte |= (pa & GENMASK(51, 48)) >> (48 - 12);

nit: this is a bit of an odd transformation, of course courtesy of the
architecture. FIELD_GET() might make it a bit more readable:

		pte |= FIELD_GET(GENMASK(51, 48), pa) << 12;
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 5972a23b2765..13f28d96521c 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -58,10 +58,27 @@  static uint64_t pte_index(struct kvm_vm *vm, vm_vaddr_t gva)
 	return (gva >> vm->page_shift) & mask;
 }
 
-static uint64_t pte_addr(struct kvm_vm *vm, uint64_t entry)
+static uint64_t addr_pte(struct kvm_vm *vm, uint64_t pa, uint64_t attrs)
 {
-	uint64_t mask = ((1UL << (vm->va_bits - vm->page_shift)) - 1) << vm->page_shift;
-	return entry & mask;
+	uint64_t pte;
+
+	pte = pa & GENMASK(47, vm->page_shift);
+	if (vm->page_shift == 16)
+		pte |= (pa & GENMASK(51, 48)) >> (48 - 12);
+	pte |= attrs;
+
+	return pte;
+}
+
+static uint64_t pte_addr(struct kvm_vm *vm, uint64_t pte)
+{
+	uint64_t pa;
+
+	pa = pte & GENMASK(47, vm->page_shift);
+	if (vm->page_shift == 16)
+		pa |= (pte & GENMASK(15, 12)) << (48 - 12);
+
+	return pa;
 }
 
 static uint64_t ptrs_per_pgd(struct kvm_vm *vm)
@@ -110,18 +127,18 @@  static void _virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
 
 	ptep = addr_gpa2hva(vm, vm->pgd) + pgd_index(vm, vaddr) * 8;
 	if (!*ptep)
-		*ptep = vm_alloc_page_table(vm) | 3;
+		*ptep = addr_pte(vm, vm_alloc_page_table(vm), 3);
 
 	switch (vm->pgtable_levels) {
 	case 4:
 		ptep = addr_gpa2hva(vm, pte_addr(vm, *ptep)) + pud_index(vm, vaddr) * 8;
 		if (!*ptep)
-			*ptep = vm_alloc_page_table(vm) | 3;
+			*ptep = addr_pte(vm, vm_alloc_page_table(vm), 3);
 		/* fall through */
 	case 3:
 		ptep = addr_gpa2hva(vm, pte_addr(vm, *ptep)) + pmd_index(vm, vaddr) * 8;
 		if (!*ptep)
-			*ptep = vm_alloc_page_table(vm) | 3;
+			*ptep = addr_pte(vm, vm_alloc_page_table(vm), 3);
 		/* fall through */
 	case 2:
 		ptep = addr_gpa2hva(vm, pte_addr(vm, *ptep)) + pte_index(vm, vaddr) * 8;
@@ -130,8 +147,7 @@  static void _virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
 		TEST_FAIL("Page table levels must be 2, 3, or 4");
 	}
 
-	*ptep = paddr | 3;
-	*ptep |= (attr_idx << 2) | (1 << 10) /* Access Flag */;
+	*ptep = addr_pte(vm, paddr, (attr_idx << 2) | (1 << 10) | 3);  /* AF */
 }
 
 void virt_arch_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr)