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

Konstantin Fastov <[email protected]>
Newsgroups dev.linux.lists.regressions,org.kernel.vger.linux-kernel,org.kernel.vger.stable,org.kvack.linux-mm
Message-ID <[email protected]>
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: [email protected]
Closes: https://github.com/AsahiLinux/linux/issues/560
Assisted-by: Claude:claude-fable-5
Signed-off-by: Konstantin Fastov <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.