Tim Wawrzynczak submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Julius Werner: Looks good to me, approved
lib/libpayload: Replace strapping_ids with new board configuration entry

There are currently 3 different strapping ID entries in the coreboot
table, which adds overhead. The new fw_config field is also desired in
the coreboot table, which is another kind of strapping id. Therefore,
this patch deprecates the 3 current strapping ID entries (board ID, RAM
code, and SKU ID), and adds a new entry ("board_config") which provides
board ID, RAM code, SKU ID, as well as FW_CONFIG together.

Signed-off-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Change-Id: I1ecec847ee77b72233587c1ad7f124e2027470bf
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46605
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
---
M payloads/libpayload/include/coreboot_tables.h
M payloads/libpayload/include/sysinfo.h
M payloads/libpayload/libc/coreboot.c
M src/commonlib/include/commonlib/coreboot_tables.h
M src/include/fw_config.h
M src/lib/coreboot_table.c
6 files changed, 70 insertions(+), 97 deletions(-)

diff --git a/payloads/libpayload/include/coreboot_tables.h b/payloads/libpayload/include/coreboot_tables.h
index bfdd21e..64db83b 100644
--- a/payloads/libpayload/include/coreboot_tables.h
+++ b/payloads/libpayload/include/coreboot_tables.h
@@ -80,6 +80,7 @@
CB_TAG_TCPA_LOG = 0x0036,
CB_TAG_FMAP = 0x0037,
CB_TAG_SMMSTOREV2 = 0x0039,
+ CB_TAG_BOARD_CONFIG = 0x0040,
CB_TAG_CMOS_OPTION_TABLE = 0x00c8,
CB_TAG_OPTION = 0x00c9,
CB_TAG_OPTION_ENUM = 0x00ca,
@@ -260,12 +261,6 @@
uint32_t index;
};

-struct cb_strapping_id {
- uint32_t tag;
- uint32_t size;
- uint32_t id_code;
-};
-
struct cb_spi_flash {
uint32_t tag;
uint32_t size;
@@ -317,6 +312,16 @@
int32_t early_cmd1_status;
};

+struct cb_board_config {
+ uint32_t tag;
+ uint32_t size;
+
+ struct cbuint64 fw_config;
+ uint32_t board_id;
+ uint32_t ram_code;
+ uint32_t sku_id;
+};
+
#define CB_MAX_SERIALNO_LENGTH 32

struct cb_cmos_option_table {
diff --git a/payloads/libpayload/include/sysinfo.h b/payloads/libpayload/include/sysinfo.h
index a3f61e7..dd739ab 100644
--- a/payloads/libpayload/include/sysinfo.h
+++ b/payloads/libpayload/include/sysinfo.h
@@ -107,11 +107,18 @@
uintptr_t mrc_cache;
uintptr_t acpi_gnvs;

-#define UNDEFINED_STRAPPING_ID (~0)
+#define UNDEFINED_STRAPPING_ID (~0)
+#define UNDEFINED_FW_CONFIG ~((uint64_t)0)
u32 board_id;
u32 ram_code;
u32 sku_id;

+ /*
+ * A payload using this field is responsible for ensuring it checks its
+ * value against UNDEFINED_FW_CONFIG before using it.
+ */
+ u64 fw_config;
+
uintptr_t wifi_calibration;
uint64_t ramoops_buffer;
uint32_t ramoops_buffer_size;
diff --git a/payloads/libpayload/libc/coreboot.c b/payloads/libpayload/libc/coreboot.c
index c48b6cf..b7d2a53 100644
--- a/payloads/libpayload/libc/coreboot.c
+++ b/payloads/libpayload/libc/coreboot.c
@@ -143,22 +143,13 @@
info->acpi_gnvs = get_cbmem_addr(ptr);
}

-static void cb_parse_board_id(unsigned char *ptr, struct sysinfo_t *info)
+static void cb_parse_board_config(unsigned char *ptr, struct sysinfo_t *info)
{
- struct cb_strapping_id *const cbbid = (struct cb_strapping_id *)ptr;
- info->board_id = cbbid->id_code;
-}
-
-static void cb_parse_ram_code(unsigned char *ptr, struct sysinfo_t *info)
-{
- struct cb_strapping_id *const ram_code = (struct cb_strapping_id *)ptr;
- info->ram_code = ram_code->id_code;
-}
-
-static void cb_parse_sku_id(unsigned char *ptr, struct sysinfo_t *info)
-{
- struct cb_strapping_id *const sku_id = (struct cb_strapping_id *)ptr;
- info->sku_id = sku_id->id_code;
+ struct cb_board_config *const config = (struct cb_board_config *)ptr;
+ info->fw_config = cb_unpack64(config->fw_config);
+ info->board_id = config->board_id;
+ info->ram_code = config->ram_code;
+ info->sku_id = config->sku_id;
}

