[PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Beleswar Padhi]

Some TI K3 SoCs like J721S2, and J784S4 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. Request Secure Entity to Authenticate and Load HSM firmware into
   core's internal SRAM memory region. For GP device, load the firmware
   manually into core's SRAM region.
4. Deassert Reset on the HSM M4F remote processor.
5. Release control of HSM M4F remote processor.

This series adds support to boot HSM M4 core from R5 SPL stage. The HSM
firmware is packed inside the tispl.bin fit image. The firmware is
unpacked into a temporary DDR address which is then used to load HSM
core. The configs to boot HSM M4 core are disabled by default.

v2: Changelog:
[Andrew]:
 1. Added support in SPL to load FIT images with no 'load' property.
 2. Removed 'default = n' in CONFIG option.
 3. Used __maybe_unused to decrease preprocessing.
 4. Better error messages with error code. 
[Udit]:
 1. Added 'HSM' entries in enum at the last.
 2. Added error condition in if-elseif-else ladder.
 3. Hang System boot when HSM failed to boot properly.

Link to v1:
https://lore.kernel.org/all/20250422095430.363792-1-b-padhi@ti.com/

Test logs after enabling HSM boot configs:
https://gist.github.com/3V3RYONE/ad33683652c8c49e4fedab49f0493e79

Beleswar Padhi (7):
  arm: mach-k3: Add config option for booting HSM core
  spl: Use FIT data address as fallback when 'load' property is absent
  arm: dts: k3-binman: Add template for packing HSM firmware
  arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside
    tispl.bin
  arm: mach-k3: Use FIT image data addr as fallback if 'load' prop is
    missing
  arm: mach-k3: Explicitly identify TIFSSTUB images when discarding
    buffers
  arm: mach-k3: r5: common: Add support to boot HSM M4 core

 arch/arm/dts/k3-binman.dtsi        |   9 +++
 arch/arm/dts/k3-j721s2-binman.dtsi |  12 ++++
 arch/arm/dts/k3-j784s4-binman.dtsi |  14 ++++
 arch/arm/mach-k3/Kconfig           |   7 ++
 arch/arm/mach-k3/r5/common.c       | 111 +++++++++++++++++++++++++++--
 common/spl/spl_fit.c               |  16 ++++-
 6 files changed, 164 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH v2 1/7] arm: mach-k3: Add config option for booting HSM core

[Beleswar Padhi]

Some K3 SoCs like J721S2, and J784S4 have a HSM (High Security Module)
M4F core in the Wakeup Voltage Domain which could be used to run secure
services like Authentication. Add a config option which indicates that
the SoC has a HSM core and the firmware for the core is available.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
v2: Changelog:
1. Removed 'default = n' line from CONFIG.

Link to v1:
https://lore.kernel.org/all/20250422095430.363792-2-b-padhi@ti.com/

 arch/arm/mach-k3/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
index 1b8c0b1eb96..8448f23c053 100644
--- a/arch/arm/mach-k3/Kconfig
+++ b/arch/arm/mach-k3/Kconfig
@@ -181,6 +181,13 @@ config K3_REMOTEPROC_PRU
 	imply REMOTEPROC_TI_PRU
 	default y if (SOC_K3_AM642 || SOC_K3_AM654)
 
+config K3_HSM_FW
+	bool "Load and Boot HSM core"
+	help
+	  Enabling this will indicate that the system has a HSM (High Security
+	  Module) M4 Core and the firmware for the core is present. When
+	  enabled, HSM core will be booted at R5 SPL execution time.
+
 if CPU_V7R
 source "arch/arm/mach-k3/r5/Kconfig"
 endif
-- 
2.34.1

[PATCH v2 2/7] spl: Use FIT data address as fallback when 'load' property is absent

[Beleswar Padhi]

In cases where the 'load' property is not defined in a FIT image node,
fallback to using the data address returned by `fit_image_get_data()`.
This enables FIT images to omit the 'load' property during FIT creation.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
v2: Changelog:
1. New patch. Add support to load images without 'load' property.

 common/spl/spl_fit.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 86506d6905c..bf26f96c77b 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -742,6 +742,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 {
 	struct spl_image_info image_info;
 	struct spl_fit_info ctx;
+	const void *fit_image_loadaddr;
+	size_t fit_image_size;
 	int node = -1;
 	int ret;
 	int index = 0;
@@ -832,7 +834,19 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 		if (firmware_node == node)
 			continue;
 
-		image_info.load_addr = 0;
+		/*
+		 * If the 'load' property is not present in the image node,
+		 * use the FIT image's data address as the fallback load
+		 * address. This allows flexibility in omitting the load address
+		 * during FIT creation time.
+		 */
+		ret = fit_image_get_data(ctx.fit, node,
+					 &fit_image_loadaddr, &fit_image_size);
+		if (ret < 0)
+			panic("Error accessing node = %d in FIT (%d)\n", node,
+			      ret);
+
+		image_info.load_addr = (ulong)fit_image_loadaddr;
 		ret = load_simple_fit(info, offset, &ctx, node, &image_info);
 		if (ret < 0 && ret != -EPERM) {
 			printf("%s: can't load image loadables index %d (ret = %d)\n",
-- 
2.34.1

[PATCH v2 3/7] arm: dts: k3-binman: Add template for packing HSM firmware

[Beleswar Padhi]

The HSM M4 core needs to be booted at R5 SPL stage so that it can be
used for further Authentication and security services. Therefore, the
firmware for the HSM core needs to be packed in tispl.bin fit image so
that it can be used by R5 SPL to boot the HSM core.

Add a template for packing the HSM firmware in tispl.bin.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
v2: Changelog:
1. Got rid of 'load' and 'entry' properties. Rely on U-Boot to set it.

Link to v1:
https://lore.kernel.org/all/20250422095430.363792-3-b-padhi@ti.com/

 arch/arm/dts/k3-binman.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
index 5163161b94d..1c65ac44626 100644
--- a/arch/arm/dts/k3-binman.dtsi
+++ b/arch/arm/dts/k3-binman.dtsi
@@ -297,6 +297,15 @@
 					};
 				};
 
+#ifdef CONFIG_K3_HSM_FW
+				hsm {
+					description = "HSM binary";
+					type = "standalone";
+					compression = "none";
+					os = "hsm";
+				};
+#endif
+
 				dm {
 					description = "DM binary";
 					type = "firmware";
-- 
2.34.1

[PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Beleswar Padhi]

Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
and used by R5 SPL to boot the HSM core. By default, point to the
firmware for HS-SE device type. This needs to be changed to point to
appropriate firmware when using a different device type.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
v2: Changelog:
None to this patch.

Link to v1:
https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/

 arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
 arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
index 73af184d27e..9c8b29f53bb 100644
--- a/arch/arm/dts/k3-j721s2-binman.dtsi
+++ b/arch/arm/dts/k3-j721s2-binman.dtsi
@@ -273,6 +273,14 @@
 
 					};
 				};
