Re: [PATCH v2 6/7] iommu/dma: Centralise iommu_setup_dma_ops()

[Jean-Philippe Brucker]

On Wed, Dec 13, 2023 at 05:17:59PM +0000, Robin Murphy wrote:
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 27a167f4cd3e..d808c8dcf5cb 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1724,25 +1724,20 @@ static const struct dma_map_ops iommu_dma_ops = {
>  	.opt_mapping_size	= iommu_dma_opt_mapping_size,
>  };
>  
> -/*
> - * The IOMMU core code allocates the default DMA domain, which the underlying
> - * IOMMU driver needs to support via the dma-iommu layer.
> - */
> -void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit)
> +void iommu_setup_dma_ops(struct device *dev)
>  {
>  	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
>  
> -	if (!domain)
> -		goto out_err;
> +	if (dev_is_pci(dev))
> +		dev->iommu->pci_32bit_workaround = !iommu_dma_forcedac;
>  
> -	/*
> -	 * The IOMMU core code allocates the default DMA domain, which the
> -	 * underlying IOMMU driver needs to support via the dma-iommu layer.
> -	 */
>  	if (iommu_is_dma_domain(domain)) {

...

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 824989874dee..43f630d0530e 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -560,10 +560,10 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
>  		if (list_empty(&group->entry))
>  			list_add_tail(&group->entry, group_list);
>  	}
> -	mutex_unlock(&group->mutex);
>  
> -	if (dev_is_pci(dev))
> -		iommu_dma_set_pci_32bit_workaround(dev);
> +	iommu_setup_dma_ops(dev);

With Intel VT-d (QEMU emulation) I get a crash in iommu_setup_dma_ops()
because at this point group->domain and group->default_domain are still
NULL, group_list is non-NULL.

No problem with x86 virtio-iommu, it goes through
iommu_setup_default_domain() earlier in the function. Arm virtio-iommu is
fine too.

Thanks,
Jean