coreboot-gerrit November 2019

coreboot-gerrit@coreboot.org

1 participants
2418 discussions

Change in coreboot[master]: mb/google/drallion: Disable GPIO dynamic PM configuration
by Subrata Banik (Code Review) 28 Nov '19

28 Nov '19

Hello EricR Lai, Selma Bensaid, Bora Guvendik, build bot (Jenkins), Furquan Shaikh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37320 to look at the new patch set (#2). Change subject: mb/google/drallion: Disable GPIO dynamic PM configuration ...................................................................... mb/google/drallion: Disable GPIO dynamic PM configuration BUG=b:144002424 TEST=Ensured no TPM time out issue and system can boot to OS Change-Id: I7282e6c2d9627846039638bdc0db3ee7ebba5f12 Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> --- M src/mainboard/google/drallion/variants/arcada_cml/devicetree.cb M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/drallion/variants/sarien_cml/devicetree.cb 3 files changed, 21 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/37320/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37320 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7282e6c2d9627846039638bdc0db3ee7ebba5f12 Gerrit-Change-Number: 37320 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/drallion: [WIP] Disable GPIO dynamic PM configuration
by Subrata Banik (Code Review) 28 Nov '19

28 Nov '19

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37320 ) Change subject: mb/google/drallion: [WIP] Disable GPIO dynamic PM configuration ...................................................................... Patch Set 1: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/37320 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7282e6c2d9627846039638bdc0db3ee7ebba5f12 Gerrit-Change-Number: 37320 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 28 Nov 2019 09:27:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cannonlake: [WIP] Ensure bootblock disable GPIO PM configur...
by Subrata Banik (Code Review) 28 Nov '19

28 Nov '19

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37319 ) Change subject: soc/intel/cannonlake: [WIP] Ensure bootblock disable GPIO PM configuration ...................................................................... Patch Set 1: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/37319 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I54061d556d62462d9012bc47bb9f3604a3e5a250 Gerrit-Change-Number: 37319 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 28 Nov 2019 09:27:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/poppy: auto-select correct audio blobs
by Matt DeVillier (Code Review) 28 Nov '19

28 Nov '19

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37316 ) Change subject: mb/google/poppy: auto-select correct audio blobs ...................................................................... mb/google/poppy: auto-select correct audio blobs Auto-select the correct audio blobs needed for each variant if INCLUDE_NHLT_BLOBS is selected. Audio is non-functional without them, so include as appropriate. Change-Id: Iedee3fa41a69ea1b8077bbe4bd0be278fd5c5343 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/mainboard/google/poppy/Kconfig 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/37316/1 diff --git a/src/mainboard/google/poppy/Kconfig b/src/mainboard/google/poppy/Kconfig index d7808ed..590bd7f 100644 --- a/src/mainboard/google/poppy/Kconfig +++ b/src/mainboard/google/poppy/Kconfig @@ -142,6 +142,7 @@ select DRIVERS_USB_ACPI select EXCLUDE_NATIVE_SD_INTERFACE select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR + select INCLUDE_NHLT_BLOBS_ATLAS if INCLUDE_NHLT_BLOBS select INTEL_GMA_HAVE_VBT select MAINBOARD_HAS_SPI_TPM_CR50 select VARIANT_HAS_CAMERA_ACPI @@ -160,6 +161,7 @@ select DRIVERS_I2C_DA7219 select DRIVERS_SPI_ACPI select EXCLUDE_NATIVE_SD_INTERFACE + select INCLUDE_NHLT_BLOBS_NAMI if INCLUDE_NHLT_BLOBS select INTEL_GMA_HAVE_VBT select MAINBOARD_HAS_SPI_TPM_CR50 @@ -168,6 +170,7 @@ select CHROMEOS_WIFI_SAR if CHROMEOS select DRIVERS_GENERIC_MAX98357A select DRIVERS_I2C_DA7219 + select INCLUDE_NHLT_BLOBS_NAUTILUS if INCLUDE_NHLT_BLOBS select INTEL_GMA_HAVE_VBT select MAINBOARD_HAS_I2C_TPM_CR50 select VARIANT_HAS_CAMERA_ACPI @@ -182,6 +185,7 @@ select DRIVERS_USB_ACPI select EXCLUDE_NATIVE_SD_INTERFACE select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR + select INCLUDE_NHLT_BLOBS_NOCTURNE if INCLUDE_NHLT_BLOBS select INTEL_GMA_HAVE_VBT select MAINBOARD_HAS_SPI_TPM_CR50 select NO_FADT_8042 @@ -194,6 +198,7 @@ select DRIVERS_I2C_DA7219 select DRIVERS_SPI_ACPI select DRIVERS_USB_ACPI + select INCLUDE_NHLT_BLOBS_RAMMUS if INCLUDE_NHLT_BLOBS select INTEL_GMA_HAVE_VBT select MAINBOARD_HAS_LIBGFXINIT select MAINBOARD_HAS_SPI_TPM_CR50 -- To view, visit https://review.coreboot.org/c/coreboot/+/37316 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iedee3fa41a69ea1b8077bbe4bd0be278fd5c5343 Gerrit-Change-Number: 37316 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: mb/google/poppy: auto-select correct audio blobs
by Matt DeVillier (Code Review) 28 Nov '19

