Nico Huber has uploaded this change for review.
spi: Move ICH BBAR quirk out of the way
Get rid of the layering violations around ICH's BBAR. Move all the weird
address handling into (surprise, surprise) `ichspi.c`. Might fix writes
for the `BBAR != 0` case by accident.
Drop the weird alignment fixup in spi_chip_read(). It looks broken
(always sets `addrbase` below the valid base address) and there is
no indication in the CL that it was ever tested.
Background: Some ICHs have a BBAR (BIOS Base Address Configuration
Register) that, if set, limits the valid address range to [BBAR, 2^24).
Current code lifted addresses for REMS, RES and READ operations by BBAR,
now we do it for all addresses in ichspi.
Change-Id: Icbac513c5339e8aff624870252133284ef85ab73
Signed-off-by: Nico Huber <nico.h@gmx.de>
---
M flash.h
M ichspi.c
M programmer.h
M spi.c
M spi25.c
5 files changed, 18 insertions(+), 83 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/22396/1
diff --git a/flash.h b/flash.h
index 4f0ab15..bab4873 100644
--- a/flash.h
+++ b/flash.h
@@ -371,7 +371,6 @@
#define NULL_SPI_CMD { 0, 0, NULL, NULL, }
int spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
int spi_send_multicommand(struct flashctx *flash, struct spi_command *cmds);
-uint32_t spi_get_valid_read_addr(struct flashctx *flash);
enum chipbustype get_buses_supported(void);
#endif /* !__FLASH_H__ */
diff --git a/ichspi.c b/ichspi.c
index 859d55f..9a4cf5d 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -229,7 +229,7 @@
static int ichspi_lock = 0;
static enum ich_chipset ich_generation = CHIPSET_ICH_UNKNOWN;
-uint32_t ichspi_bbar = 0;
+static uint32_t ichspi_bbar;
static void *ich_spibar = NULL;
@@ -1150,19 +1150,6 @@
return SPI_INVALID_LENGTH;
}
- /* if opcode-type requires an address */
- if (opcode->spi_type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS ||
- opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) {
- addr = (writearr[1] << 16) |
- (writearr[2] << 8) | (writearr[3] << 0);
- 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;
- }
- }
-
/* Translate read/write array/count.
* The maximum data length is identical for the maximum read length and
* for the maximum write length excluding opcode and address. Opcode and
@@ -1181,6 +1168,20 @@
count = readcnt;
}
+ /* if opcode-type requires an address */
+ if (opcode->spi_type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS ||
+ opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) {
+ addr = writearr[1] << 16 | writearr[2] << 8 | writearr[3];
+ if (count > UINT32_MAX - addr || /* just an integer overflow check */
+ ichspi_bbar > UINT32_MAX - addr - count || /* another integer overflow check */
+ ichspi_bbar + addr + count > 1 << 24) { /* last address must be in 24-bit space */
+ msg_perr("%s: Addresses 0x%06x-0x%06x not in allowed range 0x000000-0x%06x\n",
+ __func__, addr, addr + count - 1, (1 << 24) - ichspi_bbar - 1);
+ return SPI_INVALID_ADDRESS;
+ }
+ addr += ichspi_bbar;
+ }
+
result = run_opcode(flash, *opcode, addr, count, data);
if (result) {
msg_pdbg("Running OPCODE 0x%02x failed ", opcode->opcode);
diff --git a/programmer.h b/programmer.h
index a98b713..b390a53 100644
--- a/programmer.h
+++ b/programmer.h
@@ -661,7 +661,6 @@
/* ichspi.c */
#if CONFIG_INTERNAL == 1
-extern uint32_t ichspi_bbar;
int ich_init_spi(void *spibar, enum ich_chipset ich_generation);
int via_init_spi(uint32_t mmio_base);
diff --git a/spi.c b/spi.c
index 0a4a618..56f1fdf 100644
--- a/spi.c
+++ b/spi.c
@@ -103,31 +103,7 @@
int spi_chip_read(struct flashctx *flash, uint8_t *buf, unsigned int start,
unsigned int len)
{
- unsigned int addrbase = 0;
-
- /* Check if the chip fits between lowest valid and highest possible
- * address. Highest possible address with the current SPI implementation
- * means 0xffffff, the highest unsigned 24bit number.
- */
- addrbase = spi_get_valid_read_addr(flash);
- /* Show flash chip size warning if flash chip doesn't support
- 4-Bytes Addressing mode and last address excedes 24 bits */
- if (!(flash->chip->feature_bits & FEATURE_4BA_SUPPORT) &&
- addrbase + flash->chip->total_size * 1024 > (1 << 24)) {
- msg_perr("Flash chip size exceeds the allowed access window. ");
- msg_perr("Read will probably fail.\n");
- /* Try to get the best alignment subject to constraints. */
- addrbase = (1 << 24) - flash->chip->total_size * 1024;
- }
- /* Check if alignment is native (at least the largest power of two which
- * is a factor of the mapped size of the chip).
- */
- if (ffs(flash->chip->total_size * 1024) > (ffs(addrbase) ? : 33)) {
- msg_perr("Flash chip is not aligned natively in the allowed "
- "access window.\n");
- msg_perr("Read will probably return garbage.\n");
- }
- return flash->mst->spi.read(flash, buf, addrbase + start, len);
+ return flash->mst->spi.read(flash, buf, start, len);
}
/*
@@ -140,27 +116,6 @@
int spi_chip_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len)
{
return flash->mst->spi.write_256(flash, buf, start, len);
-}
-
-/*
- * 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 master limitation.
- */
-uint32_t spi_get_valid_read_addr(struct flashctx *flash)
-{
- switch (flash->mst->spi.type) {
-#if CONFIG_INTERNAL == 1
-#if defined(__i386__) || defined(__x86_64__)
- case SPI_CONTROLLER_ICH7:
- case SPI_CONTROLLER_ICH9:
- /* Return BBAR for ICH chipsets. */
- return ichspi_bbar;
-#endif
-#endif
- default:
- return 0;
- }
}
int spi_aai_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len)
diff --git a/spi25.c b/spi25.c
index df0ebbe..06eea9f 100644
--- a/spi25.c
+++ b/spi25.c
@@ -49,21 +49,11 @@
static int spi_rems(struct flashctx *flash, unsigned char *readarr)
{
- unsigned char cmd[JEDEC_REMS_OUTSIZE] = { JEDEC_REMS, 0, 0, 0 };
- uint32_t readaddr;
+ const unsigned char cmd[JEDEC_REMS_OUTSIZE] = { JEDEC_REMS, 0, 0, 0 };
int ret;
ret = spi_send_command(flash, sizeof(cmd), JEDEC_REMS_INSIZE, cmd,
readarr);
- if (ret == SPI_INVALID_ADDRESS) {
- /* Find the lowest even address allowed for reads. */
- readaddr = (spi_get_valid_read_addr(flash) + 1) & ~1;
- cmd[1] = (readaddr >> 16) & 0xff,
- cmd[2] = (readaddr >> 8) & 0xff,
- cmd[3] = (readaddr >> 0) & 0xff,
- ret = spi_send_command(flash, sizeof(cmd), JEDEC_REMS_INSIZE,
- cmd, readarr);
- }
if (ret)
return ret;
msg_cspew("REMS returned 0x%02x 0x%02x. ", readarr[0], readarr[1]);
@@ -72,20 +62,11 @@
static int spi_res(struct flashctx *flash, unsigned char *readarr, int bytes)
{
- unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
- uint32_t readaddr;
+ const unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
int ret;
int i;
ret = spi_send_command(flash, sizeof(cmd), bytes, cmd, readarr);
- if (ret == SPI_INVALID_ADDRESS) {
- /* Find the lowest even address allowed for reads. */
- readaddr = (spi_get_valid_read_addr(flash) + 1) & ~1;
- cmd[1] = (readaddr >> 16) & 0xff,
- cmd[2] = (readaddr >> 8) & 0xff,
- cmd[3] = (readaddr >> 0) & 0xff,
- ret = spi_send_command(flash, sizeof(cmd), bytes, cmd, readarr);
- }
if (ret)
return ret;
msg_cspew("RES returned");
To view, visit change 22396. To unsubscribe, visit settings.