coreboot-gerrit October 2020

coreboot-gerrit@coreboot.org

1 participants
3474 discussions

Change in coreboot[master]: sec/intel/txt/ramstage.c: Do not init the heap on S3 resume
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46495 ) Change subject: sec/intel/txt/ramstage.c: Do not init the heap on S3 resume ...................................................................... sec/intel/txt/ramstage.c: Do not init the heap on S3 resume It causes problems on Haswell: SINIT detects that the heap tables differ in size, and then issues a Class Code 9, Major Error Code 1 TXT reset. Change-Id: I26f3d291abc7b2263e0b115e94426ac6ec8e5c48 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/ramstage.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/46495/1 diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c index 2d56d1f..00e9ce7 100644 --- a/src/security/intel/txt/ramstage.c +++ b/src/security/intel/txt/ramstage.c @@ -407,7 +407,9 @@ write64((void *)TXT_MSEG_SIZE, 0); write64((void *)TXT_MSEG_BASE, 0); - txt_initialize_heap(); + /* Only initialize the heap on regular boots */ + if (!acpi_is_wakeup_s3()) + txt_initialize_heap(); if (CONFIG(INTEL_TXT_LOGGING)) txt_dump_regions(); -- To view, visit https://review.coreboot.org/c/coreboot/+/46495 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I26f3d291abc7b2263e0b115e94426ac6ec8e5c48 Gerrit-Change-Number: 46495 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: sec/intel/txt/ramstage.c: Extract heap init into a function
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46494 ) Change subject: sec/intel/txt/ramstage.c: Extract heap init into a function ...................................................................... sec/intel/txt/ramstage.c: Extract heap init into a function Heap initialization is self-contained, so place it into a separate function. Also, do it after the MSEG registers have been written, so that all register writes are grouped together. This has no impact. Change-Id: Id108f4cfcd2896d881d9ba267888f7ed5dd984fa Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/ramstage.c 1 file changed, 101 insertions(+), 96 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/46494/1 diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c index 5d0d121..2d56d1f 100644 --- a/src/security/intel/txt/ramstage.c +++ b/src/security/intel/txt/ramstage.c @@ -194,103 +194,8 @@ } } -/** - * Finalize the TXT device. - * - * - Lock TXT register. - * - Protect TSEG using DMA protected regions. - * - Setup TXT regions. - * - Place SINIT ACM in TXT_SINIT memory segment. - * - Fill TXT BIOSDATA region. - */ -static void lockdown_intel_txt(void *unused) +static void txt_initialize_heap(void) { - const uint64_t status = read64((void *)TXT_SPAD); - - uintptr_t tseg_base; - size_t tseg_size; - - smm_region(&tseg_base, &tseg_size); - - if (status & ACMSTS_TXT_DISABLED) - return; - - printk(BIOS_INFO, "TEE-TXT: Locking TEE...\n"); - - /* Lock TXT config, unlocks TXT_HEAP_BASE */ - if (intel_txt_run_bios_acm(ACMINPUT_LOCK_CONFIG) < 0) { - printk(BIOS_ERR, "TEE-TXT: Failed to lock registers.\n"); - printk(BIOS_ERR, "TEE-TXT: SINIT won't be supported.\n"); - return; - } - - /* - * Document Number: 558294 - * Chapter 5.5.6.1 DMA Protection Memory Region - */ - - const u8 dpr_capable = !!(read64((void *)TXT_CAPABILITIES) & - TXT_CAPABILITIES_DPR); - printk(BIOS_INFO, "TEE-TXT: DPR capable %x\n", dpr_capable); - - if (dpr_capable) { - /* Verify the DPR settings on the MCH and mirror them to TXT public space */ - union dpr_register dpr = txt_get_chipset_dpr(); - - printk(BIOS_DEBUG, "TEE-TXT: MCH DPR 0x%08x\n", dpr.raw); - - printk(BIOS_DEBUG, "TEE-TXT: MCH DPR base @ 0x%08x size %u MiB\n", - (dpr.top - dpr.size) * MiB, dpr.size); - - // DPR TODO: implement SA_ENABLE_DPR in the intelblocks - - if (!dpr.lock) { - printk(BIOS_ERR, "TEE-TXT: MCH DPR not locked.\n"); - return; - } - - if (!dpr.epm || !dpr.prs) { - printk(BIOS_ERR, "TEE-TXT: MCH DPR protection not active.\n"); - return; - } - - if (dpr.size < CONFIG_INTEL_TXT_DPR_SIZE) { - printk(BIOS_ERR, "TEE-TXT: MCH DPR configured size is too small.\n"); - return; - } - - if (dpr.top * MiB != tseg_base) { - printk(BIOS_ERR, "TEE-TXT: MCH DPR top does not equal TSEG base.\n"); - return; - } - - /* Clear reserved bits */ - dpr.prs = 0; - dpr.epm = 0; - - write64((void *)TXT_DPR, dpr.raw); - - printk(BIOS_INFO, "TEE-TXT: TXT.DPR 0x%08x\n", - read32((void *)TXT_DPR)); - } - - /* - * Document Number: 558294 - * Chapter 5.5.6.3 Intel TXT Heap Memory Region - */ - write64((void *)TXT_HEAP_SIZE, 0xE0000); - write64((void *)TXT_HEAP_BASE, - ALIGN_DOWN(tseg_base - read64((void *)TXT_HEAP_SIZE), 4096)); - - /* - * Document Number: 558294 - * Chapter 5.5.6.2 SINIT Memory Region - */ - write64((void *)TXT_SINIT_SIZE, 0x20000); - write64((void *)TXT_SINIT_BASE, - ALIGN_DOWN(read64((void *)TXT_HEAP_BASE) - - read64((void *)TXT_SINIT_SIZE), 4096)); - /* * BIOS Data Format * Chapter C.2 @@ -392,6 +297,104 @@ /* SinitMLEData */ /* FIXME: Does firmware need to write this? */ push_sinit_heap(&heap_struct, NULL, 0); +} + +/** + * Finalize the TXT device. + * + * - Lock TXT register. + * - Protect TSEG using DMA protected regions. + * - Setup TXT regions. + * - Place SINIT ACM in TXT_SINIT memory segment. + * - Fill TXT BIOSDATA region. + */ +static void lockdown_intel_txt(void *unused) +{ + const uint64_t status = read64((void *)TXT_SPAD); + + uintptr_t tseg_base; + size_t tseg_size; + + smm_region(&tseg_base, &tseg_size); + + if (status & ACMSTS_TXT_DISABLED) + return; + + printk(BIOS_INFO, "TEE-TXT: Locking TEE...\n"); + + /* Lock TXT config, unlocks TXT_HEAP_BASE */ + if (intel_txt_run_bios_acm(ACMINPUT_LOCK_CONFIG) < 0) { + printk(BIOS_ERR, "TEE-TXT: Failed to lock registers.\n"); + printk(BIOS_ERR, "TEE-TXT: SINIT won't be supported.\n"); + return; + } + + /* + * Document Number: 558294 + * Chapter 5.5.6.1 DMA Protection Memory Region + */ + + const u8 dpr_capable = !!(read64((void *)TXT_CAPABILITIES) & + TXT_CAPABILITIES_DPR); + printk(BIOS_INFO, "TEE-TXT: DPR capable %x\n", dpr_capable); + + if (dpr_capable) { + /* Verify the DPR settings on the MCH and mirror them to TXT public space */ + union dpr_register dpr = txt_get_chipset_dpr(); + + printk(BIOS_DEBUG, "TEE-TXT: MCH DPR 0x%08x\n", dpr.raw); + + printk(BIOS_DEBUG, "TEE-TXT: MCH DPR base @ 0x%08x size %u MiB\n", + (dpr.top - dpr.size) * MiB, dpr.size); + + // DPR TODO: implement SA_ENABLE_DPR in the intelblocks + + if (!dpr.lock) { + printk(BIOS_ERR, "TEE-TXT: MCH DPR not locked.\n"); + return; + } + + if (!dpr.epm || !dpr.prs) { + printk(BIOS_ERR, "TEE-TXT: MCH DPR protection not active.\n"); + return; + } + + if (dpr.size < CONFIG_INTEL_TXT_DPR_SIZE) { + printk(BIOS_ERR, "TEE-TXT: MCH DPR configured size is too small.\n"); + return; + } + + if (dpr.top * MiB != tseg_base) { + printk(BIOS_ERR, "TEE-TXT: MCH DPR top does not equal TSEG base.\n"); + return; + } + + /* Clear reserved bits */ + dpr.prs = 0; + dpr.epm = 0; + + write64((void *)TXT_DPR, dpr.raw); + + printk(BIOS_INFO, "TEE-TXT: TXT.DPR 0x%08x\n", + read32((void *)TXT_DPR)); + } + + /* + * Document Number: 558294 + * Chapter 5.5.6.3 Intel TXT Heap Memory Region + */ + write64((void *)TXT_HEAP_SIZE, 0xE0000); + write64((void *)TXT_HEAP_BASE, + ALIGN_DOWN(tseg_base - read64((void *)TXT_HEAP_SIZE), 4096)); + + /* + * Document Number: 558294 + * Chapter 5.5.6.2 SINIT Memory Region + */ + write64((void *)TXT_SINIT_SIZE, 0x20000); + write64((void *)TXT_SINIT_BASE, + ALIGN_DOWN(read64((void *)TXT_HEAP_BASE) - + read64((void *)TXT_SINIT_SIZE), 4096)); /* * FIXME: Server-TXT capable platforms need to install an STM in SMM and set up MSEG. @@ -404,6 +407,8 @@ write64((void *)TXT_MSEG_SIZE, 0); write64((void *)TXT_MSEG_BASE, 0); + txt_initialize_heap(); + if (CONFIG(INTEL_TXT_LOGGING)) txt_dump_regions(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/46494 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id108f4cfcd2896d881d9ba267888f7ed5dd984fa Gerrit-Change-Number: 46494 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: sec/intel/txt: Add and fill in BIOS Specification info
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46493 ) Change subject: sec/intel/txt: Add and fill in BIOS Specification info ...................................................................... sec/intel/txt: Add and fill in BIOS Specification info This is not critical to function, but is nice to have. Change-Id: Ieb5f41f3e4c5644a31606434916c35542d35617a Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/ramstage.c 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46493/1 diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c index 8d9f5d9..5d0d121 100644 --- a/src/security/intel/txt/ramstage.c +++ b/src/security/intel/txt/ramstage.c @@ -298,6 +298,7 @@ */ struct { struct txt_biosdataregion bdr; + struct txt_bios_spec_ver_element spec; struct txt_heap_acm_element heap_acm; struct txt_extended_data_element_header end; } __packed data = {0}; @@ -350,6 +351,13 @@ data.bdr.support_acpi_ppi = 0; data.bdr.platform_type = 0; + /* Fill in the version of the used TXT BIOS Specification */ + data.spec.header.type = HEAP_EXTDATA_TYPE_BIOS_SPEC_VER; + data.spec.header.size = sizeof(data.spec); + data.spec.ver_major = 2; + data.spec.ver_minor = 1; + data.spec.ver_revision = 0; + /* Extended elements - ACM addresses */ data.heap_acm.header.type = HEAP_EXTDATA_TYPE_ACM; data.heap_acm.header.size = sizeof(data.heap_acm); -- To view, visit https://review.coreboot.org/c/coreboot/+/46493 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieb5f41f3e4c5644a31606434916c35542d35617a Gerrit-Change-Number: 46493 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 4

Change in coreboot[master]: sec/intel/txt/common.c: Only log ACM error on failure
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46492 ) Change subject: sec/intel/txt/common.c: Only log ACM error on failure ...................................................................... sec/intel/txt/common.c: Only log ACM error on failure The TXT_BIOSACM_ERRORCODE register is only valid if bit 62 is set, or if CBnT is supported and bit 61 is set. Moreover, it is specific to LT-SX. This allows TXT to work on client platforms, where these registers are regular scratchpads and are not necessarily written to by the BIOS ACM. Change-Id: If047ad79f12de5e0f34227198ee742b9e2b5eb54 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/common.c 1 file changed, 0 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/46492/1 diff --git a/src/security/intel/txt/common.c b/src/security/intel/txt/common.c index 5f8a976..737ab0a 100644 --- a/src/security/intel/txt/common.c +++ b/src/security/intel/txt/common.c @@ -303,8 +303,6 @@ intel_txt_log_acm_error(read32((void *)TXT_BIOSACM_ERRORCODE)); return -1; } - if (intel_txt_log_acm_error(read32((void *)TXT_BIOSACM_ERRORCODE)) != 1) - return -1; return 0; } -- To view, visit https://review.coreboot.org/c/coreboot/+/46492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If047ad79f12de5e0f34227198ee742b9e2b5eb54 Gerrit-Change-Number: 46492 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

