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 04:29:24PM +0100, Robin Murphy wrote:
> On 2022-05-04 15:54, Jason Gunthorpe wrote:
> > 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.
> 
> Well, the comment could effectively just be "New drivers should support
> default domains."

I made it this:

	/*
	 * New drivers should support default domains and so the detach_dev() op
	 * will never be called. Otheriwse the NULL domain indicates the
	 * translation for the group should be set so it will work with the
	 * platform DMA ops.
	 */

It also seems clear to me we should delete a bunch of detach_dev ops
from drivers?

I naively see that these drivers have the word IOMMU_DOMAIN_DMA in
them and also define detach_dev:
 amd iommu
 apple-dart
 qcom_iommu
 exynos-iommu
 intel iommu
 ipmmu-vmsa
 mtk_iommu
 rockchip-iommu
 sprd-iommu
 sun50i-iommu

These ones have IOMMU_DOMAIN_DMA but don't define detach_dev:
 arm-smmuv3
 arm-smmu
 virtio-iommu

And I suppose these are the 'old' drivers:
 fsl_pamu
 omap-iommu
 tegra-gart
 tegra-smmu

I can get a patch for this as well, I think it will be clarifying to
remove this cruft.

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