Re: [PATCH 3/3] firmware: add external firmware PBL support

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 16.08.22 10:42, Sascha Hauer wrote:
> On Mon, Aug 15, 2022 at 03:09:55PM +0200, Ahmad Fatoum wrote:
>> Normally, barebox embds firmware into the binary referencing it, which
>> means that device tree blobs, RAM training code and e.g. TF-A for i.MX8M
>> end up in the prebootloader, while, e.g. Freescale FMan microcode ends
>> up in barebox proper. The only exception so far was barebox proper:
>> When only the PBL fits in on-chip SRAM, barebox proper is chainloaded
>> from the boot medium. To avoid TOCTOU attack, it's read fully into DRAM
>> after setup and then a SHA256 is calculated and compared against the
>> hash embedded in barebox PBL, which in a secure boot system would be
>> trusted by virtue of the PBL as a whole being verified beforehand by
>> the BootROM.
>>
>> Reuse this mechanism to support arbitrary firmware, which is now termed
>> external firmware. Such firmware is placed beyond the piggydata (barebox
>> proper) and only offset and hash are included in the prebootloader
>> image. The new get_builtin_firmware_ext() is used to retrieve this
>> external firmware after integrity verification with SHA256.
> 
> Does it make sense to use this mechanism for barebox proper as well?

I'd rather we leave this as future exercise..

> 
>>  #define get_builtin_firmware(name, start, size) \
>>  	{							\
>>  		extern char _fw_##name##_start[];		\
>> @@ -65,4 +78,22 @@ void firmwaremgr_list_handlers(void);
>>  		*size = _fw_##name##_end - _fw_##name##_start;	\
>>  	}
>>  
>> +#define get_builtin_firmware_ext(name, base, start, size)		\
> 
> base is unused.

Ouch. It's called bl33 in the macro. Will fix.

> It would be nice if users could use the same macro for both internal and
> external firmware, but I have no idea how and if this could be
> implemented.

We can unconditionally emit _fw_*_sha_{start,end} symbols.
If they happen to be equal, we skip the SHA verification.
The problem however is that we need to know the offset
between the currently running binary and the chainloaded one
to arrive at the address of external firmware. We can use 0 for
that (i.e. base = _text) to have internal firmware a special case
of external firmware, but for external firmware, we must specify
explicit an offset. So we can't avoid here.

Cheers,
Ahmad

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |