| Message ID | 20241111102749.82761-7-iorlov@amazon.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enhance event delivery error handling | expand |
On Mon, Nov 11, 2024, Ivan Orlov wrote: > Extend the 'set_memory_region_test' with a test case which covers the > MMIO during vectoring error handling. The test case > > 1) Sets an IDT descriptor base to point to an MMIO address > 2) Generates a #GP in the guest > 3) Verifies that we got a correct exit reason and suberror code > 4) Verifies that we got a corrent reported GPA in internal.data[3] > > Also, add a definition of non-canonical address to processor.h > > Signed-off-by: Ivan Orlov <iorlov@amazon.com> > --- > V1 -> V2: > - Get rid of pronouns, redundant comments and incorrect wording > - Define noncanonical address in processor.h > - Fix indentation and wrap lines at 80 columns > > .../selftests/kvm/include/x86_64/processor.h | 2 + > .../selftests/kvm/set_memory_region_test.c | 51 +++++++++++++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h > index 1a60c99b5833..997df5003edb 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/processor.h > +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h > @@ -1165,6 +1165,8 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector, > /* If a toddler were to say "abracadabra". */ > #define KVM_EXCEPTION_MAGIC 0xabacadabaULL > > +#define NONCANONICAL 0xaaaaaaaaaaaaaaaaull Uber nit, I much prefer to place this definition at the top of the header. More specifically, it needs to not be in the middle of the selftest's exception fixup code. > + > /* > * KVM selftest exception fixup uses registers to coordinate with the exception > * handler, versus the kernel's in-memory tables and KVM-Unit-Tests's in-memory > diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c > index a1c53cc854a5..d65a9f20aa1a 100644 > --- a/tools/testing/selftests/kvm/set_memory_region_test.c > +++ b/tools/testing/selftests/kvm/set_memory_region_test.c > @@ -553,6 +553,56 @@ static void test_add_overlapping_private_memory_regions(void) > close(memfd); > kvm_vm_free(vm); > } > + > +static void guest_code_mmio_during_vectoring(void) > +{ > + const struct desc_ptr idt_desc = { > + .address = MEM_REGION_GPA, > + .size = 0xFFF, > + }; > + > + set_idt(&idt_desc); > + > + /* Generate a #GP by dereferencing a non-canonical address */ > + *((uint8_t *)NONCANONICAL) = 0x1; Now I'm curious what happens if this uses vcpu_arch_put_guest(), i.e. if the test forces KVM to emulate the write. No action needed, the test is a-ok as-is. I'm really just curious :-)
On Wed, Dec 11, 2024 at 10:19:40AM -0800, Sean Christopherson wrote: > > +static void guest_code_mmio_during_vectoring(void) > > +{ > > + const struct desc_ptr idt_desc = { > > + .address = MEM_REGION_GPA, > > + .size = 0xFFF, > > + }; > > + > > + set_idt(&idt_desc); > > + > > + /* Generate a #GP by dereferencing a non-canonical address */ > > + *((uint8_t *)NONCANONICAL) = 0x1; > > Now I'm curious what happens if this uses vcpu_arch_put_guest(), i.e. if the > test forces KVM to emulate the write. > > No action needed, the test is a-ok as-is. I'm really just curious :-) :) Just tried enabling `force_emulation_prefix` kvm parameter and replacing the write with vcpu_arch_put_guest(*((uint8_t *)NONCANONICAL), 0x1); And the test simply passes (so it returns the same internal error with suberror=3, patches applied) -- Kind regards, Ivan Orlov
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 1a60c99b5833..997df5003edb 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -1165,6 +1165,8 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector, /* If a toddler were to say "abracadabra". */ #define KVM_EXCEPTION_MAGIC 0xabacadabaULL +#define NONCANONICAL 0xaaaaaaaaaaaaaaaaull + /* * KVM selftest exception fixup uses registers to coordinate with the exception * handler, versus the kernel's in-memory tables and KVM-Unit-Tests's in-memory diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c index a1c53cc854a5..d65a9f20aa1a 100644 --- a/tools/testing/selftests/kvm/set_memory_region_test.c +++ b/tools/testing/selftests/kvm/set_memory_region_test.c @@ -553,6 +553,56 @@ static void test_add_overlapping_private_memory_regions(void) close(memfd); kvm_vm_free(vm); } + +static void guest_code_mmio_during_vectoring(void) +{ + const struct desc_ptr idt_desc = { + .address = MEM_REGION_GPA, + .size = 0xFFF, + }; + + set_idt(&idt_desc); + + /* Generate a #GP by dereferencing a non-canonical address */ + *((uint8_t *)NONCANONICAL) = 0x1; + + GUEST_ASSERT(0); +} + +/* + * This test points the IDT descriptor base to an MMIO address. It should cause + * a KVM internal error when an event occurs in the guest. + */ +static void test_mmio_during_vectoring(void) +{ + struct kvm_vcpu *vcpu; + struct kvm_run *run; + struct kvm_vm *vm; + u64 expected_gpa; + + pr_info("Testing MMIO during vectoring error handling\n"); + + vm = vm_create_with_one_vcpu(&vcpu, guest_code_mmio_during_vectoring); + virt_map(vm, MEM_REGION_GPA, MEM_REGION_GPA, 1); + + run = vcpu->run; + + vcpu_run(vcpu); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_INTERNAL_ERROR); + TEST_ASSERT(run->internal.suberror == KVM_INTERNAL_ERROR_DELIVERY_EV, + "Unexpected suberror = %d", vcpu->run->internal.suberror); + TEST_ASSERT(run->internal.ndata != 4, "Unexpected internal error data array size = %d", + run->internal.ndata); + + /* The reported GPA should be IDT base + offset of the GP vector */ + expected_gpa = MEM_REGION_GPA + GP_VECTOR * sizeof(struct idt_entry); + + TEST_ASSERT(run->internal.data[3] == expected_gpa, + "Unexpected GPA = %llx (expected %lx)", + vcpu->run->internal.data[3], expected_gpa); + + kvm_vm_free(vm); +} #endif int main(int argc, char *argv[]) @@ -568,6 +618,7 @@ int main(int argc, char *argv[]) * KVM_RUN fails with ENOEXEC or EFAULT. */ test_zero_memory_regions(); + test_mmio_during_vectoring(); #endif test_invalid_memory_region_flags();
Extend the 'set_memory_region_test' with a test case which covers the MMIO during vectoring error handling. The test case 1) Sets an IDT descriptor base to point to an MMIO address 2) Generates a #GP in the guest 3) Verifies that we got a correct exit reason and suberror code 4) Verifies that we got a corrent reported GPA in internal.data[3] Also, add a definition of non-canonical address to processor.h Signed-off-by: Ivan Orlov <iorlov@amazon.com> --- V1 -> V2: - Get rid of pronouns, redundant comments and incorrect wording - Define noncanonical address in processor.h - Fix indentation and wrap lines at 80 columns .../selftests/kvm/include/x86_64/processor.h | 2 + .../selftests/kvm/set_memory_region_test.c | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+)