[coreboot-gerrit] New patch to review for coreboot: dcef372 CBFS: Automate ROM image layout and remove hardcoded offsets

Patrick Georgi (pgeorgi@google.com) gerrit at coreboot.org
Mon Apr 13 21:21:15 CEST 2015


Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/9620

-gerrit

commit dcef372295dde7444d2b6e677ff101955dcc0674
Author: Julius Werner <jwerner at chromium.org>
Date:   Mon Nov 10 13:14:24 2014 -0800

    CBFS: Automate ROM image layout and remove hardcoded offsets
    
    Non-x86 boards currently need to hardcode the position of their CBFS
    master header in a Kconfig. This is very brittle because it is usually
    put in between the bootblock and the first CBFS entry, without any
    checks to guarantee that it won't overlap either of those. It is not fun
    to debug random failures that move and disappear with tiny alignment
    changes because someone decided to write "ORBC1112" over some part of
    your data section (in a way that is not visible in the symbolized .elf
    binaries, only in the final image). This patch seeks to prevent those
    issues and reduce the need for manual configuration by making the image
    layout a completely automated part of cbfstool.
    
    Since automated placement of the CBFS header means we can no longer
    hardcode its position into coreboot, this patch takes the existing x86
    solution of placing a pointer to the header at the very end of the
    CBFS-managed section of the ROM and generalizes it to all architectures.
    This is now even possible with the read-only/read-write split in
    ChromeOS, since coreboot knows how large that section is from the
    CBFS_SIZE Kconfig (which is by default equal to ROM_SIZE, but can be
    changed on systems that place other data next to coreboot/CBFS in ROM).
    
    Also adds a feature to cbfstool that makes the -B (bootblock file name)
    argument on image creation optional, since we have recently found valid
    use cases for CBFS images that are not the first boot medium of the
    device (instead opened by an earlier bootloader that can already
    interpret CBFS) and therefore don't really need a bootblock.
    
    BRANCH=None
    BUG=None
    TEST=Built and booted on Veyron_Pinky, Nyan_Blaze and Falco.
    
    Change-Id: Ib715bb8db258e602991b34f994750a2d3e2d5adf
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Original-Commit-Id: e9879c0fbd57f105254c54bacb3e592acdcad35c
    Original-Change-Id: Ifcc755326832755cfbccd6f0a12104cba28a20af
    Original-Signed-off-by: Julius Werner <jwerner at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/229975
---
 payloads/libpayload/include/cbfs_core.h    |   7 +-
 payloads/libpayload/libcbfs/cbfs.c         |  13 ----
 payloads/libpayload/libcbfs/cbfs_core.c    |  27 ++++---
 src/arch/arm/Makefile.inc                  |   4 +-
 src/arch/arm64/Makefile.inc                |   4 +-
 src/arch/mips/Makefile.inc                 |   4 +-
 src/cpu/mips/Kconfig                       |   5 --
 src/include/cbfs_serialized.h              |   7 +-
 src/lib/cbfs_core.c                        |  33 +++++---
 src/lib/cbfs_core.h                        |   7 --
 src/mainboard/emulation/qemu-armv7/Kconfig |  12 ---
 src/soc/imgtec/pistachio/Kconfig           |   9 ---
 src/soc/marvell/bg4cd/Kconfig              |  12 ---
 src/soc/nvidia/tegra124/Kconfig            |  12 ---
 src/soc/nvidia/tegra132/Kconfig            |  12 ---
 src/soc/qualcomm/ipq806x/Kconfig           |  12 ---
 src/soc/rockchip/rk3288/Kconfig            |  18 -----
 src/soc/samsung/exynos5250/Kconfig         |  25 ------
 src/soc/samsung/exynos5420/Kconfig         |  25 ------
 util/cbfstool/cbfs_image.c                 |  54 +++++++------
 util/cbfstool/cbfstool.c                   | 120 +++++++++++++++++------------
 util/cbfstool/common.h                     |   7 ++
 22 files changed, 152 insertions(+), 277 deletions(-)

diff --git a/payloads/libpayload/include/cbfs_core.h b/payloads/libpayload/include/cbfs_core.h
index 42f2dc8..4599524 100644
--- a/payloads/libpayload/include/cbfs_core.h
+++ b/payloads/libpayload/include/cbfs_core.h
@@ -85,10 +85,9 @@
 
 #define CBFS_HEADER_INVALID_ADDRESS	((void*)(0xffffffff))
 
