[RISC-V] [tech-unixplatformspec] [RFC 1/1] fw_base: header for vendor information

[Heinrich Schuchardt]

On 9/3/21 9:09 AM, Anup Patel wrote:
> On Tue, Aug 31, 2021 at 10:39 PM Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>> To implement secure boot OpenSBI should provide a well defined header
>> structure with reserved space in which a vendor can place information
>> related to a signature that the boot ROM code can check.
> 
> I totally agree.
> 
>>
>> Typically this space will be used to add
>>
>> * vendor magic
>> * type of signature
>> * offset to signature
> 
> Better to include few other stuff like size of image including the header.
> 
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>> Ideally such a header would be standardized. This would allow alternative
>> firmware to be validated by the same boot ROM.
>>
>> Could this be a topic for the platform specification?
> 
> The platform specification only points to SBI specification and sets
> expectations around SBI extensions available to the S-mode software.
> 
> A standard OpenSBI image header format is more of an OpenSBI
> specific topic.
> 
> I suggest the following:
> 1) We define the OpenSBI image header format in docs/firmware/fw.md
> 2) Update fw_base.S (like this patch) to use the OpenSBI image header
> 3) Provide a reference C header (i.e. include/sbi/fw_image.h)  for the
>      OpenSBI image format.
> 4) Provide scripts/tools for vendors to embed vendor specific data in
>      the OpenSBI image header
> 
> Vendors can certainly re-use OpenSBI image header format (and tools)
> for their vendor-specific early booting stages as well.
> 
> If OpenSBI image header format becomes widely used then we can
> even create a dedicated RISC-V project to host this as well.

The question of file headers goes beyond SBI.

Kumar Sankara suggested that the topic of file headers for secure boot 
should be taken to the TEE TG.

> 
>>
>> Best regards
>>
>> Heinrich
>> ---
>>   firmware/fw_base.S | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
>> index 1569e60..e2cc5cb 100644
>> --- a/firmware/fw_base.S
>> +++ b/firmware/fw_base.S
>> @@ -13,6 +13,7 @@
>>   #include <sbi/sbi_platform.h>
>>   #include <sbi/sbi_scratch.h>
>>   #include <sbi/sbi_trap.h>
>> +#include <sbi/sbi_version.h>
>>
>>   #define BOOT_STATUS_RELOCATE_DONE      1
>>   #define BOOT_STATUS_BOOT_HART_DONE     2
>> @@ -47,6 +48,15 @@
>>          .globl _start
>>          .globl _start_warm
>>   _start:
>> +       j       _real_start
>> +       .align  2
>> +       .ascii  "OpenSBI\0"
>> +       .short  OPENSBI_VERSION_MAJOR
>> +       .short  OPENSBI_VERSION_MINOR
> 
> I think we should also have:
> 1) Firmware size in bytes
> 2) 32bit type field (where type == 0 means no vendor data)
> 
>> +       /* Space reserved for vendor information, e.g. offset to a signature */
>> +       .long   0
>> +       .skip   12
> 
> Is 12 longs sufficient ??

For adding a pointer to a table placed behind the binary it would be 
enough. But we should first define requirements in the TEE TG.

Best regards

Heinrich

> 
>> +_real_start:
>>          /* Find preferred boot HART id */
>>          MOV_3R  s0, a0, s1, a1, s2, a2
>>          call    fw_boot_hart
>> --
>> 2.30.2
>>
>>
>>
>> ------------
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#1291): https://lists.riscv.org/g/tech-unixplatformspec/message/1291
>> Mute This Topic: https://lists.riscv.org/mt/85281393/6366717
>> Group Owner: tech-unixplatformspec+owner at lists.riscv.org
>> Unsubscribe: https://lists.riscv.org/g/tech-unixplatformspec/unsub [anup at brainfault.org]
>> ------------
>>
>>
> 
> Regards,
> Anup
>