Joel Kitching has uploaded this change for review.

View Change

vboot: standardize on working data size

Previously, the size of memory made for vb2_working_data
through the macro VBOOT2_WORK was always specified
in each individual memlayout file. However, there is
effectively no reason to provide this customizability --
the workbuf size required for verifying firmware has
never been more than 12K. (This could potentially
increase in the future if key sizes or algorithms are
changed, but this could be applied globally rather than
for each individual platform.)

This CL creates a new Kconfig option called
VBOOT_WORKING_DATA_SIZE, which is tied to the value
provided by vboot2 as VB2_WORKBUF_RECOMMENDED_DATA_SIZE
via a _Static_assert in vboot_loader.c

BUG=b:124141368, b:124192753
TEST=Build locally for eve
TEST=util/lint/checkpatch.pl -g origin/master..HEAD
TEST=util/abuild/abuild -B -e -y -c 50 -p none -x
TEST=make clean && make test-abuild
BRANCH=none

Change-Id: Id71a8ab2401efcc0194d48c8af9017fc90513cb8
Signed-off-by: Joel Kitching <kitching@google.com>
---
M src/arch/x86/car.ld
M src/include/memlayout.h
M src/security/vboot/Kconfig
M src/security/vboot/Makefile.inc
M src/security/vboot/common.c
M src/security/vboot/vboot_loader.c
M src/soc/cavium/cn81xx/include/soc/memlayout.ld
M src/soc/imgtec/pistachio/include/soc/memlayout.ld
M src/soc/mediatek/mt8173/include/soc/memlayout.ld
M src/soc/mediatek/mt8183/include/soc/memlayout.ld
M src/soc/nvidia/tegra124/include/soc/memlayout.ld
M src/soc/nvidia/tegra210/include/soc/memlayout.ld
M src/soc/qualcomm/ipq40xx/include/soc/memlayout.ld
M src/soc/qualcomm/ipq806x/include/soc/memlayout.ld
M src/soc/qualcomm/sdm845/include/soc/memlayout.ld
M src/soc/rockchip/rk3288/include/soc/memlayout.ld
M src/soc/rockchip/rk3399/include/soc/memlayout.ld
M src/soc/samsung/exynos5250/include/soc/memlayout.ld
18 files changed, 40 insertions(+), 36 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/31474/1
diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index 6fe8f14..b8d8c03 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -26,11 +26,11 @@
. += 4096 * CONFIG_NUM_CAR_PAGE_TABLE_PAGES;
_epagetables = . ;
#endif
- /* Vboot work buffer is completely volatile outside of verstage and
- * romstage. Appropriate code needs to handle the transition. */
-#if IS_ENABLED(CONFIG_VBOOT_SEPARATE_VERSTAGE)
- VBOOT2_WORK(., 16K)
-#endif
+ /* Vboot work buffer only needs to be available when verified boot
+ * starts in bootblock. */
+#if IS_ENABLED(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK)
+ VBOOT2_WORK(.)
+ #endif
/* Stack for CAR stages. Since it persists across all stages that
* use CAR it can be reused. The chipset/SoC is expected to provide
* the stack size. */
diff --git a/src/include/memlayout.h b/src/include/memlayout.h
index a8b3a60..4fa1802 100644
--- a/src/include/memlayout.h
+++ b/src/include/memlayout.h
@@ -160,9 +160,8 @@
/* Careful: required work buffer size depends on RW properties such as key size
* and algorithm -- what works for you might stop working after an update. Do
* NOT lower the asserted minimum without consulting vboot devs (rspangler)! */
-#define VBOOT2_WORK(addr, size) \
- REGION(vboot2_work, addr, size, 16) \
- _ = ASSERT(size >= 12K, "vboot2 work buffer must be at least 12K!");
+#define VBOOT2_WORK(addr) \
+ REGION(vboot2_work, addr, CONFIG_VBOOT_WORKING_DATA_SIZE, 16)