3 8

Change in coreboot[master]: sec/intel/txt: Move DPR size to Kconfig
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46491 ) Change subject: sec/intel/txt: Move DPR size to Kconfig ...................................................................... sec/intel/txt: Move DPR size to Kconfig Instead of hardcoding the size in code, expose it as a Kconfig symbol. This allows platform code to program the size in the MCH DPR register. Change-Id: I9b9bcfc7ceefea6882f8133a6c3755da2e64a80c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/security/intel/txt/Kconfig M src/security/intel/txt/ramstage.c 2 files changed, 10 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46491/1 diff --git a/src/security/intel/txt/Kconfig b/src/security/intel/txt/Kconfig index 3dd912e..c69a217 100644 --- a/src/security/intel/txt/Kconfig +++ b/src/security/intel/txt/Kconfig @@ -31,6 +31,15 @@ access to Intel resources. Or for some platforms found inside the blob repository. +config INTEL_TXT_DPR_SIZE + int "DMA Protected Region size in MiB" + range 0 255 + default 3 + help + Specify the size the DPR region needs to have. On at least Haswell, + the MRC does not have an input to specify the size of DPR, so this + field is only used to check if the programmed size is large enough. + config INTEL_TXT_LOGGING bool "Enable verbose logging" help diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c index f532a2f..8d9f5d9 100644 --- a/src/security/intel/txt/ramstage.c +++ b/src/security/intel/txt/ramstage.c @@ -254,7 +254,7 @@ return; } - if (dpr.size < 3) { + if (dpr.size < CONFIG_INTEL_TXT_DPR_SIZE) { printk(BIOS_ERR, "TEE-TXT: MCH DPR configured size is too small.\n"); return; } -- To view, visit https://review.coreboot.org/c/coreboot/+/46491 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9b9bcfc7ceefea6882f8133a6c3755da2e64a80c Gerrit-Change-Number: 46491 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 7

Change in coreboot[master]: configs: Add TXT-enabled config for Asrock B85M Pro4
by Angel Pons (Code Review) 22 Oct '20

22 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46489 ) Change subject: configs: Add TXT-enabled config for Asrock B85M Pro4 ...................................................................... configs: Add TXT-enabled config for Asrock B85M Pro4 This config selects the necessary options to enable Intel TXT on the Asrock B85M Pro4, and allows the code to be build-tested. Note that the current TXT code will not work, as it was written for Broadwell-DE. Subsequent commits will adapt the code as necessary to work on Haswell. Compatible BIOS and SINIT ACMs can be retrieved from a firmware update for the Supermicro X10SLH. As they are not in the blobs repository, use the STM binary as a placeholder so as to allow build-testing the code. Change-Id: Ibf8db5fdfac5b527520023277c6370f6efa71717 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- A configs/config.asrock_b85m_pro4.tpm2_txt_placeholder_acms 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/46489/1 diff --git a/configs/config.asrock_b85m_pro4.tpm2_txt_placeholder_acms b/configs/config.asrock_b85m_pro4.tpm2_txt_placeholder_acms new file mode 100644 index 0000000..856701f --- /dev/null +++ b/configs/config.asrock_b85m_pro4.tpm2_txt_placeholder_acms @@ -0,0 +1,10 @@ +# Known-working configuration to boot with TXT enabled. Since BIOS +# and SINIT ACM blobs are missing, use something else as placeholder. +# Used ACMs were extracted from a Supermicro X10SLH firmware update. +CONFIG_VENDOR_ASROCK=y +CONFIG_BOARD_ASROCK_B85M_PRO4=y +CONFIG_USER_TPM2=y +CONFIG_INTEL_TXT=y +CONFIG_INTEL_TXT_BIOSACM_FILE="3rdparty/blobs/cpu/intel/stm/stm.bin" +CONFIG_INTEL_TXT_SINITACM_FILE="3rdparty/blobs/cpu/intel/stm/stm.bin" +CONFIG_INTEL_TXT_LOGGING=y -- To view, visit https://review.coreboot.org/c/coreboot/+/46489 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf8db5fdfac5b527520023277c6370f6efa71717 Gerrit-Change-Number: 46489 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