-/** this is the master cbfs header - it needs to be located somewhere available
-    to bootblock (to load romstage).  Where it actually lives is up to coreboot.
-    On x86, a pointer to this header will live at 0xFFFFFFFC.
-    For other platforms, you need to define CONFIG_LP_CBFS_HEADER_ROM_OFFSET */
+/* this is the master cbfs header - it must be located somewhere available
+ * to bootblock (to load romstage). The last 4 bytes in the image contain its
+ * relative offset from the end of the image (as a 32-bit signed integer). */
 
 struct cbfs_header {
 	uint32_t magic;
diff --git a/payloads/libpayload/libcbfs/cbfs.c b/payloads/libpayload/libcbfs/cbfs.c
index 5ab7ffa..b2b9d61 100644
--- a/payloads/libpayload/libcbfs/cbfs.c
+++ b/payloads/libpayload/libcbfs/cbfs.c
@@ -62,19 +62,6 @@
 # endif
 #endif
 
-#if defined(CONFIG_LP_CBFS_HEADER_ROM_OFFSET) && (CONFIG_LP_CBFS_HEADER_ROM_OFFSET)
-# define CBFS_HEADER_ROM_ADDRESS (CONFIG_LP_CBFS_HEADER_ROM_OFFSET)
-#else
-/* ugly hack: this assumes that "media" exists
-              in the scope where the macro is used. */
-static uint32_t fetch_x86_header(struct cbfs_media *media)
-{
-	uint32_t *header_ptr = media->map(media, 0xfffffffc, 4);
-	return *header_ptr;
-}
-# define CBFS_HEADER_ROM_ADDRESS fetch_x86_header(media)
-#endif
-
 #include "cbfs_core.c"
 
 #ifndef __SMM__
diff --git a/payloads/libpayload/libcbfs/cbfs_core.c b/payloads/libpayload/libcbfs/cbfs_core.c
index fb15744..eb07e2d 100644
--- a/payloads/libpayload/libcbfs/cbfs_core.c
+++ b/payloads/libpayload/libcbfs/cbfs_core.c
@@ -34,10 +34,6 @@
  * CBFS_CORE_WITH_LZMA (must be #define)
  *      if defined, ulzma() must exist for decompression of data streams
  *
- * CBFS_HEADER_ROM_ADDRESS
- *	ROM address (offset) of CBFS header. Underlying CBFS media may interpret
- *	it in other way so we call this "address".
- *
  * ERROR(x...)
  *      print an error message x (in printf format)
  *
@@ -56,6 +52,7 @@
  *  on failure */
 const struct cbfs_header *cbfs_get_header(struct cbfs_media *media)
 {
+	int32_t rel_offset;
 	const struct cbfs_header *header;
 	struct cbfs_media default_media;
 
@@ -66,22 +63,28 @@ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media)
 			return CBFS_HEADER_INVALID_ADDRESS;
 		}
 	}
-
 	media->open(media);
