vboot: Fix linking error with USE_OPTION_TABLE enabled

Fix a linking problem with VBOOT and USE_OPTION_TABLE enabled.
Make use of cbfs_locate_file_in_region() and always search the
cmos_layout.bin in the 'COREBOOT' region.

With this change applied there's no need to include the vboot_locator
in SMM any more, we can't break NVRAM with different CMOS layouts,
and we keep VBOOT and non VBOOT behaviour the same.

Only include cmos_layout.bin and cmos.default in RO region.

Add notes explaining the decisions.

Tested on Intel Sandybridge, builds and boots with vboot enabled.

Change-Id: I10ae94d7936581bbb5ea49384122062bd4934ea5
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/26863
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

3 files changed, 65 insertions, 12 deletions

diff --git a/src/drivers/pc80/rtc/mc146818rtc.c b/src/drivers/pc80/rtc/mc146818rtc.c
index 928b403239..34db4c3424 100644
--- a/src/drivers/pc80/rtc/mc146818rtc.c
+++ b/src/drivers/pc80/rtc/mc146818rtc.c
@@ -240,9 +240,34 @@ static enum cb_err get_cmos_value(unsigned long bit, unsigned long length,
 	return CB_SUCCESS;
 }
 
+static enum cb_err locate_cmos_layout(struct region_device *rdev)
+{
+	uint32_t cbfs_type = CBFS_COMPONENT_CMOS_LAYOUT;
+	struct cbfsf fh;
+
+	/*
+	 * In case VBOOT is enabled and this function is called from SMM,
+	 * we have multiple CMOS layout files and to locate them we'd need to
+	 * include VBOOT into SMM...
+	 *
+	 * Support only one CMOS layout in the 'COREBOOT' region for now.
+	 */
+	if (cbfs_locate_file_in_region(&fh, "COREBOOT", "cmos_layout.bin",
+				       &cbfs_type)) {
+		printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. "
+						"Options are disabled\n");
+		return CB_CMOS_LAYOUT_NOT_FOUND;
+	}
+
+	cbfs_file_data(rdev, &fh);
+
+	return CB_SUCCESS;
+}
+
 enum cb_err get_option(void *dest, const char *name)
 {
 	struct cmos_option_table *ct;
+	struct region_device rdev;
 	struct cmos_entries *ce;
 	size_t namelen;
 	int found = 0;
@@ -255,16 +280,20 @@ enum cb_err get_option(void *dest, const char *name)
 	/* Figure out how long name is */
 	namelen = strnlen(name, CMOS_MAX_NAME_LENGTH);
 
-	/* find the requested entry record */
-	ct = cbfs_boot_map_with_leak("cmos_layout.bin",
-					CBFS_COMPONENT_CMOS_LAYOUT, NULL);
+	if (locate_cmos_layout(&rdev) != CB_SUCCESS) {
+		UNLOCK_NVRAM_CBFS_SPINLOCK();
+		return CB_CMOS_LAYOUT_NOT_FOUND;
+	}
+	ct = rdev_mmap_full(&rdev);
 	if (!ct) {
-		printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. "
-						"Options are disabled\n");
+		printk(BIOS_ERR, "RTC: cmos_layout.bin could not be mapped. "
+		       "Options are disabled\n");
 
 		UNLOCK_NVRAM_CBFS_SPINLOCK();
 		return CB_CMOS_LAYOUT_NOT_FOUND;
 	}
+
+	/* find the requested entry record */
 	ce = (struct cmos_entries *)((unsigned char *)ct + ct->header_length);
 	for (; ce->tag == LB_TAG_OPTION;
 		ce = (struct cmos_entries *)((unsigned char *)ce + ce->size)) {
@@ -275,18 +304,22 @@ enum cb_err get_option(void *dest, const char *name)
 	}
 	if (!found) {
 		printk(BIOS_DEBUG, "WARNING: No CMOS option '%s'.\n", name);
+		rdev_munmap(&rdev, ct);
 		UNLOCK_NVRAM_CBFS_SPINLOCK();
 		return CB_CMOS_OPTION_NOT_FOUND;
 	}
 
 	if (!cmos_checksum_valid(LB_CKS_RANGE_START, LB_CKS_RANGE_END, LB_CKS_LOC)) {
+		rdev_munmap(&rdev, ct);
 		UNLOCK_NVRAM_CBFS_SPINLOCK();
 		return CB_CMOS_CHECKSUM_INVALID;
 	}
 	if (get_cmos_value(ce->bit, ce->length, dest) != CB_SUCCESS) {
+		rdev_munmap(&rdev, ct);
 		UNLOCK_NVRAM_CBFS_SPINLOCK();
 		return CB_CMOS_ACCESS_ERROR;
 	}
