Re: [PATCH v3 10/11] arm64: tegra: Hook up VPR to host1x

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing explicit `of_reserved_mem_device_init()` API call for VPR devices.
- [High] Omitted `memory-region` assignment on the `host1x` parent node.
- [Low] Dead self-assignment of a zero-initialized pointer member in `tegra_vpr_attach()`.
--

commit d66858609809fa2e5e478b229dab43684ed2e277
Author: Thierry Reding <treding@nvidia.com>

arm64: tegra: Hook up VPR to host1x

This commit adds the memory-region property to the vic and nvdec device
tree nodes in tegra234.dtsi. It intends to hook up the video protection
region to host1x and its clients.

> The host1x needs access to the VPR region, so make sure to reference it
> via the memory-region property.

[Severity: High]
Are we missing the memory-region assignment on the host1x parent node?

The commit message explicitly states that host1x needs access to the vpr
region and should reference it via the memory-region property. However,
this commit omits adding the memory-region property to the host1x@13e00000
parent node itself, only applying it to the child nodes (vic and nvdec).

Since the host1x dma engines fetch command streams from memory, if those
command buffers reside in the vpr region, host1x must be tracked as a vpr
device and frozen during vpr resizes. Omitting the property prevents host1x
from being added to the vpr devices list, leaving the system vulnerable to
hardware crashes if a resize is triggered.

> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 52ff11873580d..38637e8e6fc92 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -4479,6 +4479,9 @@ vic@15340000 {
>  				interconnect-names = "dma-mem", "write";
>  				iommus = <&smmu_niso1 TEGRA234_SID_VIC>;
>  				dma-coherent;
> +
> +				memory-region = <&vpr>;
> +				memory-region-names = "protected";

[Severity: High]
Does this node assignment lack the explicit of_reserved_mem_device_init()
call required in the driver?

The generic of_dma_configure_id() logic only automatically invokes
of_reserved_mem_device_init_by_idx() for reserved regions compatible with
restricted-dma-pool. Since the vpr region uses
nvidia,tegra-video-protection-region, this automatic initialization is
skipped.

The drivers for vic and nvdec do not explicitly call
of_reserved_mem_device_init() to initialize the reserved memory. As a result,
the tegra_vpr_device_init() callback is never triggered, and the devices are
never added to the vpr->devices list.

Since they are missing from the list, they will not be suspended by
tegra_vpr_resize() during vpr resizing. Because untrusted userspace
applications can allocate memory from the vpr dma-buf heap to trigger a
resize, an active hardware workload on the gpu, vic, or nvdec during the
resize will lead to memory corruption, hardware faults, or a system crash.

>  			};
>  
>  			nvdec@15480000 {
> @@ -4497,6 +4500,9 @@ nvdec@15480000 {
>  				iommus = <&smmu_niso1 TEGRA234_SID_NVDEC>;
>  				dma-coherent;
>  
> +				memory-region = <&vpr>;
> +				memory-region-names = "protected";
> +

[Severity: High]
Could this assignment also lead to a hardware crash for similar reasons?

The generic of_dma_configure_id() logic skips automatic initialization for
nvidia,tegra-video-protection-region. The nvdec driver does not explicitly
call of_reserved_mem_device_init(), so the device is never added to the
vpr->devices list and will not be suspended during vpr resizes, leading to
potential hardware faults or crashes when the region is resized.

>  				nvidia,memory-controller = <&mc>;

Additionally, looking at the driver changes introduced in this series, there
is a regression in tegra_vpr_attach():

drivers/dma-buf/heaps/tegra-vpr.c:tegra_vpr_attach() {
[ ... ]
	if (err < 0)
		goto free;

	attach->dev = attach->dev;
	INIT_LIST_HEAD(&attach->list);
[ ... ]
}

[Severity: Low]
Is this intended to capture attachment->dev instead?

tegra_vpr_attach() allocates the attach structure using kzalloc_obj(),
which zero-initializes the memory. It then executes the dead assignment
attach->dev = attach->dev, which effectively assigns null to the struct
member.

While this doesn't cause an immediate crash because the driver uses
attachment->dev directly in tegra_vpr_map_dma_buf() and leaves the member
unread, it appears to be a logic flaw introduced by this patch series.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-tegra-vpr-v3-0-d80f7b871bb4@nvidia.com?part=10