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 <[email protected]> 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/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