+	rdev_munmap(&rdev, ct);
 	UNLOCK_NVRAM_CBFS_SPINLOCK();
 	return CB_SUCCESS;
 }
@@ -347,6 +380,7 @@ unsigned int read_option_lowlevel(unsigned int start, unsigned int size,
 enum cb_err set_option(const char *name, void *value)
 {
 	struct cmos_option_table *ct;
+	struct region_device rdev;
 	struct cmos_entries *ce;
 	unsigned long length;
 	size_t namelen;
@@ -358,14 +392,20 @@ enum cb_err set_option(const char *name, void *value)
 	/* Figure out how long name is */
 	namelen = strnlen(name, CMOS_MAX_NAME_LENGTH);
 
-	/* find the requested entry record */
-	ct = cbfs_boot_map_with_leak("cmos_layout.bin",
-					CBFS_COMPONENT_CMOS_LAYOUT, NULL);
+	if (locate_cmos_layout(&rdev) != CB_SUCCESS) {
+		UNLOCK_NVRAM_CBFS_SPINLOCK();
+		return CB_CMOS_LAYOUT_NOT_FOUND;
+	}
+	ct = rdev_mmap_full(&rdev);
 	if (!ct) {
-		printk(BIOS_ERR, "cmos_layout.bin could not be found. "
-				 "Options are disabled\n");
+		printk(BIOS_ERR, "RTC: cmos_layout.bin could not be mapped. "
+		       "Options are disabled\n");
+
+		UNLOCK_NVRAM_CBFS_SPINLOCK();
 		return CB_CMOS_LAYOUT_NOT_FOUND;
 	}
+
+	/* find the requested entry record */
 	ce = (struct cmos_entries *)((unsigned char *)ct + ct->header_length);
 	for (; ce->tag == LB_TAG_OPTION;
 		ce = (struct cmos_entries *)((unsigned char *)ce + ce->size)) {
@@ -376,6 +416,7 @@ enum cb_err set_option(const char *name, void *value)
 	}
 	if (!found) {
 		printk(BIOS_DEBUG, "WARNING: No CMOS option '%s'.\n", name);
+		rdev_munmap(&rdev, ct);
 		return CB_CMOS_OPTION_NOT_FOUND;
 	}
 
@@ -384,13 +425,18 @@ enum cb_err set_option(const char *name, void *value)
 		length = MAX(strlen((const char *)value) * 8, ce->length - 8);
 		/* make sure the string is null terminated */
 		if (set_cmos_value(ce->bit + ce->length - 8, 8, &(u8[]){0})
-		    != CB_SUCCESS)
+		    != CB_SUCCESS) {
+			rdev_munmap(&rdev, ct);
 			return CB_CMOS_ACCESS_ERROR;
+		}
 	}
 
-	if (set_cmos_value(ce->bit, length, value) != CB_SUCCESS)
+	if (set_cmos_value(ce->bit, length, value) != CB_SUCCESS) {
+		rdev_munmap(&rdev, ct);
 		return CB_CMOS_ACCESS_ERROR;
+	}
 
+	rdev_munmap(&rdev, ct);
 	return CB_SUCCESS;
 }
 
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 9938e6abbe..ca1fc8477e 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -321,6 +321,11 @@ extern const struct cbfs_locator vboot_locator;
 
 static const struct cbfs_locator *locators[] = {
 #if IS_ENABLED(CONFIG_VBOOT)
+	/*
+	 * NOTE: Does not link in SMM, as the vboot_locator isn't compiled.
+	 * ATM there's no need for VBOOT functionality in SMM and it's not
+	 * a problem.
+	 */
 	&vboot_locator,
 #endif
 	&cbfs_master_header_locator,
diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc
index 6f18a35de5..75c2a9e44c 100644
--- a/src/security/vboot/Makefile.inc
+++ b/src/security/vboot/Makefile.inc
@@ -154,6 +154,8 @@ regions-for-file = $(subst $(spc),$(comma),$(sort \
 		font.bin \
 		vbgfx.bin \
 		rmu.bin \
+		cmos_layout.bin \
+		cmos.default \
 		$(call strip_quotes,$(CONFIG_RO_REGION_ONLY)) \
 		,$(1)),COREBOOT,COREBOOT FW_MAIN_A FW_MAIN_B)))