Re: [PATCH v2 13/22] iommufd: Add mmap interface

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Apr 28, 2025 at 10:50:32AM +0800, Baolu Lu wrote:
> On 4/26/25 13:58, Nicolin Chen wrote:
> > For vIOMMU passing through HW resources to user space (VMs), add an mmap
> > infrastructure to map a region of hardware MMIO pages.
> > 
> > Maintain an mt_mmap per ictx for validations. To allow IOMMU drivers to
> > add and delete mmappable regions to/from the mt_mmap, add a pair of new
> > helpers: iommufd_ctx_alloc_mmap() and iommufd_ctx_free_mmap().
> 
> I am wondering why the dma_buf mechanism isn't used here, considering
> that this also involves an export and import pattern.

The provider will be a memfd or something, that isn't where we want to
put dmabuf..

> > +/* Entry for iommufd_ctx::mt_mmap */
> > +struct iommufd_mmap {
> > +	unsigned long pfn_start;
> > +	unsigned long pfn_end;
> > +};
> 
> This structure is introduced to represent a mappable/mapped region,
> right? It would be better to add comments specifying whether the start
> and end are inclusive or exclusive.

start/end are supposed to be non-inclusive range in iommufd
land. start/last for inclusive.

This should be a u64 too

> > +void iommufd_ctx_free_mmap(struct iommufd_ctx *ictx, unsigned long immap_id)
> > +{
> > +	kfree(mtree_erase(&ictx->mt_mmap, immap_id >> PAGE_SHIFT));
> 
> MMIO lifecycle question: what happens if a region is removed from the
> maple tree (and is therefore no longer mappable), but is still mapped
> and in use by userspace?

I think we should probably zap it and make any existing VMAs
SIGBUS... Otherwise it is hard to reason about from the kernel side

Jason