On Thu, Oct 17, 2024, Chao Gao wrote: > >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > >> index 5bb481aefcbc..d6a03c30f085 100644 > >> --- a/arch/x86/kvm/lapic.c > >> +++ b/arch/x86/kvm/lapic.c > >> @@ -800,6 +800,9 @@ static inline void apic_clear_isr(int vec, struct kvm_lapic *apic) > >> if (!__apic_test_and_clear_vector(vec, apic->regs + APIC_ISR)) > >> return; > >> > >> + if (is_guest_mode(apic->vcpu)) > > > >As above, I think this needs to be > > > > if (is_guest_mode(apic->vcpu) && !nested_cpu_has_vid(get_vmcs12(vcpu))) > > > >because if virtual interrupt delivery is enabled, then EOIs are virtualized. > >Which means that this needs to be handled in vmx_hwapic_isr_update(). > > I'm not sure if nested_cpu_has_vid() is necessary. My understanding is that > when a bit in the vCPU's vISR is cleared, the vCPU's SVI (i.e., SVI in vmcs01) > may be stale and so needs an update if vmcs01 isn't the active VMCS (i.e., the > vCPU is in guest mode). > > If L1 enables VID and EOIs from L2 are virtualized by KVM (L0), KVM shouldn't > call this function in the first place. Because KVM should update the > 'virt-APIC' page in VMCS12, rather than updating the vISR of the vCPU. Ah, right. And KVM handles that in nested_vmx_l1_wants_exit(), by forwarding all APICv exits to L1: case EXIT_REASON_APIC_ACCESS: case EXIT_REASON_APIC_WRITE: case EXIT_REASON_EOI_INDUCED: /* * The controls for "virtualize APIC accesses," "APIC- * register virtualization," and "virtual-interrupt * delivery" only come from vmcs12. */ return true;
