Attention is currently required from: Jason Glenesk, Marshall Dawson, Tim Wawrzynczak, Nick Vaccaro, Kyösti Mälkki.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58921 )
Change subject: mb/google,intel: Fix indirect includes
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58921
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e7200d60db4333551e34a615433fa21c3135db6
Gerrit-Change-Number: 58921
Gerrit-PatchSet: 2
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.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: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Thu, 04 Nov 2021 00:05:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Patrick Rudolph.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58922 )
Change subject: nb/intel/haswell/northbridge.c: Drop stale comment
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0fdefc5b4339acc843443c551892b397ed39c2e
Gerrit-Change-Number: 58922
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 03 Nov 2021 23:58:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Bao Zheng, Martin Roth, Marshall Dawson, Kangheui Won, Paul Menzel, Zheng Bao, Eric Peers, Karthikeyan Ramasubramanian, Felix Held.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56438 )
Change subject: amdfwtool: Use relative address for EFS gen2
......................................................................
Patch Set 15:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56438/comment/88cd0a3d_431aeb3a
PS2, Line 9: The second generation EFS (offset 0x24[0]=0) uses
: "binary relative" offsets and not "x86 physical
: MMIO address" like gen1.
:
: Chips like Cezanne can run in both cases, so no problem
: comes up so far.
:
: TODO: Need to fix the problem in psp_verstage.
> Done
We still need to wait until the PSP verstage is signed and signed verstage is landed in the downstream tree. ETA - 11/08/2021.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56438
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7701c7819f03586d4ecab3d744056c8c902b630f
Gerrit-Change-Number: 56438
Gerrit-PatchSet: 15
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Bao Zheng <zheng.bao(a)amd.corp-partner.google.com>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 03 Nov 2021 22:56:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bao Zheng <fishbaozi(a)gmail.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Eric Peers <epeers(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58918 )
Change subject: mb/emulation/qemu-i440fx: Refactor `fw_cfg_max_cpus()`
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58918
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0f83fdc41c20ed3aae80829432fc84024f5b9b47
Gerrit-Change-Number: 58918
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 03 Nov 2021 22:50:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Christian Walter, Angel Pons, Arthur Heymans, Patrick Rudolph, Tim Chu.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58914 )
Change subject: soc/intel/xeon_sp: Refactor `get_threads_per_package()`
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie71730d9a89eb7c4bb82d09d140fbcec7a6fe5f3
Gerrit-Change-Number: 58914
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Wed, 03 Nov 2021 22:49:02 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Michal Motyl, Patrick Rudolph.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58912 )
Change subject: soc/intel/denverton_ns: Refactor `detect_num_cpus_via_mch()`
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
looks good to me, but might be worth mentioning that this function might now detect more than CONFIG_MAX_CPUS cores; don't expect that the code will ever run into that case though
--
To view, visit https://review.coreboot.org/c/coreboot/+/58912
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iae6b16991fcf07c9ad67d2b737e490212b8deedd
Gerrit-Change-Number: 58912
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 03 Nov 2021 22:48:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held.
Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58926 )
Change subject: soc/amd/common/block: Add spi_hw mutex
......................................................................
soc/amd/common/block: Add spi_hw mutex
There are currently two users of the SPI hardware, the LPC SPI DMA
controller, and the boot_device_rw device. We need to ensure exclusivity
to the SPI hardware otherwise the SPI DMA controller can be interrupted
and it will silently skip transferring some blocks.
Depending on the SPI speed, this change might add a small delay when
clearing the elog since a DMA transaction might be in flight. I'll
continue optimizing the boot flow to avoid the delay.
BUG=b:179699789
TEST=Hack up the code to interleave SPI transactions and verify this
patch fixes the silent data corruption.
Signed-off-by: Raul E Rangel <rrangel(a)chromium.org>
Change-Id: I5eee812a6979c8c0fb313dd2fbccc14b73d7d741
---
M src/soc/amd/common/block/include/amdblocks/spi.h
M src/soc/amd/common/block/lpc/spi_dma.c
M src/soc/amd/common/block/spi/fch_spi_ctrl.c
M src/soc/amd/common/block/spi/fch_spi_util.c
4 files changed, 27 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/58926/1
diff --git a/src/soc/amd/common/block/include/amdblocks/spi.h b/src/soc/amd/common/block/include/amdblocks/spi.h
index 35a3782..81da5dd 100644
--- a/src/soc/amd/common/block/include/amdblocks/spi.h
+++ b/src/soc/amd/common/block/include/amdblocks/spi.h
@@ -3,6 +3,7 @@
#ifndef AMD_BLOCK_SPI_H
#define AMD_BLOCK_SPI_H
+#include <thread.h>
#include <types.h>
#define SPI_CNTRL0 0x00
@@ -118,4 +119,8 @@
void fch_spi_config_modes(void);
void mainboard_spi_fast_speed_override(uint8_t *fast_speed);
+
+/* Ensure you hold the mutex when performing SPI transactions */
+extern struct thread_mutex spi_hw_mutex;
+
#endif /* AMD_BLOCK_SPI_H */
diff --git a/src/soc/amd/common/block/lpc/spi_dma.c b/src/soc/amd/common/block/lpc/spi_dma.c
index 5c69779..baf1eb8 100644
--- a/src/soc/amd/common/block/lpc/spi_dma.c
+++ b/src/soc/amd/common/block/lpc/spi_dma.c
@@ -122,6 +122,12 @@
ctrl |= LPC_ROM_DMA_CTRL_ERROR; /* Clear error */
ctrl |= LPC_ROM_DMA_CTRL_START;
+ /*
+ * Ensure we have exclusive access to the SPI controller before starting the LPC SPI DMA
+ * transaction.
+ */
+ thread_mutex_lock(&spi_hw_mutex);
+
pci_write_config32(SOC_LPC_DEV, LPC_ROM_DMA_EC_HOST_CONTROL, ctrl);
}
@@ -135,6 +141,12 @@
if (spi_dma_is_busy())
return true;
+ /*
+ * Unlock the SPI mutex between DMA transactions to allow other users of the SPI
+ * controller to interleave their transactions.
+ */
+ thread_mutex_unlock(&spi_hw_mutex);
+
if (spi_dma_has_error()) {
printk(BIOS_ERR,
"ERROR: SPI DMA failure: dest: %p, source: %#zx, size: %zu\n",
diff --git a/src/soc/amd/common/block/spi/fch_spi_ctrl.c b/src/soc/amd/common/block/spi/fch_spi_ctrl.c
index 565fdbe..d95b642 100644
--- a/src/soc/amd/common/block/spi/fch_spi_ctrl.c
+++ b/src/soc/amd/common/block/spi/fch_spi_ctrl.c
@@ -142,7 +142,13 @@
static int xfer_vectors(const struct spi_slave *slave,
struct spi_op vectors[], size_t count)
{
- return spi_flash_vector_helper(slave, vectors, count, spi_ctrlr_xfer);
+ int rc;
+
+ thread_mutex_lock(&spi_hw_mutex);
+ rc = spi_flash_vector_helper(slave, vectors, count, spi_ctrlr_xfer);
+ thread_mutex_unlock(&spi_hw_mutex);
+
+ return rc;
}
static int protect_a_range(u32 value)
diff --git a/src/soc/amd/common/block/spi/fch_spi_util.c b/src/soc/amd/common/block/spi/fch_spi_util.c
index 5cef565..862962f 100644
--- a/src/soc/amd/common/block/spi/fch_spi_util.c
+++ b/src/soc/amd/common/block/spi/fch_spi_util.c
@@ -6,6 +6,9 @@
#include <assert.h>
#include <stdint.h>
+/* Global SPI controller mutex */
+struct thread_mutex spi_hw_mutex;
+
static uintptr_t spi_base;
void spi_set_base(void *base)
--
To view, visit https://review.coreboot.org/c/coreboot/+/58926
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5eee812a6979c8c0fb313dd2fbccc14b73d7d741
Gerrit-Change-Number: 58926
Gerrit-PatchSet: 1
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange