KVM: x86: WARN if KVM emulates an IPI without clearing the BUSY flag

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>

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 2b5db464c7aa832841cb0489525ab40b605ca9f4..90414c59f0be485d4a4ee13a05d0bd557a00b148 100644 (file)
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -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;

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d226012f849fe01b95dc029bd9a148e4d3abcd7d..7a8e8756c01610383dca9caaebff427b72764c94 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -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);