[flashrom] [PATCH] SB600 SPI paranoid checks
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Wed Aug 18 14:26:39 CEST 2010
Hi Matthias,
this is the last version of the corruption detection. flashrom will now
abort if it detects corruption instead of continuing.
On 18.08.2010 13:58, Carl-Daniel Hailfinger wrote:
> On 18.08.2010 13:31, Carl-Daniel Hailfinger wrote:
>
>> Hi Matthias,
>>
>> thanks for the test. Can you repeat the test with the following patch?
>>
>> On 18.08.2010 13:04, Carl-Daniel Hailfinger wrote:
>>
>>
>>> Hi Matthias, hi Hony,
>>>
>>> you are both seeing a very similar issue. This mail contains a patch
>>> which should add the debugging we need.
>>>
>>> New patch, with additional error checks.
>>>
>>> Add paranoid checks for the essential readcnt/writecnt registers in the
>>> SB600/SB700/... SPI driver. This will detect most concurrent access, but
>>> not all.
>>>
>>> I would love to see the logs in double verbose mode for a read:
>>>
>>> flashrom -VV -r broken_fifo1.bin >broken_fifo1.txt 2>&1
>>>
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
If this aborts for you with an error message about corruption and if the
read image is empty (or not created), the patch does what it should. In
that case, please respond with
> Acked-by: Your Name <your at email>
so I can check in the patch and work with you to have flashrom retry
corrupted accesses.
Index: flashrom-sb600_spi_paranoia_concurrent_access/spi.h
===================================================================
--- flashrom-sb600_spi_paranoia_concurrent_access/spi.h (Revision 1144)
+++ flashrom-sb600_spi_paranoia_concurrent_access/spi.h (Arbeitskopie)
@@ -124,5 +124,6 @@
#define SPI_INVALID_ADDRESS -3
#define SPI_INVALID_LENGTH -4
#define SPI_FLASHROM_BUG -5
+#define SPI_PROGRAMMER_ERROR -6
#endif /* !__SPI_H__ */
Index: flashrom-sb600_spi_paranoia_concurrent_access/sb600spi.c
===================================================================
--- flashrom-sb600_spi_paranoia_concurrent_access/sb600spi.c (Revision 1144)
+++ flashrom-sb600_spi_paranoia_concurrent_access/sb600spi.c (Arbeitskopie)
@@ -58,10 +58,40 @@
{
mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2);
+ /* FIXME: This loop makes no sense at all. */
while (mmio_readb(sb600_spibar + 0xD) & 0x7)
msg_pspew("reset\n");
}
+static int compare_internal_fifo_pointer(uint8_t want)
+{
+ uint8_t tmp;
+
+ tmp = mmio_readb(sb600_spibar + 0xd) & 0x07;
+ want &= 0x7;
+ if (want != tmp) {
+ msg_perr("SB600 FIFO pointer corruption! Pointer is %d, wanted "
+ "%d\n", tmp, want);
+ msg_perr("Something else is accessing the flash chip and "
+ "causes random corruption.\nPlease stop all "
+ "applications and drivers and IPMI which access the "
+ "flash chip.\n");
+ return 1;
+ } else {
+ msg_pspew("SB600 FIFO pointer is %d, wanted %d\n", tmp, want);
+ return 0;
+ }
+}
+
+static int reset_compare_internal_fifo_pointer(uint8_t want)
+{
+ int ret;
+
+ ret = compare_internal_fifo_pointer(want);
+ reset_internal_fifo_pointer();
+ return ret;
+}
+
static void execute_command(void)
{
mmio_writeb(mmio_readb(sb600_spibar + 2) | 1, sb600_spibar + 2);
@@ -77,6 +107,7 @@
/* First byte is cmd which can not being sent through FIFO. */
unsigned char cmd = *writearr++;
unsigned int readoffby1;
+ unsigned char readwrite;
writecnt--;
@@ -102,15 +133,17 @@
* It is unclear if the CS# line is set high too early as well.
*/
readoffby1 = (writecnt) ? 0 : 1;
- mmio_writeb((readcnt + readoffby1) << 4 | (writecnt), sb600_spibar + 1);
+ readwrite = (readcnt + readoffby1) << 4 | (writecnt);
+ mmio_writeb(readwrite, sb600_spibar + 1);
mmio_writeb(cmd, sb600_spibar + 0);
/* Before we use the FIFO, reset it first. */
reset_internal_fifo_pointer();
/* Send the write byte to FIFO. */
+ msg_pspew("Writing: ");
for (count = 0; count < writecnt; count++, writearr++) {
- msg_pspew(" [%x]", *writearr);
+ msg_pspew("[%02x]", *writearr);
mmio_writeb(*writearr, sb600_spibar + 0xC);
}
msg_pspew("\n");
@@ -119,8 +152,10 @@
* We should send the data by sequence, which means we need to reset
* the FIFO pointer to the first byte we want to send.
*/
- reset_internal_fifo_pointer();
+ if (reset_compare_internal_fifo_pointer(writecnt))
+ return SPI_PROGRAMMER_ERROR;
+ msg_pspew("Executing: \n");
execute_command();
/*
@@ -134,22 +169,37 @@
* the opcode, the FIFO already stores the response from the chip.
* Usually, the chip will respond with 0x00 or 0xff.
*/
- reset_internal_fifo_pointer();
+ if (reset_compare_internal_fifo_pointer(writecnt + readcnt))
+ return SPI_PROGRAMMER_ERROR;
/* Skip the bytes we sent. */
+ msg_pspew("Skipping: ");
for (count = 0; count < writecnt; count++) {
cmd = mmio_readb(sb600_spibar + 0xC);
- msg_pspew("[ %2x]", cmd);
+ msg_pspew("[%02x]", cmd);
}
+ msg_pspew("\n");
+ if (compare_internal_fifo_pointer(writecnt))
+ return SPI_PROGRAMMER_ERROR;
- msg_pspew("The FIFO pointer after skipping is %d.\n",
- mmio_readb(sb600_spibar + 0xd) & 0x07);
+ msg_pspew("Reading: ");
for (count = 0; count < readcnt; count++, readarr++) {
*readarr = mmio_readb(sb600_spibar + 0xC);
msg_pspew("[%02x]", *readarr);
}
msg_pspew("\n");
+ if (reset_compare_internal_fifo_pointer(readcnt + writecnt))
+ return SPI_PROGRAMMER_ERROR;
+ if (mmio_readb(sb600_spibar + 1) != readwrite) {
+ msg_perr("Unexpected change in SB600 read/write count!\n");
+ msg_perr("Something else is accessing the flash chip and "
+ "causes random corruption.\nPlease stop all "
+ "applications and drivers and IPMI which access the "
+ "flash chip.\n");
+ return SPI_PROGRAMMER_ERROR;
+ }
+
return 0;
}
@@ -158,6 +208,10 @@
struct pci_dev *smbus_dev;
uint32_t tmp;
uint8_t reg;
+ const char *speed_names[4] = {
+ "Reserved", "33", "22", "16.5"
+ };
+
/* Read SPI_BaseAddr */
tmp = pci_read_long(dev, 0xa0);
tmp &= 0xffffffe0; /* remove bits 4-0 (reserved) */
@@ -183,15 +237,25 @@
msg_pdbg("PrefetchEnSPIFromIMC=%i, ", tmp);
tmp = pci_read_byte(dev, 0xbb);
+ /* FIXME: Set bit 3,6,7 if not already set.
+ * Set bit 5, otherwise SPI accesses are pointless in LPC mode.
+ * See doc 42413 AMD SB700/710/750 RPR.
+ */
msg_pdbg("PrefetchEnSPIFromHost=%i, SpiOpEnInLpcMode=%i\n",
tmp & 0x1, (tmp & 0x20) >> 5);
tmp = mmio_readl(sb600_spibar);
+ /* FIXME: If SpiAccessMacRomEn or SpiHostAccessRomEn are zero on
+ * SB700 or later, reads and writes will be corrupted. Abort in this
+ * case. Make sure to avoid this check on SB600.
+ */
msg_pdbg("SpiArbEnable=%i, SpiAccessMacRomEn=%i, "
"SpiHostAccessRomEn=%i, ArbWaitCount=%i, "
"SpiBridgeDisable=%i, DropOneClkOnRd=%i\n",
(tmp >> 19) & 0x1, (tmp >> 22) & 0x1,
(tmp >> 23) & 0x1, (tmp >> 24) & 0x7,
(tmp >> 27) & 0x1, (tmp >> 28) & 0x1);
+ tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3;
+ msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]);
/* Look for the SMBus device. */
smbus_dev = pci_dev_find(0x1002, 0x4385);
@@ -230,6 +294,9 @@
return 0;
}
+ /* Bring the FIFO to a clean state. */
+ reset_internal_fifo_pointer();
+
buses_supported |= CHIP_BUSTYPE_SPI;
spi_controller = SPI_CONTROLLER_SB600;
return 0;
--
http://www.hailfinger.org/
More information about the flashrom
mailing list