Re: [PATCH v9 7/9] arm64: dts: ti: k3-am62a: Split r5f memory region

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] remoteproc: ti_k3: k3_rproc_da_to_va() rejects segments spanning split memory regions
- [High] arm64: dts: ti: missing bootph-pre-ram breaks SPL DM firmware loading
- [Medium] arm64: dts: ti: hardcoding volatile firmware layout into Device Tree
--

commit 33643294863f45f1ac37617eafcfc525d230effc
Author: Markus Schneider-Pargmann (TI) <msp@baylibre.com>

arm64: dts: ti: k3-am62a: Split r5f memory region

Split the firmware memory region in more specific parts so it is better
described where to find which information. Specifically the LPM metadata
region is important as bootloader software like U-Boot has to know where
that data is to be able to read that data.

> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi
> index 06d4e815b1670..94c1d60d8d7a1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi
> @@ -65,8 +65,14 @@ &wkup_r5fss0 {
>  &wkup_r5fss0_core0 {
>  	mboxes = <&mailbox0_cluster0>, <&mbox_r5_0>;
>  	memory-region = <&wkup_r5fss0_core0_dma_memory_region>,
> -			<&wkup_r5fss0_core0_memory_region>;
> -	memory-region-names = "dma", "firmware";
> +			<&wkup_r5fss0_core0_ipc_region>,
> +			<&wkup_r5fss0_core0_lpm_fs_stub_region>,
> +			<&wkup_r5fss0_core0_lpm_metadata_region>,
> +			<&wkup_r5fss0_core0_lpm_rest_region>,
> +			<&wkup_r5fss0_core0_dm_region>;

[Severity: High]
Does this multi-region split cause a regression when loading a monolithic
firmware ELF whose segments span across the new chunk boundaries?

Looking at k3_rproc_da_to_va() in drivers/remoteproc/ti_k3_common.c, it
evaluates segments against each reserved memory region individually and
lacks logic to aggregate sequential chunks:

drivers/remoteproc/ti_k3_common.c:k3_rproc_da_to_va() {
    ...
    if (da >= dev_addr && ((da + len) <= (dev_addr + size))) {
        offset = da - dev_addr;
        va = kproc->rmem[i].cpu_addr + offset;
        return (__force void *)va;
    }
    ...
}

Will this bounds check fail and abort the boot sequence for any system using
standard firmware linked under the assumption of a single contiguous memory
block?

> +	memory-region-names = "dma", "ipc", "lpm-stub",
> +			      "lpm-metadata", "lpm-context",
> +			      "dm-firmware";

[ ... ]

> diff --git a/arch/arm64/boot/dts/ti/k3-am62d2-evm.dts b/arch/arm64/boot/dts/ti/k3-am62d2-evm.dts
> index a8c794d073431..7cba3ff99e6a3 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62d2-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am62d2-evm.dts
> @@ -65,13 +65,37 @@ wkup_r5fss0_core0_dma_memory_region: memory@9c800000 {
[ ... ]
> +		wkup_r5fss0_core0_lpm_fs_stub_region: memory@9ca00000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x00 0x9ca00000 0x00 0x8000>;
> +			no-map;
> +		};

[Severity: Medium]
Does this create a regression by hardcoding a software-defined volatile
firmware layout directly into the Device Tree?

If a subsequent firmware build increases the size of lpm_fs_stub beyond the
32KB hardcoded here, will this crash or require a lockstep DT update to
function properly?

> +
> +		wkup_r5fss0_core0_lpm_metadata_region: memory@9ca08000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x00 0x9ca08000 0x00 0x1000>;
>  			no-map;
>  			bootph-pre-ram;
>  		};
>  
> +		wkup_r5fss0_core0_lpm_rest_region: memory@9ca09000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x00 0x9ca09000 0x00 0x97000>;
> +			no-map;
> +		};
> +
> +		wkup_r5fss0_core0_dm_region: memory@9caa0000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x00 0x9caa0000 0x00 0x1c60000>;
> +			no-map;
> +		};

[Severity: High]
Does the omission of the bootph-pre-ram tag on this new DM region create a
regression for SPL DM firmware loading?

Since U-Boot fdtgrep processes the SPL DTB by keeping only nodes tagged
with bootph-pre-ram, these split regions will be stripped.

Will this result in the no-map protections being lost, causing U-Boot's
remoteproc driver to error out parsing dangling phandles when attempting
to load the Device Manager (DM) firmware?

> +
>  		secure_ddr: optee@9e800000 {
>  			reg = <0x00 0x9e800000 0x00 0x01800000>; /* for OP-TEE */
>  			no-map;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-topic-am62a-ioddr-dt-v6-19-v9-0-40972ef1f663@baylibre.com?part=7