#if ENV_VERSTAGE
#define VERSTAGE(addr, sz) \
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig
index a3e9b86..1b34c91 100644
--- a/src/security/vboot/Kconfig
+++ b/src/security/vboot/Kconfig
@@ -92,6 +92,13 @@
memory initialization). This implies that vboot working data is
allocated in CBMEM.

+config VBOOT_WORKING_DATA_SIZE
+ int
+ default 12288 # 12K, must match VB2_WORKBUF_RECOMMENDED_SIZE
+ help
+ Size in bytes that should be allocated for verified boot's working
+ data structure.
+
config VBOOT_MOCK_SECDATA
bool "Mock secdata for firmware verification"
default n
diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc
index 6c63f7b..d1e2fe4 100644
--- a/src/security/vboot/Makefile.inc
+++ b/src/security/vboot/Makefile.inc
@@ -22,7 +22,13 @@
verstage-y += bootmode.c
postcar-y += bootmode.c

-verstage-generic-ccopts += -D__PRE_RAM__ -D__VERSTAGE__
+# When VBOOT_STARTS_IN_ROMSTAGE is selected, DRAM is already up by the time
+# verstage runs.
+ifneq ($(CONFIG_VBOOT_STARTS_IN_ROMSTAGE),y)
+verstage-generic-ccopts += -D__PRE_RAM__
+endif
+
+verstage-generic-ccopts += -D__VERSTAGE__

ramstage-y += gbb.c

diff --git a/src/security/vboot/common.c b/src/security/vboot/common.c
index 747644a..c3d065e 100644
--- a/src/security/vboot/common.c
+++ b/src/security/vboot/common.c
@@ -40,25 +40,16 @@
uint32_t buffer_size;
};

-static const size_t vb_work_buf_size = 16 * KiB;
-
static struct vb2_working_data * const vboot_get_working_data(void)
{
if (IS_ENABLED(CONFIG_VBOOT_STARTS_IN_ROMSTAGE))
/* cbmem_add() does a cbmem_find() first. */
- return cbmem_add(CBMEM_ID_VBOOT_WORKBUF, vb_work_buf_size);
+ return cbmem_add(CBMEM_ID_VBOOT_WORKBUF,
+ CONFIG_VBOOT_WORKING_DATA_SIZE);
else
return (struct vb2_working_data *)_vboot2_work;
}

