| Message ID | 20230914055504.151365-1-tao1.su@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] KVM: x86: Clear bit12 of ICR after APIC-write VM-exit | expand |
On Thu, 14 Sep 2023 13:55:04 +0800, Tao Su wrote: > 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. > > Under the x2APIC section, regarding ICR, the SDM says: > > [...] Applied to kvm-x86 vmx. I dropped the TODO and replaced with an explanation of why the "extra" write is necessary, and why trying to avoid it isn't worth "fixing". Thanks! [1/1] KVM: x86: Clear bit12 of ICR after APIC-write VM-exit https://github.com/kvm-x86/linux/commit/629d3698f695 -- https://github.com/kvm-x86/linux/tree/next
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); }
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. Under the x2APIC section, regarding ICR, the SDM says: It remains readable only to aid in debugging; however, software should not assume the value returned by reading the ICR is the last written value. I.e. KVM basically has free reign to do whatever it wants, so long as it doesn't confuse userspace or break KVM's ABI. Clear bit12 so that it reads back as '0'. This approach is safer than "do nothing" and is consistent with the case where IPI virtualization is disabled or not supported, i.e., handle_fastpath_set_x2apic_icr_irqoff() -> kvm_x2apic_icr_write() Link: https://lore.kernel.org/all/ZPj6iF0Q7iynn62p@google.com/ Fixes: 5413bcba7ed5 ("KVM: x86: Add support for vICR APIC-write VM-Exits in x2APIC mode") Signed-off-by: Tao Su <tao1.su@linux.intel.com> Tested-by: Yi Lai <yi1.lai@intel.com> Reviewed-by: Chao Gao <chao.gao@intel.com> --- Changelog: v3: - Correct commit message. - Add Chao's Reviewed-by. v2: https://lore.kernel.org/all/20230908041115.987682-1-tao1.su@linux.intel.com/ v1: https://lore.kernel.org/all/20230904013555.725413-1-tao1.su@linux.intel.com/ --- arch/x86/kvm/lapic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) base-commit: aed8aee11130a954356200afa3f1b8753e8a9482