28 Nov '19

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37310 ) Change subject: mb/google/poppy: auto-select correct audio blobs ...................................................................... mb/google/poppy: auto-select correct audio blobs Auto-select the correct audio blobs needed for each variant if USE_BLOBS is selected. Audio is non-functional without them, so only reason to not select is for build testing w/o USE_BLOBS. Alphabetize config options selected by each variant since we're already touching them. Change-Id: I96b9e21aa0b392d07b4f49ed6b39b49d39892fb6 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/mainboard/google/poppy/Kconfig 1 file changed, 11 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/37310/1 diff --git a/src/mainboard/google/poppy/Kconfig b/src/mainboard/google/poppy/Kconfig index 85dc9a5..e8fb38d 100644 --- a/src/mainboard/google/poppy/Kconfig +++ b/src/mainboard/google/poppy/Kconfig @@ -49,6 +49,7 @@ config INCLUDE_NHLT_BLOBS bool "Include blobs for audio." + default y if USE_BLOBS select NHLT_DMIC_2CH select NHLT_DMIC_4CH select NHLT_MAX98927 @@ -142,16 +143,17 @@ select DRIVERS_SPI_ACPI select DRIVERS_USB_ACPI select EXCLUDE_NATIVE_SD_INTERFACE + select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR + select INCLUDE_NHLT_BLOBS_ATLAS if USE_BLOBS select MAINBOARD_HAS_SPI_TPM_CR50 select VARIANT_HAS_CAMERA_ACPI - select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR config VARIANT_SPECIFIC_OPTIONS_POPPY def_bool n select DRIVERS_I2C_MAX98927 + select MAINBOARD_HAS_I2C_TPM_CR50 select NO_FADT_8042 select VARIANT_HAS_CAMERA_ACPI - select MAINBOARD_HAS_I2C_TPM_CR50 config VARIANT_SPECIFIC_OPTIONS_NAMI def_bool n @@ -160,6 +162,7 @@ select DRIVERS_I2C_DA7219 select DRIVERS_SPI_ACPI select EXCLUDE_NATIVE_SD_INTERFACE + select INCLUDE_NHLT_BLOBS_NAMI if USE_BLOBS select MAINBOARD_HAS_SPI_TPM_CR50 config VARIANT_SPECIFIC_OPTIONS_NAUTILUS @@ -167,8 +170,9 @@ select CHROMEOS_WIFI_SAR if CHROMEOS select DRIVERS_GENERIC_MAX98357A select DRIVERS_I2C_DA7219 - select VARIANT_HAS_CAMERA_ACPI + select INCLUDE_NHLT_BLOBS_NAUTILUS if USE_BLOBS select MAINBOARD_HAS_I2C_TPM_CR50 + select VARIANT_HAS_CAMERA_ACPI config VARIANT_SPECIFIC_OPTIONS_NOCTURNE def_bool n @@ -179,10 +183,11 @@ select DRIVERS_SPI_ACPI select DRIVERS_USB_ACPI select EXCLUDE_NATIVE_SD_INTERFACE + select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR + select INCLUDE_NHLT_BLOBS_NOCTURNE if USE_BLOBS select MAINBOARD_HAS_SPI_TPM_CR50 select NO_FADT_8042 select VARIANT_HAS_CAMERA_ACPI - select GEO_SAR_ENABLE if CHROMEOS_WIFI_SAR config VARIANT_SPECIFIC_OPTIONS_RAMMUS def_bool n @@ -191,14 +196,15 @@ select DRIVERS_I2C_DA7219 select DRIVERS_SPI_ACPI select DRIVERS_USB_ACPI + select INCLUDE_NHLT_BLOBS_RAMMUS if USE_BLOBS select MAINBOARD_HAS_SPI_TPM_CR50 config VARIANT_SPECIFIC_OPTIONS_SORAKA def_bool n select DRIVERS_I2C_MAX98927 select NO_FADT_8042 - select VARIANT_HAS_CAMERA_ACPI select MAINBOARD_HAS_I2C_TPM_CR50 + select VARIANT_HAS_CAMERA_ACPI config VBOOT select EC_GOOGLE_CHROMEEC_SWITCHES -- To view, visit https://review.coreboot.org/c/coreboot/+/37310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I96b9e21aa0b392d07b4f49ed6b39b49d39892fb6 Gerrit-Change-Number: 37310 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-MessageType: newchange

