KVM: x86: WARN if KVM emulates an IPI without clearing the BUSY flag
authorSean Christopherson <seanjc@google.com>
Fri, 4 Feb 2022 21:42:00 +0000 (21:42 +0000)
committerPaolo Bonzini <pbonzini@redhat.com>
Tue, 1 Mar 2022 13:50:47 +0000 (08:50 -0500)
WARN if KVM emulates an IPI without clearing the BUSY flag, failure to do
so could hang the guest if it waits for the IPI be sent.

Opportunistically use APIC_ICR_BUSY macro instead of open coding the
magic number, and add a comment to clarify why kvm_recalculate_apic_map()
is unconditionally invoked (it's really, really confusing for IPIs due to
the existence of fast paths that don't trigger a potential recalc).

Signed-off-by: Sean Christopherson <seanjc@google.com>
Message-Id: <20220204214205.3306634-7-seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
arch/x86/kvm/lapic.c
arch/x86/kvm/x86.c

index 2b5db46..90414c5 100644 (file)
@@ -1275,6 +1275,9 @@ void kvm_apic_send_ipi(struct kvm_lapic *apic, u32 icr_low, u32 icr_high)
 {
        struct kvm_lapic_irq irq;
 
+       /* KVM has no delay and should always clear the BUSY/PENDING flag. */
+       WARN_ON_ONCE(icr_low & APIC_ICR_BUSY);
+
        irq.vector = icr_low & APIC_VECTOR_MASK;
        irq.delivery_mode = icr_low & APIC_MODE_MASK;
        irq.dest_mode = icr_low & APIC_DEST_MASK;
@@ -2053,7 +2056,7 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
        }
        case APIC_ICR:
                /* No delay here, so we always clear the pending bit */
-               val &= ~(1 << 12);
+               val &= ~APIC_ICR_BUSY;
                kvm_apic_send_ipi(apic, val, kvm_lapic_get_reg(apic, APIC_ICR2));
                kvm_lapic_set_reg(apic, APIC_ICR, val);
                break;
@@ -2131,6 +2134,11 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
                break;
        }
 
+       /*
+        * Recalculate APIC maps if necessary, e.g. if the software enable bit
+        * was toggled, the APIC ID changed, etc...   The maps are marked dirty
+        * on relevant changes, i.e. this is a nop for most writes.
+        */
        kvm_recalculate_apic_map(apic->vcpu->kvm);
 
        return ret;
index d226012..7a8e875 100644 (file)
@@ -2039,11 +2039,10 @@ static int handle_fastpath_set_x2apic_icr_irqoff(struct kvm_vcpu *vcpu, u64 data
                return 1;
 
        if (((data & APIC_SHORT_MASK) == APIC_DEST_NOSHORT) &&
-               ((data & APIC_DEST_MASK) == APIC_DEST_PHYSICAL) &&
-               ((data & APIC_MODE_MASK) == APIC_DM_FIXED) &&
-               ((u32)(data >> 32) != X2APIC_BROADCAST)) {
-
-               data &= ~(1 << 12);
+           ((data & APIC_DEST_MASK) == APIC_DEST_PHYSICAL) &&
+           ((data & APIC_MODE_MASK) == APIC_DM_FIXED) &&
+           ((u32)(data >> 32) != X2APIC_BROADCAST)) {
+               data &= ~APIC_ICR_BUSY;
                kvm_apic_send_ipi(vcpu->arch.apic, (u32)data, (u32)(data >> 32));
                kvm_lapic_set_reg(vcpu->arch.apic, APIC_ICR2, (u32)(data >> 32));
                kvm_lapic_set_reg(vcpu->arch.apic, APIC_ICR, (u32)data);