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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZxE1s6FWPkH07usG@google.com>
Date: Thu, 17 Oct 2024 09:05:07 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: "Markku Ahvenjärvi" <mankku@...il.com>, bp@...en8.de, dave.hansen@...ux.intel.com, 
	hpa@...or.com, janne.karhunen@...il.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, mingo@...hat.com, pbonzini@...hat.com, 
	tglx@...utronix.de, x86@...nel.org
Subject: Re: [PATCH 1/1] KVM: nVMX: update VPPR on vmlaunch/vmresume

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;