Change in coreboot[master]: soc/amd/common/block: Add spi_hw mutex

[Raul Rangel (Code Review)]

Raul Rangel has submitted this change. ( 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 
Change-Id: I5eee812a6979c8c0fb313dd2fbccc14b73d7d741
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58926
Tested-by: build bot (Jenkins) 
Reviewed-by: Karthik Ramasubramanian 
---
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(-)

Approvals:
  build bot (Jenkins): Verified
  Karthik Ramasubramanian: Looks good to me, approved

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)