[PATCH v4 10/11] remoteproc: k3-hsm: Introduce a remoteproc driver for K3 HSM core

[Simon Glass]

Hi Beleswar,

On 2026-04-25T03:37:39, Padhi, Beleswar <b-padhi at ti.com> wrote:
> remoteproc: k3-hsm: Introduce a remoteproc driver for K3 HSM core
>
> Some K3 SoCs like J721S2, J784S4, J722S, AM62X and AM62PX have a HSM
> (High Security Module) M4F core in the Wakeup Voltage Domain which could
> be used to run secure services like Authentication. Boot flow for HSM M4
> core is different than the general purpose M4F cores, and is as below:
>
> 1. Request control of HSM M4F remote processor.
> 2. Assert Reset on the HSM M4F remote processor.
> 3. For HS devices, Request Secure Entity to Authenticate and Load HSM
>    firmware into core's internal SRAM memory region. For GP devices,
>    load the unsigned firmware manually.
> 4. Deassert Reset on the HSM M4F remote processor.
> 5. Release control of HSM M4F remote processor.
>
> Add a new remoteproc driver to support this boot flow for HSM M4F core.
>
> Signed-off-by: Beleswar Padhi <b-padhi at ti.com>
>
> MAINTAINERS                          |   1 +
>  drivers/remoteproc/Kconfig           |  10 ++
>  drivers/remoteproc/Makefile          |   1 +
>  drivers/remoteproc/ti_k3_hsm_rproc.c | 252 +++++++++++++++++++++++++++++++++++
>  4 files changed, 264 insertions(+)

> diff --git a/drivers/remoteproc/ti_k3_hsm_rproc.c b/drivers/remoteproc/ti_k3_hsm_rproc.c
> @@ -0,0 +1,252 @@
> +static int k3_hsm_load(struct udevice *dev, ulong addr, ulong size)
> +{
> +     struct k3_hsm_privdata *hsm = dev_get_priv(dev);
> +     size_t image_size = size;
> +     int ret;
> +
> +     ret = ti_sci_proc_request(&hsm->tsp);
> +     if (ret)
> +             return ret;
> +
> +     ret = ti_sci_proc_set_control(&hsm->tsp,
> +                                   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4, 0);
> +     if (ret)
> +             goto proc_release;
> +
> +     ti_secure_image_post_process((void *)&addr, &image_size);

Better to pass a proper 'void *image_addr = (void *)addr' here.
Casting &addr (a ulong *) to void ** happens to work on a 32-bit R5
SPL but it relies on sizeof(ulong) == sizeof(void *) and on the
compiler being willing to alias-write a void * back through that ulong
storage.

> diff --git a/drivers/remoteproc/ti_k3_hsm_rproc.c b/drivers/remoteproc/ti_k3_hsm_rproc.c
> @@ -0,0 +1,252 @@
> +     if (image_size == size) {
> +             debug("Loading HSM GP binary into SRAM0_0\n");
> +             memcpy((void *)hsm->mem[0].cpu_addr, (void *)(u32)addr, size);
> +             flush_dcache_range((unsigned long)hsm->mem[0].cpu_addr,
> +                                (unsigned long)(hsm->mem[0].cpu_addr + size));
> +     }

A few things on this block:

1. Detecting "this is a GP device, do the manual load" by comparing
image_size against the original size is brittle - e.g. on HS-FS
without a certificate, ti_secure_image_post_process() also returns
with the size unchanged (after printing a warning), so we would
silently fall into the GP path. Can you add a check for test
get_device_type() == K3_DEVICE_TYPE_GP explicitly - that would match
the commit message and the debug() line.

2. The (u32)addr cast truncates addr on any 64-bit build. R5 SPL is
32-bit today, but please drop the (u32) and just do (void *)addr so
the code is correct independent of target width.

3. There is no check that size fits in hsm->mem[0].size before the
memcpy(). If a malformed FIT delivers an oversized image, this
scribbles past SRAM0_0. A simple bounds test with an error return
would help.

> diff --git a/drivers/remoteproc/ti_k3_hsm_rproc.c b/drivers/remoteproc/ti_k3_hsm_rproc.c
> @@ -0,0 +1,252 @@
> +static int k3_hsm_stop(struct udevice *dev)
> +{
> +     struct k3_hsm_privdata *hsm = dev_get_priv(dev);
> +     int ret;
> +
> +     ti_sci_proc_request(&hsm->tsp);
> +
> +     ret = ti_sci_proc_set_control(&hsm->tsp,
> +                                   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4, 0);
> +
> +     ti_sci_proc_release(&hsm->tsp);
> +
> +     return ret;
> +}

The return value of ti_sci_proc_request() is silently dropped. If the
request fails, the set_control() call below will also fail and the
user gets a misleading error. Please check it and bail early like the
rest of the driver does.

> diff --git a/drivers/remoteproc/ti_k3_hsm_rproc.c b/drivers/remoteproc/ti_k3_hsm_rproc.c
> @@ -0,0 +1,252 @@
> +     tsp->dev_id = dev_read_u32_default(dev, 'ti,sci-dev-id',
> +                                        TI_SCI_RESOURCE_NULL);
> +     if (tsp->dev_id == TI_SCI_RESOURCE_NULL) {
> +             dev_err(dev, "Device ID not populated %d\n", ret);
> +             return -ENODEV;
> +     }

ret is 0 here (last set by the successful dev_read_u32_array() call),
so the %d is meaningless. I realise this was copied from
ti_k3_m4_rproc.c, but please drop the %d and the argument while
introducing a new copy.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon