Vladimir Serbinenko (phcoder@gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4790
-gerrit
commit 867748d6ca2ab053ef5863a96898dad8594fac8d Author: Vladimir Serbinenko phcoder@gmail.com Date: Fri Jan 24 17:40:44 2014 +0100
cmos_options: Add and check version field.
This solves the issue with adding or moving options.
The version field is a 16-bit checksum (of universal hashing family) of options and their positions but not of defaults. This way no need to maintain it manually
This is proof of concept. If idea is accepted then there will be a laborous but easy task of adding version field to all mobos.
If 16 bits is too much we can reduce it to 8 bits.
Change-Id: Id3bb904fac44279dd420c2bf617199f890bb3e0e Signed-off-by: Vladimir Serbinenko phcoder@gmail.com --- payloads/nvramcui/nvramcui.c | 4 +-- src/drivers/pc80/mc146818rtc_early.c | 29 +++++++++------- src/mainboard/lenovo/x230/cmos.layout | 8 +++-- util/nvramtool/accessors/layout-bin.c | 4 +++ util/nvramtool/accessors/layout-text.c | 60 ++++++++++++++++++++++++++++++++++ util/nvramtool/cli/nvramtool.c | 18 ++++++++-- util/nvramtool/cmos_ops.c | 4 +++ util/nvramtool/input_file.c | 9 +++++ util/nvramtool/layout.c | 6 ++-- util/nvramtool/layout.h | 1 + util/nvramtool/lbtable.c | 4 +++ 11 files changed, 127 insertions(+), 20 deletions(-)
diff --git a/payloads/nvramcui/nvramcui.c b/payloads/nvramcui/nvramcui.c index daf153b..4765921 100644 --- a/payloads/nvramcui/nvramcui.c +++ b/payloads/nvramcui/nvramcui.c @@ -97,7 +97,7 @@ int main() int maxlength=0; struct cb_cmos_entries *option = first_cmos_entry(opttbl); while (option) { - if ((option->config != 'r') && (strcmp("check_sum", option->name) != 0)) { + if ((option->config != 'r') && (option->config != 'v') && (strcmp("check_sum", option->name) != 0)) { maxlength = max(maxlength, strlen(option->name)); numopts++; } @@ -113,7 +113,7 @@ int main() /* walk over options, fetch details */ option = first_cmos_entry(opttbl); for (i=0;i<numopts;i++) { - while ((option->config == 'r') || (strcmp("check_sum", option->name) == 0)) { + while ((option->config == 'r') || (option->config == 'v') || (strcmp("check_sum", option->name) == 0)) { option = next_cmos_entry(option); } fields[2*i] = new_field(1, strlen(option->name), i*2, 1, 0, 0); diff --git a/src/drivers/pc80/mc146818rtc_early.c b/src/drivers/pc80/mc146818rtc_early.c index 0652f27..6309103 100644 --- a/src/drivers/pc80/mc146818rtc_early.c +++ b/src/drivers/pc80/mc146818rtc_early.c @@ -20,11 +20,29 @@ static int cmos_error(void) return (reg_d & RTC_VRT) == 0; }
+unsigned read_option_lowlevel(unsigned start, unsigned size, unsigned def) +{ +#if CONFIG_USE_OPTION_TABLE + unsigned byte; + byte = cmos_read(start/8); + return (byte >> (start & 7U)) & ((1U << size) - 1U); +#else + return def; +#endif +} + static int cmos_chksum_valid(void) { #if CONFIG_USE_OPTION_TABLE unsigned char addr; u16 sum, old_sum; + unsigned version; + + version = read_option_lowlevel(CMOS_VSTART_version, CMOS_VLEN_version, + 0); + if (version != CMOS_version_expected) + return 0; + sum = 0; /* Compute the cmos checksum */ for(addr = LB_CKS_RANGE_START; addr <= LB_CKS_RANGE_END; addr++) { @@ -91,14 +109,3 @@ static inline int do_normal_boot(void)
return (byte & (1<<1)); } - -unsigned read_option_lowlevel(unsigned start, unsigned size, unsigned def) -{ -#if CONFIG_USE_OPTION_TABLE - unsigned byte; - byte = cmos_read(start/8); - return (byte >> (start & 7U)) & ((1U << size) - 1U); -#else - return def; -#endif -} diff --git a/src/mainboard/lenovo/x230/cmos.layout b/src/mainboard/lenovo/x230/cmos.layout index 6f8822f..17e2b0a 100644 --- a/src/mainboard/lenovo/x230/cmos.layout +++ b/src/mainboard/lenovo/x230/cmos.layout @@ -101,14 +101,18 @@ entries # coreboot config options: cpu 424 1 e 2 hyper_threading
-#425 559 r 0 unused +#425 7 r 0 unused + +432 16 v 0 version + +#448 535 r 0 unused
# SandyBridge MRC Scrambler Seed values 896 32 r 0 mrc_scrambler_seed 928 32 r 0 mrc_scrambler_seed_s3 960 16 r 0 mrc_scrambler_seed_chk
-# coreboot config options: check sums +# coreboot config options: check sums and versions 984 16 h 0 check_sum
# ----------------------------------------------------------------- diff --git a/util/nvramtool/accessors/layout-bin.c b/util/nvramtool/accessors/layout-bin.c index 4b7f8d6..79d2007 100644 --- a/util/nvramtool/accessors/layout-bin.c +++ b/util/nvramtool/accessors/layout-bin.c @@ -273,6 +273,10 @@ static void process_cmos_table(void) cmos_entry.config = CMOS_ENTRY_HEX; break;
+ case 'v': + cmos_entry.config = CMOS_ENTRY_VERSION; + break; + case 'r': cmos_entry.config = CMOS_ENTRY_RESERVED; break; diff --git a/util/nvramtool/accessors/layout-text.c b/util/nvramtool/accessors/layout-text.c index a06f560..a3ad5bc 100644 --- a/util/nvramtool/accessors/layout-text.c +++ b/util/nvramtool/accessors/layout-text.c @@ -192,6 +192,9 @@ static int line_num;
static const char *layout_filename = NULL;
+static unsigned int layout_checksum; +static int version_bit = -1; + /**************************************************************************** * set_layout_filename * @@ -258,6 +261,9 @@ void write_cmos_layout_header(const char *header_filename) cmos_entry->name, cmos_entry->bit); fprintf(fp, "#define CMOS_VLEN_%s\t%d\n", cmos_entry->name, cmos_entry->length); + if (strcmp (cmos_entry->name, "version") == 0) + fprintf(fp, "#define CMOS_version_expected\t%d\n", + cmos_entry->config_id); }
layout.summed_area_start = cmos_checksum_start; @@ -311,6 +317,26 @@ void write_cmos_layout(FILE * f) layout.summed_area_end, layout.checksum_at); }
+static void process_layout_checksum(void) +{ + cmos_entry_t cmos_entry; + + if (version_bit < 0) { + fprintf(stderr, + "%s: version field not found.\n", prog_name); + exit(1); + } + + memset (&cmos_entry, 0, sizeof (cmos_entry)); + + cmos_entry.bit = version_bit; + cmos_entry.length = 16; + cmos_entry.config_id = layout_checksum; + cmos_entry.config = 'v'; + strcpy (cmos_entry.name, "version"); + try_add_layout_file_entry(&cmos_entry); +} + /**************************************************************************** * process_layout_file * @@ -370,6 +396,8 @@ static void process_layout_file(FILE * f) /* Skip past all enums. They have already been processed. */ while (!process_enum(f, 1)) ;
+ process_layout_checksum(); + /* Process CMOS checksum info. */ process_checksum_info(f);
@@ -471,6 +499,31 @@ static int process_entry(FILE * f, int skip_add) create_entry(&cmos_entry, &line[match[1].rm_so], &line[match[2].rm_so], &line[match[3].rm_so], &line[match[4].rm_so], &line[match[5].rm_so]); + + + /* Special case of universal hashing. Small primes, + different, chosen at random. + */ + layout_checksum = (layout_checksum * 307) % 0xffff; + layout_checksum += cmos_entry.bit * 43 + + cmos_entry.length * 257 + + cmos_entry.config_id * 313 + + cmos_entry.config * 509; + layout_checksum %= 0xffff; + layout_checksum += compute_ip_checksum(cmos_entry.name, + strlen (cmos_entry.name)) + * 89; + layout_checksum %= 0xffff; + + if (cmos_entry.config == 'v') { + if (cmos_entry.length != 16) { + fprintf(stderr, + "%s: version is not 16-bit long.\n", prog_name); + exit(1); + } + version_bit = cmos_entry.bit; + break; + } try_add_layout_file_entry(&cmos_entry); break; } @@ -625,6 +678,10 @@ static void create_entry(cmos_entry_t * cmos_entry, cmos_entry->config = CMOS_ENTRY_HEX; break;
+ case 'v': + cmos_entry->config = CMOS_ENTRY_VERSION; + break; + case 's': cmos_entry->config = CMOS_ENTRY_STRING; break; @@ -853,6 +910,9 @@ static char cmos_entry_char_value(cmos_entry_config_t config) case CMOS_ENTRY_HEX: return 'h';
+ case CMOS_ENTRY_VERSION: + return 'v'; + case CMOS_ENTRY_RESERVED: return 'r';
diff --git a/util/nvramtool/cli/nvramtool.c b/util/nvramtool/cli/nvramtool.c index d5acc58..d2f833a 100644 --- a/util/nvramtool/cli/nvramtool.c +++ b/util/nvramtool/cli/nvramtool.c @@ -522,7 +522,8 @@ static int list_one_param(const char name[], int show_name) exit(1); }
- if (e->config == CMOS_ENTRY_RESERVED) { + if (e->config == CMOS_ENTRY_RESERVED + || e->config == CMOS_ENTRY_VERSION) { fprintf(stderr, "%s: Parameter %s is reserved.\n", prog_name, name); exit(1); @@ -546,6 +547,7 @@ static int list_all_params(void)
for (e = first_cmos_entry(); e != NULL; e = next_cmos_entry(e)) { if ((e->config == CMOS_ENTRY_RESERVED) + || (e->config == CMOS_ENTRY_VERSION) || is_checksum_name(e->name)) continue;
@@ -591,6 +593,7 @@ static void list_param_enums(const char name[]) break;
case CMOS_ENTRY_RESERVED: + case CMOS_ENTRY_VERSION: printf("Parameter %s is reserved.\n", name); break;
@@ -611,8 +614,8 @@ static void list_param_enums(const char name[]) ****************************************************************************/ static void set_one_param(const char name[], const char value[]) { - const cmos_entry_t *e; - unsigned long long n; + const cmos_entry_t *e, *ev; + unsigned long long n, nv;
if (is_checksum_name(name) || (e = find_cmos_entry(name)) == NULL) { fprintf(stderr, "%s: CMOS parameter %s not found.", prog_name, @@ -686,9 +689,18 @@ static void set_one_param(const char name[], const char value[]) goto fail; }
+ if ((ev = find_cmos_entry("version")) == NULL) { + fprintf(stderr, "%s: CMOS parameter %s not found.", prog_name, + "version"); + exit(1); + } + + prepare_cmos_write(ev, 0, &nv); + /* write the value to nonvolatile RAM */ set_iopl(3); cmos_write(e, n); + cmos_write(ev, nv); cmos_checksum_write(cmos_checksum_compute()); set_iopl(0); return; diff --git a/util/nvramtool/cmos_ops.c b/util/nvramtool/cmos_ops.c index 91c9f45..e20b24c 100644 --- a/util/nvramtool/cmos_ops.c +++ b/util/nvramtool/cmos_ops.c @@ -71,6 +71,7 @@ int prepare_cmos_read(const cmos_entry_t * e) switch (e->config) { case CMOS_ENTRY_ENUM: case CMOS_ENTRY_HEX: + case CMOS_ENTRY_VERSION: case CMOS_ENTRY_STRING: break;
@@ -141,6 +142,9 @@ int prepare_cmos_write(const cmos_entry_t * e, const char value_str[], return CMOS_OP_NEGATIVE_INT;
break; + case CMOS_ENTRY_VERSION: + out = e->config_id; + break;
case CMOS_ENTRY_STRING: if (e->length < (8 * strlen(value_str))) diff --git a/util/nvramtool/input_file.c b/util/nvramtool/input_file.c index 1520930..6ce2c5b 100644 --- a/util/nvramtool/input_file.c +++ b/util/nvramtool/input_file.c @@ -157,6 +157,14 @@ cmos_write_t *process_input_file(FILE * f) void do_cmos_writes(cmos_write_t * list) { cmos_write_t *item; + const cmos_entry_t *ev; + + ev = find_cmos_entry("version"); + if (ev == NULL) { + fprintf(stderr, "%s: CMOS parameter %s not found.", prog_name, + "version"); + exit(1); + }
set_iopl(3);
@@ -171,6 +179,7 @@ void do_cmos_writes(cmos_write_t * list) free(item); }
+ cmos_write(ev, ev->config_id); cmos_checksum_write(cmos_checksum_compute()); set_iopl(0); } diff --git a/util/nvramtool/layout.c b/util/nvramtool/layout.c index f543d12..1872e2c 100644 --- a/util/nvramtool/layout.c +++ b/util/nvramtool/layout.c @@ -53,6 +53,7 @@ static int entries_overlap(const cmos_entry_t * p, const cmos_entry_t * q); static const cmos_enum_item_t *find_first_cmos_enum_id(unsigned config_id);
const char checksum_param_name[] = "check_sum"; +const char version_param_name[] = "version";
/* Newer versions of coreboot store the 3 pieces of information below in the * coreboot table so we don't have to rely on hardcoded values. @@ -424,11 +425,12 @@ const cmos_enum_t *next_cmos_enum_id(const cmos_enum_t * last) * is_checksum_name * * Return 1 if 'name' matches the name of the parameter representing the CMOS - * checksum. Else return 0. + * checksum or version. Else return 0. ****************************************************************************/ int is_checksum_name(const char name[]) { - return !strcmp(name, checksum_param_name); + return !strcmp(name, checksum_param_name) + && !strcmp(name, version_param_name); }
/**************************************************************************** diff --git a/util/nvramtool/layout.h b/util/nvramtool/layout.h index 082c31b..b4e6d6e 100644 --- a/util/nvramtool/layout.h +++ b/util/nvramtool/layout.h @@ -50,6 +50,7 @@ typedef enum { CMOS_ENTRY_HEX = 'h', CMOS_ENTRY_STRING = 's', CMOS_ENTRY_RESERVED = 'r', + CMOS_ENTRY_VERSION = 'v', } cmos_entry_config_t;
/* This represents a CMOS parameter. */ diff --git a/util/nvramtool/lbtable.c b/util/nvramtool/lbtable.c index 5f1f22d..60f8b44 100644 --- a/util/nvramtool/lbtable.c +++ b/util/nvramtool/lbtable.c @@ -775,6 +775,10 @@ static void print_option_record(const struct cmos_entries *cmos_entry) strcpy(s, "HEX"); break;
+ case 'v': + strcpy(s, "VERSION"); + break; + case 'r': strcpy(s, "RESERVED"); break;