+#ifdef CONFIG_K3_HSM_FW
+				hsm {
+					hsm: blob-ext {
+						filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
+					};
+				};
+#endif
+
 				dm {
 					ti-secure {
 						content = <&dm>;
@@ -306,7 +314,11 @@
 				conf-0 {
 					description = "k3-j721s2-common-proc-board";
 					firmware = "atf";
+#ifdef CONFIG_K3_HSM_FW
+					loadables = "hsm", "tee", "dm", "spl";
+#else
 					loadables = "tee", "dm", "spl";
+#endif
 					fdt = "fdt-0";
 				};
 			};
diff --git a/arch/arm/dts/k3-j784s4-binman.dtsi b/arch/arm/dts/k3-j784s4-binman.dtsi
index cb1fbc65923..7c8e580a8a3 100644
--- a/arch/arm/dts/k3-j784s4-binman.dtsi
+++ b/arch/arm/dts/k3-j784s4-binman.dtsi
@@ -159,6 +159,16 @@
 
 		fit {
 			images {
+
+#ifdef CONFIG_K3_HSM_FW
+				hsm {
+					hsm: blob-ext {
+						filename = "ti-hsm/hsm-demo-firmware-j784s4-hs.bin";
+					};
+				};
+
+#endif
+
 				dm {
 					ti-secure {
 						content = <&dm>;
@@ -194,7 +204,11 @@
 				conf-0 {
 					description = BOARD_DESCRIPTION;
 					firmware = "atf";
+#ifdef CONFIG_K3_HSM_FW
+					loadables = "hsm", "tee", "dm", "spl";
+#else
 					loadables = "tee", "dm", "spl";
+#endif
 					fdt = "fdt-0";
 				};
 			};
-- 
2.34.1

[PATCH v2 5/7] arm: mach-k3: Use FIT image data addr as fallback if 'load' prop is missing

[Beleswar Padhi]

It is possible for FIT Images to skip mentioning the pre-defined 'load'
address property. In those cases, SPL uses the actual address of the FIT
Image data as the fallback load address. Use this FIT data address for
referencing the image when the 'load' property is missing.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
v2: Changelog:
1. New patch. Fetch FDT Data addr for accessing image if 'load' prop is
not present in node.

 arch/arm/mach-k3/r5/common.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
index 0b6604039f3..da5184e615c 100644
--- a/arch/arm/mach-k3/r5/common.c
+++ b/arch/arm/mach-k3/r5/common.c
@@ -327,12 +327,15 @@ void board_fit_image_post_process(const void *fit, int node, void **p_image,
 				  size_t *p_size)
 {
 	int len;
-	int i;
+	int i, ret;
 	const char *os;
-	u32 addr;
+	u32 addr, load_addr;
+	const void *fit_image_loadaddr;
+	size_t fit_image_size;
 
 	os = fdt_getprop(fit, node, "os", &len);
 	addr = fdt_getprop_u32_default_node(fit, node, 0, "entry", -1);
+	load_addr = fdt_getprop_u32_default_node(fit, node, 0, "load", -1);
 
 	debug("%s: processing image: addr=%x, size=%d, os=%s\n", __func__,
 	      addr, *p_size, os);
@@ -341,6 +344,22 @@ void board_fit_image_post_process(const void *fit, int node, void **p_image,
 		if (!strcmp(os, image_os_match[i])) {
 			fit_image_info[i].image_start = addr;
 			fit_image_info[i].image_len = *p_size;
+			/*
+			 * If the 'load' property is missing in the FIT image,
+			 * fall back to using the actual in-memory address of
+			 * the FIT image data.
+			 */
+			if (load_addr == -1) {
+				ret = fit_image_get_data(fit, node,
+							 &fit_image_loadaddr,
+							 &fit_image_size);
+				if (ret < 0)
+					panic("Error accessing node os = %s in FIT (%d)\n",
+					      os, ret);
+				fit_image_info[i].load = (ulong)fit_image_loadaddr;
+			} else {
+				fit_image_info[i].load = load_addr;
+			}
 			debug("%s: matched image for ID %d\n", __func__, i);
 			break;
 		}
-- 
2.34.1

[PATCH v2 6/7] arm: mach-k3: Explicitly identify TIFSSTUB images when discarding buffers

[Beleswar Padhi]

The board_fit_image_post_process() function assumes that all TIFSSTUB
images appear at the end of the image_os_match[] array by using
the condition:

	i > IMAGE_ID_DM_FW

However, this assumption breaks when new image types are appended to the
enum and the array, causing unintended image types to match this
condition and having their buffer sizes incorrectly set to 0 with:

	*p_size = 0

To avoid this issue, replace the range-based check with an explicit
match for TIFSSTUB image IDs.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
v2: Changelog:
1. New patch. Fixes condition to allow new images to be added at the end
of the enum.

 arch/arm/mach-k3/r5/common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
index da5184e615c..81fc67a0023 100644
--- a/arch/arm/mach-k3/r5/common.c
+++ b/arch/arm/mach-k3/r5/common.c
@@ -365,7 +365,9 @@ void board_fit_image_post_process(const void *fit, int node, void **p_image,
 		}
 	}
 
-	if (i < IMAGE_AMT && i > IMAGE_ID_DM_FW) {
+	if (i < IMAGE_AMT &&
+	    (i == IMAGE_ID_TIFSSTUB_HS || i == IMAGE_ID_TIFSSTUB_FS ||
+	     i == IMAGE_ID_T)) {
 		int device_type = get_device_type();
 
 		if ((device_type == K3_DEVICE_TYPE_HS_SE &&
-- 
2.34.1

[PATCH v2 7/7] arm: mach-k3: r5: common: Add support to boot HSM M4 core

[Beleswar Padhi]

The tispl.bin fit image is packed with the HSM firmware image. Populate
the "os" info of the image so that it can be detected and used to load
the HSM core. Further, invoke the load and boot of HSM core at R5 SPL
stage. Boot flow for HSM M4 core 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.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
v2: Changelog:
1. Hang system boot if HSM firmware failed to boot.
2. __maybe_unused to decrease preprocessor usage.
3. Better error messages with return code.
4. Added Error case in if-elseif-else ladder.

Note:
#define PROC_ID_HSM_M4F seems to have extra tab in the diff/patch.
But when patch gets applied in file, all of them have consistent
tabs.

Link to v1:
https://lore.kernel.org/all/18e01808-499d-4690-995a-45ac5fd727d9@ti.com

 arch/arm/mach-k3/r5/common.c | 84 +++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
index 81fc67a0023..1aa3869cec2 100644
--- a/arch/arm/mach-k3/r5/common.c
+++ b/arch/arm/mach-k3/r5/common.c
@@ -15,9 +15,14 @@
 #include <spl.h>
 #include <remoteproc.h>
 #include <elf.h>
+#include <cpu_func.h>
 
 #include "../common.h"
 
+#define PROC_BOOT_CTRL_RESET_FLAG_HSM_M4	0x00000001
+#define HSM_SRAM0_0_ADDR			0x43C00000
+#define PROC_ID_HSM_M4F				0x00000080
+
 #if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
 enum {
 	IMAGE_ID_ATF,
@@ -27,6 +32,7 @@ enum {
 	IMAGE_ID_TIFSSTUB_HS,
 	IMAGE_ID_TIFSSTUB_FS,
 	IMAGE_ID_T,
+	IMAGE_ID_HSM,
 	IMAGE_AMT,
 };
 
@@ -39,6 +45,7 @@ static const char *image_os_match[IMAGE_AMT] = {
 	"tifsstub-hs",
 	"tifsstub-fs",
 	"tifsstub-gp",
+	"hsm",
 };
 #endif
 
@@ -136,6 +143,73 @@ void release_resources_for_core_shutdown(void)
 	}
 }
 
+static int __maybe_unused boot_hsm_core(void)
+{
+	struct ti_sci_handle *ti_sci = get_ti_sci_handle();
+	struct ti_sci_proc_ops *proc_ops = &ti_sci->ops.proc_ops;
+	u64 hsm_image_addr;
+	u32 hsm_image_size;
+	int device_type, ret;
+
+	hsm_image_addr = (u64)fit_image_info[IMAGE_ID_HSM].load;
+	hsm_image_size = (u32)fit_image_info[IMAGE_ID_HSM].image_len;
+
+	/* Request Control of Remote Processor */
+	ret = proc_ops->proc_request(ti_sci, PROC_ID_HSM_M4F);
+	if (ret) {
+		printf("Unable to request HSM processor control: %d\n", ret);
+		return ret;
+	}
+
+	/* Put the remote processor into reset */
+	ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F,
+					   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4, 0);
+	if (ret) {
+		printf("Unable to Halt HSM core: %d\n", ret);
+		goto release_proc_ctrl;
+	}
+
+	/*
+	 * Load the HSM firmware into core's internal memory.
+	 *
+	 * In case of HS device types, request secure entity to authenticate and
+	 * load the HSM firmware into the core memory.
+	 * In case of GP device types, copy the HSM firmware into the core
+	 * memory manually.
+	 */
+	device_type = get_device_type();
+	if (device_type == K3_DEVICE_TYPE_HS_SE ||
+	    device_type == K3_DEVICE_TYPE_HS_FS) {
+		ret = proc_ops->proc_auth_boot_image(ti_sci, &hsm_image_addr,
+						     &hsm_image_size);
+		if (ret) {
+			printf("Unable to Authenticate and Boot HSM image; ret = %d\n",
+			       ret);
+			goto release_proc_ctrl;
+		}
+	} else if (device_type == K3_DEVICE_TYPE_GP) {
+		debug("Loading HSM GP binary into SRAM0_0\n");
+		memcpy((void *)HSM_SRAM0_0_ADDR, (void *)(u32)hsm_image_addr,
+		       hsm_image_size);
+		flush_dcache_range(HSM_SRAM0_0_ADDR,
+				   HSM_SRAM0_0_ADDR + hsm_image_size);
+	} else {
+		printf("Invalid Device Type\n");
+		return -EINVAL;
+	}
+
+	/* Release the reset from the remote processor*/
+	ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F, 0,
+					   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4);
+	if (ret)
+		printf("Unable to Run HSM core: %d\n", ret);
+
+release_proc_ctrl:
+	/* Release Control of Remote Processor */
+	proc_ops->proc_release(ti_sci, PROC_ID_HSM_M4F);
+	return ret;
+}
+
 void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 {
 	typedef void __noreturn (*image_entry_noargs_t)(void);
@@ -157,6 +231,14 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 				     &loadaddr);
 	}
 
+#if IS_ENABLED(CONFIG_K3_HSM_FW)
+	ret = boot_hsm_core();
+	if (ret)
+		panic("HSM core failed to boot, %d\n", ret);
+	else
+		printf("Successfully booted HSM core\n");
+#endif
+
 	/*
 	 * It is assumed that remoteproc device 1 is the corresponding
 	 * Cortex-A core which runs ATF. Make sure DT reflects the same.
@@ -388,7 +470,7 @@ void board_fit_image_post_process(const void *fit, int node, void **p_image,
 	 * Only DM and the DTBs are being authenticated here,
 	 * rest will be authenticated when A72 cluster is up
 	 */
-	if ((i != IMAGE_ID_ATF) && (i != IMAGE_ID_OPTEE)) {
+	if (i != IMAGE_ID_ATF && i != IMAGE_ID_OPTEE && i != IMAGE_ID_HSM) {
 		ti_secure_image_check_binary(p_image, p_size);
 		ti_secure_image_post_process(p_image, p_size);
 	} else {
-- 
2.34.1

Re: [PATCH v2 7/7] arm: mach-k3: r5: common: Add support to boot HSM M4 core

[Andrew Davis]

On 5/6/25 5:42 AM, Beleswar Padhi wrote:
> The tispl.bin fit image is packed with the HSM firmware image. Populate
> the "os" info of the image so that it can be detected and used to load
> the HSM core. Further, invoke the load and boot of HSM core at R5 SPL
> stage. Boot flow for HSM M4 core 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.
> 
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---
> v2: Changelog:
> 1. Hang system boot if HSM firmware failed to boot.
> 2. __maybe_unused to decrease preprocessor usage.
> 3. Better error messages with return code.
> 4. Added Error case in if-elseif-else ladder.
> 
> Note:
> #define PROC_ID_HSM_M4F seems to have extra tab in the diff/patch.
> But when patch gets applied in file, all of them have consistent
> tabs.
> 
> Link to v1:
> https://lore.kernel.org/all/18e01808-499d-4690-995a-45ac5fd727d9@ti.com
> 
>   arch/arm/mach-k3/r5/common.c | 84 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
> index 81fc67a0023..1aa3869cec2 100644
> --- a/arch/arm/mach-k3/r5/common.c
> +++ b/arch/arm/mach-k3/r5/common.c
> @@ -15,9 +15,14 @@
>   #include <spl.h>
>   #include <remoteproc.h>
>   #include <elf.h>
> +#include <cpu_func.h>
>   
>   #include "../common.h"
>   
> +#define PROC_BOOT_CTRL_RESET_FLAG_HSM_M4	0x00000001
> +#define HSM_SRAM0_0_ADDR			0x43C00000
> +#define PROC_ID_HSM_M4F				0x00000080
> +
>   #if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
>   enum {
>   	IMAGE_ID_ATF,
> @@ -27,6 +32,7 @@ enum {
>   	IMAGE_ID_TIFSSTUB_HS,
>   	IMAGE_ID_TIFSSTUB_FS,
>   	IMAGE_ID_T,

The above line looks to be a typo, must have been meant to be IMAGE_ID_TIFSSTUB_GP.

I know this series didn't introduce this typo, but could you fix it in this series
since you touch lines around it and use it in patch [6/7]. That way we don't get
any merge conflicts if fixed in standalone patch.

> +	IMAGE_ID_HSM,
>   	IMAGE_AMT,
>   };
>   
> @@ -39,6 +45,7 @@ static const char *image_os_match[IMAGE_AMT] = {
>   	"tifsstub-hs",
>   	"tifsstub-fs",
>   	"tifsstub-gp",
> +	"hsm",
>   };
>   #endif
>   
> @@ -136,6 +143,73 @@ void release_resources_for_core_shutdown(void)
>   	}
>   }
>   
> +static int __maybe_unused boot_hsm_core(void)
> +{
> +	struct ti_sci_handle *ti_sci = get_ti_sci_handle();
> +	struct ti_sci_proc_ops *proc_ops = &ti_sci->ops.proc_ops;
> +	u64 hsm_image_addr;
> +	u32 hsm_image_size;
> +	int device_type, ret;
> +
> +	hsm_image_addr = (u64)fit_image_info[IMAGE_ID_HSM].load;
> +	hsm_image_size = (u32)fit_image_info[IMAGE_ID_HSM].image_len;
> +
> +	/* Request Control of Remote Processor */
> +	ret = proc_ops->proc_request(ti_sci, PROC_ID_HSM_M4F);
> +	if (ret) {
> +		printf("Unable to request HSM processor control: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Put the remote processor into reset */
> +	ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F,
> +					   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4, 0);
> +	if (ret) {
> +		printf("Unable to Halt HSM core: %d\n", ret);
> +		goto release_proc_ctrl;
> +	}
> +
> +	/*
> +	 * Load the HSM firmware into core's internal memory.
> +	 *
> +	 * In case of HS device types, request secure entity to authenticate and
> +	 * load the HSM firmware into the core memory.
> +	 * In case of GP device types, copy the HSM firmware into the core
> +	 * memory manually.
> +	 */
> +	device_type = get_device_type();
> +	if (device_type == K3_DEVICE_TYPE_HS_SE ||
> +	    device_type == K3_DEVICE_TYPE_HS_FS) {
> +		ret = proc_ops->proc_auth_boot_image(ti_sci, &hsm_image_addr,
> +						     &hsm_image_size);

Could you use ti_secure_image_post_process() here? It should be able to replace
a bunch of the surrounding code too. If it returns without modifying &size the
you can go down the GP path.

ti_secure_image_post_process() also flushes the image out of caches before
the TI-SCI call, without that there are cases where the image will not be
readable by the TIFS core.

Andrew

> +		if (ret) {
> +			printf("Unable to Authenticate and Boot HSM image; ret = %d\n",
> +			       ret);
> +			goto release_proc_ctrl;
> +		}
> +	} else if (device_type == K3_DEVICE_TYPE_GP) {
> +		debug("Loading HSM GP binary into SRAM0_0\n");
> +		memcpy((void *)HSM_SRAM0_0_ADDR, (void *)(u32)hsm_image_addr,
> +		       hsm_image_size);
> +		flush_dcache_range(HSM_SRAM0_0_ADDR,
> +				   HSM_SRAM0_0_ADDR + hsm_image_size);
> +	} else {
> +		printf("Invalid Device Type\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Release the reset from the remote processor*/
> +	ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F, 0,
> +					   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4);
> +	if (ret)
> +		printf("Unable to Run HSM core: %d\n", ret);
> +
> +release_proc_ctrl:
> +	/* Release Control of Remote Processor */
> +	proc_ops->proc_release(ti_sci, PROC_ID_HSM_M4F);
> +	return ret;
> +}
> +
>   void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>   {
>   	typedef void __noreturn (*image_entry_noargs_t)(void);
> @@ -157,6 +231,14 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>   				     &loadaddr);
>   	}
>   
> +#if IS_ENABLED(CONFIG_K3_HSM_FW)
> +	ret = boot_hsm_core();
> +	if (ret)
> +		panic("HSM core failed to boot, %d\n", ret);
> +	else
> +		printf("Successfully booted HSM core\n");
> +#endif
> +
>   	/*
>   	 * It is assumed that remoteproc device 1 is the corresponding
>   	 * Cortex-A core which runs ATF. Make sure DT reflects the same.
> @@ -388,7 +470,7 @@ void board_fit_image_post_process(const void *fit, int node, void **p_image,
>   	 * Only DM and the DTBs are being authenticated here,
>   	 * rest will be authenticated when A72 cluster is up
>   	 */
> -	if ((i != IMAGE_ID_ATF) && (i != IMAGE_ID_OPTEE)) {
> +	if (i != IMAGE_ID_ATF && i != IMAGE_ID_OPTEE && i != IMAGE_ID_HSM) {
>   		ti_secure_image_check_binary(p_image, p_size);
>   		ti_secure_image_post_process(p_image, p_size);
>   	} else {

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Andrew Davis]

On 5/6/25 5:41 AM, Beleswar Padhi wrote:
> Some TI K3 SoCs like J721S2, and J784S4 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:
> 

The below flow looks exactly like the general purpose M4F cores..
Why is the HSM core treated differently and this loader not made into
a normal remote proc driver?

Andrew

> 1. Request control of HSM M4F remote processor.
> 2. Assert Reset on the HSM M4F remote processor.
> 3. Request Secure Entity to Authenticate and Load HSM firmware into
>     core's internal SRAM memory region. For GP device, load the firmware
>     manually into core's SRAM region.
> 4. Deassert Reset on the HSM M4F remote processor.
> 5. Release control of HSM M4F remote processor.
> 
> This series adds support to boot HSM M4 core from R5 SPL stage. The HSM
> firmware is packed inside the tispl.bin fit image. The firmware is
> unpacked into a temporary DDR address which is then used to load HSM
> core. The configs to boot HSM M4 core are disabled by default.
> 
> v2: Changelog:
> [Andrew]:
>   1. Added support in SPL to load FIT images with no 'load' property.
>   2. Removed 'default = n' in CONFIG option.
>   3. Used __maybe_unused to decrease preprocessing.
>   4. Better error messages with error code.
> [Udit]:
>   1. Added 'HSM' entries in enum at the last.
>   2. Added error condition in if-elseif-else ladder.
>   3. Hang System boot when HSM failed to boot properly.
> 
> Link to v1:
> https://lore.kernel.org/all/20250422095430.363792-1-b-padhi@ti.com/
> 
> Test logs after enabling HSM boot configs:
> https://gist.github.com/3V3RYONE/ad33683652c8c49e4fedab49f0493e79
> 
> Beleswar Padhi (7):
>    arm: mach-k3: Add config option for booting HSM core
>    spl: Use FIT data address as fallback when 'load' property is absent
>    arm: dts: k3-binman: Add template for packing HSM firmware
>    arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside
>      tispl.bin
>    arm: mach-k3: Use FIT image data addr as fallback if 'load' prop is
>      missing
>    arm: mach-k3: Explicitly identify TIFSSTUB images when discarding
>      buffers
>    arm: mach-k3: r5: common: Add support to boot HSM M4 core
> 
>   arch/arm/dts/k3-binman.dtsi        |   9 +++
>   arch/arm/dts/k3-j721s2-binman.dtsi |  12 ++++
>   arch/arm/dts/k3-j784s4-binman.dtsi |  14 ++++
>   arch/arm/mach-k3/Kconfig           |   7 ++
>   arch/arm/mach-k3/r5/common.c       | 111 +++++++++++++++++++++++++++--
>   common/spl/spl_fit.c               |  16 ++++-
>   6 files changed, 164 insertions(+), 5 deletions(-)
> 

Re: [PATCH v2 7/7] arm: mach-k3: r5: common: Add support to boot HSM M4 core

[Beleswar Prasad Padhi]

Hi Andrew,

On 5/6/2025 4:36 PM, Andrew Davis wrote:
> On 5/6/25 5:42 AM, Beleswar Padhi wrote:
>> The tispl.bin fit image is packed with the HSM firmware image. Populate
>> the "os" info of the image so that it can be detected and used to load
>> the HSM core. Further, invoke the load and boot of HSM core at R5 SPL
>> stage. Boot flow for HSM M4 core 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.
>>
>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>> ---
>> v2: Changelog:
>> 1. Hang system boot if HSM firmware failed to boot.
>> 2. __maybe_unused to decrease preprocessor usage.
>> 3. Better error messages with return code.
>> 4. Added Error case in if-elseif-else ladder.
>>
>> Note:
>> #define PROC_ID_HSM_M4F seems to have extra tab in the diff/patch.
>> But when patch gets applied in file, all of them have consistent
>> tabs.
>>
>> Link to v1:
>> https://lore.kernel.org/all/18e01808-499d-4690-995a-45ac5fd727d9@ti.com
>>
>>   arch/arm/mach-k3/r5/common.c | 84 +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 83 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
>> index 81fc67a0023..1aa3869cec2 100644
>> --- a/arch/arm/mach-k3/r5/common.c
>> +++ b/arch/arm/mach-k3/r5/common.c
>> @@ -15,9 +15,14 @@
>>   #include <spl.h>
>>   #include <remoteproc.h>
>>   #include <elf.h>
>> +#include <cpu_func.h>
>>     #include "../common.h"
>>   +#define PROC_BOOT_CTRL_RESET_FLAG_HSM_M4    0x00000001
>> +#define HSM_SRAM0_0_ADDR            0x43C00000
>> +#define PROC_ID_HSM_M4F                0x00000080
>> +
>>   #if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
>>   enum {
>>       IMAGE_ID_ATF,
>> @@ -27,6 +32,7 @@ enum {
>>       IMAGE_ID_TIFSSTUB_HS,
>>       IMAGE_ID_TIFSSTUB_FS,
>>       IMAGE_ID_T,
>
> The above line looks to be a typo, must have been meant to be 
> IMAGE_ID_TIFSSTUB_GP.
>
> I know this series didn't introduce this typo, but could you fix it in 
> this series
> since you touch lines around it and use it in patch [6/7]. That way we 
> don't get
> any merge conflicts if fixed in standalone patch.


Sure, I will fix that in PATCH 6/7.

>
>> +    IMAGE_ID_HSM,
>>       IMAGE_AMT,
>>   };
>>   @@ -39,6 +45,7 @@ static const char *image_os_match[IMAGE_AMT] = {
>>       "tifsstub-hs",
>>       "tifsstub-fs",
>>       "tifsstub-gp",
>> +    "hsm",
>>   };
>>   #endif
>>   @@ -136,6 +143,73 @@ void release_resources_for_core_shutdown(void)
>>       }
>>   }
>>   +static int __maybe_unused boot_hsm_core(void)
>> +{
>> +    struct ti_sci_handle *ti_sci = get_ti_sci_handle();
>> +    struct ti_sci_proc_ops *proc_ops = &ti_sci->ops.proc_ops;
>> +    u64 hsm_image_addr;
>> +    u32 hsm_image_size;
>> +    int device_type, ret;
>> +
>> +    hsm_image_addr = (u64)fit_image_info[IMAGE_ID_HSM].load;
>> +    hsm_image_size = (u32)fit_image_info[IMAGE_ID_HSM].image_len;
>> +
>> +    /* Request Control of Remote Processor */
>> +    ret = proc_ops->proc_request(ti_sci, PROC_ID_HSM_M4F);
>> +    if (ret) {
>> +        printf("Unable to request HSM processor control: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    /* Put the remote processor into reset */
>> +    ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F,
>> +                       PROC_BOOT_CTRL_RESET_FLAG_HSM_M4, 0);
>> +    if (ret) {
>> +        printf("Unable to Halt HSM core: %d\n", ret);
>> +        goto release_proc_ctrl;
>> +    }
>> +
>> +    /*
>> +     * Load the HSM firmware into core's internal memory.
>> +     *
>> +     * In case of HS device types, request secure entity to 
>> authenticate and
>> +     * load the HSM firmware into the core memory.
>> +     * In case of GP device types, copy the HSM firmware into the core
>> +     * memory manually.
>> +     */
>> +    device_type = get_device_type();
>> +    if (device_type == K3_DEVICE_TYPE_HS_SE ||
>> +        device_type == K3_DEVICE_TYPE_HS_FS) {
>> +        ret = proc_ops->proc_auth_boot_image(ti_sci, &hsm_image_addr,
>> +                             &hsm_image_size);
>
> Could you use ti_secure_image_post_process() here? It should be able 
> to replace
> a bunch of the surrounding code too. If it returns without modifying 
> &size the
> you can go down the GP path.
>
> ti_secure_image_post_process() also flushes the image out of caches 
> before
> the TI-SCI call, without that there are cases where the image will not be
> readable by the TIFS core.


Thanks, that's cleaner, will do. Will wait for more reviews before 
re-spinning.

Thanks,
Beleswar

>
> Andrew
>
>> +        if (ret) {
>> +            printf("Unable to Authenticate and Boot HSM image; ret = 
>> %d\n",
>> +                   ret);
>> +            goto release_proc_ctrl;
>> +        }
>> +    } else if (device_type == K3_DEVICE_TYPE_GP) {
>> +        debug("Loading HSM GP binary into SRAM0_0\n");
>> +        memcpy((void *)HSM_SRAM0_0_ADDR, (void *)(u32)hsm_image_addr,
>> +               hsm_image_size);
>> +        flush_dcache_range(HSM_SRAM0_0_ADDR,
>> +                   HSM_SRAM0_0_ADDR + hsm_image_size);
>> +    } else {
>> +        printf("Invalid Device Type\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Release the reset from the remote processor*/
>> +    ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F, 0,
>> +                       PROC_BOOT_CTRL_RESET_FLAG_HSM_M4);
>> +    if (ret)
>> +        printf("Unable to Run HSM core: %d\n", ret);
>> +
>> +release_proc_ctrl:
>> +    /* Release Control of Remote Processor */
>> +    proc_ops->proc_release(ti_sci, PROC_ID_HSM_M4F);
>> +    return ret;
>> +}
>> +
>>   void __noreturn jump_to_image_no_args(struct spl_image_info 
>> *spl_image)
>>   {
>>       typedef void __noreturn (*image_entry_noargs_t)(void);
>> @@ -157,6 +231,14 @@ void __noreturn jump_to_image_no_args(struct 
>> spl_image_info *spl_image)
>>                        &loadaddr);
>>       }
>>   +#if IS_ENABLED(CONFIG_K3_HSM_FW)
>> +    ret = boot_hsm_core();
>> +    if (ret)
>> +        panic("HSM core failed to boot, %d\n", ret);
>> +    else
>> +        printf("Successfully booted HSM core\n");
>> +#endif
>> +
>>       /*
>>        * It is assumed that remoteproc device 1 is the corresponding
>>        * Cortex-A core which runs ATF. Make sure DT reflects the same.
>> @@ -388,7 +470,7 @@ void board_fit_image_post_process(const void 
>> *fit, int node, void **p_image,
>>        * Only DM and the DTBs are being authenticated here,
>>        * rest will be authenticated when A72 cluster is up
>>        */
>> -    if ((i != IMAGE_ID_ATF) && (i != IMAGE_ID_OPTEE)) {
>> +    if (i != IMAGE_ID_ATF && i != IMAGE_ID_OPTEE && i != 
>> IMAGE_ID_HSM) {
>>           ti_secure_image_check_binary(p_image, p_size);
>>           ti_secure_image_post_process(p_image, p_size);
>>       } else {

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Beleswar Prasad Padhi]

Hi Andrew,

On 5/6/2025 4:38 PM, Andrew Davis wrote:
> On 5/6/25 5:41 AM, Beleswar Padhi wrote:
>> Some TI K3 SoCs like J721S2, and J784S4 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:
>>
>
> The below flow looks exactly like the general purpose M4F cores..
> Why is the HSM core treated differently and this loader not made into
> a normal remote proc driver?


Not exactly, HSM core is treated differently because of following 
exceptions:
1. Device operations for HSM core (like reset/reset release) are not 
handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.
2. The HSM firmware is not an ELF image. So we can't use rproc elf 
loader with it. Manual memcpy has to be done.

All of that can still be accounted in the existing M4 rproc driver, but 
it will be a lot of if-else checks, which I don't prefer. Let me know if 
you prefer that way.

Patch having HSM support in M4 remoteproc driver:
https://gist.github.com/3V3RYONE/a15a5c6933bbc83278da9860c25ec21c

>
> Andrew
>
>> 1. Request control of HSM M4F remote processor.
>> 2. Assert Reset on the HSM M4F remote processor.
>> 3. Request Secure Entity to Authenticate and Load HSM firmware into
>>     core's internal SRAM memory region. For GP device, load the firmware
>>     manually into core's SRAM region.
>> 4. Deassert Reset on the HSM M4F remote processor.
>> 5. Release control of HSM M4F remote processor.
>>
>> This series adds support to boot HSM M4 core from R5 SPL stage. The HSM
>> firmware is packed inside the tispl.bin fit image. The firmware is
>> unpacked into a temporary DDR address which is then used to load HSM
>> core. The configs to boot HSM M4 core are disabled by default.
>>
>> v2: Changelog:
>> [Andrew]:
>>   1. Added support in SPL to load FIT images with no 'load' property.
>>   2. Removed 'default = n' in CONFIG option.
>>   3. Used __maybe_unused to decrease preprocessing.
>>   4. Better error messages with error code.
>> [Udit]:
>>   1. Added 'HSM' entries in enum at the last.
>>   2. Added error condition in if-elseif-else ladder.
>>   3. Hang System boot when HSM failed to boot properly.
>>
>> Link to v1:
>> https://lore.kernel.org/all/20250422095430.363792-1-b-padhi@ti.com/
>>
>> Test logs after enabling HSM boot configs:
>> https://gist.github.com/3V3RYONE/ad33683652c8c49e4fedab49f0493e79
>>
>> Beleswar Padhi (7):
>>    arm: mach-k3: Add config option for booting HSM core
>>    spl: Use FIT data address as fallback when 'load' property is absent
>>    arm: dts: k3-binman: Add template for packing HSM firmware
>>    arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside
>>      tispl.bin
>>    arm: mach-k3: Use FIT image data addr as fallback if 'load' prop is
>>      missing
>>    arm: mach-k3: Explicitly identify TIFSSTUB images when discarding
>>      buffers
>>    arm: mach-k3: r5: common: Add support to boot HSM M4 core
>>
>>   arch/arm/dts/k3-binman.dtsi        |   9 +++
>>   arch/arm/dts/k3-j721s2-binman.dtsi |  12 ++++
>>   arch/arm/dts/k3-j784s4-binman.dtsi |  14 ++++
>>   arch/arm/mach-k3/Kconfig           |   7 ++
>>   arch/arm/mach-k3/r5/common.c       | 111 +++++++++++++++++++++++++++--
>>   common/spl/spl_fit.c               |  16 ++++-
>>   6 files changed, 164 insertions(+), 5 deletions(-)
>>

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Andrew Davis]

On 5/6/25 9:51 AM, Beleswar Prasad Padhi wrote:
> Hi Andrew,
> 
> On 5/6/2025 4:38 PM, Andrew Davis wrote:
>> On 5/6/25 5:41 AM, Beleswar Padhi wrote:
>>> Some TI K3 SoCs like J721S2, and J784S4 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:
>>>
>>
>> The below flow looks exactly like the general purpose M4F cores..
>> Why is the HSM core treated differently and this loader not made into
>> a normal remote proc driver?
> 
> 
> Not exactly, HSM core is treated differently because of following exceptions:
> 1. Device operations for HSM core (like reset/reset release) are not handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.

This could be added to the commit message then.

> 2. The HSM firmware is not an ELF image. So we can't use rproc elf loader with it. Manual memcpy has to be done.
> 

Why is the HSM firmware not an ELF image?

> All of that can still be accounted in the existing M4 rproc driver, but it will be a lot of if-else checks, which I don't prefer. Let me know if you prefer that way.
> 

I'm not a fan of if-else checks either, just trying to get an idea
of how many would really be needed. Judging from the below commit
I'd agree keeping it here would be the cleaner option for now.

Andrew

> Patch having HSM support in M4 remoteproc driver:
> https://gist.github.com/3V3RYONE/a15a5c6933bbc83278da9860c25ec21c
> 
>>
>> Andrew
>>
>>> 1. Request control of HSM M4F remote processor.
>>> 2. Assert Reset on the HSM M4F remote processor.
>>> 3. Request Secure Entity to Authenticate and Load HSM firmware into
>>>     core's internal SRAM memory region. For GP device, load the firmware
>>>     manually into core's SRAM region.
>>> 4. Deassert Reset on the HSM M4F remote processor.
>>> 5. Release control of HSM M4F remote processor.
>>>
>>> This series adds support to boot HSM M4 core from R5 SPL stage. The HSM
>>> firmware is packed inside the tispl.bin fit image. The firmware is
>>> unpacked into a temporary DDR address which is then used to load HSM
>>> core. The configs to boot HSM M4 core are disabled by default.
>>>
>>> v2: Changelog:
>>> [Andrew]:
>>>   1. Added support in SPL to load FIT images with no 'load' property.
>>>   2. Removed 'default = n' in CONFIG option.
>>>   3. Used __maybe_unused to decrease preprocessing.
>>>   4. Better error messages with error code.
>>> [Udit]:
>>>   1. Added 'HSM' entries in enum at the last.
>>>   2. Added error condition in if-elseif-else ladder.
>>>   3. Hang System boot when HSM failed to boot properly.
>>>
>>> Link to v1:
>>> https://lore.kernel.org/all/20250422095430.363792-1-b-padhi@ti.com/
>>>
>>> Test logs after enabling HSM boot configs:
>>> https://gist.github.com/3V3RYONE/ad33683652c8c49e4fedab49f0493e79
>>>
>>> Beleswar Padhi (7):
>>>    arm: mach-k3: Add config option for booting HSM core
>>>    spl: Use FIT data address as fallback when 'load' property is absent
>>>    arm: dts: k3-binman: Add template for packing HSM firmware
>>>    arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside
>>>      tispl.bin
>>>    arm: mach-k3: Use FIT image data addr as fallback if 'load' prop is
>>>      missing
>>>    arm: mach-k3: Explicitly identify TIFSSTUB images when discarding
>>>      buffers
>>>    arm: mach-k3: r5: common: Add support to boot HSM M4 core
>>>
>>>   arch/arm/dts/k3-binman.dtsi        |   9 +++
>>>   arch/arm/dts/k3-j721s2-binman.dtsi |  12 ++++
>>>   arch/arm/dts/k3-j784s4-binman.dtsi |  14 ++++
>>>   arch/arm/mach-k3/Kconfig           |   7 ++
>>>   arch/arm/mach-k3/r5/common.c       | 111 +++++++++++++++++++++++++++--
>>>   common/spl/spl_fit.c               |  16 ++++-
>>>   6 files changed, 164 insertions(+), 5 deletions(-)
>>>

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Kumar, Udit]

On 5/6/2025 8:38 PM, Andrew Davis wrote:
> On 5/6/25 9:51 AM, Beleswar Prasad Padhi wrote:
>> Hi Andrew,
>>
>> On 5/6/2025 4:38 PM, Andrew Davis wrote:
>>> On 5/6/25 5:41 AM, Beleswar Padhi wrote:
>>>> Some TI K3 SoCs like J721S2, and J784S4 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:
>>>>
>>>
>>> The below flow looks exactly like the general purpose M4F cores..
>>> Why is the HSM core treated differently and this loader not made into
>>> a normal remote proc driver?
>>
>>
>> Not exactly, HSM core is treated differently because of following 
>> exceptions:
>> 1. Device operations for HSM core (like reset/reset release) are not 
>> handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.
>
> This could be added to the commit message then.
>
>> 2. The HSM firmware is not an ELF image. So we can't use rproc elf 
>> loader with it. Manual memcpy has to be done.
>>
>
> Why is the HSM firmware not an ELF image?


HSM firmwares are loaded by TIFS (in non-GP) flow, and TIFS does not 
have elf parser.



>
>> All of that can still be accounted in the existing M4 rproc driver, 
>> but it will be a lot of if-else checks, which I don't prefer. Let me 
>> know if you prefer that way.
>>
>
> I'm not a fan of if-else checks either, just trying to get an idea
> of how many would really be needed. Judging from the below commit
> I'd agree keeping it here would be the cleaner option for now.
>
> Andrew
>
>> Patch having HSM support in M4 remoteproc driver:
>> https://gist.github.com/3V3RYONE/a15a5c6933bbc83278da9860c25ec21c
>>
>>>
>>> Andrew
>>>
>>>> 1. Request control of HSM M4F remote processor.
>>>> 2. Assert Reset on the HSM M4F remote processor.
>>>> 3. Request Secure Entity to Authenticate and Load HSM firmware into
>>>>     core's internal SRAM memory region. For GP device, load the 
>>>> firmware
>>>>     manually into core's SRAM region.
>>>> 4. Deassert Reset on the HSM M4F remote processor.
>>>> 5. Release control of HSM M4F remote processor.
>>>>
>>>> This series adds support to boot HSM M4 core from R5 SPL stage. The 
>>>> HSM
>>>> firmware is packed inside the tispl.bin fit image. The firmware is
>>>> unpacked into a temporary DDR address which is then used to load HSM
>>>> core. The configs to boot HSM M4 core are disabled by default.
>>>>
>>>> v2: Changelog:
>>>> [Andrew]:
>>>>   1. Added support in SPL to load FIT images with no 'load' property.
>>>>   2. Removed 'default = n' in CONFIG option.
>>>>   3. Used __maybe_unused to decrease preprocessing.
>>>>   4. Better error messages with error code.
>>>> [Udit]:
>>>>   1. Added 'HSM' entries in enum at the last.
>>>>   2. Added error condition in if-elseif-else ladder.
>>>>   3. Hang System boot when HSM failed to boot properly.
>>>>
>>>> Link to v1:
>>>> https://lore.kernel.org/all/20250422095430.363792-1-b-padhi@ti.com/
>>>>
>>>> Test logs after enabling HSM boot configs:
>>>> https://gist.github.com/3V3RYONE/ad33683652c8c49e4fedab49f0493e79
>>>>
>>>> Beleswar Padhi (7):
>>>>    arm: mach-k3: Add config option for booting HSM core
>>>>    spl: Use FIT data address as fallback when 'load' property is 
>>>> absent
>>>>    arm: dts: k3-binman: Add template for packing HSM firmware
>>>>    arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside
>>>>      tispl.bin
>>>>    arm: mach-k3: Use FIT image data addr as fallback if 'load' prop is
>>>>      missing
>>>>    arm: mach-k3: Explicitly identify TIFSSTUB images when discarding
>>>>      buffers
>>>>    arm: mach-k3: r5: common: Add support to boot HSM M4 core
>>>>
>>>>   arch/arm/dts/k3-binman.dtsi        |   9 +++
>>>>   arch/arm/dts/k3-j721s2-binman.dtsi |  12 ++++
>>>>   arch/arm/dts/k3-j784s4-binman.dtsi |  14 ++++
>>>>   arch/arm/mach-k3/Kconfig           |   7 ++
>>>>   arch/arm/mach-k3/r5/common.c       | 111 
>>>> +++++++++++++++++++++++++++--
>>>>   common/spl/spl_fit.c               |  16 ++++-
>>>>   6 files changed, 164 insertions(+), 5 deletions(-)
>>>>

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

On Tue, May 06, 2025 at 10:47:57PM +0530, Kumar, Udit wrote:
> 
> On 5/6/2025 8:38 PM, Andrew Davis wrote:
> > On 5/6/25 9:51 AM, Beleswar Prasad Padhi wrote:
> > > Hi Andrew,
> > > 
> > > On 5/6/2025 4:38 PM, Andrew Davis wrote:
> > > > On 5/6/25 5:41 AM, Beleswar Padhi wrote:
> > > > > Some TI K3 SoCs like J721S2, and J784S4 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:
> > > > > 
> > > > 
> > > > The below flow looks exactly like the general purpose M4F cores..
> > > > Why is the HSM core treated differently and this loader not made into
> > > > a normal remote proc driver?
> > > 
> > > 
> > > Not exactly, HSM core is treated differently because of following
> > > exceptions:
> > > 1. Device operations for HSM core (like reset/reset release) are not
> > > handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.
> > 
> > This could be added to the commit message then.
> > 
> > > 2. The HSM firmware is not an ELF image. So we can't use rproc elf
> > > loader with it. Manual memcpy has to be done.
> > > 
> > 
> > Why is the HSM firmware not an ELF image?
> 
> 
> HSM firmwares are loaded by TIFS (in non-GP) flow, and TIFS does not have
> elf parser.

And to be clear, why is that? Since it's preventing the use other
existing workflows...

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Andrew Davis]

On 5/6/25 12:19 PM, Tom Rini wrote:
> On Tue, May 06, 2025 at 10:47:57PM +0530, Kumar, Udit wrote:
>>
>> On 5/6/2025 8:38 PM, Andrew Davis wrote:
>>> On 5/6/25 9:51 AM, Beleswar Prasad Padhi wrote:
>>>> Hi Andrew,
>>>>
>>>> On 5/6/2025 4:38 PM, Andrew Davis wrote:
>>>>> On 5/6/25 5:41 AM, Beleswar Padhi wrote:
>>>>>> Some TI K3 SoCs like J721S2, and J784S4 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:
>>>>>>
>>>>>
>>>>> The below flow looks exactly like the general purpose M4F cores..
>>>>> Why is the HSM core treated differently and this loader not made into
>>>>> a normal remote proc driver?
>>>>
>>>>
>>>> Not exactly, HSM core is treated differently because of following
>>>> exceptions:
>>>> 1. Device operations for HSM core (like reset/reset release) are not
>>>> handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.
>>>
>>> This could be added to the commit message then.
>>>
>>>> 2. The HSM firmware is not an ELF image. So we can't use rproc elf
>>>> loader with it. Manual memcpy has to be done.
>>>>
>>>
>>> Why is the HSM firmware not an ELF image?
>>
>>
>> HSM firmwares are loaded by TIFS (in non-GP) flow, and TIFS does not have
>> elf parser.
> 
> And to be clear, why is that? Since it's preventing the use other
> existing workflows...
> 

Even if the firmware is only delivered as a raw blob we should still
be able to use existing workflows and make a proper rproc driver for
this HSM core here.

TF-A, TIFS, and PMMC firmware are all raw bins and we have
ti_k3_arm64_rproc.c / k3_system_controller.c / ti_power_proc.c
rproc drivers.

Andrew

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Kumar, Udit]

On 5/6/2025 11:42 PM, Andrew Davis wrote:
> On 5/6/25 12:19 PM, Tom Rini wrote:
>> On Tue, May 06, 2025 at 10:47:57PM +0530, Kumar, Udit wrote:
>>>
>>> On 5/6/2025 8:38 PM, Andrew Davis wrote:
>>>> On 5/6/25 9:51 AM, Beleswar Prasad Padhi wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> On 5/6/2025 4:38 PM, Andrew Davis wrote:
>>>>>> On 5/6/25 5:41 AM, Beleswar Padhi wrote:
>>>>>>> Some TI K3 SoCs like J721S2, and J784S4 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:
>>>>>>>
>>>>>>
>>>>>> The below flow looks exactly like the general purpose M4F cores..
>>>>>> Why is the HSM core treated differently and this loader not made 
>>>>>> into
>>>>>> a normal remote proc driver?
>>>>>
>>>>>
>>>>> Not exactly, HSM core is treated differently because of following
>>>>> exceptions:
>>>>> 1. Device operations for HSM core (like reset/reset release) are not
>>>>> handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.
>>>>
>>>> This could be added to the commit message then.
>>>>
>>>>> 2. The HSM firmware is not an ELF image. So we can't use rproc elf
>>>>> loader with it. Manual memcpy has to be done.
>>>>>
>>>>
>>>> Why is the HSM firmware not an ELF image?
>>>
>>>
>>> HSM firmwares are loaded by TIFS (in non-GP) flow, and TIFS does not 
>>> have
>>> elf parser.
>>
>> And to be clear, why is that? Since it's preventing the use other
>> existing workflows...
>>
>
> Even if the firmware is only delivered as a raw blob we should still
> be able to use existing workflows and make a proper rproc driver for
> this HSM core here.

I misunderstood your previous comment,

I thought you was asking to move to elf based flow.  which is not possible.


>
> TF-A, TIFS, and PMMC firmware are all raw bins and we have
> ti_k3_arm64_rproc.c / k3_system_controller.c / ti_power_proc.c
> rproc drivers.
>
another option to add support in ti_k3_m4_rproc.c driver for HSM


> Andrew

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Anshul Dalal]

On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
> and used by R5 SPL to boot the HSM core. By default, point to the
> firmware for HS-SE device type. This needs to be changed to point to
> appropriate firmware when using a different device type.
>
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---
> v2: Changelog:
> None to this patch.
>
> Link to v1:
> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>
>  arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>  arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
> index 73af184d27e..9c8b29f53bb 100644
> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
> @@ -273,6 +273,14 @@
>  
>  					};
>  				};
> +#ifdef CONFIG_K3_HSM_FW
> +				hsm {
> +					hsm: blob-ext {
> +						filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
> +					};
> +				};
> +#endif
> +

Why do we have the hsm binaries pre-signed? Having a common binary like
the DM with signing using ti-secure might be a better option.

Regards,

>  				dm {
>  					ti-secure {
>  						content = <&dm>;
> @@ -306,7 +314,11 @@
>  				conf-0 {
>  					description = "k3-j721s2-common-proc-board";
>  					firmware = "atf";
> +#ifdef CONFIG_K3_HSM_FW
> +					loadables = "hsm", "tee", "dm", "spl";
> +#else
>  					loadables = "tee", "dm", "spl";
> +#endif
>  					fdt = "fdt-0";
>  				};
>  			};
> diff --git a/arch/arm/dts/k3-j784s4-binman.dtsi b/arch/arm/dts/k3-j784s4-binman.dtsi
> index cb1fbc65923..7c8e580a8a3 100644
> --- a/arch/arm/dts/k3-j784s4-binman.dtsi
> +++ b/arch/arm/dts/k3-j784s4-binman.dtsi
> @@ -159,6 +159,16 @@
>  
>  		fit {
>  			images {
> +
> +#ifdef CONFIG_K3_HSM_FW
> +				hsm {
> +					hsm: blob-ext {
> +						filename = "ti-hsm/hsm-demo-firmware-j784s4-hs.bin";
> +					};
> +				};
> +
> +#endif
> +
>  				dm {
>  					ti-secure {
>  						content = <&dm>;
> @@ -194,7 +204,11 @@
>  				conf-0 {
>  					description = BOARD_DESCRIPTION;
>  					firmware = "atf";
> +#ifdef CONFIG_K3_HSM_FW
> +					loadables = "hsm", "tee", "dm", "spl";
> +#else
>  					loadables = "tee", "dm", "spl";
> +#endif
>  					fdt = "fdt-0";
>  				};
>  			};

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Beleswar Prasad Padhi]

Hi Andrew,

On 06/05/25 23:42, Andrew Davis wrote:
> On 5/6/25 12:19 PM, Tom Rini wrote:
>> On Tue, May 06, 2025 at 10:47:57PM +0530, Kumar, Udit wrote:
>>>
>>> On 5/6/2025 8:38 PM, Andrew Davis wrote:
>>>> On 5/6/25 9:51 AM, Beleswar Prasad Padhi wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> On 5/6/2025 4:38 PM, Andrew Davis wrote:
>>>>>> On 5/6/25 5:41 AM, Beleswar Padhi wrote:
>>>>>>> Some TI K3 SoCs like J721S2, and J784S4 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:
>>>>>>>
>>>>>>
>>>>>> The below flow looks exactly like the general purpose M4F cores..
>>>>>> Why is the HSM core treated differently and this loader not made into
>>>>>> a normal remote proc driver?
>>>>>
>>>>>
>>>>> Not exactly, HSM core is treated differently because of following
>>>>> exceptions:
>>>>> 1. Device operations for HSM core (like reset/reset release) are not
>>>>> handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.
>>>>
>>>> This could be added to the commit message then.
>>>>
>>>>> 2. The HSM firmware is not an ELF image. So we can't use rproc elf
>>>>> loader with it. Manual memcpy has to be done.
>>>>>
>>>>
>>>> Why is the HSM firmware not an ELF image?
>>>
>>>
>>> HSM firmwares are loaded by TIFS (in non-GP) flow, and TIFS does not have
>>> elf parser.
>>
>> And to be clear, why is that? Since it's preventing the use other
>> existing workflows...
>>
>
> Even if the firmware is only delivered as a raw blob we should still
> be able to use existing workflows and make a proper rproc driver for
> this HSM core here.


Sure, so do you wish to see a new HSM rproc driver rather than putting the code in R5 SPL Common file?

Thanks,
Beleswar

>
> TF-A, TIFS, and PMMC firmware are all raw bins and we have
> ti_k3_arm64_rproc.c / k3_system_controller.c / ti_power_proc.c
> rproc drivers.
>
> Andrew

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Beleswar Prasad Padhi]

On 5/7/2025 3:09 PM, Anshul Dalal wrote:
> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>> and used by R5 SPL to boot the HSM core. By default, point to the
>> firmware for HS-SE device type. This needs to be changed to point to
>> appropriate firmware when using a different device type.
>>
>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>> ---
>> v2: Changelog:
>> None to this patch.
>>
>> Link to v1:
>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>
>>   arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>   arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>> index 73af184d27e..9c8b29f53bb 100644
>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>> @@ -273,6 +273,14 @@
>>   
>>   					};
>>   				};
>> +#ifdef CONFIG_K3_HSM_FW
>> +				hsm {
>> +					hsm: blob-ext {
>> +						filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>> +					};
>> +				};
>> +#endif
>> +
> Why do we have the hsm binaries pre-signed? Having a common binary like
> the DM with signing using ti-secure might be a better option.


Andrew can correct me if I am wrong,
HSM is meant to run secure software stack and services like 
Authentication etc. It is a +1 to TIFS. To establish ROT, we need the 
HSM binary to be encrypted, and authenticated by TIFS first before it 
can do stuff by itself. DM is not a secure entity, so signing the image 
doesn't make sense for me.

>
> Regards,
>
>>   				dm {
>>   					ti-secure {
>>   						content = <&dm>;
>> @@ -306,7 +314,11 @@
>>   				conf-0 {
>>   					description = "k3-j721s2-common-proc-board";
>>   					firmware = "atf";
>> +#ifdef CONFIG_K3_HSM_FW
>> +					loadables = "hsm", "tee", "dm", "spl";
>> +#else
>>   					loadables = "tee", "dm", "spl";
>> +#endif
>>   					fdt = "fdt-0";
>>   				};
>>   			};
>> diff --git a/arch/arm/dts/k3-j784s4-binman.dtsi b/arch/arm/dts/k3-j784s4-binman.dtsi
>> index cb1fbc65923..7c8e580a8a3 100644
>> --- a/arch/arm/dts/k3-j784s4-binman.dtsi
>> +++ b/arch/arm/dts/k3-j784s4-binman.dtsi
>> @@ -159,6 +159,16 @@
>>   
>>   		fit {
>>   			images {
>> +
>> +#ifdef CONFIG_K3_HSM_FW
>> +				hsm {
>> +					hsm: blob-ext {
>> +						filename = "ti-hsm/hsm-demo-firmware-j784s4-hs.bin";
>> +					};
>> +				};
>> +
>> +#endif
>> +
>>   				dm {
>>   					ti-secure {
>>   						content = <&dm>;
>> @@ -194,7 +204,11 @@
>>   				conf-0 {
>>   					description = BOARD_DESCRIPTION;
>>   					firmware = "atf";
>> +#ifdef CONFIG_K3_HSM_FW
>> +					loadables = "hsm", "tee", "dm", "spl";
>> +#else
>>   					loadables = "tee", "dm", "spl";
>> +#endif
>>   					fdt = "fdt-0";
>>   				};
>>   			};

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Andrew Davis]

On 5/7/25 9:56 AM, Beleswar Prasad Padhi wrote:
> 
> On 5/7/2025 3:09 PM, Anshul Dalal wrote:
>> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>>> and used by R5 SPL to boot the HSM core. By default, point to the
>>> firmware for HS-SE device type. This needs to be changed to point to
>>> appropriate firmware when using a different device type.
>>>
>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>> ---
>>> v2: Changelog:
>>> None to this patch.
>>>
>>> Link to v1:
>>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>>
>>>   arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>>   arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>>   2 files changed, 26 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>>> index 73af184d27e..9c8b29f53bb 100644
>>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>>> @@ -273,6 +273,14 @@
>>>                       };
>>>                   };
>>> +#ifdef CONFIG_K3_HSM_FW
>>> +                hsm {
>>> +                    hsm: blob-ext {
>>> +                        filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>>> +                    };
>>> +                };
>>> +#endif
>>> +
>> Why do we have the hsm binaries pre-signed? Having a common binary like
>> the DM with signing using ti-secure might be a better option.
> 
> 
> Andrew can correct me if I am wrong,
> HSM is meant to run secure software stack and services like Authentication etc. It is a +1 to TIFS. To establish ROT, we need the HSM binary to be encrypted, and authenticated by TIFS first before it can do stuff by itself. DM is not a secure entity, so signing the image doesn't make sense for me.
> 

I think Anshul is not suggesting that the HSM binary be unencrypted/unauthenticated.
Rather that the encrypting/signing be done here in binman like we do with TF-A/OP-TEE.
(which both are part trusted images to be loaded by TIFS).

To that suggestion I agree, the customer will be doing the signing of this binary, right?
If so then since all other customer signing is done as part of binman, it makes sense
to also sign HSM firmware here too.

Andrew

>>
>> Regards,
>>
>>>                   dm {
>>>                       ti-secure {
>>>                           content = <&dm>;
>>> @@ -306,7 +314,11 @@
>>>                   conf-0 {
>>>                       description = "k3-j721s2-common-proc-board";
>>>                       firmware = "atf";
>>> +#ifdef CONFIG_K3_HSM_FW
>>> +                    loadables = "hsm", "tee", "dm", "spl";
>>> +#else
>>>                       loadables = "tee", "dm", "spl";
>>> +#endif
>>>                       fdt = "fdt-0";
>>>                   };
>>>               };
>>> diff --git a/arch/arm/dts/k3-j784s4-binman.dtsi b/arch/arm/dts/k3-j784s4-binman.dtsi
>>> index cb1fbc65923..7c8e580a8a3 100644
>>> --- a/arch/arm/dts/k3-j784s4-binman.dtsi
>>> +++ b/arch/arm/dts/k3-j784s4-binman.dtsi
>>> @@ -159,6 +159,16 @@
>>>           fit {
>>>               images {
>>> +
>>> +#ifdef CONFIG_K3_HSM_FW
>>> +                hsm {
>>> +                    hsm: blob-ext {
>>> +                        filename = "ti-hsm/hsm-demo-firmware-j784s4-hs.bin";
>>> +                    };
>>> +                };
>>> +
>>> +#endif
>>> +
>>>                   dm {
>>>                       ti-secure {
>>>                           content = <&dm>;
>>> @@ -194,7 +204,11 @@
>>>                   conf-0 {
>>>                       description = BOARD_DESCRIPTION;
>>>                       firmware = "atf";
>>> +#ifdef CONFIG_K3_HSM_FW
>>> +                    loadables = "hsm", "tee", "dm", "spl";
>>> +#else
>>>                       loadables = "tee", "dm", "spl";
>>> +#endif
>>>                       fdt = "fdt-0";
>>>                   };
>>>               };

Re: [PATCH v2 0/7] Add support to boot TI K3 HSM M4 core

[Andrew Davis]

On 5/7/25 5:28 AM, Beleswar Prasad Padhi wrote:
> Hi Andrew,
> 
> On 06/05/25 23:42, Andrew Davis wrote:
>> On 5/6/25 12:19 PM, Tom Rini wrote:
>>> On Tue, May 06, 2025 at 10:47:57PM +0530, Kumar, Udit wrote:
>>>>
>>>> On 5/6/2025 8:38 PM, Andrew Davis wrote:
>>>>> On 5/6/25 9:51 AM, Beleswar Prasad Padhi wrote:
>>>>>> Hi Andrew,
>>>>>>
>>>>>> On 5/6/2025 4:38 PM, Andrew Davis wrote:
>>>>>>> On 5/6/25 5:41 AM, Beleswar Padhi wrote:
>>>>>>>> Some TI K3 SoCs like J721S2, and J784S4 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:
>>>>>>>>
>>>>>>>
>>>>>>> The below flow looks exactly like the general purpose M4F cores..
>>>>>>> Why is the HSM core treated differently and this loader not made into
>>>>>>> a normal remote proc driver?
>>>>>>
>>>>>>
>>>>>> Not exactly, HSM core is treated differently because of following
>>>>>> exceptions:
>>>>>> 1. Device operations for HSM core (like reset/reset release) are not
>>>>>> handled by DM. TIFS handles that with proc_boot_ctrl TI-SCI calls.
>>>>>
>>>>> This could be added to the commit message then.
>>>>>
>>>>>> 2. The HSM firmware is not an ELF image. So we can't use rproc elf
>>>>>> loader with it. Manual memcpy has to be done.
>>>>>>
>>>>>
>>>>> Why is the HSM firmware not an ELF image?
>>>>
>>>>
>>>> HSM firmwares are loaded by TIFS (in non-GP) flow, and TIFS does not have
>>>> elf parser.
>>>
>>> And to be clear, why is that? Since it's preventing the use other
>>> existing workflows...
>>>
>>
>> Even if the firmware is only delivered as a raw blob we should still
>> be able to use existing workflows and make a proper rproc driver for
>> this HSM core here.
> 
> 
> Sure, so do you wish to see a new HSM rproc driver rather than putting the code in R5 SPL Common file?
> 

That does seem more clean, it will probably result in more code in total,
but it will be mostly in the rproc driver. And that is better than putting
all the rproc loading directly in the R5 SPL common file.

Andrew

> Thanks,
> Beleswar
> 
>>
>> TF-A, TIFS, and PMMC firmware are all raw bins and we have
>> ti_k3_arm64_rproc.c / k3_system_controller.c / ti_power_proc.c
>> rproc drivers.
>>
>> Andrew

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Anshul Dalal]

On Wed May 7, 2025 at 8:53 PM IST, Andrew Davis wrote:
> On 5/7/25 9:56 AM, Beleswar Prasad Padhi wrote:
>> 
>> On 5/7/2025 3:09 PM, Anshul Dalal wrote:
>>> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>>>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>>>> and used by R5 SPL to boot the HSM core. By default, point to the
>>>> firmware for HS-SE device type. This needs to be changed to point to
>>>> appropriate firmware when using a different device type.
>>>>
>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>>> ---
>>>> v2: Changelog:
>>>> None to this patch.
>>>>
>>>> Link to v1:
>>>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>>>
>>>>   arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>>>   arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>>>   2 files changed, 26 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>> index 73af184d27e..9c8b29f53bb 100644
>>>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>>>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>> @@ -273,6 +273,14 @@
>>>>                       };
>>>>                   };
>>>> +#ifdef CONFIG_K3_HSM_FW
>>>> +                hsm {
>>>> +                    hsm: blob-ext {
>>>> +                        filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>>>> +                    };
>>>> +                };
>>>> +#endif
>>>> +
>>> Why do we have the hsm binaries pre-signed? Having a common binary like
>>> the DM with signing using ti-secure might be a better option.
>> 
>> 
>> Andrew can correct me if I am wrong,
>> HSM is meant to run secure software stack and services like Authentication etc. It is a +1 to TIFS. To establish ROT, we need the HSM binary to be encrypted, and authenticated by TIFS first before it can do stuff by itself. DM is not a secure entity, so signing the image doesn't make sense for me.
>> 
>
> I think Anshul is not suggesting that the HSM binary be unencrypted/unauthenticated.
> Rather that the encrypting/signing be done here in binman like we do with TF-A/OP-TEE.
> (which both are part trusted images to be loaded by TIFS).
>
> To that suggestion I agree, the customer will be doing the signing of this binary, right?
> If so then since all other customer signing is done as part of binman, it makes sense
> to also sign HSM firmware here too.
>

Yeah, that is what I was going for. With that change it could be
possible to also have a single binary for all platforms (gp, hs, hs-fs)
in ti-linux-firmware?

Also, why are we not adding an unsigned variant of the hsm binary in
tispl.bin_unsigned?

[snip]

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Anshul Dalal]

On Wed May 7, 2025 at 8:53 PM IST, Andrew Davis wrote:
> On 5/7/25 9:56 AM, Beleswar Prasad Padhi wrote:
>> 
>> On 5/7/2025 3:09 PM, Anshul Dalal wrote:
>>> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>>>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>>>> and used by R5 SPL to boot the HSM core. By default, point to the
>>>> firmware for HS-SE device type. This needs to be changed to point to
>>>> appropriate firmware when using a different device type.
>>>>
>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>>> ---
>>>> v2: Changelog:
>>>> None to this patch.
>>>>
>>>> Link to v1:
>>>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>>>
>>>>   arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>>>   arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>>>   2 files changed, 26 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>> index 73af184d27e..9c8b29f53bb 100644
>>>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>>>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>> @@ -273,6 +273,14 @@
>>>>                       };
>>>>                   };
>>>> +#ifdef CONFIG_K3_HSM_FW
>>>> +                hsm {
>>>> +                    hsm: blob-ext {
>>>> +                        filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>>>> +                    };
>>>> +                };
>>>> +#endif
>>>> +
>>> Why do we have the hsm binaries pre-signed? Having a common binary like
>>> the DM with signing using ti-secure might be a better option.
>> 
>> 
>> Andrew can correct me if I am wrong,
>> HSM is meant to run secure software stack and services like Authentication etc. It is a +1 to TIFS. To establish ROT, we need the HSM binary to be encrypted, and authenticated by TIFS first before it can do stuff by itself. DM is not a secure entity, so signing the image doesn't make sense for me.
>> 
>
> I think Anshul is not suggesting that the HSM binary be unencrypted/unauthenticated.
> Rather that the encrypting/signing be done here in binman like we do with TF-A/OP-TEE.
> (which both are part trusted images to be loaded by TIFS).
>
> To that suggestion I agree, the customer will be doing the signing of this binary, right?
> If so then since all other customer signing is done as part of binman, it makes sense
> to also sign HSM firmware here too.
>
> Andrew

Yeah, that is what I was going for. With that change it could be
possible to also have a single binary for all platforms (gp, hs, hs-fs)
in ti-linux-firmware?

Also, why are we not adding an unsigned variant of the hsm binary in
tispl.bin_unsigned?

[snip]

Re: [PATCH v2 7/7] arm: mach-k3: r5: common: Add support to boot HSM M4 core

[Anshul Dalal]

On Tue May 6, 2025 at 4:12 PM IST, Beleswar Padhi wrote:
> The tispl.bin fit image is packed with the HSM firmware image. Populate
> the "os" info of the image so that it can be detected and used to load
> the HSM core. Further, invoke the load and boot of HSM core at R5 SPL
> stage. Boot flow for HSM M4 core 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.
>
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---
> v2: Changelog:
> 1. Hang system boot if HSM firmware failed to boot.
> 2. __maybe_unused to decrease preprocessor usage.
> 3. Better error messages with return code.
> 4. Added Error case in if-elseif-else ladder.
>
> Note:
> #define PROC_ID_HSM_M4F seems to have extra tab in the diff/patch.
> But when patch gets applied in file, all of them have consistent
> tabs.
>
> Link to v1:
> https://lore.kernel.org/all/18e01808-499d-4690-995a-45ac5fd727d9@ti.com
>
>  arch/arm/mach-k3/r5/common.c | 84 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
> index 81fc67a0023..1aa3869cec2 100644
> --- a/arch/arm/mach-k3/r5/common.c
> +++ b/arch/arm/mach-k3/r5/common.c
> @@ -15,9 +15,14 @@
>  #include <spl.h>
>  #include <remoteproc.h>
>  #include <elf.h>
> +#include <cpu_func.h>
>  
>  #include "../common.h"
>  
> +#define PROC_BOOT_CTRL_RESET_FLAG_HSM_M4	0x00000001
> +#define HSM_SRAM0_0_ADDR			0x43C00000
> +#define PROC_ID_HSM_M4F				0x00000080
> +
>  #if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
>  enum {
>  	IMAGE_ID_ATF,
> @@ -27,6 +32,7 @@ enum {
>  	IMAGE_ID_TIFSSTUB_HS,
>  	IMAGE_ID_TIFSSTUB_FS,
>  	IMAGE_ID_T,
> +	IMAGE_ID_HSM,
>  	IMAGE_AMT,
>  };
>  
> @@ -39,6 +45,7 @@ static const char *image_os_match[IMAGE_AMT] = {
>  	"tifsstub-hs",
>  	"tifsstub-fs",
>  	"tifsstub-gp",
> +	"hsm",
>  };
>  #endif
>  
> @@ -136,6 +143,73 @@ void release_resources_for_core_shutdown(void)
>  	}
>  }
>  
> +static int __maybe_unused boot_hsm_core(void)
> +{
> +	struct ti_sci_handle *ti_sci = get_ti_sci_handle();
> +	struct ti_sci_proc_ops *proc_ops = &ti_sci->ops.proc_ops;
> +	u64 hsm_image_addr;
> +	u32 hsm_image_size;
> +	int device_type, ret;
> +
> +	hsm_image_addr = (u64)fit_image_info[IMAGE_ID_HSM].load;
> +	hsm_image_size = (u32)fit_image_info[IMAGE_ID_HSM].image_len;
> +
> +	/* Request Control of Remote Processor */
> +	ret = proc_ops->proc_request(ti_sci, PROC_ID_HSM_M4F);
> +	if (ret) {
> +		printf("Unable to request HSM processor control: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Put the remote processor into reset */
> +	ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F,
> +					   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4, 0);
> +	if (ret) {
> +		printf("Unable to Halt HSM core: %d\n", ret);
> +		goto release_proc_ctrl;
> +	}
> +
> +	/*
> +	 * Load the HSM firmware into core's internal memory.
> +	 *
> +	 * In case of HS device types, request secure entity to authenticate and
> +	 * load the HSM firmware into the core memory.
> +	 * In case of GP device types, copy the HSM firmware into the core
> +	 * memory manually.
> +	 */
> +	device_type = get_device_type();
> +	if (device_type == K3_DEVICE_TYPE_HS_SE ||
> +	    device_type == K3_DEVICE_TYPE_HS_FS) {
> +		ret = proc_ops->proc_auth_boot_image(ti_sci, &hsm_image_addr,
> +						     &hsm_image_size);
> +		if (ret) {
> +			printf("Unable to Authenticate and Boot HSM image; ret = %d\n",
> +			       ret);
> +			goto release_proc_ctrl;
> +		}
> +	} else if (device_type == K3_DEVICE_TYPE_GP) {
> +		debug("Loading HSM GP binary into SRAM0_0\n");
> +		memcpy((void *)HSM_SRAM0_0_ADDR, (void *)(u32)hsm_image_addr,
> +		       hsm_image_size);
> +		flush_dcache_range(HSM_SRAM0_0_ADDR,
> +				   HSM_SRAM0_0_ADDR + hsm_image_size);
> +	} else {
> +		printf("Invalid Device Type\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Release the reset from the remote processor*/
> +	ret = proc_ops->set_proc_boot_ctrl(ti_sci, PROC_ID_HSM_M4F, 0,
> +					   PROC_BOOT_CTRL_RESET_FLAG_HSM_M4);
> +	if (ret)
> +		printf("Unable to Run HSM core: %d\n", ret);
> +
> +release_proc_ctrl:
> +	/* Release Control of Remote Processor */
> +	proc_ops->proc_release(ti_sci, PROC_ID_HSM_M4F);
> +	return ret;
> +}
> +
>  void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>  {
>  	typedef void __noreturn (*image_entry_noargs_t)(void);
> @@ -157,6 +231,14 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>  				     &loadaddr);
>  	}
>  
> +#if IS_ENABLED(CONFIG_K3_HSM_FW)
> +	ret = boot_hsm_core();
> +	if (ret)
> +		panic("HSM core failed to boot, %d\n", ret);
> +	else
> +		printf("Successfully booted HSM core\n");
> +#endif
> +
>  	/*
>  	 * It is assumed that remoteproc device 1 is the corresponding
>  	 * Cortex-A core which runs ATF. Make sure DT reflects the same.
> @@ -388,7 +470,7 @@ void board_fit_image_post_process(const void *fit, int node, void **p_image,
>  	 * Only DM and the DTBs are being authenticated here,
>  	 * rest will be authenticated when A72 cluster is up
>  	 */
> -	if ((i != IMAGE_ID_ATF) && (i != IMAGE_ID_OPTEE)) {
> +	if (i != IMAGE_ID_ATF && i != IMAGE_ID_OPTEE && i != IMAGE_ID_HSM) {
>  		ti_secure_image_check_binary(p_image, p_size);
>  		ti_secure_image_post_process(p_image, p_size);
>  	} else {

A minor nit but this and the following else block can be refactored as:

ti_secure_image_check_binary(p_image, p_size);
if (i != IMAGE_ID_ATF && i != IMAGE_ID_OPTEE && i != IMAGE_ID_HSM) {
	ti_secure_image_post_process(p_image, p_size);
}

Regards,
Anshul

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Beleswar Prasad Padhi]

On 5/8/2025 5:29 PM, Anshul Dalal wrote:
> On Wed May 7, 2025 at 8:53 PM IST, Andrew Davis wrote:
>> On 5/7/25 9:56 AM, Beleswar Prasad Padhi wrote:
>>> On 5/7/2025 3:09 PM, Anshul Dalal wrote:
>>>> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>>>>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>>>>> and used by R5 SPL to boot the HSM core. By default, point to the
>>>>> firmware for HS-SE device type. This needs to be changed to point to
>>>>> appropriate firmware when using a different device type.
>>>>>
>>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>>>> ---
>>>>> v2: Changelog:
>>>>> None to this patch.
>>>>>
>>>>> Link to v1:
>>>>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>>>>
>>>>>    arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>>>>    arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>>>>    2 files changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>> index 73af184d27e..9c8b29f53bb 100644
>>>>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>> @@ -273,6 +273,14 @@
>>>>>                        };
>>>>>                    };
>>>>> +#ifdef CONFIG_K3_HSM_FW
>>>>> +                hsm {
>>>>> +                    hsm: blob-ext {
>>>>> +                        filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>>>>> +                    };
>>>>> +                };
>>>>> +#endif
>>>>> +
>>>> Why do we have the hsm binaries pre-signed? Having a common binary like
>>>> the DM with signing using ti-secure might be a better option.
>>>
>>> Andrew can correct me if I am wrong,
>>> HSM is meant to run secure software stack and services like Authentication etc. It is a +1 to TIFS. To establish ROT, we need the HSM binary to be encrypted, and authenticated by TIFS first before it can do stuff by itself. DM is not a secure entity, so signing the image doesn't make sense for me.
>>>
>> I think Anshul is not suggesting that the HSM binary be unencrypted/unauthenticated.
>> Rather that the encrypting/signing be done here in binman like we do with TF-A/OP-TEE.
>> (which both are part trusted images to be loaded by TIFS).
>>
>> To that suggestion I agree, the customer will be doing the signing of this binary, right?
>> If so then since all other customer signing is done as part of binman, it makes sense
>> to also sign HSM firmware here too.
>>
>> Andrew
> Yeah, that is what I was going for. With that change it could be
> possible to also have a single binary for all platforms (gp, hs, hs-fs)
> in ti-linux-firmware?
>
> Also, why are we not adding an unsigned variant of the hsm binary in
> tispl.bin_unsigned?


What's the use case for that? I think we established that HSM won't be 
used unsigned. So it will just bloat the FIT and never be used.

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Andrew Davis]

