Re: [PATCH v2 7/8] board: ti: j721e: During resume spl restores TF-A and DM-Firmware

[Andrew Davis <afd@ti.com>]

On 11/9/23 5:29 AM, Thomas Richard wrote:
> On 11/8/23 18:30, Andrew Davis wrote:
>>>    void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>>>    {
>>>        typedef void __noreturn (*image_entry_noargs_t)(void);
>>> @@ -235,6 +241,32 @@ void __noreturn jump_to_image_no_args(struct
>>> spl_image_info *spl_image)
>>>        if (ret)
>>>            panic("rproc failed to be initialized (%d)\n", ret);
>>>    +    if (board_is_resuming()) {
>>> +#if IS_ENABLED(CONFIG_SOC_K3_J721E)
>>> +        if (!valid_elf_image(LPM_DM_SAVE))
>>> +            panic("%s: DM-Firmware image is not valid, it cannot be
>>> loaded\n",
>>> +                  __func__);
>>> +
>>> +        loadaddr = load_elf_image_phdr(LPM_DM_SAVE);
>>> +
>>> +        /*
>>> +         * Check if the start address of TF-A is in DRAM.
>>> +         * If not it means TF-A was running in SRAM, so it shall be
>>> +         * restored.
>>> +         */
>>> +        if (*(ulong *)(LPM_BL31_START_SAVE) < CFG_SYS_SDRAM_BASE)
>>> +            memcpy((void *)*(uintptr_t *)(LPM_BL31_START_SAVE),
>>> +                   (void *)LPM_BL31_SAVE, BL31_SIZE);
>>
>> This will not work. The memory where TF-A is running will be firewalled and
>> SPL absolutely cannot be securely trusted to load TF-A. Especially from an
>> unencrypted location in DDR. TF-A must be loaded as it is today using
>> signed
>> certificate images. You should know this, I explained it all when you tried
>> the same in TF-A:
>>
>> https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/23992
> 
> Hi Andrew,
> 
> We understood that GP devices are not impacted (we had this information
> from TI, probably Manorit I don't remember), and Manorit confirmed it in
> the TF-A review.
> 
> Maybe I could add a check of the device type to not impact HS devices.
> 

I'm not interested in GP devices, and neither are most our customers.
Those are development devices, customers go to production with secured
devices.

Saying "let's make it work on GP only, then we will figure it out on HS
later" was a mistake we made back in OMAP class device days. It made
bringing support to production secured devices (HS) miserable as we had
to unroll all the hacks that only worked on the development devices (GP).

Your method here is completely unusable on HS and will need a ground up
rewrite for HS. Since the solution for HS will also work on GP, but
not the other way around, you need to start with the HS solution.

I'll make this same point over on the TF-A review then let's continue
discussion over there only. If you cant get the TF-A part in then no
need for this U-Boot part.

Andrew

> Regards,
> 
> Thomas
> 
>>
>> NAK
>>
>> Andrew
>>
>