-	DEBUG("CBFS_HEADER_ROM_ADDRESS: 0x%x/0x%x\n", CBFS_HEADER_ROM_ADDRESS,
-	      CONFIG_LP_ROM_SIZE);
-	header = media->map(media, CBFS_HEADER_ROM_ADDRESS, sizeof(*header));
+
+	if (!media->read(media, &rel_offset, (size_t)(0 - sizeof(int32_t)),
+			 sizeof(int32_t))) {
+		ERROR("Could not read CBFS master header offset!\n");
+		return CBFS_HEADER_INVALID_ADDRESS;
+	}
+	header = media->map(media, (size_t)rel_offset, sizeof(*header));
+	DEBUG("CBFS header at %#zx (-%#zx from end of image).\n",
+		(size_t)rel_offset, (size_t)-rel_offset);
 	media->close(media);
 
 	if (header == CBFS_MEDIA_INVALID_MAP_ADDRESS) {
-		ERROR("Failed to load CBFS header from 0x%x\n",
-		      CBFS_HEADER_ROM_ADDRESS);
+		ERROR("Failed to load CBFS header from %#zx(-%#zx)\n",
+			(size_t)rel_offset, (size_t)-rel_offset);
 		return CBFS_HEADER_INVALID_ADDRESS;
 	}
 
 	if (CBFS_HEADER_MAGIC != ntohl(header->magic)) {
-		ERROR("Could not find valid CBFS master header at %x: "
-		      "%x vs %x.\n", CBFS_HEADER_ROM_ADDRESS, CBFS_HEADER_MAGIC,
+		ERROR("Could not find valid CBFS master header at %#zx(-%#zx): "
+		      "magic %#.8x vs %#.8x.\n", (size_t)rel_offset,
+		      (size_t)-rel_offset, CBFS_HEADER_MAGIC,
 		      ntohl(header->magic));
 		if (header->magic == 0xffffffff) {
 			ERROR("Maybe ROM is not mapped properly?\n");
diff --git a/src/arch/arm/Makefile.inc b/src/arch/arm/Makefile.inc
index cfdb576..4e23f84 100644
--- a/src/arch/arm/Makefile.inc
+++ b/src/arch/arm/Makefile.inc
@@ -31,9 +31,7 @@ subdirs-y += armv4/ armv7/
 ###############################################################################
 
 ifeq ($(CONFIG_ARCH_ROMSTAGE_ARM),y)
-CBFSTOOL_PRE1_OPTS = -m arm -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) \
-		     -H $(CONFIG_CBFS_HEADER_ROM_OFFSET) \
-		     -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
+CBFSTOOL_PRE1_OPTS = -m arm -s $(CONFIG_CBFS_SIZE)
 CBFSTOOL_PRE_OPTS = -b 0
 endif
 
diff --git a/src/arch/arm64/Makefile.inc b/src/arch/arm64/Makefile.inc
index 8e88d9c..c9c93b1 100644
--- a/src/arch/arm64/Makefile.inc
+++ b/src/arch/arm64/Makefile.inc
@@ -34,9 +34,7 @@ subdirs-y += armv8/
 ################################################################################
 
 ifeq ($(CONFIG_ARCH_ROMSTAGE_ARM64),y)
-CBFSTOOL_PRE1_OPTS = -m arm64 -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) \
-		     -H $(CONFIG_CBFS_HEADER_ROM_OFFSET) \
-		     -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
+CBFSTOOL_PRE1_OPTS = -m arm64 -s $(CONFIG_CBFS_SIZE)
 endif
 
 ifeq ($(CONFIG_ARCH_ARM64),y)
diff --git a/src/arch/mips/Makefile.inc b/src/arch/mips/Makefile.inc
index ee88adf..1016f1f 100644
--- a/src/arch/mips/Makefile.inc
+++ b/src/arch/mips/Makefile.inc
@@ -24,9 +24,7 @@
 ###############################################################################
 
 ifeq ($(CONFIG_ARCH_ROMSTAGE_MIPS),y)
-CBFSTOOL_PRE1_OPTS = -m mips -b $(CONFIG_BOOTBLOCK_ROM_OFFSET) \
-		     -H $(CONFIG_CBFS_HEADER_ROM_OFFSET) \
-		     -o $(CONFIG_CBFS_ROM_OFFSET) -s $(CONFIG_CBFS_SIZE)
+CBFSTOOL_PRE1_OPTS = -m mips -s $(CONFIG_CBFS_SIZE)
 endif
 
 ###############################################################################
diff --git a/src/cpu/mips/Kconfig b/src/cpu/mips/Kconfig
index 2230c14..85410bb 100644
--- a/src/cpu/mips/Kconfig
+++ b/src/cpu/mips/Kconfig
@@ -25,8 +25,3 @@ config CPU_MIPS
 	select ARCH_VERSTAGE_MIPS
 	select ARCH_ROMSTAGE_MIPS
 	select ARCH_RAMSTAGE_MIPS
-
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	depends on CPU_MIPS
-	default 0x00
diff --git a/src/include/cbfs_serialized.h b/src/include/cbfs_serialized.h
index 30b2314..95dcb1a 100644
--- a/src/include/cbfs_serialized.h
+++ b/src/include/cbfs_serialized.h
@@ -86,10 +86,9 @@
 #define CBFS_HEADER_VERSION2 0x31313132
 #define CBFS_HEADER_VERSION  CBFS_HEADER_VERSION2
 
-/** this is the master cbfs header - it need to be located somewhere available
-    to bootblock (to load romstage).  Where it actually lives is up to coreboot.
-    On x86, a pointer to this header will live at 0xFFFFFFFC.
-    For other platforms, you need to define CONFIG_CBFS_HEADER_ROM_OFFSET */
+/* this is the master cbfs header - it must be located somewhere available
+ * to bootblock (to load romstage). The last 4 bytes in the image contain its
+ * relative offset from the end of the image (as a 32-bit signed integer). */
 
 struct cbfs_header {
 	uint32_t magic;
diff --git a/src/lib/cbfs_core.c b/src/lib/cbfs_core.c
index acd434f..80d66ca 100644
--- a/src/lib/cbfs_core.c
+++ b/src/lib/cbfs_core.c
@@ -34,10 +34,6 @@
  * CBFS_CORE_WITH_LZMA (must be #define)
  *      if defined, ulzma() must exist for decompression of data streams
  *
- * CBFS_HEADER_ROM_ADDRESS
- *	ROM address (offset) of CBFS header. Underlying CBFS media may interpret
- *	it in other way so we call this "address".
- *
  * ERROR(x...)
  *      print an error message x (in printf format)
  *
@@ -58,6 +54,7 @@
  *  on failure */
 const struct cbfs_header *cbfs_get_header(struct cbfs_media *media)
 {
+	size_t offset;
 	const struct cbfs_header *header;
 	struct cbfs_media default_media;
 
@@ -68,22 +65,34 @@ const struct cbfs_header *cbfs_get_header(struct cbfs_media *media)
 			return CBFS_HEADER_INVALID_ADDRESS;
 		}
 	}
-
 	media->open(media);
-	DEBUG("CBFS_HEADER_ROM_ADDRESS: 0x%x/0x%x\n", CBFS_HEADER_ROM_ADDRESS,
-	      CONFIG_ROM_SIZE);
-	header = media->map(media, CBFS_HEADER_ROM_ADDRESS, sizeof(*header));
+
+	/* TODO: allow negative offsets from the end of the CBFS image at media
+	 * layer (like libpayload) so we can combine these two cases. */
+	if (IS_ENABLED(CONFIG_ARCH_X86)) {
+		offset = *(int32_t *)(uintptr_t)0xfffffffc;
+		header = media->map(media, offset, sizeof(*header));
+	} else {
+		int32_t rel_offset;
+		if (!media->read(media, &rel_offset, CONFIG_CBFS_SIZE -
+				 sizeof(int32_t), sizeof(int32_t))) {
+			ERROR("Could not read CBFS master header offset!\n");
+			return CBFS_HEADER_INVALID_ADDRESS;
+		}
+		offset = CONFIG_CBFS_SIZE + rel_offset;
+		header = media->map(media, offset, sizeof(*header));
+	}
+	DEBUG("CBFS header offset: 0x%zx/0x%x\n", offset, CONFIG_ROM_SIZE);
 	media->close(media);
 
 	if (header == CBFS_MEDIA_INVALID_MAP_ADDRESS) {
-		ERROR("Failed to load CBFS header from 0x%x\n",
-		      CBFS_HEADER_ROM_ADDRESS);
+		ERROR("Failed to load CBFS header from 0x%zx\n", offset);
 		return CBFS_HEADER_INVALID_ADDRESS;
 	}
 
 	if (CBFS_HEADER_MAGIC != ntohl(header->magic)) {
-		ERROR("Could not find valid CBFS master header at %x: "
-		      "%x vs %x.\n", CBFS_HEADER_ROM_ADDRESS, CBFS_HEADER_MAGIC,
+		ERROR("Could not find valid CBFS master header at %#zx: "
+		      "magic %#.8x vs %#.8x.\n", offset, CBFS_HEADER_MAGIC,
 		      ntohl(header->magic));
 		if (header->magic == 0xffffffff) {
 			ERROR("Maybe ROM is not mapped properly?\n");
diff --git a/src/lib/cbfs_core.h b/src/lib/cbfs_core.h
index 221fe5d..c301c33 100644
--- a/src/lib/cbfs_core.h
+++ b/src/lib/cbfs_core.h
@@ -42,11 +42,4 @@
 # endif
 #endif
 
-#if defined(CONFIG_CBFS_HEADER_ROM_OFFSET) && (CONFIG_CBFS_HEADER_ROM_OFFSET)
-# define CBFS_HEADER_ROM_ADDRESS (CONFIG_CBFS_HEADER_ROM_OFFSET)
-#else
-// Indirect address: only works on 32bit top-aligned systems.
-# define CBFS_HEADER_ROM_ADDRESS (*(uint32_t *)0xfffffffc)
-#endif
-
 #endif /* __LIB_CBFS_CORE */
diff --git a/src/mainboard/emulation/qemu-armv7/Kconfig b/src/mainboard/emulation/qemu-armv7/Kconfig
index c0e7095..58ddd4e 100644
--- a/src/mainboard/emulation/qemu-armv7/Kconfig
+++ b/src/mainboard/emulation/qemu-armv7/Kconfig
@@ -53,16 +53,4 @@ config DRAM_SIZE_MB
 	int
 	default 1024
 
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0x0
-
-config CBFS_HEADER_ROM_OFFSET
-	hex
-	default 0x0100000
-
-config CBFS_ROM_OFFSET
-	hex
-	default 0x0110000
-
 endif #  BOARD_EMULATION_QEMU_ARMV7
diff --git a/src/soc/imgtec/pistachio/Kconfig b/src/soc/imgtec/pistachio/Kconfig
index 4671444..9eb001d 100644
--- a/src/soc/imgtec/pistachio/Kconfig
+++ b/src/soc/imgtec/pistachio/Kconfig
@@ -35,13 +35,4 @@ config BOOTBLOCK_CPU_INIT
 	string
 	default "soc/imgtec/pistachio/bootblock.c"
 
-config CBFS_ROM_OFFSET
-	hex
-	default 0x8100
-
-config CBFS_HEADER_ROM_OFFSET
-	# Effectively the maximum size of the bootblock
-	hex
-	default 0x8000
-
 endif
diff --git a/src/soc/marvell/bg4cd/Kconfig b/src/soc/marvell/bg4cd/Kconfig
index 1d51ebd..d7f8327 100644
--- a/src/soc/marvell/bg4cd/Kconfig
+++ b/src/soc/marvell/bg4cd/Kconfig
@@ -37,16 +37,4 @@ config BOOTBLOCK_CPU_INIT
 	string
 	default "soc/marvell/bg4cd/bootblock.c"
 
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0x0
-
-config CBFS_HEADER_ROM_OFFSET
-	hex
-	default 0x0008000
-
-config CBFS_ROM_OFFSET
-	hex
-	default 0x0018000
-
 endif
diff --git a/src/soc/nvidia/tegra124/Kconfig b/src/soc/nvidia/tegra124/Kconfig
index 3ce368b..b934f4e 100644
--- a/src/soc/nvidia/tegra124/Kconfig
+++ b/src/soc/nvidia/tegra124/Kconfig
@@ -24,18 +24,6 @@ config BOOTBLOCK_CPU_INIT
 	  bootblock must load microcode or copy data from ROM before
 	  searching for the bootblock.
 
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0x0
-
-config CBFS_HEADER_ROM_OFFSET
-	hex "offset of master CBFS header in ROM"
-	default 0x18000
-
-config CBFS_ROM_OFFSET
-	hex "offset of CBFS data in ROM"
-	default 0x18080
-
 config TEGRA124_MODEL_TD570D
 	bool "TD570D"
 
diff --git a/src/soc/nvidia/tegra132/Kconfig b/src/soc/nvidia/tegra132/Kconfig
index 0870c7e..4c927a8 100644
--- a/src/soc/nvidia/tegra132/Kconfig
+++ b/src/soc/nvidia/tegra132/Kconfig
@@ -42,22 +42,10 @@ config BOOTBLOCK_CPU_INIT
 	  bootblock must load microcode or copy data from ROM before
 	  searching for the bootblock.
 
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0x0
-
 config MAX_CPUS
 	int
 	default 2
 
-config CBFS_HEADER_ROM_OFFSET
-	hex "offset of master CBFS header in ROM"
-	default 0x40000
-
-config CBFS_ROM_OFFSET
-	hex "offset of CBFS data in ROM"
-	default 0x40080
-
 config MTS_DIRECTORY
 	string "Directory where MTS microcode files are located"
 	default "3rdparty/cpu/nvidia/tegra132/current/prod"
diff --git a/src/soc/qualcomm/ipq806x/Kconfig b/src/soc/qualcomm/ipq806x/Kconfig
index 8cc7233..29153ca 100644
--- a/src/soc/qualcomm/ipq806x/Kconfig
+++ b/src/soc/qualcomm/ipq806x/Kconfig
@@ -21,18 +21,6 @@ config CBFS_SIZE
 	  coreboot blob elsewhere in the system. Make sure this config option
 	  is fine tuned in the board config file.
 
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0x0
-
-config CBFS_HEADER_ROM_OFFSET
-	hex "offset of master CBFS header in ROM"
-	default 0x1b4000
-
-config CBFS_ROM_OFFSET
-	hex "offset of CBFS data in ROM"
-	default 0x1b4080
-
 config MBN_ENCAPSULATION
 	depends on USE_BLOBS
 	bool "bootblock encapsulation for ipq8064"
diff --git a/src/soc/rockchip/rk3288/Kconfig b/src/soc/rockchip/rk3288/Kconfig
index ada3d58..95113f3 100644
--- a/src/soc/rockchip/rk3288/Kconfig
+++ b/src/soc/rockchip/rk3288/Kconfig
@@ -37,22 +37,4 @@ config BOOTBLOCK_CPU_INIT
 	string
 	default "soc/rockchip/rk3288/bootblock.c"
 
-# ROM image layout.
-#
-# 0x00000 Combined bootblock and ID Block
-# 0x08000 Master CBFS header.
-# 0x18000 Free for CBFS data.
-
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0x0
-
-config CBFS_HEADER_ROM_OFFSET
-	hex
-	default 0x0010000
-
-config CBFS_ROM_OFFSET
-	hex
-	default 0x0010100
-
 endif
diff --git a/src/soc/samsung/exynos5250/Kconfig b/src/soc/samsung/exynos5250/Kconfig
index 7034e96..ea63e9c 100644
--- a/src/soc/samsung/exynos5250/Kconfig
+++ b/src/soc/samsung/exynos5250/Kconfig
@@ -9,28 +9,3 @@ config CPU_SAMSUNG_EXYNOS5250
 	select HAVE_UART_SPECIAL
 	bool
 	default n
-
-if CPU_SAMSUNG_EXYNOS5250
-
-# ROM image layout.
-#
-# 0x0000: vendor-provided BL1 (8k).
-# 0x2000: bootblock
-# 0x9FFC-0xA000: BL2 checksum
-# 0xA000-0xA080: reserved for CBFS master header.
-# 0xA080: Free for CBFS data.
-
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0
-
-config CBFS_HEADER_ROM_OFFSET
-	hex "offset of master CBFS header in ROM"
-	default 0x9F80
-
-config CBFS_ROM_OFFSET
-	# Calculated by BOOTBLOCK_ROM_OFFSET + max bootblock size.
-	hex "offset of CBFS data in ROM"
-	default 0x0A080
-
-endif
diff --git a/src/soc/samsung/exynos5420/Kconfig b/src/soc/samsung/exynos5420/Kconfig
index 072976a..56ffed1 100644
--- a/src/soc/samsung/exynos5420/Kconfig
+++ b/src/soc/samsung/exynos5420/Kconfig
@@ -10,28 +10,3 @@ config CPU_SAMSUNG_EXYNOS5420
 	select RELOCATABLE_MODULES
 	bool
 	default n
-
-if CPU_SAMSUNG_EXYNOS5420
-
-# ROM image layout.
-#
-# 0x0000: vendor-provided BL1 (8k).
-# 0x2000: variable length bootblock checksum header
-# 0x2010: bootblock
-#  0x9F80-0xA000: reserved for CBFS master header.
-# 0xA000: Free for CBFS data.
-
-config BOOTBLOCK_ROM_OFFSET
-	hex
-	default 0
-
-config CBFS_HEADER_ROM_OFFSET
-	hex "offset of master CBFS header in ROM"
-	default 0x9F80
-
-config CBFS_ROM_OFFSET
-	# Calculated by BOOTBLOCK_ROM_OFFSET + max bootblock size.
-	hex "offset of CBFS data in ROM"
-	default 0x0A000
-
-endif
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index 1f60d68..0230d80 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -66,13 +66,6 @@ static const struct typedesc_t types_cbfs_compression[] = {
 	{0, NULL},
 };
 
-static uint32_t align_up(uint32_t value, uint32_t align)
-{
-	if (value % align)
-		value += align - (value % align);
-	return value;
-}
-
 static const char *lookup_name_by_type(const struct typedesc_t *desc, uint32_t type,
 				const char *default_value)
 {
@@ -180,6 +173,7 @@ int cbfs_image_create(struct cbfs_image *image,
 {
 	struct cbfs_header header;
 	struct cbfs_file *entry;
+	int32_t *rel_offset;
 	uint32_t cbfs_len;
 	size_t entry_header_len;
 	void *header_loc;
@@ -207,9 +201,6 @@ int cbfs_image_create(struct cbfs_image *image,
 	      "header=0x%x, entries_offset=0x%x\n",
 	      bootblock_offset, header_offset, entries_offset);
 
-	if (align == 0)
-		align = 64;  // default align size.
-
 	// Prepare bootblock
 	if (bootblock_offset + bootblock->size > size) {
 		ERROR("Bootblock (0x%x+0x%zx) exceed ROM size (0x%zx)\n",
@@ -226,7 +217,7 @@ int cbfs_image_create(struct cbfs_image *image,
 	       bootblock->size);
 
 	// Prepare header
-	if (header_offset + sizeof(header) > size) {
+	if (header_offset + sizeof(header) > size - sizeof(int32_t)) {
 		ERROR("Header (0x%x+0x%zx) exceed ROM size (0x%zx)\n",
 		      header_offset, sizeof(header), size);
 		return -1;
@@ -242,6 +233,14 @@ int cbfs_image_create(struct cbfs_image *image,
 	header_loc = (image->buffer.data + header_offset);
 	cbfs_put_header(header_loc, image->header);
 
+	// The last 4 byte of the image contain the relative offset from the end
+	// of the image to the master header as a 32-bit signed integer. x86
+	// relies on this also being its (memory-mapped, top-aligned) absolute
+	// 32-bit address by virtue of how two's complement numbers work.
+	assert(size % sizeof(int32_t) == 0);
+	rel_offset = (int32_t *)(image->buffer.data + size - sizeof(int32_t));
+	*rel_offset = header_offset - size;
+
 	// Prepare entries
 	if (align_up(entries_offset, align) != entries_offset) {
 		ERROR("Offset (0x%x) must be aligned to 0x%x.\n",
@@ -256,8 +255,8 @@ int cbfs_image_create(struct cbfs_image *image,
 	}
 	entry = (struct cbfs_file *)(image->buffer.data + entries_offset);
 	// To calculate available length, find
-	//   e = min(bootblock, header, size) where e > entries_offset.
-	cbfs_len = size;
+	//   e = min(bootblock, header, rel_offset) where e > entries_offset.
+	cbfs_len = size - sizeof(int32_t);
 	if (bootblock_offset > entries_offset && bootblock_offset < cbfs_len)
 		cbfs_len = bootblock_offset;
 	if (header_offset > entries_offset && header_offset < cbfs_len)
@@ -772,17 +771,21 @@ struct cbfs_header *cbfs_find_header(char *data, size_t size)
 {
 	size_t offset;
 	int found = 0;
-	uint32_t x86sig;
+	int32_t rel_offset;
 	struct cbfs_header *header, *result = NULL;
 
-	// Try x86 style (check signature in bottom) header first.
-	x86sig = *(uint32_t *)(data + size - sizeof(uint32_t));
-	offset = (x86sig + (uint32_t)size);
-	DEBUG("x86sig: 0x%x, offset: 0x%zx\n", x86sig, offset);
+	// Try finding relative offset of master header at end of file first.
+	rel_offset = *(int32_t *)(data + size - sizeof(int32_t));
+	offset = size + rel_offset;
+	DEBUG("relative offset: %#zx(-%#zx), offset: %#zx\n",
+	      (size_t)rel_offset, (size_t)-rel_offset, offset);
 	if (offset >= size - sizeof(*header) ||
 	    ntohl(((struct cbfs_header *)(data + offset))->magic) !=
-	    CBFS_HEADER_MAGIC)
+	    CBFS_HEADER_MAGIC) {
+		// Some use cases append non-CBFS data to the end of the ROM.
+		DEBUG("relative offset seems wrong, scanning whole image...\n");
 		offset = 0;
+	}
 
 	for (; offset + sizeof(*header) < size; offset++) {
 		header = (struct cbfs_header *)(data + offset);
@@ -793,14 +796,15 @@ struct cbfs_header *cbfs_find_header(char *data, size_t size)
 			// Probably not a real CBFS header?
 			continue;
 		}
-		found++;
-		result = header;
+		if (!found++)
+			result = header;
 	}
-	if (found > 1) {
-		ERROR("multiple (%d) CBFS headers found!\n",
+	if (found > 1)
+		// Top-aligned images usually have a working relative offset
+		// field, so this is more likely to happen on bottom-aligned
+		// ones (where the first header is the "outermost" one)
+		WARN("Multiple (%d) CBFS headers found, using the first one.\n",
 		       found);
-		result = NULL;
-	}
 	return result;
 }
 
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index f57f288..9c61119 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -53,7 +53,8 @@ static struct param {
 	uint32_t size;
 	uint32_t alignment;
 	uint32_t pagesize;
-	uint32_t offset;
+	uint32_t cbfsoffset;
+	uint32_t cbfsoffset_assigned;
 	uint32_t top_aligned;
 	uint32_t arch;
 	int fit_empty_entries;
@@ -339,40 +340,58 @@ static int cbfs_create(void)
 		return 1;
 	}
 
-	if (!param.bootblock) {
-		ERROR("You need to specify -B/--bootblock.\n");
-		return 1;
-	}
-
 	if (param.arch == CBFS_ARCHITECTURE_UNKNOWN) {
 		ERROR("You need to specify -m/--machine arch.\n");
 		return 1;
 	}
 
-	if (buffer_from_file(&bootblock, param.bootblock) != 0) {
+	if (!param.bootblock) {
+		DEBUG("-B not given, creating image without bootblock.\n");
+		buffer_create(&bootblock, 0, "(dummy)");
+	} else if (buffer_from_file(&bootblock, param.bootblock)) {
 		return 1;
 	}
 
-	// Setup default boot offset and header offset.
+	if (!param.alignment)
+		param.alignment = 64;	// default CBFS entry alignment
+
+	// Set default offsets. x86, as usual, needs to be a special snowflake.
 	if (!param.baseaddress_assigned) {
-		// put boot block before end of ROM.
-		param.baseaddress = param.size - bootblock.size;
-		DEBUG("bootblock in end of ROM.\n");
+		if (param.arch == CBFS_ARCHITECTURE_X86) {
+			// Make sure there's at least enough room for rel_offset
+			param.baseaddress = param.size - (
+				bootblock.size > sizeof(int32_t) ?
+				bootblock.size : sizeof(int32_t));
+			DEBUG("x86 -> bootblock lies at end of ROM (%#x).\n",
+			      param.baseaddress);
+		} else {
+			param.baseaddress = 0;
+			DEBUG("bootblock starts at address 0x0.\n");
+		}
 	}
 	if (!param.headeroffset_assigned) {
-		// Put header before bootblock, and make a reference in end of
-		// bootblock.
-		param.headeroffset = (
-				param.baseaddress -
-				sizeof(struct cbfs_header));
-		if (bootblock.size >= sizeof(uint32_t)) {
-			// TODO this only works for 32b top-aligned system now...
-			uint32_t ptr = param.headeroffset - param.size;
-			uint32_t *sig = (uint32_t *)(bootblock.data +
-						     bootblock.size -
-						     sizeof(ptr));
-			*sig = ptr;
-			DEBUG("CBFS header reference in end of bootblock.\n");
+		if (param.arch == CBFS_ARCHITECTURE_X86) {
+			param.headeroffset = param.baseaddress -
+					     sizeof(struct cbfs_header);
+			DEBUG("x86 -> CBFS header before bootblock (%#x).\n",
+				param.headeroffset);
+		} else {
+			param.headeroffset = align_up(param.baseaddress +
+				bootblock.size, sizeof(uint32_t));
+			DEBUG("CBFS header placed behind bootblock (%#x).\n",
+				param.headeroffset);
+		}
+	}
+	if (!param.cbfsoffset_assigned) {
+		if (param.arch == CBFS_ARCHITECTURE_X86) {
+			param.cbfsoffset = 0;
+			DEBUG("x86 -> CBFS entries start at address 0x0.\n");
+		} else {
+			param.cbfsoffset = align_up(param.headeroffset +
+						    sizeof(struct cbfs_header),
+						    param.alignment);
+			DEBUG("CBFS entries start beind master header (%#x).\n",
+			      param.cbfsoffset);
 		}
 	}
 
@@ -383,7 +402,7 @@ static int cbfs_create(void)
 			      &bootblock,
 			      param.baseaddress,
 			      param.headeroffset,
-			      param.offset) != 0) {
+			      param.cbfsoffset) != 0) {
 		ERROR("Failed to create %s.\n", param.cbfs_name);
 		return 1;
 	}
@@ -533,28 +552,29 @@ static const struct command commands[] = {
 };
 
 static struct option long_options[] = {
-	{"name",         required_argument, 0, 'n' },
-	{"type",         required_argument, 0, 't' },
-	{"compression",  required_argument, 0, 'c' },
-	{"base-address", required_argument, 0, 'b' },
-	{"load-address", required_argument, 0, 'l' },
-	{"top-aligned",  required_argument, 0, 'T' },
-	{"entry-point",  required_argument, 0, 'e' },
-	{"size",         required_argument, 0, 's' },
-	{"bootblock",    required_argument, 0, 'B' },
-	{"alignment",    required_argument, 0, 'a' },
-	{"page-size",    required_argument, 0, 'P' },
-	{"offset",       required_argument, 0, 'o' },
-	{"file",         required_argument, 0, 'f' },
-	{"int",          required_argument, 0, 'i' },
-	{"machine",      required_argument, 0, 'm' },
-	{"empty-fits",   required_argument, 0, 'x' },
-	{"initrd",       required_argument, 0, 'I' },
-	{"cmdline",      required_argument, 0, 'C' },
-	{"ignore-sec",   required_argument, 0, 'S' },
-	{"verbose",      no_argument,       0, 'v' },
-	{"help",         no_argument,       0, 'h' },
-	{NULL,           0,                 0,  0  }
+	{"name",          required_argument, 0, 'n' },
+	{"type",          required_argument, 0, 't' },
+	{"compression",   required_argument, 0, 'c' },
+	{"base-address",  required_argument, 0, 'b' },
+	{"load-address",  required_argument, 0, 'l' },
+	{"top-aligned",   required_argument, 0, 'T' },
+	{"entry-point",   required_argument, 0, 'e' },
+	{"size",          required_argument, 0, 's' },
+	{"bootblock",     required_argument, 0, 'B' },
+	{"header-offset", required_argument, 0, 'H' },
+	{"alignment",     required_argument, 0, 'a' },
+	{"page-size",     required_argument, 0, 'P' },
+	{"offset",        required_argument, 0, 'o' },
+	{"file",          required_argument, 0, 'f' },
+	{"int",           required_argument, 0, 'i' },
+	{"machine",       required_argument, 0, 'm' },
+	{"empty-fits",    required_argument, 0, 'x' },
+	{"initrd",        required_argument, 0, 'I' },
+	{"cmdline",       required_argument, 0, 'C' },
+	{"ignore-sec",    required_argument, 0, 'S' },
+	{"verbose",       no_argument,       0, 'v' },
+	{"help",          no_argument,       0, 'h' },
+	{NULL,            0,                 0,  0  }
 };
 
 static void usage(char *name)
@@ -582,7 +602,8 @@ static void usage(char *name)
 			"Add a raw 64-bit integer value\n"
 	     " remove -n NAME                                              "
 			"Remove a component\n"
-	     " create -s size -B bootblock -m ARCH [-a align] [-o offset]  "
+	     " create -s size -m ARCH [-B bootblock] [-b bootblock offset] \\\n"
+	     "        [-o CBFS offset] [-H header offset] [-a align]       "
 			"Create a ROM file\n"
 	     " locate -f FILE -n NAME [-P page-size] [-a align] [-T]       "
 			"Find a place for a file of that size\n"
@@ -694,7 +715,8 @@ int main(int argc, char **argv)
 				param.pagesize = strtoul(optarg, NULL, 0);
 				break;
 			case 'o':
-				param.offset = strtoul(optarg, NULL, 0);
+				param.cbfsoffset = strtoul(optarg, NULL, 0);
+				param.cbfsoffset_assigned = 1;
 				break;
 			case 'f':
 				param.filename = optarg;
diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h
index 02a1076..0284742 100644
--- a/util/cbfstool/common.h
+++ b/util/cbfstool/common.h
@@ -47,6 +47,13 @@ extern int verbose;
 
 #define unused __attribute__((unused))
 
+static inline uint32_t align_up(uint32_t value, uint32_t align)
+{
+	if (value % align)
+		value += align - (value % align);
+	return value;
+}
+
 /* Buffer and file I/O */
 struct buffer {
 	char *name;



More information about the coreboot-gerrit mailing list