From 24b4af668b3f3995a5844560ce1885d30d8d8bfd Mon Sep 17 00:00:00 2001 From: Tim Wawrzynczak Date: Thu, 1 Oct 2020 15:41:31 -0600 Subject: [PATCH] fw_config: Convert fw_config to a 64-bit field We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once. BUG=b:169668368 TEST=Hacked up this code to OR 0x1_000_0000 with CBI-sourced FW_CONFIG and verify the console print contained that bit. Signed-off-by: Tim Wawrzynczak Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45939 Reviewed-by: Furquan Shaikh Reviewed-by: Angel Pons Tested-by: build bot (Jenkins) --- Documentation/lib/fw_config.md | 10 +++--- src/ec/google/chromeec/ec.c | 11 +++++-- src/ec/google/chromeec/ec.h | 2 +- src/include/fw_config.h | 6 ++-- src/lib/fw_config.c | 24 +++++++------- src/mainboard/google/dedede/board_info.c | 2 +- .../baseboard/include/baseboard/variants.h | 2 +- .../google/zork/variants/baseboard/helpers.c | 8 ++--- util/sconfig/main.c | 31 ++++++++++--------- util/sconfig/sconfig.h | 5 +-- util/sconfig/sconfig.y | 5 +-- 11 files changed, 60 insertions(+), 46 deletions(-) diff --git a/Documentation/lib/fw_config.md b/Documentation/lib/fw_config.md index 63a56dcd7b..dcf1bb4e95 100644 --- a/Documentation/lib/fw_config.md +++ b/Documentation/lib/fw_config.md @@ -73,18 +73,18 @@ return true. ## Firmware Configuration Value -The 32bit value used as the firmware configuration bitmask is meant to be determined at runtime +The 64-bit value used as the firmware configuration bitmask is meant to be determined at runtime but could also be defined at compile time if needed. There are two supported sources for providing this information to coreboot. ### CBFS -The value can be provided with a 32bit raw value in CBFS that is read by coreboot. The value +The value can be provided with a 64-bit raw value in CBFS that is read by coreboot. The value can be set at build time but also adjusted in an existing image with `cbfstool`. To enable this select the `CONFIG_FW_CONFIG_CBFS` option in the build configuration and add a -raw 32bit value to CBFS with the name of the current prefix at `CONFIG_FW_PREFIX/fw_config`. +raw 64-bit value to CBFS with the name of the current prefix at `CONFIG_FW_PREFIX/fw_config`. When `fw_config_probe_device()` or `fw_config_probe()` is called it will look for the specified file in CBFS use the value it contains when matching fields and options. @@ -291,8 +291,8 @@ field and option to check. struct fw_config { const char *field_name; const char *option_name; - uint32_t mask; - uint32_t value; + uint64_t mask; + uint64_t value; }; ``` diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 39cf89512f..2ffccbc77c 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -841,9 +841,16 @@ int google_chromeec_cbi_get_sku_id(uint32_t *id) return cbi_get_uint32(id, CBI_TAG_SKU_ID); } -int google_chromeec_cbi_get_fw_config(uint32_t *fw_config) +int google_chromeec_cbi_get_fw_config(uint64_t *fw_config) { - return cbi_get_uint32(fw_config, CBI_TAG_FW_CONFIG); + uint32_t config; + + if (cbi_get_uint32(&config, CBI_TAG_FW_CONFIG)) + return -1; + + /* FIXME: Yet to determine source of other 32 bits... */ + *fw_config = (uint64_t)config; + return 0; } int google_chromeec_cbi_get_oem_id(uint32_t *id) diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h index c2ceff831f..bed8594a8b 100644 --- a/src/ec/google/chromeec/ec.h +++ b/src/ec/google/chromeec/ec.h @@ -83,7 +83,7 @@ int google_chromeec_reboot(int dev_idx, enum ec_reboot_cmd type, uint8_t flags); */ int google_chromeec_cbi_get_oem_id(uint32_t *id); int google_chromeec_cbi_get_sku_id(uint32_t *id); -int google_chromeec_cbi_get_fw_config(uint32_t *fw_config); +int google_chromeec_cbi_get_fw_config(uint64_t *fw_config); int google_chromeec_cbi_get_dram_part_num(char *buf, size_t bufsize); int google_chromeec_cbi_get_oem_name(char *buf, size_t bufsize); /* version may be stored in CBI as a smaller integer width, but the EC code diff --git a/src/include/fw_config.h b/src/include/fw_config.h index 81980b93ae..494ce7f389 100644 --- a/src/include/fw_config.h +++ b/src/include/fw_config.h @@ -18,8 +18,8 @@ struct fw_config { const char *field_name; const char *option_name; - uint32_t mask; - uint32_t value; + uint64_t mask; + uint64_t value; }; /* Generate a pointer to a compound literal of the fw_config structure. */ @@ -53,7 +53,7 @@ void fw_config_for_each_found(void (*cb)(const struct fw_config *config, void *a * * Return pointer to cached `struct fw_config` if successfully probed, otherwise NULL. */ -const struct fw_config *fw_config_get_found(uint32_t field_mask); +const struct fw_config *fw_config_get_found(uint64_t field_mask); #else diff --git a/src/lib/fw_config.c b/src/lib/fw_config.c index ec3205958d..0973cbeeff 100644 --- a/src/lib/fw_config.c +++ b/src/lib/fw_config.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -14,11 +15,11 @@ /** * fw_config_get() - Provide firmware configuration value. * - * Return 32bit firmware configuration value determined for the system. + * Return 64bit firmware configuration value determined for the system. */ -static uint32_t fw_config_get(void) +static uint64_t fw_config_get(void) { - static uint32_t fw_config_value; + static uint64_t fw_config_value; static bool fw_config_value_initialized; /* Nothing to prepare if setup is already done. */ @@ -35,7 +36,7 @@ static uint32_t fw_config_get(void) __func__); fw_config_value = 0; } else { - printk(BIOS_INFO, "FW_CONFIG value from CBFS is 0x%08x\n", + printk(BIOS_INFO, "FW_CONFIG value from CBFS is 0x%" PRIx64 "\n", fw_config_value); return fw_config_value; } @@ -47,7 +48,7 @@ static uint32_t fw_config_get(void) printk(BIOS_WARNING, "%s: Could not get fw_config from EC\n", __func__); } - printk(BIOS_INFO, "FW_CONFIG value is 0x%08x\n", fw_config_value); + printk(BIOS_INFO, "FW_CONFIG value is 0x%" PRIx64 "\n", fw_config_value); return fw_config_value; } @@ -59,7 +60,8 @@ bool fw_config_probe(const struct fw_config *match) printk(BIOS_INFO, "fw_config match found: %s=%s\n", match->field_name, match->option_name); else - printk(BIOS_INFO, "fw_config match found: mask=0x%08x value=0x%08x\n", + printk(BIOS_INFO, "fw_config match found: mask=0x%" PRIx64 " value=0x%" + PRIx64 "\n", match->mask, match->value); return true; } @@ -70,20 +72,20 @@ bool fw_config_probe(const struct fw_config *match) #if ENV_RAMSTAGE /* - * The maximum number of fw_config fields is limited by the 32-bit mask that is used to + * The maximum number of fw_config fields is limited by the 64-bit mask that is used to * represent them. */ -#define MAX_CACHE_ELEMENTS (8 * sizeof(uint32_t)) +#define MAX_CACHE_ELEMENTS (8 * sizeof(uint64_t)) static const struct fw_config *cached_configs[MAX_CACHE_ELEMENTS]; -static size_t probe_index(uint32_t mask) +static size_t probe_index(uint64_t mask) { assert(mask); - return __ffs(mask); + return __ffs64(mask); } -const struct fw_config *fw_config_get_found(uint32_t field_mask) +const struct fw_config *fw_config_get_found(uint64_t field_mask) { const struct fw_config *config; config = cached_configs[probe_index(field_mask)]; diff --git a/src/mainboard/google/dedede/board_info.c b/src/mainboard/google/dedede/board_info.c index fdb4b5ff64..22d35d7475 100644 --- a/src/mainboard/google/dedede/board_info.c +++ b/src/mainboard/google/dedede/board_info.c @@ -3,7 +3,7 @@ #include #include -int board_info_get_fw_config(uint32_t *fw_config) +int board_info_get_fw_config(uint64_t *fw_config) { return google_chromeec_cbi_get_fw_config(fw_config); } diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h index dc855c63cd..bb41e45931 100644 --- a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h @@ -21,7 +21,7 @@ const struct pad_config *variant_override_gpio_table(size_t *num); * @param fw_config Address where the fw_config is stored. * @return 0 on success or negative integer for errors. */ -int board_info_get_fw_config(uint32_t *fw_config); +int board_info_get_fw_config(uint64_t *fw_config); /* Return memory configuration structure. */ const struct mb_cfg *variant_memcfg_config(void); diff --git a/src/mainboard/google/zork/variants/baseboard/helpers.c b/src/mainboard/google/zork/variants/baseboard/helpers.c index cc07fe18d6..70710351d2 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -48,9 +48,9 @@ enum { FW_CONFIG_SHIFT_FAN = 27, }; -static int get_fw_config(uint32_t *val) +static int get_fw_config(uint64_t *val) { - static uint32_t known_value; + static uint64_t known_value; if (known_value) { *val = known_value; @@ -67,9 +67,9 @@ static int get_fw_config(uint32_t *val) return 0; } -static unsigned int extract_field(uint32_t mask, int shift) +static unsigned int extract_field(uint64_t mask, int shift) { - uint32_t fw_config; + uint64_t fw_config; /* On errors nothing is assumed to be set. */ if (get_fw_config(&fw_config)) diff --git a/util/sconfig/main.c b/util/sconfig/main.c index 4f13293a98..a7b2ce676e 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -4,8 +4,9 @@ #include #include #include -/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ +#include #include +/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ #include #include #include @@ -402,8 +403,8 @@ struct fw_config_field *new_fw_config_field(const char *name, { struct fw_config_field *field = find_fw_config_field(name); - /* Check that field is within 32bits. */ - if (start_bit > end_bit || end_bit > 31) { + /* Check that field is within 64 bits. */ + if (start_bit > end_bit || end_bit > 63) { printf("ERROR: fw_config field %s has invalid range %u-%u\n", name, start_bit, end_bit); exit(1); @@ -452,15 +453,16 @@ static void append_fw_config_option_to_field(struct fw_config_field *field, } } -void add_fw_config_option(struct fw_config_field *field, const char *name, unsigned int value) +void add_fw_config_option(struct fw_config_field *field, const char *name, uint64_t value) { struct fw_config_option *option; - uint32_t field_max_value; + uint64_t field_max_value; /* Check that option value fits within field mask. */ - field_max_value = (1 << (1 + field->end_bit - field->start_bit)) - 1; + field_max_value = (1ull << (1ull + field->end_bit - field->start_bit)) - 1ull; if (value > field_max_value) { - printf("ERROR: fw_config option %s:%s value %u larger than field max %u\n", + printf("ERROR: fw_config option %s:%s value %" PRIx64 " larger than field max %" + PRIx64 "\n", field->name, name, value, field_max_value); exit(1); } @@ -475,7 +477,7 @@ void add_fw_config_option(struct fw_config_field *field, const char *name, unsig } /* Compare values. */ if (value == option->value) { - printf("ERROR: fw_config option %s:%s[%u] redefined as %s\n", + printf("ERROR: fw_config option %s:%s[%" PRIx64 "] redefined as %s\n", field->name, option->name, value, name); exit(1); } @@ -532,23 +534,24 @@ static void emit_fw_config(FILE *fil) while (field) { struct fw_config_option *option = field->options; - uint32_t mask; + uint64_t mask; fprintf(fil, "#define FW_CONFIG_FIELD_%s_NAME \"%s\"\n", field->name, field->name); /* Compute mask from start and end bit. */ - mask = ((1 << (1 + field->end_bit - field->start_bit)) - 1); + mask = ((1ull << (1ull + field->end_bit - field->start_bit)) - 1ull); mask <<= field->start_bit; - fprintf(fil, "#define FW_CONFIG_FIELD_%s_MASK 0x%08x\n", + fprintf(fil, "#define FW_CONFIG_FIELD_%s_MASK 0x%" PRIx64 "\n", field->name, mask); while (option) { fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_NAME \"%s\"\n", field->name, option->name, option->name); - fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_VALUE 0x%08x\n", - field->name, option->name, option->value << field->start_bit); + fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_VALUE 0x%" + PRIx64 "\n", field->name, option->name, + option->value << field->start_bit); option = option->next; } @@ -569,7 +572,7 @@ static int emit_fw_config_probe(FILE *fil, struct device *dev) /* Find matching field. */ struct fw_config_field *field; struct fw_config_option *option; - uint32_t mask, value; + uint64_t mask, value; field = find_fw_config_field(probe->field); if (!field) { diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h index e2ff4c786b..0db1ce59c9 100644 --- a/util/sconfig/sconfig.h +++ b/util/sconfig/sconfig.h @@ -1,6 +1,7 @@ /* sconfig, coreboot device tree compiler */ /* SPDX-License-Identifier: GPL-2.0-only */ +#include #include #include #include @@ -31,7 +32,7 @@ struct pci_irq_info { struct fw_config_option; struct fw_config_option { const char *name; - unsigned int value; + uint64_t value; struct fw_config_option *next; }; struct fw_config_field; @@ -213,6 +214,6 @@ struct fw_config_field *new_fw_config_field(const char *name, unsigned int start_bit, unsigned int end_bit); void add_fw_config_option(struct fw_config_field *field, const char *name, - unsigned int value); + uint64_t value); void add_fw_config_probe(struct bus *bus, const char *field, const char *option); diff --git a/util/sconfig/sconfig.y b/util/sconfig/sconfig.y index cf71b02f3e..84dfe248fd 100755 --- a/util/sconfig/sconfig.y +++ b/util/sconfig/sconfig.y @@ -2,6 +2,7 @@ /* sconfig, coreboot device tree compiler */ /* SPDX-License-Identifier: GPL-2.0-only */ +#include #include "sconfig.h" int yylex(); @@ -16,7 +17,7 @@ static struct fw_config_field *cur_field; struct device *dev; struct chip_instance *chip_instance; char *string; - int number; + uint64_t number; } %token CHIP DEVICE REGISTER ALIAS REFERENCE ASSOCIATION BOOL STATUS MANDATORY BUS RESOURCE END EQUALS HEX STRING PCI PNP I2C APIC CPU_CLUSTER CPU DOMAIN IRQ DRQ SLOT_DESC IO NUMBER SUBSYSTEMID INHERIT IOAPIC_IRQ IOAPIC PCIINT GENERIC SPI USB MMIO LPC ESPI FW_CONFIG_TABLE FW_CONFIG_FIELD FW_CONFIG_OPTION FW_CONFIG_PROBE @@ -116,7 +117,7 @@ fw_config_field: FW_CONFIG_FIELD STRING { /* option */ fw_config_option: FW_CONFIG_OPTION STRING NUMBER /* == field value */ - { add_fw_config_option(cur_field, $2, strtoul($3, NULL, 0)); }; + { add_fw_config_option(cur_field, $2, strtoull($3, NULL, 0)); }; /* probe