KVM: x86: detect SPIV changes under APICv

APIC-write VM exits are "trap-like": they save CS:RIP values for the
instruction after the write, and more importantly, the handler will
already see the new value in the virtual-APIC page.

This caused a bug if you used KVM_SET_IRQCHIP to set the SW-enabled bit
in the SPIV register.  The chain of events is as follows:

* When the irqchip is added to the destination VM, the apic_sw_disabled
static key is incremented (1)

* When the KVM_SET_IRQCHIP ioctl is invoked, it is decremented (0)

* When the guest disables the bit in the SPIV register, e.g. as part of
shutdown, apic_set_spiv does not notice the change and the static key is
_not_ incremented.

* When the guest is destroyed, the static key is decremented (-1),
resulting in this trace:

  WARNING: at kernel/jump_label.c:81 __static_key_slow_dec+0xa6/0xb0()
  jump label: negative count!

  [<ffffffff816bf898>] dump_stack+0x19/0x1b
  [<ffffffff8107c6f1>] warn_slowpath_common+0x61/0x80
  [<ffffffff8107c76c>] warn_slowpath_fmt+0x5c/0x80
  [<ffffffff811931e6>] __static_key_slow_dec+0xa6/0xb0
  [<ffffffff81193226>] static_key_slow_dec_deferred+0x16/0x20
  [<ffffffffa0637698>] kvm_free_lapic+0x88/0xa0 [kvm]
  [<ffffffffa061c63e>] kvm_arch_vcpu_uninit+0x2e/0xe0 [kvm]
  [<ffffffffa05ff301>] kvm_vcpu_uninit+0x21/0x40 [kvm]
  [<ffffffffa067cec7>] vmx_free_vcpu+0x47/0x70 [kvm_intel]
  [<ffffffffa061bc50>] kvm_arch_vcpu_free+0x50/0x60 [kvm]
  [<ffffffffa061ca22>] kvm_arch_destroy_vm+0x102/0x260 [kvm]
  [<ffffffff810b68fd>] ? synchronize_srcu+0x1d/0x20
  [<ffffffffa06030d1>] kvm_put_kvm+0xe1/0x1c0 [kvm]
  [<ffffffffa06036f8>] kvm_vcpu_release+0x18/0x20 [kvm]
  [<ffffffff81215c62>] __fput+0x102/0x310
  [<ffffffff81215f4e>] ____fput+0xe/0x10
  [<ffffffff810ab664>] task_work_run+0xb4/0xe0
  [<ffffffff81083944>] do_exit+0x304/0xc60
  [<ffffffff816c8dfc>] ? _raw_spin_unlock_irq+0x2c/0x50
  [<ffffffff810fd22d>] ?  trace_hardirqs_on_caller+0xfd/0x1c0
  [<ffffffff8108432c>] do_group_exit+0x4c/0xc0
  [<ffffffff810843b4>] SyS_exit_group+0x14/0x20
  [<ffffffff816d33a9>] system_call_fastpath+0x16/0x1b

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 66dd1731ab99593991aa950ec6ec377d67728836..80df439d40861496f585fd8b9a485662948289d3 100644 (file)
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -206,11 +206,13 @@ out:
 
 static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
 {
-       u32 prev = kvm_apic_get_reg(apic, APIC_SPIV);
+       bool enabled = val & APIC_SPIV_APIC_ENABLED;
 
        apic_set_reg(apic, APIC_SPIV, val);
-       if ((prev ^ val) & APIC_SPIV_APIC_ENABLED) {
-               if (val & APIC_SPIV_APIC_ENABLED) {
+
+       if (enabled != apic->sw_enabled) {
+               apic->sw_enabled = enabled;
+               if (enabled) {
                        static_key_slow_dec_deferred(&apic_sw_disabled);
                        recalculate_apic_map(apic->vcpu->kvm);
                } else
@@ -1357,7 +1359,7 @@ void kvm_free_lapic(struct kvm_vcpu *vcpu)
        if (!(vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE))
                static_key_slow_dec_deferred(&apic_hw_disabled);
 
-       if (!(kvm_apic_get_reg(apic, APIC_SPIV) & APIC_SPIV_APIC_ENABLED))
+       if (!apic->sw_enabled)
                static_key_slow_dec_deferred(&apic_sw_disabled);
 
        if (apic->regs)

diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 3fd6c22d187d7114f430d89a94c645d5619f8acc..87e0fae1f4c66f9e068ed1119c9ac96495b8b305 100644 (file)
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -22,6 +22,7 @@ struct kvm_lapic {
        struct kvm_timer lapic_timer;
        u32 divide_count;
        struct kvm_vcpu *vcpu;
+       bool sw_enabled;
        bool irr_pending;
        /* Number of bits set in ISR. */
        s16 isr_count;