[coreboot-gerrit] [S] Change in coreboot[master]: coreboot_tables: Make existing alignment conventions more explicit

[Julius Werner (Code Review)]

Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/70158 )

Change subject: coreboot_tables: Make existing alignment conventions more explicit
......................................................................

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
---
M src/commonlib/include/commonlib/coreboot_tables.h
M src/lib/coreboot_table.c
2 files changed, 33 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/70158/1

diff --git a/src/commonlib/include/commonlib/coreboot_tables.h b/src/commonlib/include/commonlib/coreboot_tables.h
index 6b5eb59..f80166d 100644
--- a/src/commonlib/include/commonlib/coreboot_tables.h
+++ b/src/commonlib/include/commonlib/coreboot_tables.h
@@ -106,6 +106,9 @@

typedef __aligned(4) uint64_t lb_uint64_t;

+/* All table entry base addresses and sizes must be 4-byte aligned. */
+#define LB_ENTRY_ALIGN 4
+
 struct lb_header {
    uint8_t  signature[4]; /* LBIO */
    uint32_t header_bytes;
@@ -203,6 +206,7 @@
    uint32_t tag;
    uint32_t size;
    uint16_t type;
+	uint8_t pad[2];
 };

#define LB_TAG_CONSOLE_SERIAL8250	0
@@ -288,6 +292,7 @@
    uint8_t reserved_mask_pos;
    uint8_t reserved_mask_size;
    uint8_t orientation;
+	uint8_t pad[2];
 };

struct lb_gpio {
@@ -553,6 +558,7 @@
    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 0f20735..eaf487f 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -76,9 +76,11 @@
 {
    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 @@

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 @@
    	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 @@
 {
    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,