1 2

Change in coreboot[master]: mb/google/hatch: Support separate fixed PL1 setting for tablet mode
by shkim (Code Review) 28 Nov '19

28 Nov '19

Hello Seunghwan Kim, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/35533 to review the following change. Change subject: mb/google/hatch: Support separate fixed PL1 setting for tablet mode ...................................................................... mb/google/hatch: Support separate fixed PL1 setting for tablet mode Some variants need separate PL1 setting since thier thermal tuning, but DPTF passive policy 1.0 cannot support it. So we would use MMIO PL1 register which is currently unused, it can reserve another PL1 setting for tablet mode. If the MMIO PL1 setting is lower than DPTF PL1 max setting, it will limit PL1 when DPTF sets higher PL1 in tablet mode. Otherwise, if the MMIO PL1 setting is lower than DPTF PL1 min setting, system will have a fixed PL1 value in tablet mode. BUG=b:138395625 BRANCH=none TEST=Verified MMIO PL1 value and it's enabled in tablet mode when it set Change-Id: I81c33f9df3e5431f04a08395141b5dc989474289 Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.com> --- M src/ec/google/chromeec/acpi/ec.asl M src/mainboard/google/hatch/mainboard.asl M src/mainboard/google/hatch/variants/kohaku/include/variant/ec.h M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/cpu.c 5 files changed, 32 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/35533/1 diff --git a/src/ec/google/chromeec/acpi/ec.asl b/src/ec/google/chromeec/acpi/ec.asl index 962988e..0c4cc35 100644 --- a/src/ec/google/chromeec/acpi/ec.asl +++ b/src/ec/google/chromeec/acpi/ec.asl @@ -378,6 +378,9 @@ #ifdef EC_ENABLE_TBMC_DEVICE Notify (TBMC, 0x80) #endif +#ifdef EC_ENABLE_MAINBOERD_TABLET_MODE_EVENT + \_SB.MTME(^TBMD) +#endif } /* diff --git a/src/mainboard/google/hatch/mainboard.asl b/src/mainboard/google/hatch/mainboard.asl index dff1a75..769d429 100644 --- a/src/mainboard/google/hatch/mainboard.asl +++ b/src/mainboard/google/hatch/mainboard.asl @@ -55,3 +55,22 @@ LOCL (0) } } + +/* + * Additional action for tablet mode switch event + * Called from \_SB.PCI0.LPCB.EC0._Q1D + */ +Method (MTME, 1, Serialized) +{ + OperationRegion (MCHB, + SystemMemory, Add (\_SB.PCI0.GMHB(), 0x5000), 0x1000) + Field (MCHB, DWordAcc, Lock, Preserve) + { + Offset (0x9a0), /* PKG_POWER_LIMIT_LO */ + , 15, /* PKG_POWER_LIMIT_MASK */ + MP1E, 1, /* PKG_POWER_LIMIT_EN */ + } + + /* Enable MMIO PL1 at tablet mode */ + Store(Arg0, MP1E) +} diff --git a/src/mainboard/google/hatch/variants/kohaku/include/variant/ec.h b/src/mainboard/google/hatch/variants/kohaku/include/variant/ec.h index 84e050e..7cc5269 100644 --- a/src/mainboard/google/hatch/variants/kohaku/include/variant/ec.h +++ b/src/mainboard/google/hatch/variants/kohaku/include/variant/ec.h @@ -19,5 +19,6 @@ #include <baseboard/ec.h> #define EC_ENABLE_MULTIPLE_DPTF_PROFILES +#define EC_ENABLE_MAINBOERD_TABLET_MODE_EVENT #endif diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index 451c920..df6be35 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -232,6 +232,8 @@ /* PL1 Override value in Watts */ uint32_t tdp_pl1_override; + /* PL1 MMIO Override value in Watts */ + uint32_t mmio_pl1_override; /* PL2 Override value in Watts */ uint32_t tdp_pl2_override; /* SysPL2 Value in Watts */ diff --git a/src/soc/intel/cannonlake/cpu.c b/src/soc/intel/cannonlake/cpu.c index 0f4d52e..4ec45e5 100644 --- a/src/soc/intel/cannonlake/cpu.c +++ b/src/soc/intel/cannonlake/cpu.c @@ -160,6 +160,13 @@ limit.hi |= PKG_POWER_LIMIT_CLAMP; limit.hi |= PKG_POWER_LIMIT_EN; + /* Set PL1 MMIO override value */ + if (conf->mmio_pl1_override) { + tdp_pl1 = (conf->mmio_pl1_override * power_unit); + limit.lo &= (~(PKG_POWER_LIMIT_MASK)); + limit.lo |= tdp_pl1 & PKG_POWER_LIMIT_MASK; + } + /* Power limit 2 time is only programmable on server SKU */ wrmsr(MSR_PKG_POWER_LIMIT, limit); -- To view, visit https://review.coreboot.org/c/coreboot/+/35533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81c33f9df3e5431f04a08395141b5dc989474289 Gerrit-Change-Number: 35533 Gerrit-PatchSet: 1 Gerrit-Owner: shkim <sh_.kim(a)samsung.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Seunghwan Kim <sh_.kim(a)samsung.com> Gerrit-Reviewer: shkim <sh_.kim(a)samsung.com> Gerrit-MessageType: newchange

