Messages in this thread | | | Date | Wed, 6 Apr 2022 14:46:59 +0800 | Subject | Re: [PATCH v5 1/3] KVM: X86: Save&restore the triple fault request | From | Chenyi Qiang <> |
| |
On 4/6/2022 7:31 AM, Sean Christopherson wrote: > On Fri, Mar 18, 2022, Chenyi Qiang wrote: >> For the triple fault sythesized by KVM, e.g. the RSM path or >> nested_vmx_abort(), if KVM exits to userspace before the request is >> serviced, userspace could migrate the VM and lose the triple fault. >> Fix this issue by adding a new event KVM_VCPUEVENT_TRIPLE_FAULT in >> get/set_vcpu_events() to track the triple fault request. >> >> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> >> --- >> Documentation/virt/kvm/api.rst | 6 ++++++ >> arch/x86/include/uapi/asm/kvm.h | 1 + >> arch/x86/kvm/x86.c | 9 ++++++++- >> 3 files changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst >> index 691ff84444bd..9682b0a438bd 100644 >> --- a/Documentation/virt/kvm/api.rst >> +++ b/Documentation/virt/kvm/api.rst >> @@ -1146,6 +1146,9 @@ The following bits are defined in the flags field: >> fields contain a valid state. This bit will be set whenever >> KVM_CAP_EXCEPTION_PAYLOAD is enabled. >> >> +- KVM_VCPUEVENT_TRIPLE_FAULT may be set to signal that there's a >> + triple fault request waiting to be serviced. > > Please avoid "request" in the docs, as before, that's a KVM implemenation detail. > For this one, maybe "there's a pending triple fault event"? > >> + >> ARM/ARM64: >> ^^^^^^^^^^ >> >> @@ -1241,6 +1244,9 @@ can be set in the flags field to signal that the >> exception_has_payload, exception_payload, and exception.pending fields >> contain a valid state and shall be written into the VCPU. >> >> +KVM_VCPUEVENT_TRIPLE_FAULT can be set in flags field to signal that a >> +triple fault request should be made. > > > And here, "to signal that KVM should synthesize a triple fault for the guest"? > >> + >> ARM/ARM64: >> ^^^^^^^^^^ >> >> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h >> index bf6e96011dfe..d8ef0d993e86 100644 >> --- a/arch/x86/include/uapi/asm/kvm.h >> +++ b/arch/x86/include/uapi/asm/kvm.h >> @@ -325,6 +325,7 @@ struct kvm_reinject_control { >> #define KVM_VCPUEVENT_VALID_SHADOW 0x00000004 >> #define KVM_VCPUEVENT_VALID_SMM 0x00000008 >> #define KVM_VCPUEVENT_VALID_PAYLOAD 0x00000010 >> +#define KVM_VCPUEVENT_TRIPLE_FAULT 0x00000020 >> >> /* Interrupt shadow states */ >> #define KVM_X86_SHADOW_INT_MOV_SS 0x01 >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 4fa4d8269e5b..fee402a700df 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -4891,6 +4891,9 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu, >> if (vcpu->kvm->arch.exception_payload_enabled) >> events->flags |= KVM_VCPUEVENT_VALID_PAYLOAD; >> >> + if (kvm_check_request(KVM_REQ_TRIPLE_FAULT, vcpu)) >> + events->flags |= KVM_VCPUEVENT_TRIPLE_FAULT; >> + >> memset(&events->reserved, 0, sizeof(events->reserved)); >> } >> >> @@ -4903,7 +4906,8 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu, >> | KVM_VCPUEVENT_VALID_SIPI_VECTOR >> | KVM_VCPUEVENT_VALID_SHADOW >> | KVM_VCPUEVENT_VALID_SMM >> - | KVM_VCPUEVENT_VALID_PAYLOAD)) >> + | KVM_VCPUEVENT_VALID_PAYLOAD >> + | KVM_VCPUEVENT_TRIPLE_FAULT)) >> return -EINVAL; >> >> if (events->flags & KVM_VCPUEVENT_VALID_PAYLOAD) { >> @@ -4976,6 +4980,9 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu, >> } >> } >> >> + if (events->flags & KVM_VCPUEVENT_TRIPLE_FAULT) >> + kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu); >> + >> kvm_make_request(KVM_REQ_EVENT, vcpu); > > Looks correct, but this really needs a selftest, at least for the SET path since > the intent is to use that for the NOTIFY handling. Doesn't need to be super fancy, > e.g. do port I/O from L2, inject a triple fault, and verify L1 sees the appropriate > exit. > > Aha! And for the GET path, abuse KVM_X86_SET_MCE with CR4.MCE=0 to coerce KVM into > making a KVM_REQ_TRIPLE_FAULT, that way there's no need to try and hit a timing > window to intercept the request.
OK, will cook a selftest to verify it.
| |