[flashrom] [PATCH] SB600 SPI paranoid checks

[Carl-Daniel Hailfinger]

Hi Hony,

here is the patch I talked about. If your mailer corrupts the patch, you
can find it at the top of
http://patchwork.coreboot.org/project/flashrom/list/ (look for "SB600
SPI paranoid checks"), click on the link and then select "download patch".

On 13.08.2010 03:25, Carl-Daniel Hailfinger wrote:
> my suspicion is that either the third party application may access flash
> regions between 0xFFC00000 and 0xFFFFFFFF or that the builtin management
> engine in the southbridge may be asked by the third party application to
> access those flash regions or run those commands.
>
> You wrote that the problem does not exist if the third party application
> (IBM UpdateXpress System Pack Installer) is not running. I see three
> ways to handle this:
> Always terminate UpdateXpress before running flashrom.
> Have flashrom detect a running UpdateXpress instance and freeze it as
>
> long as it accesses flash.
> Ask IBM to modify UpdateXpress in a way that does not access flash
>
> unless explicitly requested.
>
> As an alternative, we could add paranoid checks to the SB600 SPI driver
> and hope that those checks will help detect the issue. Once we detect
> the issue, we can print a warning and tell the user to stop all other
> programs accessing the flash.
>
> I will send a SB600 paranoid checks patch as reply to this mail.

Add paranoid checks for the essential readcnt/writecnt registers in the
SB600/SB700/... SPI driver. This will detect some concurrent access, but
not all.

If you want to check which application is accessing /dev/mem, please run
(as root)
fuser -v /dev/mem
You will see Xorg (which is OK), but you probably will see another
application as well. Can you please send the output of the above command?
It would be a good idea to strace that command from startup, and check
the strace log for open(/dev/mem) and subsequent mmap calls on the fd
for /dev/mem. Then we know exactly in which region the access is
happening. A good way is to run it as
strace -ff -F -o strace.log /bin/thirdparty_application
and then strace.log.* will contain some useful info. grep for "open" and
"mmap". This should help.

Please run flashrom with this patch while UpdateXpress is not running
(should still work and print no warnings).
Then please run flashrom a few times with this patch while UpdateXpress
is running (should still have the same problems, but it should print
warnings).

I would love to see the logs, and it would also be great if you could
send a few logs and tell us about the typical addresses where writing
fails or where the verify fails. Maybe there is a pattern.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-sb600_spi_paranoia_concurrent_access/sb600spi.c
===================================================================
--- flashrom-sb600_spi_paranoia_concurrent_access/sb600spi.c	(Revision 1137)
+++ flashrom-sb600_spi_paranoia_concurrent_access/sb600spi.c	(Arbeitskopie)
@@ -77,6 +77,7 @@
    /* First byte is cmd which can not being sent through FIFO. */
    unsigned char cmd = *writearr++;
    unsigned int readoffby1;
+	unsigned char readwrite;

writecnt--;

@@ -102,7 +103,8 @@
     * 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. */
@@ -149,6 +151,14 @@
    	msg_pspew("[%02x]", *readarr);
    }
    msg_pspew("\n");
+	if (mmio_readb(sb600_spibar + 1) != readwrite) {
+		msg_perr("Unexpected change in SB600 read/write count! "
+			 "Something else is accessing the flash chip and "
+			 "causes random corruption. Please stop all "
+			 "applications and drivers which access the flash "
+			 "chip.\n");
+		/* FIXME: Abort here? */
+	}

return 0;
 }
@@ -158,6 +168,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 +197,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);