On Wed, Sep 21, 2022, Vitaly Kuznetsov wrote:
> Make it possible for tests to mangle guest's page table entries in
> addition to just getting them (available with vm_get_page_table_entry()).
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
> tools/testing/selftests/kvm/include/x86_64/processor.h | 2 ++
> tools/testing/selftests/kvm/lib/x86_64/processor.c | 5 ++---
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 1c7805de8c27..500d711eb989 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -827,6 +827,8 @@ static inline uint8_t wrmsr_safe(uint32_t msr, uint64_t val)
> return kvm_asm_safe("wrmsr", "a"(val & -1u), "d"(val >> 32), "c"(msr));
> }
>
> +uint64_t *_vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> + uint64_t vaddr);
> uint64_t vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> uint64_t vaddr);
> void vm_set_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> index 2e6e61bbe81b..5c135f896ada 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> @@ -214,9 +214,8 @@ void virt_arch_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr)
> __virt_pg_map(vm, vaddr, paddr, PG_LEVEL_4K);
> }
>
> -static uint64_t *_vm_get_page_table_entry(struct kvm_vm *vm,
> - struct kvm_vcpu *vcpu,
> - uint64_t vaddr)
> +uint64_t *_vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> + uint64_t vaddr)
Ugh, obviously not your fault, but this is a terrible name. Aside from using a
single underscore, it's semantically very different than vm_get_page_table_entry(),
i.e. violates the stand "double underscores is an inner helper".
The innards of vm_{g,s}et_page_table_entry() are quite hilarious too as they cast
a "uint64_t *" to "uint64_t*" now that KVM no longer uses structs to manage PTEs
(commit f18b4aebe107 ("kvm: selftests: do not use bitfields larger than 32-bits
for PTEs")).
And looking at the sole usage in emulator_error_test.c, provide get+set helpers
is silly.
Rather than expose this weirdness, what about slotting in the below to drop the
wrappers and just let tests modify PTEs directly?
---
From: Sean Christopherson <seanjc@xxxxxxxxxx>
Date: Wed, 21 Sep 2022 15:08:49 -0700
Subject: [PATCH] KVM: selftests: Drop helpers to read/write page table entries
Drop vm_{g,s}et_page_table_entry() and instead expose the "inner"
helper (was _vm_get_page_table_entry()) that returns a _pointer_ to the
PTE, i.e. let tests directly modify PTEs instead of bouncing through
helpers that just make life difficult.
Opportunsitically use BIT_ULL() in emulator_error_test, and use the
MAXPHYADDR define to set the "rogue" GPA bit instead of open coding the
same value.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
---
.../selftests/kvm/include/x86_64/processor.h | 6 ++----
.../selftests/kvm/lib/x86_64/processor.c | 21 ++-----------------
.../kvm/x86_64/emulator_error_test.c | 6 ++++--
3 files changed, 8 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 0cbc71b7af50..5999e974a150 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -825,10 +825,8 @@ static inline uint8_t wrmsr_safe(uint32_t msr, uint64_t val)
return kvm_asm_safe("wrmsr", "a"(val & -1u), "d"(val >> 32), "c"(msr));
}
-uint64_t vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
- uint64_t vaddr);
-void vm_set_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
- uint64_t vaddr, uint64_t pte);
+uint64_t *vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
+ uint64_t vaddr);
uint64_t kvm_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2,
uint64_t a3);
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 2e6e61bbe81b..5e4bbe71dbff 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -214,9 +214,8 @@ void virt_arch_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr)
__virt_pg_map(vm, vaddr, paddr, PG_LEVEL_4K);
}
-static uint64_t *_vm_get_page_table_entry(struct kvm_vm *vm,
- struct kvm_vcpu *vcpu,
- uint64_t vaddr)
+uint64_t *vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
+ uint64_t vaddr)
{
uint16_t index[4];
uint64_t *pml4e, *pdpe, *pde;
@@ -286,22 +285,6 @@ static uint64_t *_vm_get_page_table_entry(struct kvm_vm *vm,
return &pte[index[0]];
}
-uint64_t vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
- uint64_t vaddr)
-{
- uint64_t *pte = _vm_get_page_table_entry(vm, vcpu, vaddr);
-
- return *(uint64_t *)pte;
-}
-
-void vm_set_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
- uint64_t vaddr, uint64_t pte)
-{
- uint64_t *new_pte = _vm_get_page_table_entry(vm, vcpu, vaddr);
-
- *(uint64_t *)new_pte = pte;
-}
-
void virt_arch_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
{
uint64_t *pml4e, *pml4e_start;
diff --git a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
index 236e11755ba6..bde247f3c8a1 100644
--- a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
+++ b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
@@ -152,8 +152,9 @@ int main(int argc, char *argv[])
{
struct kvm_vcpu *vcpu;
struct kvm_vm *vm;
- uint64_t gpa, pte;
+ uint64_t *pte;
uint64_t *hva;
+ uint64_t gpa;
int rc;
/* Tell stdout not to buffer its content */
@@ -178,8 +179,9 @@ int main(int argc, char *argv[])
virt_map(vm, MEM_REGION_GVA, MEM_REGION_GPA, 1);
hva = addr_gpa2hva(vm, MEM_REGION_GPA);
memset(hva, 0, PAGE_SIZE);
+
pte = vm_get_page_table_entry(vm, vcpu, MEM_REGION_GVA);
- vm_set_page_table_entry(vm, vcpu, MEM_REGION_GVA, pte | (1ull << 36));
+ *pte |= BIT_ULL(MAXPHYADDR);
vcpu_run(vcpu);
process_exit_on_emulation_error(vcpu);
base-commit: 3b69d246e2f1eef553508c79f5d3b2dfc4978bc1
--
