coreboot-gerrit November 2020

coreboot-gerrit@coreboot.org

1 participants
3241 discussions

Change in coreboot[master]: FSP 2.0: Add fsp_relax_security
by Michael Niewöhner (Code Review) 11 Nov '20

11 Nov '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25437 ) Change subject: FSP 2.0: Add fsp_relax_security ...................................................................... Patch Set 20: (1 comment) https://review.coreboot.org/c/coreboot/+/25437/18/src/drivers/intel/fsp2_0/… File src/drivers/intel/fsp2_0/notify.c: https://review.coreboot.org/c/coreboot/+/25437/18/src/drivers/intel/fsp2_0/… PS18, Line 70: __attribute__((weak)) bool fsp_relax_security(void) > Well the purpose was to set it at runtime, not compile time ;) Huh? Tbh I'm not sure how that would be used, anyway. However, I wonder if we want to always add debug/testing functionality, even in production images. "Relax security" doesn't sound like something that should be always there. When using CHIPSET_LOCKDOWN_COREBOOT, wouldn't that be the same (setting it at build time)? -- To view, visit https://review.coreboot.org/c/coreboot/+/25437 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I972e7387c2612ee0053df21afb55b0b264961174 Gerrit-Change-Number: 25437 Gerrit-PatchSet: 20 Gerrit-Owner: Julien Viard de Galbert <coreboot-review-ju(a)vdg.name> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: David Guckian <d.guckian20(a)gmail.com> Gerrit-Reviewer: Julien Viard de Galbert <coreboot-review-ju(a)vdg.name> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 11 Nov 2020 17:06:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julien Viard de Galbert <coreboot-review-ju(a)vdg.name> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload/usb: Add format string checking to usb_debug
by Tim Wawrzynczak (Code Review) 11 Nov '20

11 Nov '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45025 ) Change subject: libpayload/usb: Add format string checking to usb_debug ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/45025 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic94ebcbafdde8a5f79278b5635111b99af40f892 Gerrit-Change-Number: 45025 Gerrit-PatchSet: 2 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 11 Nov 2020 16:28:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload/usb: Fix printf format string mismatches in debug messages
by Tim Wawrzynczak (Code Review) 11 Nov '20

11 Nov '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45024 ) Change subject: libpayload/usb: Fix printf format string mismatches in debug messages ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/45024/2/payloads/libpayload/driver… File payloads/libpayload/drivers/usb/usb.c: PS2: Does `#include <inttypes.h>` help? Maybe ARM isn't implicitly including it, but you should be adding the include anyway. -- To view, visit https://review.coreboot.org/c/coreboot/+/45024 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4dc70baefb3cd82fcc915cc2e7f68719cf6870cc Gerrit-Change-Number: 45024 Gerrit-PatchSet: 2 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 11 Nov 2020 16:23:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/asrock/h110m: add libgfxinit support
by Angel Pons (Code Review) 11 Nov '20

11 Nov '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39104 ) Change subject: mb/asrock/h110m: add libgfxinit support ...................................................................... Patch Set 15: (1 comment) https://review.coreboot.org/c/coreboot/+/39104/10//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/39104/10//COMMIT_MSG@7 PS10, Line 7: mb/asrock/h110m: add libgfxinit support > Unfortunately VGA works on Windows only. Uh, libgfxinit doesn't care about the OS (in fact, it never interacts with the OS). Does VGA light up with libgfxinit *before* an OS takes over? Also, does VGA work on linux with vendor firmware? -- To view, visit https://review.coreboot.org/c/coreboot/+/39104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d87413f87d00986111ecb7b046423ff5eac1bf1 Gerrit-Change-Number: 39104 Gerrit-PatchSet: 15 Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 11 Nov 2020 16:21:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Maxim Polyakov <max.senia.poliak(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] Add new board configuration entry to coreboot tables
by Tim Wawrzynczak (Code Review) 11 Nov '20

