Simon Glass has uploaded this change for review.
Support CCB at a fixed SPI-flash offset
Enhance the CCB to allow it to be in an FMAP region, instead of embedded
in the bootblock. This adds quite a bit of logic, particularly in
cbfstool which must now support adjusting the CCB region automatically.
This creates a CCB region in the FMAP containing the CCB. This is added
to the ROM during the build.
BUG=b:172341184, b:262546009, b:249105972
BRANCH=none
TEST=manually test each of the three options using:
make menuconfig; (select option); rm -r build && make -j30
qemu-system-x86_64 -bios build/coreboot.rom -nographic
(see that console appears)
build/util/cbfstool/cbfstool build/coreboot.rom
configure -n console -V quiet
qemu-system-x86_64 -bios build/coreboot.rom -nographic
(see that console is suppressed, except for first part of bootblock with
CCB_CBFS and CCB_FMAP)
Change-Id: I8abc5ba55d75a3defdea548fffcedba74d4737c2
Signed-off-by: Simon Glass <sjg@chromium.org>
---
M Documentation/technotes/ccb.md
M Makefile.mk
M src/Kconfig
M src/commonlib/include/commonlib/ccb.h
M src/commonlib/include/commonlib/region.h
M src/lib/bootblock.c
M src/lib/ccb.c
M util/cbfstool/cbfstool.c
M util/cbfstool/default-x86.fmd
9 files changed, 145 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/83293/1
diff --git a/Documentation/technotes/ccb.md b/Documentation/technotes/ccb.md
index bdb9b18..63cca83 100644
--- a/Documentation/technotes/ccb.md
+++ b/Documentation/technotes/ccb.md
@@ -84,11 +84,22 @@
the CCB_CBFS option. This applies mostly to AMD, since Intel platforms, do not
have a signed bootblock.
-## Future extensions
+For boards where it is not possible or desirable to access CBFS files early in
+bootblock, an FMAP region can be used to hold the CCB. Care should be taken that
+this region is in read-only flash, if preventing users from changing it is
+important.
-CCB could be stored in an FMAP region.
+If something goes wrong with the CCB init while the console is off, the
+ccb_check() function can be called to see what happened.
+
+## Future extensions
The CMOS option feature (in `include/option.h`) could be expanded to provide an
API for CCB. Using strings for options might be too inefficient for bootblock,
so an enum could be introduced for common option types, with a lookup table used
to convert between strings and integers.
+
+The initial bootblock output could be suppressed, even with the CCB_CBFS and
+CCB_FMAP options, by just living with the risk that something goes wrong in
+setting up FMAP / CBFS. This would be best handled by enabling the CCB option
+only when the board is working correctly.
diff --git a/Makefile.mk b/Makefile.mk
index 16b6539..077898f 100644
--- a/Makefile.mk
+++ b/Makefile.mk
@@ -485,6 +485,10 @@
CPPFLAGS_common += -D__BUILD_DIR__=\"$(obj)\"
CPPFLAGS_common += -D__COREBOOT__
+# Must be >= sizeof(struct ccb)
+CCB_SIZE := 16
+CPPFLAGS_common += -DCCB_SIZE=$(CCB_SIZE)
+
ifeq ($(BUILD_TIMELESS),1)
CPPFLAGS_common += -D__TIMELESS__
endif
@@ -1078,6 +1082,17 @@
FMAP_SPD_CACHE_ENTRY :=
endif
+# Align CCB end so that FMAP doesn't end up in a strange place and bootblock
+# won't fit in what remains
+ifeq ($(CONFIG_CCB_FMAP),y)
+FMAP_CCB_BASE := $(call int-align, $(FMAP_CURRENT_BASE), 0x10)
+FMAP_CCB_SIZE := $(call int-align, $(CCB_SIZE), 0x200)
+FMAP_CCB_ENTRY := CCB@$(FMAP_CCB_BASE) $(FMAP_CCB_SIZE)
+FMAP_CURRENT_BASE := $(call int-add, $(FMAP_CCB_BASE) $(FMAP_CCB_SIZE))
+else
+FMAP_CCB_ENTRY :=
+endif
+
ifeq ($(CONFIG_VPD),y)
FMAP_VPD_BASE := $(call int-align, $(FMAP_CURRENT_BASE), 0x4000)
FMAP_VPD_SIZE := $(CONFIG_VPD_FMAP_SIZE)
@@ -1173,6 +1188,7 @@
-e "s,##MRC_CACHE_ENTRY##,$(FMAP_MRC_CACHE_ENTRY)," \
-e "s,##SMMSTORE_ENTRY##,$(FMAP_SMMSTORE_ENTRY)," \
-e "s,##SPD_CACHE_ENTRY##,$(FMAP_SPD_CACHE_ENTRY)," \
+ -e "s,##CCB_ENTRY##,$(FMAP_CCB_ENTRY)," \
-e "s,##VPD_ENTRY##,$(FMAP_VPD_ENTRY)," \
-e "s,##HSPHY_FW_ENTRY##,$(FMAP_HSPHY_FW_ENTRY)," \
-e "s,##CBFS_BASE##,$(FMAP_CBFS_BASE)," \
@@ -1203,6 +1219,10 @@
add_bootblock = $(CBFSTOOL) $(1) write -u -r BOOTBLOCK -f $(2)
endif
+ifneq ($(CONFIG_ARCH_X86),)
+add_ccb = $(CBFSTOOL) $(1) write -u -r CCB -f $(2)
+endif
+
# coreboot.pre doesn't follow the standard Make conventions. It gets modified
# by multiple rules, and thus we can't compute the dependencies correctly.
$(shell rm -f $(obj)/coreboot.pre)
@@ -1212,6 +1232,12 @@
$(CBFSTOOL) $@.tmp create -M $(obj)/fmap.fmap -r $(shell cat $(obj)/fmap.desc)
printf " BOOTBLOCK\n"
$(call add_bootblock,$@.tmp,$(objcbfs)/bootblock.bin)
+ifneq ($(CONFIG_CCB_FMAP),)
+ # Force use of shell so these hex values work
+ /usr/bin/printf "\x01\xb0\x43\xc0" >$(objcbfs)/ccb.bin
+ truncate -s $(CCB_SIZE) $(objcbfs)/ccb.bin
+ $(call add_ccb,$@.tmp,$(objcbfs)/ccb.bin)
+endif
$(prebuild-files) true
mv $@.tmp $@
else # ifneq ($(CONFIG_UPDATE_IMAGE),y)
diff --git a/src/Kconfig b/src/Kconfig
index bee8363..3594e60 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -616,6 +616,18 @@
See Documentation/util/cbfstool/ccb.md for more information.
+config CCB_FMAP
+ bool "Read coreboot control block from fixed ROM offset"
+ depends on CCB
+ help
+ Enable this to read the CCB (coreboot control block) from a fixed ROM
+ offset. The file is read after CBFS is inited in bootblock.
+
+ The CCB provides a few simple settings for coreboot which can be
+ changed using the 'cbfstool set-ccb' command.
+
+ See Documentation/util/cbfstool/ccb.md for more information.
+
endchoice
menu "Software Bill Of Materials (SBOM)"
diff --git a/src/commonlib/include/commonlib/ccb.h b/src/commonlib/include/commonlib/ccb.h
index b77a256..26bdb8d 100644
--- a/src/commonlib/include/commonlib/ccb.h
+++ b/src/commonlib/include/commonlib/ccb.h
@@ -25,6 +25,12 @@
/* Magic number at the top of the CCB and used to detect it in the bootblock */
#define CCB_MAGIC 0xc043b001
+/* Name of CCB FMAP region, if CONFIG_CCB_FMAP is enabled */
+#define CCB_REGION "CCB"
+
+/* Assumed maximum size of CCB (can be larger than sizeof(struct ccb) */
+#define CCB_MAX_SIZE 0x10
+
/**
* struct ccb - Data in the CCB
*
diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h
index 25efcc8..a224cb6 100644
--- a/src/commonlib/include/commonlib/region.h
+++ b/src/commonlib/include/commonlib/region.h
@@ -94,6 +94,11 @@
}, \
}
+static inline bool rdev_valid(const struct region_device *rdev)
+{
+ return rdev->ops != NULL;
+}
+
/* Helper to dynamically initialize region device. */
void region_device_init(struct region_device *rdev,
const struct region_device_ops *ops, size_t offset,
diff --git a/src/lib/bootblock.c b/src/lib/bootblock.c
index 454e538..9284c66 100644
--- a/src/lib/bootblock.c
+++ b/src/lib/bootblock.c
@@ -60,7 +60,7 @@
exception_init();
}
- /* late init of CCB for when CCB is in CBFS */
+ /* late init of CCB for when CCB is in CBFS or FMAP */
if (!ENV_HOLDS_CCB) {
ccb_init();
diff --git a/src/lib/ccb.c b/src/lib/ccb.c
index c45ea04..0c61a23 100644
--- a/src/lib/ccb.c
+++ b/src/lib/ccb.c
@@ -5,6 +5,7 @@
#include <cbmem.h>
#include <commonlib/ccb_api.h>
#include <console/console.h>
+#include <fmap.h>
#include <program_loading.h>
#include <string.h>
#include <symbols.h>
@@ -16,6 +17,12 @@
};
#endif
+_Static_assert(sizeof(struct ccb) <= CCB_SIZE,
+ "CCB_SIZE must be no smaller than sizeof(struct ccb)");
+
+/* copy of CCB when read from an rdev */
+static struct ccb ccb_holder;
+
/* holds the global pointer to the coreboot Control Block, NULL if none */
static const struct ccb *ccb_glob;
@@ -49,7 +56,7 @@
CBMEM_READY_HOOK(add_ccb_to_cbmem);
/* Get a pointer to the CCB. Returns NULL if not found */
-static struct ccb *locate_ccb(void)
+static struct ccb *locate_ccb(struct region_device *rdev)
{
struct ccb *ccb;
@@ -62,6 +69,7 @@
* entry so it is accessible through the post-RAM stages
* 3. (ramstage) The CCB is in CBMEM. It can be accessed directly.
*/
+ memset(rdev, '\0', sizeof(*rdev));
#if ENV_HOLDS_CCB
ccb = &ccb_static;
if (REGION_SIZE(ccb) < sizeof(*ccb))
@@ -79,18 +87,28 @@
printk(BIOS_ERR, "CCB: Not found in CBMEM\n");
} else if (CONFIG(CCB_CBFS)) {
struct prog ccb_file = PROG_INIT(PROG_CCB, "ccb");
- struct region_device rdev;
union cbfs_mdata mdata;
- if (_cbfs_boot_lookup(prog_name(&ccb_file), true, &mdata, &rdev)) {
+ if (_cbfs_boot_lookup(prog_name(&ccb_file), true, &mdata, rdev)) {
printk(BIOS_DEBUG, "CCB: No file in CBFS\n");
return NULL;
}
- if (region_device_sz(&rdev) != sizeof(struct ccb)) {
+ if (region_device_sz(rdev) != sizeof(struct ccb)) {
printk(BIOS_ERR, "CCB: Incorrect file size in CBFS\n");
return NULL;
}
- ccb = rdev_mmap_full(&rdev);
+ ccb = rdev_mmap_full(rdev);
+ } else if (CONFIG(CCB_FMAP)) {
+ if (fmap_locate_area_as_rdev(CCB_REGION, rdev)) {
+ printk(BIOS_ERR, "CCB: Not found in FMAP\n");
+ return NULL;
+ }
+ ccb = rdev_mmap_full(rdev);
+ if (!ccb) {
+ printk(BIOS_ERR, "CCB: Cannot map\n");
+ return NULL;
+ }
+ printk(BIOS_DEBUG, "CCB: Found in FMAP\n");
} else {
/* we cannot get here */
BUG();
@@ -102,24 +120,35 @@
void ccb_check(void)
{
+ struct region_device rdev;
struct ccb *ccb;
- ccb = locate_ccb();
- if (ccb)
+ ccb = locate_ccb(&rdev);
+ if (ccb) {
printk(BIOS_DEBUG, "CCB: ready\n");
+ if (rdev_valid(&rdev))
+ rdev_munmap(&rdev, ccb);
+ }
}
void ccb_init(void)
{
+ struct region_device rdev;
struct ccb *ccb;
- ccb = locate_ccb();
+ ccb = locate_ccb(&rdev);
if (ccb) {
#if ENV_BOOTBLOCK
/* Copy the CCB into the cache for use by romstage. In the event
* that CCB is missing, zero values will be used */
memcpy((void *)_ccb, ccb, sizeof(*ccb));
#endif
+
+ if (rdev_valid(&rdev)) {
+ ccb_holder = *ccb;
+ rdev_munmap(&rdev, ccb);
+ ccb = &ccb_holder;
+ }
ccb_glob = ccb;
}
}
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 987f71e..57fbc6b6 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -13,6 +13,7 @@
#include "cbfs_image.h"
#include "cbfs_sections.h"
#include "elfparsing.h"
+#include "fmap.h"
#include "partitioned_file.h"
#include "lz4/lib/xxhash.h"
#include <commonlib/bsd/cbfs_private.h>
@@ -676,13 +677,20 @@
return ret;
}
+/* location of CCB, with respect to the image regions (>0 is an error) */
+enum ccb_location {
+ CCB_IN_PRIMARY_CBFS = 0,
+ CCB_IN_OWN_REGION = -1,
+ CCB_NOT_FOUND = 1
+};
+
/**
* locate_ccb() - Locate the coreboot Control Block in the CBFS image
*
* This is located in the bootblock and has a special signature
*
* @ccbp: Returns a pointer to the CCB
- * Return: 0 if OK, 1 on error
+ * Return: 0 if OK, 1 on error, -1 if the CCB region must be written back
*/
static int locate_ccb(struct buffer *buffer, struct ccb **ccbp)
{
@@ -695,10 +703,12 @@
goto no_bootblock;
if (fmap_find_area(fmap, SECTION_NAME_BOOTBLOCK)) {
+ INFO("Reading bootblock\n");
if (!partitioned_file_read_region(buffer, param.image_file,
SECTION_NAME_BOOTBLOCK))
goto no_bootblock;
} else {
+ INFO("Reading CBFS\n");
if (!partitioned_file_read_region(buffer, param.image_file,
SECTION_NAME_PRIMARY_CBFS))
goto no_bootblock;
@@ -708,22 +718,37 @@
size = buffer_size(buffer);
for (ptr = (uint32_t *)data, end = (uint32_t *)(data + size); ptr < end;
- ptr++) {
+ ptr++) {
if (*ptr == CCB_MAGIC) {
*ccbp = (struct ccb *)ptr;
- return 0;
+ INFO("CCB at %p, from size %lx\n", ptr, size);
+ return CCB_IN_PRIMARY_CBFS;
}
}
INFO("CCB not in bootblock\n");
+ /* Now try FMAP */
+ const struct fmap_area *area;
+
+ area = fmap_find_area(fmap, CCB_REGION);
+ if (area) {
+ printk(BIOS_DEBUG, "CCB: Found in FMAP\n");
+ if (!partitioned_file_read_region(buffer, param.image_file,
+ CCB_REGION))
+ goto no_bootblock;
+
+ *ccbp = (void *)buffer_get(buffer); // + area->offset;
+ return CCB_IN_OWN_REGION;
+ }
+
struct cbfs_image image;
struct cbfs_file *ccb_file;
const char *filename = "ccb";
struct buffer ccb_buf;
if (cbfs_image_from_buffer(&image, param.image_region, param.headeroffset))
- return 1;
+ return CCB_NOT_FOUND;
ccb_file = cbfs_get_entry(&image, filename);
if (!ccb_file) {
@@ -734,7 +759,7 @@
INFO("CCB not in CBFS: creating\n");
if (buffer_create(&ccb_buf, sizeof(struct ccb), filename))
- return 1;
+ return CCB_NOT_FOUND;
header = cbfs_create_file_header(CBFS_TYPE_RAW, ccb_buf.size,
filename);
@@ -748,24 +773,24 @@
buffer_delete(&ccb_buf);
if (ret) {
ERROR("Failed to add '%s' into ROM image.\n", filename);
- return 1;
+ return CCB_NOT_FOUND;
}
ccb_file = cbfs_get_entry(&image, filename);
}
if (!ccb_file) {
ERROR("Cannot get file\n");
- return 1;
+ return CCB_NOT_FOUND;
}
/* Locate the CCB */
*ccbp = (void *)ccb_file + be32toh(ccb_file->offset);
- return 0;
+ return CCB_IN_PRIMARY_CBFS;
no_bootblock:
ERROR("CCB not in ROM image?!?\n");
- return 1;
+ return CCB_NOT_FOUND;
}
/**
@@ -777,11 +802,13 @@
*/
static int cbfs_ccb_set_value(unused const char *name, unused const char *value)
{
+ enum ccb_location ccb_loc;
struct buffer buffer;
struct ccb *ccb;
uint val;
- if (locate_ccb(&buffer, &ccb))
+ ccb_loc = locate_ccb(&buffer, &ccb);
+ if (ccb_loc > 0)
return 1;
/*
@@ -803,6 +830,12 @@
printf("%s=%s\n", name, value);
ccb->flags = val;
+ if (ccb_loc == CCB_IN_OWN_REGION) {
+ INFO("Performing operation on '%s' region...\n", CCB_REGION);
+ if (!partitioned_file_write_region(param.image_file, &buffer))
+ return 1;
+ }
+
return 0;
}
@@ -819,7 +852,7 @@
struct buffer buffer;
struct ccb *ccb;
- if (locate_ccb(&buffer, &ccb))
+ if (locate_ccb(&buffer, &ccb) > 0)
return 1;
/*
* For now this code is very simple as we only have one setting. We can
diff --git a/util/cbfstool/default-x86.fmd b/util/cbfstool/default-x86.fmd
index f008889..ec57c06 100644
--- a/util/cbfstool/default-x86.fmd
+++ b/util/cbfstool/default-x86.fmd
@@ -15,6 +15,7 @@
##SPD_CACHE_ENTRY##
##VPD_ENTRY##
##HSPHY_FW_ENTRY##
+ ##CCB_ENTRY##
FMAP@##FMAP_BASE## ##FMAP_SIZE##
COREBOOT(CBFS)@##CBFS_BASE## ##CBFS_SIZE##
}
To view, visit change 83293. To unsubscribe, or for help writing mail filters, visit settings.