-static size_t vb2_working_data_size(void)
-{
- if (IS_ENABLED(CONFIG_VBOOT_STARTS_IN_ROMSTAGE))
- return vb_work_buf_size;
- else
- return _vboot2_work_size;
-}
-
static struct selected_region *vb2_selected_region(void)
{
struct selected_region *sel_reg = NULL;
@@ -86,19 +77,17 @@
void vb2_init_work_context(struct vb2_context *ctx)
{
struct vb2_working_data *wd;
- size_t work_size;

/* First initialize the working data region. */
- work_size = vb2_working_data_size();
wd = vboot_get_working_data();
- memset(wd, 0, work_size);
+ memset(wd, 0, CONFIG_VBOOT_WORKING_DATA_SIZE);

/*
* vboot prefers 16-byte alignment. This takes away 16 bytes
* from the VBOOT2_WORK region, but the vboot devs said that's okay.
*/
wd->buffer_offset = ALIGN_UP(sizeof(*wd), 16);
- wd->buffer_size = work_size - wd->buffer_offset;
+ wd->buffer_size = CONFIG_VBOOT_WORKING_DATA_SIZE - wd->buffer_offset;

/* Initialize the vb2_context. */
memset(ctx, 0, sizeof(*ctx));
diff --git a/src/security/vboot/vboot_loader.c b/src/security/vboot/vboot_loader.c
index 75f75b5..7b0b8d8 100644
--- a/src/security/vboot/vboot_loader.c
+++ b/src/security/vboot/vboot_loader.c
@@ -22,6 +22,7 @@
#include <security/vboot/misc.h>
#include <security/vboot/symbols.h>
#include <security/vboot/vboot_common.h>
+#include <vb2_api.h>

/* Ensure vboot configuration is valid: */
_Static_assert(IS_ENABLED(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK) +
@@ -33,6 +34,8 @@
_Static_assert(!IS_ENABLED(CONFIG_VBOOT_RETURN_FROM_VERSTAGE) ||
IS_ENABLED(CONFIG_VBOOT_SEPARATE_VERSTAGE),
"return from verstage only makes sense for separate verstages");
+_Static_assert(CONFIG_VBOOT_WORKING_DATA_SIZE == VB2_WORKBUF_RECOMMENDED_SIZE,
+ "working data size must be set to VB2_WORKBUF_RECOMMENDED_SIZE");

/* The stage loading code is compiled and entered from multiple stages. The
* helper functions below attempt to provide more clarity on when certain
diff --git a/src/soc/cavium/cn81xx/include/soc/memlayout.ld b/src/soc/cavium/cn81xx/include/soc/memlayout.ld
index 2222617..ab47f1f 100644
--- a/src/soc/cavium/cn81xx/include/soc/memlayout.ld
+++ b/src/soc/cavium/cn81xx/include/soc/memlayout.ld
@@ -34,7 +34,7 @@
PRERAM_CBFS_CACHE(BOOTROM_OFFSET + 0x6000, 8K)
PRERAM_CBMEM_CONSOLE(BOOTROM_OFFSET + 0x8000, 8K)
BOOTBLOCK(BOOTROM_OFFSET + 0x20000, 64K)
- VBOOT2_WORK(BOOTROM_OFFSET + 0x30000, 12K)
+ VBOOT2_WORK(BOOTROM_OFFSET + 0x30000)
VERSTAGE(BOOTROM_OFFSET + 0x33000, 52K)
ROMSTAGE(BOOTROM_OFFSET + 0x40000, 256K)

diff --git a/src/soc/imgtec/pistachio/include/soc/memlayout.ld b/src/soc/imgtec/pistachio/include/soc/memlayout.ld
index 05042ef..e0a92cf8 100644
--- a/src/soc/imgtec/pistachio/include/soc/memlayout.ld
+++ b/src/soc/imgtec/pistachio/include/soc/memlayout.ld
@@ -45,7 +45,7 @@
SRAM_START(0x1a000000)
REGION(gram_bootblock, 0x1a000000, 28K, 1)
ROMSTAGE(0x1a007000, 60K)
- VBOOT2_WORK(0x1a016000, 12K)
+ VBOOT2_WORK(0x1a016000)
PRERAM_CBFS_CACHE(0x1a019000, 48K)
SRAM_END(0x1a066000)

diff --git a/src/soc/mediatek/mt8173/include/soc/memlayout.ld b/src/soc/mediatek/mt8173/include/soc/memlayout.ld
index 5b92153..3705ca1 100644
--- a/src/soc/mediatek/mt8173/include/soc/memlayout.ld
+++ b/src/soc/mediatek/mt8173/include/soc/memlayout.ld
@@ -38,7 +38,7 @@
SRAM_L2C_END(0x00100000)

SRAM_START(0x00100000)
- VBOOT2_WORK(0x00100000, 12K)
+ VBOOT2_WORK(0x00100000)
PRERAM_CBMEM_CONSOLE(0x00103000, 16K)
WATCHDOG_TOMBSTONE(0x00107000, 4)
PRERAM_CBFS_CACHE(0x00107004, 16K - 4)
diff --git a/src/soc/mediatek/mt8183/include/soc/memlayout.ld b/src/soc/mediatek/mt8183/include/soc/memlayout.ld
index a547083..4d065c7 100644
--- a/src/soc/mediatek/mt8183/include/soc/memlayout.ld
+++ b/src/soc/mediatek/mt8183/include/soc/memlayout.ld
@@ -28,7 +28,7 @@
SECTIONS
{
SRAM_START(0x00100000)
- VBOOT2_WORK(0x00100000, 12K)
+ VBOOT2_WORK(0x00100000)
PRERAM_CBMEM_CONSOLE(0x00103000, 16K)
WATCHDOG_TOMBSTONE(0x00107000, 4)
PRERAM_CBFS_CACHE(0x00107004, 16K - 4)
diff --git a/src/soc/nvidia/tegra124/include/soc/memlayout.ld b/src/soc/nvidia/tegra124/include/soc/memlayout.ld
index 615f0563..9907afe 100644
--- a/src/soc/nvidia/tegra124/include/soc/memlayout.ld
+++ b/src/soc/nvidia/tegra124/include/soc/memlayout.ld
@@ -29,7 +29,7 @@
TTB(0x40000000, 16K + 32)
PRERAM_CBMEM_CONSOLE(0x40004020, 8K - 32)
PRERAM_CBFS_CACHE(0x40006000, 16K)
- VBOOT2_WORK(0x4000A000, 16K)
+ VBOOT2_WORK(0x4000A000)
STACK(0x4000E000, 8K)
BOOTBLOCK(0x40010000, 30K)
VERSTAGE(0x40017800, 72K)
diff --git a/src/soc/nvidia/tegra210/include/soc/memlayout.ld b/src/soc/nvidia/tegra210/include/soc/memlayout.ld
index d807c06..1521431 100644
--- a/src/soc/nvidia/tegra210/include/soc/memlayout.ld
+++ b/src/soc/nvidia/tegra210/include/soc/memlayout.ld
@@ -30,7 +30,7 @@
SRAM_START(0x40000000)
PRERAM_CBMEM_CONSOLE(0x40000000, 4K)
PRERAM_CBFS_CACHE(0x40001000, 36K)
- VBOOT2_WORK(0x4000A000, 12K)
+ VBOOT2_WORK(0x4000A000)
#if ENV_ARM64
STACK(0x4000D000, 3K)
#else /* AVP gets a separate stack to avoid any chance of handoff races. */
diff --git a/src/soc/qualcomm/ipq40xx/include/soc/memlayout.ld b/src/soc/qualcomm/ipq40xx/include/soc/memlayout.ld
index 5e97077..59281cc 100644
--- a/src/soc/qualcomm/ipq40xx/include/soc/memlayout.ld
+++ b/src/soc/qualcomm/ipq40xx/include/soc/memlayout.ld
@@ -46,7 +46,7 @@
REGION_START(wifi_imem_1, 0x0A8C0000)
BOOTBLOCK(0x0A8C0000, 24K)
OVERLAP_VERSTAGE_ROMSTAGE(0x0A8C6000, 64K)
- VBOOT2_WORK(0x0A8D6000, 16K)
+ VBOOT2_WORK(0x0A8D6000)
PRERAM_CBMEM_CONSOLE(0x0A8DA000, 32K)
TIMESTAMP(0x0A8E2000, 1K)

diff --git a/src/soc/qualcomm/ipq806x/include/soc/memlayout.ld b/src/soc/qualcomm/ipq806x/include/soc/memlayout.ld
index 6ff2b77..8ecd924 100644
--- a/src/soc/qualcomm/ipq806x/include/soc/memlayout.ld
+++ b/src/soc/qualcomm/ipq806x/include/soc/memlayout.ld
@@ -27,7 +27,7 @@
/* DDR(0x2A000000, 48K) */
BOOTBLOCK(0x2A00C000, 24K)
OVERLAP_VERSTAGE_ROMSTAGE(0x2A012000, 64K)
- VBOOT2_WORK(0x2A022000, 16K)
+ VBOOT2_WORK(0x2A022000)
PRERAM_CBMEM_CONSOLE(0x2A026000, 32K)
TIMESTAMP(0x2A02E000, 1K)

diff --git a/src/soc/qualcomm/sdm845/include/soc/memlayout.ld b/src/soc/qualcomm/sdm845/include/soc/memlayout.ld
index 5d99a8e..7a6394e 100644
--- a/src/soc/qualcomm/sdm845/include/soc/memlayout.ld
+++ b/src/soc/qualcomm/sdm845/include/soc/memlayout.ld
@@ -35,7 +35,7 @@
REGION(fw_reserved2, 0x14800000, 0x16000, 4096)
BOOTBLOCK(0x14816000, 40K)
TTB(0x14820000, 56K)
- VBOOT2_WORK(0x1482E000, 16K)
+ VBOOT2_WORK(0x1482E000)
STACK(0x14832000, 16K)
TIMESTAMP(0x14836000, 1K)
PRERAM_CBMEM_CONSOLE(0x14836400, 32K)
diff --git a/src/soc/rockchip/rk3288/include/soc/memlayout.ld b/src/soc/rockchip/rk3288/include/soc/memlayout.ld
index 6320fad..de957d5 100644
--- a/src/soc/rockchip/rk3288/include/soc/memlayout.ld
+++ b/src/soc/rockchip/rk3288/include/soc/memlayout.ld
@@ -32,7 +32,7 @@
TTB(0xFF700000, 16K)
BOOTBLOCK(0xFF704004, 20K - 4)
PRERAM_CBMEM_CONSOLE(0xFF709000, 2K)
- VBOOT2_WORK(0xFF709800, 12K)
+ VBOOT2_WORK(0xFF709800)
OVERLAP_VERSTAGE_ROMSTAGE(0xFF70C800, 42K)
PRERAM_CBFS_CACHE(0xFF717000, 1K)
TIMESTAMP(0xFF717400, 0x180)
diff --git a/src/soc/rockchip/rk3399/include/soc/memlayout.ld b/src/soc/rockchip/rk3399/include/soc/memlayout.ld
index 01e352f..a3581a8 100644
--- a/src/soc/rockchip/rk3399/include/soc/memlayout.ld
+++ b/src/soc/rockchip/rk3399/include/soc/memlayout.ld
@@ -35,7 +35,7 @@
/* 0xFF8C2004 is the entry point address the masked ROM will jump to. */
OVERLAP_DECOMPRESSOR_VERSTAGE_ROMSTAGE(0xFF8C2004, 88K - 4)
BOOTBLOCK(0xFF8D8000, 40K)
- VBOOT2_WORK(0XFF8E2000, 12K)
+ VBOOT2_WORK(0XFF8E2000)
TTB(0xFF8E5000, 24K)
PRERAM_CBMEM_CONSOLE(0xFF8EB000, 8K)
STACK(0xFF8ED000, 12K)
diff --git a/src/soc/samsung/exynos5250/include/soc/memlayout.ld b/src/soc/samsung/exynos5250/include/soc/memlayout.ld
index 4daf2b9..9bdf105 100644
--- a/src/soc/samsung/exynos5250/include/soc/memlayout.ld
+++ b/src/soc/samsung/exynos5250/include/soc/memlayout.ld
@@ -32,7 +32,7 @@
/* 32K hole */
TTB(0x2058000, 16K)
PRERAM_CBFS_CACHE(0x205C000, 80K)
- VBOOT2_WORK(0x2070000, 16K)
+ VBOOT2_WORK(0x2070000)
STACK(0x2074000, 16K)
SRAM_END(0x2078000)


To view, visit change 31474. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id71a8ab2401efcc0194d48c8af9017fc90513cb8
Gerrit-Change-Number: 31474
Gerrit-PatchSet: 1
Gerrit-Owner: Joel Kitching <kitching@google.com>
Gerrit-MessageType: newchange