[PATCH] drm/gem-shmem: Install writable PTEs for write faults

[PATCH] drm/gem-shmem: Install writable PTEs for write faults

[Konstantin Fastov]

Since the introduction of dirty tracking, drm_gem_shmem_vm_ops has a
.pfn_mkwrite handler.  Its presence makes vma_wants_writenotify() true,
so vma_set_page_prot() removes the write bit from vm_page_prot of
shared mappings, and the vmf_insert_pfn() call in the fault handler now
installs read-only PTEs even when serving a write fault.

For regular CPU accesses this is transparent: the retried access faults
again on the present read-only PTE, goes through wp_pfn_shared() into
.pfn_mkwrite() and the PTE is upgraded to writable.  But consumers that
resolve faults through fixup_user_fault() + follow_pfnmap_start()
perform no such retry.  In particular KVM's hva_to_pfn_remapped(),
after "successfully" handling a write fault, finds a present read-only
PTE, treats it as KVM_PFN_ERR_RO_FAULT and fails the vcpu run with
EFAULT.  Observed as AsahiLinux/linux#560: muvm/libkrun microVMs
mapping virtio-gpu blob resources die with EFAULT on first GPU access.
The traced failing sequence:

  follow_pfnmap_start()        -> -EINVAL   (no PTE yet)
  fixup_user_fault(WRITE)
    drm_gem_shmem_fault()
      vmf_insert_pfn()         -> NOPAGE    (read-only PTE installed)
  fixup_user_fault()           -> 0         (fault "handled")
  follow_pfnmap_start()        -> 0, !writable
  hva_to_pfn()                 -> KVM_PFN_ERR_RO_FAULT

