When IPI virtualization is enabled, a WARN is triggered if bit12 of ICR MSR is set after APIC-write VM-exit. The reason is kvm_apic_send_ipi() thinks the APIC_ICR_BUSY bit should be cleared because KVM has no delay, but kvm_apic_write_nodecode() doesn't clear the APIC_ICR_BUSY bit. Bit12 of ICR is different from other reserved bits(31:20, 17:16 and 13). When bit12 is set, it will cause APIC-wirte VM-exit but not #GP. For reading bit12 back as '0' which is a safer approach, clearing bit12 in x2APIC mode is needed. Although bit12 of ICR is no longer APIC_ICR_BUSY in x2APIC, keeping it is far easier to understand what's going on, especially given that it may be repurposed for something new. Link: https://lore.kernel.org/all/ZPj6iF0Q7iynn62p@xxxxxxxxxx/ Fixes: 5413bcba7ed5 ("KVM: x86: Add support for vICR APIC-write VM-Exits in x2APIC mode") Signed-off-by: Tao Su <tao1.su@xxxxxxxxxxxxxxx> Tested-by: Yi Lai <yi1.lai@xxxxxxxxx> --- Changelog: v2: - Drop the unnecessary alias for bit12 of ICR. - Add back kvm_lapic_get_reg64() that was removed by mistake. - Modify the commit message to make it clearer. v1: https://lore.kernel.org/all/20230904013555.725413-1-tao1.su@xxxxxxxxxxxxxxx/ --- arch/x86/kvm/lapic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index dcd60b39e794..664d5a78b46a 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2450,13 +2450,13 @@ void kvm_apic_write_nodecode(struct kvm_vcpu *vcpu, u32 offset) * ICR is a single 64-bit register when x2APIC is enabled. For legacy * xAPIC, ICR writes need to go down the common (slightly slower) path * to get the upper half from ICR2. + * + * TODO: optimize to just emulate side effect w/o one more write */ if (apic_x2apic_mode(apic) && offset == APIC_ICR) { val = kvm_lapic_get_reg64(apic, APIC_ICR); - kvm_apic_send_ipi(apic, (u32)val, (u32)(val >> 32)); - trace_kvm_apic_write(APIC_ICR, val); + kvm_x2apic_icr_write(apic, val); } else { - /* TODO: optimize to just emulate side effect w/o one more write */ val = kvm_lapic_get_reg(apic, offset); kvm_lapic_reg_write(apic, offset, (u32)val); } base-commit: a48fa7efaf1161c1c898931fe4c7f0070964233a -- 2.34.1