#if CONFIG(LP_NVRAM)
@@ -290,6 +281,7 @@
info->board_id = UNDEFINED_STRAPPING_ID;
info->ram_code = UNDEFINED_STRAPPING_ID;
info->sku_id = UNDEFINED_STRAPPING_ID;
+ info->fw_config = UNDEFINED_FW_CONFIG;

/* Now, walk the tables. */
ptr += header->header_bytes;
@@ -381,14 +373,8 @@
case CB_TAG_ACPI_GNVS:
cb_parse_acpi_gnvs(ptr, info);
break;
- case CB_TAG_BOARD_ID:
- cb_parse_board_id(ptr, info);
- break;
- case CB_TAG_RAM_CODE:
- cb_parse_ram_code(ptr, info);
- break;
- case CB_TAG_SKU_ID:
- cb_parse_sku_id(ptr, info);
+ case CB_TAG_BOARD_CONFIG:
+ cb_parse_board_config(ptr, info);
break;
case CB_TAG_WIFI_CALIBRATION:
cb_parse_wifi_calibration(ptr, info);
diff --git a/src/commonlib/include/commonlib/coreboot_tables.h b/src/commonlib/include/commonlib/coreboot_tables.h
index 4406002..3e74e6b 100644
--- a/src/commonlib/include/commonlib/coreboot_tables.h
+++ b/src/commonlib/include/commonlib/coreboot_tables.h
@@ -62,15 +62,15 @@
LB_TAG_DMA = 0x0022,
LB_TAG_RAM_OOPS = 0x0023,
LB_TAG_ACPI_GNVS = 0x0024,
- LB_TAG_BOARD_ID = 0x0025,
+ LB_TAG_BOARD_ID = 0x0025, /* deprecated */
LB_TAG_VERSION_TIMESTAMP = 0x0026,
LB_TAG_WIFI_CALIBRATION = 0x0027,
- LB_TAG_RAM_CODE = 0x0028,
+ LB_TAG_RAM_CODE = 0x0028, /* deprecated */
LB_TAG_SPI_FLASH = 0x0029,
LB_TAG_SERIALNO = 0x002a,
LB_TAG_MTC = 0x002b,
LB_TAG_VPD = 0x002c,
- LB_TAG_SKU_ID = 0x002d,
+ LB_TAG_SKU_ID = 0x002d, /* deprecated */
LB_TAG_BOOT_MEDIA_PARAMS = 0x0030,
LB_TAG_CBMEM_ENTRY = 0x0031,
LB_TAG_TSC_INFO = 0x0032,
@@ -81,6 +81,8 @@
LB_TAG_FMAP = 0x0037,
LB_TAG_PLATFORM_BLOB_VERSION = 0x0038,
LB_TAG_SMMSTOREV2 = 0x0039,
+ LB_TAG_BOARD_CONFIG = 0x0040,
+ /* The following options are CMOS-related */
LB_TAG_CMOS_OPTION_TABLE = 0x00c8,
LB_TAG_OPTION = 0x00c9,
LB_TAG_OPTION_ENUM = 0x00ca,
@@ -347,12 +349,6 @@
uint32_t index;
};

-struct lb_strapping_id {
- uint32_t tag;
- uint32_t size;
- uint32_t id_code;
-};
-
struct lb_spi_flash {
uint32_t tag;
uint32_t size;
@@ -416,6 +412,16 @@
struct mac_address mac_addrs[0];
};

+struct lb_board_config {
+ uint32_t tag;
+ uint32_t size;
+
+ struct lb_uint64 fw_config;
+ uint32_t board_id;
+ uint32_t ram_code;
+ uint32_t sku_id;
+};
+
#define MAX_SERIALNO_LENGTH 32

/* The following structures are for the CMOS definitions table */
diff --git a/src/include/fw_config.h b/src/include/fw_config.h
index c9ef75f..3c87725 100644
--- a/src/include/fw_config.h
+++ b/src/include/fw_config.h
@@ -8,6 +8,8 @@
#include <stdbool.h>
#include <stdint.h>