11 Nov '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46605 ) Change subject: [RFC] Add new board configuration entry to coreboot tables ...................................................................... [RFC] Add new board configuration entry to coreboot tables The new entry is intended to be the configuration entry to rule them all, possibly deprecating the three separate "strapping ID" tables with which it is slightly redundant with. In one entry, it should provide enough information to give firmware the information it needs at runtime to make decisions based on what kind of board it is or what is attached to it, all in one place. The new entry contains: - Board ID - DRAM ID (RAM code) - SKU ID - FW Config Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I1ecec847ee77b72233587c1ad7f124e2027470bf --- M payloads/libpayload/include/coreboot_tables.h M src/commonlib/include/commonlib/coreboot_tables.h M src/include/fw_config.h M src/lib/coreboot_table.c 4 files changed, 59 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/46605/1 diff --git a/payloads/libpayload/include/coreboot_tables.h b/payloads/libpayload/include/coreboot_tables.h index bf8c0d9..fc10031 100644 --- a/payloads/libpayload/include/coreboot_tables.h +++ b/payloads/libpayload/include/coreboot_tables.h @@ -79,6 +79,7 @@ CB_TAG_MMC_INFO = 0x0035, CB_TAG_TCPA_LOG = 0x0036, CB_TAG_FMAP = 0x0037, + CB_TAG_BOARD_CONFIG = 0x0039, CB_TAG_CMOS_OPTION_TABLE = 0x00c8, CB_TAG_OPTION = 0x00c9, CB_TAG_OPTION_ENUM = 0x00ca, @@ -316,6 +317,16 @@ int32_t early_cmd1_status; }; +struct cb_board_config { + uint32_t tag; + uint32_t size; + + struct cbuint64 fw_config; + uint32_t board_id; + uint32_t dram_id; + uint32_t sku_id; +}; + #define CB_MAX_SERIALNO_LENGTH 32 struct cb_cmos_option_table { diff --git a/src/commonlib/include/commonlib/coreboot_tables.h b/src/commonlib/include/commonlib/coreboot_tables.h index 6393c01..4fb7318 100644 --- a/src/commonlib/include/commonlib/coreboot_tables.h +++ b/src/commonlib/include/commonlib/coreboot_tables.h @@ -80,6 +80,8 @@ LB_TAG_TCPA_LOG = 0x0036, LB_TAG_FMAP = 0x0037, LB_TAG_PLATFORM_BLOB_VERSION = 0x0038, + LB_TAG_BOARD_CONFIG = 0x0039, + /* The following options are CMOS-related */ LB_TAG_CMOS_OPTION_TABLE = 0x00c8, LB_TAG_OPTION = 0x00c9, LB_TAG_OPTION_ENUM = 0x00ca, @@ -415,6 +417,16 @@ struct mac_address mac_addrs[0]; }; +struct lb_board_config { + uint32_t tag; + uint32_t size; + + struct lb_uint64 fw_config; + uint32_t board_id; + uint32_t dram_id; + uint32_t sku_id; +}; + #define MAX_SERIALNO_LENGTH 32 /* The following structures are for the CMOS definitions table */ diff --git a/src/include/fw_config.h b/src/include/fw_config.h index c9ef75f..7eaad7d 100644 --- a/src/include/fw_config.h +++ b/src/include/fw_config.h @@ -8,6 +8,8 @@ #include <stdbool.h> #include <stdint.h> +#define DEFAULT_FW_CONFIG (uint64_t)0 + /** * struct fw_config - Firmware configuration field and option. * @field_name: Name of the field that this option belongs to. diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 9148405..82b718a 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -1,22 +1,23 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <arch/cbconfig.h> -#include <console/console.h> -#include <console/uart.h> -#include <ip_checksum.h> +#include <boardid.h> #include <boot/coreboot_tables.h> #include <boot/tables.h> #include <boot_device.h> -#include <string.h> -#include <version.h> -#include <boardid.h> -#include <device/device.h> -#include <fmap.h> -#include <stdlib.h> -#include <cbfs.h> -#include <cbmem.h> #include <bootmem.h> #include <bootsplash.h> +#include <cbfs.h> +#include <cbmem.h> +#include <console/console.h> +#include <console/uart.h> +#include <device/device.h> +#include <fmap.h> +#include <fw_config.h> +#include <ip_checksum.h> +#include <stdlib.h> +#include <string.h> +#include <version.h> #include <spi_flash.h> #include <security/vboot/misc.h> #include <security/vboot/vbnv_layout.h> @@ -211,6 +212,7 @@ __weak uint32_t board_id(void) { return UNDEFINED_STRAPPING_ID; } __weak uint32_t ram_code(void) { return UNDEFINED_STRAPPING_ID; } __weak uint32_t sku_id(void) { return UNDEFINED_STRAPPING_ID; } +__weak uint64_t fw_config_get(void) { return DEFAULT_FW_CONFIG; } static void lb_board_id(struct lb_header *header) { @@ -368,6 +370,24 @@ return mainboard; } +static struct lb_board_config *lb_board_config(struct lb_header *header) +{ + struct lb_record *rec; + struct lb_board_config *config; + rec = lb_new_record(header); + config = (struct lb_board_config *)rec; + + config->tag = LB_TAG_BOARD_CONFIG; + config->size = sizeof(*config); + + config->board_id = board_id(); + config->dram_id = ram_code(); + config->sku_id = sku_id(); + config->fw_config = pack_lb64(fw_config_get()); + + return config; +} + #if CONFIG(USE_OPTION_TABLE) static struct cmos_checksum *lb_cmos_checksum(struct lb_header *header) { @@ -557,6 +577,9 @@ lb_boot_media_params(head); + /* Board configuration information */ + lb_board_config(head); + /* Add architecture records. */ lb_arch_add_records(head); -- To view, visit https://review.coreboot.org/c/coreboot/+/46605 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1ecec847ee77b72233587c1ad7f124e2027470bf Gerrit-Change-Number: 46605 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

