On Tue, Sep 21, 2021 at 03:40:20PM +0000, Peter Stuge wrote:
Alec Brown wrote:
Below is how the layout of these logs would store their data.
bf_log_header: +-------------------+ u32 | version | u32 | size | u8[64] | producer | u8[64] | log_format | u64 | flags | u64 | next_bflh_addr | u64 | log_addr | u32 | log_size | +-------------------+
I suggest to include a .magic at least in bf_log_header and an .xor_checksum or .crc32 only in bf_log_header.
.magic doubles as endianess indicator when the structures are stored on movable media. (Pick an asymmetric magic bit pattern!)
This is something we will need to think about.
I suggest renaming .next_bflh_addr to .next_log_header and .log_addr to .log_buffer_addr.
I suggest to remove .size and .log_size:
The rationale for .size is "to allow for backward compatibility" but that seems redundant thanks to .version.
.log_size can be calculated from the subordinate data and is thus mostly an unneccessary source of potential inconsistency.
Looking back, I agree with removing .size since .version accomplishes the same task. I'm not so sure on removing .log_size as it can be very convenient, and .log_size won't need to be calculated every time someone wants to know the size of the logs generated from the boot component.
bf_log_buffer: +-------------------+ u32 | version | u32 | size | u8[64] | producer | u32 | next_msg_off | bf_log_msg[l] | msgs | +-------------------+
I suggest replacing .size and .next_msg_off with .messages containing l:
.size can then be calculated from .messages; again, reliably avoiding inconsistency between .size and .next_msg_off.
Allocated size doesn't seem useful if writers must anyway maintain state containing the starting address. If writers must be allowed to be completely stateless then maybe at least rename .size to .allocated_size and see below for discovery.
Having .messages also eliminates the need for an end-of-messages marker when the allocated space is not yet filled.
After some thinking, it makes sense to replace the meaning of .size with the meaning of .next_msg_off and removing .next_msg_off from the structure. This wouldn't be useful to store for the readers of the log.
bf_log_msg: +-------------------+ u32 | size | u64 | ts_nsec | u32 | level | u32 | facility | u32 | msg_off | u8[n] | type | u8[m] | msg | +-------------------+
It seems inconsistent that log_header.size and log_msg.size cover only the respective struct itself while log_buffer.size also covers all subordinate messages. Skipping all .size in this version fixes that.
And log_msg.size is not very useful since both .type and .msg have variable length; it's not possible to access .msg without scanning .type. Please at a minimum add .type_size but better yet replace .size with .type_size and .msg_size.
You bring up some good points about the names for the fields and that they need to be more consistent. By removing .size in bf_log_header, this should make it more consistent.
There is still the outstanding issue of how the logs will be sent to the OS. If UEFI is used, we can use config tables. If ACPI or Device Tree is used, we can use bf_log_header.next_bflh_addr to present the logs. If none of these platforms are used, it becomes a lot trickier to solve this issue.
Any suggestions are much appreciated and will be taken into consideration.
Having bf_log_header.magic and some bf_log_header.$checksum, a strict rule for bf_log_header start address granularity and a strict maximum offset for the first header from top and/or bottom of memory allows to quickly discover a log in memory without explicit handover.
This is something we'll have to think about some more. We aren't convinced about using .magic for log discovery and are looking for a more explicit way of doing this.
LPC System Boot and Security Micro-conference on the 22nd of September at 7:50 AM PDT (14:50 UTC).
Have fun! :)
Heinrich Schuchardt wrote:
We already the EFI_TCG2_PROTOCOL and RFC 5424 (The syslog protocol). Why do we need to start from scratch?
That's a good question. I guess noone wants to settle for a standard from somewhere else. ;)
I wouldn't mind if log_msg was a syslog transport, but I can understand if that's rejected because syslog messages require a lot of parsing for presentation while Alec's proposal seems focused on efficiency and simplicity.
It's also nice to be able to strictly mandate UTF-8 for all fields. (RFC 5424 allows MSG to be anything.)
Kind regards
//Peter