Re: mapping files for iommufd

[Steven Sistare <steven.sistare@oracle.com>]

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.

> I recall we talked about allowing old stuff to keep working but I
> don't see any implementation of that?

Nor I.  Unless "keep old stuff working" is implemented by omitting the CONFIG
definitions for iommufd.

- Steve