Attention is currently required from: Frank Wu.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52127 )
Change subject: mb/google/zork/vilboz: Update the ACPI name of ALC1015 AMP
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52127/comment/7244d46c_796a3859
PS1, Line 9: Update the ACPI name from AMDP1015 to 1002105 based on b/177971830#180.
Why is the new ACPI name needed? (Please refer to the Linux kernel changes, if that is related.)
Please always summarize the bug report in the commit message, as not all folks are granted access.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52127
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id8f378ad6f3328d7db949ecdb609a2f16acd3884
Gerrit-Change-Number: 52127
Gerrit-PatchSet: 1
Gerrit-Owner: Frank Wu <frank_wu(a)compal.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Frank Wu <frank_wu(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 06 Apr 2021 07:54:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48901 )
Change subject: util/crossgcc: Add date to the toolchain revision
......................................................................
util/crossgcc: Add date to the toolchain revision
With the current version method, it's not possible to determine if
a different version is older or newer than the current version without
digging into the repository and finding the dates for the version
numbers.
This change adds the commit date to the start of the toolchain version
which will let us tell at a glance how old or new the toolchain is.
It's not perfect because multiple toolchain commits can go in on the
same day, but adding the time made the string even longer, and really
doesn't help that much.
Signed-off-by: Martin Roth <martin(a)coreboot.org>
Change-Id: I9c6d27667b922dc15e7a6e132e1beff69eed839c
---
M util/crossgcc/Makefile.inc
M util/crossgcc/buildgcc
2 files changed, 7 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48901/1
diff --git a/util/crossgcc/Makefile.inc b/util/crossgcc/Makefile.inc
index 74de5bd..f032964 100644
--- a/util/crossgcc/Makefile.inc
+++ b/util/crossgcc/Makefile.inc
@@ -54,7 +54,7 @@
test-toolchain:
ifeq ($(COMPILER_OUT_OF_DATE),1)
echo "The coreboot toolchain is not the current version."
- $(error )
+ $(error Halting)
else
echo "The coreboot toolchain is the current version."
endif # ifeq ($(COMPILER_OUT_OF_DATE),1)
diff --git a/util/crossgcc/buildgcc b/util/crossgcc/buildgcc
index 5af3d5a..25b0d27 100755
--- a/util/crossgcc/buildgcc
+++ b/util/crossgcc/buildgcc
@@ -16,8 +16,7 @@
cd "$(dirname "$0")" || exit 1
-CROSSGCC_DATE="$(git log -n 1 --pretty=%ad --date=short .)"
-CROSSGCC_VERSION="$(git log -n 1 --pretty=%h .)"
+CROSSGCC_VERSION="$(git log -n 1 --pretty=%ad --date=short .)_$(git log -n 1 --pretty=%h .)"
CROSSGCC_COMMIT=$( git describe )
# default settings
@@ -591,7 +590,7 @@
}
printversion() {
- printf "${blue}Welcome to the ${red}coreboot${blue} cross toolchain builder v$CROSSGCC_VERSION ($CROSSGCC_DATE)${NC}\n\n"
+ printf "${blue}Welcome to the ${red}coreboot${blue} cross toolchain builder v$CROSSGCC_VERSION ${NC}\n\n"
}
myversion()
@@ -728,7 +727,7 @@
${GCC_OPTIONS} --enable-languages="${LANGUAGES}" \
--with-gmp="$DESTDIR$TARGETDIR" --with-mpfr="$DESTDIR$TARGETDIR" \
--with-mpc="$DESTDIR$TARGETDIR" \
- --with-pkgversion="coreboot bootstrap v$CROSSGCC_VERSION $CROSSGCC_DATE" \
+ --with-pkgversion="coreboot bootstrap v$CROSSGCC_VERSION" \
&& \
# shellcheck disable=SC2086
$MAKE $JOBS BOOT_CFLAGS="$HOSTCFLAGS" BUILD_CONFIG="" bootstrap && \
@@ -774,7 +773,7 @@
--with-gmp="$DESTDIR$TARGETDIR" --with-mpfr="$DESTDIR$TARGETDIR" \
--with-mpc="$DESTDIR$TARGETDIR" \
--with-gnu-as --with-gnu-ld \
- --with-pkgversion="coreboot toolchain v$CROSSGCC_VERSION $CROSSGCC_DATE" \
+ --with-pkgversion="coreboot toolchain v$CROSSGCC_VERSION" \
&& \
mkdir -p gcc/$TARGETARCH && \
ln -s "$DESTDIR$TARGETDIR/$TARGETARCH/bin" "gcc/$TARGETARCH/$GCC_VERSION" && \
@@ -842,7 +841,7 @@
test "$UNAME" = "FreeBSD" && HOST="_FreeBSD"
test "$UNAME" = "Cygwin" && HOST="_CYGWIN"
HOST="$HOST" CFLAGS="$CFLAGS" \
- OPT_CFLAGS="-O -D_FORTIFY_SOURCE=2 -D COREBOOT_TOOLCHAIN_VERSION='\"coreboot toolchain v$CROSSGCC_VERSION $CROSSGCC_DATE\"' " \
+ OPT_CFLAGS="-O -D_FORTIFY_SOURCE=2 -D COREBOOT_TOOLCHAIN_VERSION='\"coreboot toolchain v$CROSSGCC_VERSION\"' " \
$MAKE $JOBS CC="$(hostcc host)" iasl acpibin acpidump acpiexec acpihelp acpisrc acpixtract
rm -f "$DESTDIR$TARGETDIR/bin/{iasl,acpibin,acpidump,acpiexec,acpihelp,acpisrc,acpixtract}" || touch "$RDIR/.failed"
cp bin/{iasl,acpibin,acpidump,acpiexec,acpihelp,acpisrc,acpixtract} "$DESTDIR$TARGETDIR/bin" || touch "$RDIR/.failed"
@@ -857,7 +856,7 @@
cd - || exit 1
$CMAKE -G "Unix Makefiles" -DCMAKE_INSTALL_PREFIX="$DESTDIR$TARGETDIR" \
- -DCLANG_VENDOR="coreboot toolchain v$CROSSGCC_VERSION $CROSSGCC_DATE - " \
+ -DCLANG_VENDOR="coreboot toolchain v$CROSSGCC_VERSION - " \
-DCMAKE_BUILD_TYPE=Release ../$LLVM_DIR || touch .failed
# shellcheck disable=SC2086
$MAKE $JOBS || touch .failed
--
To view, visit https://review.coreboot.org/c/coreboot/+/48901
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c6d27667b922dc15e7a6e132e1beff69eed839c
Gerrit-Change-Number: 48901
Gerrit-PatchSet: 1
Gerrit-Owner: Martin Roth <martinroth(a)google.com>
Gerrit-MessageType: newchange
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52087 )
Change subject: mem_pool: Track the last two allocations (not just one)
......................................................................
mem_pool: Track the last two allocations (not just one)
This patch changes the mem_pool implementation to track the last two
allocations (instead of just the last) and allow them both to be freed
if the mem_pool_free() calls come in in reverse order. This is intended
as a specific optimization for the CBFS cache case when a compressed
file is mapped on a platform that doesn't natively support
memory-mapping flash. In this case, cbfs_map() (chaining through to
_cbfs_alloc() with allocator == NULL) will call
mem_pool_alloc(&cbfs_cache) to allocate space for the uncompressed file
data. It will then call cbfs_load_and_decompress() to fill that
allocation, which will notice the compression and in turn call
rdev_mmap_full() to map the compressed data (which on platforms without
memory-mapped flash usually results in a second call to
mem_pool_alloc(&cbfs_cache)). It then runs the decompression algorithm
and calls rdev_munmap() on the compressed data buffer (the latter one in
the allocation sequence), leading to a mem_pool_free(). The remaining
buffer with the uncompressed data is returned out of cbfs_map() to the
caller, which should eventually call cbfs_unmap() to mem_pool_free()
that as well. This patch allows this simple case to succeed without
leaking any permanent allocations on the cache. (More complicated cases
where the caller maps other files before cbfs_unmap()ing the first one
may still lead to leaks, but those are very rare in practice.)
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: Ic5c4c56a8482752ed65e10cf35565f9b2d3e4b17
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52087
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Aaron Durbin <adurbin(a)chromium.org>
---
M src/commonlib/include/commonlib/mem_pool.h
M src/commonlib/mem_pool.c
2 files changed, 20 insertions(+), 12 deletions(-)
Approvals:
build bot (Jenkins): Verified
Aaron Durbin: Looks good to me, approved
diff --git a/src/commonlib/include/commonlib/mem_pool.h b/src/commonlib/include/commonlib/mem_pool.h
index bde9e41..6c85397 100644
--- a/src/commonlib/include/commonlib/mem_pool.h
+++ b/src/commonlib/include/commonlib/mem_pool.h
@@ -7,11 +7,14 @@
#include <stdint.h>
/*
- * The memory pool allows one to allocate memory from a fixed size buffer
- * that also allows freeing semantics for reuse. However, the current
- * limitation is that the most recent allocation is the only one that
- * can be freed. If one tries to free any allocation that isn't the
- * most recently allocated it will result in a leak within the memory pool.
+ * The memory pool allows one to allocate memory from a fixed size buffer that
+ * also allows freeing semantics for reuse. However, the current limitation is
+ * that only the two most recent allocations can be freed (in exact reverse
+ * order). If one tries to free any allocation that isn't at the top of the
+ * allocation stack, or one allocates more than two buffers in a row without
+ * freeing, it will result in a leak within the memory pool. (Two allocations
+ * were chosen to optimize for the CBFS cache case which may need two buffers
+ * to map a single compressed file, and will free them in reverse order.)
*
* The memory returned by allocations are at least 8 byte aligned. Note
* that this requires the backing buffer to start on at least an 8 byte
@@ -22,20 +25,23 @@
uint8_t *buf;
size_t size;
uint8_t *last_alloc;
+ uint8_t *second_to_last_alloc;
size_t free_offset;
};
-#define MEM_POOL_INIT(buf_, size_) \
- { \
- .buf = (buf_), \
- .size = (size_), \
- .last_alloc = NULL, \
- .free_offset = 0, \
+#define MEM_POOL_INIT(buf_, size_) \
+ { \
+ .buf = (buf_), \
+ .size = (size_), \
+ .last_alloc = NULL, \
+ .second_to_last_alloc = NULL, \
+ .free_offset = 0, \
}
static inline void mem_pool_reset(struct mem_pool *mp)
{
mp->last_alloc = NULL;
+ mp->second_to_last_alloc = NULL;
mp->free_offset = 0;
}
diff --git a/src/commonlib/mem_pool.c b/src/commonlib/mem_pool.c
index 2ad1099..c300c65 100644
--- a/src/commonlib/mem_pool.c
+++ b/src/commonlib/mem_pool.c
@@ -17,6 +17,7 @@
p = &mp->buf[mp->free_offset];
mp->free_offset += sz;
+ mp->second_to_last_alloc = mp->last_alloc;
mp->last_alloc = p;
return p;
@@ -29,6 +30,7 @@
return;
mp->free_offset = mp->last_alloc - mp->buf;
+ mp->last_alloc = mp->second_to_last_alloc;
/* No way to track allocation before this one. */
- mp->last_alloc = NULL;
+ mp->second_to_last_alloc = NULL;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/52087
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic5c4c56a8482752ed65e10cf35565f9b2d3e4b17
Gerrit-Change-Number: 52087
Gerrit-PatchSet: 3
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52086 )
Change subject: verstage: Add debug print when returning from verstage
......................................................................
verstage: Add debug print when returning from verstage
RETURN_FROM_VERSTAGE is a somewhat tricky construct that we don't
normally do otherwise in coreboot. While it works remarkably well in
general, new development can lead to unintentional interactions with
confusing results. This patch adds a debug print to the verstage right
before returning to the bootblock so that it's obvious this happens,
because otherwise in some cases the last printout in the verstage is
about some TPM commands which can be misleading when execution hangs
after that point.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I9ca68a32d7a50c95d9a6948d35816fee583611bc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52086
Reviewed-by: Furquan Shaikh <furquan(a)google.com>
Reviewed-by: Aaron Durbin <adurbin(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/security/vboot/verstage.c
1 file changed, 1 insertion(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Aaron Durbin: Looks good to me, approved
Furquan Shaikh: Looks good to me, approved
diff --git a/src/security/vboot/verstage.c b/src/security/vboot/verstage.c
index d2a9705..d785e0e 100644
--- a/src/security/vboot/verstage.c
+++ b/src/security/vboot/verstage.c
@@ -19,6 +19,7 @@
if (CONFIG(VBOOT_RETURN_FROM_VERSTAGE)) {
verstage_main();
+ printk(BIOS_DEBUG, "VBOOT: Returning from verstage.\n");
} else {
run_romstage();
hlt();
--
To view, visit https://review.coreboot.org/c/coreboot/+/52086
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9ca68a32d7a50c95d9a6948d35816fee583611bc
Gerrit-Change-Number: 52086
Gerrit-PatchSet: 3
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52085 )
Change subject: decompressor: Add CBFS_VERIFICATION support
......................................................................
decompressor: Add CBFS_VERIFICATION support
CBFS_VERIFICATION requires the CBFS metadata hash anchor to be linked
into an uncompressed stage, but for platforms using COMPRESS_BOOTBLOCK,
this is only the decompressor stage. The first CBFS accesses are made in
the bootblock stage after decompression, so if we want to make
CBFS_VERIFICATION work on those platforms, we have to pass the metadata
hash anchor from the decompressor into the bootblock. This patch does
just that. (Note that this relies on the decompressor data remaining
valid in memory for as long as the metadata hash anchor is needed. This
is always true even for OVERLAP_DECOMPRESSOR_ROMSTAGE() situations
because the FMAP and CBFS metadata necessarily need to have finished
verification before a new stage could be loaded.)
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I2e6d7384cfb8339a24369eb6c01fc12f911c974e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52085
Reviewed-by: Aaron Durbin <adurbin(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/include/bootblock_common.h
M src/include/metadata_hash.h
M src/lib/Kconfig.cbfs_verification
M src/lib/Makefile.inc
M src/lib/bootblock.c
M src/lib/decompressor.c
M src/lib/metadata_hash.c
7 files changed, 42 insertions(+), 4 deletions(-)
Approvals:
build bot (Jenkins): Verified
Aaron Durbin: Looks good to me, approved
diff --git a/src/include/bootblock_common.h b/src/include/bootblock_common.h
index da627d2..fccd235 100644
--- a/src/include/bootblock_common.h
+++ b/src/include/bootblock_common.h
@@ -35,6 +35,7 @@
/* This is the argument structure passed from decompressor to bootblock. */
struct bootblock_arg {
uint64_t base_timestamp;
+ void *metadata_hash_anchor;
uint32_t num_timestamps;
struct timestamp_entry timestamps[];
};
diff --git a/src/include/metadata_hash.h b/src/include/metadata_hash.h
index 2d3b8a8..bfa7ef1 100644
--- a/src/include/metadata_hash.h
+++ b/src/include/metadata_hash.h
@@ -6,6 +6,11 @@
#include <commonlib/bsd/metadata_hash.h>
+/* Return a pointer to the whole anchor. Only used for decompressor builds. */
+void *metadata_hash_export_anchor(void);
+/* Import a pointer that points to the anchor. Only used for decompressor builds. */
+void metadata_hash_import_anchor(void *ptr);
+
/* Verify the an FMAP data structure with the FMAP hash that is stored together with the CBFS
metadata hash in the bootblock's metadata hash anchor (when CBFS verification is enabled). */
vb2_error_t metadata_hash_verify_fmap(const void *fmap_base, size_t fmap_size);
diff --git a/src/lib/Kconfig.cbfs_verification b/src/lib/Kconfig.cbfs_verification
index a28df1f..fa90d9d 100644
--- a/src/lib/Kconfig.cbfs_verification
+++ b/src/lib/Kconfig.cbfs_verification
@@ -6,7 +6,6 @@
config CBFS_VERIFICATION
bool # TODO: make user selectable once it works
- depends on !COMPRESS_BOOTBLOCK # TODO: figure out decompressor anchor
depends on !VBOOT_STARTS_BEFORE_BOOTBLOCK # this is gonna get tricky...
select VBOOT_LIB
help
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 074cb2a..358035d 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -38,6 +38,7 @@
decompressor-$(CONFIG_GENERIC_GPIO_LIB) += gpio.c
decompressor-y += memchr.c
decompressor-y += memcmp.c
+decompressor-$(CONFIG_CBFS_VERIFICATION) += metadata_hash.c
decompressor-y += prog_ops.c
decompressor-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c
diff --git a/src/lib/bootblock.c b/src/lib/bootblock.c
index 1509c8c..23fb392 100644
--- a/src/lib/bootblock.c
+++ b/src/lib/bootblock.c
@@ -4,6 +4,7 @@
#include <bootblock_common.h>
#include <console/console.h>
#include <delay.h>
+#include <metadata_hash.h>
#include <option.h>
#include <post.h>
#include <program_loading.h>
@@ -88,6 +89,8 @@
void _start(struct bootblock_arg *arg);
void _start(struct bootblock_arg *arg)
{
+ if (CONFIG(CBFS_VERIFICATION))
+ metadata_hash_import_anchor(arg->metadata_hash_anchor);
bootblock_main_with_timestamp(arg->base_timestamp, arg->timestamps,
arg->num_timestamps);
}
diff --git a/src/lib/decompressor.c b/src/lib/decompressor.c
index 8ae9358..1d160e0 100644
--- a/src/lib/decompressor.c
+++ b/src/lib/decompressor.c
@@ -3,6 +3,7 @@
#include <bootblock_common.h>
#include <commonlib/bsd/compression.h>
#include <delay.h>
+#include <metadata_hash.h>
#include <program_loading.h>
#include <symbols.h>
#include <timestamp.h>
@@ -42,6 +43,9 @@
if (CONFIG(COLLECT_TIMESTAMPS))
arg.base_timestamp = timestamp_get();
+ if (CONFIG(CBFS_VERIFICATION))
+ arg.metadata_hash_anchor = metadata_hash_export_anchor();
+
decompressor_soc_init();
if (CONFIG(COLLECT_TIMESTAMPS))
diff --git a/src/lib/metadata_hash.c b/src/lib/metadata_hash.c
index a823c5f..5619efe 100644
--- a/src/lib/metadata_hash.c
+++ b/src/lib/metadata_hash.c
@@ -5,6 +5,7 @@
#include <metadata_hash.h>
#include <symbols.h>
+#if !CONFIG(COMPRESS_BOOTBLOCK) || ENV_DECOMPRESSOR
__attribute__((used, section(".metadata_hash_anchor")))
static struct metadata_hash_anchor metadata_hash_anchor = {
/* This is the only place in all of coreboot where we actually need to use this. */
@@ -12,15 +13,39 @@
.cbfs_hash = { .algo = CONFIG_CBFS_HASH_ALGO }
};
+static struct metadata_hash_anchor *get_anchor(void)
+{
+ return &metadata_hash_anchor;
+}
+
+void *metadata_hash_export_anchor(void)
+{
+ return get_anchor();
+}
+#else
+static struct metadata_hash_anchor *anchor_ptr = NULL;
+
+static struct metadata_hash_anchor *get_anchor(void)
+{
+ assert(anchor_ptr != NULL);
+ return anchor_ptr;
+}
+
+void metadata_hash_import_anchor(void *ptr)
+{
+ anchor_ptr = ptr;
+}
+#endif
+
struct vb2_hash *metadata_hash_get(void)
{
- return &metadata_hash_anchor.cbfs_hash;
+ return &get_anchor()->cbfs_hash;
}
vb2_error_t metadata_hash_verify_fmap(const void *fmap_buffer, size_t fmap_size)
{
- struct vb2_hash hash = { .algo = metadata_hash_anchor.cbfs_hash.algo };
- memcpy(hash.raw, metadata_hash_anchor_fmap_hash(&metadata_hash_anchor),
+ struct vb2_hash hash = { .algo = get_anchor()->cbfs_hash.algo };
+ memcpy(hash.raw, metadata_hash_anchor_fmap_hash(get_anchor()),
vb2_digest_size(hash.algo));
return vb2_hash_verify(fmap_buffer, fmap_size, &hash);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/52085
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2e6d7384cfb8339a24369eb6c01fc12f911c974e
Gerrit-Change-Number: 52085
Gerrit-PatchSet: 3
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged