[coreboot-gerrit] New patch to review for coreboot: 73cf51b cmos_options: Add and check version field.

Vladimir Serbinenko (phcoder@gmail.com) gerrit at coreboot.org
Fri Jan 24 17:43:17 CET 2014


Vladimir Serbinenko (phcoder at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4790

-gerrit

commit 73cf51bc1761cdc5dcc4298644f6aa294f421bf4
Author: Vladimir Serbinenko <phcoder at 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.
    
    Problem is that version number has to bemaintained manually. Another
    alternative would be to store checksum of option table descriptor.
    
    Not for commit: proof of concept.
    
    Change-Id: Id3bb904fac44279dd420c2bf617199f890bb3e0e
    Signed-off-by: Vladimir Serbinenko <phcoder at gmail.com>
---
 payloads/nvramcui/nvramcui.c           |  4 ++--
 src/drivers/pc80/mc146818rtc_early.c   | 29 ++++++++++++++++++-----------
 src/mainboard/lenovo/x230/cmos.layout  |  3 ++-
 util/nvramtool/accessors/layout-bin.c  |  4 ++++
 util/nvramtool/accessors/layout-text.c | 10 ++++++++++
 util/nvramtool/cmos_ops.c              |  4 ++++
 util/nvramtool/layout.c                |  6 ++++--
 util/nvramtool/layout.h                |  1 +
 util/nvramtool/lbtable.c               |  4 ++++
 9 files changed, 49 insertions(+), 16 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..517b43a 100644
--- a/src/mainboard/lenovo/x230/cmos.layout
+++ b/src/mainboard/lenovo/x230/cmos.layout
@@ -108,7 +108,8 @@ entries
 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
+976          8       v       1        version
 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..95bfd85 100644
--- a/util/nvramtool/accessors/layout-text.c
+++ b/util/nvramtool/accessors/layout-text.c
@@ -258,6 +258,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;
@@ -625,6 +628,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 +860,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/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/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;



More information about the coreboot-gerrit mailing list