Hi Andrew,
On 24/04/25 21:25, Andrew Davis wrote:
> On 4/24/25 12:21 AM, Beleswar Prasad Padhi wrote:
>> Hi Andrew,
>>
>> On 22/04/25 20:04, Andrew Davis wrote:
>>> On 4/22/25 4:54 AM, Beleswar Padhi wrote:
>>>> 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. The size of
>>>> HSM firmware is padded upto 256 KB. The HSM binaries are unloaded at a
>>>> temporary DDR address.
>>>>
>>>> Signed-off-by: Beleswar Padhi <b-pa...@ti.com>
>>>> ---
>>>> arch/arm/dts/k3-binman.dtsi | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
>>>> index 5163161b94d..c2b8388b06e 100644
>>>> --- a/arch/arm/dts/k3-binman.dtsi
>>>> +++ b/arch/arm/dts/k3-binman.dtsi
>>>> @@ -297,6 +297,17 @@
>>>> };
>>>> };
>>>> +#ifdef CONFIG_K3_HSM_FW
>>>> + hsm {
>>>> + description = "HSM binary";
>>>> + type = "standalone";
>>>> + compression = "none";
>>>> + os = "hsm";
>>>> + load = <0x82000000>;
>>>> + entry = <0x82000000>;
>>>
>>> These addresses might already be in use. Why do you need an `entry`
>>> address here?
>>
>>
>> SPL parses the FIT image and assigns the `image_start` based on this `entry`
>> address.
>>
>> https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-k3/r5/common.c?ref_type=heads#L317
>> https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-k3/r5/common.c?ref_type=heads#L165
>>
>
> And that `image_start` only matters for images we will be starting, what
> does it do in this case? HSM is a remote core, and the actual start address
> is not 0x82000000, that is the load address, it does not run from there.
>
> The only place `image_start` for HSM is used in is in code you add in
> patch [4/4], and you use it to populate `hsm_image_addr`. That is
> the load address, not the start address.
You are correct. Thanks, will remove this in revision.
>
>>
>>> Or a `load` address?, if one isn't given then U-Boot
>>> should pick a safe spot for you, or even do the operation in-place.
>>
>>
>> Looks like that's not the case. When I skipped `load` section, U-Boot
>> complains with following error:
>>
>
> This works for other images, maybe it is not functional for SPL I'm not
> sure, if not that is a bug and should be fixed.
Hmm, it looks like this is supported for other FIT images:
https://source.denx.de/u-boot/u-boot/-/blob/master/boot/image-fit.c?ref_type=heads#L2266
>
> Looking over the code, I see no reason you need to copy it to another
> location before working on the image. It is an extra copy before it is
> copied by rproc to the real final location.
Well you are right. I will take a stab at adding this optional loadaddr support
in SPL images.
Thanks,
Beleswar
>
> Andrew
>
>
>> ```
>> U-Boot SPL 2025.04-01182-gaa96051c7f78-dirty (Apr 24 2025 - 10:42:17 +0530)
>> SYSFW ABI: 4.0 (firmware rev 0x000b '11.0.9--v11.00.09+ (Fancy Rat)')
>> Initialized 4 DRAM controllers
>> SPL initial stack usage: 13456 bytes
>> Trying to boot from MMC2
>> Skipping authentication on GP device
>> Can't load hsm: No load address and no buffer
>> spl_load_simple_fit: can't load image loadables index 0 (ret = -105)
>> spl_load_image_fat: error reading image tispl.bin, err - -105
>> spl_load_image_ext: ext4fs mount err - 0
>> Error: -2
>> SPL: failed to boot from all boot devices
>> ### ERROR ### Please RESET the board ###
>> ```
>>
>> Thanks,
>> Beleswar
>>
>>>
>>> Andrew
>>>
>>>> + };
>>>> +#endif
>>>> +
>>>> dm {
>>>> description = "DM binary";
>>>> type = "firmware";