On 5/8/25 10:03 AM, Beleswar Prasad Padhi wrote:
> 
> On 5/8/2025 5:29 PM, Anshul Dalal wrote:
>> On Wed May 7, 2025 at 8:53 PM IST, Andrew Davis wrote:
>>> On 5/7/25 9:56 AM, Beleswar Prasad Padhi wrote:
>>>> On 5/7/2025 3:09 PM, Anshul Dalal wrote:
>>>>> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>>>>>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>>>>>> and used by R5 SPL to boot the HSM core. By default, point to the
>>>>>> firmware for HS-SE device type. This needs to be changed to point to
>>>>>> appropriate firmware when using a different device type.
>>>>>>
>>>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>>>>> ---
>>>>>> v2: Changelog:
>>>>>> None to this patch.
>>>>>>
>>>>>> Link to v1:
>>>>>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>>>>>
>>>>>>    arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>>>>>    arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>>>>>    2 files changed, 26 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>>> index 73af184d27e..9c8b29f53bb 100644
>>>>>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>>> @@ -273,6 +273,14 @@
>>>>>>                        };
>>>>>>                    };
>>>>>> +#ifdef CONFIG_K3_HSM_FW
>>>>>> +                hsm {
>>>>>> +                    hsm: blob-ext {
>>>>>> +                        filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>>>>>> +                    };
>>>>>> +                };
>>>>>> +#endif
>>>>>> +
>>>>> Why do we have the hsm binaries pre-signed? Having a common binary like
>>>>> the DM with signing using ti-secure might be a better option.
>>>>
>>>> Andrew can correct me if I am wrong,
>>>> HSM is meant to run secure software stack and services like Authentication etc. It is a +1 to TIFS. To establish ROT, we need the HSM binary to be encrypted, and authenticated by TIFS first before it can do stuff by itself. DM is not a secure entity, so signing the image doesn't make sense for me.
>>>>
>>> I think Anshul is not suggesting that the HSM binary be unencrypted/unauthenticated.
>>> Rather that the encrypting/signing be done here in binman like we do with TF-A/OP-TEE.
>>> (which both are part trusted images to be loaded by TIFS).
>>>
>>> To that suggestion I agree, the customer will be doing the signing of this binary, right?
>>> If so then since all other customer signing is done as part of binman, it makes sense
>>> to also sign HSM firmware here too.
>>>
>>> Andrew
>> Yeah, that is what I was going for. With that change it could be
>> possible to also have a single binary for all platforms (gp, hs, hs-fs)
>> in ti-linux-firmware?
>>
>> Also, why are we not adding an unsigned variant of the hsm binary in
>> tispl.bin_unsigned?
> 
> 
> What's the use case for that? I think we established that HSM won't be used unsigned. So it will just bloat the FIT and never be used.
> 

