From 9a9b2778a1f04ba5d570de154f4b6f38f3b5807a Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Wed, 30 Nov 2022 16:18:01 -0800 Subject: coreboot_tables: Make existing alignment conventions more explicit There seem to be some recurring vague concerns about the alignment of coreboot table entries. While the existing implementation has been producing tables with a well-defined alignment (4 bytes) for a long time, the code doesn't always make it very clear. This patch adds an explicit constant to codify that alignment, assertions to check it after each entry, and adds explicit padding to the few entry structures that were relying on compiler padding to return a correct sizeof() value. Signed-off-by: Julius Werner Change-Id: Iaeef29ef255047a855066469e03b5481812e5975 Reviewed-on: https://review.coreboot.org/c/coreboot/+/70158 Reviewed-by: Arthur Heymans Reviewed-by: Jakub Czapiga Tested-by: build bot (Jenkins) Reviewed-by: Peter Stuge --- src/commonlib/include/commonlib/coreboot_tables.h | 8 +++++++- src/lib/coreboot_table.c | 14 +++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/commonlib/include/commonlib/coreboot_tables.h b/src/commonlib/include/commonlib/coreboot_tables.h index 6b5eb59cf8..76dce0a41b 100644 --- a/src/commonlib/include/commonlib/coreboot_tables.h +++ b/src/commonlib/include/commonlib/coreboot_tables.h @@ -95,6 +95,9 @@ enum { LB_TAG_OPTION_CHECKSUM = 0x00cc, }; +/* All table entry base addresses and sizes must be 4-byte aligned. */ +#define LB_ENTRY_ALIGN 4 + /* Since coreboot is usually compiled 32bit, gcc will align 64bit * types to 32bit boundaries. If the coreboot table is dumped on a * 64bit system, a uint64_t would be aligned to 64bit boundaries, @@ -104,7 +107,7 @@ enum { * to ensure compatibility. */ -typedef __aligned(4) uint64_t lb_uint64_t; +typedef __aligned(LB_ENTRY_ALIGN) uint64_t lb_uint64_t; struct lb_header { uint8_t signature[4]; /* LBIO */ @@ -203,6 +206,7 @@ struct lb_console { uint32_t tag; uint32_t size; uint16_t type; + uint8_t pad[2]; }; #define LB_TAG_CONSOLE_SERIAL8250 0 @@ -288,6 +292,7 @@ struct lb_framebuffer { uint8_t reserved_mask_pos; uint8_t reserved_mask_size; uint8_t orientation; + uint8_t pad[2]; }; struct lb_gpio { @@ -553,6 +558,7 @@ struct lb_tpm_physical_presence { uint32_t ppi_address; /* Address of ACPI PPI communication buffer */ uint8_t tpm_version; /* 1: TPM1.2, 2: TPM2.0 */ uint8_t ppi_version; /* BCD encoded */ + uint8_t pad[2]; }; diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 0f20735204..eaf487ff4e 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -76,9 +76,11 @@ struct lb_record *lb_new_record(struct lb_header *header) { struct lb_record *rec; rec = lb_last_record(header); - if (header->table_entries) + if (header->table_entries) { + assert(IS_ALIGNED(rec->size, LB_ENTRY_ALIGN)); header->table_bytes += rec->size; - rec = lb_last_record(header); + rec = lb_last_record(header); + } header->table_entries++; rec->tag = LB_TAG_UNUSED; rec->size = sizeof(*rec); @@ -301,7 +303,7 @@ static struct lb_mainboard *lb_mainboard(struct lb_header *header) mainboard->size = ALIGN_UP(sizeof(*mainboard) + strlen(mainboard_vendor) + 1 + - strlen(mainboard_part_number) + 1, 8); + strlen(mainboard_part_number) + 1, LB_ENTRY_ALIGN); mainboard->vendor_idx = 0; mainboard->part_number_idx = strlen(mainboard_vendor) + 1; @@ -380,7 +382,7 @@ static void lb_strings(struct lb_header *header) rec = (struct lb_string *)lb_new_record(header); len = strlen(strings[i].string); rec->tag = strings[i].tag; - rec->size = ALIGN_UP(sizeof(*rec) + len + 1, 8); + rec->size = ALIGN_UP(sizeof(*rec) + len + 1, LB_ENTRY_ALIGN); memcpy(rec->string, strings[i].string, len+1); } @@ -422,8 +424,10 @@ static unsigned long lb_table_fini(struct lb_header *head) { struct lb_record *rec, *first_rec; rec = lb_last_record(head); - if (head->table_entries) + if (head->table_entries) { + assert(IS_ALIGNED(rec->size, LB_ENTRY_ALIGN)); head->table_bytes += rec->size; + } first_rec = lb_first_record(head); head->table_checksum = compute_ip_checksum(first_rec, -- cgit v1.2.3