Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

[Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>]

On Wed, May 04, 2022 at 03:42:09PM +0100, Robin Murphy wrote:

> > This fixes an oops with VFIO and SMMUv3 because VFIO will call
> > iommu_detach_group() and then immediately iommu_domain_free(), but
> > SMMUv3 has no way to know that the domain it is holding a pointer to
> > has been freed. Now the iommu_detach_group() will assign the blocking
> > domain and SMMUv3 will no longer hold a stale domain reference.
> 
> Thanks for taking this on! I do like the overall structure and naming much
> more than my initial sketch :)

Thanks, no problem!
 
> >   	/*
> > -	 * If the group has been claimed already, do not re-attach the default
> > -	 * domain.
> > +	 * A NULL domain means to call the detach_dev() op. New drivers should
> > +	 * use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain
> 
> Nit: IOMMU_DOMAIN_DMA is the baseline of default domain support, passthrough
> is more of an optional extra.

Can you elaborate on this a bit more for the comment, I'm not sure I
understand all the historical stuff here.

Here we are looking at a case where group->domain becomes NULL - what
does this mean in the historical world? ie what should the iommu
driver do when detach_dev is called?

I had guessed it was remove all translation - ie IOMMU_DOMAIN_IDENTITY?

> > +	 * and detatch_dev().
> 
> Nit: detach_dev
> 
> Otherwise, modulo the other things already pointed out, this looks OK to me.

Done, thanks

Jason 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu