[flashrom] [PATCH] Avoid transaction errors on ICH SPI
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Fri May 28 17:04:21 CEST 2010
On 28.05.2010 16:32, Carl-Daniel Hailfinger wrote:
> ICH SPI can enforces address restrictions for all accesses which take an
> address (well, it could if the chipset implementation was not broken).
> Since exploiting the broken implementation is harder than conforming to
> the address restrictions wherever possible, conform to the address
> restrictions instead.
> This patch will eliminate a lot of transaction errors people were seeing
> (for probe, anyway).
>
New patch.
Added feature:
Explicitly check for invalid addresses in the ichspi driver and abort
early with SPI_INVALID_ADDRESS.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Index: flashrom-ichspi_avoid_transaction_error_use_bbar/flash.h
===================================================================
--- flashrom-ichspi_avoid_transaction_error_use_bbar/flash.h (Revision 1015)
+++ flashrom-ichspi_avoid_transaction_error_use_bbar/flash.h (Arbeitskopie)
@@ -663,6 +663,8 @@
uint32_t spi_get_valid_read_addr(void);
/* ichspi.c */
+extern int ichspi_lock;
+extern uint32_t ichspi_bbar;
int ich_init_opcodes(void);
int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
Index: flashrom-ichspi_avoid_transaction_error_use_bbar/spi.c
===================================================================
--- flashrom-ichspi_avoid_transaction_error_use_bbar/spi.c (Revision 1015)
+++ flashrom-ichspi_avoid_transaction_error_use_bbar/spi.c (Arbeitskopie)
@@ -207,8 +207,22 @@
return spi_programmer[spi_controller].write_256(flash, buf);
}
+/*
+ * Get the lowest allowed address for read accesses. This often happens to
+ * be the lowest allowed address for all commands which take an address.
+ * This is a programmer limitation.
+ */
uint32_t spi_get_valid_read_addr(void)
{
- /* Need to return BBAR for ICH chipsets. */
- return 0;
+ switch (spi_controller) {
+#if INTERNAL_SUPPORT == 1
+#if defined(__i386__) || defined(__x86_64__)
+ case SPI_CONTROLLER_ICH7:
+ /* Return BBAR for ICH chipsets. */
+ return ichspi_bbar;
+#endif
+#endif
+ default:
+ return 0;
+ }
}
Index: flashrom-ichspi_avoid_transaction_error_use_bbar/chipset_enable.c
===================================================================
--- flashrom-ichspi_avoid_transaction_error_use_bbar/chipset_enable.c (Revision 1015)
+++ flashrom-ichspi_avoid_transaction_error_use_bbar/chipset_enable.c (Arbeitskopie)
@@ -36,8 +36,6 @@
#if defined(__i386__) || defined(__x86_64__)
-extern int ichspi_lock;
-
static int enable_flash_ali_m1533(struct pci_dev *dev, const char *name)
{
uint8_t tmp;
@@ -515,8 +513,9 @@
msg_pdbg("0x%02x: 0x%08x (SPID%d+4)\n", offs + 4,
mmio_readl(spibar + offs + 4), i);
}
+ ichspi_bbar = mmio_readl(spibar + 0x50);
msg_pdbg("0x50: 0x%08x (BBAR)\n",
- mmio_readl(spibar + 0x50));
+ ichspi_bbar);
msg_pdbg("0x54: 0x%04x (PREOP)\n",
mmio_readw(spibar + 0x54));
msg_pdbg("0x56: 0x%04x (OPTYPE)\n",
@@ -587,8 +586,9 @@
mmio_readl(spibar + 0x98));
msg_pdbg("0x9C: 0x%08x (OPMENU+4)\n",
mmio_readl(spibar + 0x9C));
+ ichspi_bbar = mmio_readl(spibar + 0xA0);
msg_pdbg("0xA0: 0x%08x (BBAR)\n",
- mmio_readl(spibar + 0xA0));
+ ichspi_bbar);
msg_pdbg("0xB0: 0x%08x (FDOC)\n",
mmio_readl(spibar + 0xB0));
if (tmp2 & (1 << 15)) {
Index: flashrom-ichspi_avoid_transaction_error_use_bbar/ichspi.c
===================================================================
--- flashrom-ichspi_avoid_transaction_error_use_bbar/ichspi.c (Revision 1015)
+++ flashrom-ichspi_avoid_transaction_error_use_bbar/ichspi.c (Arbeitskopie)
@@ -103,6 +103,8 @@
/* ICH SPI configuration lock-down. May be set during chipset enabling. */
int ichspi_lock = 0;
+uint32_t ichspi_bbar = 0;
+
typedef struct _OPCODE {
uint8_t opcode; //This commands spi opcode
uint8_t spi_type; //This commands spi type
@@ -327,6 +329,34 @@
return 0;
}
+/*
+ * Try to set BBAR (BIOS Base Address Register), but read back the value in case
+ * it didn't stick.
+ */
+void ich_set_bbar(uint32_t minaddr)
+{
+ switch (spi_controller) {
+ case SPI_CONTROLLER_ICH7:
+ mmio_writel(minaddr, spibar + 0x50);
+ ichspi_bbar = mmio_readl(spibar + 0x50);
+ /* We don't have any option except complaining. */
+ if (ichspi_bbar != minaddr)
+ msg_perr("Setting BBAR failed!\n");
+ break;
+ case SPI_CONTROLLER_ICH9:
+ mmio_writel(minaddr, spibar + 0xA0);
+ ichspi_bbar = mmio_readl(spibar + 0xA0);
+ /* We don't have any option except complaining. */
+ if (ichspi_bbar != minaddr)
+ msg_perr("Setting BBAR failed!\n");
+ break;
+ default:
+ /* Not sure if BBAR actually exists on VIA. */
+ msg_pdbg("Setting BBAR is not implemented for VIA yet.\n");
+ break;
+ }
+}
+
/* This function generates OPCODES from or programs OPCODES to ICH according to
* the chipset's SPI configuration lock.
*
@@ -341,13 +371,18 @@
return 0;
if (ichspi_lock) {
- msg_pdbg("Generating OPCODES... ");
+ msg_pdbg("Reading OPCODES... ");
curopcodes_done = &O_EXISTING;
rc = generate_opcodes(curopcodes_done);
} else {
msg_pdbg("Programming OPCODES... ");
curopcodes_done = &O_ST_M25P;
rc = program_opcodes(curopcodes_done);
+ /* Technically not part of opcode init, but it allows opcodes
+ * to run without transaction errors by setting the lowest
+ * allowed address to zero.
+ */
+ ich_set_bbar(0);
}
if (rc) {
@@ -743,8 +778,22 @@
opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) {
addr = (writearr[1] << 16) |
(writearr[2] << 8) | (writearr[3] << 0);
+ switch (spi_controller) {
+ case SPI_CONTROLLER_ICH7:
+ case SPI_CONTROLLER_ICH9:
+ if (addr < ichspi_bbar) {
+ msg_perr("%s: Address 0x%06x below allowed "
+ "range 0x%06x-0xffffff\n", __func__,
+ addr, ichspi_bbar);
+ return SPI_INVALID_ADDRESS;
+ }
+ break;
+ default:
+ break;
+ }
}
+
/* translate read/write array/count */
if (opcode->spi_type == SPI_OPCODE_TYPE_WRITE_NO_ADDRESS) {
data = (uint8_t *) (writearr + 1);
--
http://www.hailfinger.org/
More information about the flashrom
mailing list