We ship an unsigned HSM firmware for GP devices[0], and in patch [7/7] of this
series you add support for loading that unsigned HSM firmware. Seems odd to
then not ever package it here in binman and claim we will never use it?

Andrew

[0] https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/tree/ti-hsm?h=ti-linux-firmware

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Beleswar Prasad Padhi]

Hi Andrew,

On 08/05/25 20:58, Andrew Davis wrote:
> On 5/8/25 10:03 AM, Beleswar Prasad Padhi wrote:
>>
>> On 5/8/2025 5:29 PM, Anshul Dalal wrote:
>>> On Wed May 7, 2025 at 8:53 PM IST, Andrew Davis wrote:
>>>> On 5/7/25 9:56 AM, Beleswar Prasad Padhi wrote:
>>>>> On 5/7/2025 3:09 PM, Anshul Dalal wrote:
>>>>>> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>>>>>>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>>>>>>> and used by R5 SPL to boot the HSM core. By default, point to the
>>>>>>> firmware for HS-SE device type. This needs to be changed to point to
>>>>>>> appropriate firmware when using a different device type.
>>>>>>>
>>>>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>>>>>> ---
>>>>>>> v2: Changelog:
>>>>>>> None to this patch.
>>>>>>>
>>>>>>> Link to v1:
>>>>>>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>>>>>>
>>>>>>>    arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>>>>>>    arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>>>>>>    2 files changed, 26 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>>>> index 73af184d27e..9c8b29f53bb 100644
>>>>>>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>>>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>>>> @@ -273,6 +273,14 @@
>>>>>>>                        };
>>>>>>>                    };
>>>>>>> +#ifdef CONFIG_K3_HSM_FW
>>>>>>> +                hsm {
>>>>>>> +                    hsm: blob-ext {
>>>>>>> +                        filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>>>>>>> +                    };
>>>>>>> +                };
>>>>>>> +#endif
>>>>>>> +
>>>>>> Why do we have the hsm binaries pre-signed? Having a common binary like
>>>>>> the DM with signing using ti-secure might be a better option.
>>>>>
>>>>> Andrew can correct me if I am wrong,
>>>>> HSM is meant to run secure software stack and services like Authentication etc. It is a +1 to TIFS. To establish ROT, we need the HSM binary to be encrypted, and authenticated by TIFS first before it can do stuff by itself. DM is not a secure entity, so signing the image doesn't make sense for me.
>>>>>
>>>> I think Anshul is not suggesting that the HSM binary be unencrypted/unauthenticated.
>>>> Rather that the encrypting/signing be done here in binman like we do with TF-A/OP-TEE.
>>>> (which both are part trusted images to be loaded by TIFS).
>>>>
>>>> To that suggestion I agree, the customer will be doing the signing of this binary, right?
>>>> If so then since all other customer signing is done as part of binman, it makes sense
>>>> to also sign HSM firmware here too.
>>>>
>>>> Andrew
>>> Yeah, that is what I was going for. With that change it could be
>>> possible to also have a single binary for all platforms (gp, hs, hs-fs)
>>> in ti-linux-firmware?
>>>
>>> Also, why are we not adding an unsigned variant of the hsm binary in
>>> tispl.bin_unsigned?
>>
>>
>> What's the use case for that? I think we established that HSM won't be used unsigned. So it will just bloat the FIT and never be used.
>>
>
> We ship an unsigned HSM firmware for GP devices[0], and in patch [7/7] of this
> series you add support for loading that unsigned HSM firmware. 