4 8

Change in coreboot[master]: Revert "soc/intel/jasperlake: Allow mainboard to override chip config...
by Karthik Ramasubramanian (Code Review) 22 Oct '20

22 Oct '20

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46653 ) Change subject: Revert "soc/intel/jasperlake: Allow mainboard to override chip configuration" ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/46653 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I166ba7e5ee50a6329032eae8e17b9a554b094e2e Gerrit-Change-Number: 46653 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 22 Oct 2020 19:47:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Revert "mb/google/dedede: Add mainboard acpi support for GPIO PM conf...
by Karthik Ramasubramanian (Code Review) 22 Oct '20

22 Oct '20

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46652 ) Change subject: Revert "mb/google/dedede: Add mainboard acpi support for GPIO PM configuration" ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/46652 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e3be42cd82fd3ae919d23d6f19c84a90b9c737a Gerrit-Change-Number: 46652 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 22 Oct 2020 19:47:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] console/init: Drop get_console_loglevel() API
by Nico Huber (Code Review) 22 Oct '20

22 Oct '20

Hello Martin Roth, Marc Jones, Johnny Lin, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/45771 to review the following change. Change subject: [RFC] console/init: Drop get_console_loglevel() API ...................................................................... [RFC] console/init: Drop get_console_loglevel() API It's not used anymore. We keep the CONSOLE_OVERRIDE_LOGLEVEL Kconfig, though, as it provides some user Kconfig comfort (we can hide unneces- sary prompts). Change-Id: Id6dd54534267c5419cbe81ee4b66f7b753a6e6cf Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/console/Kconfig M src/console/init.c M src/include/console/console.h 3 files changed, 4 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/45771/1 diff --git a/src/console/Kconfig b/src/console/Kconfig index bad6c56..57b932d 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -312,8 +312,9 @@ config CONSOLE_OVERRIDE_LOGLEVEL bool help - Set to "y" when the platform overrides the loglevel by providing - a get_console_loglevel routine. + Set to "y" when the platform unconditionally overrides the + loglevel default. Then we don't need to present the choice + below. if !CONSOLE_OVERRIDE_LOGLEVEL diff --git a/src/console/init.c b/src/console/init.c index 9776e2a..2be2e5a 100644 --- a/src/console/init.c +++ b/src/console/init.c @@ -21,7 +21,7 @@ static void init_log_level(void) { - int debug_level = get_console_loglevel(); + int debug_level = CONFIG_DEFAULT_CONSOLE_LOGLEVEL; get_option(&debug_level, "debug_level"); diff --git a/src/include/console/console.h b/src/include/console/console.h index a96eb15..bb19cb7 100644 --- a/src/include/console/console.h +++ b/src/include/console/console.h @@ -49,18 +49,6 @@ enum { CONSOLE_LOG_NONE = 0, CONSOLE_LOG_FAST, CONSOLE_LOG_ALL }; -#if CONFIG(CONSOLE_OVERRIDE_LOGLEVEL) -/* - * This function should be implemented at mainboard level. - * The returned value will _replace_ the loglevel value; - */ -int get_console_loglevel(void); -#else -static inline int get_console_loglevel(void) -{ - return CONFIG_DEFAULT_CONSOLE_LOGLEVEL; -} -#endif #else static inline void console_init(void) {} static inline int console_log_level(int msg_level) { return 0; } -- To view, visit https://review.coreboot.org/c/coreboot/+/45771 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6dd54534267c5419cbe81ee4b66f7b753a6e6cf Gerrit-Change-Number: 45771 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Marc Jones <marc.jones(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: [RFC] mb/scaleway: Use mainboard_get_option() API
by Nico Huber (Code Review) 22 Oct '20

22 Oct '20

Hello Martin Roth, Marc Jones, Johnny Lin, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/45770 to review the following change. Change subject: [RFC] mb/scaleway: Use mainboard_get_option() API ...................................................................... [RFC] mb/scaleway: Use mainboard_get_option() API Change-Id: I1acb11d76fd7620001f84b2c3331820dcaab8875 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/mainboard/scaleway/tagada/Kconfig M src/mainboard/scaleway/tagada/bmcinfo.c 2 files changed, 10 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/45770/1 diff --git a/src/mainboard/scaleway/tagada/Kconfig b/src/mainboard/scaleway/tagada/Kconfig index cbbb6ae..ab51b1e 100644 --- a/src/mainboard/scaleway/tagada/Kconfig +++ b/src/mainboard/scaleway/tagada/Kconfig @@ -8,6 +8,7 @@ select BOARD_ROMSIZE_KB_16384 select HAVE_ACPI_TABLES select CONSOLE_OVERRIDE_LOGLEVEL + select USE_MAINBOARD_OPTIONS select UART_OVERRIDE_BAUDRATE config MAINBOARD_DIR diff --git a/src/mainboard/scaleway/tagada/bmcinfo.c b/src/mainboard/scaleway/tagada/bmcinfo.c index 099d37e..982c09c 100644 --- a/src/mainboard/scaleway/tagada/bmcinfo.c +++ b/src/mainboard/scaleway/tagada/bmcinfo.c @@ -1,8 +1,10 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <stdint.h> +#include <string.h> #include <console/console.h> #include <console/uart.h> +#include <option.h> #include "bmcinfo.h" @@ -172,10 +174,13 @@ return 115200; } -#if __CONSOLE_ENABLE__ /* Override default console loglevel */ -int get_console_loglevel(void) +enum cb_err mainboard_get_option(void *const dest, const char *const name) { - return bmcinfo_coreboot_verbosity_level(); + if (strcmp(name, "debug_level") == 0) { + *(char *)dest = bmcinfo_coreboot_verbosity_level(); + return CB_SUCCESS; + } + + return CB_CMOS_OPTION_NOT_FOUND; } -#endif -- To view, visit https://review.coreboot.org/c/coreboot/+/45770 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1acb11d76fd7620001f84b2c3331820dcaab8875 Gerrit-Change-Number: 45770 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Marc Jones <marc.jones(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newchange

2 3

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2020