7 38

Change in coreboot[master]: mb/asrock/h110m: add libgfxinit support
by Maxim Polyakov (Code Review) 11 Nov '20

11 Nov '20

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39104 ) Change subject: mb/asrock/h110m: add libgfxinit support ...................................................................... Patch Set 15: (3 comments) https://review.coreboot.org/c/coreboot/+/39104/10//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/39104/10//COMMIT_MSG@7 PS10, Line 7: mb/asrock/h110m: add libgfxinit support > You can add `DP2, HDMI2, Internal` (the latter will be `eDP` in the future) […] Unfortunately VGA works on Windows only. https://review.coreboot.org/c/coreboot/+/39104/13//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/39104/13//COMMIT_MSG@14 PS13, Line 14: - EDP_TXN[2] -> EDP_VGA_TX0# : - EDP_TXN[3] -> EDP_VGA_TX1# : - EDP_TXP[2] -> EDP_VGA_TX0 : - EDP_TXP[2] -> EDP_VGA_TX1 > Where does this come from? boardview https://review.coreboot.org/c/coreboot/+/39104/13/src/mainboard/asrock/h110… File src/mainboard/asrock/h110m/gma-mainboard.ads: https://review.coreboot.org/c/coreboot/+/39104/13/src/mainboard/asrock/h110… PS13, Line 12: (DP2, -- DVI (DDI2) : HDMI3, -- HDMI (DDI3) : eDP, -- eDP->VGA (PTN3356R1BS) : others => Disabled); > On SKL/KBL-S there are three DDIs plus one eDP port. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/39104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d87413f87d00986111ecb7b046423ff5eac1bf1 Gerrit-Change-Number: 39104 Gerrit-PatchSet: 15 Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 11 Nov 2020 15:26:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Maxim Polyakov <max.senia.poliak(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: FSP 2.0: Add fsp_relax_security
by Julien Viard de Galbert (Code Review) 11 Nov '20

11 Nov '20

Julien Viard de Galbert has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25437 ) Change subject: FSP 2.0: Add fsp_relax_security ...................................................................... Patch Set 20: (2 comments) Thanks for the hint, I'll use CHIPSET_LOCKDOWN_{FSP,COREBOOT} on the next update of this patch. https://review.coreboot.org/c/coreboot/+/25437/18/src/drivers/intel/fsp2_0/… File src/drivers/intel/fsp2_0/notify.c: https://review.coreboot.org/c/coreboot/+/25437/18/src/drivers/intel/fsp2_0/… PS18, Line 70: __attribute__((weak)) bool fsp_relax_security(void) > instead of *another* callback, why not have a Kconfig? Well the purpose was to set it at runtime, not compile time ;) https://review.coreboot.org/c/coreboot/+/25437/18/src/drivers/intel/fsp2_0/… PS18, Line 70: fsp_relax_security > Maybe rename this to sound less like an action? E. g. […] Right I missed the introduction of CHIPSET_LOCKDOWN_COREBOOT, I can certainly rewrite it using that. Will do. -- To view, visit https://review.coreboot.org/c/coreboot/+/25437 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I972e7387c2612ee0053df21afb55b0b264961174 Gerrit-Change-Number: 25437 Gerrit-PatchSet: 20 Gerrit-Owner: Julien Viard de Galbert <coreboot-review-ju(a)vdg.name> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: David Guckian <d.guckian20(a)gmail.com> Gerrit-Reviewer: Julien Viard de Galbert <coreboot-review-ju(a)vdg.name> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 11 Nov 2020 15:19:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/asrock/h110m: add libgfxinit support
by Maxim Polyakov (Code Review) 11 Nov '20

