From 91d0231213629674c3a551d900c1b79a8f520caf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 26 Jun 2019 21:54:46 +0200 Subject: [PATCH] tests/pflash-cfi02: Refactor to support testing multiple configurations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the FlashConfig structure, to be able to run the same set of tests on different flash models/configurations. Signed-off-by: Stephen Checkoway Message-Id: <20190426162624.55977-6-stephen.checkoway@oberlin.edu> Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé [PMD: Extracted from bigger patch] Reviewed-by: Alistair Francis Signed-off-by: Philippe Mathieu-Daudé --- 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;