diff mbox series

[3/3] KVM: nVMX: Clear IDT vectoring on nested VM-Exit for double/triple fault

Message ID 20220407002315.78092-4-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86: Nested fixes (mostly #DF/#TF) | expand

Commit Message

Sean Christopherson April 7, 2022, 12:23 a.m. UTC
Clear the IDT vectoring field in vmcs12 on next VM-Exit due to a double
or triple fault.  Per the SDM, a VM-Exit isn't considered to occur during
event delivery if the exit is due to an intercepted double fault or a
triple fault.  Opportunistically move the default clearing (no event
"pending") into the helper so that it's more obvious that KVM does indeed
handle this case.

Note, the double fault case is worded rather wierdly in the SDM:

  The original event results in a double-fault exception that causes the
  VM exit directly.

Temporarily ignoring injected events, double faults can _only_ occur if
an exception occurs while attempting to deliver a different exception,
i.e. there's _always_ an original event.  And for injected double fault,
while there's no original event, injected events are never subject to
interception.

Presumably the SDM is calling out that a the vectoring info will be valid
if a different exit occurs after a double fault, e.g. if a #PF occurs and
is intercepted while vectoring #DF, then the vectoring info will show the
double fault.  In other words, the clause can simply be read as:

  The VM exit is caused by a double-fault exception.

Fixes: 4704d0befb07 ("KVM: nVMX: Exiting from L2 to L1")
Cc: Chenyi Qiang <chenyi.qiang@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.c | 32 ++++++++++++++++++++++++++++----
 arch/x86/kvm/vmx/vmcs.h   |  5 +++++
 2 files changed, 33 insertions(+), 4 deletions(-)

Comments

Xiaoyao Li April 7, 2022, 8:02 a.m. UTC | #1
On 4/7/2022 8:23 AM, Sean Christopherson wrote:
> Clear the IDT vectoring field in vmcs12 on next VM-Exit due to a double
> or triple fault.  Per the SDM, a VM-Exit isn't considered to occur during
> event delivery if the exit is due to an intercepted double fault or a
> triple fault.  Opportunistically move the default clearing (no event
> "pending") into the helper so that it's more obvious that KVM does indeed
> handle this case.
> 
> Note, the double fault case is worded rather wierdly in the SDM:
> 
>    The original event results in a double-fault exception that causes the
>    VM exit directly.
> 
> Temporarily ignoring injected events, double faults can _only_ occur if
> an exception occurs while attempting to deliver a different exception,
> i.e. there's _always_ an original event.  And for injected double fault,
> while there's no original event, injected events are never subject to
> interception.
> 
> Presumably the SDM is calling out that a the vectoring info will be valid
> if a different exit occurs after a double fault, e.g. if a #PF occurs and
> is intercepted while vectoring #DF, then the vectoring info will show the
> double fault.  

Wouldn't it be a tripe fault exit in this case?

> In other words, the clause can simply be read as:
> 
>    The VM exit is caused by a double-fault exception.
Xiaoyao Li April 7, 2022, 8:42 a.m. UTC | #2
On 4/7/2022 4:02 PM, Xiaoyao Li wrote:
> On 4/7/2022 8:23 AM, Sean Christopherson wrote:
>> Clear the IDT vectoring field in vmcs12 on next VM-Exit due to a double
>> or triple fault.  Per the SDM, a VM-Exit isn't considered to occur during
>> event delivery if the exit is due to an intercepted double fault or a
>> triple fault.  Opportunistically move the default clearing (no event
>> "pending") into the helper so that it's more obvious that KVM does indeed
>> handle this case.
>>
>> Note, the double fault case is worded rather wierdly in the SDM:
>>
>>    The original event results in a double-fault exception that causes the
>>    VM exit directly.
>>
>> Temporarily ignoring injected events, double faults can _only_ occur if
>> an exception occurs while attempting to deliver a different exception,
>> i.e. there's _always_ an original event.  And for injected double fault,
>> while there's no original event, injected events are never subject to
>> interception.
>>
>> Presumably the SDM is calling out that a the vectoring info will be valid
>> if a different exit occurs after a double fault, e.g. if a #PF occurs and
>> is intercepted while vectoring #DF, then the vectoring info will show the
>> double fault. 
> 
> Wouldn't it be a tripe fault exit in this case?

It won't since #PF is intercepted by exception bitmap in your case.

>> In other words, the clause can simply be read as:
>>
>>    The VM exit is caused by a double-fault exception.
>
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 9a4938955bad..a6688663da4d 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3691,12 +3691,34 @@  vmcs12_guest_cr4(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 }
 
 static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
-				      struct vmcs12 *vmcs12)
+				      struct vmcs12 *vmcs12,
+				      u32 vm_exit_reason, u32 exit_intr_info)
 {
 	u32 idt_vectoring;
 	unsigned int nr;
 
-	if (vcpu->arch.exception.injected) {
+	/*
+	 * Per the SDM, VM-Exits due to double and triple faults are never
+	 * considered to occur during event delivery, even if the double/triple
+	 * fault is the result of an escalating vectoring issue.
+	 *
+	 * Note, the SDM qualifies the double fault behavior with "The original
+	 * event results in a double-fault exception".  It's unclear why the
+	 * qualification exists since exits due to double fault can occur only
+	 * while vectoring a different exception (injected events are never
+	 * subject to interception), i.e. there's _always_ an original event.
+	 *
+	 * The SDM also uses NMI as a confusing example for the "original event
+	 * causes the VM exit directly" clause.  NMI isn't special in any way,
+	 * the same rule applies to all events that cause an exit directly.
+	 * NMI is an odd choice for the example because NMIs can only occur on
+	 * instruction boundaries, i.e. they _can't_ occur during vectoring.
+	 */
+	if ((u16)vm_exit_reason == EXIT_REASON_TRIPLE_FAULT ||
+	    ((u16)vm_exit_reason == EXIT_REASON_EXCEPTION_NMI &&
+	     is_double_fault(exit_intr_info))) {
+		vmcs12->idt_vectoring_info_field = 0;
+	} else if (vcpu->arch.exception.injected) {
 		nr = vcpu->arch.exception.nr;
 		idt_vectoring = nr | VECTORING_INFO_VALID_MASK;
 
@@ -3729,6 +3751,8 @@  static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
 			idt_vectoring |= INTR_TYPE_EXT_INTR;
 
 		vmcs12->idt_vectoring_info_field = idt_vectoring;
+	} else {
+		vmcs12->idt_vectoring_info_field = 0;
 	}
 }
 
@@ -4215,8 +4239,8 @@  static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 		 * Transfer the event that L0 or L1 may wanted to inject into
 		 * L2 to IDT_VECTORING_INFO_FIELD.
 		 */
-		vmcs12->idt_vectoring_info_field = 0;
-		vmcs12_save_pending_event(vcpu, vmcs12);
+		vmcs12_save_pending_event(vcpu, vmcs12,
+					  vm_exit_reason, exit_intr_info);
 
 		vmcs12->vm_exit_intr_info = exit_intr_info;
 		vmcs12->vm_exit_instruction_len = vmcs_read32(VM_EXIT_INSTRUCTION_LEN);
diff --git a/arch/x86/kvm/vmx/vmcs.h b/arch/x86/kvm/vmx/vmcs.h
index e325c290a816..2b9d7a7e83f7 100644
--- a/arch/x86/kvm/vmx/vmcs.h
+++ b/arch/x86/kvm/vmx/vmcs.h
@@ -104,6 +104,11 @@  static inline bool is_breakpoint(u32 intr_info)
 	return is_exception_n(intr_info, BP_VECTOR);
 }
 
+static inline bool is_double_fault(u32 intr_info)
+{
+	return is_exception_n(intr_info, DF_VECTOR);
+}
+
 static inline bool is_page_fault(u32 intr_info)
 {
 	return is_exception_n(intr_info, PF_VECTOR);