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

[sashiko-bot@kernel.org]

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105181742.3718448-1-seanjc@google.com?part=1