11 Nov '20

Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/39104 to look at the new patch set (#15). Change subject: mb/asrock/h110m: add libgfxinit support ...................................................................... mb/asrock/h110m: add libgfxinit support Add libgfxinit support for Asrock H110M-DVS-R2 [1]. - DVI (DDI2); - HDMI (DDI3); - VGA (eDP), PTN3356R1BS, DisplayPort to VGA adapter [2]. VGA works with Windows 10 only. [1] https://www.2by2.be/images/photolib/604201.jpg [2] https://www.nxp.com/docs/en/data-sheet/PTN3356R1.pdf Change-Id: I4d87413f87d00986111ecb7b046423ff5eac1bf1 Signed-off-by: Maxim Polyakov <max.senia.poliak(a)gmail.com> --- M src/mainboard/asrock/h110m/Kconfig M src/mainboard/asrock/h110m/gma-mainboard.ads 2 files changed, 4 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/39104/15 -- To view, visit https://review.coreboot.org/c/coreboot/+/39104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d87413f87d00986111ecb7b046423ff5eac1bf1 Gerrit-Change-Number: 39104 Gerrit-PatchSet: 15 Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common: Add support for CSE Lite driver to handle CSE RW an...
by Sridhar Siricilla (Code Review) 11 Nov '20

11 Nov '20

Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46552 ) Change subject: soc/intel/common: Add support for CSE Lite driver to handle CSE RW and metadata ...................................................................... soc/intel/common: Add support for CSE Lite driver to handle CSE RW and metadata The patch modifies CSE Lite driver to support to have separate CBFS file for metadata of RW blobs. So, the driver expects separate CBFS files for CSE RW and it's metadata. Also, the driver initiates CSE Firmware update path if it finds the metadata's CBFS file. The change gives flexibility to modify RW blob's FMAP locations while maintaining uniform or little modified CSE Lite driver. Currently, CSE RW blob and it's metadata were unified into one RW blob. TEST=Verified on hatch. BRANCH=Puff Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I316344d23987d9907b64efafbe61f36ae7ade337 --- M src/soc/intel/common/block/cse/Kconfig M src/soc/intel/common/block/cse/Makefile.inc M src/soc/intel/common/block/cse/cse_lite.c 3 files changed, 128 insertions(+), 71 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/46552/1 diff --git a/src/soc/intel/common/block/cse/Kconfig b/src/soc/intel/common/block/cse/Kconfig index 1cb7d35..6512e3d 100644 --- a/src/soc/intel/common/block/cse/Kconfig +++ b/src/soc/intel/common/block/cse/Kconfig @@ -37,3 +37,15 @@ default "" help Intel CSE CBFS RW blob path and file name + +config SOC_INTEL_CSE_RW_METADATA_CBFS_NAME + string "CBFS entry name for CSE RW blob" + default "me_rw.metadata" + help + CBFS entry name for Intel CSE CBFS RW blob + +config SOC_INTEL_CSE_RW_METADATA_FILE + string "Intel CSE CBFS RW path and filename" + default "" + help + Intel CSE CBFS RW blob path and file name diff --git a/src/soc/intel/common/block/cse/Makefile.inc b/src/soc/intel/common/block/cse/Makefile.inc index 11cc3c2..59ba0cb 100644 --- a/src/soc/intel/common/block/cse/Makefile.inc +++ b/src/soc/intel/common/block/cse/Makefile.inc @@ -4,11 +4,20 @@ ramstage-$(CONFIG_SOC_INTEL_CSE_LITE_SKU) += cse_lite.c smm-$(CONFIG_SOC_INTEL_COMMON_BLOCK_HECI_DISABLE_IN_SMM) += disable_heci.c +ifneq ($(CONFIG_SOC_INTEL_CSE_RW_METADATA_FILE),"") +CSE_LITE_RW_METADATA = $(call strip_quotes,$(CONFIG_SOC_INTEL_CSE_RW_METADATA_CBFS_NAME)) +regions-for-file-$(CSE_LITE_RW_METADATA) = FW_MAIN_A,FW_MAIN_B +cbfs-files-y += $(CSE_LITE_RW_METADATA) +$(CSE_LITE_RW_METADATA)-file := $(call strip_quotes,$(CONFIG_SOC_INTEL_CSE_RW_METADATA_FILE)) +$(CSE_LITE_RW_METADATA)-name := $(CSE_LITE_RW_METADATA) +$(CSE_LITE_RW_METADATA)-type := raw +endif + ifneq ($(CONFIG_SOC_INTEL_CSE_RW_FILE),"") -CSE_LITE_ME_RW = $(call strip_quotes,$(CONFIG_SOC_INTEL_CSE_RW_CBFS_NAME)) -regions-for-file-$(CSE_LITE_ME_RW) = FW_MAIN_A,FW_MAIN_B -cbfs-files-y += $(CSE_LITE_ME_RW) -$(CSE_LITE_ME_RW)-file := $(call strip_quotes,$(CONFIG_SOC_INTEL_CSE_RW_FILE)) -$(CSE_LITE_ME_RW)-name := $(CSE_LITE_ME_RW) -$(CSE_LITE_ME_RW)-type := raw +CSE_LITE_RW = $(call strip_quotes, $(CONFIG_SOC_INTEL_CSE_RW_CBFS_NAME)) +regions-for-file-$(CSE_LITE_RW) = FW_MAIN_A, FW_MAIN_B +cbfs-files-y += $(CSE_LITE_RW) +$(CSE_LITE_RW)-file := $(call strip_quotes,$(CONFIG_SOC_INTEL_CSE_RW_FILE)) +$(CSE_LITE_RW)-name := $(CSE_LITE_RW) +$(CSE_LITE_RW)-type := raw endif diff --git a/src/soc/intel/common/block/cse/cse_lite.c b/src/soc/intel/common/block/cse/cse_lite.c index c9e4e1f..7218a92 100644 --- a/src/soc/intel/common/block/cse/cse_lite.c +++ b/src/soc/intel/common/block/cse/cse_lite.c @@ -12,9 +12,6 @@ #include <vb2_api.h> #include <soc/intel/common/reset.h> -/* CSE RW version size reserved in the CSE CBFS RW binary */ -#define CSE_RW_VERSION_SZ 16 - /* Converts bp index to boot partition string */ #define GET_BP_STR(bp_index) (bp_index ? "RW" : "RO") @@ -45,6 +42,9 @@ /* CSE recovery sub-error codes */ enum csme_failure_reason { + + /* No error */ + CSE_LITE_SKU_NO_ERROR = 0, /* Unspecified error */ CSE_LITE_SKU_UNSPECIFIED = 1, @@ -64,7 +64,10 @@ CSE_LITE_SKU_COMMUNICATION_ERROR = 6, /* Fails to wipe CSE runtime data */ - CSE_LITE_SKU_DATA_WIPE_ERROR = 7 + CSE_LITE_SKU_DATA_WIPE_ERROR = 7, + + /* CSE RW is not found */ + CSE_LITE_SKU_RW_BLOB_NOT_FOUND = 8, }; /* @@ -157,6 +160,13 @@ struct cse_bp_info bp_info; } __packed; +/* CSE CBFS RW metadata */ +struct cse_cbfs_rw_info { + /* CSE CBFS RW version */ + struct fw_version fw_ver; + uint8_t reserved[8]; +}; + static void cse_log_status_registers(void) { printk(BIOS_DEBUG, "cse_lite: CSE status registers: HFSTS1: 0x%x, HFSTS2: 0x%x " @@ -423,7 +433,7 @@ { struct cbfsf file_desc; - if (cbfs_boot_locate(&file_desc, CONFIG_SOC_INTEL_CSE_RW_CBFS_NAME, NULL) < 0) + if (cbfs_boot_locate(&file_desc, CONFIG_SOC_INTEL_CSE_RW_METADATA_CBFS_NAME, NULL) < 0) return false; cbfs_file_data(source_rdev, &file_desc); @@ -503,15 +513,33 @@ return true; } -static bool cse_get_cbfs_rw_version(const struct region_device *source_rdev, - void *cse_cbfs_rw_ver) +static bool cse_get_cbfs_rw(struct region_device *rdev) +{ + struct cbfsf file_desc; + + if (cbfs_boot_locate(&file_desc, CONFIG_SOC_INTEL_CSE_RW_CBFS_NAME, NULL) < 0) + return false; + + cbfs_file_data(rdev, &file_desc); + + return true; +} + +static bool cse_get_cbfs_rw_metadata(const struct region_device *source_info_rdev, + struct cse_cbfs_rw_info *source_info) { - if (rdev_readat(source_rdev, (void *) cse_cbfs_rw_ver, 0, sizeof(struct fw_version)) - != sizeof(struct fw_version)) { - printk(BIOS_ERR, "cse_lite: Failed to read CSE CBFW RW version\n"); + if (rdev_readat(source_info_rdev, (void *) source_info, 0, + sizeof(struct cse_cbfs_rw_info)) != sizeof(struct cse_cbfs_rw_info)) { + printk(BIOS_ERR, "cse_lite: Failed to read CSE CBFS RW metadata\n"); return false; } + + printk(BIOS_SPEW, "cse_lite: CSE CBFS RW version : %d.%d.%d.%d\n", + source_info->fw_ver.major, + source_info->fw_ver.minor, + source_info->fw_ver.hotfix, + source_info->fw_ver.build); return true; } @@ -522,30 +550,26 @@ * If ver_cmp_status > 0, coreboot upgrades CSE RW region */ static int cse_check_version_mismatch(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev) + const struct cse_cbfs_rw_info *source_info) { - struct fw_version cse_cbfs_rw_ver; const struct fw_version *cse_rw_ver; - if (!cse_get_cbfs_rw_version(source_rdev, &cse_cbfs_rw_ver)) - return false; - printk(BIOS_DEBUG, "cse_lite: CSE CBFS RW version : %d.%d.%d.%d\n", - cse_cbfs_rw_ver.major, - cse_cbfs_rw_ver.minor, - cse_cbfs_rw_ver.hotfix, - cse_cbfs_rw_ver.build); + source_info->fw_ver.major, + source_info->fw_ver.minor, + source_info->fw_ver.hotfix, + source_info->fw_ver.build); cse_rw_ver = cse_get_rw_version(cse_bp_info); - if (cse_cbfs_rw_ver.major != cse_rw_ver->major) - return cse_cbfs_rw_ver.major - cse_rw_ver->major; - else if (cse_cbfs_rw_ver.minor != cse_rw_ver->minor) - return cse_cbfs_rw_ver.minor - cse_rw_ver->minor; - else if (cse_cbfs_rw_ver.hotfix != cse_rw_ver->hotfix) - return cse_cbfs_rw_ver.hotfix - cse_rw_ver->hotfix; + if (source_info->fw_ver.major != cse_rw_ver->major) + return source_info->fw_ver.major - cse_rw_ver->major; + else if (source_info->fw_ver.minor != cse_rw_ver->minor) + return source_info->fw_ver.minor - cse_rw_ver->minor; + else if (source_info->fw_ver.hotfix != cse_rw_ver->hotfix) + return source_info->fw_ver.hotfix - cse_rw_ver->hotfix; else - return cse_cbfs_rw_ver.build - cse_rw_ver->build; + return source_info->fw_ver.build - cse_rw_ver->build; } /* Check if CSE RW data partition is valid or not */ @@ -587,8 +611,8 @@ return true; } -static bool cse_copy_rw(const struct region_device *target_rdev, const void *buf, size_t offset, - size_t size) +static bool cse_copy_rw(const struct region_device *target_rdev, const void *buf, + size_t offset, size_t size) { if (rdev_writeat(target_rdev, buf, offset, size) < 0) { printk(BIOS_ERR, "cse_lite: Failed to update CSE firmware\n"); @@ -599,36 +623,35 @@ } static bool cse_is_rw_version_latest(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev) + const struct cse_cbfs_rw_info *source_info) { - return !cse_check_version_mismatch(cse_bp_info, source_rdev); + return !cse_check_version_mismatch(cse_bp_info, source_info); } static bool cse_is_downgrade_instance(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev) + const struct cse_cbfs_rw_info *source_info) { - return cse_check_version_mismatch(cse_bp_info, source_rdev) < 0; + return cse_check_version_mismatch(cse_bp_info, source_info) < 0; } static bool cse_is_update_required(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev, struct region_device *target_rdev) + const struct cse_cbfs_rw_info *source_info, struct region_device *target_rdev) { return (!cse_is_rw_bp_sign_valid(target_rdev) || - !cse_is_rw_version_latest(cse_bp_info, source_rdev)); + !cse_is_rw_version_latest(cse_bp_info, source_info)); } static bool cse_write_rw_region(const struct region_device *target_rdev, const struct region_device *source_rdev) { - void *cse_cbfs_rw = rdev_mmap(source_rdev, CSE_RW_VERSION_SZ, - region_device_sz(source_rdev) - CSE_RW_VERSION_SZ); + + void *cse_cbfs_rw = rdev_mmap(source_rdev, 0, region_device_sz(source_rdev)); /* Points to CSE CBFS RW image after boot partition signature */ uint8_t *cse_cbfs_rw_wo_sign = (uint8_t *)cse_cbfs_rw + CSE_RW_SIGN_SIZE; /* Size of CSE CBFS RW image without boot partition signature */ - uint32_t cse_cbfs_rw_wo_sign_sz = region_device_sz(source_rdev) - - (CSE_RW_VERSION_SZ + CSE_RW_SIGN_SIZE); + uint32_t cse_cbfs_rw_wo_sign_sz = region_device_sz(source_rdev) - CSE_RW_SIGN_SIZE; /* Update except CSE RW signature */ if (!cse_copy_rw(target_rdev, cse_cbfs_rw_wo_sign, CSE_RW_SIGN_SIZE, @@ -639,29 +662,32 @@ if (!cse_copy_rw(target_rdev, (void *)cse_cbfs_rw, 0, CSE_RW_SIGN_SIZE)) goto exit_rw_update; - rdev_munmap(source_rdev, cse_cbfs_rw_wo_sign); - return true; + printk(BIOS_INFO, "cse_lite: CSE RW Update Successful\n"); + rdev_munmap(source_rdev, cse_cbfs_rw); + return CSE_LITE_SKU_NO_ERROR; exit_rw_update: - rdev_munmap(source_rdev, cse_cbfs_rw_wo_sign); - return false; + rdev_munmap(source_rdev, cse_cbfs_rw); + return CSE_LITE_SKU_FW_UPDATE_ERROR; } static bool cse_update_rw(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev, struct region_device *target_rdev) + struct region_device *target_rdev) { + + struct region_device source_rdev; + + if (!cse_get_cbfs_rw(&source_rdev)) + return CSE_LITE_SKU_RW_BLOB_NOT_FOUND; + if (!cse_erase_rw_region(target_rdev)) - return false; + return CSE_LITE_SKU_FW_UPDATE_ERROR; - if (!cse_write_rw_region(target_rdev, source_rdev)) - return false; - - printk(BIOS_INFO, "cse_lite: CSE RW Update Successful\n"); - return true; + return cse_write_rw_region(target_rdev, &source_rdev); } static bool cse_prep_for_rw_update(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev) + const struct cse_cbfs_rw_info *source_info) { /* * To set CSE's operation mode to HMRFPO mode: @@ -671,7 +697,7 @@ if (!cse_boot_to_ro(cse_bp_info)) return false; - if (cse_is_downgrade_instance(cse_bp_info, source_rdev) && + if (cse_is_downgrade_instance(cse_bp_info, source_info) && !cse_data_clear_request(cse_bp_info)) { printk(BIOS_ERR, "cse_lite: CSE FW downgrade is aborted\n"); return false; @@ -680,20 +706,18 @@ return cse_hmrfpo_enable(); } -static uint8_t cse_trigger_fw_update(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev, struct region_device *target_rdev) +static enum csme_failure_reason cse_trigger_fw_update(const struct cse_bp_info *cse_bp_info, + const struct cse_cbfs_rw_info *source_info, struct region_device *target_rdev) { - if (!cse_prep_for_rw_update(cse_bp_info, source_rdev)) + + if (!cse_prep_for_rw_update(cse_bp_info, source_info)) return CSE_LITE_SKU_COMMUNICATION_ERROR; - if (!cse_update_rw(cse_bp_info, source_rdev, target_rdev)) - return CSE_LITE_SKU_FW_UPDATE_ERROR; - - return 0; + return cse_update_rw(cse_bp_info, target_rdev); } static uint8_t cse_fw_update(const struct cse_bp_info *cse_bp_info, - const struct region_device *source_rdev) + const struct region_device *source_info_rdev) { struct region_device target_rdev; @@ -702,9 +726,17 @@ return CSE_LITE_SKU_RW_ACCESS_ERROR; } - if (cse_is_update_required(cse_bp_info, source_rdev, &target_rdev)) { + struct cse_cbfs_rw_info source_info; + + /* Read CSE CBFS RW metadata */ + if (!cse_get_cbfs_rw_metadata(source_info_rdev, &source_info)) { + printk(BIOS_ERR, "cse_lite: Failed to get CSE RW blob metadata\n"); + return CSE_LITE_SKU_UNSPECIFIED; + } + + if (cse_is_update_required(cse_bp_info, &source_info, &target_rdev)) { printk(BIOS_DEBUG, "cse_lite: CSE RW update is initiated\n"); - return cse_trigger_fw_update(cse_bp_info, source_rdev, &target_rdev); + return cse_trigger_fw_update(cse_bp_info, &source_info, &target_rdev); } if (!cse_is_rw_bp_status_valid(cse_bp_info)) @@ -736,11 +768,15 @@ if (!cse_fix_data_failure_err(&cse_bp_info.bp_info)) cse_trigger_recovery(CSE_LITE_SKU_DATA_WIPE_ERROR); - /* If RW blob is present in CBFS, then trigger CSE firmware update */ + /* + * If CSE CBFS RW blob metadata is present in CBFS, then trigger CSE firmware update. + * The driver triggers recovery if CSE CBFS RW metadata is present in the CBFS and CSE + * CBFS RW blob is not available. + */ uint8_t rv; - struct region_device source_rdev; - if (cse_get_cbfs_rdev(&source_rdev)) { - rv = cse_fw_update(&cse_bp_info.bp_info, &source_rdev); + struct region_device source_info_rdev; + if (cse_get_cbfs_rdev(&source_info_rdev)) { + rv = cse_fw_update(&cse_bp_info.bp_info, &source_info_rdev); if (rv) cse_trigger_recovery(rv); } -- To view, visit https://review.coreboot.org/c/coreboot/+/46552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I316344d23987d9907b64efafbe61f36ae7ade337 Gerrit-Change-Number: 46552 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 11

