RE: [PATCH 01/11] iommu: Add device dma ownership set/release interfaces

From: Tian, Kevin
Date: Wed Nov 17 2021 - 21:40:00 EST


> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, November 16, 2021 9:46 PM
>
> On Tue, Nov 16, 2021 at 09:57:30AM +0800, Lu Baolu wrote:
> > Hi Christoph,
> >
> > On 11/15/21 9:14 PM, Christoph Hellwig wrote:
> > > On Mon, Nov 15, 2021 at 10:05:42AM +0800, Lu Baolu wrote:
> > > > +enum iommu_dma_owner {
> > > > + DMA_OWNER_NONE,
> > > > + DMA_OWNER_KERNEL,
> > > > + DMA_OWNER_USER,
> > > > +};
> > > > +
> > >
> > > > + enum iommu_dma_owner dma_owner;
> > > > + refcount_t owner_cnt;
> > > > + struct file *owner_user_file;
> > >
> > > I'd just overload the ownership into owner_user_file,
> > >
> > > NULL -> no owner
> > > (struct file *)1UL) -> kernel
> > > real pointer -> user
> > >
> > > Which could simplify a lot of the code dealing with the owner.
> > >
> >
> > Yeah! Sounds reasonable. I will make this in the next version.
>
> It would be good to figure out how to make iommu_attach_device()
> enforce no other driver binding as a kernel user without a file *, as
> Robin pointed to, before optimizing this.
>
> This fixes an existing bug where iommu_attach_device() only checks the
> group size and is vunerable to a hot plug increasing the group size
> after it returns. That check should be replaced by this series's logic
> instead.
>

I think this existing bug in iommu_attach_devce() is different from
what this series is attempting to solve. To avoid breaking singleton
group assumption there the ideal band-aid is to fail device hotplug.
Otherwise some IOVA ranges which are supposed to go upstream
to IOMMU may be considered as p2p and routed to the hotplugged
device instead. In concept a singleton group is different from a
multi-devices group which has only one device bound to driver...

This series aims to avoid conflict having both user and kernel drivers
mixed in a multi-devices group.

Thanks
Kevin