That is a System Firmware limitation; i.e., no support of loading HSM FW for GP devices. We are providing a workaround for that by manually loading it from MCU R5 core. Besides, HSM SRAM should not even be accessible by MCU R5F core in the first place. HSM should only be accessible & loaded by TIFS. So once the SRAM is firewalled, this flow for loading GP bins won't work, and SYSFW will somehow have to support that, or we just give up unsigned bin load support.

> Seems odd to
> then not ever package it here in binman and claim we will never use it?


IMHO, users can "choose" to package that unsigned HSM bin and boot it in the current flow. Please let me know, if you are okay with packaging an unsigned HSM binary "by default", given the above caveat?

Thanks,
Beleswar

>
> Andrew
>
> [0] https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/tree/ti-hsm?h=ti-linux-firmware

Re: [PATCH v2 4/7] arm: dts: k3-{j721s2/j784s4}-binman: Pack HSM firmware inside tispl.bin

[Beleswar Prasad Padhi]

Hi Andrew, Anshul, all

Reviving this old thread, this is ready for a re-spin
now as unsigned HSM firmware is available.
Please see below.

On 08/05/25 17:29, Anshul Dalal wrote:
> On Wed May 7, 2025 at 8:53 PM IST, Andrew Davis wrote:
>> On 5/7/25 9:56 AM, Beleswar Prasad Padhi wrote:
>>> On 5/7/2025 3:09 PM, Anshul Dalal wrote:
>>>> On Tue May 6, 2025 at 4:11 PM IST, Beleswar Padhi wrote:
>>>>> Pack the HSM firmware in tispl.bin fit image so that it can be unloaded
>>>>> and used by R5 SPL to boot the HSM core. By default, point to the
>>>>> firmware for HS-SE device type. This needs to be changed to point to
>>>>> appropriate firmware when using a different device type.
>>>>>
>>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>>>> ---
>>>>> v2: Changelog:
>>>>> None to this patch.
>>>>>
>>>>> Link to v1:
>>>>> https://lore.kernel.org/all/20250422095430.363792-4-b-padhi@ti.com/
>>>>>
>>>>>   arch/arm/dts/k3-j721s2-binman.dtsi | 12 ++++++++++++
>>>>>   arch/arm/dts/k3-j784s4-binman.dtsi | 14 ++++++++++++++
>>>>>   2 files changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>> index 73af184d27e..9c8b29f53bb 100644
>>>>> --- a/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>> +++ b/arch/arm/dts/k3-j721s2-binman.dtsi
>>>>> @@ -273,6 +273,14 @@
>>>>>                       };
>>>>>                   };
>>>>> +#ifdef CONFIG_K3_HSM_FW
>>>>> +                hsm {
>>>>> +                    hsm: blob-ext {
>>>>> +                        filename = "ti-hsm/hsm-demo-firmware-j721s2-hs.bin";
>>>>> +                    };
>>>>> +                };
>>>>> +#endif
>>>>> +
>>>> Why do we have the hsm binaries pre-signed? Having a common binary like
>>>> the DM with signing using ti-secure might be a better option.
>>>
>>> Andrew can correct me if I am wrong,
>>> HSM is meant to run secure software stack and services like Authentication etc. It is a +1 to TIFS. To establish ROT, we need the HSM binary to be encrypted, and authenticated by TIFS first before it can do stuff by itself. DM is not a secure entity, so signing the image doesn't make sense for me.
>>>
>> I think Anshul is not suggesting that the HSM binary be unencrypted/unauthenticated.
>> Rather that the encrypting/signing be done here in binman like we do with TF-A/OP-TEE.
>> (which both are part trusted images to be loaded by TIFS).
>>
>> To that suggestion I agree, the customer will be doing the signing of this binary, right?
>> If so then since all other customer signing is done as part of binman, it makes sense
>> to also sign HSM firmware here too.
>>
>> Andrew
> Yeah, that is what I was going for. With that change it could be
> possible to also have a single binary for all platforms (gp, hs, hs-fs)
> in ti-linux-firmware?