Change in coreboot[master]: sc7180: add qclib_post_fixup to allow SoC specific code
by mturney mturney (Code Review) 11 Nov '20

11 Nov '20

mturney mturney has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37795 ) Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ...................................................................... sc7180: add qclib_post_fixup to allow SoC specific code * Provide hook for SoC specific code to run after QCLib Change-Id: I5507187fddfd7d0af0ee0983b06d67077de0dbc8 Signed-off-by: T Michael Turney <mturney(a)codeaurora.org> --- M src/soc/qualcomm/common/include/soc/qclib_common.h M src/soc/qualcomm/common/qclib.c 2 files changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/37795/1 diff --git a/src/soc/qualcomm/common/include/soc/qclib_common.h b/src/soc/qualcomm/common/include/soc/qclib_common.h index 19ec083..5817c08 100644 --- a/src/soc/qualcomm/common/include/soc/qclib_common.h +++ b/src/soc/qualcomm/common/include/soc/qclib_common.h @@ -72,5 +72,6 @@ uint32_t size, uint32_t attrs); void qclib_load_and_run(void); int qclib_soc_blob_load(void); +int qclib_post_fixup(void); #endif // _SOC_QUALCOMM_QCLIB_COMMON_H_ diff --git a/src/soc/qualcomm/common/qclib.c b/src/soc/qualcomm/common/qclib.c index ac80a76..ce07078 100644 --- a/src/soc/qualcomm/common/qclib.c +++ b/src/soc/qualcomm/common/qclib.c @@ -119,6 +119,7 @@ } __weak int qclib_soc_blob_load(void) { return 0; } +__weak int qclib_post_fixup(void) { return 0; } void qclib_load_and_run(void) { @@ -209,6 +210,12 @@ mmu_restore_context(&pre_qclib_mmu_context); mmu_enable(); + /* hook for SoC specific fixup after QCLib runs */ + if (qclib_post_fixup()) { + printk(BIOS_ERR, "qclib_post_fixup failed\n"); + goto fail; + } + /* step through I/F table, handling return values */ for (i = 0; i < qclib_cb_if_table.num_entries; i++) if (qclib_cb_if_table.te[i].blob_attributes & -- To view, visit https://review.coreboot.org/c/coreboot/+/37795 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5507187fddfd7d0af0ee0983b06d67077de0dbc8 Gerrit-Change-Number: 37795 Gerrit-PatchSet: 1 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-MessageType: newchange

2 7

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2020