* [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 01/13] tests/pflash-cfi02: Add test for supported CFI commands Philippe Mathieu-Daudé
` (13 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
Hi,
While reviewing Stephen Checkoway's v4 "Implement missing AMD
pflash functionality" [*] I found it hard (for me) to digest,
so I took step by step notes. This series is the result of
those notes.
Regarding Stephen's series, this series only contains the
generic code movement and trivial cleanup. The other patches
are rather dense and I need more time to study the specs.
Stephen: If you take out the patch #2 ("Use the GLib API"),
you can rebase your series on top of this.
I'd appreciate if you can adapt your tests to use the GLib
functions, else I plan to do it later.
Regards,
Phil.
[*] https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg04595.html
Philippe Mathieu-Daudé (10):
tests/pflash-cfi02: Use the GLib API
tests/pflash-cfi02: Use IEC binary prefixes for size constants
hw/block/pflash_cfi02: Fix debug format string
hw/block/pflash_cfi02: Add an enum to define the write cycles
hw/block/pflash_cfi02: Add helpers to manipulate the status bits
hw/block/pflash_cfi02: Simplify a statement using fall through
hw/block/pflash_cfi02: Use the ldst API in pflash_write()
hw/block/pflash_cfi02: Use the ldst API in pflash_read()
hw/block/pflash_cfi02: Extract the pflash_data_read() function
hw/block/pflash_cfi02: Unify the MemoryRegionOps
Stephen Checkoway (3):
tests/pflash-cfi02: Add test for supported CFI commands
hw/block/pflash_cfi02: Fix command address comparison
hw/block/pflash_cfi02: Use the chip erase time specified in the CFI
table
hw/block/pflash_cfi02.c | 234 +++++++++++++++++---------------------
tests/Makefile.include | 2 +
tests/pflash-cfi02-test.c | 232 +++++++++++++++++++++++++++++++++++++
3 files changed, 339 insertions(+), 129 deletions(-)
create mode 100644 tests/pflash-cfi02-test.c
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 01/13] tests/pflash-cfi02: Add test for supported CFI commands
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 02/13] tests/pflash-cfi02: Use the GLib API Philippe Mathieu-Daudé
` (12 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
From: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Test the AMD command set for parallel flash chips. This test uses an
ARM musicpal board with a pflash drive to test the following list of
currently-supported commands.
- Autoselect
- CFI
- Sector erase
- Chip erase
- Program
- Unlock bypass
- Reset
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-2-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: reworded the patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/Makefile.include | 2 +
tests/pflash-cfi02-test.c | 225 ++++++++++++++++++++++++++++++++++++++
2 files changed, 227 insertions(+)
create mode 100644 tests/pflash-cfi02-test.c
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7c8b9c84b24..b84a308d9a1 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -263,6 +263,7 @@ check-qtest-arm-y += tests/m25p80-test$(EXESUF)
check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
check-qtest-arm-y += tests/hexloader-test$(EXESUF)
+check-qtest-arm-$(CONFIG_PFLASH_CFI02) += tests/pflash-cfi02-test$(EXESUF)
check-qtest-aarch64-y = tests/numa-test$(EXESUF)
check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
@@ -773,6 +774,7 @@ tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
tests/rtc-test$(EXESUF): tests/rtc-test.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o
tests/hexloader-test$(EXESUF): tests/hexloader-test.o
+tests/pflash-cfi02$(EXESUF): tests/pflash-cfi02-test.o
tests/endianness-test$(EXESUF): tests/endianness-test.o
tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-spapr-obj-y)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
new file mode 100644
index 00000000000..40af1bb523e
--- /dev/null
+++ b/tests/pflash-cfi02-test.c
@@ -0,0 +1,225 @@
+/*
+ * QTest testcase for parallel flash with AMD command set
+ *
+ * Copyright (c) 2019 Stephen Checkoway
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+/*
+ * To test the pflash_cfi02 device, we run QEMU with the musicpal machine with
+ * a pflash drive. This enables us to test some flash configurations, but not
+ * all. In particular, we're limited to a 16-bit wide flash device.
+ */
+
+#define MP_FLASH_SIZE_MAX (32 * 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 * 0x5555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+
+#define CFI_CMD 0x98
+#define UNLOCK0_CMD 0xAA
+#define UNLOCK1_CMD 0x55
+#define AUTOSELECT_CMD 0x90
+#define RESET_CMD 0xF0
+#define PROGRAM_CMD 0xA0
+#define SECTOR_ERASE_CMD 0x30
+#define CHIP_ERASE_CMD 0x10
+#define UNLOCK_BYPASS_CMD 0x20
+#define UNLOCK_BYPASS_RESET_CMD 0x00
+
+static char image_path[] = "/tmp/qtest.XXXXXX";
+
+static inline void flash_write(uint64_t byte_addr, uint16_t data)
+{
+ qtest_writew(global_qtest, BASE_ADDR + byte_addr, data);
+}
+
+static inline uint16_t flash_read(uint64_t byte_addr)
+{
+ return qtest_readw(global_qtest, BASE_ADDR + byte_addr);
+}
+
+static void unlock(void)
+{
+ flash_write(UNLOCK0_ADDR, UNLOCK0_CMD);
+ flash_write(UNLOCK1_ADDR, UNLOCK1_CMD);
+}
+
+static void reset(void)
+{
+ 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_cmpint(flash_read(byte_addr), ==, flash_read(byte_addr));
+ }
+}
+
+static void bypass_program(uint64_t byte_addr, uint16_t data)
+{
+ flash_write(UNLOCK0_ADDR, PROGRAM_CMD);
+ flash_write(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);
+}
+
+static void program(uint64_t byte_addr, uint16_t data)
+{
+ unlock();
+ bypass_program(byte_addr, data);
+}
+
+static void chip_erase(void)
+{
+ unlock();
+ flash_write(UNLOCK0_ADDR, 0x80);
+ unlock();
+ flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD);
+}
+
+static void test_flash(void)
+{
+ global_qtest = qtest_initf("-M musicpal,accel=qtest "
+ "-drive if=pflash,file=%s,format=raw,copy-on-read",
+ image_path);
+ /* Check the IDs. */
+ unlock();
+ flash_write(UNLOCK0_ADDR, AUTOSELECT_CMD);
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF);
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D);
+ reset();
+
+ /* Check the erase blocks. */
+ flash_write(CFI_ADDR, CFI_CMD);
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x10), ==, 'Q');
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x11), ==, 'R');
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x12), ==, 'Y');
+ /* Num erase regions. */
+ g_assert_cmpint(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();
+
+ /* Erase and program sector. */
+ for (uint32_t i = 0; i < nb_sectors; ++i) {
+ uint64_t byte_addr = i * sector_len;
+ sector_erase(byte_addr);
+ /* Read toggle. */
+ uint16_t status0 = flash_read(byte_addr);
+ /* DQ7 is 0 during an erase. */
+ g_assert_cmpint(status0 & 0x80, ==, 0);
+ uint16_t status1 = flash_read(byte_addr);
+ /* DQ6 toggles during an erase. */
+ g_assert_cmpint(status0 & 0x40, !=, status1 & 0x40);
+ /* Wait for erase to complete. */
+ clock_step_next();
+ /* Ensure DQ6 has stopped toggling. */
+ g_assert_cmpint(flash_read(byte_addr), ==, flash_read(byte_addr));
+ /* Now the data should be valid. */
+ g_assert_cmpint(flash_read(byte_addr), ==, 0xFFFF);
+
+ /* Program a bit pattern. */
+ program(byte_addr, 0x5555);
+ g_assert_cmpint(flash_read(byte_addr), ==, 0x5555);
+ program(byte_addr, 0xAA55);
+ g_assert_cmpint(flash_read(byte_addr), ==, 0x0055);
+ }
+
+ /* Erase the chip. */
+ chip_erase();
+ /* Read toggle. */
+ uint16_t status0 = flash_read(0);
+ /* DQ7 is 0 during an erase. */
+ g_assert_cmpint(status0 & 0x80, ==, 0);
+ uint16_t status1 = flash_read(0);
+ /* DQ6 toggles during an erase. */
+ g_assert_cmpint(status0 & 0x40, !=, status1 & 0x40);
+ /* Wait for erase to complete. */
+ clock_step_next();
+ /* Ensure DQ6 has stopped toggling. */
+ g_assert_cmpint(flash_read(0), ==, flash_read(0));
+ /* Now the data should be valid. */
+ g_assert_cmpint(flash_read(0), ==, 0xFFFF);
+
+ /* Unlock bypass */
+ unlock();
+ flash_write(UNLOCK0_ADDR, UNLOCK_BYPASS_CMD);
+ bypass_program(0, 0x0123);
+ bypass_program(2, 0x4567);
+ bypass_program(4, 0x89AB);
+ /*
+ * 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_cmpint(flash_read(0), ==, 0x0123);
+ g_assert_cmpint(flash_read(2), ==, 0x4567);
+ g_assert_cmpint(flash_read(4), ==, 0x89AB);
+ g_assert_cmpint(flash_read(6), ==, 0xCDEF);
+ g_assert_cmpint(flash_read(8), ==, 0xFFFF);
+
+ qtest_quit(global_qtest);
+}
+
+static void cleanup(void *opaque)
+{
+ unlink(image_path);
+}
+
+int main(int argc, char **argv)
+{
+ int fd = mkstemp(image_path);
+ if (fd == -1) {
+ g_printerr("Failed to create temporary file %s: %s\n", image_path,
+ strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+ if (ftruncate(fd, 8 * 1024 * 1024) < 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));
+ exit(EXIT_FAILURE);
+ }
+ close(fd);
+
+ qtest_add_abrt_handler(cleanup, NULL);
+ g_test_init(&argc, &argv, NULL);
+ qtest_add_func("pflash-cfi02", test_flash);
+ int result = g_test_run();
+ cleanup(NULL);
+ return result;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 01/13] tests/pflash-cfi02: Add test for supported CFI commands
2019-05-05 22:15 ` [Qemu-devel] [PATCH 01/13] tests/pflash-cfi02: Add test for supported CFI commands Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
From: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Test the AMD command set for parallel flash chips. This test uses an
ARM musicpal board with a pflash drive to test the following list of
currently-supported commands.
- Autoselect
- CFI
- Sector erase
- Chip erase
- Program
- Unlock bypass
- Reset
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-2-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: reworded the patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/Makefile.include | 2 +
tests/pflash-cfi02-test.c | 225 ++++++++++++++++++++++++++++++++++++++
2 files changed, 227 insertions(+)
create mode 100644 tests/pflash-cfi02-test.c
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7c8b9c84b24..b84a308d9a1 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -263,6 +263,7 @@ check-qtest-arm-y += tests/m25p80-test$(EXESUF)
check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
check-qtest-arm-y += tests/hexloader-test$(EXESUF)
+check-qtest-arm-$(CONFIG_PFLASH_CFI02) += tests/pflash-cfi02-test$(EXESUF)
check-qtest-aarch64-y = tests/numa-test$(EXESUF)
check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
@@ -773,6 +774,7 @@ tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
tests/rtc-test$(EXESUF): tests/rtc-test.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o
tests/hexloader-test$(EXESUF): tests/hexloader-test.o
+tests/pflash-cfi02$(EXESUF): tests/pflash-cfi02-test.o
tests/endianness-test$(EXESUF): tests/endianness-test.o
tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-spapr-obj-y)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
new file mode 100644
index 00000000000..40af1bb523e
--- /dev/null
+++ b/tests/pflash-cfi02-test.c
@@ -0,0 +1,225 @@
+/*
+ * QTest testcase for parallel flash with AMD command set
+ *
+ * Copyright (c) 2019 Stephen Checkoway
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+/*
+ * To test the pflash_cfi02 device, we run QEMU with the musicpal machine with
+ * a pflash drive. This enables us to test some flash configurations, but not
+ * all. In particular, we're limited to a 16-bit wide flash device.
+ */
+
+#define MP_FLASH_SIZE_MAX (32 * 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 * 0x5555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+
+#define CFI_CMD 0x98
+#define UNLOCK0_CMD 0xAA
+#define UNLOCK1_CMD 0x55
+#define AUTOSELECT_CMD 0x90
+#define RESET_CMD 0xF0
+#define PROGRAM_CMD 0xA0
+#define SECTOR_ERASE_CMD 0x30
+#define CHIP_ERASE_CMD 0x10
+#define UNLOCK_BYPASS_CMD 0x20
+#define UNLOCK_BYPASS_RESET_CMD 0x00
+
+static char image_path[] = "/tmp/qtest.XXXXXX";
+
+static inline void flash_write(uint64_t byte_addr, uint16_t data)
+{
+ qtest_writew(global_qtest, BASE_ADDR + byte_addr, data);
+}
+
+static inline uint16_t flash_read(uint64_t byte_addr)
+{
+ return qtest_readw(global_qtest, BASE_ADDR + byte_addr);
+}
+
+static void unlock(void)
+{
+ flash_write(UNLOCK0_ADDR, UNLOCK0_CMD);
+ flash_write(UNLOCK1_ADDR, UNLOCK1_CMD);
+}
+
+static void reset(void)
+{
+ 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_cmpint(flash_read(byte_addr), ==, flash_read(byte_addr));
+ }
+}
+
+static void bypass_program(uint64_t byte_addr, uint16_t data)
+{
+ flash_write(UNLOCK0_ADDR, PROGRAM_CMD);
+ flash_write(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);
+}
+
+static void program(uint64_t byte_addr, uint16_t data)
+{
+ unlock();
+ bypass_program(byte_addr, data);
+}
+
+static void chip_erase(void)
+{
+ unlock();
+ flash_write(UNLOCK0_ADDR, 0x80);
+ unlock();
+ flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD);
+}
+
+static void test_flash(void)
+{
+ global_qtest = qtest_initf("-M musicpal,accel=qtest "
+ "-drive if=pflash,file=%s,format=raw,copy-on-read",
+ image_path);
+ /* Check the IDs. */
+ unlock();
+ flash_write(UNLOCK0_ADDR, AUTOSELECT_CMD);
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF);
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D);
+ reset();
+
+ /* Check the erase blocks. */
+ flash_write(CFI_ADDR, CFI_CMD);
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x10), ==, 'Q');
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x11), ==, 'R');
+ g_assert_cmpint(flash_read(FLASH_WIDTH * 0x12), ==, 'Y');
+ /* Num erase regions. */
+ g_assert_cmpint(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();
+
+ /* Erase and program sector. */
+ for (uint32_t i = 0; i < nb_sectors; ++i) {
+ uint64_t byte_addr = i * sector_len;
+ sector_erase(byte_addr);
+ /* Read toggle. */
+ uint16_t status0 = flash_read(byte_addr);
+ /* DQ7 is 0 during an erase. */
+ g_assert_cmpint(status0 & 0x80, ==, 0);
+ uint16_t status1 = flash_read(byte_addr);
+ /* DQ6 toggles during an erase. */
+ g_assert_cmpint(status0 & 0x40, !=, status1 & 0x40);
+ /* Wait for erase to complete. */
+ clock_step_next();
+ /* Ensure DQ6 has stopped toggling. */
+ g_assert_cmpint(flash_read(byte_addr), ==, flash_read(byte_addr));
+ /* Now the data should be valid. */
+ g_assert_cmpint(flash_read(byte_addr), ==, 0xFFFF);
+
+ /* Program a bit pattern. */
+ program(byte_addr, 0x5555);
+ g_assert_cmpint(flash_read(byte_addr), ==, 0x5555);
+ program(byte_addr, 0xAA55);
+ g_assert_cmpint(flash_read(byte_addr), ==, 0x0055);
+ }
+
+ /* Erase the chip. */
+ chip_erase();
+ /* Read toggle. */
+ uint16_t status0 = flash_read(0);
+ /* DQ7 is 0 during an erase. */
+ g_assert_cmpint(status0 & 0x80, ==, 0);
+ uint16_t status1 = flash_read(0);
+ /* DQ6 toggles during an erase. */
+ g_assert_cmpint(status0 & 0x40, !=, status1 & 0x40);
+ /* Wait for erase to complete. */
+ clock_step_next();
+ /* Ensure DQ6 has stopped toggling. */
+ g_assert_cmpint(flash_read(0), ==, flash_read(0));
+ /* Now the data should be valid. */
+ g_assert_cmpint(flash_read(0), ==, 0xFFFF);
+
+ /* Unlock bypass */
+ unlock();
+ flash_write(UNLOCK0_ADDR, UNLOCK_BYPASS_CMD);
+ bypass_program(0, 0x0123);
+ bypass_program(2, 0x4567);
+ bypass_program(4, 0x89AB);
+ /*
+ * 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_cmpint(flash_read(0), ==, 0x0123);
+ g_assert_cmpint(flash_read(2), ==, 0x4567);
+ g_assert_cmpint(flash_read(4), ==, 0x89AB);
+ g_assert_cmpint(flash_read(6), ==, 0xCDEF);
+ g_assert_cmpint(flash_read(8), ==, 0xFFFF);
+
+ qtest_quit(global_qtest);
+}
+
+static void cleanup(void *opaque)
+{
+ unlink(image_path);
+}
+
+int main(int argc, char **argv)
+{
+ int fd = mkstemp(image_path);
+ if (fd == -1) {
+ g_printerr("Failed to create temporary file %s: %s\n", image_path,
+ strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+ if (ftruncate(fd, 8 * 1024 * 1024) < 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));
+ exit(EXIT_FAILURE);
+ }
+ close(fd);
+
+ qtest_add_abrt_handler(cleanup, NULL);
+ g_test_init(&argc, &argv, NULL);
+ qtest_add_func("pflash-cfi02", test_flash);
+ int result = g_test_run();
+ cleanup(NULL);
+ return result;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 02/13] tests/pflash-cfi02: Use the GLib API
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 01/13] tests/pflash-cfi02: Add test for supported CFI commands Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 03/13] tests/pflash-cfi02: Use IEC binary prefixes for size constants Philippe Mathieu-Daudé
` (11 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/pflash-cfi02-test.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 40af1bb523e..ff775618c02 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -35,8 +35,6 @@
#define UNLOCK_BYPASS_CMD 0x20
#define UNLOCK_BYPASS_RESET_CMD 0x00
-static char image_path[] = "/tmp/qtest.XXXXXX";
-
static inline void flash_write(uint64_t byte_addr, uint16_t data)
{
qtest_writew(global_qtest, BASE_ADDR + byte_addr, data);
@@ -103,8 +101,9 @@ static void chip_erase(void)
flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD);
}
-static void test_flash(void)
+static void test_flash(const void *opaque)
{
+ const char *image_path = opaque;
global_qtest = qtest_initf("-M musicpal,accel=qtest "
"-drive if=pflash,file=%s,format=raw,copy-on-read",
image_path);
@@ -195,31 +194,30 @@ static void test_flash(void)
static void cleanup(void *opaque)
{
+ char *image_path = opaque;
unlink(image_path);
+ g_free(image_path);
}
int main(int argc, char **argv)
{
- int fd = mkstemp(image_path);
- if (fd == -1) {
- g_printerr("Failed to create temporary file %s: %s\n", image_path,
- strerror(errno));
- exit(EXIT_FAILURE);
- }
+ GError *error = NULL;
+ char *image_path;
+ int fd = g_file_open_tmp("pflash-cfi02-XXXXXX.raw", &image_path, &error);
+ g_assert_no_error(error);
if (ftruncate(fd, 8 * 1024 * 1024) < 0) {
- int error_code = errno;
+ g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
+ strerror(errno));
close(fd);
unlink(image_path);
- g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
- strerror(error_code));
exit(EXIT_FAILURE);
}
close(fd);
- qtest_add_abrt_handler(cleanup, NULL);
+ qtest_add_abrt_handler(cleanup, image_path);
g_test_init(&argc, &argv, NULL);
- qtest_add_func("pflash-cfi02", test_flash);
+ qtest_add_data_func("pflash-cfi02", image_path, test_flash);
int result = g_test_run();
- cleanup(NULL);
+ cleanup(image_path);
return result;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 02/13] tests/pflash-cfi02: Use the GLib API
2019-05-05 22:15 ` [Qemu-devel] [PATCH 02/13] tests/pflash-cfi02: Use the GLib API Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/pflash-cfi02-test.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 40af1bb523e..ff775618c02 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -35,8 +35,6 @@
#define UNLOCK_BYPASS_CMD 0x20
#define UNLOCK_BYPASS_RESET_CMD 0x00
-static char image_path[] = "/tmp/qtest.XXXXXX";
-
static inline void flash_write(uint64_t byte_addr, uint16_t data)
{
qtest_writew(global_qtest, BASE_ADDR + byte_addr, data);
@@ -103,8 +101,9 @@ static void chip_erase(void)
flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD);
}
-static void test_flash(void)
+static void test_flash(const void *opaque)
{
+ const char *image_path = opaque;
global_qtest = qtest_initf("-M musicpal,accel=qtest "
"-drive if=pflash,file=%s,format=raw,copy-on-read",
image_path);
@@ -195,31 +194,30 @@ static void test_flash(void)
static void cleanup(void *opaque)
{
+ char *image_path = opaque;
unlink(image_path);
+ g_free(image_path);
}
int main(int argc, char **argv)
{
- int fd = mkstemp(image_path);
- if (fd == -1) {
- g_printerr("Failed to create temporary file %s: %s\n", image_path,
- strerror(errno));
- exit(EXIT_FAILURE);
- }
+ GError *error = NULL;
+ char *image_path;
+ int fd = g_file_open_tmp("pflash-cfi02-XXXXXX.raw", &image_path, &error);
+ g_assert_no_error(error);
if (ftruncate(fd, 8 * 1024 * 1024) < 0) {
- int error_code = errno;
+ g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
+ strerror(errno));
close(fd);
unlink(image_path);
- g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
- strerror(error_code));
exit(EXIT_FAILURE);
}
close(fd);
- qtest_add_abrt_handler(cleanup, NULL);
+ qtest_add_abrt_handler(cleanup, image_path);
g_test_init(&argc, &argv, NULL);
- qtest_add_func("pflash-cfi02", test_flash);
+ qtest_add_data_func("pflash-cfi02", image_path, test_flash);
int result = g_test_run();
- cleanup(NULL);
+ cleanup(image_path);
return result;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 03/13] tests/pflash-cfi02: Use IEC binary prefixes for size constants
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 02/13] tests/pflash-cfi02: Use the GLib API Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 04/13] hw/block/pflash_cfi02: Fix debug format string Philippe Mathieu-Daudé
` (10 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
Using IEC binary prefixes in order to make the code more readable.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/pflash-cfi02-test.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index ff775618c02..3c37465499a 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -8,6 +8,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "libqtest.h"
/*
@@ -16,7 +17,7 @@
* all. In particular, we're limited to a 16-bit wide flash device.
*/
-#define MP_FLASH_SIZE_MAX (32 * 1024 * 1024)
+#define MP_FLASH_SIZE_MAX (32 * MiB)
#define BASE_ADDR (0x100000000ULL - MP_FLASH_SIZE_MAX)
#define FLASH_WIDTH 2
@@ -205,7 +206,7 @@ int main(int argc, char **argv)
char *image_path;
int fd = g_file_open_tmp("pflash-cfi02-XXXXXX.raw", &image_path, &error);
g_assert_no_error(error);
- if (ftruncate(fd, 8 * 1024 * 1024) < 0) {
+ if (ftruncate(fd, 8 * MiB) < 0) {
g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
strerror(errno));
close(fd);
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 03/13] tests/pflash-cfi02: Use IEC binary prefixes for size constants
2019-05-05 22:15 ` [Qemu-devel] [PATCH 03/13] tests/pflash-cfi02: Use IEC binary prefixes for size constants Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
Using IEC binary prefixes in order to make the code more readable.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/pflash-cfi02-test.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index ff775618c02..3c37465499a 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -8,6 +8,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "libqtest.h"
/*
@@ -16,7 +17,7 @@
* all. In particular, we're limited to a 16-bit wide flash device.
*/
-#define MP_FLASH_SIZE_MAX (32 * 1024 * 1024)
+#define MP_FLASH_SIZE_MAX (32 * MiB)
#define BASE_ADDR (0x100000000ULL - MP_FLASH_SIZE_MAX)
#define FLASH_WIDTH 2
@@ -205,7 +206,7 @@ int main(int argc, char **argv)
char *image_path;
int fd = g_file_open_tmp("pflash-cfi02-XXXXXX.raw", &image_path, &error);
g_assert_no_error(error);
- if (ftruncate(fd, 8 * 1024 * 1024) < 0) {
+ if (ftruncate(fd, 8 * MiB) < 0) {
g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
strerror(errno));
close(fd);
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 04/13] hw/block/pflash_cfi02: Fix debug format string
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 03/13] tests/pflash-cfi02: Use IEC binary prefixes for size constants Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 05/13] hw/block/pflash_cfi02: Add an enum to define the write cycles Philippe Mathieu-Daudé
` (9 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
Always compile the debug code to prevent format string to bitrot.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch, use PRIx32]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f2c6201f813..f88e09cebaa 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -46,15 +46,13 @@
#include "hw/sysbus.h"
#include "trace.h"
-//#define PFLASH_DEBUG
-#ifdef PFLASH_DEBUG
+#define PFLASH_DEBUG false
#define DPRINTF(fmt, ...) \
do { \
- fprintf(stderr, "PFLASH: " fmt , ## __VA_ARGS__); \
+ if (PFLASH_DEBUG) { \
+ fprintf(stderr, "PFLASH: " fmt, ## __VA_ARGS__); \
+ } \
} while (0)
-#else
-#define DPRINTF(fmt, ...) do { } while (0)
-#endif
#define PFLASH_LAZY_ROMD_THRESHOLD 42
@@ -220,14 +218,14 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
default:
goto flash_read;
}
- DPRINTF("%s: ID " TARGET_FMT_plx " %x\n", __func__, boff, ret);
+ DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, ret);
break;
case 0xA0:
case 0x10:
case 0x30:
/* Status register read */
ret = pfl->status;
- DPRINTF("%s: status %x\n", __func__, ret);
+ DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
/* Toggle bit 6 */
pfl->status ^= 0x40;
break;
@@ -277,7 +275,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
trace_pflash_write(offset, value, width, pfl->wcycle);
offset &= pfl->chip_len - 1;
- DPRINTF("%s: offset " TARGET_FMT_plx " %08x %d\n", __func__,
+ DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx32 " %d\n", __func__,
offset, value, width);
boff = offset & (pfl->sector_len - 1);
if (pfl->width == 2)
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 04/13] hw/block/pflash_cfi02: Fix debug format string
2019-05-05 22:15 ` [Qemu-devel] [PATCH 04/13] hw/block/pflash_cfi02: Fix debug format string Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
Always compile the debug code to prevent format string to bitrot.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch, use PRIx32]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f2c6201f813..f88e09cebaa 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -46,15 +46,13 @@
#include "hw/sysbus.h"
#include "trace.h"
-//#define PFLASH_DEBUG
-#ifdef PFLASH_DEBUG
+#define PFLASH_DEBUG false
#define DPRINTF(fmt, ...) \
do { \
- fprintf(stderr, "PFLASH: " fmt , ## __VA_ARGS__); \
+ if (PFLASH_DEBUG) { \
+ fprintf(stderr, "PFLASH: " fmt, ## __VA_ARGS__); \
+ } \
} while (0)
-#else
-#define DPRINTF(fmt, ...) do { } while (0)
-#endif
#define PFLASH_LAZY_ROMD_THRESHOLD 42
@@ -220,14 +218,14 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
default:
goto flash_read;
}
- DPRINTF("%s: ID " TARGET_FMT_plx " %x\n", __func__, boff, ret);
+ DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, ret);
break;
case 0xA0:
case 0x10:
case 0x30:
/* Status register read */
ret = pfl->status;
- DPRINTF("%s: status %x\n", __func__, ret);
+ DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
/* Toggle bit 6 */
pfl->status ^= 0x40;
break;
@@ -277,7 +275,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
trace_pflash_write(offset, value, width, pfl->wcycle);
offset &= pfl->chip_len - 1;
- DPRINTF("%s: offset " TARGET_FMT_plx " %08x %d\n", __func__,
+ DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx32 " %d\n", __func__,
offset, value, width);
boff = offset & (pfl->sector_len - 1);
if (pfl->width == 2)
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 05/13] hw/block/pflash_cfi02: Add an enum to define the write cycles
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 04/13] hw/block/pflash_cfi02: Fix debug format string Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 06/13] hw/block/pflash_cfi02: Add helpers to manipulate the status bits Philippe Mathieu-Daudé
` (8 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
No change in functionality is intended with this commit.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f88e09cebaa..b27796d74d2 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -56,6 +56,11 @@ do { \
#define PFLASH_LAZY_ROMD_THRESHOLD 42
+/* Special write cycles for CFI queries. */
+enum {
+ WCYCLE_CFI = 7,
+};
+
struct PFlashCFI02 {
/*< private >*/
SysBusDevice parent_obj;
@@ -293,7 +298,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
if (boff == 0x55 && cmd == 0x98) {
enter_CFI_mode:
/* Enter CFI query mode */
- pfl->wcycle = 7;
+ pfl->wcycle = WCYCLE_CFI;
pfl->cmd = 0x98;
return;
}
@@ -465,7 +470,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
goto reset_flash;
}
break;
- case 7: /* Special value for CFI queries */
+ case WCYCLE_CFI: /* Special value for CFI queries */
DPRINTF("%s: invalid write in CFI query mode\n", __func__);
goto reset_flash;
default:
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 05/13] hw/block/pflash_cfi02: Add an enum to define the write cycles
2019-05-05 22:15 ` [Qemu-devel] [PATCH 05/13] hw/block/pflash_cfi02: Add an enum to define the write cycles Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
No change in functionality is intended with this commit.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f88e09cebaa..b27796d74d2 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -56,6 +56,11 @@ do { \
#define PFLASH_LAZY_ROMD_THRESHOLD 42
+/* Special write cycles for CFI queries. */
+enum {
+ WCYCLE_CFI = 7,
+};
+
struct PFlashCFI02 {
/*< private >*/
SysBusDevice parent_obj;
@@ -293,7 +298,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
if (boff == 0x55 && cmd == 0x98) {
enter_CFI_mode:
/* Enter CFI query mode */
- pfl->wcycle = 7;
+ pfl->wcycle = WCYCLE_CFI;
pfl->cmd = 0x98;
return;
}
@@ -465,7 +470,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
goto reset_flash;
}
break;
- case 7: /* Special value for CFI queries */
+ case WCYCLE_CFI: /* Special value for CFI queries */
DPRINTF("%s: invalid write in CFI query mode\n", __func__);
goto reset_flash;
default:
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 06/13] hw/block/pflash_cfi02: Add helpers to manipulate the status bits
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 05/13] hw/block/pflash_cfi02: Add an enum to define the write cycles Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 07/13] hw/block/pflash_cfi02: Simplify a statement using fall through Philippe Mathieu-Daudé
` (7 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
Pull out all of the code to modify the status into simple helper
functions. Status handling becomes more complex once multiple
chips are interleaved to produce a single device.
No change in functionality is intended with this commit.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index b27796d74d2..9673eee969f 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -100,6 +100,31 @@ struct PFlashCFI02 {
void *storage;
};
+/*
+ * Toggle status bit DQ7.
+ */
+static inline void toggle_dq7(PFlashCFI02 *pfl)
+{
+ pfl->status ^= 0x80;
+}
+
+/*
+ * Set status bit DQ7 to bit 7 of value.
+ */
+static inline void set_dq7(PFlashCFI02 *pfl, uint8_t value)
+{
+ pfl->status &= 0x7F;
+ pfl->status |= value & 0x80;
+}
+
+/*
+ * Toggle status bit DQ6.
+ */
+static inline void toggle_dq6(PFlashCFI02 *pfl)
+{
+ pfl->status ^= 0x40;
+}
+
/*
* Set up replicated mappings of the same region.
*/
@@ -129,7 +154,7 @@ static void pflash_timer (void *opaque)
trace_pflash_timer_expired(pfl->cmd);
/* Reset flash */
- pfl->status ^= 0x80;
+ toggle_dq7(pfl);
if (pfl->bypass) {
pfl->wcycle = 2;
} else {
@@ -232,7 +257,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
ret = pfl->status;
DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
/* Toggle bit 6 */
- pfl->status ^= 0x40;
+ toggle_dq6(pfl);
break;
case 0x98:
/* CFI query mode */
@@ -381,7 +406,11 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
break;
}
}
- pfl->status = 0x00 | ~(value & 0x80);
+ /*
+ * While programming, status bit DQ7 should hold the opposite
+ * value from how it was programmed.
+ */
+ set_dq7(pfl, ~value);
/* Let's pretend write is immediate */
if (pfl->bypass)
goto do_bypass;
@@ -429,7 +458,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
memset(pfl->storage, 0xFF, pfl->chip_len);
pflash_update(pfl, 0, pfl->chip_len);
}
- pfl->status = 0x00;
+ set_dq7(pfl, 0x00);
/* Let's wait 5 seconds before chip erase is done */
timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
(NANOSECONDS_PER_SECOND * 5));
@@ -444,7 +473,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
memset(p + offset, 0xFF, pfl->sector_len);
pflash_update(pfl, offset, pfl->sector_len);
}
- pfl->status = 0x00;
+ set_dq7(pfl, 0x00);
/* Let's wait 1/2 second before sector erase is done */
timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
(NANOSECONDS_PER_SECOND / 2));
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 06/13] hw/block/pflash_cfi02: Add helpers to manipulate the status bits
2019-05-05 22:15 ` [Qemu-devel] [PATCH 06/13] hw/block/pflash_cfi02: Add helpers to manipulate the status bits Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
Pull out all of the code to modify the status into simple helper
functions. Status handling becomes more complex once multiple
chips are interleaved to produce a single device.
No change in functionality is intended with this commit.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index b27796d74d2..9673eee969f 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -100,6 +100,31 @@ struct PFlashCFI02 {
void *storage;
};
+/*
+ * Toggle status bit DQ7.
+ */
+static inline void toggle_dq7(PFlashCFI02 *pfl)
+{
+ pfl->status ^= 0x80;
+}
+
+/*
+ * Set status bit DQ7 to bit 7 of value.
+ */
+static inline void set_dq7(PFlashCFI02 *pfl, uint8_t value)
+{
+ pfl->status &= 0x7F;
+ pfl->status |= value & 0x80;
+}
+
+/*
+ * Toggle status bit DQ6.
+ */
+static inline void toggle_dq6(PFlashCFI02 *pfl)
+{
+ pfl->status ^= 0x40;
+}
+
/*
* Set up replicated mappings of the same region.
*/
@@ -129,7 +154,7 @@ static void pflash_timer (void *opaque)
trace_pflash_timer_expired(pfl->cmd);
/* Reset flash */
- pfl->status ^= 0x80;
+ toggle_dq7(pfl);
if (pfl->bypass) {
pfl->wcycle = 2;
} else {
@@ -232,7 +257,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
ret = pfl->status;
DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
/* Toggle bit 6 */
- pfl->status ^= 0x40;
+ toggle_dq6(pfl);
break;
case 0x98:
/* CFI query mode */
@@ -381,7 +406,11 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
break;
}
}
- pfl->status = 0x00 | ~(value & 0x80);
+ /*
+ * While programming, status bit DQ7 should hold the opposite
+ * value from how it was programmed.
+ */
+ set_dq7(pfl, ~value);
/* Let's pretend write is immediate */
if (pfl->bypass)
goto do_bypass;
@@ -429,7 +458,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
memset(pfl->storage, 0xFF, pfl->chip_len);
pflash_update(pfl, 0, pfl->chip_len);
}
- pfl->status = 0x00;
+ set_dq7(pfl, 0x00);
/* Let's wait 5 seconds before chip erase is done */
timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
(NANOSECONDS_PER_SECOND * 5));
@@ -444,7 +473,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
memset(p + offset, 0xFF, pfl->sector_len);
pflash_update(pfl, offset, pfl->sector_len);
}
- pfl->status = 0x00;
+ set_dq7(pfl, 0x00);
/* Let's wait 1/2 second before sector erase is done */
timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
(NANOSECONDS_PER_SECOND / 2));
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 07/13] hw/block/pflash_cfi02: Simplify a statement using fall through
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 06/13] hw/block/pflash_cfi02: Add helpers to manipulate the status bits Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 08/13] hw/block/pflash_cfi02: Use the ldst API in pflash_write() Philippe Mathieu-Daudé
` (6 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 9673eee969f..1e006e7bf3a 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -241,10 +241,10 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
case 0x0E:
case 0x0F:
ret = boff & 0x01 ? pfl->ident3 : pfl->ident2;
- if (ret == (uint8_t)-1) {
- goto flash_read;
+ if (ret != (uint8_t)-1) {
+ break;
}
- break;
+ /* Fall through to data read. */
default:
goto flash_read;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 07/13] hw/block/pflash_cfi02: Simplify a statement using fall through
2019-05-05 22:15 ` [Qemu-devel] [PATCH 07/13] hw/block/pflash_cfi02: Simplify a statement using fall through Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 9673eee969f..1e006e7bf3a 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -241,10 +241,10 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
case 0x0E:
case 0x0F:
ret = boff & 0x01 ? pfl->ident3 : pfl->ident2;
- if (ret == (uint8_t)-1) {
- goto flash_read;
+ if (ret != (uint8_t)-1) {
+ break;
}
- break;
+ /* Fall through to data read. */
default:
goto flash_read;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 08/13] hw/block/pflash_cfi02: Use the ldst API in pflash_write()
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 07/13] hw/block/pflash_cfi02: Simplify a statement using fall through Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 09/13] hw/block/pflash_cfi02: Use the ldst API in pflash_read() Philippe Mathieu-Daudé
` (5 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
The load/store API eases code review.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 38 ++++++++------------------------------
1 file changed, 8 insertions(+), 30 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 1e006e7bf3a..69e0086324e 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -373,38 +373,16 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
goto check_unlock0;
case 0xA0:
trace_pflash_data_write(offset, value, width, 0);
- p = pfl->storage;
if (!pfl->ro) {
- switch (width) {
- case 1:
- p[offset] &= value;
- pflash_update(pfl, offset, 1);
- break;
- case 2:
- if (be) {
- p[offset] &= value >> 8;
- p[offset + 1] &= value;
- } else {
- p[offset] &= value;
- p[offset + 1] &= value >> 8;
- }
- pflash_update(pfl, offset, 2);
- break;
- case 4:
- if (be) {
- p[offset] &= value >> 24;
- p[offset + 1] &= value >> 16;
- p[offset + 2] &= value >> 8;
- p[offset + 3] &= value;
- } else {
- p[offset] &= value;
- p[offset + 1] &= value >> 8;
- p[offset + 2] &= value >> 16;
- p[offset + 3] &= value >> 24;
- }
- pflash_update(pfl, offset, 4);
- break;
+ p = (uint8_t *)pfl->storage + offset;
+ if (pfl->be) {
+ uint64_t current = ldn_be_p(p, width);
+ stn_be_p(p, width, current & value);
+ } else {
+ uint64_t current = ldn_le_p(p, width);
+ stn_le_p(p, width, current & value);
}
+ pflash_update(pfl, offset, width);
}
/*
* While programming, status bit DQ7 should hold the opposite
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 08/13] hw/block/pflash_cfi02: Use the ldst API in pflash_write()
2019-05-05 22:15 ` [Qemu-devel] [PATCH 08/13] hw/block/pflash_cfi02: Use the ldst API in pflash_write() Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
The load/store API eases code review.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 38 ++++++++------------------------------
1 file changed, 8 insertions(+), 30 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 1e006e7bf3a..69e0086324e 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -373,38 +373,16 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
goto check_unlock0;
case 0xA0:
trace_pflash_data_write(offset, value, width, 0);
- p = pfl->storage;
if (!pfl->ro) {
- switch (width) {
- case 1:
- p[offset] &= value;
- pflash_update(pfl, offset, 1);
- break;
- case 2:
- if (be) {
- p[offset] &= value >> 8;
- p[offset + 1] &= value;
- } else {
- p[offset] &= value;
- p[offset + 1] &= value >> 8;
- }
- pflash_update(pfl, offset, 2);
- break;
- case 4:
- if (be) {
- p[offset] &= value >> 24;
- p[offset + 1] &= value >> 16;
- p[offset + 2] &= value >> 8;
- p[offset + 3] &= value;
- } else {
- p[offset] &= value;
- p[offset + 1] &= value >> 8;
- p[offset + 2] &= value >> 16;
- p[offset + 3] &= value >> 24;
- }
- pflash_update(pfl, offset, 4);
- break;
+ p = (uint8_t *)pfl->storage + offset;
+ if (pfl->be) {
+ uint64_t current = ldn_be_p(p, width);
+ stn_be_p(p, width, current & value);
+ } else {
+ uint64_t current = ldn_le_p(p, width);
+ stn_le_p(p, width, current & value);
}
+ pflash_update(pfl, offset, width);
}
/*
* While programming, status bit DQ7 should hold the opposite
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 09/13] hw/block/pflash_cfi02: Use the ldst API in pflash_read()
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 08/13] hw/block/pflash_cfi02: Use the ldst API in pflash_write() Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 10/13] hw/block/pflash_cfi02: Extract the pflash_data_read() function Philippe Mathieu-Daudé
` (4 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
The load/store API eases code review.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 69e0086324e..2777167af11 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -196,34 +196,20 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
case 0x00:
flash_read:
/* Flash area read */
- p = pfl->storage;
+ p = (uint8_t *)pfl->storage + offset;
+ if (pfl->be) {
+ ret = ldn_be_p(p, width);
+ } else {
+ ret = ldn_le_p(p, width);
+ }
switch (width) {
case 1:
- ret = p[offset];
trace_pflash_data_read8(offset, ret);
break;
case 2:
- if (be) {
- ret = p[offset] << 8;
- ret |= p[offset + 1];
- } else {
- ret = p[offset];
- ret |= p[offset + 1] << 8;
- }
trace_pflash_data_read16(offset, ret);
break;
case 4:
- if (be) {
- ret = p[offset] << 24;
- ret |= p[offset + 1] << 16;
- ret |= p[offset + 2] << 8;
- ret |= p[offset + 3];
- } else {
- ret = p[offset];
- ret |= p[offset + 1] << 8;
- ret |= p[offset + 2] << 16;
- ret |= p[offset + 3] << 24;
- }
trace_pflash_data_read32(offset, ret);
break;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 09/13] hw/block/pflash_cfi02: Use the ldst API in pflash_read()
2019-05-05 22:15 ` [Qemu-devel] [PATCH 09/13] hw/block/pflash_cfi02: Use the ldst API in pflash_read() Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
The load/store API eases code review.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 69e0086324e..2777167af11 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -196,34 +196,20 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
case 0x00:
flash_read:
/* Flash area read */
- p = pfl->storage;
+ p = (uint8_t *)pfl->storage + offset;
+ if (pfl->be) {
+ ret = ldn_be_p(p, width);
+ } else {
+ ret = ldn_le_p(p, width);
+ }
switch (width) {
case 1:
- ret = p[offset];
trace_pflash_data_read8(offset, ret);
break;
case 2:
- if (be) {
- ret = p[offset] << 8;
- ret |= p[offset + 1];
- } else {
- ret = p[offset];
- ret |= p[offset + 1] << 8;
- }
trace_pflash_data_read16(offset, ret);
break;
case 4:
- if (be) {
- ret = p[offset] << 24;
- ret |= p[offset + 1] << 16;
- ret |= p[offset + 2] << 8;
- ret |= p[offset + 3];
- } else {
- ret = p[offset];
- ret |= p[offset + 1] << 8;
- ret |= p[offset + 2] << 16;
- ret |= p[offset + 3] << 24;
- }
trace_pflash_data_read32(offset, ret);
break;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 10/13] hw/block/pflash_cfi02: Extract the pflash_data_read() function
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 09/13] hw/block/pflash_cfi02: Use the ldst API in pflash_read() Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 11/13] hw/block/pflash_cfi02: Unify the MemoryRegionOps Philippe Mathieu-Daudé
` (3 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
Extract the code block in a new function, remove a goto statement.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch, remove the XXX tracing comment]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 44 ++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2777167af11..adfa39f9b5e 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -164,12 +164,33 @@ static void pflash_timer (void *opaque)
pfl->cmd = 0;
}
+/*
+ * Read data from flash.
+ */
+static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr offset,
+ unsigned int width)
+{
+ uint8_t *p = (uint8_t *)pfl->storage + offset;
+ uint64_t ret = pfl->be ? ldn_be_p(p, width) : ldn_le_p(p, width);
+ switch (width) {
+ case 1:
+ trace_pflash_data_read8(offset, ret);
+ break;
+ case 2:
+ trace_pflash_data_read16(offset, ret);
+ break;
+ case 4:
+ trace_pflash_data_read32(offset, ret);
+ break;
+ }
+ return ret;
+}
+
static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
int width, int be)
{
hwaddr boff;
uint32_t ret;
- uint8_t *p;
ret = -1;
trace_pflash_read(offset, pfl->cmd, width, pfl->wcycle);
@@ -194,25 +215,8 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
case 0x80:
/* We accept reads during second unlock sequence... */
case 0x00:
- flash_read:
/* Flash area read */
- p = (uint8_t *)pfl->storage + offset;
- if (pfl->be) {
- ret = ldn_be_p(p, width);
- } else {
- ret = ldn_le_p(p, width);
- }
- switch (width) {
- case 1:
- trace_pflash_data_read8(offset, ret);
- break;
- case 2:
- trace_pflash_data_read16(offset, ret);
- break;
- case 4:
- trace_pflash_data_read32(offset, ret);
- break;
- }
+ ret = pflash_data_read(pfl, offset, width);
break;
case 0x90:
/* flash ID read */
@@ -232,7 +236,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
}
/* Fall through to data read. */
default:
- goto flash_read;
+ ret = pflash_data_read(pfl, offset, width);
}
DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, ret);
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 10/13] hw/block/pflash_cfi02: Extract the pflash_data_read() function
2019-05-05 22:15 ` [Qemu-devel] [PATCH 10/13] hw/block/pflash_cfi02: Extract the pflash_data_read() function Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
Extract the code block in a new function, remove a goto statement.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch, remove the XXX tracing comment]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 44 ++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2777167af11..adfa39f9b5e 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -164,12 +164,33 @@ static void pflash_timer (void *opaque)
pfl->cmd = 0;
}
+/*
+ * Read data from flash.
+ */
+static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr offset,
+ unsigned int width)
+{
+ uint8_t *p = (uint8_t *)pfl->storage + offset;
+ uint64_t ret = pfl->be ? ldn_be_p(p, width) : ldn_le_p(p, width);
+ switch (width) {
+ case 1:
+ trace_pflash_data_read8(offset, ret);
+ break;
+ case 2:
+ trace_pflash_data_read16(offset, ret);
+ break;
+ case 4:
+ trace_pflash_data_read32(offset, ret);
+ break;
+ }
+ return ret;
+}
+
static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
int width, int be)
{
hwaddr boff;
uint32_t ret;
- uint8_t *p;
ret = -1;
trace_pflash_read(offset, pfl->cmd, width, pfl->wcycle);
@@ -194,25 +215,8 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
case 0x80:
/* We accept reads during second unlock sequence... */
case 0x00:
- flash_read:
/* Flash area read */
- p = (uint8_t *)pfl->storage + offset;
- if (pfl->be) {
- ret = ldn_be_p(p, width);
- } else {
- ret = ldn_le_p(p, width);
- }
- switch (width) {
- case 1:
- trace_pflash_data_read8(offset, ret);
- break;
- case 2:
- trace_pflash_data_read16(offset, ret);
- break;
- case 4:
- trace_pflash_data_read32(offset, ret);
- break;
- }
+ ret = pflash_data_read(pfl, offset, width);
break;
case 0x90:
/* flash ID read */
@@ -232,7 +236,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
}
/* Fall through to data read. */
default:
- goto flash_read;
+ ret = pflash_data_read(pfl, offset, width);
}
DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, ret);
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 11/13] hw/block/pflash_cfi02: Unify the MemoryRegionOps
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 10/13] hw/block/pflash_cfi02: Extract the pflash_data_read() function Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison Philippe Mathieu-Daudé
` (2 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
The pflash_read()/pflash_write() can check the device endianess
via the pfl->be variable, so remove the 'int be' argument.
Since the big/little MemoryRegionOps are now identical, it is
pointless to declare them both. Unify them.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch to ease review]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 62 +++++++++++------------------------------
1 file changed, 16 insertions(+), 46 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index adfa39f9b5e..59daade2ee6 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -186,11 +186,11 @@ static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr offset,
return ret;
}
-static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
- int width, int be)
+static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
{
+ PFlashCFI02 *pfl = opaque;
hwaddr boff;
- uint32_t ret;
+ uint64_t ret;
ret = -1;
trace_pflash_read(offset, pfl->cmd, width, pfl->wcycle);
@@ -238,14 +238,14 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
default:
ret = pflash_data_read(pfl, offset, width);
}
- DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, ret);
+ DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx64 "\n", __func__, boff, ret);
break;
case 0xA0:
case 0x10:
case 0x30:
/* Status register read */
ret = pfl->status;
- DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
+ DPRINTF("%s: status %" PRIx64 "\n", __func__, ret);
/* Toggle bit 6 */
toggle_dq6(pfl);
break;
@@ -263,8 +263,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
}
/* update flash content on disk */
-static void pflash_update(PFlashCFI02 *pfl, int offset,
- int size)
+static void pflash_update(PFlashCFI02 *pfl, int offset, int size)
{
int offset_end;
if (pfl->blk) {
@@ -277,9 +276,10 @@ static void pflash_update(PFlashCFI02 *pfl, int offset,
}
}
-static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
- uint32_t value, int width, int be)
+static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned int width)
{
+ PFlashCFI02 *pfl = opaque;
hwaddr boff;
uint8_t *p;
uint8_t cmd;
@@ -295,7 +295,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
trace_pflash_write(offset, value, width, pfl->wcycle);
offset &= pfl->chip_len - 1;
- DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx32 " %d\n", __func__,
+ DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
offset, value, width);
boff = offset & (pfl->sector_len - 1);
if (pfl->width == 2)
@@ -492,39 +492,9 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
pfl->cmd = 0;
}
-static uint64_t pflash_be_readfn(void *opaque, hwaddr addr, unsigned size)
-{
- return pflash_read(opaque, addr, size, 1);
-}
-
-static void pflash_be_writefn(void *opaque, hwaddr addr,
- uint64_t value, unsigned size)
-{
- pflash_write(opaque, addr, value, size, 1);
-}
-
-static uint64_t pflash_le_readfn(void *opaque, hwaddr addr, unsigned size)
-{
- return pflash_read(opaque, addr, size, 0);
-}
-
-static void pflash_le_writefn(void *opaque, hwaddr addr,
- uint64_t value, unsigned size)
-{
- pflash_write(opaque, addr, value, size, 0);
-}
-
-static const MemoryRegionOps pflash_cfi02_ops_be = {
- .read = pflash_be_readfn,
- .write = pflash_be_writefn,
- .valid.min_access_size = 1,
- .valid.max_access_size = 4,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static const MemoryRegionOps pflash_cfi02_ops_le = {
- .read = pflash_le_readfn,
- .write = pflash_le_writefn,
+static const MemoryRegionOps pflash_cfi02_ops = {
+ .read = pflash_read,
+ .write = pflash_write,
.valid.min_access_size = 1,
.valid.max_access_size = 4,
.endianness = DEVICE_NATIVE_ENDIAN,
@@ -552,9 +522,9 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
chip_len = pfl->sector_len * pfl->nb_blocs;
- memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ?
- &pflash_cfi02_ops_be : &pflash_cfi02_ops_le,
- pfl, pfl->name, chip_len, &local_err);
+ memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
+ &pflash_cfi02_ops, pfl, pfl->name,
+ chip_len, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 11/13] hw/block/pflash_cfi02: Unify the MemoryRegionOps
2019-05-05 22:15 ` [Qemu-devel] [PATCH 11/13] hw/block/pflash_cfi02: Unify the MemoryRegionOps Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
The pflash_read()/pflash_write() can check the device endianess
via the pfl->be variable, so remove the 'int be' argument.
Since the big/little MemoryRegionOps are now identical, it is
pointless to declare them both. Unify them.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-3-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Extracted from bigger patch to ease review]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 62 +++++++++++------------------------------
1 file changed, 16 insertions(+), 46 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index adfa39f9b5e..59daade2ee6 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -186,11 +186,11 @@ static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr offset,
return ret;
}
-static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
- int width, int be)
+static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
{
+ PFlashCFI02 *pfl = opaque;
hwaddr boff;
- uint32_t ret;
+ uint64_t ret;
ret = -1;
trace_pflash_read(offset, pfl->cmd, width, pfl->wcycle);
@@ -238,14 +238,14 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
default:
ret = pflash_data_read(pfl, offset, width);
}
- DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, ret);
+ DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx64 "\n", __func__, boff, ret);
break;
case 0xA0:
case 0x10:
case 0x30:
/* Status register read */
ret = pfl->status;
- DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
+ DPRINTF("%s: status %" PRIx64 "\n", __func__, ret);
/* Toggle bit 6 */
toggle_dq6(pfl);
break;
@@ -263,8 +263,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
}
/* update flash content on disk */
-static void pflash_update(PFlashCFI02 *pfl, int offset,
- int size)
+static void pflash_update(PFlashCFI02 *pfl, int offset, int size)
{
int offset_end;
if (pfl->blk) {
@@ -277,9 +276,10 @@ static void pflash_update(PFlashCFI02 *pfl, int offset,
}
}
-static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
- uint32_t value, int width, int be)
+static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned int width)
{
+ PFlashCFI02 *pfl = opaque;
hwaddr boff;
uint8_t *p;
uint8_t cmd;
@@ -295,7 +295,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
trace_pflash_write(offset, value, width, pfl->wcycle);
offset &= pfl->chip_len - 1;
- DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx32 " %d\n", __func__,
+ DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
offset, value, width);
boff = offset & (pfl->sector_len - 1);
if (pfl->width == 2)
@@ -492,39 +492,9 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
pfl->cmd = 0;
}
-static uint64_t pflash_be_readfn(void *opaque, hwaddr addr, unsigned size)
-{
- return pflash_read(opaque, addr, size, 1);
-}
-
-static void pflash_be_writefn(void *opaque, hwaddr addr,
- uint64_t value, unsigned size)
-{
- pflash_write(opaque, addr, value, size, 1);
-}
-
-static uint64_t pflash_le_readfn(void *opaque, hwaddr addr, unsigned size)
-{
- return pflash_read(opaque, addr, size, 0);
-}
-
-static void pflash_le_writefn(void *opaque, hwaddr addr,
- uint64_t value, unsigned size)
-{
- pflash_write(opaque, addr, value, size, 0);
-}
-
-static const MemoryRegionOps pflash_cfi02_ops_be = {
- .read = pflash_be_readfn,
- .write = pflash_be_writefn,
- .valid.min_access_size = 1,
- .valid.max_access_size = 4,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static const MemoryRegionOps pflash_cfi02_ops_le = {
- .read = pflash_le_readfn,
- .write = pflash_le_writefn,
+static const MemoryRegionOps pflash_cfi02_ops = {
+ .read = pflash_read,
+ .write = pflash_write,
.valid.min_access_size = 1,
.valid.max_access_size = 4,
.endianness = DEVICE_NATIVE_ENDIAN,
@@ -552,9 +522,9 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
chip_len = pfl->sector_len * pfl->nb_blocs;
- memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ?
- &pflash_cfi02_ops_be : &pflash_cfi02_ops_le,
- pfl, pfl->name, chip_len, &local_err);
+ memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
+ &pflash_cfi02_ops, pfl, pfl->name,
+ chip_len, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 11/13] hw/block/pflash_cfi02: Unify the MemoryRegionOps Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 13/13] hw/block/pflash_cfi02: Use the chip erase time specified in the CFI table Philippe Mathieu-Daudé
2019-06-26 20:33 ` [Qemu-devel] [Qemu-block] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes John Snow
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
From: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Most AMD commands only examine 11 bits of the address. This masks the
addresses used in the comparison to 11 bits. The exceptions are word or
sector addresses which use offset directly rather than the shifted
offset, boff.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20190426162624.55977-4-stephen.checkoway@oberlin.edu>
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 8 +++++++-
tests/pflash-cfi02-test.c | 12 ++++++++++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 59daade2ee6..4c17dbf99f4 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -297,11 +297,13 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
offset, value, width);
- boff = offset & (pfl->sector_len - 1);
+ boff = offset;
if (pfl->width == 2)
boff = boff >> 1;
else if (pfl->width == 4)
boff = boff >> 2;
+ /* Only the least-significant 11 bits are used in most cases. */
+ boff &= 0x7FF;
switch (pfl->wcycle) {
case 0:
/* Set the device in I/O access mode if required */
@@ -520,6 +522,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
return;
}
+ /* Only 11 bits are used in the comparison. */
+ pfl->unlock_addr0 &= 0x7FF;
+ pfl->unlock_addr1 &= 0x7FF;
+
chip_len = pfl->sector_len * pfl->nb_blocs;
memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 3c37465499a..1e5d9124b71 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -22,8 +22,8 @@
#define FLASH_WIDTH 2
#define CFI_ADDR (FLASH_WIDTH * 0x55)
-#define UNLOCK0_ADDR (FLASH_WIDTH * 0x5555)
-#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA)
#define CFI_CMD 0x98
#define UNLOCK0_CMD 0xAA
@@ -190,6 +190,14 @@ static void test_flash(const void *opaque)
g_assert_cmpint(flash_read(6), ==, 0xCDEF);
g_assert_cmpint(flash_read(8), ==, 0xFFFF);
+ /* 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();
+
qtest_quit(global_qtest);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison
2019-05-05 22:15 ` [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
From: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Most AMD commands only examine 11 bits of the address. This masks the
addresses used in the comparison to 11 bits. The exceptions are word or
sector addresses which use offset directly rather than the shifted
offset, boff.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20190426162624.55977-4-stephen.checkoway@oberlin.edu>
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 8 +++++++-
tests/pflash-cfi02-test.c | 12 ++++++++++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 59daade2ee6..4c17dbf99f4 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -297,11 +297,13 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
offset, value, width);
- boff = offset & (pfl->sector_len - 1);
+ boff = offset;
if (pfl->width == 2)
boff = boff >> 1;
else if (pfl->width == 4)
boff = boff >> 2;
+ /* Only the least-significant 11 bits are used in most cases. */
+ boff &= 0x7FF;
switch (pfl->wcycle) {
case 0:
/* Set the device in I/O access mode if required */
@@ -520,6 +522,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
return;
}
+ /* Only 11 bits are used in the comparison. */
+ pfl->unlock_addr0 &= 0x7FF;
+ pfl->unlock_addr1 &= 0x7FF;
+
chip_len = pfl->sector_len * pfl->nb_blocs;
memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 3c37465499a..1e5d9124b71 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -22,8 +22,8 @@
#define FLASH_WIDTH 2
#define CFI_ADDR (FLASH_WIDTH * 0x55)
-#define UNLOCK0_ADDR (FLASH_WIDTH * 0x5555)
-#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA)
#define CFI_CMD 0x98
#define UNLOCK0_CMD 0xAA
@@ -190,6 +190,14 @@ static void test_flash(const void *opaque)
g_assert_cmpint(flash_read(6), ==, 0xCDEF);
g_assert_cmpint(flash_read(8), ==, 0xFFFF);
+ /* 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();
+
qtest_quit(global_qtest);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH 13/13] hw/block/pflash_cfi02: Use the chip erase time specified in the CFI table
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-06-26 20:33 ` [Qemu-devel] [Qemu-block] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes John Snow
14 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Markus Armbruster, Max Reitz, Thomas Huth, Laurent Vivier,
Paolo Bonzini, qemu-block, Alex Bennée, Kevin Wolf,
Peter Maydell, Philippe Mathieu-Daudé
From: Stephen Checkoway <stephen.checkoway@oberlin.edu>
When erasing the chip, use the typical time specified in the CFI table
rather than arbitrarily selecting 5 seconds.
Since the currently unconfigurable value set in the table is 12, this
means a chip erase takes 4096 ms so this isn't a big change in behavior.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-11-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 4c17dbf99f4..49cd9ed0f91 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -429,9 +429,9 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
pflash_update(pfl, 0, pfl->chip_len);
}
set_dq7(pfl, 0x00);
- /* Let's wait 5 seconds before chip erase is done */
+ /* Wait the time specified at CFI address 0x22. */
timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
- (NANOSECONDS_PER_SECOND * 5));
+ (1ULL << pfl->cfi_table[0x22]) * SCALE_MS);
break;
case 0x30:
/* Sector erase */
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Qemu-devel] [PATCH 13/13] hw/block/pflash_cfi02: Use the chip erase time specified in the CFI table
2019-05-05 22:15 ` [Qemu-devel] [PATCH 13/13] hw/block/pflash_cfi02: Use the chip erase time specified in the CFI table Philippe Mathieu-Daudé
@ 2019-05-05 22:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-05 22:15 UTC (permalink / raw)
To: qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Paolo Bonzini, Alex Bennée
From: Stephen Checkoway <stephen.checkoway@oberlin.edu>
When erasing the chip, use the typical time specified in the CFI table
rather than arbitrarily selecting 5 seconds.
Since the currently unconfigurable value set in the table is 12, this
means a chip erase takes 4096 ms so this isn't a big change in behavior.
Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Message-Id: <20190426162624.55977-11-stephen.checkoway@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi02.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 4c17dbf99f4..49cd9ed0f91 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -429,9 +429,9 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
pflash_update(pfl, 0, pfl->chip_len);
}
set_dq7(pfl, 0x00);
- /* Let's wait 5 seconds before chip erase is done */
+ /* Wait the time specified at CFI address 0x22. */
timer_mod(&pfl->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
- (NANOSECONDS_PER_SECOND * 5));
+ (1ULL << pfl->cfi_table[0x22]) * SCALE_MS);
break;
case 0x30:
/* Sector erase */
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
` (13 preceding siblings ...)
2019-05-05 22:15 ` [Qemu-devel] [PATCH 13/13] hw/block/pflash_cfi02: Use the chip erase time specified in the CFI table Philippe Mathieu-Daudé
@ 2019-06-26 20:33 ` John Snow
2019-06-26 21:06 ` Philippe Mathieu-Daudé
14 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2019-06-26 20:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Markus Armbruster, Max Reitz, Paolo Bonzini,
Alex Bennée
I don't think this series ever made it upstream, and it's now well past
30 days, so I might encourage a resend when you can if this is still
important to pursue.
--js
On 5/5/19 6:15 PM, Philippe Mathieu-Daudé wrote:
> Hi,
>
> While reviewing Stephen Checkoway's v4 "Implement missing AMD
> pflash functionality" [*] I found it hard (for me) to digest,
> so I took step by step notes. This series is the result of
> those notes.
> Regarding Stephen's series, this series only contains the
> generic code movement and trivial cleanup. The other patches
> are rather dense and I need more time to study the specs.
>
> Stephen: If you take out the patch #2 ("Use the GLib API"),
> you can rebase your series on top of this.
> I'd appreciate if you can adapt your tests to use the GLib
> functions, else I plan to do it later.
>
> Regards,
>
> Phil.
>
> [*] https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg04595.html
>
> Philippe Mathieu-Daudé (10):
> tests/pflash-cfi02: Use the GLib API
> tests/pflash-cfi02: Use IEC binary prefixes for size constants
> hw/block/pflash_cfi02: Fix debug format string
> hw/block/pflash_cfi02: Add an enum to define the write cycles
> hw/block/pflash_cfi02: Add helpers to manipulate the status bits
> hw/block/pflash_cfi02: Simplify a statement using fall through
> hw/block/pflash_cfi02: Use the ldst API in pflash_write()
> hw/block/pflash_cfi02: Use the ldst API in pflash_read()
> hw/block/pflash_cfi02: Extract the pflash_data_read() function
> hw/block/pflash_cfi02: Unify the MemoryRegionOps
>
> Stephen Checkoway (3):
> tests/pflash-cfi02: Add test for supported CFI commands
> hw/block/pflash_cfi02: Fix command address comparison
> hw/block/pflash_cfi02: Use the chip erase time specified in the CFI
> table
>
> hw/block/pflash_cfi02.c | 234 +++++++++++++++++---------------------
> tests/Makefile.include | 2 +
> tests/pflash-cfi02-test.c | 232 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 339 insertions(+), 129 deletions(-)
> create mode 100644 tests/pflash-cfi02-test.c
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes
2019-06-26 20:33 ` [Qemu-devel] [Qemu-block] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes John Snow
@ 2019-06-26 21:06 ` Philippe Mathieu-Daudé
2019-06-26 21:09 ` John Snow
0 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-26 21:06 UTC (permalink / raw)
To: John Snow, qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Markus Armbruster, Max Reitz, Paolo Bonzini,
Alex Bennée
Hi John,
On 6/26/19 10:33 PM, John Snow wrote:
> I don't think this series ever made it upstream, and it's now well past
> 30 days, so I might encourage a resend when you can if this is still
> important to pursue.
I should have sent a 'ping' indeed.
I keep rebasing because I have it in my pflash-next queue, and I added
more patches from Stephen. I am still running tests, and it is a pain to
test things that have never been tested.
Anyway, my plan is to send a "current status of pflash-next" series.
Thanks for worrying :)
Phil.
> --js
>
> On 5/5/19 6:15 PM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> While reviewing Stephen Checkoway's v4 "Implement missing AMD
>> pflash functionality" [*] I found it hard (for me) to digest,
>> so I took step by step notes. This series is the result of
>> those notes.
>> Regarding Stephen's series, this series only contains the
>> generic code movement and trivial cleanup. The other patches
>> are rather dense and I need more time to study the specs.
>>
>> Stephen: If you take out the patch #2 ("Use the GLib API"),
>> you can rebase your series on top of this.
>> I'd appreciate if you can adapt your tests to use the GLib
>> functions, else I plan to do it later.
PD: Oh, Stephen, if you read it, forget about this comment!
>> [*] https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg04595.html
>>
>> Philippe Mathieu-Daudé (10):
>> tests/pflash-cfi02: Use the GLib API
>> tests/pflash-cfi02: Use IEC binary prefixes for size constants
>> hw/block/pflash_cfi02: Fix debug format string
>> hw/block/pflash_cfi02: Add an enum to define the write cycles
>> hw/block/pflash_cfi02: Add helpers to manipulate the status bits
>> hw/block/pflash_cfi02: Simplify a statement using fall through
>> hw/block/pflash_cfi02: Use the ldst API in pflash_write()
>> hw/block/pflash_cfi02: Use the ldst API in pflash_read()
>> hw/block/pflash_cfi02: Extract the pflash_data_read() function
>> hw/block/pflash_cfi02: Unify the MemoryRegionOps
>>
>> Stephen Checkoway (3):
>> tests/pflash-cfi02: Add test for supported CFI commands
>> hw/block/pflash_cfi02: Fix command address comparison
>> hw/block/pflash_cfi02: Use the chip erase time specified in the CFI
>> table
>>
>> hw/block/pflash_cfi02.c | 234 +++++++++++++++++---------------------
>> tests/Makefile.include | 2 +
>> tests/pflash-cfi02-test.c | 232 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 339 insertions(+), 129 deletions(-)
>> create mode 100644 tests/pflash-cfi02-test.c
>>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes
2019-06-26 21:06 ` Philippe Mathieu-Daudé
@ 2019-06-26 21:09 ` John Snow
0 siblings, 0 replies; 31+ messages in thread
From: John Snow @ 2019-06-26 21:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, Stephen Checkoway
Cc: Laurent Vivier, Kevin Wolf, Thomas Huth, qemu-block,
Peter Maydell, Markus Armbruster, Max Reitz, Paolo Bonzini,
Alex Bennée
On 6/26/19 5:06 PM, Philippe Mathieu-Daudé wrote:
> Hi John,
>
> On 6/26/19 10:33 PM, John Snow wrote:
>> I don't think this series ever made it upstream, and it's now well past
>> 30 days, so I might encourage a resend when you can if this is still
>> important to pursue.
>
> I should have sent a 'ping' indeed.
> I keep rebasing because I have it in my pflash-next queue, and I added
> more patches from Stephen. I am still running tests, and it is a pain to
> test things that have never been tested.
> Anyway, my plan is to send a "current status of pflash-next" series.
>
> Thanks for worrying :)
>
> Phil.
>
Great, thanks!
^ permalink raw reply [flat|nested] 31+ messages in thread