Re: [PATCH] iommu/dma: Finalize deferred attachments when mapping MSI pages

[Robin Murphy]

On 08/09/2026 5:01 pm, Federico Parola wrote:
> When a device's attachment to its default domain is deferred, the IOMMU
> is still translating the device with the tables inherited from the
> previous kernel. Any mapping installed in that domain does not take
> effect until the deferred attachment is finalized.
> iommu_deferred_attach() covers the DMA mapping paths, but nothing covers
> iommu_dma_sw_msi(): programming an MSI installs the MSI page in
> group->domain and hands the resulting IOVA to the irqchip, so an MSI
> programmed before the first DMA map is written using an IOVA that the
> hardware does not yet translate.
>
> This issue is currently not triggerable as the two IOMMU implementations
> supporting deferred attachments, Intel and AMD, do not rely on DMA
> translations for MSI transactions. However, the Arm implementation being
> introduced in [1] will be subject to it.
>
> Finalize the deferred attachment from iommu_dma_sw_msi() too, right
> before the MSI page is mapped. iommu_dma_prepare_msi() already holds
> group->mutex across the call, so factor the locked part of
> iommu_deferred_attach() out into __iommu_deferred_attach() and call that
> instead of taking the mutex recursively. The static branch on
> iommu_deferred_attach_enabled can be omitted as MSI page allocation is
> not on a fast path.

So why not just invoke iommu_deferred_attach() from 
iommu_dma_prepare_msi() itself, before taking the mutex at all, and save 
all the other churn? The two operations might happen to serialise on the 
same lock for their own different reasons, but they still have no need 
to be atomic relative to each other. If it's not a fast path in general 
then there should equally be little reason to worry about also checking 
in the cases which might not dispatch to iommu_dma_sw_msi().

Thanks,
Robin.

> The behaviour of a rejected attach changes slightly: an attach refused
> because the device is being reset (-EBUSY) now fails MSI setup rather
> than only the first DMA map.
>
> Link: https://lore.kernel.org/linux-iommu/cover.1788130528.git.nicolinc@xxxxxxxxxx/ [1]
> Signed-off-by: Federico Parola <fparola@xxxxxxxxx>
> ---
>   drivers/iommu/dma-iommu.c  |  7 +++++++
>   drivers/iommu/iommu-priv.h |  2 ++
>   drivers/iommu/iommu.c      | 25 ++++++++++++++++++-------
>   3 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 58c624513cd4..fa7de1b3d538 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -37,6 +37,7 @@
>   
>   #include "dma-iommu.h"
>   #include "iommu-pages.h"
> +#include "iommu-priv.h"
>   
>   struct iommu_dma_msi_page {
>   	struct list_head	list;
> @@ -2247,6 +2248,7 @@ int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
>   {
>   	struct device *dev = msi_desc_to_dev(desc);
>   	const struct iommu_dma_msi_page *msi_page;
> +	int ret;
>   
>   	if (!has_msi_cookie(domain)) {
>   		msi_desc_set_iommu_msi_iova(desc, 0, 0);
> @@ -2254,6 +2256,11 @@ int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
>   	}
>   
>   	iommu_group_mutex_assert(dev);
> +
> +	ret = __iommu_deferred_attach(dev, domain);
> +	if (ret)
> +		return ret;
> +
>   	msi_page = iommu_dma_get_msi_page(dev, msi_addr, domain);
>   	if (!msi_page)
>   		return -ENOMEM;
> diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
> index aaffad5854fc..ba72f9a03b0b 100644
> --- a/drivers/iommu/iommu-priv.h
> +++ b/drivers/iommu/iommu-priv.h
> @@ -21,6 +21,8 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
>   
>   void dev_iommu_free(struct device *dev);
>   
> +int __iommu_deferred_attach(struct device *dev, struct iommu_domain *domain);
> +
>   const struct iommu_ops *iommu_ops_from_fwnode(const struct fwnode_handle *fwnode);
>   
>   static inline const struct iommu_ops *iommu_fwspec_ops(struct iommu_fwspec *fwspec)
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index cd1bca7ede9a..d7d70689c784 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2214,19 +2214,15 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
>   }
>   EXPORT_SYMBOL_GPL(iommu_attach_device);
>   
> -int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
> +/* Caller must hold dev->iommu_group->mutex */
> +int __iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
>   {
>   	struct group_device *gdev;
>   
> -	/*
> -	 * This is called on the dma mapping fast path so avoid locking. This is
> -	 * racy, but we have an expectation that the driver will setup its DMAs
> -	 * inside probe while being single threaded to avoid racing.
> -	 */
>   	if (!dev->iommu || !dev->iommu->attach_deferred)
>   		return 0;
>   
> -	guard(mutex)(&dev->iommu_group->mutex);
> +	lockdep_assert_held(&dev->iommu_group->mutex);
>   
>   	gdev = __dev_to_gdev(dev);
>   	if (WARN_ON(!gdev))
> @@ -2244,6 +2240,21 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
>   	return __iommu_attach_device(domain, dev, NULL);
>   }
>   
> +int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain)
> +{
> +	/*
> +	 * This is called on the dma mapping fast path so avoid locking. This is
> +	 * racy, but we have an expectation that the driver will setup its DMAs
> +	 * inside probe while being single threaded to avoid racing.
> +	 */
> +	if (!dev->iommu || !dev->iommu->attach_deferred)
> +		return 0;
> +
> +	guard(mutex)(&dev->iommu_group->mutex);
> +
> +	return __iommu_deferred_attach(dev, domain);
> +}
> +
>   void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
>   {
>   	/* Caller must be a probed driver on dev */