linux-kernel - Re: [PATCH 11/16] media: rockchip: rga: add iommu restore function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <97879b9b078055fb130edfd126d253320ce616a1.camel@ndufresne.ca>
Date: Tue, 07 Oct 2025 14:30:34 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Sven Püschel <s.pueschel@...gutronix.de>, Jacob Chen
	 <jacob-chen@...wrt.com>, Ezequiel Garcia <ezequiel@...guardiasur.com.ar>, 
 Mauro Carvalho Chehab
	 <mchehab@...nel.org>, Heiko Stuebner <heiko@...ech.de>, Rob Herring
	 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
	 <conor+dt@...nel.org>
Cc: linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH 11/16] media: rockchip: rga: add iommu restore function

Hi,

Le mardi 07 octobre 2025 à 10:32 +0200, Sven Püschel a écrit :
> Add an iommu restore function in preparation for the rga3 addition.
> This is necessary for a soft reset, as the rga3 will also reset
> it's iommu paging table to 0 and disable paging.
> 
> The empty domain attach/detach to restore the iommu is copied
> from the rkvdec driver.

We did receive negative feedback after the fact on this one. We will likely
upset further the iommu subsystem maintainers with that. Have you considered
adding a restore function in the rkiommu driver, similar to TI mmu and Benjamin
VSI MMU proposal ?

I have no precise objection, I know it works, but adding a restore function
seems also pretty straight forward.

> 
> Signed-off-by: Sven Püschel <s.pueschel@...gutronix.de>
> ---
>  drivers/media/platform/rockchip/rga/rga.c | 24 ++++++++++++++++++++++++
>  drivers/media/platform/rockchip/rga/rga.h |  7 +++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga.c
> b/drivers/media/platform/rockchip/rga/rga.c
> index
> cd4da01645611e5fb51ed94e09b5f1463dad72c5..0a725841b0cfa41bbc5b861b8f5ceac2452f
> c2b5 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -9,6 +9,7 @@
>  #include <linux/delay.h>
>  #include <linux/fs.h>
>  #include <linux/interrupt.h>
> +#include <linux/iommu.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/pm_runtime.h>
> @@ -560,6 +561,19 @@ static const struct video_device rga_videodev = {
>  	.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING,
>  };
>  
> +void rga_iommu_restore(struct rockchip_rga *rga)
> +{
> +	if (rga->empty_domain) {
> +		/*
> +		 * To rewrite mapping into the attached IOMMU core, attach a
> new empty domain that
> +		 * will program an empty table, then detach it to restore the
> default domain and
> +		 * all cached mappings.
> +		 */
> +		iommu_attach_device(rga->empty_domain, rga->dev);
> +		iommu_detach_device(rga->empty_domain, rga->dev);
> +	}
> +}
> +
>  static int rga_parse_dt(struct rockchip_rga *rga)
>  {
>  	struct reset_control *core_rst, *axi_rst, *ahb_rst;
> @@ -657,6 +671,13 @@ static int rga_probe(struct platform_device *pdev)
>  		goto err_put_clk;
>  	}
>  
> +	if (iommu_get_domain_for_dev(rga->dev)) {
> +		rga->empty_domain = iommu_paging_domain_alloc(rga->dev);
> +
> +		if (!rga->empty_domain)

Its an error pointer, see:

https://gitlab.freedesktop.org/linux-media/media-committers/-/commit/6347dc7fb967521a77f9ff0774d25ef0cca4c6cd

> +			dev_warn(rga->dev, "cannot alloc new empty
> domain\n");
> +	}
> +
>  	ret = v4l2_device_register(&pdev->dev, &rga->v4l2_dev);
>  	if (ret)
>  		goto err_put_clk;
> @@ -741,6 +762,9 @@ static void rga_remove(struct platform_device *pdev)
>  	v4l2_device_unregister(&rga->v4l2_dev);
>  
>  	pm_runtime_disable(rga->dev);
> +
> +	if (rga->empty_domain)
> +		iommu_domain_free(rga->empty_domain);
>  }
>  
>  static int __maybe_unused rga_runtime_suspend(struct device *dev)
> diff --git a/drivers/media/platform/rockchip/rga/rga.h
> b/drivers/media/platform/rockchip/rga/rga.h
> index
> fc4805ba4e8ef7fb311f780a198ba6ba4d3aff17..e19c4c82aca5ae2056f52d525138093fbbb8
> 1af8 100644
> --- a/drivers/media/platform/rockchip/rga/rga.h
> +++ b/drivers/media/platform/rockchip/rga/rga.h
> @@ -75,6 +75,7 @@ struct rockchip_rga {
>  	void __iomem *regs;
>  	struct clk_bulk_data clks[3];
>  	struct rockchip_rga_version version;
> +	struct iommu_domain *empty_domain;
>  
>  	/* vfd lock */
>  	struct mutex mutex;
> @@ -114,6 +115,12 @@ static inline struct rga_vb_buffer *vb_to_rga(struct
> vb2_v4l2_buffer *vb)
>  
>  struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type
> type);
>  
> +/*
> + * This should be called in an interrupt handler to make sure no memory
> + * is mapped through the IOMMU while the empty domain is attached.
> + */
> +void rga_iommu_restore(struct rockchip_rga *rga);
> +
>  /* RGA Buffers Manage */
>  extern const struct vb2_ops rga_qops;
>  

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)