Attention is currently required from: Cliff Huang, Furquan Shaikh, Bernardo Perez Priego.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58126 )
Change subject: mb/google/brya: Add sub-regions to SI_ME in chromeos.fmd
......................................................................
Patch Set 18:
(1 comment)
File src/mainboard/google/brya/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/58126/comment/ec75b6dc_a0aa1333
PS18, Line 8: CSE_RW
Can CSE RW's starting offset be aligned @ 64K? This will help reducing the flashing time. We have to adjust 56K block as part of RO or DATA section. It is better we adjust the 56K size as part of R0!
--
To view, visit https://review.coreboot.org/c/coreboot/+/58126
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4da677da2e24b0398d04786e71490611db635ead
Gerrit-Change-Number: 58126
Gerrit-PatchSet: 18
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 07:47:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, SRIDHAR SIRICILLA, Furquan Shaikh, Paul Menzel, Rizwan Qureshi, Angel Pons, Subrata Banik, Sridhar Siricilla, Arthur Heymans, Evgeny Zinoviev, Patrick Rudolph, Felix Held.
Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52800 )
Change subject: soc/intel: Allow enable/disable ME via CMOS
......................................................................
Patch Set 77:
(2 comments)
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/52800/comment/39d071ca_af12dc77
PS77, Line 1003: printk(BIOS_ERR, "Error: Failed to change ME state in %u attempts!\n",
> is there a way to recover from this failure case? maybe clear the me_state_counter on a cold reset? […]
I think that could inadvertently create a loop if we made it turn off at attempt 3. Right now, it'll still boot - just with the ME in the wrong state.
https://review.coreboot.org/c/coreboot/+/52800/comment/9f5384bb_9a81bf11
PS77, Line 1009: do_global_reset();
> I think both options are required to be present to properly handle this, not sure though.
We've been using this for a while now without the counter, and apart from the one version of the ME that needs to be kicked 3 times before it will behave, we haven't had any issues.
I'd lean towards leaving it as but adding a comment saying "Add `me_state` and `me_state_counter`, you don't need `me_state_counter` but it's a good idea." (I wouldn't be surprised if there were a few people who wouldn't want their computers to turn on if the ME was in the wrong state.)
Or just make them both required:
```
if me_state == UINT_MAX && me_state_counter == UINT_MAX
return;
```
I don't mind?
--
To view, visit https://review.coreboot.org/c/coreboot/+/52800
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I374db3b7c0ded71cdc18f27970252fec7220cc20
Gerrit-Change-Number: 52800
Gerrit-PatchSet: 77
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: SRIDHAR SIRICILLA
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: SRIDHAR SIRICILLA
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 12 Oct 2021 07:39:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Werner Zeh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58240 )
Change subject: drivers/pc80/tpm: Use stopwatch for timeout-loops
......................................................................
drivers/pc80/tpm: Use stopwatch for timeout-loops
There are manual timeout-loops which use a fixed value and udelay().
In all cases there is a debug printk() inside this loop which, when
enabled, takes way longer than the counted microsecond delay. This
leads to the result that e.g. a 1 second delay takes nearly an eternity
if the debug messages are enabled due to the longer function execution
time.
This patch uses the stopwatch scheme for the timeout-loops which still
makes sure that the timeout period is maintained while it takes longer
function calls like printk() into account.
TEST=Enable TPM debug messages on a board where the TPM hits a timeout
by failure and make sure that the debug messages occur in the log
just in the timeout period.
Change-Id: I8fd261c9d60a9a60509c847dbc4983bc05f41d48
Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com>
---
M src/drivers/pc80/tpm/tis.c
1 file changed, 13 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/58240/1
diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c
index 2b29acf..d3ba221 100644
--- a/src/drivers/pc80/tpm/tis.c
+++ b/src/drivers/pc80/tpm/tis.c
@@ -22,6 +22,7 @@
#include <security/tpm/tis.h>
#include <device/pnp.h>
#include <drivers/tpm/tpm_ppi.h>
+#include <timer.h>
#include "chip.h"
#define PREFIX "lpc_tpm: "
@@ -249,13 +250,13 @@
*/
static int tis_wait_sts(int locality, u8 mask, u8 expected)
{
- u32 time_us = MAX_DELAY_US;
- while (time_us > 0) {
+ struct stopwatch sw;
+
+ stopwatch_init_usecs_expire(&sw, MAX_DELAY_US);
+ while (!stopwatch_expired(&sw)) {
u8 value = tpm_read_status(locality);
if ((value & mask) == expected)
return 0;
- udelay(1); /* 1 us */
- time_us--;
}
return TPM_TIMEOUT_ERR;
}
@@ -302,13 +303,13 @@
*/
static int tis_wait_access(int locality, u8 mask, u8 expected)
{
- u32 time_us = MAX_DELAY_US;
- while (time_us > 0) {
+ struct stopwatch sw;
+
+ stopwatch_init_usecs_expire(&sw, MAX_DELAY_US);
+ while (!stopwatch_expired(&sw)) {
u8 value = tpm_read_access(locality);
if ((value & mask) == expected)
return 0;
- udelay(1); /* 1 us */
- time_us--;
}
return TPM_TIMEOUT_ERR;
}
@@ -440,7 +441,6 @@
{
u32 offset = 0;
u16 burst = 0;
- u32 max_cycles = 0;
u8 locality = 0;
if (tis_wait_ready(locality)) {
@@ -452,20 +452,20 @@
while (1) {
unsigned int count;
+ struct stopwatch sw;
+
+ stopwatch_init_usecs_expire(&sw, MAX_DELAY_US);
/* Wait till the device is ready to accept more data. */
while (!burst) {
- if (max_cycles++ == MAX_DELAY_US) {
+ if (stopwatch_expired(&sw)) {
printf("%s:%d failed to feed %d bytes of %d\n",
__FILE__, __LINE__, len - offset, len);
return TPM_DRIVER_ERR;
}
- udelay(1);
burst = tpm_read_burst_count(locality);
}
- max_cycles = 0;
-
/*
* Calculate number of bytes the TPM is ready to accept in one
* shot.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58240
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8fd261c9d60a9a60509c847dbc4983bc05f41d48
Gerrit-Change-Number: 58240
Gerrit-PatchSet: 1
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: newchange
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58114 )
Change subject: soc/amd/common/block/espi_util: Refactor ESPI Setup
......................................................................
Patch Set 4:
(4 comments)
File src/soc/amd/cezanne/early_fch.c:
https://review.coreboot.org/c/coreboot/+/58114/comment/8b6ffdbd_ddfb4ac8
PS2, Line 39: enable_acpimmio_decode_pm04();
> this should probably be done in another patch, but enable_acpimmio_decode_pm04 needs to be moved to […]
Done
File src/soc/amd/common/block/lpc/lpc_util.c:
https://review.coreboot.org/c/coreboot/+/58114/comment/49c859c0_fa8b6ecd
PS2, Line 365: configure_espi
> yes, moving this function to espi_util. […]
Done
https://review.coreboot.org/c/coreboot/+/58114/comment/6552a642_29326d17
PS2, Line 367: ESPI
> i think it's eSPI with a lower case e. […]
Done
https://review.coreboot.org/c/coreboot/+/58114/comment/08021867_50945a4a
PS2, Line 369: !CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) &&
> this gets skipped in the case of verstage on psp to not initialize eSPI twice, right?
Yes, if eSPI is already initialized in PSP verstage we dont need to re-initialize it in x86 again.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58114
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icfeba17dae0a964c9ca73686e29c18d965589934
Gerrit-Change-Number: 58114
Gerrit-PatchSet: 4
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 12 Oct 2021 07:00:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Furquan Shaikh, Felix Held.
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58117
to look at the new patch set (#5).
Change subject: mb/google/guybrush: Override SPI Fast speeds
......................................................................
mb/google/guybrush: Override SPI Fast speeds
Add support to override SPI fast speeds based on board version from both
bootblock and verstage. Overrides apply for Guybrush only and SPI speed
is overridden from 66 MHz to 100 MHz starting board version 4. This will
help to improve the boot time on board version by ~60 ms and still allow
the old boards to boot with 66 MHz.
BUG=b:199779306
TEST=Build and boot to OS in Guybrush. Perform S5->S0, G3->S0, warm
reset and suspend/resume cycles for 50 iterations each.
Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com>
Change-Id: I5bf03ab8772f27aca346589e9c5662caf014d0d2
---
M src/mainboard/google/guybrush/Kconfig
M src/mainboard/google/guybrush/bootblock.c
M src/mainboard/google/guybrush/verstage.c
3 files changed, 37 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/58117/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/58117
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5bf03ab8772f27aca346589e9c5662caf014d0d2
Gerrit-Change-Number: 58117
Gerrit-PatchSet: 5
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jason Glenesk, Furquan Shaikh, Marshall Dawson, Felix Held.
Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58116
to look at the new patch set (#5).
Change subject: soc/amd/common/block/spi: Support fast speed override
......................................................................
soc/amd/common/block/spi: Support fast speed override
Add support to override SPI ROM fast speed based on board version. This
will allow boards to start at lower speeds during bringup and then
switch to higher speeds after assessing the signal integrity. Also
implement a default no-op override.
BUG=None
TEST=Build and boot to OS in Guybrush. Perform S5->S0, G3->S0, warm
reset and suspend/resume cycles for 50 iterations each.
Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com>
Change-Id: Ia8ff3b3bdb53fee142527ae63aa7785945909304
---
M src/soc/amd/common/block/include/amdblocks/spi.h
M src/soc/amd/common/block/spi/fch_spi.c
2 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/58116/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/58116
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia8ff3b3bdb53fee142527ae63aa7785945909304
Gerrit-Change-Number: 58116
Gerrit-PatchSet: 5
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Felix Held.
Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58115
to look at the new patch set (#5).
Change subject: soc/amd/common: Add support to read and set SPI speeds from verstage
......................................................................
soc/amd/common: Add support to read and set SPI speeds from verstage
Currently all SPI speed configurations are done through EFS at build
time. There is a need to apply SPI speed overrides at run-time - eg.
based on board version after assessing the signal integrity. This
override configuration can be carried out by PSP verstage and bootblock.
Export the APIs to set and read SPI speeds from both PSP verstage and
bootblock.
BUG=None
TEST=Build and boot to OS in guybrush. Perform S5->S0, G3->S0, warm
reset and suspend/resume cycles for 50 iterations each.
Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com>
Change-Id: I281531e506b56173471b918c746f58d1ad97162c
---
M src/soc/amd/common/block/include/amdblocks/psp_efs.h
M src/soc/amd/common/block/include/amdblocks/spi.h
M src/soc/amd/common/block/psp/Makefile.inc
M src/soc/amd/common/block/psp/psp_efs.c
M src/soc/amd/common/block/spi/fch_spi.c
M src/soc/amd/common/psp_verstage/fch.c
6 files changed, 12 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/58115/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/58115
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I281531e506b56173471b918c746f58d1ad97162c
Gerrit-Change-Number: 58115
Gerrit-PatchSet: 5
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset