Re: [PATCH 2/3] ARM: dma-mapping: reset DMA ops before detaching from the IOMMU

Robin Murphy <[email protected]>
Newsgroups org.freedesktop.lists.dri-devel,dev.linux.lists.iommu,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-rockchip,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On 30/07/2026 2:29 pm, Dmitry Baryshkov wrote:
> arm_iommu_attach_device() installs the IOMMU-aware dma_map_ops only
> after iommu_attach_device() has run, so a driver that builds its page
> tables during attach (msm_iommu via io-pgtable) maps them while the
> device still has the direct DMA ops: the resulting dma_handle equals the
> physical address.
> 
> arm_iommu_detach_device() does the reverse in the wrong order. It calls
> iommu_detach_device() first and clears the DMA ops afterwards. When
> detaching frees those page tables (msm_iommu_identity_attach() ->
> free_io_pgtable_ops()), the freeing dma_unmap now goes through the still
> installed IOMMU ops,

Hang on, that sounds majorly wrong...

> which treat the stored physical address as an IOVA.
> iommu_unmap() warns "region not mapped" and __free_iova() indexes an
> unallocated bitmap extension, dereferencing NULL:
> 
>    __bitmap_clear from arm_iommu_unmap_phys
>    arm_iommu_unmap_phys from dma_unmap_phys

*this* dma_unmap is for the IOMMU device itself - the client device's 
ops should be irrelevant, and the IOMMU device should never be trying to 
translate itself through its own ops either.

 From a quick look, the real issue appears to be that the assignment of 
priv->dev in msm_iommu_attach_dev() and subsequent use in 
msm_iommu_domain_config() is completely bogus.

Thanks,
Robin.

>    dma_unmap_phys from __arm_v7s_free_table
>    __arm_v7s_free_table from arm_v7s_free_pgtable
>    arm_v7s_free_pgtable from msm_iommu_identity_attach
>    msm_iommu_identity_attach from __iommu_attach_device
>    __iommu_attach_device from iommu_detach_device
>    iommu_detach_device from arm_iommu_detach_device
>    arm_iommu_detach_device from arch_teardown_dma_ops
>    arch_teardown_dma_ops from device_unbind_cleanup
> 
> This is reached on apq8064 (ifc6410) when the adreno GPU probe fails and
> its DMA ops are torn down.
> 
> Reset the DMA ops before iommu_detach_device() so page-table teardown
> runs with the same direct ops that mapped them at attach time.
> 
> Fixes: 78fc30b4bb35 ("iommu/msm: Implement an IDENTITY domain")
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> Assisted-by: Claude:claude-opus-5
> ---
>   arch/arm/mm/dma-mapping.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index f9bc53b60f99..a467e0d01a5b 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1642,10 +1642,13 @@ void arm_iommu_detach_device(struct device *dev)
>   		return;
>   	}
>   
> +	/* Reset DMA ops before detach so page-table teardown uses the same
> +	 * direct ops that mapped them at attach time.
> +	 */
> +	set_dma_ops(dev, NULL);
>   	iommu_detach_device(mapping->domain, dev);
>   	kref_put(&mapping->kref, release_iommu_mapping);
>   	to_dma_iommu_mapping(dev) = NULL;
> -	set_dma_ops(dev, NULL);
>   
>   	pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
>   }
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.