+#define UNDEFINED_FW_CONFIG ~((uint64_t)0)
+
/**
* struct fw_config - Firmware configuration field and option.
* @field_name: Name of the field that this option belongs to.
@@ -30,8 +32,6 @@
.value = FW_CONFIG_FIELD_##__field##_OPTION_##__option##_VALUE \
})

-#if CONFIG(FW_CONFIG)
-
/**
* fw_config_get() - Provide firmware configuration value.
*
@@ -39,6 +39,8 @@
*/
uint64_t fw_config_get(void);

+#if CONFIG(FW_CONFIG)
+
/**
* fw_config_probe() - Check if field and option matches.
* @match: Structure containing field and option to probe.
diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c
index 857f5a5..69ded3c 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -12,6 +12,7 @@
#include <boardid.h>
#include <device/device.h>
#include <fmap.h>
+#include <fw_config.h>
#include <stdlib.h>
#include <cbfs.h>
#include <cbmem.h>
@@ -213,23 +214,7 @@
__weak uint32_t board_id(void) { return UNDEFINED_STRAPPING_ID; }
__weak uint32_t ram_code(void) { return UNDEFINED_STRAPPING_ID; }
__weak uint32_t sku_id(void) { return UNDEFINED_STRAPPING_ID; }
-
-static void lb_board_id(struct lb_header *header)
-{
- struct lb_strapping_id *rec;
- uint32_t bid = board_id();
-
- if (bid == UNDEFINED_STRAPPING_ID)
- return;
-
- rec = (struct lb_strapping_id *)lb_new_record(header);
-
- rec->tag = LB_TAG_BOARD_ID;
- rec->size = sizeof(*rec);
- rec->id_code = bid;
-
- printk(BIOS_INFO, "Board ID: %d\n", bid);
-}
+__weak uint64_t fw_config_get(void) { return UNDEFINED_FW_CONFIG; }

static void lb_boot_media_params(struct lb_header *header)
{
@@ -257,40 +242,6 @@
bmp->fmap_offset = get_fmap_flash_offset();
}

-static void lb_ram_code(struct lb_header *header)
-{
- struct lb_strapping_id *rec;
- uint32_t code = ram_code();
-
- if (code == UNDEFINED_STRAPPING_ID)
- return;
-
- rec = (struct lb_strapping_id *)lb_new_record(header);
-
- rec->tag = LB_TAG_RAM_CODE;
- rec->size = sizeof(*rec);
- rec->id_code = code;
-
- printk(BIOS_INFO, "RAM code: %d\n", code);
-}
-
-static void lb_sku_id(struct lb_header *header)
-{
- struct lb_strapping_id *rec;
- uint32_t sid = sku_id();
-
- if (sid == UNDEFINED_STRAPPING_ID)
- return;
-
- rec = (struct lb_strapping_id *)lb_new_record(header);
-
- rec->tag = LB_TAG_SKU_ID;
- rec->size = sizeof(*rec);
- rec->id_code = sid;
-
- printk(BIOS_INFO, "SKU ID: %d\n", sid);
-}
-
static void lb_mmc_info(struct lb_header *header)
{
struct lb_mmc_info *rec;
@@ -370,6 +321,24 @@
return mainboard;
}

+static struct lb_board_config *lb_board_config(struct lb_header *header)
+{
+ struct lb_record *rec;
+ struct lb_board_config *config;
+ rec = lb_new_record(header);
+ config = (struct lb_board_config *)rec;
+
+ config->tag = LB_TAG_BOARD_CONFIG;
+ config->size = sizeof(*config);
+
+ config->board_id = board_id();
+ config->ram_code = ram_code();
+ config->sku_id = sku_id();
+ config->fw_config = pack_lb64(fw_config_get());
+
+ return config;
+}
+
#if CONFIG(USE_OPTION_TABLE)
static struct cmos_checksum *lb_cmos_checksum(struct lb_header *header)
{
@@ -536,11 +505,6 @@
lb_vbnv(head);
#endif

- /* Add strapping IDs if available */
- lb_board_id(head);
- lb_ram_code(head);
- lb_sku_id(head);
-
/* Pass mmc early init status */
lb_mmc_info(head);

@@ -563,6 +527,9 @@

lb_boot_media_params(head);

+ /* Board configuration information (including straps) */
+ lb_board_config(head);
+
/* Add architecture records. */
lb_arch_add_records(head);


To view, visit change 46605. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1ecec847ee77b72233587c1ad7f124e2027470bf
Gerrit-Change-Number: 46605
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie@chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged