Re: [PATCH v10 13/21] spl: imx: use trampoline buffer to load images to secure region

["Lothar Waßmann" <LW@KARO-electronics.de>]

Hi,

On Thu, 24 Apr 2025 14:33:09 +0800 Alice Guo (OSS) wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> When SPL loading image to secure region, for example, ATF and tee to
> DDR secure region. Because the USDHC controller is non-secure master,
> it can't access this region and will cause loading issue.
> 
> So use a trampoline buffer in non-secure region, then use CPU to copy the
> image from trampoline buffer to destination secure region.
> 
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm/mach-imx/imx9/scmi/soc.c | 18 +++++++++++++++++
>  common/spl/Kconfig                |  6 ++++++
>  common/spl/spl_imx_container.c    | 41 +++++++++++++++++++++++++++++++++------
>  3 files changed, 59 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/imx9/scmi/soc.c b/arch/arm/mach-imx/imx9/scmi/soc.c
> index 3a5bdddd785..d2b0455bff9 100644
> --- a/arch/arm/mach-imx/imx9/scmi/soc.c
> +++ b/arch/arm/mach-imx/imx9/scmi/soc.c
> @@ -729,3 +729,21 @@ enum boot_device get_boot_device(void)
>  	return boot_dev;
>  }
>  #endif
> +
> +bool arch_check_dst_in_secure(void *start, ulong size)
> +{
> +	ulong ns_end = CFG_SYS_SDRAM_BASE + PHYS_SDRAM_SIZE;
> +#ifdef PHYS_SDRAM_2_SIZE
> +	ns_end += PHYS_SDRAM_2_SIZE;
> +#endif
> +
> +	if ((ulong)start < CFG_SYS_SDRAM_BASE || (ulong)start + size > ns_end)
> +		return true;
> +
> +	return false;
> +}
> +
> +void *arch_get_container_trampoline(void)
> +{
> +	return (void *)((ulong)CFG_SYS_SDRAM_BASE + PHYS_SDRAM_SIZE - SZ_16M);
> +}
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 3282e08a754..a2b214f3d74 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -361,6 +361,12 @@ config SPL_LOAD_IMX_CONTAINER
>  	  Support booting U-Boot from an i.MX8 container image. If you are not
>  	  using i.MX8, say 'n'.
>  
> +config SPL_IMX_CONTAINER_USE_TRAMPOLINE
> +	bool
> +	depends on SPL
> +	help
> +	  Enable SPL load reader to load data to a trampoline buffer.
> +
>  config IMX_CONTAINER_CFG
>  	string "i.MX8 Container config file"
>  	depends on SPL && SPL_LOAD_IMX_CONTAINER
> diff --git a/common/spl/spl_imx_container.c b/common/spl/spl_imx_container.c
> index 2c31777fcd3..6e6afec1ff3 100644
> --- a/common/spl/spl_imx_container.c
> +++ b/common/spl/spl_imx_container.c
[...]
> @@ -42,12 +53,30 @@ static struct boot_img_t *read_auth_image(struct spl_image_info *spl_image,
[...]
> +		if (info->read(info, offset, size,
> +			       map_sysmem(images[image_index].dst - overhead,
> +					  images[image_index].size)) <
> +		    images[image_index].size) {
> +				printf("%s: failed to load image to a non-secure region\n", __func__);
> +			return NULL;
> +		}
Strange indentation and rather unreadable code...
Since 'map_sysmem(images[image_index].dst - overhead, images[image_index].size)'
has been assigned to 'buf' before, this could be written as:
	if (info->read(info, offset, size, buf) < images[image_index].size) {
		printf("%s: failed to load image to a non-secure region\n", __func__);
		return NULL;
	}

Lothar Waßmann