Re: mapping files for iommufd

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 12 Aug 2024 14:56:03 -0400
Steven Sistare <steven.sistare@oracle.com> wrote:

> On 8/12/2024 2:40 PM, Jason Gunthorpe wrote:
> > On Mon, Aug 12, 2024 at 02:09:11PM -0400, Steven Sistare wrote:  
> >> I have been using QEMU to test legacy vfio vs iommufd compatibility vfio.
> >> I find that backing guest memory with an ext4 file /root/guest.ram
> >> succeeds for legacy but fails for iommufd, with EFAULT in
> >> IOMMU_IOAS_MAP -> ... __get_user_pages -> check_vma_flags.  Ditto for a
> >> hugetlbfs file.  This is with unmodified kernels, no live update changes.
> >>
> >> Is this expected?  
> > 
> > Yes - I'm surprised that VFIO is not failing though?
> > 
> > /*
> >   * Writing to file-backed mappings which require folio dirty tracking using GUP
> >   * is a fundamentally broken operation, as kernel write access to GUP mappings
> >   * do not adhere to the semantics expected by a file system.
> >   *
> >   * Consider the following scenario:-
> >   *
> >   * 1. A folio is written to via GUP which write-faults the memory, notifying
> >   *    the file system and dirtying the folio.
> >   * 2. Later, writeback is triggered, resulting in the folio being cleaned and
> >   *    the PTE being marked read-only.
> >   * 3. The GUP caller writes to the folio, as it is mapped read/write via the
> >   *    direct mapping.
> >   * 4. The GUP caller, now done with the page, unpins it and sets it dirty
> >   *    (though it does not have to).
> >   *
> >   * This results in both data being written to a folio without writenotify, and
> >   * the folio being dirtied unexpectedly (if the caller decides to do so).
> >   */
> > static bool writable_file_mapping_allowed(struct vm_area_struct *vma,
> > 					  unsigned long gup_flags)
> > {
> > 
> > Why doesn't that fail in the VFIO case? I didn't notice anything
> > special there that would make it work???  
> 
> vfio_dma_do_map -> vfio_pin_map_dma -> vfio_pin_pages_remote does not call GUP,
> it just grabs a reference to each page.

Can you be more specific, because...

vfio_pin_pages_remote
  vaddr_get_pfns
    pin_user_pages_remote
      __gup_longterm_locked
        __get_user_pages_locked
          __get_user_pages

Thanks,
Alex