diff mbox series

[v2,6/7] arm64: dts: allwinner: sun50i-h6: Add missing IOMMU for video engine

Message ID 20260711135959.3616919-7-wens@kernel.org (mailing list archive)
State New
Headers
Series arm64: allwinner: h616: Support Video Engine |

Commit Message

Chen-Yu Tsai July 11, 2026, 1:59 p.m. UTC 
The video engine on the H6 actually has two IOMMU ports. The one already
listed is simply named "VE" in the user manual. The missing one is named
"VE_R" and is probably either read-only or used for reference frames.
Either way, it should be listed.

Add a reference to the "VE_R" IOMMU port to the video engine node.

Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage")
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

sashiko-bot@kernel.org July 11, 2026, 2:08 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The sun50i-iommu driver fails to properly detach domains from the hardware due to an unbalanced refcount, leading to a use-after-free of the domain structure.
- [High] The sun50i-iommu driver assigns a separate IOMMU group/domain per device despite the hardware only supporting a single global translation table, causing devices to overwrite each other's page tables.
--

--- Patch [6]: [PATCH v2 6/7] arm64: dts: allwinner: sun50i-h6: Add missing IOMMU for video engine ---
commit 8a79dfe07f469620b0b60592b02052d1c198fe97
Author: Chen-Yu Tsai <wens@kernel.org>

arm64: dts: allwinner: sun50i-h6: Add missing IOMMU for video engine
    
This commit adds a missing IOMMU port ("VE_R") to the video engine node
in the sun50i-h6 device tree.

> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index 88c6e3e105c07..48c771223af4d 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -206,7 +206,7 @@ video-codec@1c0e000 {
>  			resets = <&ccu RST_BUS_VE>;
>  			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>  			allwinner,sram = <&ve_sram 1>, <&de2_sram 1>;
> -			iommus = <&iommu 3>;
> +			iommus = <&iommu 3>, <&iommu 1>;

[Severity: High]
This is a pre-existing issue, but does adding a second IOMMU trigger a
use-after-free during device unbind?

Looking at the sun50i-iommu driver, sun50i_iommu_domain_alloc_paging() sets
the domain refcount to 1:

drivers/iommu/sun50i-iommu.c:sun50i_iommu_domain_alloc_paging() {
    ...
	refcount_set(&sun50i_domain->refcnt, 1);
    ...
}

Later, when a device is detached, sun50i_iommu_identity_attach()
decrements it:

drivers/iommu/sun50i-iommu.c:sun50i_iommu_identity_attach() {
    ...
	sun50i_domain = to_sun50i_domain(iommu->domain);
	if (refcount_dec_and_test(&sun50i_domain->refcnt))
		sun50i_iommu_detach_domain(iommu, sun50i_domain);
    ...
}

Because the reference count drops from 2 to 1 here, it fails to reach 0 and
bypasses the domain detachment. This leaves iommu->domain as a dangling
pointer which is later freed by sun50i_iommu_domain_free(). Could this
cause a kernel panic on a subsequent IOMMU page fault interrupt since it
dereferences the freed pointer?

[Severity: High]
This isn't a bug introduced by this patch, but does adding another IOMMU
reference here break DMA due to a single global translation table?

The sun50i-iommu driver assigns a separate IOMMU group per device:

drivers/iommu/sun50i-iommu.c:
static const struct iommu_ops sun50i_iommu_ops = {
    ...
	.device_group	= generic_single_device_group,
    ...
};

Since the hardware only supports a single global translation table
(IOMMU_TTB_REG), placing every device into its own independent group and
domain means multiple devices will repeatedly overwrite the shared IOMMU's
domain on probe. Will enabling this second IOMMU port overwrite previously
configured translation tables and corrupt memory for other devices?

>  		};
>  
>  		gpu: gpu@1800000 {
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 88c6e3e105c0..48c771223af4 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -206,7 +206,7 @@  video-codec@1c0e000 {
 			resets = <&ccu RST_BUS_VE>;
 			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
 			allwinner,sram = <&ve_sram 1>, <&de2_sram 1>;
-			iommus = <&iommu 3>;
+			iommus = <&iommu 3>, <&iommu 1>;
 		};
 
 		gpu: gpu@1800000 {