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

[Sergio Lopez Pascual]

Sean Christopherson <seanjc@xxxxxxxxxx> 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 <slp@xxxxxxxxxx>
>> ---
>
> 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.

Thanks,
Sergio.