Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
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=added 0x1 to SSFC and 0x201 to FW_CONFIG from ectool on Volteer. Verified via console print that FW_CONFIG is 0x100000201.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c 8 files changed, 50 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/1
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 40285dc..f9e8c7d 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -841,9 +841,20 @@ return cbi_get_uint32(id, CBI_TAG_SKU_ID); }
-int google_chromeec_cbi_get_fw_config(uint32_t *fw_config) +/* ChromeOS firmware is treating FW_CONFIG & SSFC as one 64-bit field */ +int google_chromeec_cbi_get_fw_config(uint64_t *fw_config) { - return cbi_get_uint32(fw_config, CBI_TAG_FW_CONFIG); + uint32_t config; + uint32_t ssfc; + + if (cbi_get_uint32(&config, CBI_TAG_FW_CONFIG)) + return -1; + + if (cbi_get_uint32(&ssfc, CBI_TAG_SSFC)) + return -1; + + *fw_config = (uint64_t)((uint64_t)ssfc << 32) | (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 9d4e588..d074d1a 100644 --- a/src/ec/google/chromeec/ec.h +++ b/src/ec/google/chromeec/ec.h @@ -74,7 +74,7 @@ */ 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 81980b9..494ce7f 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 @@ * * 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 fdfab0a..a4aa777 100644 --- a/src/lib/fw_config.c +++ b/src/lib/fw_config.c @@ -7,6 +7,7 @@ #include <device/device.h> #include <ec/google/chromeec/ec.h> #include <fw_config.h> +#include <inttypes.h> #include <lib.h> #include <stdbool.h> #include <stdint.h> @@ -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 @@ __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 @@ 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 @@ 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 @@ #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 __ffs_64(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 fdb4b5f..22d35d7 100644 --- a/src/mainboard/google/dedede/board_info.c +++ b/src/mainboard/google/dedede/board_info.c @@ -3,7 +3,7 @@ #include <baseboard/variants.h> #include <ec/google/chromeec/ec.h>
-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 bd94ef4..44b2b72 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 @@ * @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 d95ab82..63ff24a 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -48,9 +48,9 @@ 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 @@ 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 d5504cd..30e08c3 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -4,8 +4,9 @@ #include <assert.h> #include <ctype.h> #include <getopt.h> -/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ +#include <inttypes.h> #include <libgen.h> +/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ #include <sys/stat.h> #include <commonlib/helpers.h> #include <stdint.h> @@ -402,8 +403,8 @@ { 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); @@ -455,12 +456,12 @@ void add_fw_config_option(struct fw_config_field *field, const char *name, unsigned int 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; - if (value > field_max_value) { - printf("ERROR: fw_config option %s:%s value %u larger than field max %u\n", + field_max_value = (1ull << (1ull + field->end_bit - field->start_bit)) - 1ull; + if ((uint64_t)value > field_max_value) { + printf("ERROR: fw_config option %s:%s value %u larger than field max %" PRIx64 "\n", field->name, name, value, field_max_value); exit(1); } @@ -532,23 +533,23 @@
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, (uint64_t)(option->value << (uint64_t)field->start_bit));
option = option->next; } @@ -569,7 +570,7 @@ /* 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) {