Skip to content

Commit 363010e

Browse files
committed
KVM: nVMX: Get to-be-acknowledge IRQ for nested VM-Exit at injection site
Move the logic to get the to-be-acknowledge IRQ for a nested VM-Exit from nested_vmx_vmexit() to vmx_check_nested_events(), which is subtly the one and only path where KVM invokes nested_vmx_vmexit() with EXIT_REASON_EXTERNAL_INTERRUPT. A future fix will perform a last-minute check on L2's nested posted interrupt notification vector, just before injecting a nested VM-Exit. To handle that scenario correctly, KVM needs to get the interrupt _before_ injecting VM-Exit, as simply querying the highest priority interrupt, via kvm_cpu_has_interrupt(), would result in TOCTOU bug, as a new, higher priority interrupt could arrive between kvm_cpu_has_interrupt() and kvm_cpu_get_interrupt(). Unfortunately, simply moving the call to kvm_cpu_get_interrupt() doesn't suffice, as a VMWRITE to GUEST_INTERRUPT_STATUS.SVI is hiding in kvm_get_apic_interrupt(), and acknowledging the interrupt before nested VM-Exit would cause the VMWRITE to hit vmcs02 instead of vmcs01. Open code a rough equivalent to kvm_cpu_get_interrupt() so that the IRQ is acknowledged after emulating VM-Exit, taking care to avoid the TOCTOU issue described above. Opportunistically convert the WARN_ON() to a WARN_ON_ONCE(). If KVM has a bug that results in a false positive from kvm_cpu_has_interrupt(), spamming dmesg won't help the situation. Note, nested_vmx_reflect_vmexit() can never reflect external interrupts as they are always "wanted" by L0. Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sean Christopherson <[email protected]>
1 parent a194a3a commit 363010e

File tree

3 files changed

+30
-10
lines changed

3 files changed

+30
-10
lines changed

arch/x86/include/asm/kvm_host.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2256,6 +2256,7 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v);
22562256
int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu);
22572257
int kvm_cpu_has_extint(struct kvm_vcpu *v);
22582258
int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu);
2259+
int kvm_cpu_get_extint(struct kvm_vcpu *v);
22592260
int kvm_cpu_get_interrupt(struct kvm_vcpu *v);
22602261
void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event);
22612262

arch/x86/kvm/irq.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt);
108108
* Read pending interrupt(from non-APIC source)
109109
* vector and intack.
110110
*/
111-
static int kvm_cpu_get_extint(struct kvm_vcpu *v)
111+
int kvm_cpu_get_extint(struct kvm_vcpu *v)
112112
{
113113
if (!kvm_cpu_has_extint(v)) {
114114
WARN_ON(!lapic_in_kernel(v));
@@ -131,6 +131,7 @@ static int kvm_cpu_get_extint(struct kvm_vcpu *v)
131131
} else
132132
return kvm_pic_read_irq(v->kvm); /* PIC */
133133
}
134+
EXPORT_SYMBOL_GPL(kvm_cpu_get_extint);
134135

135136
/*
136137
* Read pending interrupt vector and intack.

arch/x86/kvm/vmx/nested.c

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4285,11 +4285,37 @@ static int vmx_check_nested_events(struct kvm_vcpu *vcpu)
42854285
}
42864286

42874287
if (kvm_cpu_has_interrupt(vcpu) && !vmx_interrupt_blocked(vcpu)) {
4288+
int irq;
4289+
42884290
if (block_nested_events)
42894291
return -EBUSY;
42904292
if (!nested_exit_on_intr(vcpu))
42914293
goto no_vmexit;
4292-
nested_vmx_vmexit(vcpu, EXIT_REASON_EXTERNAL_INTERRUPT, 0, 0);
4294+
4295+
if (!nested_exit_intr_ack_set(vcpu)) {
4296+
nested_vmx_vmexit(vcpu, EXIT_REASON_EXTERNAL_INTERRUPT, 0, 0);
4297+
return 0;
4298+
}
4299+
4300+
irq = kvm_cpu_get_extint(vcpu);
4301+
if (irq != -1) {
4302+
nested_vmx_vmexit(vcpu, EXIT_REASON_EXTERNAL_INTERRUPT,
4303+
INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR | irq, 0);
4304+
return 0;
4305+
}
4306+
4307+
irq = kvm_apic_has_interrupt(vcpu);
4308+
WARN_ON_ONCE(irq < 0);
4309+
4310+
nested_vmx_vmexit(vcpu, EXIT_REASON_EXTERNAL_INTERRUPT,
4311+
INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR | irq, 0);
4312+
4313+
/*
4314+
* ACK the interrupt _after_ emulating VM-Exit, as the IRQ must
4315+
* be marked as in-service in vmcs01.GUEST_INTERRUPT_STATUS.SVI
4316+
* if APICv is active.
4317+
*/
4318+
kvm_apic_ack_interrupt(vcpu, irq);
42934319
return 0;
42944320
}
42954321

@@ -4970,14 +4996,6 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
49704996
vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
49714997

49724998
if (likely(!vmx->fail)) {
4973-
if ((u16)vm_exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
4974-
nested_exit_intr_ack_set(vcpu)) {
4975-
int irq = kvm_cpu_get_interrupt(vcpu);
4976-
WARN_ON(irq < 0);
4977-
vmcs12->vm_exit_intr_info = irq |
4978-
INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
4979-
}
4980-
49814999
if (vm_exit_reason != -1)
49825000
trace_kvm_nested_vmexit_inject(vmcs12->vm_exit_reason,
49835001
vmcs12->exit_qualification,

0 commit comments

Comments
 (0)