I don't think its possible to have a single HSM firmware
for all device types currently. GP devices might not use
all security IPs which the hs-se device uses. So having
a common firmware would mean we sacrifice using
those secure services on a HS-SE capable device
as well. Of course, whenever we can support a single
TIFS binary for all device types, we can support a single
HSM too. I have copied firmware folks here (@Tanu,
@Saurabh) who can help in answering this query better,
if any follow up.

But yes, we do have the unsigned HSM firmware for each
device type now[0]: GP, HS, HS-FS. Which means we can
add the signing support here in U-Boot to match the flow
with other components like ATF/OPTEE/DM etc.

So, I will re-spin the v3 for this patch series with following
changes:
1. Dedicated remoteproc driver for booting HSM M4 core.
2. Signing HSM binary in binman/U-Boot.
3. Other minor code comments from Anshul/Andrew.

Note: We will continue to pack the HS-SE variant of HSM
firmware in tispl.bin fit image by default. Any other
variant can be packaged by changing the path in binman
node to corresponding firmware blob.

Let me know, if you have any comments over the above,
otherwise I will send out an v3 soon. Thanks!

[0]: https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/commit/ti-hsm?h=ti-linux-firmware&id=560c226d763018de7adb892fc215b31286cc2831

Thanks,
Beleswar