Messages in this thread | | | Date | Mon, 4 Apr 2022 18:59:35 +0200 | Subject | Re: [PATCH 8/8] KVM: selftests: nSVM: Add svm_nested_soft_inject_test | From | "Maciej S. Szmigiero" <> |
| |
On 4.04.2022 14:27, Maxim Levitsky wrote: > On Sat, 2022-04-02 at 01:09 +0000, Sean Christopherson wrote: >> From: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> >> >> Add a KVM self-test that checks whether a nSVM L1 is able to successfully >> inject a software interrupt and a soft exception into its L2 guest. >> >> In practice, this tests both the next_rip field consistency and >> L1-injected event with intervening L0 VMEXIT during its delivery: >> the first nested VMRUN (that's also trying to inject a software interrupt) >> will immediately trigger a L0 NPF. >> This L0 NPF will have zero in its CPU-returned next_rip field, which if >> incorrectly reused by KVM will trigger a #PF when trying to return to >> such address 0 from the interrupt handler. >> >> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> >> Signed-off-by: Sean Christopherson <seanjc@google.com> >> --- >> tools/testing/selftests/kvm/.gitignore | 1 + >> tools/testing/selftests/kvm/Makefile | 1 + >> .../selftests/kvm/include/x86_64/svm_util.h | 2 + >> .../kvm/x86_64/svm_nested_soft_inject_test.c | 147 ++++++++++++++++++ >> 4 files changed, 151 insertions(+) >> create mode 100644 tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c >> (..) >> diff --git a/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c b/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c >> new file mode 100644 >> index 000000000000..d39be5d885c1 >> --- /dev/null >> +++ b/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c >> @@ -0,0 +1,147 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (C) 2022 Oracle and/or its affiliates. >> + * >> + * Based on: >> + * svm_int_ctl_test >> + * >> + * Copyright (C) 2021, Red Hat, Inc. >> + * >> + */ >> + >> +#include "test_util.h" >> +#include "kvm_util.h" >> +#include "processor.h" >> +#include "svm_util.h" >> + >> +#define VCPU_ID 0 >> +#define INT_NR 0x20 >> +#define X86_FEATURE_NRIPS BIT(3) >> + >> +#define vmcall() \ >> + __asm__ __volatile__( \ >> + "vmmcall\n" \ >> + ) >> + >> +#define ud2() \ >> + __asm__ __volatile__( \ >> + "ud2\n" \ >> + ) >> + >> +#define hlt() \ >> + __asm__ __volatile__( \ >> + "hlt\n" \ >> + ) > > Minor nitpick: I guess it would be nice to put these in some common header file.
Will move these to include/x86_64/processor.h (that's probably the most matching header file).
>> + >> +static void l1_guest_code(struct svm_test_data *svm) >> +{ >> + #define L2_GUEST_STACK_SIZE 64 >> + unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE]; >> + struct vmcb *vmcb = svm->vmcb; >> + >> + /* Prepare for L2 execution. */ >> + generic_svm_setup(svm, l2_guest_code, >> + &l2_guest_stack[L2_GUEST_STACK_SIZE]); >> + >> + vmcb->control.intercept_exceptions |= BIT(PF_VECTOR) | BIT(UD_VECTOR); >> + vmcb->control.intercept |= BIT(INTERCEPT_HLT); >> + >> + vmcb->control.event_inj = INT_NR | SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_SOFT; >> + /* The return address pushed on stack */ >> + vmcb->control.next_rip = vmcb->save.rip; > > I'll would be putting even something more spicy here just to see that KVM preserves this field > like say put two ud2 in the start of guest code, and here have > > vmcb->control.next_rip = vmcb->save.rip + 4; // skip over two ud2 instructions. > > That way KVM won't be able to skip over a single instruction to get correct next_rip
Good point, will add these two additional ud2s at the start of L2 guest code.
> > > Other that nitpicks: > > Reviewed-by: Maxim levitsky <mlevitsk@redhat.com> > > Best regards, > Maxim Levitsky > >
Thanks, Maciej
| |