* [PATCH v1 0/2] KVM: selftests: Fixes for broken tests
@ 2023-02-28 17:07 Ryan Roberts
2023-02-28 17:07 ` [PATCH v1 1/2] KVM: selftests: Fixup config fragment for access_tracking_perf_test Ryan Roberts
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ryan Roberts @ 2023-02-28 17:07 UTC (permalink / raw
To: Marc Zyngier, Oliver Upton
Cc: Ryan Roberts, Paolo Bonzini, Shuah Khan, linux-kselftest, kvmarm
During the course of implementing FEAT_LPA2 within the arm64 KVM port, I found a
couple of issues within the KVM selftest code, which I thought were worth
posting independently. The LPA2 patches, for which I will post v2 in the next
few days, depend on these fixes for its testing.
Ryan Roberts (2):
KVM: selftests: Fixup config fragment for access_tracking_perf_test
KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48
tools/testing/selftests/kvm/config | 1 +
.../selftests/kvm/lib/aarch64/processor.c | 32 ++++++++++++++-----
2 files changed, 25 insertions(+), 8 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/2] KVM: selftests: Fixup config fragment for access_tracking_perf_test
2023-02-28 17:07 [PATCH v1 0/2] KVM: selftests: Fixes for broken tests Ryan Roberts
@ 2023-02-28 17:07 ` Ryan Roberts
2023-02-28 17:07 ` [PATCH v1 2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48 Ryan Roberts
2023-03-02 15:27 ` [PATCH v1 0/2] KVM: selftests: Fixes for broken tests Ryan Roberts
2 siblings, 0 replies; 6+ messages in thread
From: Ryan Roberts @ 2023-02-28 17:07 UTC (permalink / raw
To: Marc Zyngier, Oliver Upton
Cc: Ryan Roberts, Paolo Bonzini, Shuah Khan, linux-kselftest, kvmarm
access_tracking_perf_test requires CONFIG_IDLE_PAGE_TRACKING. However
this is missing from the config fragment, so add it in so that this test
is no longer skipped.
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
tools/testing/selftests/kvm/config | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/config b/tools/testing/selftests/kvm/config
index d011b38e259e..8835fed09e9f 100644
--- a/tools/testing/selftests/kvm/config
+++ b/tools/testing/selftests/kvm/config
@@ -2,3 +2,4 @@ CONFIG_KVM=y
CONFIG_KVM_INTEL=y
CONFIG_KVM_AMD=y
CONFIG_USERFAULTFD=y
+CONFIG_IDLE_PAGE_TRACKING=y
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48
2023-02-28 17:07 [PATCH v1 0/2] KVM: selftests: Fixes for broken tests Ryan Roberts
2023-02-28 17:07 ` [PATCH v1 1/2] KVM: selftests: Fixup config fragment for access_tracking_perf_test Ryan Roberts
@ 2023-02-28 17:07 ` Ryan Roberts
2023-03-07 17:44 ` Oliver Upton
2023-03-02 15:27 ` [PATCH v1 0/2] KVM: selftests: Fixes for broken tests Ryan Roberts
2 siblings, 1 reply; 6+ messages in thread
From: Ryan Roberts @ 2023-02-28 17:07 UTC (permalink / raw
To: Marc Zyngier, Oliver Upton
Cc: Ryan Roberts, Paolo Bonzini, Shuah Khan, linux-kselftest, kvmarm
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(-)
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)
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 0/2] KVM: selftests: Fixes for broken tests
2023-02-28 17:07 [PATCH v1 0/2] KVM: selftests: Fixes for broken tests Ryan Roberts
2023-02-28 17:07 ` [PATCH v1 1/2] KVM: selftests: Fixup config fragment for access_tracking_perf_test Ryan Roberts
2023-02-28 17:07 ` [PATCH v1 2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48 Ryan Roberts
@ 2023-03-02 15:27 ` Ryan Roberts
2 siblings, 0 replies; 6+ messages in thread
From: Ryan Roberts @ 2023-03-02 15:27 UTC (permalink / raw
To: Marc Zyngier, Oliver Upton
Cc: Paolo Bonzini, Shuah Khan, linux-kselftest, kvmarm
On 28/02/2023 17:07, Ryan Roberts wrote:
> During the course of implementing FEAT_LPA2 within the arm64 KVM port, I found a
> couple of issues within the KVM selftest code, which I thought were worth
> posting independently. The LPA2 patches, for which I will post v2 in the next
> few days, depend on these fixes for its testing.
It turned out that there was another issue, for which I have posted a fix at
https://lore.kernel.org/kvmarm/20230302152033.242073-1-ryan.roberts@arm.com/
With these 3 fixes, I see a clean run of the kvm selftests with the 64K kernel +
FEAT_LPA on FVP.
Thanks,
Ryan
>
> Ryan Roberts (2):
> KVM: selftests: Fixup config fragment for access_tracking_perf_test
> KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48
>
> tools/testing/selftests/kvm/config | 1 +
> .../selftests/kvm/lib/aarch64/processor.c | 32 ++++++++++++++-----
> 2 files changed, 25 insertions(+), 8 deletions(-)
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48
2023-02-28 17:07 ` [PATCH v1 2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48 Ryan Roberts
@ 2023-03-07 17:44 ` Oliver Upton
2023-03-08 11:18 ` Ryan Roberts
0 siblings, 1 reply; 6+ messages in thread
From: Oliver Upton @ 2023-03-07 17:44 UTC (permalink / raw
To: Ryan Roberts
Cc: Marc Zyngier, Paolo Bonzini, Shuah Khan, linux-kselftest, kvmarm
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;
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48
2023-03-07 17:44 ` Oliver Upton
@ 2023-03-08 11:18 ` Ryan Roberts
0 siblings, 0 replies; 6+ messages in thread
From: Ryan Roberts @ 2023-03-08 11:18 UTC (permalink / raw
To: Oliver Upton
Cc: Marc Zyngier, Paolo Bonzini, Shuah Khan, linux-kselftest, kvmarm
On 07/03/2023 17:44, Oliver Upton wrote:
> 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.
Sorry my bad; I'm slowly learning the conventions - I'll get there eventually!
>
>> ---
>> .../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;
>
Ahh yes, that does look better. I did consider this originally, but thought I
would also need FIELD_PREP() which selftests is not currently using anywhere. So
thought I would steer clear entirely. Anyway, on review, I don't need
FIELD_PREP(), so I've just sent you a respun series using FIELD_GET() in all the
sensible places. I hope I'm not jumping the gun by respinning so quickly - I
didn't think the series was particularly controversial so unlikely to get any
more comments.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-08 11:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-28 17:07 [PATCH v1 0/2] KVM: selftests: Fixes for broken tests Ryan Roberts
2023-02-28 17:07 ` [PATCH v1 1/2] KVM: selftests: Fixup config fragment for access_tracking_perf_test Ryan Roberts
2023-02-28 17:07 ` [PATCH v1 2/2] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48 Ryan Roberts
2023-03-07 17:44 ` Oliver Upton
2023-03-08 11:18 ` Ryan Roberts
2023-03-02 15:27 ` [PATCH v1 0/2] KVM: selftests: Fixes for broken tests Ryan Roberts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.