Patrick Georgi merged this change.

View Change

Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
vboot: standardize on working data size

Previously, the size of memory made for vboot_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 binds the VBOOT2_WORK macro to directly use the
VB2_WORKBUF_RECOMMENDED_DATA_SIZE constant as defined by vboot
API. Since the constant needs to be used in a linker script, we
may not include the full vboot API, and must instead directly
include the vb2_constants.h header.

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
CQ-DEPEND=CL:1504490

Change-Id: Id71a8ab2401efcc0194d48c8af9017fc90513cb8
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31474
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
---
M src/arch/x86/car.ld
M src/include/memlayout.h
M src/security/vboot/common.c
M src/soc/nvidia/tegra124/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/qcs405/include/soc/memlayout.ld
M src/soc/qualcomm/sdm845/include/soc/memlayout.ld
M src/soc/samsung/exynos5250/include/soc/memlayout.ld
9 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index 4637362..37fb087 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -29,7 +29,7 @@
/* Vboot work buffer only needs to be available when verified boot
* starts in bootblock. */
#if CONFIG(VBOOT_STARTS_IN_BOOTBLOCK)
- VBOOT2_WORK(., 16K)
+ VBOOT2_WORK(., 12K)
#endif
/* Vboot measured boot TCPA log measurements.
* Needs to be transferred until CBMEM is available
diff --git a/src/include/memlayout.h b/src/include/memlayout.h
index 1ccb9f7..273a80d 100644
--- a/src/include/memlayout.h
+++ b/src/include/memlayout.h
@@ -19,6 +19,7 @@
#define __MEMLAYOUT_H

#include <arch/memlayout.h>
+#include <vb2_constants.h>

/* Macros that the architecture can override. */
#ifndef ARCH_POINTER_ALIGN_SIZE
@@ -157,12 +158,13 @@
REGION(ramstage, addr, sz, 1)
#endif

-/* 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!");
+/* VBOOT2_WORK must always use VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE for its
+ * size argument. The constant is imported via vb2_workbuf_size.h. */
+#define VBOOT2_WORK(addr, sz) \
+ REGION(vboot2_work, addr, sz, 16) \
+ _ = ASSERT(sz == VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE, \
+ STR(vboot2 work buffer size must be equivalent to \
+ VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE! (sz)));

#define VBOOT2_TPM_LOG(addr, size) \
REGION(vboot2_tpm_log, addr, size, 16) \
diff --git a/src/security/vboot/common.c b/src/security/vboot/common.c
index 2348d31..5b49ebf 100644
--- a/src/security/vboot/common.c
+++ b/src/security/vboot/common.c
@@ -25,19 +25,6 @@
#include <security/vboot/symbols.h>
#include <security/vboot/vboot_common.h>

-/* TODO(kitching): Use VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE instead. */
-static size_t vboot_working_data_size(void)
-{
- if (CONFIG(VBOOT_STARTS_IN_ROMSTAGE))
- return 12 * KiB;
-
- else if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) &&
- preram_symbols_available())
- return REGION_SIZE(vboot2_work);
-
- die("impossible!");
-}
-
struct vboot_working_data * const vboot_get_working_data(void)
{
struct vboot_working_data *wd = NULL;
@@ -58,16 +45,17 @@
{
struct vboot_working_data *wd;

- /* First initialize the working data struct. */
+ /* First initialize the working data region. */
wd = vboot_get_working_data();
- memset(wd, 0, sizeof(struct vboot_working_data));
+ memset(wd, 0, VB2_FIRMWARE_WORKBUF_RECOMMENDED_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 = vboot_working_data_size() - wd->buffer_offset;
+ wd->buffer_size = VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE
+ - wd->buffer_offset;

/* Initialize the vb2_context. */
memset(ctx, 0, sizeof(*ctx));
@@ -157,7 +145,8 @@
static void vboot_setup_cbmem(int unused)
{
struct vboot_working_data *wd_cbmem =
- cbmem_add(CBMEM_ID_VBOOT_WORKBUF, vboot_working_data_size());
+ cbmem_add(CBMEM_ID_VBOOT_WORKBUF,
+ VB2_FIRMWARE_WORKBUF_RECOMMENDED_SIZE);
assert(wd_cbmem != NULL);
}
ROMSTAGE_CBMEM_INIT_HOOK(vboot_setup_cbmem)
diff --git a/src/soc/nvidia/tegra124/include/soc/memlayout.ld b/src/soc/nvidia/tegra124/include/soc/memlayout.ld
index 40af0d5..7e2f9ec 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, 6K - 32)
PRERAM_CBFS_CACHE(0x40005800, 16K)
- VBOOT2_WORK(0x40009800, 16K)
+ VBOOT2_WORK(0x40009800, 12K)
VBOOT2_TPM_LOG(0x4000D800, 2K)
STACK(0x4000E000, 8K)
BOOTBLOCK(0x40010000, 30K)
diff --git a/src/soc/qualcomm/ipq40xx/include/soc/memlayout.ld b/src/soc/qualcomm/ipq40xx/include/soc/memlayout.ld
index 5e97077..a69b60b 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, 12K)
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..25db175 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, 12K)
PRERAM_CBMEM_CONSOLE(0x2A026000, 32K)
TIMESTAMP(0x2A02E000, 1K)

diff --git a/src/soc/qualcomm/qcs405/include/soc/memlayout.ld b/src/soc/qualcomm/qcs405/include/soc/memlayout.ld
index 8ed258b..68642d6 100644
--- a/src/soc/qualcomm/qcs405/include/soc/memlayout.ld
+++ b/src/soc/qualcomm/qcs405/include/soc/memlayout.ld
@@ -34,7 +34,7 @@
REGION(fw_reserved2, 0x8C19000, 0x16000, 4096)
BOOTBLOCK(0x8C2F000, 40K)
TTB(0x8C39000, 56K)
- VBOOT2_WORK(0x8C47000, 16K)
+ VBOOT2_WORK(0x8C47000, 12K)
STACK(0x8C4B000, 16K)
TIMESTAMP(0x8C4F000, 1K)
PRERAM_CBMEM_CONSOLE(0x8C4F400, 32K)
diff --git a/src/soc/qualcomm/sdm845/include/soc/memlayout.ld b/src/soc/qualcomm/sdm845/include/soc/memlayout.ld
index 5d99a8e..7063c69 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, 12K)
STACK(0x14832000, 16K)
TIMESTAMP(0x14836000, 1K)
PRERAM_CBMEM_CONSOLE(0x14836400, 32K)
diff --git a/src/soc/samsung/exynos5250/include/soc/memlayout.ld b/src/soc/samsung/exynos5250/include/soc/memlayout.ld
index ab79594..0bd319e 100644
--- a/src/soc/samsung/exynos5250/include/soc/memlayout.ld
+++ b/src/soc/samsung/exynos5250/include/soc/memlayout.ld
@@ -33,7 +33,7 @@
TTB(0x2058000, 16K)
PRERAM_CBFS_CACHE(0x205C000, 78K)
VBOOT2_TPM_LOG(0x206F800, 2K)
- VBOOT2_WORK(0x2070000, 16K)
+ VBOOT2_WORK(0x2070000, 12K)
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: 15
Gerrit-Owner: Joel Kitching <kitching@google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Joel Kitching <kitching@google.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: Randall Spangler <rspangler@google.com>
Gerrit-Reviewer: Simon Glass <sjg@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged