Re: [PATCH 4/7] arm64: dts: imx8ulp-evk: rename vdevbuffer to vdev0buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] An off-by-one error in address bounds checking incorrectly rejects memory translations that exactly fit within a region.
- [Medium] DT ABI breakage and incomplete fix by renaming device tree nodes instead of fixing the driver.
--

--- Patch [4]: [PATCH 4/7] arm64: dts: imx8ulp-evk: rename vdevbuffer to vdev0buffer ---
commit 6ba801ee20ee0b1a91efe7b3138304a45163e3aa
Author: Peng Fan <peng.fan@nxp.com>

arm64: dts: imx8ulp-evk: rename vdevbuffer to vdev0buffer

The imx_rproc driver expects the virtio buffer node to be named
"vdev0buffer" to skip mapping the vdev buffer region. This patch
renames the node to match what the driver expects.

> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> index 5dea66c1e7aa0..48a3e40053299 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> @@ -67,7 +67,7 @@ vdev1vring1: vdev1vring1@aff18000 {
>  			no-map;
>  		};
>  
> -		vdevbuffer: vdevbuffer@a8400000 {
> +		vdev0buffer: vdev0buffer@a8400000 {

[Severity: Medium]
This is a pre-existing issue, but does renaming the node here break Device
Tree ABI compatibility?

Users running newer kernels with older DTBs will still experience the mapping
bug because the driver remains hardcoded to expect "vdev0buffer".
Additionally, there are other in-tree boards (such as
imx93-11x11-evk-common.dtsi) that still use "vdevbuffer".

Should the underlying driver bug in imx_rproc_prepare() be fixed instead
to support the legacy node name?

drivers/remoteproc/imx_rproc.c:imx_rproc_prepare() {
    ...
    if (strstarts(res.name, "vdev0buffer"))
    ...
}

>  			compatible = "shared-dma-pool";
>  			reg = <0 0xa8400000 0 0x100000>;
>  			no-map;

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while looking at the
imx_rproc driver, is there an off-by-one error in the address bounds
checking?

In imx_rproc_da_to_sys() and imx_rproc_da_to_va(), a strictly-less-than
operator is used for the upper bound limit:

drivers/remoteproc/imx_rproc.c:imx_rproc_da_to_sys() {
    ...
    if (da >= att->da && da + len < att->da + att->size) {
    ...
}

If a firmware segment request perfectly fills the remainder of the region
(where da + len == att->da + att->size), it appears this valid request
evaluates to false and the translation gets wrongly rejected.

Could this cause valid firmware segments to fail to load, and should this
check use <= instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-rproc-dts-v1-0-368bca0b309f@nxp.com?part=4