Re: [PATCH 1/1] KVM: nVMX: update VPPR on vmlaunch/vmresume

[Sean Christopherson]

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;