Adds the infrastructure parts that allow for sanity checks of transactions without executing them. This includes new fields in struct spi_programmer and default implementations as well as concrete implementations for existing programmer where it makes sense. Some additional cleanups are done as a bonus where i took the opportunity, namely in sb600 and serprog.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at --- bitbang_spi.c | 1 + buspirate_spi.c | 14 +++++++++++++- dediprog.c | 24 ++++++++++++++++-------- dummyflasher.c | 1 + ft2232_spi.c | 14 +++++++++++++- ichspi.c | 9 +++++++++ it85spi.c | 1 + it87spi.c | 31 +++++++++++++++++++++++-------- linux_spi.c | 31 +++++++++++++++++++++++-------- programmer.h | 9 +++++++++ sb600spi.c | 51 ++++++++++++++++++++++++++++----------------------- serprog.c | 32 +++++++++++++++++++++----------- spi.c | 9 +++++++-- wbsio_spi.c | 10 ++++++++++ 14 files changed, 175 insertions(+), 62 deletions(-)
diff --git a/bitbang_spi.c b/bitbang_spi.c index 11d2de1..4b9f83a 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -67,6 +67,7 @@ static const struct spi_programmer spi_programmer_bitbang = { .type = SPI_CONTROLLER_BITBANG, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, + .check_trans = default_spi_check_trans, .command = bitbang_spi_send_command, .multicommand = default_spi_send_multicommand, .read = default_spi_read, diff --git a/buspirate_spi.c b/buspirate_spi.c index 054b4ff..340091c 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -129,6 +129,14 @@ static int buspirate_wait_for_string(unsigned char *buf, char *key) return ret; }
+static int buspirate_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + if (writecnt > 16 || readcnt > 16 || (readcnt + writecnt) > 16) + return SPI_INVALID_LENGTH; + return 0; +} + static int buspirate_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
@@ -136,6 +144,7 @@ static const struct spi_programmer spi_programmer_buspirate = { .type = SPI_CONTROLLER_BUSPIRATE, .max_data_read = 12, .max_data_write = 12, + .check_trans = buspirate_spi_check_trans, .command = buspirate_spi_send_command, .multicommand = default_spi_send_multicommand, .read = default_spi_read, @@ -421,8 +430,11 @@ static int buspirate_spi_send_command(struct flashctx *flash, unsigned int i = 0; int ret = 0;
- if (writecnt > 16 || readcnt > 16 || (readcnt + writecnt) > 16) + if (buspirate_spi_check_trans(flash, writecnt, readcnt, writearr) != 0) { + msg_pspew("%s called with an unsupported transaction layout: readcnt = %d, writecnt = %d.\n", + __func__, readcnt, writecnt); return SPI_INVALID_LENGTH; + }
/* 3 bytes extra for CS#, len, CS#. */ if (buspirate_commbuf_grow(writecnt + readcnt + 3)) diff --git a/dediprog.c b/dediprog.c index a81cf83..643b324 100644 --- a/dediprog.c +++ b/dediprog.c @@ -439,6 +439,18 @@ static int dediprog_spi_write_aai(struct flashctx *flash, uint8_t *buf, unsigned return dediprog_spi_write(flash, buf, start, len, DEDI_SPI_CMD_AAIWRITE); }
+static int dediprog_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + /* Paranoid, but I don't want to be blamed if anything explodes. */ + if (writecnt > 16) + return 1; + /* 16 byte reads should work. */ + if (readcnt > 16) + return -1; + return 0; +} + static int dediprog_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, @@ -448,14 +460,9 @@ static int dediprog_spi_send_command(struct flashctx *flash, int ret;
msg_pspew("%s, writecnt=%i, readcnt=%i\n", __func__, writecnt, readcnt); - /* Paranoid, but I don't want to be blamed if anything explodes. */ - if (writecnt > 16) { - msg_perr("Untested writecnt=%i, aborting.\n", writecnt); - return 1; - } - /* 16 byte reads should work. */ - if (readcnt > 16) { - msg_perr("Untested readcnt=%i, aborting.\n", readcnt); + if (dediprog_spi_check_trans(flash, writecnt, readcnt, writearr) != 0) { + msg_perr("%s called with an unsupported transaction layout: readcnt = %d, writecnt = %d.\n", + __func__, readcnt, writecnt); return 1; } @@ -740,6 +747,7 @@ static const struct spi_programmer spi_programmer_dediprog = { .type = SPI_CONTROLLER_DEDIPROG, .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, + .check_trans = dediprog_spi_check_trans, .command = dediprog_spi_send_command, .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, diff --git a/dummyflasher.c b/dummyflasher.c index 655b678..4e08386 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -123,6 +123,7 @@ static const struct spi_programmer spi_programmer_dummyflasher = { .type = SPI_CONTROLLER_DUMMY, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_UNSPECIFIED, + .check_trans = default_spi_check_trans, /* FIXME */ .command = dummy_spi_send_command, .multicommand = default_spi_send_multicommand, .read = default_spi_read, diff --git a/ft2232_spi.c b/ft2232_spi.c index 31a6c5c..d139886 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -139,6 +139,14 @@ static int get_buf(struct ftdi_context *ftdic, const unsigned char *buf, return 0; }
+static int ft2232_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + if (writecnt > 65536 || readcnt > 65536) + return SPI_INVALID_LENGTH; + return 0; +} + static int ft2232_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -148,6 +156,7 @@ static const struct spi_programmer spi_programmer_ft2232 = { .type = SPI_CONTROLLER_FT2232, .max_data_read = 64 * 1024, .max_data_write = 256, + .check_trans = ft2232_spi_check_trans, .command = ft2232_spi_send_command, .multicommand = default_spi_send_multicommand, .read = default_spi_read, @@ -407,8 +416,11 @@ static int ft2232_spi_send_command(struct flashctx *flash, int bufsize; static int oldbufsize = 0;
- if (writecnt > 65536 || readcnt > 65536) + if (ft2232_spi_check_trans(flash, writecnt, readcnt, writearr) != 0) { + msg_pspew("%s called with an unsupported transaction layout: readcnt = %d, writecnt = %d.\n", + __func__, readcnt, writecnt); return SPI_INVALID_LENGTH; + }
/* buf is not used for the response from the chip. */ bufsize = max(writecnt + 9, 260 + 9); diff --git a/ichspi.c b/ichspi.c index e60913c..0ce9dc2 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1082,6 +1082,12 @@ static int prepare_opcode(struct flashctx *flash, unsigned int writecnt, unsigne return 0; }
+static int ich_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + return prepare_opcode(flash, writecnt, readcnt, writearr, true, NULL); +} + static int ich_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -1563,6 +1569,7 @@ static const struct spi_programmer spi_programmer_ich7 = { .type = SPI_CONTROLLER_ICH7, .max_data_read = 64, .max_data_write = 64, + .check_trans = ich_spi_check_trans, .command = ich_spi_send_command, .multicommand = ich_spi_send_multicommand, .read = default_spi_read, @@ -1574,6 +1581,7 @@ static const struct spi_programmer spi_programmer_ich9 = { .type = SPI_CONTROLLER_ICH9, .max_data_read = 64, .max_data_write = 64, + .check_trans = ich_spi_check_trans, .command = ich_spi_send_command, .multicommand = ich_spi_send_multicommand, .read = default_spi_read, @@ -1881,6 +1889,7 @@ static const struct spi_programmer spi_programmer_via = { .type = SPI_CONTROLLER_VIA, .max_data_read = 16, .max_data_write = 16, + .check_trans = ich_spi_check_trans, .command = ich_spi_send_command, .multicommand = ich_spi_send_multicommand, .read = default_spi_read, diff --git a/it85spi.c b/it85spi.c index 0b074eb..67c7185 100644 --- a/it85spi.c +++ b/it85spi.c @@ -280,6 +280,7 @@ static const struct spi_programmer spi_programmer_it85xx = { .type = SPI_CONTROLLER_IT85XX, .max_data_read = 64, .max_data_write = 64, + .check_trans = default_spi_check_trans, .command = it85xx_spi_send_command, .multicommand = default_spi_send_multicommand, .read = default_spi_read, diff --git a/it87spi.c b/it87spi.c index a35ddc0..f7b2a9e 100644 --- a/it87spi.c +++ b/it87spi.c @@ -104,6 +104,8 @@ void probe_superio_ite(void) return; }
+static int it8716f_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr); static int it8716f_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -117,6 +119,7 @@ static const struct spi_programmer spi_programmer_it87xx = { .type = SPI_CONTROLLER_IT87XX, .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, + .check_trans = it8716f_spi_check_trans, .command = it8716f_spi_send_command, .multicommand = default_spi_send_multicommand, .read = it8716f_spi_chip_read, @@ -242,6 +245,18 @@ int init_superio_ite(void) return ret; }
+static int it8716f_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + if (readcnt > 3) { + return -1; + } + if (writecnt != 1 && writecnt != 2 && writecnt != 4 && writecnt != 5) { + return 1; + } + return 0; +} + /* * The IT8716F only supports commands with length 1,2,4,5 bytes including * command byte and can not read more than 3 bytes from the device. @@ -259,14 +274,16 @@ static int it8716f_spi_send_command(struct flashctx *flash, uint8_t busy, writeenc; int i;
+ if (it8716f_spi_check_trans(flash, writecnt, readcnt, writearr) != 0) { + msg_pspew("%s called with an unsupported transaction layout: readcnt = %d, writecnt = %d.\n", + __func__, readcnt, writecnt); + return SPI_INVALID_LENGTH; + } + do { busy = INB(it8716f_flashport) & 0x80; } while (busy); - if (readcnt > 3) { - msg_pinfo("%s called with unsupported readcnt %i.\n", - __func__, readcnt); - return SPI_INVALID_LENGTH; - } + switch (writecnt) { case 1: OUTB(writearr[0], it8716f_flashport + 1); @@ -292,9 +309,7 @@ static int it8716f_spi_send_command(struct flashctx *flash, OUTB(writearr[4], it8716f_flashport + 7); writeenc = 0x3; break; - default: - msg_pinfo("%s called with unsupported writecnt %i.\n", - __func__, writecnt); + default: /* Should never happen due to the checks in it8716f_spi_check_trans() */ return SPI_INVALID_LENGTH; } /* diff --git a/linux_spi.c b/linux_spi.c index 2f46463..2301b84 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -37,6 +37,8 @@ static int fd = -1;
static int linux_spi_shutdown(void *data); +static int linux_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr); static int linux_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *txbuf, @@ -50,6 +52,7 @@ static const struct spi_programmer spi_programmer_linux = { .type = SPI_CONTROLLER_LINUX, .max_data_read = MAX_DATA_UNSPECIFIED, /* TODO? */ .max_data_write = MAX_DATA_UNSPECIFIED, /* TODO? */ + .check_trans = linux_spi_check_trans, .command = linux_spi_send_command, .multicommand = default_spi_send_multicommand, .read = linux_spi_read, @@ -129,29 +132,41 @@ static int linux_spi_shutdown(void *data) return 0; }
+static int linux_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + /* The implementation currently does not support requests that + don't start with sending a command. */ + if (writecnt == 0) + return SPI_INVALID_LENGTH; + return 0; +} + static int linux_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, - const unsigned char *txbuf, - unsigned char *rxbuf) + const unsigned char *writearr, + unsigned char *readarr) { int iocontrol_code; struct spi_ioc_transfer msg[2] = { { - .tx_buf = (uint64_t)(ptrdiff_t)txbuf, + .tx_buf = (uint64_t)(ptrdiff_t)writearr, .len = writecnt, }, { - .rx_buf = (uint64_t)(ptrdiff_t)rxbuf, + .rx_buf = (uint64_t)(ptrdiff_t)readarr, .len = readcnt, }, };
+ int ret = linux_spi_check_trans(flash, writecnt, readcnt, writearr); + if (ret != 0) + msg_perr("%s called with an unsupported transaction layout: readcnt = %d, writecnt = %d.\n", + __func__, readcnt, writecnt); + return ret; + if (fd == -1) return -1; - /* The implementation currently does not support requests that - don't start with sending a command. */ - if (writecnt == 0) - return SPI_INVALID_LENGTH;
/* Just submit the first (write) request in case there is nothing to read. Otherwise submit both requests. */ diff --git a/programmer.h b/programmer.h index dedec67..bcd9547 100644 --- a/programmer.h +++ b/programmer.h @@ -529,9 +529,18 @@ struct spi_programmer { int (*read)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write_256)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write_aai)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); + /* Returns 0 if the supplied data can be sent by this programmer. Possible error codes include: + * SPI_INVALID_LENGTH if readcnt, writecnt or their combination is not allowed + * negative values if readcnt is too big + * positive values if writecnt is too big */ + int (*check_trans)(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr); + int (*check_transs)(struct flashctx *flash, struct spi_command *cmds); const void *data; };
+int default_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr); int default_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int default_spi_send_multicommand(struct flashctx *flash, struct spi_command *cmds); diff --git a/sb600spi.c b/sb600spi.c index fe60aa9..9acf476 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -89,33 +89,37 @@ static void execute_command(void) ; }
+static int sb600_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + /* We expect 1 opcode byte plus up to 8 data bytes (in any direction): + * the opcode has its own register, the data bytes are sent via an 8 byte FIFO. */ + if ((writecnt == 0) || (writecnt - 1 + readcnt > 8)) { + return SPI_INVALID_LENGTH; + } + return 0; +} + + static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { int count; - /* First byte is cmd which can not being sent through FIFO. */ - unsigned char cmd = *writearr++; + unsigned char cmd = *writearr; unsigned int readoffby1; unsigned char readwrite;
- writecnt--; - - msg_pspew("%s, cmd=%x, writecnt=%x, readcnt=%x\n", - __func__, cmd, writecnt, readcnt); - - if (readcnt > 8) { - msg_pinfo("%s, SB600 SPI controller can not receive %d bytes, " - "it is limited to 8 bytes\n", __func__, readcnt); + if (sb600_spi_check_trans(flash, writecnt, readcnt, writearr) != 0) { + msg_perr("%s called with an unsupported transaction layout: readcnt = %d, writecnt = %d.\n", + __func__, readcnt, writecnt); return SPI_INVALID_LENGTH; }
- if (writecnt > 8) { - msg_pinfo("%s, SB600 SPI controller can not send %d bytes, " - "it is limited to 8 bytes\n", __func__, writecnt); - return SPI_INVALID_LENGTH; - } + /* First byte is cmd which can not be sent through FIFO. */ + writecnt--; + writearr++;
/* This is a workaround for a bug in SB600 and SB700. If we only send * an opcode and no additional data/address, the SPI controller will @@ -123,7 +127,7 @@ static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, * the chip response is discarded and will not end up in the FIFO. * It is unclear if the CS# line is set high too early as well. */ - readoffby1 = (writecnt) ? 0 : 1; + readoffby1 = (writecnt == 0) ? 1 : 0; readwrite = (readcnt + readoffby1) << 4 | (writecnt); mmio_writeb(readwrite, sb600_spibar + 1); mmio_writeb(cmd, sb600_spibar + 0); @@ -196,13 +200,14 @@ static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt,
static const struct spi_programmer spi_programmer_sb600 = { .type = SPI_CONTROLLER_SB600, - .max_data_read = 8, - .max_data_write = 5, - .command = sb600_spi_send_command, - .multicommand = default_spi_send_multicommand, - .read = default_spi_read, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, + .max_data_read = 8, + .max_data_write = 5, + .check_trans = sb600_spi_check_trans, + .command = sb600_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, };
int sb600_probe_spi(struct pci_dev *dev) diff --git a/serprog.c b/serprog.c index b179ea4..4b0dc19 100644 --- a/serprog.c +++ b/serprog.c @@ -234,31 +234,31 @@ err_out: return 1; }
-static int sp_check_commandavail(uint8_t command) +static int sp_check_commandavail(uint8_t cmd) { int byteoffs, bitoffs; - byteoffs = command / 8; - bitoffs = command % 8; + byteoffs = cmd / 8; + bitoffs = cmd % 8; return (sp_cmdmap[byteoffs] & (1 << bitoffs)) ? 1 : 0; }
static int sp_automatic_cmdcheck(uint8_t cmd) { - if ((sp_check_avail_automatic) && (sp_check_commandavail(cmd) == 0)) { - msg_pdbg("Warning: Automatic command availability check failed " - "for cmd 0x%x - won't execute cmd\n", cmd); + if ((sp_check_avail_automatic) && (sp_check_commandavail(cmd) == 0)) return 1; - } return 0; }
-static int sp_docommand(uint8_t command, uint32_t parmlen, +static int sp_docommand(uint8_t cmd, uint32_t parmlen, uint8_t *params, uint32_t retlen, void *retparms) { unsigned char c; - if (sp_automatic_cmdcheck(command)) + if (sp_automatic_cmdcheck(cmd)) { + msg_pdbg("Warning: Automatic command availability check failed " + "for cmd 0x%x - won't execute cmd\n", cmd); return 1; - if (write(sp_fd, &command, 1) != 1) { + } + if (write(sp_fd, &cmd, 1) != 1) { msg_perr("Error: cannot write op code: %s\n", strerror(errno)); return 1; } @@ -316,8 +316,11 @@ static void sp_flush_stream(void) static int sp_stream_buffer_op(uint8_t cmd, uint32_t parmlen, uint8_t * parms) { uint8_t *sp; - if (sp_automatic_cmdcheck(cmd)) + if (sp_automatic_cmdcheck(cmd)) { + msg_pdbg("Warning: Automatic command availability check failed " + "for cmd 0x%x - won't execute cmd\n", cmd); return 1; + } sp = malloc(1 + parmlen); if (!sp) sp_die("Error: cannot malloc command buffer"); sp[0] = cmd; @@ -332,6 +335,12 @@ static int sp_stream_buffer_op(uint8_t cmd, uint32_t parmlen, uint8_t * parms) return 0; }
+static int serprog_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + return sp_automatic_cmdcheck(S_CMD_O_SPIOP); +} + static int serprog_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -342,6 +351,7 @@ static struct spi_programmer spi_programmer_serprog = { .type = SPI_CONTROLLER_SERPROG, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, + .check_trans = serprog_spi_check_trans, .command = serprog_spi_send_command, .multicommand = default_spi_send_multicommand, .read = serprog_spi_read, diff --git a/spi.c b/spi.c index 94a76a7..046d095 100644 --- a/spi.c +++ b/spi.c @@ -30,12 +30,17 @@ #include "programmer.h" #include "spi.h"
+int default_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + return 0; +} + int spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { - return flash->pgm->spi.command(flash, writecnt, readcnt, writearr, - readarr); + return flash->pgm->spi.command(flash, writecnt, readcnt, writearr, readarr); }
int spi_send_multicommand(struct flashctx *flash, struct spi_command *cmds) diff --git a/wbsio_spi.c b/wbsio_spi.c index 778b983..ec14c28 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -108,6 +108,15 @@ static uint8_t determine_mode(struct flashctx *flash, unsigned int writecnt, uns return 0; }
+static int wbsio_spi_check_trans(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr) +{ + if (determine_mode(flash, writecnt, readcnt, writearr) == 0) + return SPI_INVALID_LENGTH; + + return 0; +} + static int wbsio_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, @@ -119,6 +128,7 @@ static const struct spi_programmer spi_programmer_wbsio = { .type = SPI_CONTROLLER_WBSIO, .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, + .check_trans = wbsio_spi_check_trans, .command = wbsio_spi_send_command, .multicommand = default_spi_send_multicommand, .read = wbsio_spi_read,