6 9

Change in coreboot[master]: mb/google/kohaku: Set mmio_pl1_override value for tablet mode
by shkim (Code Review) 28 Nov '19

28 Nov '19

Hello Seunghwan Kim, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/35534 to review the following change. Change subject: mb/google/kohaku: Set mmio_pl1_override value for tablet mode ...................................................................... mb/google/kohaku: Set mmio_pl1_override value for tablet mode This change limits PL1 to 5W in tablet mode for kohaku. BUG=b:138395625 BRANCH=none TEST=Verified PL1 is limited in tablet mode Change-Id: I79bea32cf46ffa50f83af3905f85a471cb94b339 Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.com> --- M src/mainboard/google/hatch/variants/kohaku/overridetree.cb 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/35534/1 diff --git a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb index 8c7bb1f..9614ef8 100644 --- a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb +++ b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb @@ -1,6 +1,7 @@ chip soc/intel/cannonlake register "tdp_pl1_override" = "8" register "tdp_pl2_override" = "51" + register "mmio_pl1_override" = "5" register "tcc_offset" = "35" # TCC of 65C -- To view, visit https://review.coreboot.org/c/coreboot/+/35534 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I79bea32cf46ffa50f83af3905f85a471cb94b339 Gerrit-Change-Number: 35534 Gerrit-PatchSet: 1 Gerrit-Owner: shkim <sh_.kim(a)samsung.com> Gerrit-Reviewer: Seunghwan Kim <sh_.kim(a)samsung.com> Gerrit-MessageType: newchange

4 6

Change in coreboot[master]: vboot: Don't perform EC sync in recovery mode
by Tim Wawrzynczak (Code Review) 28 Nov '19

