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

From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Qian Cai <quic_qiancai@quicinc.com>,
	iommu@lists.linux-foundation.org, Will Deacon <will@kernel.org>
Subject: Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain
Date: Wed, 4 May 2022 11:54:54 -0300	[thread overview]
Message-ID: <20220504145454.GI49344@nvidia.com> (raw)
In-Reply-To: <0a8cdb2a-91a3-9953-b7a1-8517ffcadb75@arm.com>

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