| Message ID | 20190627202719.17739-14-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | block/pflash_cfi02: Implement missing AMD pflash functionality | expand |
On Thu, Jun 27, 2019 at 1:50 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Introduce the FlashConfig structure, to be able to run the same set > of tests on different flash models/configurations. > > Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu> > Message-Id: <20190426162624.55977-6-stephen.checkoway@oberlin.edu> > [PMD: Extracted from bigger patch] > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > tests/pflash-cfi02-test.c | 386 +++++++++++++++++++++++++++----------- > 1 file changed, 277 insertions(+), 109 deletions(-) > > diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c > index e090b2e3a0..b00f5ca2e7 100644 > --- a/tests/pflash-cfi02-test.c > +++ b/tests/pflash-cfi02-test.c > @@ -17,12 +17,18 @@ > */ > > #define MP_FLASH_SIZE_MAX (32 * 1024 * 1024) > +#define FLASH_SIZE (8 * 1024 * 1024) > #define BASE_ADDR (0x100000000ULL - MP_FLASH_SIZE_MAX) > > -#define FLASH_WIDTH 2 > -#define CFI_ADDR (FLASH_WIDTH * 0x55) > -#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555) > -#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA) > +/* Use a newtype to keep flash addresses separate from byte addresses. */ > +typedef struct { > + uint64_t addr; > +} faddr; > +#define FLASH_ADDR(x) ((faddr) { .addr = (x) }) > + > +#define CFI_ADDR FLASH_ADDR(0x55) > +#define UNLOCK0_ADDR FLASH_ADDR(0x555) > +#define UNLOCK1_ADDR FLASH_ADDR(0x2AA) > > #define CFI_CMD 0x98 > #define UNLOCK0_CMD 0xAA > @@ -35,170 +41,313 @@ > #define UNLOCK_BYPASS_CMD 0x20 > #define UNLOCK_BYPASS_RESET_CMD 0x00 > > +typedef struct { > + int bank_width; > + > + QTestState *qtest; > +} FlashConfig; > + > static char image_path[] = "/tmp/qtest.XXXXXX"; > > -static inline void flash_write(uint64_t byte_addr, uint16_t data) > +/* > + * The pflash implementation allows some parameters to be unspecified. We want > + * to test those configurations but we also need to know the real values in > + * our testing code. So after we launch qemu, we'll need a new FlashConfig > + * with the correct values filled in. > + */ > +static FlashConfig expand_config_defaults(const FlashConfig *c) > { > - qtest_writew(global_qtest, BASE_ADDR + byte_addr, data); > + FlashConfig ret = *c; > + > + if (ret.bank_width == 0) { > + ret.bank_width = 2; > + } > + > + /* XXX: Limitations of test harness. */ > + assert(ret.bank_width == 2); > + return ret; > } > > -static inline uint16_t flash_read(uint64_t byte_addr) > +/* > + * Return a bit mask suitable for extracting the least significant > + * status/query response from an interleaved response. > + */ > +static inline uint64_t device_mask(const FlashConfig *c) > { > - return qtest_readw(global_qtest, BASE_ADDR + byte_addr); > + return (uint64_t)-1; > } > > -static void unlock(void) > +/* > + * Return a bit mask exactly as long as the bank_width. > + */ > +static inline uint64_t bank_mask(const FlashConfig *c) > { > - flash_write(UNLOCK0_ADDR, UNLOCK0_CMD); > - flash_write(UNLOCK1_ADDR, UNLOCK1_CMD); > + if (c->bank_width == 8) { > + return (uint64_t)-1; > + } > + return (1ULL << (c->bank_width * 8)) - 1ULL; > } > > -static void reset(void) > +static inline void flash_write(const FlashConfig *c, uint64_t byte_addr, > + uint64_t data) > { > - flash_write(0, RESET_CMD); > -} > - > -static void sector_erase(uint64_t byte_addr) > -{ > - unlock(); > - flash_write(UNLOCK0_ADDR, 0x80); > - unlock(); > - flash_write(byte_addr, SECTOR_ERASE_CMD); > -} > - > -static void wait_for_completion(uint64_t byte_addr) > -{ > - /* If DQ6 is toggling, step the clock and ensure the toggle stops. */ > - if ((flash_read(byte_addr) & 0x40) ^ (flash_read(byte_addr) & 0x40)) { > - /* Wait for erase or program to finish. */ > - clock_step_next(); > - /* Ensure that DQ6 has stopped toggling. */ > - g_assert_cmphex(flash_read(byte_addr), ==, flash_read(byte_addr)); > + /* Sanity check our tests. */ > + assert((data & ~bank_mask(c)) == 0); > + uint64_t addr = BASE_ADDR + byte_addr; > + switch (c->bank_width) { > + case 1: > + qtest_writeb(c->qtest, addr, data); > + break; > + case 2: > + qtest_writew(c->qtest, addr, data); > + break; > + case 4: > + qtest_writel(c->qtest, addr, data); > + break; > + case 8: > + qtest_writeq(c->qtest, addr, data); > + break; > + default: > + abort(); > } > } > > -static void bypass_program(uint64_t byte_addr, uint16_t data) > +static inline uint64_t flash_read(const FlashConfig *c, uint64_t byte_addr) > { > - flash_write(UNLOCK0_ADDR, PROGRAM_CMD); > - flash_write(byte_addr, data); > + uint64_t addr = BASE_ADDR + byte_addr; > + switch (c->bank_width) { > + case 1: > + return qtest_readb(c->qtest, addr); > + case 2: > + return qtest_readw(c->qtest, addr); > + case 4: > + return qtest_readl(c->qtest, addr); > + case 8: > + return qtest_readq(c->qtest, addr); > + default: > + abort(); > + } > +} > + > +/* > + * Convert a flash address expressed in the maximum width of the device as a > + * byte address. > + */ > +static inline uint64_t as_byte_addr(const FlashConfig *c, faddr flash_addr) > +{ > + /* > + * Command addresses are always given as addresses in the maximum > + * supported bus size for the flash chip. So an x8/x16 chip in x8 mode > + * uses addresses 0xAAA and 0x555 to unlock because the least significant > + * bit is ignored. (0x555 rather than 0x554 is traditional.) > + * > + * In general we need to multiply by the maximum device width. > + */ > + return flash_addr.addr * c->bank_width; > +} > + > +/* > + * Return the command value or expected status replicated across all devices. > + */ > +static inline uint64_t replicate(const FlashConfig *c, uint64_t data) > +{ > + /* Sanity check our tests. */ > + assert((data & ~device_mask(c)) == 0); > + return data; > +} > + > +static inline void flash_cmd(const FlashConfig *c, faddr cmd_addr, > + uint8_t cmd) > +{ > + flash_write(c, as_byte_addr(c, cmd_addr), replicate(c, cmd)); > +} > + > +static inline uint64_t flash_query(const FlashConfig *c, faddr query_addr) > +{ > + return flash_read(c, as_byte_addr(c, query_addr)); > +} > + > +static inline uint64_t flash_query_1(const FlashConfig *c, faddr query_addr) > +{ > + return flash_query(c, query_addr) & device_mask(c); > +} > + > +static void unlock(const FlashConfig *c) > +{ > + flash_cmd(c, UNLOCK0_ADDR, UNLOCK0_CMD); > + flash_cmd(c, UNLOCK1_ADDR, UNLOCK1_CMD); > +} > + > +static void reset(const FlashConfig *c) > +{ > + flash_cmd(c, FLASH_ADDR(0), RESET_CMD); > +} > + > +static void sector_erase(const FlashConfig *c, uint64_t byte_addr) > +{ > + unlock(c); > + flash_cmd(c, UNLOCK0_ADDR, 0x80); > + unlock(c); > + flash_write(c, byte_addr, replicate(c, SECTOR_ERASE_CMD)); > +} > + > +static void wait_for_completion(const FlashConfig *c, uint64_t byte_addr) > +{ > + /* If DQ6 is toggling, step the clock and ensure the toggle stops. */ > + const uint64_t dq6 = replicate(c, 0x40); > + if ((flash_read(c, byte_addr) & dq6) ^ (flash_read(c, byte_addr) & dq6)) { > + /* Wait for erase or program to finish. */ > + qtest_clock_step_next(c->qtest); > + /* Ensure that DQ6 has stopped toggling. */ > + g_assert_cmphex(flash_read(c, byte_addr), ==, flash_read(c, byte_addr)); > + } > +} > + > +static void bypass_program(const FlashConfig *c, uint64_t byte_addr, > + uint16_t data) > +{ > + flash_cmd(c, UNLOCK0_ADDR, PROGRAM_CMD); > + flash_write(c, byte_addr, data); > /* > * Data isn't valid until DQ6 stops toggling. We don't model this as > * writes are immediate, but if this changes in the future, we can wait > * until the program is complete. > */ > - wait_for_completion(byte_addr); > + wait_for_completion(c, byte_addr); > } > > -static void program(uint64_t byte_addr, uint16_t data) > +static void program(const FlashConfig *c, uint64_t byte_addr, uint16_t data) > { > - unlock(); > - bypass_program(byte_addr, data); > + unlock(c); > + bypass_program(c, byte_addr, data); > } > > -static void chip_erase(void) > +static void chip_erase(const FlashConfig *c) > { > - unlock(); > - flash_write(UNLOCK0_ADDR, 0x80); > - unlock(); > - flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD); > + unlock(c); > + flash_cmd(c, UNLOCK0_ADDR, 0x80); > + unlock(c); > + flash_cmd(c, UNLOCK0_ADDR, CHIP_ERASE_CMD); > } > > -static void test_flash(void) > +static void test_flash(const void *opaque) > { > - global_qtest = qtest_initf("-M musicpal,accel=qtest " > - "-drive if=pflash,file=%s,format=raw,copy-on-read", > - image_path); > + const FlashConfig *config = opaque; > + QTestState *qtest; > + qtest = qtest_initf("-M musicpal,accel=qtest" > + " -drive if=pflash,file=%s,format=raw,copy-on-read", > + image_path); > + FlashConfig explicit_config = expand_config_defaults(config); > + explicit_config.qtest = qtest; > + const FlashConfig *c = &explicit_config; > + > /* Check the IDs. */ > - unlock(); > - flash_write(UNLOCK0_ADDR, AUTOSELECT_CMD); > - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF); > - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D); > - reset(); > + unlock(c); > + flash_cmd(c, UNLOCK0_ADDR, AUTOSELECT_CMD); > + g_assert_cmphex(flash_query(c, FLASH_ADDR(0)), ==, replicate(c, 0xBF)); > + if (c->bank_width >= 2) { > + /* > + * XXX: The ID returned by the musicpal flash chip is 16 bits which > + * wouldn't happen with an 8-bit device. It would probably be best to > + * prohibit addresses larger than the device width in pflash_cfi02.c, > + * but then we couldn't test smaller device widths at all. > + */ > + g_assert_cmphex(flash_query(c, FLASH_ADDR(1)), ==, > + replicate(c, 0x236D)); > + } > + reset(c); > > /* Check the erase blocks. */ > - flash_write(CFI_ADDR, CFI_CMD); > - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x10), ==, 'Q'); > - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x11), ==, 'R'); > - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x12), ==, 'Y'); > - /* Num erase regions. */ > - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x2C), >=, 1); > - uint32_t nb_sectors = flash_read(FLASH_WIDTH * 0x2D) + > - (flash_read(FLASH_WIDTH * 0x2E) << 8) + 1; > - uint32_t sector_len = (flash_read(FLASH_WIDTH * 0x2F) << 8) + > - (flash_read(FLASH_WIDTH * 0x30) << 16); > - reset(); > + flash_cmd(c, CFI_ADDR, CFI_CMD); > + g_assert_cmphex(flash_query(c, FLASH_ADDR(0x10)), ==, replicate(c, 'Q')); > + g_assert_cmphex(flash_query(c, FLASH_ADDR(0x11)), ==, replicate(c, 'R')); > + g_assert_cmphex(flash_query(c, FLASH_ADDR(0x12)), ==, replicate(c, 'Y')); > > + /* Num erase regions. */ > + g_assert_cmphex(flash_query_1(c, FLASH_ADDR(0x2C)), >=, 1); > + > + uint32_t nb_sectors = flash_query_1(c, FLASH_ADDR(0x2D)) + > + (flash_query_1(c, FLASH_ADDR(0x2E)) << 8) + 1; > + uint32_t sector_len = (flash_query_1(c, FLASH_ADDR(0x2F)) << 8) + > + (flash_query_1(c, FLASH_ADDR(0x30)) << 16); > + reset(c); > + > + const uint64_t dq7 = replicate(c, 0x80); > + const uint64_t dq6 = replicate(c, 0x40); > /* Erase and program sector. */ > for (uint32_t i = 0; i < nb_sectors; ++i) { > uint64_t byte_addr = i * sector_len; > - sector_erase(byte_addr); > + sector_erase(c, byte_addr); > /* Read toggle. */ > - uint16_t status0 = flash_read(byte_addr); > + uint64_t status0 = flash_read(c, byte_addr); > /* DQ7 is 0 during an erase. */ > - g_assert_cmphex(status0 & 0x80, ==, 0); > - uint16_t status1 = flash_read(byte_addr); > + g_assert_cmphex(status0 & dq7, ==, 0); > + uint64_t status1 = flash_read(c, byte_addr); > /* DQ6 toggles during an erase. */ > - g_assert_cmphex(status0 & 0x40, !=, status1 & 0x40); > + g_assert_cmphex(status0 & dq6, ==, ~status1 & dq6); > /* Wait for erase to complete. */ > - clock_step_next(); > + qtest_clock_step_next(c->qtest); > /* Ensure DQ6 has stopped toggling. */ > - g_assert_cmphex(flash_read(byte_addr), ==, flash_read(byte_addr)); > + g_assert_cmphex(flash_read(c, byte_addr), ==, flash_read(c, byte_addr)); > /* Now the data should be valid. */ > - g_assert_cmphex(flash_read(byte_addr), ==, 0xFFFF); > + g_assert_cmphex(flash_read(c, byte_addr), ==, bank_mask(c)); > > /* Program a bit pattern. */ > - program(byte_addr, 0x5555); > - g_assert_cmphex(flash_read(byte_addr), ==, 0x5555); > - program(byte_addr, 0xAA55); > - g_assert_cmphex(flash_read(byte_addr), ==, 0x0055); > + program(c, byte_addr, 0x55); > + g_assert_cmphex(flash_read(c, byte_addr) & 0xFF, ==, 0x55); > + program(c, byte_addr, 0xA5); > + g_assert_cmphex(flash_read(c, byte_addr) & 0xFF, ==, 0x05); > } > > /* Erase the chip. */ > - chip_erase(); > + chip_erase(c); > /* Read toggle. */ > - uint16_t status0 = flash_read(0); > + uint64_t status0 = flash_read(c, 0); > /* DQ7 is 0 during an erase. */ > - g_assert_cmphex(status0 & 0x80, ==, 0); > - uint16_t status1 = flash_read(0); > + g_assert_cmphex(status0 & dq7, ==, 0); > + uint64_t status1 = flash_read(c, 0); > /* DQ6 toggles during an erase. */ > - g_assert_cmphex(status0 & 0x40, !=, status1 & 0x40); > + g_assert_cmphex(status0 & dq6, ==, ~status1 & dq6); > /* Wait for erase to complete. */ > - clock_step_next(); > + qtest_clock_step_next(c->qtest); > /* Ensure DQ6 has stopped toggling. */ > - g_assert_cmphex(flash_read(0), ==, flash_read(0)); > + g_assert_cmphex(flash_read(c, 0), ==, flash_read(c, 0)); > /* Now the data should be valid. */ > - g_assert_cmphex(flash_read(0), ==, 0xFFFF); > + > + for (uint32_t i = 0; i < nb_sectors; ++i) { > + uint64_t byte_addr = i * sector_len; > + g_assert_cmphex(flash_read(c, byte_addr), ==, bank_mask(c)); > + } > > /* Unlock bypass */ > - unlock(); > - flash_write(UNLOCK0_ADDR, UNLOCK_BYPASS_CMD); > - bypass_program(0, 0x0123); > - bypass_program(2, 0x4567); > - bypass_program(4, 0x89AB); > + unlock(c); > + flash_cmd(c, UNLOCK0_ADDR, UNLOCK_BYPASS_CMD); > + bypass_program(c, 0 * c->bank_width, 0x01); > + bypass_program(c, 1 * c->bank_width, 0x23); > + bypass_program(c, 2 * c->bank_width, 0x45); > /* > * Test that bypass programming, unlike normal programming can use any > * address for the PROGRAM_CMD. > */ > - flash_write(6, PROGRAM_CMD); > - flash_write(6, 0xCDEF); > - wait_for_completion(6); > - flash_write(0, UNLOCK_BYPASS_RESET_CMD); > - bypass_program(8, 0x55AA); /* Should fail. */ > - g_assert_cmphex(flash_read(0), ==, 0x0123); > - g_assert_cmphex(flash_read(2), ==, 0x4567); > - g_assert_cmphex(flash_read(4), ==, 0x89AB); > - g_assert_cmphex(flash_read(6), ==, 0xCDEF); > - g_assert_cmphex(flash_read(8), ==, 0xFFFF); > + flash_cmd(c, FLASH_ADDR(3 * c->bank_width), PROGRAM_CMD); > + flash_write(c, 3 * c->bank_width, 0x67); > + wait_for_completion(c, 3 * c->bank_width); > + flash_cmd(c, FLASH_ADDR(0), UNLOCK_BYPASS_RESET_CMD); > + bypass_program(c, 4 * c->bank_width, 0x89); /* Should fail. */ > + g_assert_cmphex(flash_read(c, 0 * c->bank_width), ==, 0x01); > + g_assert_cmphex(flash_read(c, 1 * c->bank_width), ==, 0x23); > + g_assert_cmphex(flash_read(c, 2 * c->bank_width), ==, 0x45); > + g_assert_cmphex(flash_read(c, 3 * c->bank_width), ==, 0x67); > + g_assert_cmphex(flash_read(c, 4 * c->bank_width), ==, bank_mask(c)); > > /* Test ignored high order bits of address. */ > - flash_write(FLASH_WIDTH * 0x5555, UNLOCK0_CMD); > - flash_write(FLASH_WIDTH * 0x2AAA, UNLOCK1_CMD); > - flash_write(FLASH_WIDTH * 0x5555, AUTOSELECT_CMD); > - g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF); > - g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D); > - reset(); > + flash_cmd(c, FLASH_ADDR(0x5555), UNLOCK0_CMD); > + flash_cmd(c, FLASH_ADDR(0x2AAA), UNLOCK1_CMD); > + flash_cmd(c, FLASH_ADDR(0x5555), AUTOSELECT_CMD); > + g_assert_cmphex(flash_query(c, FLASH_ADDR(0)), ==, replicate(c, 0xBF)); > + reset(c); > > - qtest_quit(global_qtest); > + qtest_quit(qtest); > } > > static void cleanup(void *opaque) > @@ -206,6 +355,17 @@ static void cleanup(void *opaque) > unlink(image_path); > } > > +/* > + * XXX: Tests are limited to bank_width = 2 for now because that's what > + * hw/arm/musicpal.c has. > + */ > +static const FlashConfig configuration[] = { > + /* One x16 device. */ > + { > + .bank_width = 2, > + }, > +}; > + > int main(int argc, char **argv) > { > int fd = mkstemp(image_path); > @@ -214,19 +374,27 @@ int main(int argc, char **argv) > strerror(errno)); > exit(EXIT_FAILURE); > } > - if (ftruncate(fd, 8 * 1024 * 1024) < 0) { > + if (ftruncate(fd, FLASH_SIZE) < 0) { > int error_code = errno; > close(fd); > unlink(image_path); > - g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path, > - strerror(error_code)); > + g_printerr("Failed to truncate file %s to %u MB: %s\n", image_path, > + FLASH_SIZE, strerror(error_code)); > exit(EXIT_FAILURE); > } > close(fd); > > qtest_add_abrt_handler(cleanup, NULL); > g_test_init(&argc, &argv, NULL); > - qtest_add_func("pflash-cfi02", test_flash); > + > + size_t nb_configurations = sizeof configuration / sizeof configuration[0]; > + for (size_t i = 0; i < nb_configurations; ++i) { > + const FlashConfig *config = &configuration[i]; > + char *path = g_strdup_printf("pflash-cfi02/%d", > + config->bank_width); > + qtest_add_data_func(path, config, test_flash); > + g_free(path); > + } > int result = g_test_run(); > cleanup(NULL); > return result; > -- > 2.20.1 > >
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c index e090b2e3a0..b00f5ca2e7 100644 --- a/tests/pflash-cfi02-test.c +++ b/tests/pflash-cfi02-test.c @@ -17,12 +17,18 @@ */ #define MP_FLASH_SIZE_MAX (32 * 1024 * 1024) +#define FLASH_SIZE (8 * 1024 * 1024) #define BASE_ADDR (0x100000000ULL - MP_FLASH_SIZE_MAX) -#define FLASH_WIDTH 2 -#define CFI_ADDR (FLASH_WIDTH * 0x55) -#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555) -#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA) +/* Use a newtype to keep flash addresses separate from byte addresses. */ +typedef struct { + uint64_t addr; +} faddr; +#define FLASH_ADDR(x) ((faddr) { .addr = (x) }) + +#define CFI_ADDR FLASH_ADDR(0x55) +#define UNLOCK0_ADDR FLASH_ADDR(0x555) +#define UNLOCK1_ADDR FLASH_ADDR(0x2AA) #define CFI_CMD 0x98 #define UNLOCK0_CMD 0xAA @@ -35,170 +41,313 @@ #define UNLOCK_BYPASS_CMD 0x20 #define UNLOCK_BYPASS_RESET_CMD 0x00 +typedef struct { + int bank_width; + + QTestState *qtest; +} FlashConfig; + static char image_path[] = "/tmp/qtest.XXXXXX"; -static inline void flash_write(uint64_t byte_addr, uint16_t data) +/* + * The pflash implementation allows some parameters to be unspecified. We want + * to test those configurations but we also need to know the real values in + * our testing code. So after we launch qemu, we'll need a new FlashConfig + * with the correct values filled in. + */ +static FlashConfig expand_config_defaults(const FlashConfig *c) { - qtest_writew(global_qtest, BASE_ADDR + byte_addr, data); + FlashConfig ret = *c; + + if (ret.bank_width == 0) { + ret.bank_width = 2; + } + + /* XXX: Limitations of test harness. */ + assert(ret.bank_width == 2); + return ret; } -static inline uint16_t flash_read(uint64_t byte_addr) +/* + * Return a bit mask suitable for extracting the least significant + * status/query response from an interleaved response. + */ +static inline uint64_t device_mask(const FlashConfig *c) { - return qtest_readw(global_qtest, BASE_ADDR + byte_addr); + return (uint64_t)-1; } -static void unlock(void) +/* + * Return a bit mask exactly as long as the bank_width. + */ +static inline uint64_t bank_mask(const FlashConfig *c) { - flash_write(UNLOCK0_ADDR, UNLOCK0_CMD); - flash_write(UNLOCK1_ADDR, UNLOCK1_CMD); + if (c->bank_width == 8) { + return (uint64_t)-1; + } + return (1ULL << (c->bank_width * 8)) - 1ULL; } -static void reset(void) +static inline void flash_write(const FlashConfig *c, uint64_t byte_addr, + uint64_t data) { - flash_write(0, RESET_CMD); -} - -static void sector_erase(uint64_t byte_addr) -{ - unlock(); - flash_write(UNLOCK0_ADDR, 0x80); - unlock(); - flash_write(byte_addr, SECTOR_ERASE_CMD); -} - -static void wait_for_completion(uint64_t byte_addr) -{ - /* If DQ6 is toggling, step the clock and ensure the toggle stops. */ - if ((flash_read(byte_addr) & 0x40) ^ (flash_read(byte_addr) & 0x40)) { - /* Wait for erase or program to finish. */ - clock_step_next(); - /* Ensure that DQ6 has stopped toggling. */ - g_assert_cmphex(flash_read(byte_addr), ==, flash_read(byte_addr)); + /* Sanity check our tests. */ + assert((data & ~bank_mask(c)) == 0); + uint64_t addr = BASE_ADDR + byte_addr; + switch (c->bank_width) { + case 1: + qtest_writeb(c->qtest, addr, data); + break; + case 2: + qtest_writew(c->qtest, addr, data); + break; + case 4: + qtest_writel(c->qtest, addr, data); + break; + case 8: + qtest_writeq(c->qtest, addr, data); + break; + default: + abort(); } } -static void bypass_program(uint64_t byte_addr, uint16_t data) +static inline uint64_t flash_read(const FlashConfig *c, uint64_t byte_addr) { - flash_write(UNLOCK0_ADDR, PROGRAM_CMD); - flash_write(byte_addr, data); + uint64_t addr = BASE_ADDR + byte_addr; + switch (c->bank_width) { + case 1: + return qtest_readb(c->qtest, addr); + case 2: + return qtest_readw(c->qtest, addr); + case 4: + return qtest_readl(c->qtest, addr); + case 8: + return qtest_readq(c->qtest, addr); + default: + abort(); + } +} + +/* + * Convert a flash address expressed in the maximum width of the device as a + * byte address. + */ +static inline uint64_t as_byte_addr(const FlashConfig *c, faddr flash_addr) +{ + /* + * Command addresses are always given as addresses in the maximum + * supported bus size for the flash chip. So an x8/x16 chip in x8 mode + * uses addresses 0xAAA and 0x555 to unlock because the least significant + * bit is ignored. (0x555 rather than 0x554 is traditional.) + * + * In general we need to multiply by the maximum device width. + */ + return flash_addr.addr * c->bank_width; +} + +/* + * Return the command value or expected status replicated across all devices. + */ +static inline uint64_t replicate(const FlashConfig *c, uint64_t data) +{ + /* Sanity check our tests. */ + assert((data & ~device_mask(c)) == 0); + return data; +} + +static inline void flash_cmd(const FlashConfig *c, faddr cmd_addr, + uint8_t cmd) +{ + flash_write(c, as_byte_addr(c, cmd_addr), replicate(c, cmd)); +} + +static inline uint64_t flash_query(const FlashConfig *c, faddr query_addr) +{ + return flash_read(c, as_byte_addr(c, query_addr)); +} + +static inline uint64_t flash_query_1(const FlashConfig *c, faddr query_addr) +{ + return flash_query(c, query_addr) & device_mask(c); +} + +static void unlock(const FlashConfig *c) +{ + flash_cmd(c, UNLOCK0_ADDR, UNLOCK0_CMD); + flash_cmd(c, UNLOCK1_ADDR, UNLOCK1_CMD); +} + +static void reset(const FlashConfig *c) +{ + flash_cmd(c, FLASH_ADDR(0), RESET_CMD); +} + +static void sector_erase(const FlashConfig *c, uint64_t byte_addr) +{ + unlock(c); + flash_cmd(c, UNLOCK0_ADDR, 0x80); + unlock(c); + flash_write(c, byte_addr, replicate(c, SECTOR_ERASE_CMD)); +} + +static void wait_for_completion(const FlashConfig *c, uint64_t byte_addr) +{ + /* If DQ6 is toggling, step the clock and ensure the toggle stops. */ + const uint64_t dq6 = replicate(c, 0x40); + if ((flash_read(c, byte_addr) & dq6) ^ (flash_read(c, byte_addr) & dq6)) { + /* Wait for erase or program to finish. */ + qtest_clock_step_next(c->qtest); + /* Ensure that DQ6 has stopped toggling. */ + g_assert_cmphex(flash_read(c, byte_addr), ==, flash_read(c, byte_addr)); + } +} + +static void bypass_program(const FlashConfig *c, uint64_t byte_addr, + uint16_t data) +{ + flash_cmd(c, UNLOCK0_ADDR, PROGRAM_CMD); + flash_write(c, byte_addr, data); /* * Data isn't valid until DQ6 stops toggling. We don't model this as * writes are immediate, but if this changes in the future, we can wait * until the program is complete. */ - wait_for_completion(byte_addr); + wait_for_completion(c, byte_addr); } -static void program(uint64_t byte_addr, uint16_t data) +static void program(const FlashConfig *c, uint64_t byte_addr, uint16_t data) { - unlock(); - bypass_program(byte_addr, data); + unlock(c); + bypass_program(c, byte_addr, data); } -static void chip_erase(void) +static void chip_erase(const FlashConfig *c) { - unlock(); - flash_write(UNLOCK0_ADDR, 0x80); - unlock(); - flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD); + unlock(c); + flash_cmd(c, UNLOCK0_ADDR, 0x80); + unlock(c); + flash_cmd(c, UNLOCK0_ADDR, CHIP_ERASE_CMD); } -static void test_flash(void) +static void test_flash(const void *opaque) { - global_qtest = qtest_initf("-M musicpal,accel=qtest " - "-drive if=pflash,file=%s,format=raw,copy-on-read", - image_path); + const FlashConfig *config = opaque; + QTestState *qtest; + qtest = qtest_initf("-M musicpal,accel=qtest" + " -drive if=pflash,file=%s,format=raw,copy-on-read", + image_path); + FlashConfig explicit_config = expand_config_defaults(config); + explicit_config.qtest = qtest; + const FlashConfig *c = &explicit_config; + /* Check the IDs. */ - unlock(); - flash_write(UNLOCK0_ADDR, AUTOSELECT_CMD); - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF); - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D); - reset(); + unlock(c); + flash_cmd(c, UNLOCK0_ADDR, AUTOSELECT_CMD); + g_assert_cmphex(flash_query(c, FLASH_ADDR(0)), ==, replicate(c, 0xBF)); + if (c->bank_width >= 2) { + /* + * XXX: The ID returned by the musicpal flash chip is 16 bits which + * wouldn't happen with an 8-bit device. It would probably be best to + * prohibit addresses larger than the device width in pflash_cfi02.c, + * but then we couldn't test smaller device widths at all. + */ + g_assert_cmphex(flash_query(c, FLASH_ADDR(1)), ==, + replicate(c, 0x236D)); + } + reset(c); /* Check the erase blocks. */ - flash_write(CFI_ADDR, CFI_CMD); - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x10), ==, 'Q'); - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x11), ==, 'R'); - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x12), ==, 'Y'); - /* Num erase regions. */ - g_assert_cmphex(flash_read(FLASH_WIDTH * 0x2C), >=, 1); - uint32_t nb_sectors = flash_read(FLASH_WIDTH * 0x2D) + - (flash_read(FLASH_WIDTH * 0x2E) << 8) + 1; - uint32_t sector_len = (flash_read(FLASH_WIDTH * 0x2F) << 8) + - (flash_read(FLASH_WIDTH * 0x30) << 16); - reset(); + flash_cmd(c, CFI_ADDR, CFI_CMD); + g_assert_cmphex(flash_query(c, FLASH_ADDR(0x10)), ==, replicate(c, 'Q')); + g_assert_cmphex(flash_query(c, FLASH_ADDR(0x11)), ==, replicate(c, 'R')); + g_assert_cmphex(flash_query(c, FLASH_ADDR(0x12)), ==, replicate(c, 'Y')); + /* Num erase regions. */ + g_assert_cmphex(flash_query_1(c, FLASH_ADDR(0x2C)), >=, 1); + + uint32_t nb_sectors = flash_query_1(c, FLASH_ADDR(0x2D)) + + (flash_query_1(c, FLASH_ADDR(0x2E)) << 8) + 1; + uint32_t sector_len = (flash_query_1(c, FLASH_ADDR(0x2F)) << 8) + + (flash_query_1(c, FLASH_ADDR(0x30)) << 16); + reset(c); + + const uint64_t dq7 = replicate(c, 0x80); + const uint64_t dq6 = replicate(c, 0x40); /* Erase and program sector. */ for (uint32_t i = 0; i < nb_sectors; ++i) { uint64_t byte_addr = i * sector_len; - sector_erase(byte_addr); + sector_erase(c, byte_addr); /* Read toggle. */ - uint16_t status0 = flash_read(byte_addr); + uint64_t status0 = flash_read(c, byte_addr); /* DQ7 is 0 during an erase. */ - g_assert_cmphex(status0 & 0x80, ==, 0); - uint16_t status1 = flash_read(byte_addr); + g_assert_cmphex(status0 & dq7, ==, 0); + uint64_t status1 = flash_read(c, byte_addr); /* DQ6 toggles during an erase. */ - g_assert_cmphex(status0 & 0x40, !=, status1 & 0x40); + g_assert_cmphex(status0 & dq6, ==, ~status1 & dq6); /* Wait for erase to complete. */ - clock_step_next(); + qtest_clock_step_next(c->qtest); /* Ensure DQ6 has stopped toggling. */ - g_assert_cmphex(flash_read(byte_addr), ==, flash_read(byte_addr)); + g_assert_cmphex(flash_read(c, byte_addr), ==, flash_read(c, byte_addr)); /* Now the data should be valid. */ - g_assert_cmphex(flash_read(byte_addr), ==, 0xFFFF); + g_assert_cmphex(flash_read(c, byte_addr), ==, bank_mask(c)); /* Program a bit pattern. */ - program(byte_addr, 0x5555); - g_assert_cmphex(flash_read(byte_addr), ==, 0x5555); - program(byte_addr, 0xAA55); - g_assert_cmphex(flash_read(byte_addr), ==, 0x0055); + program(c, byte_addr, 0x55); + g_assert_cmphex(flash_read(c, byte_addr) & 0xFF, ==, 0x55); + program(c, byte_addr, 0xA5); + g_assert_cmphex(flash_read(c, byte_addr) & 0xFF, ==, 0x05); } /* Erase the chip. */ - chip_erase(); + chip_erase(c); /* Read toggle. */ - uint16_t status0 = flash_read(0); + uint64_t status0 = flash_read(c, 0); /* DQ7 is 0 during an erase. */ - g_assert_cmphex(status0 & 0x80, ==, 0); - uint16_t status1 = flash_read(0); + g_assert_cmphex(status0 & dq7, ==, 0); + uint64_t status1 = flash_read(c, 0); /* DQ6 toggles during an erase. */ - g_assert_cmphex(status0 & 0x40, !=, status1 & 0x40); + g_assert_cmphex(status0 & dq6, ==, ~status1 & dq6); /* Wait for erase to complete. */ - clock_step_next(); + qtest_clock_step_next(c->qtest); /* Ensure DQ6 has stopped toggling. */ - g_assert_cmphex(flash_read(0), ==, flash_read(0)); + g_assert_cmphex(flash_read(c, 0), ==, flash_read(c, 0)); /* Now the data should be valid. */ - g_assert_cmphex(flash_read(0), ==, 0xFFFF); + + for (uint32_t i = 0; i < nb_sectors; ++i) { + uint64_t byte_addr = i * sector_len; + g_assert_cmphex(flash_read(c, byte_addr), ==, bank_mask(c)); + } /* Unlock bypass */ - unlock(); - flash_write(UNLOCK0_ADDR, UNLOCK_BYPASS_CMD); - bypass_program(0, 0x0123); - bypass_program(2, 0x4567); - bypass_program(4, 0x89AB); + unlock(c); + flash_cmd(c, UNLOCK0_ADDR, UNLOCK_BYPASS_CMD); + bypass_program(c, 0 * c->bank_width, 0x01); + bypass_program(c, 1 * c->bank_width, 0x23); + bypass_program(c, 2 * c->bank_width, 0x45); /* * Test that bypass programming, unlike normal programming can use any * address for the PROGRAM_CMD. */ - flash_write(6, PROGRAM_CMD); - flash_write(6, 0xCDEF); - wait_for_completion(6); - flash_write(0, UNLOCK_BYPASS_RESET_CMD); - bypass_program(8, 0x55AA); /* Should fail. */ - g_assert_cmphex(flash_read(0), ==, 0x0123); - g_assert_cmphex(flash_read(2), ==, 0x4567); - g_assert_cmphex(flash_read(4), ==, 0x89AB); - g_assert_cmphex(flash_read(6), ==, 0xCDEF); - g_assert_cmphex(flash_read(8), ==, 0xFFFF); + flash_cmd(c, FLASH_ADDR(3 * c->bank_width), PROGRAM_CMD); + flash_write(c, 3 * c->bank_width, 0x67); + wait_for_completion(c, 3 * c->bank_width); + flash_cmd(c, FLASH_ADDR(0), UNLOCK_BYPASS_RESET_CMD); + bypass_program(c, 4 * c->bank_width, 0x89); /* Should fail. */ + g_assert_cmphex(flash_read(c, 0 * c->bank_width), ==, 0x01); + g_assert_cmphex(flash_read(c, 1 * c->bank_width), ==, 0x23); + g_assert_cmphex(flash_read(c, 2 * c->bank_width), ==, 0x45); + g_assert_cmphex(flash_read(c, 3 * c->bank_width), ==, 0x67); + g_assert_cmphex(flash_read(c, 4 * c->bank_width), ==, bank_mask(c)); /* Test ignored high order bits of address. */ - flash_write(FLASH_WIDTH * 0x5555, UNLOCK0_CMD); - flash_write(FLASH_WIDTH * 0x2AAA, UNLOCK1_CMD); - flash_write(FLASH_WIDTH * 0x5555, AUTOSELECT_CMD); - g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF); - g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D); - reset(); + flash_cmd(c, FLASH_ADDR(0x5555), UNLOCK0_CMD); + flash_cmd(c, FLASH_ADDR(0x2AAA), UNLOCK1_CMD); + flash_cmd(c, FLASH_ADDR(0x5555), AUTOSELECT_CMD); + g_assert_cmphex(flash_query(c, FLASH_ADDR(0)), ==, replicate(c, 0xBF)); + reset(c); - qtest_quit(global_qtest); + qtest_quit(qtest); } static void cleanup(void *opaque) @@ -206,6 +355,17 @@ static void cleanup(void *opaque) unlink(image_path); } +/* + * XXX: Tests are limited to bank_width = 2 for now because that's what + * hw/arm/musicpal.c has. + */ +static const FlashConfig configuration[] = { + /* One x16 device. */ + { + .bank_width = 2, + }, +}; + int main(int argc, char **argv) { int fd = mkstemp(image_path); @@ -214,19 +374,27 @@ int main(int argc, char **argv) strerror(errno)); exit(EXIT_FAILURE); } - if (ftruncate(fd, 8 * 1024 * 1024) < 0) { + if (ftruncate(fd, FLASH_SIZE) < 0) { int error_code = errno; close(fd); unlink(image_path); - g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path, - strerror(error_code)); + g_printerr("Failed to truncate file %s to %u MB: %s\n", image_path, + FLASH_SIZE, strerror(error_code)); exit(EXIT_FAILURE); } close(fd); qtest_add_abrt_handler(cleanup, NULL); g_test_init(&argc, &argv, NULL); - qtest_add_func("pflash-cfi02", test_flash); + + size_t nb_configurations = sizeof configuration / sizeof configuration[0]; + for (size_t i = 0; i < nb_configurations; ++i) { + const FlashConfig *config = &configuration[i]; + char *path = g_strdup_printf("pflash-cfi02/%d", + config->bank_width); + qtest_add_data_func(path, config, test_flash); + g_free(path); + } int result = g_test_run(); cleanup(NULL); return result;