28 Nov '19

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37304 ) Change subject: vboot: Don't perform EC sync in recovery mode ...................................................................... vboot: Don't perform EC sync in recovery mode In recovery mode, EC sync is not supposed to be performed (everything should be running RO firmware). Also move nvdata_save() to the failure path only, otherwise if the FSP-S decides a system reboot is necessary, any recovery reason will be lost. Note that this is the same logic that depthcharge uses as well. BUG=b:145310842 BRANCH=firmware-hatch-12672.B TEST=Verify no EC sync happens in recovery mode Change-Id: Ifb82396416d6b9af82fc75b3372f8339091469a4 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/security/vboot/ec_sync.c 1 file changed, 7 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/37304/1 diff --git a/src/security/vboot/ec_sync.c b/src/security/vboot/ec_sync.c index c2a6b25..67c5ae0 100644 --- a/src/security/vboot/ec_sync.c +++ b/src/security/vboot/ec_sync.c @@ -57,12 +57,14 @@ ctx = vboot_get_context(); ctx->flags |= VB2_CONTEXT_EC_SYNC_SUPPORTED; - retval = vb2api_ec_sync(ctx); - vboot_save_nvdata_only(ctx); + if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { + retval = vb2api_ec_sync(ctx); - if (retval != VB2_SUCCESS) { - printk(BIOS_ERR, "EC software sync failed (%#x), rebooting\n", retval); - vboot_reboot(); + if (retval != VB2_SUCCESS) { + printk(BIOS_ERR, "EC software sync failed (%#x), rebooting\n", retval); + vboot_save_nvdata_only(ctx); + vboot_reboot(); + } } timestamp_add_now(TS_END_EC_SYNC); -- To view, visit https://review.coreboot.org/c/coreboot/+/37304 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifb82396416d6b9af82fc75b3372f8339091469a4 Gerrit-Change-Number: 37304 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

4 7

Change in coreboot[master]: security/vboot: Use persistent context to read GBB flags
by Julius Werner (Code Review) 28 Nov '19

28 Nov '19

Hello Joel Kitching, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/37261 to review the following change. Change subject: security/vboot: Use persistent context to read GBB flags ...................................................................... security/vboot: Use persistent context to read GBB flags With the persistent vboot context coreboot no longer needs to read GBB flags from flash itself -- it can just ask vboot for the cached result. This patch removes the existing GBB code and provides gbb_is_flag_set() (with a slightly better namespaced name) as a static inline instead. Change-Id: Ibc3ed0f3fbeb53d630925d47df4dc474b0ed07ee Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- D src/security/vboot/gbb.c D src/security/vboot/gbb.h M src/security/vboot/misc.h M src/security/vboot/vboot_common.c 4 files changed, 13 insertions(+), 121 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37261/1 diff --git a/src/security/vboot/gbb.c b/src/security/vboot/gbb.c deleted file mode 100644 index 5293033..0000000 --- a/src/security/vboot/gbb.c +++ /dev/null @@ -1,80 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright 2018 Google LLC - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#define NEED_VB20_INTERNALS /* Peeking into vb2_gbb_header */ - -#include <commonlib/region.h> -#include <console/console.h> -#include <fmap.h> -#include <security/vboot/gbb.h> -#include <string.h> -#include <vb2_api.h> - -#define GBB_FMAP_REGION_NAME "GBB" - -/* Copy of GBB header read from boot media. */ -static struct vb2_gbb_header gbb_header; - -/* - * Read "GBB" region from SPI flash to obtain GBB header and validate - * signature. - * - * Return value: - * Success = 0 - * Error = 1 - */ -static int gbb_init(void) -{ - static bool init_done = false; - struct region_device gbb_rdev; - - if (init_done != false) - return 0; - - if (fmap_locate_area_as_rdev(GBB_FMAP_REGION_NAME, &gbb_rdev)) - return 1; - - if (rdev_readat(&gbb_rdev, &gbb_header, 0, - sizeof(struct vb2_gbb_header)) != - sizeof(struct vb2_gbb_header)) { - printk(BIOS_ERR, "%s: Failure to read GBB header!\n", __func__); - return 1; - } - - if (memcmp(gbb_header.signature, VB2_GBB_SIGNATURE, - VB2_GBB_SIGNATURE_SIZE)) { - printk(BIOS_ERR, "%s: Signature check failed!\n", __func__); - return 1; - } - - init_done = true; - return 0; -} - -uint32_t gbb_get_flags(void) -{ - if (gbb_init()) { - printk(BIOS_ERR, - "%s: Failure to initialize GBB. Returning flags as 0!\n", - __func__); - return 0; - } - return gbb_header.flags; -} - -bool gbb_is_flag_set(uint32_t flag) -{ - return !!(gbb_get_flags() & flag); -} diff --git a/src/security/vboot/gbb.h b/src/security/vboot/gbb.h deleted file mode 100644 index 389242a..0000000 --- a/src/security/vboot/gbb.h +++ /dev/null @@ -1,39 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright 2018 Google LLC - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef __SECURITY_VBOOT_GBB_H__ -#define __SECURITY_VBOOT_GBB_H__ - -#include <stdint.h> - -/* In order to use VB2_GBB_FLAG_* macros from vboot, include vb2_api.h. */ - -/* - * Read flags field from GBB header. - * Return value: - * Success: 32-bit unsigned integer representing flags field from GBB header. - * Error : 0 - */ -uint32_t gbb_get_flags(void); - -/* - * Check if given flag is set in the flags field in GBB header. - * Return value: - * true: Flag is set. - * false: Flag is not set or failure to read GBB flags. - */ -bool gbb_is_flag_set(uint32_t flag); - -#endif /* __SECURITY_VBOOT_GBB_H__ */ diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h index 1b14799..471f838 100644 --- a/src/security/vboot/misc.h +++ b/src/security/vboot/misc.h @@ -50,6 +50,17 @@ } /* + * Check if given flag is set in the flags field in GBB header. + * Return value: + * true: Flag is set. + * false: Flag is not set. + */ +static inline bool vboot_is_gbb_flag_set(enum vb2_gbb_flag flag) +{ + return !!(vb2api_gbb_get_flags(vboot_get_context()) & flag); +} + +/* * Locates firmware as a region device. Returns 0 on success, -1 on failure. */ int vboot_locate_firmware(const struct vb2_context *ctx, diff --git a/src/security/vboot/vboot_common.c b/src/security/vboot/vboot_common.c index a24b220..458ed87 100644 --- a/src/security/vboot/vboot_common.c +++ b/src/security/vboot/vboot_common.c @@ -19,7 +19,7 @@ #include <fmap.h> #include <reset.h> #include <stddef.h> -#include <security/vboot/gbb.h> +#include <security/vboot/misc.h> #include <security/vboot/vboot_common.h> #include <security/vboot/vbnv.h> #include <vb2_api.h> @@ -31,7 +31,7 @@ if (!vboot_developer_mode_enabled()) return 0; /* Enable if GBB flag is set */ - if (gbb_is_flag_set(VB2_GBB_FLAG_ENABLE_UDC)) + if (vboot_is_gbb_flag_set(VB2_GBB_FLAG_ENABLE_UDC)) return 1; /* Enable if VBNV flag is set */ if (vbnv_udc_enable_flag()) -- To view, visit https://review.coreboot.org/c/coreboot/+/37261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibc3ed0f3fbeb53d630925d47df4dc474b0ed07ee Gerrit-Change-Number: 37261 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Joel Kitching <kitching(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

2 5

Change in coreboot[4.11_branch]: soc/intel/broadwell_de: Re-read SPD on CRC error
by Werner Zeh (Code Review) 28 Nov '19

28 Nov '19

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37303 ) Change subject: soc/intel/broadwell_de: Re-read SPD on CRC error ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/37303/1/src/soc/intel/fsp_broadwel… File src/soc/intel/fsp_broadwell_de/romstage/memory.c: https://review.coreboot.org/c/coreboot/+/37303/1/src/soc/intel/fsp_broadwel… PS1, Line 63: while (res == SPD_STATUS_CRC_ERROR && tries++ < MAX_SPD_READ_TRIES) { : printk(BIOS_ERR, : "SPD CRC error on channel %d slot %d, retrying..\n", : channel, slot); : get_spd_smbus(&blk); : res = spd_decode_ddr4(&dimm, spd_data); : } : if (res == SPD_STATUS_OK) { How about using a do{}while loop here? In this case you would just need a single call to spd_decode_ddr4(&dimm, spd_data). Something like the following maybe? tries = MAX_SPD_READ_TRIES; do { get_spd_smbus(&blk); res = spd_decode_ddr4(&dimm, spd_data); if (res == SPD_STATUS_CRC_ERROR) printk(ERROR...); } while (res != SPD_STATUS_OK && tries--); -- To view, visit https://review.coreboot.org/c/coreboot/+/37303 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: 4.11_branch Gerrit-Change-Id: I650c8cd80f75b603db332024748a91af6171f096 Gerrit-Change-Number: 37303 Gerrit-PatchSet: 1 Gerrit-Owner: Andrey Petrov <anpetrov(a)fb.com> Gerrit-Reviewer: Andrey Petrov <anpetrov(a)fb.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 28 Nov 2019 05:50:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2019