| Message ID | 20221018214612.3445074-8-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: Fix and clean up emulator_error_test | expand |
On Tue, Oct 18, 2022, David Matlack wrote: > @@ -50,6 +73,9 @@ int main(int argc, char *argv[]) > TEST_REQUIRE(kvm_has_cap(KVM_CAP_SMALLER_MAXPHYADDR)); > > vm = vm_create_with_one_vcpu(&vcpu, guest_code); > + vm_init_descriptor_tables(vm); > + vcpu_init_descriptor_tables(vcpu); > + vm_install_exception_handler(vm, PF_VECTOR, guest_page_fault_handler); Instead of installing an exception handler, u8 vector = kvm_asm_safe(KVM_ASM_SAFE(FLDS_MEM_EAX), "a"(MEM_REGION_GVA)); then the guest/test can provide more precise information if a #PF doesn't occur.
On Thu, Oct 27, 2022 at 5:31 PM Sean Christopherson <seanjc@google.com> wrote: > > On Tue, Oct 18, 2022, David Matlack wrote: > > @@ -50,6 +73,9 @@ int main(int argc, char *argv[]) > > TEST_REQUIRE(kvm_has_cap(KVM_CAP_SMALLER_MAXPHYADDR)); > > > > vm = vm_create_with_one_vcpu(&vcpu, guest_code); > > + vm_init_descriptor_tables(vm); > > + vcpu_init_descriptor_tables(vcpu); > > + vm_install_exception_handler(vm, PF_VECTOR, guest_page_fault_handler); > > Instead of installing an exception handler, > > u8 vector = kvm_asm_safe(KVM_ASM_SAFE(FLDS_MEM_EAX), > "a"(MEM_REGION_GVA)); > > then the guest/test can provide more precise information if a #PF doesn't occur. I gave this a shot and realized this would prevent checking that it is a reserved #PF, since kvm_asm_safe() only returns the vector. It's probably more important to have more precise testing rather than more precise failure reporting. But what did you have in mind specifically? Maybe there's another way.
On Fri, Oct 28, 2022, David Matlack wrote: > On Thu, Oct 27, 2022 at 5:31 PM Sean Christopherson <seanjc@google.com> wrote: > > > > On Tue, Oct 18, 2022, David Matlack wrote: > > > @@ -50,6 +73,9 @@ int main(int argc, char *argv[]) > > > TEST_REQUIRE(kvm_has_cap(KVM_CAP_SMALLER_MAXPHYADDR)); > > > > > > vm = vm_create_with_one_vcpu(&vcpu, guest_code); > > > + vm_init_descriptor_tables(vm); > > > + vcpu_init_descriptor_tables(vcpu); > > > + vm_install_exception_handler(vm, PF_VECTOR, guest_page_fault_handler); > > > > Instead of installing an exception handler, > > > > u8 vector = kvm_asm_safe(KVM_ASM_SAFE(FLDS_MEM_EAX), > > "a"(MEM_REGION_GVA)); > > > > then the guest/test can provide more precise information if a #PF doesn't occur. > > I gave this a shot and realized this would prevent checking that it is > a reserved #PF, since kvm_asm_safe() only returns the vector. > > It's probably more important to have more precise testing rather than > more precise failure reporting. But what did you have in mind > specifically? Maybe there's another way. I didn't have anything in mind, I just overlooked the error code. That said, this is a good excuse to expand KVM_ASM_SAFE() to provide the error code. There's already a clobbered scratch register (two, actually) that can be used to propagate the error code from the exception handler back to the asm blob, so it's easy to squeeze into the existing framework. Patches attached.
diff --git a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c index 91a85a00b692..afa9e0b3dd8a 100644 --- a/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c +++ b/tools/testing/selftests/kvm/x86_64/smaller_maxphyaddr_emulation_test.c @@ -21,6 +21,12 @@ #define MEM_REGION_SLOT 10 #define MEM_REGION_SIZE PAGE_SIZE +static void guest_page_fault_handler(struct ex_regs *regs) +{ + GUEST_ASSERT(regs->error_code & PFERR_RSVD_MASK); + GUEST_SYNC(PF_VECTOR); +} + static void guest_code(void) { flds(MEM_REGION_GVA); @@ -36,6 +42,23 @@ static void assert_ucall_done(struct kvm_vcpu *vcpu) uc.cmd, UCALL_DONE); } +static void assert_reserved_page_fault(struct kvm_vcpu *vcpu) +{ + struct ucall uc; + + switch (get_ucall(vcpu, &uc)) { + case UCALL_ABORT: + REPORT_GUEST_ASSERT(uc); + break; + case UCALL_SYNC: + TEST_ASSERT(uc.args[1] == PF_VECTOR, + "Unexpected UCALL_SYNC: %lu", uc.args[1]); + break; + default: + TEST_FAIL("Unrecognized ucall: %lu\n", uc.cmd); + } +} + int main(int argc, char *argv[]) { struct kvm_vcpu *vcpu; @@ -50,6 +73,9 @@ int main(int argc, char *argv[]) TEST_REQUIRE(kvm_has_cap(KVM_CAP_SMALLER_MAXPHYADDR)); vm = vm_create_with_one_vcpu(&vcpu, guest_code); + vm_init_descriptor_tables(vm); + vcpu_init_descriptor_tables(vcpu); + vm_install_exception_handler(vm, PF_VECTOR, guest_page_fault_handler); vcpu_set_cpuid_maxphyaddr(vcpu, MAXPHYADDR); @@ -70,10 +96,21 @@ int main(int argc, char *argv[]) vm_set_page_table_entry(vm, vcpu, MEM_REGION_GVA, pte | (1ull << 36)); vcpu_run(vcpu); - assert_exit_for_flds_emulation_failure(vcpu); - skip_flds_instruction(vcpu); - vcpu_run(vcpu); - assert_ucall_done(vcpu); + + /* + * When TDP is enabled, KVM must emulate the flds instruction, which + * results in an emulation failure out to userspace. Otherwise, no + * instruction emulation is required so check that the instruction + * generates #PF(RSVD). + */ + if (kvm_is_tdp_enabled()) { + assert_exit_for_flds_emulation_failure(vcpu); + skip_flds_instruction(vcpu); + vcpu_run(vcpu); + assert_ucall_done(vcpu); + } else { + assert_reserved_page_fault(vcpu); + } kvm_vm_free(vm);
Change smaller_maxphyaddr_emulation_test to expect a #PF(RSVD), rather than an emulation failure, when TDP is disabled. KVM only needs to emulate instructions to emulate a smaller guest.MAXPHYADDR when TDP is enabled. Fixes: 39bbcc3a4e39 ("selftests: kvm: Allows userspace to handle emulation errors.") Signed-off-by: David Matlack <dmatlack@google.com> --- .../smaller_maxphyaddr_emulation_test.c | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-)