Re: [Devel] [PATCH hci-8.1 1/2] mm: allow MADV_DOEXEC on shared PFN mappings of device memory

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

On 10/14/26 8:14 PM, Konstantin Khorenko wrote:
> From: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> 
> QEMU live update (cpr-exec) keeps guest RAM across exec with MADV_DOEXEC,
> but VFIO device BARs mapped into QEMU are lost on exec, and the new QEMU
> has to mmap them again on the same device fd.
> 
> Some drivers don't allow that. The NVIDIA vGPU VFIO driver records each
> BAR mapping and refuses an overlapping one with -EINVAL. For VF BARs the
> record is dropped only when the device fd is closed, so after exec the
> new QEMU can't map BAR1/BAR3 and the vGPU becomes unusable.
> 
> Let MADV_DOEXEC also preserve shared VM_PFNMAP mappings of a VFIO device
> file, the BARs. vma_dup() duplicates them the way dup_mmap() duplicates a
> shared file mapping: clear the inherited VM_PAT so that track_pfn_copy()
> reserves the memtype for the copy, fork the anon_vma if the old VMA
> ever got one, link the VMA into the file's i_mmap (so
> unmap_mapping_range() on device reset still reaches it), call
> vm_ops->open() and copy the special PTEs with copy_page_range().
> vma_link() places the copy after the original in i_mmap, since an equal
> key goes right in the interval tree, which is the order dup_mmap() gets
> from vma_interval_tree_insert_after().
> 
> Admit only a VFIO file, checked with vfio_file_is_valid() as KVM does;
> of those only a device file can be mapped. Other drivers keep per-mm
> state in such mappings and rely on VM_DONTCOPY to keep fork away: perf
> charges a ring buffer to the mm's pinned_vm, so a copy would drive the
> counter of the new mm negative and lift RLIMIT_MEMLOCK, and Xen privcmd
> frees per-mapping state in ->close() without an ->open(). VM_DONTCOPY
> itself cannot be the test, because QEMU marks every RAM block
> MADV_DONTFORK, VFIO BARs included. A VFIO mapping whose vm_ops has
> ->close() but no ->open() is refused as well, since the copy would run
> ->close() a second time.
> 
> No in-tree VFIO driver keeps per-VMA state, and the vfio-pci family
> revokes BAR mappings through the device file's mapping, which reaches
> the copy. An out-of-tree driver that tracks its mappings has to count
> the copy in ->open(). One that maps with remap_pfn_range(), which sets
> VM_PAT, and revokes with unmap_mapping_range() inherits the windows fork
> has between track_pfn_copy() and a concurrent zap, and the one munmap has
> between the zap of the old VMA and a device reset; no in-tree VFIO
> driver does both.
> 
> Dropping the mark gives VM_MAYEXEC back only to anonymous memory, where
> do_mmap() decides it. A marked PFN map does not get it back, since there
> the driver had the last word.
> 
> Once the copy is in i_mmap, a zap from a device reset may clear VM_PAT
> of either VMA holding only i_mmap_rwsem. So clear VM_EXEC_KEEP of the
> copy before linking it, and touch VM_ACCOUNT of the old VMA only when it
> is set. The source VMA is write-locked explicitly, as in dup_mmap(),
> instead of through that vm_flags_clear().
> 
> Such a VMA may have NULL vm_ops, in which case vma_is_anonymous()
> returns true for it. So the new vma_is_shared_pfnmap() helper is checked
> first, also in zap_vma_mt(), whose threaded path for anonymous memory
> would skip untrack_pfn().
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-150233
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> Feature: oracle/mm: MADV_DOEXEC madvise() flag
> (cherry picked from commit f1903df83b065cd59ebe30cfcf5fde6391fcc4e3)
> 
> Changes while porting: added the untrack_pfn_clear() call, the
> unconditional anon_vma_fork(), the VFIO-only admission, the VM_MAYEXEC
> handling and the flag ordering above; the file reference now comes from
> an earlier fix that takes it for every file-backed VMA.
> 
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>  include/linux/mm.h | 12 ++++++++++--
>  mm/madvise.c       | 30 +++++++++++++++++++++++++-----
>  mm/memory.c        |  2 +-
>  mm/mmap.c          | 53 ++++++++++++++++++++++++++++++++++++++---------------
>  4 files changed, 74 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 91f305fd5c025..496cbdfc7141a 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1018,6 +1018,16 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
>  	return !vma->vm_ops;
>  }
>  
> +/*
> + * Shared PFN mapping of a file, such as a device BAR mapped through VFIO.
> + * Its vm_ops may be NULL, so it must be tested before vma_is_anonymous().
> + */
> +static inline bool vma_is_shared_pfnmap(struct vm_area_struct *vma)
> +{
> +	return vma->vm_file &&
> +	       (vma->vm_flags & (VM_SHARED | VM_PFNMAP)) == (VM_SHARED | VM_PFNMAP);
> +}
> +
>  /*
>   * Indicate if the VMA is a heap for the given task; for
>   * /proc/PID/maps that is the heap of the main task.
> @@ -2481,10 +2491,8 @@ void free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
>  		unsigned long end, unsigned long floor, unsigned long ceiling);
>  int
>  copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma);
> -#ifdef CONFIG_PADATA
>  int copy_page_range_mt(struct vm_area_struct *dst_vma,
>  			struct vm_area_struct *src_vma);
> -#endif
>  int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>  			void *buf, int len, int write);
>  
> diff --git a/mm/madvise.c b/mm/madvise.c
> index e6972bcfc9749..2b33d41b6ccf4 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -31,6 +31,7 @@
>  #include <linux/swapops.h>
>  #include <linux/shmem_fs.h>
>  #include <linux/mmu_notifier.h>
> +#include <linux/vfio.h>
>  
>  #include <asm/tlb.h>
>  
> @@ -1249,6 +1250,22 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
>  			       &guard_remove_walk_ops, NULL);
>  }
>  
> +/* Only VFIO device mappings are known to cope with a copy in another mm. */
> +static bool vma_doexec_pfnmap_ok(struct vm_area_struct *vma)
> +{
> +	bool (*is_vfio)(struct file *file);
> +	bool ret;
> +
> +	if (vma->vm_ops && vma->vm_ops->close && !vma->vm_ops->open)
> +		return false;

This breaks qemu-update with vGPU directly when attempting
madvise(MADV_DOEXEC):

  MADV_DOEXEC failed on memory region 0000:bd:00.4 region 0 mmaps[2]:
addr=0x7f5fc006c000 length=192512: Invalid argument

The reason being: NVIDIA maps the BAR0 pieces with vm_ops which has no
.open():

static const struct vm_operations_struct vgpu_sparse_ops = {
        .close = vgpu_mmio_close,
};

My previous commit didn't have this check and it worked.

Andrey


> +	is_vfio = symbol_get(vfio_file_is_valid);
> +	if (!is_vfio)
> +		return false;
> +	ret = is_vfio(vma->vm_file);
> +	symbol_put(vfio_file_is_valid);
> +	return ret;
> +}
> +
>  /*
>   * Apply an madvise behavior to a region of a vma.  madvise_update_vma
>   * will handle splitting a vm area into separate areas, each area with its own
> @@ -1309,12 +1326,15 @@ static int madvise_vma_behavior(struct vm_area_struct *vma,
>  		break;
>  	case MADV_DOEXEC:
>  		/*
> -		 * MADV_DOEXEC is only supported on private, non-executable,
> -		 * non-stack, non-special anonymous memory and if the
> -		 * VM_EXEC_KEEP flag is available.
> +		 * MADV_DOEXEC is supported on private, non-executable,
> +		 * non-stack, non-special anonymous memory, and on shared
> +		 * PFN mappings of VFIO devices, if the VM_EXEC_KEEP flag
> +		 * is available.
>  		 */
> -		if (!VM_EXEC_KEEP || !vma_is_anonymous(vma) ||
> -		    vma->vm_flags & (VM_EXEC | VM_SHARED | VM_STACK | VM_SPECIAL)) {
> +		if (!VM_EXEC_KEEP || vma->vm_flags & (VM_EXEC | VM_STACK) ||
> +		    (vma_is_shared_pfnmap(vma) ? !vma_doexec_pfnmap_ok(vma) :
> +		     !vma_is_anonymous(vma) ||
> +		     vma->vm_flags & (VM_SHARED | VM_SPECIAL))) {
>  			error = -EINVAL;
>  			goto out;
>  		}
> diff --git a/mm/memory.c b/mm/memory.c
> index 2566f02fb7d5e..2f15bc06ccf90 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2239,7 +2239,7 @@ void zap_vma_mt(struct vm_area_struct *vma)
>  	struct mmu_notifier_range range;
>  
>  	mmap_assert_write_locked(vma->vm_mm);
> -	if (vma_is_anonymous(vma)) {
> +	if (vma_is_anonymous(vma) && !vma_is_shared_pfnmap(vma)) {
>  		lru_add_drain();
>  		mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
>  					vma->vm_start, vma->vm_end);
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 69dd1514d750e..959810d8047a8 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1752,31 +1752,41 @@ int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
>  
>  /*
>   * The VM_MAYEXEC that MADV_DOEXEC took from @vma, to give back when the mark
> - * goes: what do_mmap() grants such a mapping.
> + * goes: what do_mmap() grants such a mapping. A driver may have withheld it
> + * from a PFN map, so leave that one without it.
>   */
>  vm_flags_t vma_doexec_mayexec(struct vm_area_struct *vma)
>  {
> -	if (vma->vm_file && path_noexec(&vma->vm_file->f_path))
> +	if (vma_is_shared_pfnmap(vma) ||
> +	    (vma->vm_file && path_noexec(&vma->vm_file->f_path)))
>  		return 0;
>  	return VM_MAYEXEC;
>  }
>  
>  /*
> - * Duplicate an anonymous VMA into a new mm as part of preserving
> - * it across exec.
> + * Duplicate a VMA into a new mm as part of preserving it across exec.
> + *
> + * Anonymous VMAs get their pages shared with the new mm fork-style.
> + * Shared PFN mappings (e.g. VFIO BARs) are duplicated the way fork
> + * duplicates a shared file mapping: same file, same special PTEs,
> + * nothing to copy-on-write.
>   */
>  int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
>  {
>  	struct vm_area_struct *vma;
>  	unsigned long npages;
> +	bool pfnmap = vma_is_shared_pfnmap(old_vma);
>  	int ret = -ENOMEM;
>  
> -	if (WARN_ON(!vma_is_anonymous(old_vma)))
> +	if (WARN_ON(!pfnmap && !vma_is_anonymous(old_vma)))
>  		return -EINVAL;
>  
>  	if (find_vma_intersection(mm, old_vma->vm_start, old_vma->vm_end))
>  		return -EEXIST;
>  
> +	/* As in dup_mmap(), keep per-VMA lock users out of the source. */
> +	vma_start_write(old_vma);
> +
>  	npages = vma_pages(old_vma);
>  	vm_stat_account(mm, old_vma->vm_flags, npages);
>  
> @@ -1784,6 +1794,10 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
>  	if (!vma)
>  		goto fail_nomem;
>  
> +	/* track_pfn_copy() will later take care of copying internal state. */
> +	if (pfnmap)
> +		untrack_pfn_clear(vma);
> +
>  	ret = vma_dup_policy(old_vma, vma);
>  	if (ret)
>  		goto fail_nomem_policy;
> @@ -1797,17 +1811,24 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
>  
>  	/*
>  	 * Clear functionality that should not carry over to the new
> -	 * process. Note that VM_EXEC_KEEP is cleared later to allow
> -	 * code called by copy_page_range to infer that the copying is
> -	 * for preserving over exec and not for process forking.
> +	 * process. Note that VM_EXEC_KEEP of an anonymous copy is cleared
> +	 * later to allow code called by copy_page_range to infer that the
> +	 * copying is for preserving over exec and not for process forking.
>  	 */
>  	vm_flags_clear(vma, VM_LOCKED|VM_LOCKONFAULT|VM_UFFD_MISSING|VM_UFFD_WP|VM_UFFD_MINOR);
>  	vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
> +	/* Once linked, unmap_mapping_range() may modify the flags concurrently. */
> +	if (pfnmap)
> +		vm_flags_clear(vma, VM_EXEC_KEEP);
>  
> +	/* Also links a file VMA into the file's i_mmap, for unmap_mapping_range(). */
>  	ret = vma_link(mm, vma);
>  	if (ret)
>  		goto fail_nomem_vma_link;
>  
> +	if (pfnmap && vma->vm_ops && vma->vm_ops->open)
> +		vma->vm_ops->open(vma);
> +
>  	/*
>  	 * Now that the dup vma is inserted into the mm, clear VM_ACCOUNT
>  	 * from old_vma.  Since vma_dup() is only called during exec to
> @@ -1815,14 +1836,16 @@ int vma_dup(struct vm_area_struct *old_vma, struct mm_struct *mm)
>  	 * process, this effectively transfers the accounting from the old
>  	 * vma to the new one.
>  	 */
> -	vm_flags_clear(old_vma, VM_ACCOUNT);
> +	if (old_vma->vm_flags & VM_ACCOUNT)
> +		vm_flags_clear(old_vma, VM_ACCOUNT);
>  
> -#ifdef CONFIG_PADATA
> -	ret = copy_page_range_mt(vma, old_vma);
> -#else
> -	ret = copy_page_range(vma, old_vma);
> -#endif
> -	vm_flags_mod(vma, vma_doexec_mayexec(vma), VM_EXEC_KEEP);
> +	/* For PFN maps this copies the device PTEs and the PAT memtype */
> +	if (pfnmap || !IS_ENABLED(CONFIG_PADATA))
> +		ret = copy_page_range(vma, old_vma);
> +	else
> +		ret = copy_page_range_mt(vma, old_vma);
> +	if (!pfnmap)
> +		vm_flags_mod(vma, vma_doexec_mayexec(vma), VM_EXEC_KEEP);
>  
>  	return ret;
>  
> 

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel