Re: [RFC PATCH 0/4] KVM: x86/mmu: don't zap secondary MMU on permission upgrade

[Sean Christopherson <seanjc@google.com>]

On Tue, Nov 03, 2026, Carlos López wrote:
> On 11/2/26 4:31 PM, Sean Christopherson wrote:
> > On Fri, Oct 30, 2026, Claudio Imbrenda wrote:
> >> On Fri, 30 Oct 2026 12:11:07 +0100
> >> Carlos López <clopez@suse.de> wrote:
> >>
> >> [...]
> >>
> >>>> regarding this series:
> >>>>
> >>>> on x86 you don't care what the new permissions are, because you're
> >>>> updating when re-faulting (if I understood correctly).  
> >>>
> >>> Correct.
> >>>
> >>>> for s390 it might be beneficial to know what the new permissions are;
> >>>> of course we could look up the vma and find out, but since you are
> >>>> already checking the permissions, maybe you can find a way to also
> >>>> convey the new permission bits?
> >>>>
> >>>> that would make our life on s390 easier :)  
> >>>
> >>> Right, getting the new bits from vma->flags should be trivially done
> >>> from the MMU_NOTIFY_PROTECTION_VMA callsites. There'd be no users in
> >>> this series though.  The rest of architectures still need the notifier to
> >>> tell them if the change is an upgrade, since they don't have an easy way
> >>> to grab the old RWX bits. Passing the old RWX bits from the MMU notifier
> >>> does not seem trivial to me either.
> > 
> > But why is core MM even doing mmu_notifier_invalidate_range_{start,end}() in the
> > first place?  I.e. why add MMU_NOTIFIER_RANGE_PERM_UPGRADE?  Just skip the
> > "invalidation".
> 
> I briefly mention this in the cover letter, but in short, denying other
> users the notification seems like an overly intrusive change for KVM's
> benefit. 

There are all of two other users:

  $ git grep -w "\.invalidate_range_start"
  drivers/gpu/drm/nouveau/nouveau_svm.c:  .invalidate_range_start = nouveau_svmm_invalidate_range_start,
  drivers/infiniband/hw/hfi1/mmu_rb.c:    .invalidate_range_start = mmu_notifier_range_start,
  virt/kvm/kvm_main.c:    .invalidate_range_start = kvm_mmu_notifier_invalidate_range_start,

and AFAICT neither of them wants an invalidation even for protection upgrades
either.

> A flag allows users to opt in instead.
> 
> It is also not clear to me that secondary MMUs are not relying on the
> notification in some subtle way. In fact, Sashiko points out in reply to
> patch 4 that the notification lets KVM retry the APIC access page reload
> in case userspace shoots itself in the foot by calling
> mprotect(PROT_READ) on it. This would require a really weird VMM,

s/weird/misbehaving

> as it would need to go looking for the page's address, but breaking this
> would be a "regression" nonetheless. I will address this in v2.

Nah, there's zero chance any real VMM is doing mprotect() on the APIC access page.

More importantly, doing nothing if updating the mapping fails and APICv is enabled
is wrong.  KVM needs to ensure vmcs.APIC_ACCESS_ADDR is fresh when running a vCPU
with APICv enabled, even if that means putting the vCPU into an infinite loop.
VMX doesn't support "inhibiting" the APIC access page, i.e. the memslot can't
disappear while the VM is alive, and if userspace mucks with the VMA, then KVM
needs to prevent running the vCPU until the mapping is restored, irrespective of
mmu_notifiers.

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 464d6ed862d5..11b24a42e482 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6928,8 +6928,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
@@ -6946,8 +6948,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;
+	}
 
 	read_lock(&vcpu->kvm->mmu_lock);
 	if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))