Re: [PATCH v3] KVM: have hva_to_pfn_remapped write-upgrade PTEs

On Fri, Jul 31, 2026, Sergio Lopez Pascual wrote:
> Sean Christopherson <[email protected]> writes:
> 
> > On Wed, Jul 29, 2026, Sergio Lopez wrote:
> >> After 28e39181 ("drm/gem-shmem: Track folio accessed/dirty status in
> >> mmap") was merged, a guest write to an unpopulated PTE from a mapping
> >> backed by a DRM GEM BO triggers a VM exit with EFAULT, with
> >> hva_to_pfn_remapped setting p_pfn to KVM_PFN_ERR_RO_FAULT.
> >>
> >> This happens because that commit implements pfn_mkwrite for
> >> drm_gem_shmem_vm_ops. With that function present, vma_wants_writenotify
> >> returns true in vma_set_page_prot, clearing VM_SHARED and leading to the
> >> entry to be installed as read-only. This is done on purpose so the
> >> fault handler gets notified when the entry is going to be written.
> >>
> >> In KVM, hva_to_pfn_remapped calls to fixup_user_fault to trigger the
> >> fault handler but, as seen above, this one might install a read-only PTE
> >> even with FAULT_FLAG_WRITE present in fault_flags. The check at the end
> >> of hva_to_pfn_remapped notices that the entry is not writable despite
> >> this being a write fault and sets p_pfn to KVM_PFN_ERR_RO_FAULT.
> >>
> >> To address this issue, have hva_to_pfn_remapped issue a second
> >> fixup_user_fault call when needed for write-upgrading the PTE.
> >>
> >> Signed-off-by: Sergio Lopez <[email protected]>
> >> ---
> >
> > NAK, this doesn't belong in KVM.  Expecting callers of fixup_user_fault() to
> > retry a FAULT_FLAG_WRITE fault on *success* is absurd.  Either manually do the
> > retry in fixup_user_fault(), or return VM_FAULT_RETRY so that KVM will naturally
> > retry.  I assume the latter is the correct approach.
> 
> The issue goes beyond fixup_user_fault() behavior. If we're faulting on
> a page which already has a ready-only PTE installed,
> follow_pfnmap_start() returns success, fixup_user_fault isn't called at
> all, and we're still hitting the "(write_fault && !args.writable)"
> condition, setting p_pfn to KVM_PFN_ERR_RO_FAULT.
> 
> Without this change, KVM can't deal with VM_PFNMAP vmas with read-only
> PTEs installed.

IMO, that's a bug in the APIs.  If a normal #PF handler looked at a write fault
and decided a read-only mapping sufficed, that would be a bug.  I don't see why
this is any different.

And if KVM has this problem, so will other users of follow_pfnmap_start() and
friends.