Fix it the same way commit cb2a2a5b37ad ("drm/shmem_helper: Make sure
PMD entries get the writeable upgrade") did for the PMD path: when the
fault is a write fault, install a writable entry directly and record
the write for dirty tracking, instead of relying on a refault that not
every fault-resolution path performs.

To do that at PTE level, add vmf_insert_pfn_mkwrite(), the VM_PFNMAP
counterpart of vmf_insert_mixed_mkwrite(): insert_pfn() already
implements the mkwrite semantics, there was just no wrapper exposing it
for pfn inserts with the default pgprot.

Fixes: 28e3918179aa ("drm/gem-shmem: Track folio accessed/dirty status in mmap")
Cc: stable@vger.kernel.org
Closes: https://github.com/AsahiLinux/linux/issues/560
Assisted-by: Claude:claude-fable-5
Signed-off-by: Konstantin Fastov <kfastov@gmail.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 20 +++++++++++++-
 include/linux/mm.h                     |  2 ++
 mm/memory.c                            | 36 +++++++++++++++++++++++---
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 545933c7f712..37662f6f6ed9 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -573,7 +573,25 @@ static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
 				 unsigned long pfn)
 {
 	if (!order) {
-		return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
+		vm_fault_t ret;
+
+		/* .pfn_mkwrite enables write-notify, so vm_page_prot lacks
+		 * the write bit and a plain vmf_insert_pfn() would install
+		 * a read-only PTE even for a write fault.  Not every fault
+		 * resolver refaults into .pfn_mkwrite() to upgrade it (KVM
+		 * doesn't), so install a writable PTE and record the write
+		 * directly, like the PMD path below.
+		 */
+		if (vmf->flags & FAULT_FLAG_WRITE) {
+			ret = vmf_insert_pfn_mkwrite(vmf->vma, vmf->address,
+						     pfn);
+			if (ret == VM_FAULT_NOPAGE)
+				drm_gem_shmem_record_mkwrite(vmf);
+		} else {
+			ret = vmf_insert_pfn(vmf->vma, vmf->address, pfn);
+		}
+
+		return ret;
 #ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
 	} else if (order == PMD_ORDER) {
 		unsigned long paddr = pfn << PAGE_SHIFT;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 80fce2515930..7b76a281bdd1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4551,6 +4551,8 @@ vm_fault_t vmf_insert_page_mkwrite(struct vm_fault *vmf, struct page *page,
 			bool write);
 vm_fault_t vmf_insert_pfn(struct vm_area_struct *vma, unsigned long addr,
 			unsigned long pfn);
+vm_fault_t vmf_insert_pfn_mkwrite(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn);
 vm_fault_t vmf_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr,
 			unsigned long pfn, pgprot_t pgprot);
 vm_fault_t vmf_insert_mixed(struct vm_area_struct *vma, unsigned long addr,
diff --git a/mm/memory.c b/mm/memory.c
index 86a973119bd4..6562a0d8bb68 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2751,8 +2751,9 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
  * Context: Process context.  May allocate using %GFP_KERNEL.
  * Return: vm_fault_t value.
  */
-vm_fault_t vmf_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr,
-			unsigned long pfn, pgprot_t pgprot)
+static vm_fault_t __vmf_insert_pfn_prot(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, pgprot_t pgprot,
+		bool mkwrite)
 {
 	/*
 	 * Technically, architectures with pte_special can avoid all these
@@ -2774,10 +2775,39 @@ vm_fault_t vmf_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr,
 
 	pfnmap_setup_cachemode_pfn(pfn, &pgprot);
 
-	return insert_pfn(vma, addr, pfn, pgprot, false);
+	return insert_pfn(vma, addr, pfn, pgprot, mkwrite);
+}
+
+vm_fault_t vmf_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr,
+			unsigned long pfn, pgprot_t pgprot)
+{
+	return __vmf_insert_pfn_prot(vma, addr, pfn, pgprot, false);
 }
 EXPORT_SYMBOL(vmf_insert_pfn_prot);
 
+/**
+ * vmf_insert_pfn_mkwrite - insert single pfn into user vma, making it writable
+ * @vma: user vma to map to
+ * @addr: target user address of this page
+ * @pfn: source kernel pfn
+ *
+ * Similar to vmf_insert_pfn(), but the inserted PTE is made young, dirty and
+ * writable (subject to the VMA's write permission).  For use by fault
+ * handlers of write-notify VMAs (where vma->vm_page_prot has the write bit
+ * removed) that serve a write fault and do their own dirty accounting, so
+ * that a "handled" write fault always results in a writable PTE.  The
+ * VM_PFNMAP counterpart of vmf_insert_mixed_mkwrite().
+ *
+ * Context: Process context.  May allocate using %GFP_KERNEL.
+ * Return: vm_fault_t value.
+ */
+vm_fault_t vmf_insert_pfn_mkwrite(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn)
+{
+	return __vmf_insert_pfn_prot(vma, addr, pfn, vma->vm_page_prot, true);
+}
+EXPORT_SYMBOL(vmf_insert_pfn_mkwrite);
+
 /**
  * vmf_insert_pfn - insert single pfn into user vma
  * @vma: user vma to map to
-- 
2.55.0

Re: [PATCH] drm/gem-shmem: Install writable PTEs for write faults

[Boris Brezillon]

Hello Konstantin,

On Tue,  4 Aug 2026 17:04:03 +0300
Konstantin Fastov <kfastov@gmail.com> wrote:

> Since the introduction of dirty tracking, drm_gem_shmem_vm_ops has a
> .pfn_mkwrite handler.  Its presence makes vma_wants_writenotify() true,
> so vma_set_page_prot() removes the write bit from vm_page_prot of
> shared mappings, and the vmf_insert_pfn() call in the fault handler now
> installs read-only PTEs even when serving a write fault.
> 
> For regular CPU accesses this is transparent: the retried access faults
> again on the present read-only PTE, goes through wp_pfn_shared() into
> .pfn_mkwrite() and the PTE is upgraded to writable.  But consumers that
> resolve faults through fixup_user_fault() + follow_pfnmap_start()
> perform no such retry.  In particular KVM's hva_to_pfn_remapped(),
> after "successfully" handling a write fault, finds a present read-only
> PTE, treats it as KVM_PFN_ERR_RO_FAULT and fails the vcpu run with
> EFAULT.  Observed as AsahiLinux/linux#560: muvm/libkrun microVMs
> mapping virtio-gpu blob resources die with EFAULT on first GPU access.
> The traced failing sequence:
> 
>   follow_pfnmap_start()        -> -EINVAL   (no PTE yet)
>   fixup_user_fault(WRITE)
>     drm_gem_shmem_fault()
>       vmf_insert_pfn()         -> NOPAGE    (read-only PTE installed)
>   fixup_user_fault()           -> 0         (fault "handled")
>   follow_pfnmap_start()        -> 0, !writable
>   hva_to_pfn()                 -> KVM_PFN_ERR_RO_FAULT
> 
> Fix it the same way commit cb2a2a5b37ad ("drm/shmem_helper: Make sure
> PMD entries get the writeable upgrade") did for the PMD path: when the
> fault is a write fault, install a writable entry directly and record
> the write for dirty tracking, instead of relying on a refault that not
> every fault-resolution path performs.
> 
> To do that at PTE level, add vmf_insert_pfn_mkwrite(), the VM_PFNMAP
> counterpart of vmf_insert_mixed_mkwrite(): insert_pfn() already
> implements the mkwrite semantics, there was just no wrapper exposing it
> for pfn inserts with the default pgprot.

The same issue is being discussed here [1].

Regards,

Boris

[1]https://lore.kernel.org/dri-devel/20260804120529.1730187-1-pbonzini@redhat.com/

Re: [PATCH] drm/gem-shmem: Install writable PTEs for write faults

[Konstantin Fastov]

Hello Boris,

вт, 4 авг. 2026 г. в 17:29, Boris Brezillon <boris.brezillon@collabora.com>:
>
> Hello Konstantin,
>
> On Tue,  4 Aug 2026 17:04:03 +0300
> Konstantin Fastov <kfastov@gmail.com> wrote:
>
> > Since the introduction of dirty tracking, drm_gem_shmem_vm_ops has a
> > .pfn_mkwrite handler.  Its presence makes vma_wants_writenotify() true,
> > so vma_set_page_prot() removes the write bit from vm_page_prot of
> > shared mappings, and the vmf_insert_pfn() call in the fault handler now
> > installs read-only PTEs even when serving a write fault.
> >
> > For regular CPU accesses this is transparent: the retried access faults
> > again on the present read-only PTE, goes through wp_pfn_shared() into
> > .pfn_mkwrite() and the PTE is upgraded to writable.  But consumers that
> > resolve faults through fixup_user_fault() + follow_pfnmap_start()
> > perform no such retry.  In particular KVM's hva_to_pfn_remapped(),
> > after "successfully" handling a write fault, finds a present read-only
> > PTE, treats it as KVM_PFN_ERR_RO_FAULT and fails the vcpu run with
> > EFAULT.  Observed as AsahiLinux/linux#560: muvm/libkrun microVMs
> > mapping virtio-gpu blob resources die with EFAULT on first GPU access.
> > The traced failing sequence:
> >
> >   follow_pfnmap_start()        -> -EINVAL   (no PTE yet)
> >   fixup_user_fault(WRITE)
> >     drm_gem_shmem_fault()
> >       vmf_insert_pfn()         -> NOPAGE    (read-only PTE installed)
> >   fixup_user_fault()           -> 0         (fault "handled")
> >   follow_pfnmap_start()        -> 0, !writable
> >   hva_to_pfn()                 -> KVM_PFN_ERR_RO_FAULT
> >
> > Fix it the same way commit cb2a2a5b37ad ("drm/shmem_helper: Make sure
> > PMD entries get the writeable upgrade") did for the PMD path: when the
> > fault is a write fault, install a writable entry directly and record
> > the write for dirty tracking, instead of relying on a refault that not
> > every fault-resolution path performs.
> >
> > To do that at PTE level, add vmf_insert_pfn_mkwrite(), the VM_PFNMAP
> > counterpart of vmf_insert_mixed_mkwrite(): insert_pfn() already
> > implements the mkwrite semantics, there was just no wrapper exposing it
> > for pfn inserts with the default pgprot.
>
> The same issue is being discussed here [1].
>
> Regards,
>
> Boris
>
> [1]https://lore.kernel.org/dri-devel/20260804120529.1730187-1-pbonzini@redhat.com/

Thanks, I missed that series, I searched dri-devel but not the kvm list.
Paolo's patches 1-2 are indeed the same fix (down to the same function
name), so please consider my patch superseded by his series.

I have a fast reproducer on Apple Silicon (Asahi, muvm microVM, dies in
under a second on stock 7.1.5) and an automated test rig, so I'll run
Paolo's full series on that hardware and follow up with Tested-by in his
thread.

Thanks,
Konstantin

Re: [PATCH] drm/gem-shmem: Install writable PTEs for write faults

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect page offset calculation for split VMAs in drm_gem_shmem fault handlers.
--

--- Patch [1]: [PATCH] drm/gem-shmem: Install writable PTEs for write faults ---
commit e62e2d7a0bc9f9a9b0ed15d3b6506aec1a894539
Author: Konstantin Fastov <kfastov@gmail.com>

    drm/gem-shmem: Install writable PTEs for write faults
    
    Since the introduction of dirty tracking, drm_gem_shmem_vm_ops has a
    .pfn_mkwrite handler, which causes read-only PTEs to be installed even
    for write faults. This commit fixes the issue by directly installing
    writable PTEs for write faults and recording the write for dirty tracking.
    This mirrors a similar fix previously applied to the PMD path.

    Fixes: 28e3918179aa ("drm/gem-shmem: Track folio accessed/dirty status in mmap")
    Cc: stable@vger.kernel.org
    Closes: https://github.com/AsahiLinux/linux/issues/560
    Assisted-by: Claude:claude-fable-5
    Signed-off-by: Konstantin Fastov <kfastov@gmail.com>

> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 06d019d51d3e6..2afef0b9bb5b0 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -593,7 +593,25 @@ static vm_fault_t try_insert_pfn(struct vm_fault *vmf, unsigned int order,
>  				 unsigned long pfn)
>  {
>  	if (!order) {
> -		return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
> +		vm_fault_t ret;
> +
> +		/* .pfn_mkwrite enables write-notify, so vm_page_prot lacks
> +		 * the write bit and a plain vmf_insert_pfn() would install
> +		 * a read-only PTE even for a write fault.  Not every fault
> +		 * resolver refaults into .pfn_mkwrite() to upgrade it (KVM
> +		 * doesn't), so install a writable PTE and record the write
> +		 * directly, like the PMD path below.
> +		 */
> +		if (vmf->flags & FAULT_FLAG_WRITE) {
> +			ret = vmf_insert_pfn_mkwrite(vmf->vma, vmf->address,
> +						     pfn);
> +			if (ret == VM_FAULT_NOPAGE)
> +				drm_gem_shmem_record_mkwrite(vmf);

[Severity: High]
This is a pre-existing issue, but does drm_gem_shmem_record_mkwrite()
calculate the correct page offset for split VMAs?

In drm_gem_shmem_record_mkwrite(), the page offset is calculated as:
    pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff;

If userspace maps a GEM shmem object and then splits the VMA (e.g., via a
partial munmap or mprotect), this yields the offset relative to the VMA
start rather than the GEM object. This causes the wrong folio to be marked
dirty, leading to potential data loss if the actually modified pages aren't
written back.

A similar issue appears in drm_gem_shmem_any_fault(), which also calculates
page_offset as vmf->pgoff - vma->vm_pgoff, causing subsequent faults on the
remaining split VMA to map the wrong physical pages.

> +		} else {
> +			ret = vmf_insert_pfn(vmf->vma, vmf->address, pfn);
> +		}
> +
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804140403.16337-1-kfastov@gmail.com?part=1