On Mon, May 05, 2025 at 01:50:19PM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 28, 2025 at 10:50:32AM +0800, Baolu Lu wrote:
> > On 4/26/25 13:58, Nicolin Chen wrote:
> > > +/* 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
Will fix.
> > > +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
I added in v3 a pair of open/close op that would refcount the
vIOMMU object (owner of the mmap region). This would EBUSY the
vIOMMU destroy ioctl that would call this function.
Thanks
Nicolin
