RE: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Saturday, January 28, 2023 7:54 AM
> 
> On Fri, Jan 27, 2023 at 09:58:46PM +0000, Robin Murphy wrote:
> 
> > Please just add IOMMU_CAP_IOMMUFD to represent whatever the
> nebulous
> > requirements of IOMMUFD actually are (frankly it's no less informative
> than
> > calling domains "broken"), handle that in the drivers you care about
> > and
> 
> I don't want to tie this to iommufd, that isn't the point.
> 
> We clearly have drivers that don't implement the iommu kernel API
> properly, because their only use is between the iommu driver and some
> other same-SOC driver.
> 
> As a user of the iommu API iommufd and VFIO want to avoid these
> drivers.
> 
> We have that acknowledgment already via the IOMMU_DOMAIN_DMA stuff
> protecting the dma_iommu.c from those same drivers.
> 
> So, how about this below instead. Imagine it is followed by something along
> the lines of my other sketch:
> 
> https://lore.kernel.org/linux-iommu/Y4%2FLsZKmR3iWFphU@xxxxxxxxxx/
> 
> And we completely delete IOMMU_DOMAIN_DMA/_FQ and get dma-
> iommu.c
> mostly out of driver code.
> 
> iommufd/vfio would refuse to work with drivers that don't indicate
> they support dma_iommu.c, that is good enough.

this is a good idea. Just one doubt. Robin mentioned that sprd-iommu
might have a broken unmanaged domains:

" I'd also question sprd-iommu, which hardly has a generally-useful
domain size, and has only just recently gained the ability to unmap
anything successfully."

Want to understand why that restriction is not a problem for DMA API.

what Jason proposed here assumes that existing driver support for
DMA API can be safely applied to vfio/iommufd. But above looks
warning certain exception?

> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 59df7e42fd533c..bb34d3f641f17b 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4165,8 +4165,6 @@ static struct iommu_domain
> *intel_iommu_domain_alloc(unsigned type)
>  	switch (type) {
>  	case IOMMU_DOMAIN_BLOCKED:
>  		return &blocking_domain;
> -	case IOMMU_DOMAIN_DMA:
> -	case IOMMU_DOMAIN_DMA_FQ:
>  	case IOMMU_DOMAIN_UNMANAGED:
>  		dmar_domain = alloc_domain(type);
>  		if (!dmar_domain) {
> @@ -4761,6 +4759,7 @@ static void intel_iommu_remove_dev_pasid(struct
> device *dev, ioasid_t pasid)
>  }
> 
>  const struct iommu_ops intel_iommu_ops = {
> +	.use_dma_iommu		= true,

missed:
+	.allow_dma_fq			= true,