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

[Sean Christopherson <seanjc@google.com>]

On Wed, Nov 04, 2026, Carlos López wrote:
> On 11/3/26 4:03 PM, Sean Christopherson wrote:
> > 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.
> 
> There are more users that register for notifications via

Ugh, I didn't realize we gained a variant of .invalidate_range_{start,end}(),
I thought those were for what is now arch_invalidate_secondary_tlbs().

> mmu_interval_notifier_insert():
> 
>   $ git grep -l -e "mmu_interval_notifier_insert(" \
>           -e "mmu_interval_notifier_insert_locked(" -- drivers/ | wc -l
>   12
> 
> I haven't audited all of them, but I'd say that at least amdkfd relies
> on the invalidation for upgrades. Without XNACK, GPU page faults are not
> retried, so the driver maps ranges up front with the VMA permissions
> (svm_range_validate_and_map()) and relies on an invalidation to remap
> them when those change. So mprotect(PROT_READ) followed by
> mprotect(PROT_READ|PROT_WRITE) would cause the GPU mapping to stay
> read-only and the next GPU write to fault. Unlike the APIC access page,
> this is the application's own memory, mprotect() should be perfectly valid.
> 
> This is the kind of subtle reliance that I mentioned before, and why I'd
> rather not open that can of worms.

Yeah, agreed.

> >> 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.
> 
> Fair point. Should I just carry the diff below in v2 with your
> Signed-off-by?

Nah, I'll send a standalone patch.  It doesn't have a direct relationship to your
series, and the bug is pre-existing despite what Sashiko thinks.

Thanks!