Re: [PATCH v1] vfio/pci: pass FAULT_FLAG_WRITE to vmf_insert_pfn_pmd/pud()

[Alex Williamson]

On Sat, 24 Oct 2026 01:55:40 +0000
Mina Almasry <almasrymina@xxxxxxxxxx> wrote:

> vfio_pci_vmf_insert_pfn() unconditionally passes write=false to
> vmf_insert_pfn_pmd() and vmf_insert_pfn_pud().
> 
> In insert_pmd() and insert_pud(), write=false skips marking newly
> inserted PMD/PUD entries dirty/young via pmd_mkyoung(pmd_mkdirty()) and
> maybe_pmd_mkwrite(). Furthermore, when vma_wants_writenotify() is active
> on a shared writable VMA (for example, after CLEAR_REFS_SOFT_DIRTY
> clears VM_SOFTDIRTY and calls vma_set_page_prot()), vm_page_prot is
> downgraded to read-only so insert_pmd()/insert_pud() with write=false
> installs a write-protected huge PMD/PUD. Subsequent write faults enter
> wp_huge_pmd() or wp_huge_pud(), which invoke vma->vm_ops->huge_fault()
> to upgrade the entry in place. Because vfio_pci_vmf_insert_pfn() passes
> write=false, insert_pmd() and insert_pud() see !pmd_none(*pmd) with
> !write, do nothing, and return VM_FAULT_NOPAGE without setting the write
> bit or splitting the huge page, causing an infinite write page-fault
> loop.
> 
> Pass vmf->flags & FAULT_FLAG_WRITE to vmf_insert_pfn_pmd() and
> vmf_insert_pfn_pud() so write faults properly mark huge PFNMAP entries
> dirty and writable.

This seems correct to me, but the commit log misses the human
practicality portion of the argument, or at least buries it too subtly
for me in the soft-dirty references.  AIUI, this is effectively only
reached by something like CRIU twiddling clear_refs via procfs on a
running process.  That's the kind of thing someone evaluating this for
stable would want to know.

Given this unique use case, this also looks like a selftest candidate
(Cc +David).  Thanks,

Alex
 
> Fixes: f9e54c3a2f5b ("vfio/pci: implement huge_fault support")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Ankit Agrawal <ankita@xxxxxxxxxx>
> Cc: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index fcbb2da860e2..204d959086e9 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1745,6 +1745,8 @@ vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev,
>  				   unsigned long pfn,
>  				   unsigned int order)
>  {
> +	bool write = vmf->flags & FAULT_FLAG_WRITE;
> +
>  	lockdep_assert_held_read(&vdev->memory_lock);
>  
>  	if (vdev->pm_runtime_engaged || !__vfio_pci_memory_enabled(vdev))
> @@ -1754,10 +1756,10 @@ vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev,
>  		return vmf_insert_pfn(vmf->vma, vmf->address, pfn);
>  
>  	if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PMD_PFNMAP) && order == PMD_ORDER)
> -		return vmf_insert_pfn_pmd(vmf, pfn, false);
> +		return vmf_insert_pfn_pmd(vmf, pfn, write);
>  
>  	if (IS_ENABLED(CONFIG_ARCH_SUPPORTS_PUD_PFNMAP) && order == PUD_ORDER)
> -		return vmf_insert_pfn_pud(vmf, pfn, false);
> +		return vmf_insert_pfn_pud(vmf, pfn, write);
>  
>  	return VM_FAULT_FALLBACK;
>  }
> 
> base-commit: 076d7e989cb69f6ded7d60f2a080eb909930061b