Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47072 )
Change subject: cpu/x86/mp_init.c: Allocate the smm relocation stack on the heap
......................................................................
cpu/x86/mp_init.c: Allocate the smm relocation stack on the heap
The smm_module_loader v2 places stack_top at DEFAULT_SMBASE +
per_cpu_stack_size. The smm_stub assembly will put the stack for a CPU
at stack_top - apicid * stack_size. So for loader v2 this means that
areas below DEFAULT_SMBASE get used as a CPU stack which would cause
lower memory corruption on S3 resume.
The solution is to use ramstage heap as a temporary place for the smm
relocation stacks. This completely avoids using lower memory for
CPU stacks.
On V1 all the CPU stacks would be properly allocated inside the
DEFAULT_SMBASE but this change won't hurt either.
Tested: works fine on OCP/Deltalake.
Change-Id: I125d3aa6526c923e154c41e2d586557c7d8f56c3
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/cpu/x86/mp_init.c
M src/cpu/x86/smm/smm_module_loader.c
M src/cpu/x86/smm/smm_module_loaderv2.c
3 files changed, 32 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/47072/1
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c
index 4870529..3deab6d 100644
--- a/src/cpu/x86/mp_init.c
+++ b/src/cpu/x86/mp_init.c
@@ -3,6 +3,7 @@
#include <console/console.h>
#include <stddef.h>
#include <stdint.h>
+#include <stdlib.h>
#include <string.h>
#include <rmodule.h>
#include <arch/cpu.h>
@@ -764,19 +765,21 @@
runtime->apic_id_to_cpu[i] = cpu_get_apic_id(i);
}
+void *smm_stacks;
+
static int install_relocation_handler(int num_cpus, size_t save_state_size)
{
- int cpus = num_cpus;
-#if CONFIG(X86_SMM_LOADER_VERSION2)
- /* Default SMRAM size is not big enough to concurrently
- * handle relocation for more than ~32 CPU threads
- * therefore, relocate 1 by 1. */
- cpus = 1;
-#endif
+ smm_stacks = memalign(16, num_cpus * CONFIG_SMM_STUB_STACK_SIZE);
+
+ if (smm_stacks == NULL) {
+ printk(BIOS_ERR, "%s: failed to allocate stacks.\n");
+ return -1;
+ }
struct smm_loader_params smm_params = {
+ .stack_top = (void *)((uintptr_t)smm_stacks + num_cpus * CONFIG_SMM_STUB_STACK_SIZE),
.per_cpu_stack_size = CONFIG_SMM_STUB_STACK_SIZE,
- .num_concurrent_stacks = cpus,
+ .num_concurrent_stacks = num_cpus,
.per_cpu_save_state_size = save_state_size,
.num_concurrent_save_states = 1,
.handler = smm_do_relocation,
@@ -1104,6 +1107,8 @@
restore_default_smm_area(default_smm_area);
+ free(smm_stacks);
+
/* Signal callback on success if it's provided. */
if (ret == 0 && mp_state.ops.post_mp_init != NULL)
mp_state.ops.post_mp_init();
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c
index 876fde6..3c0ed7e 100644
--- a/src/cpu/x86/smm/smm_module_loader.c
+++ b/src/cpu/x86/smm/smm_module_loader.c
@@ -244,8 +244,16 @@
size += params->per_cpu_save_state_size;
}
- /* Place the stacks in the lower half of SMRAM. */
- stacks_top = smm_stub_place_stacks(base, size, params);
+ /* Use the smbase as a proxy to know if we are installing the stub for relocation
+ * or for permanent handling. In case of relocation the SMM relocation stack will
+ * have been allocated on the ramstage heap and programmed in the smm loader params.
+ */
+ if (smbase == (void *)SMM_DEFAULT_BASE)
+ stacks_top = params->stack_top;
+ else
+ /* Place the stacks in the lower half of SMRAM. */
+ stacks_top = smm_stub_place_stacks(base, size, params);
+
if (stacks_top == NULL)
return -1;
diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c
index 7a72c10..54e46d4 100644
--- a/src/cpu/x86/smm/smm_module_loaderv2.c
+++ b/src/cpu/x86/smm/smm_module_loaderv2.c
@@ -414,8 +414,15 @@
* of SMRAM which is TSEG base
*/
const total_stack_size = params->num_concurrent_stacks * params->per_cpu_stack_size;
- stacks_top = smm_stub_place_stacks((char *)params->smram_start, total_stack_size,
- params);
+ /* Use the smbase as a proxy to know if we are installing the stub for relocation
+ * or for permanent handling. In case of relocation the SMM relocation stack will
+ * have been allocated on the ramstage heap and programmed in the smm loader params.
+ */
+ if (smbase == (void *)SMM_DEFAULT_BASE)
+ stacks_top = params->stack_top;
+ else
+ stacks_top = smm_stub_place_stacks((char *)params->smram_start, size, params);
+
if (stacks_top == NULL) {
printk(BIOS_ERR, "%s: not enough space for stacks\n", __func__);
printk(BIOS_ERR, "%s: ....need -> %p : available -> %zx\n", __func__,
--
To view, visit https://review.coreboot.org/c/coreboot/+/47072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I125d3aa6526c923e154c41e2d586557c7d8f56c3
Gerrit-Change-Number: 47072
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42563 )
Change subject: [RFC] AMD APM_CNT and SMI enablement
......................................................................
[RFC] AMD APM_CNT and SMI enablement
Some suspicious callsite caught my eye.
Change-Id: I8d1287566061bc5d6f05e687f2b30c6bcb66c999
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
---
M src/soc/amd/picasso/southbridge.c
M src/soc/amd/stoneyridge/southbridge.c
2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/42563/1
diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c
index 0ff5606..89b49f6 100644
--- a/src/soc/amd/picasso/southbridge.c
+++ b/src/soc/amd/picasso/southbridge.c
@@ -233,6 +233,7 @@
u32 reg;
msr_t cst_addr;
+
/* We use some of these ports in SMM regardless of whether or not
* ACPI tables are generated. Enable these ports indiscriminately.
*/
diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c
index e90fe1b..8a45fda 100644
--- a/src/soc/amd/stoneyridge/southbridge.c
+++ b/src/soc/amd/stoneyridge/southbridge.c
@@ -470,6 +470,7 @@
{
u32 reg;
+
/* We use some of these ports in SMM regardless of whether or not
* ACPI tables are generated. Enable these ports indiscriminately.
*/
--
To view, visit https://review.coreboot.org/c/coreboot/+/42563
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8d1287566061bc5d6f05e687f2b30c6bcb66c999
Gerrit-Change-Number: 42563
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: newchange
DaLao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40723 )
Change subject: mb/lenovo/t440p: update VBT
......................................................................
mb/lenovo/t440p: update VBT
Update T440p's VBT from to version 2179. Extracted from the
latest bios update file: T440p's bios image
https://download.lenovo.com/pccbbs/mobiles/gluj42us.iso
Test: boot t440p with both SeaBIOS and Tianocore payloads,
verify dp output and backlight control all works under both
Linux and Windows.
Signed-off-by: dalao <dalao(a)tutanota.com>
Change-Id: If8669b8de6fa0801e261138651b8b2cf50432a70
---
M src/mainboard/lenovo/t440p/data.vbt
1 file changed, 0 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/40723/1
diff --git a/src/mainboard/lenovo/t440p/data.vbt b/src/mainboard/lenovo/t440p/data.vbt
index 1b2cd87..79077cc 100644
--- a/src/mainboard/lenovo/t440p/data.vbt
+++ b/src/mainboard/lenovo/t440p/data.vbt
Binary files differ
--
To view, visit https://review.coreboot.org/c/coreboot/+/40723
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If8669b8de6fa0801e261138651b8b2cf50432a70
Gerrit-Change-Number: 40723
Gerrit-PatchSet: 1
Gerrit-Owner: DaLao <dalao(a)tutanota.com>
Gerrit-MessageType: newchange
Hello Patrick Georgi, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41122
to review the following change.
Change subject: cbfstool: Add support for platform "fixups" when modifying bootblock
......................................................................
cbfstool: Add support for platform "fixups" when modifying bootblock
To support the new CONFIG_CBFS_VERIFICATION feature, cbfstool needs to
update the master hash embedded in the bootblock code every time it adds
or removes a CBFS file. This can lead to problems on certain platforms
where the bootblock needs to be specially wrapped in some
platform-specific data structure so that the platform's masked ROM can
recognize it. If that data structure contains any form of hash or
signature of the bootblock code that is checked on every boot, it will
no longer match if cbfstool modifies it after the fact.
In general, we should always try to disable these kinds of features
where possible (they're not super useful anyway). But for platforms
where the hardware simply doesn't allow that, this patch introduces the
concept of "platform fixups" to cbfstool. Whenever cbfstool finds a
master hash anchor in a CBFS image, it will run all built-in "fixup
probe" functions on that bootblock to check if it can recognize it as
the wrapper format for a platform known to have such an issue. If so, it
will register a corresponding fixup function that will run whenever it
tries to write back modified data to that bootblock. The function can
then modify any platform-specific headers as necessary.
As first supported platform, this patch adds a fixup for Qualcomm
platforms (specifically the header format used by sc7180), which
recalculates the bootblock body hash originally added by
util/qualcomm/createxbl.py.
(Note that this feature is not intended to support platform-specific
signature schemes like BootGuard directly in cbfstool. For anything that
requires an actual secret key, it should be okay if the user needs to
run a platform-specific signing tool on the final CBFS image before
flashing. This feature is intended for the normal unsigned case (which
on some platforms may be implemented as signing with a well-known key)
so that on a board that is not "locked down" in any way the normal use
case of manipulating an image with cbfstool and then directly flashing
the output file stays working with CONFIG_CBFS_VERIFICATION.)
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I02a83a40f1d0009e6f9561ae5d2d9f37a510549a
---
M util/cbfstool/Makefile.inc
M util/cbfstool/cbfs.h
M util/cbfstool/cbfstool.c
M util/cbfstool/elf.h
A util/cbfstool/platform_fixups.c
5 files changed, 131 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/41122/1
diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc
index e98b52e..1a5882f 100644
--- a/util/cbfstool/Makefile.inc
+++ b/util/cbfstool/Makefile.inc
@@ -22,6 +22,7 @@
cbfsobj += rmodule.o
cbfsobj += xdr.o
cbfsobj += partitioned_file.o
+cbfsobj += platform_fixups.o
# COMMONLIB
cbfsobj += cbfs_private.o
cbfsobj += fsp_relocate.o
diff --git a/util/cbfstool/cbfs.h b/util/cbfstool/cbfs.h
index 10b478c..0199d9c 100644
--- a/util/cbfstool/cbfs.h
+++ b/util/cbfstool/cbfs.h
@@ -78,4 +78,9 @@
void xdr_get_seg(struct cbfs_payload_segment *out,
struct cbfs_payload_segment *in);
+/* platform_fixups.c */
+typedef int (*platform_fixup_func)(struct buffer *buffer, size_t offset);
+platform_fixup_func platform_fixups_probe(struct buffer *buffer, size_t offset,
+ const char *region_name);
+
#endif
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index bc07b6f..0b0fea7 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -108,6 +108,7 @@
const char *region;
size_t offset;
struct vb2_hash cbfs_hash;
+ platform_fixup_func fixup;
};
static struct mh_cache *get_mh_cache(void)
@@ -157,6 +158,8 @@
}
mhc.cbfs_hash = anchor->cbfs_hash;
mhc.offset = (void *)anchor - buffer_get(&buffer);
+ mhc.fixup = platform_fixups_probe(&buffer, mhc.offset,
+ mhc.region);
return &mhc;
}
@@ -198,6 +201,8 @@
master_hash_anchor_fmap_hash(anchor), fmap_hash,
vb2_digest_size(anchor->cbfs_hash.algo));
}
+ if (mhc->fixup && mhc->fixup(&buffer, mhc->offset) != 0)
+ return -1;
if (!partitioned_file_write_region(param.image_file, &buffer))
return -1;
return 0;
diff --git a/util/cbfstool/elf.h b/util/cbfstool/elf.h
index 11cee4f..9ba55a6 100644
--- a/util/cbfstool/elf.h
+++ b/util/cbfstool/elf.h
@@ -574,6 +574,8 @@
#define PF_W (1 << 1) /* Segment is writable */
#define PF_R (1 << 2) /* Segment is readable */
#define PF_MASKOS 0x0ff00000 /* OS-specific */
+#define PF_QC_SG_MASK 0x07000000 /* Qualcomm "segment" types */
+#define PF_QC_SG_HASH 0x02000000 /* Qualcomm hash segment */
#define PF_MASKPROC 0xf0000000 /* Processor-specific */
/* Legal values for note segment descriptor types for core files. */
diff --git a/util/cbfstool/platform_fixups.c b/util/cbfstool/platform_fixups.c
new file mode 100644
index 0000000..f893a4d
--- /dev/null
+++ b/util/cbfstool/platform_fixups.c
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include "cbfs.h"
+#include "cbfs_sections.h"
+#include "elfparsing.h"
+
+/*
+ * NOTE: This currently only implements support for MBN version 6 (as used by sc7180). Support
+ * for other MBN versions could probably be added but may require more parsing to tell them
+ * apart, and minor modifications (e.g. different hash algorithm). Add later as needed.
+ */
+static void *qualcomm_find_hash(struct buffer *in, size_t bb_offset, struct vb2_hash *real_hash)
+{
+ size_t search_end_size = MIN(0, buffer_size(in) - 32 * KiB);
+ struct buffer elf;
+ buffer_clone(&elf, in);
+
+ /* To identify a Qualcomm image, first we find the GPT header... */
+ while (buffer_size(&elf) > search_end_size &&
+ !buffer_check_magic(&elf, "EFI PART", 8))
+ buffer_seek(&elf, 512);
+
+ /* ...then shortly afterwards there's an ELF header... */
+ while (buffer_size(&elf) > search_end_size &&
+ !buffer_check_magic(&elf, ELFMAG, 4))
+ buffer_seek(&elf, 512);
+
+ if (buffer_size(&elf) <= search_end_size)
+ return NULL; /* Doesn't seem to be a Qualcomm image. */
+
+ struct parsed_elf pelf;
+ if (parse_elf(&elf, &pelf, ELF_PARSE_PHDR))
+ return NULL; /* Not an ELF -- guess not a Qualcomm MBN after all? */
+
+ /* Qualcomm stores an array of SHA-384 hashes in a special ELF segment. One special one
+ to start with, and then one for each segment in order. */
+ void *bb_hash = NULL;
+ void *hashtable = NULL;
+ int i;
+ int bb_segment = -1;
+ for (i = 0; i < pelf.ehdr.e_phnum; i++) {
+ Elf64_Phdr *ph = &pelf.phdr[i];
+ if ((ph->p_flags & PF_QC_SG_MASK) == PF_QC_SG_HASH) {
+ if ((int)ph->p_filesz !=
+ (pelf.ehdr.e_phnum + 1) * VB2_SHA384_DIGEST_SIZE) {
+ ERROR("fixups: Qualcomm hash segment has wrong size!\n");
+ goto destroy_elf;
+ } /* Found the table with the hashes -- store its address. */
+ hashtable = buffer_get(&elf) + ph->p_offset;
+ } else if (bb_segment < 0 && ph->p_offset + ph->p_filesz < buffer_size(&elf) &&
+ buffer_offset(&elf) + ph->p_offset <= bb_offset &&
+ buffer_offset(&elf) + ph->p_offset + ph->p_filesz > bb_offset) {
+ bb_segment = i; /* Found the bootblock segment -- store its index. */
+ }
+ }
+ if (!hashtable) /* ELF but no special QC hash segment -- guess not QC after all? */
+ goto destroy_elf;
+ if (bb_segment < 0) { /* Can assume it's QC if we found the special segment. */
+ ERROR("fixups: Cannot find bootblock code in Qualcomm MBN!\n");
+ goto destroy_elf;
+ }
+
+ /* Pass out the actual hash of the current bootblock segment in |real_hash|. */
+ if (vb2_hash_calculate(buffer_get(&elf) + pelf.phdr[bb_segment].p_offset,
+ pelf.phdr[bb_segment].p_filesz, VB2_HASH_SHA384, real_hash)) {
+ ERROR("fixups: vboot digest error\n");
+ goto destroy_elf;
+ } /* Return pointer to where the bootblock hash needs to go in Qualcomm's table. */
+ bb_hash = hashtable + (bb_segment + 1) * VB2_SHA384_DIGEST_SIZE;
+
+destroy_elf:
+ parsed_elf_destroy(&pelf);
+ return bb_hash;
+}
+
+static bool qualcomm_probe(struct buffer *buffer, size_t offset)
+{
+ struct vb2_hash real_hash;
+ void *table_hash = qualcomm_find_hash(buffer, offset, &real_hash);
+ if (!table_hash)
+ return false;
+
+ if (memcmp(real_hash.raw, table_hash, VB2_SHA384_DIGEST_SIZE)) {
+ ERROR("fixups: Identified Qualcomm MBN, but existing hash doesn't match!\n");
+ return false;
+ }
+
+ return true;
+}
+
+static int qualcomm_fixup(struct buffer *buffer, size_t offset)
+{
+ struct vb2_hash real_hash;
+ void *table_hash = qualcomm_find_hash(buffer, offset, &real_hash);
+ if (!table_hash) {
+ ERROR("fixups: Cannot find Qualcomm MBN headers anymore!\n");
+ return -1;
+ }
+
+ memcpy(table_hash, real_hash.raw, VB2_SHA384_DIGEST_SIZE);
+ INFO("fixups: Updated Qualcomm MBN header bootblock hash.\n");
+ return 0;
+}
+
+platform_fixup_func platform_fixups_probe(struct buffer *buffer, size_t offset,
+ const char *region_name)
+{
+ if (!strcmp(region_name, SECTION_NAME_BOOTBLOCK)) {
+ if (qualcomm_probe(buffer, offset))
+ return qualcomm_fixup;
+ } else if (!strcmp(region_name, SECTION_NAME_PRIMARY_CBFS)) {
+ /* TODO: add fixups for primary CBFS bootblock platforms, if needed */
+ } else {
+ ERROR("%s called for unexpected FMAP region %s!\n", __func__, region_name);
+ }
+
+ return NULL;
+}
--
To view, visit https://review.coreboot.org/c/coreboot/+/41122
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I02a83a40f1d0009e6f9561ae5d2d9f37a510549a
Gerrit-Change-Number: 41122
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newchange