Re: [PATCH] KVM: VMX: Re-pend KVM_REQ_APIC_PAGE_RELOAD if getting backing pfn fails

[Sean Christopherson <seanjc@google.com>]

On Thu, Nov 05, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] KVM: VMX: stale no-retry comment in vmx_set_apic_access_page_addr()
> --
> 
> commit 635b83243ce9cf39c9f2bbd0198d7730dbb21505
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: VMX: Re-pend KVM_REQ_APIC_PAGE_RELOAD if getting backing pfn fails
> 
> This commit re-pends KVM_REQ_APIC_PAGE_RELOAD in
> vmx_set_apic_access_page_addr() when looking up the APIC-access page memslot
> or faulting in its backing pfn fails, preventing the vCPU from running with a
> stale vmcs.APIC_ACCESS_ADDR. It also adds a WARN_ON_ONCE() if the private
> memslot is missing or invalid.
> 
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index b341af4c6057a..13dcf9c12c83a 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -6923,8 +6923,10 @@ void vmx_set_apic_access_page_addr(struct kvm_vcpu *vcpu)
> >  	 * APICv is enabled, but paranoia won't hurt in this case.
> >  	 */
> >  	slot = id_to_memslot(slots, APIC_ACCESS_PAGE_PRIVATE_MEMSLOT);
> > -	if (!slot || slot->flags & KVM_MEMSLOT_INVALID)
> > +	if (WARN_ON_ONCE(!slot || slot->flags & KVM_MEMSLOT_INVALID)) {
> > +		kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu);
> >  		return;
> > +	}
> >  
> >  	/*
> >  	 * Ensure that the mmu_notifier sequence count is read before KVM
> > @@ -6941,8 +6943,10 @@ void vmx_set_apic_access_page_addr(struct kvm_vcpu *vcpu)
> >  	 * if APICv is permanently inhibited, i.e. the memslot won't reappear.
> >  	 */
> >  	pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, &writable, &refcounted_page);
> > -	if (is_error_noslot_pfn(pfn))
> > +	if (is_error_noslot_pfn(pfn)) {
> > +		kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu);
> >  		return;
> > +	}
> 
> [Severity: Low]
> This isn't a bug, but in vmx_set_apic_access_page_addr() in
> arch/x86/kvm/vmx/vmx.c, the full comment immediately above
> __kvm_faultin_pfn() still states:
> 
> 	/*
> 	 * No need to retry if the memslot does not exist or is invalid.  KVM
> 	 * controls the APIC-access page memslot, and only deletes the memslot
> 	 * if APICv is permanently inhibited, i.e. the memslot won't reappear.
> 	 */
> 
> Since both the invalid-memslot check and the is_error_noslot_pfn(pfn) check
> now retry by calling kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu), and
> VMX does not delete or inhibit the APIC-access page memslot while the VM is
> alive, should this comment be updated or removed?

Gah, yeah, I'll fixup to this when applying:


	/*
	 * Retry if faulting in the page fails (or the memslot is unexpectedly
	 * invalid) even though KVM controls the APIC-access page memslot, and
	 * never deletes memslot for a live VM when running on VMX.  Userspace
	 * can modify the underlying VMA, and while doing so is beyond crazy,
	 * the cost of supporting that behavior is to simply retry the reload,
	 * which KVM should do anyways to avoid carrying a stale pfn pointer.
	 */