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

[Kumar, Udit]

On 4/24/2025 3:17 PM, Beleswar Prasad Padhi wrote:
> Hi Udit,
>
> On 22/04/25 20:14, Kumar, Udit wrote:
>> Hello Beleswar
>>
>> On 4/22/2025 3:24 PM, 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 loaded at
>>> corresponding DDR address. 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 at 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 fd188e7c90e..1af0f70d8d3 100644
>>> --- a/arch/arm/mach-k3/r5/common.c
>>> +++ b/arch/arm/mach-k3/r5/common.c
>>> @@ -15,11 +15,17 @@
>>>    #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
>> PROC_ID_HSM_M4F seems to have extra tab
>
> Will fix in revision.
>
>>> +
>>>    #if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
>>>    enum {
>>> +    IMAGE_ID_HSM,
>>>        IMAGE_ID_ATF,
>>>        IMAGE_ID_OPTEE,
>>>        IMAGE_ID_SPL,
>> Do you see possibility to have HSM char and enum at last .
>
> Thanks for the help in debug. A separate fix needs to be sent first, before putting new firmware at the end of the enum. I will include the fix in revision.
>
>>
>>> @@ -32,6 +38,7 @@ enum {
>>>      #if CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS)
>>>    static const char *image_os_match[IMAGE_AMT] = {
>>> +    "hsm",
>>>        "arm-trusted-firmware",
>>>        "tee",
>>>        "U-Boot",
>>> @@ -136,6 +143,73 @@ void release_resources_for_core_shutdown(void)
>>>        }
>>>    }
>>>    +#if IS_ENABLED(CONFIG_K3_HSM_FW)
>>> +static int 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;
>> ^^ may be in alphabetic order
>
> Other functions in & around the same file define structs before than int and char.
>
> Let me know if you have a preference for the other way around.

No preference but just suggestion, if you want to take it


>
>>
>>> +
>>> +    hsm_image_addr = (u64)fit_image_info[IMAGE_ID_HSM].image_start;
>>> +    hsm_image_size = (u32)fit_image_info[IMAGE_ID_HSM].image_len;
>>> +
>>> +   [..]
>>>
>>> Sure, will address in revision.
>>>
>>
>>> +
>>> +  [..]Should we go in hang state, if secure fw is not started correctly ?
>
> We can. But I am thinking of the cases when people are running some demo applications in HSM core. In that case, if HSM boot fails, it is kind of expensive to hang the entire system boot.
>
> What do you think?

This is not default option. people wants to use HSM fw they need to 
purposefully enable this option

which shall work, So hang is justified.


>
> Thanks,
> Beleswar
>
>>
>>> +
>>>        /*
>>>         * It is assumed that remoteproc device 1 is the corresponding
>>>         * Cortex-A core which runs ATF. Make sure DT reflects the same.
>>> @@ -342,7 +424,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 {