[Qemu-devel] [PATCH-for-4.1 v6 3/5] hw/block/pflash_cfi01: Extract pflash_mode_read_array()

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

The same pattern is used when setting the flash in READ_ARRAY mode:
- Set the state machine command to READ_ARRAY
- Reset the write_cycle counter
- Reset the memory region in ROMD

Refactor the current code by extracting this pattern.
It is used three times:

- On a read access (on invalid command).

  Note this default case is not reachable by the state machine
  updates in pflash_data_write(). However we might reach this
  case migrating from a future QEMU version that would implement
  newer commands, without incrementing the migration version.
  Since we never know, we keep this default case.

  Previous to this patch, an invalid read command would not reset
  the memory region in ROMD mode, so:

  . A further read access would keep going into I/O mode, calling
  the same switch in pflash_read(). Undefined behaviour, probably
  unexpected.
  . A further write access in I/O mode. Since the default case set
  (wcycle=0, cmd=0x00), we jump to reset_flash which set the flash
  in READ_ARRAY.

  After this patch, if we get an invalid read command we directly
  set (wcycle=0, cmd=0x00) and put the device in ROMD mode.
  Further I/O access are now properly handled.

- On a write access (on command failure, error, or explicitly asked)

- When the device is initialized. Here the ROMD mode is hidden
  by the memory_region_init_rom_device() call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v6: split of the label rename to ease review
    the pattern is used 3 times (was 2 times previously)
    describe the 3rd time and reset the review tags :(
---
 hw/block/pflash_cfi01.c | 31 +++++++++++++++----------------
 hw/block/trace-events   |  1 +
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 6838e8a1ab..a28d0f8cc7 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -112,6 +112,18 @@ static const VMStateDescription vmstate_pflash = {
     }
 };
 
+static void pflash_mode_read_array(PFlashCFI01 *pfl)
+{
+    trace_pflash_mode_read_array();
+    /*
+     * The command 0x00 is not assigned by the CFI open standard,
+     * but QEMU historically uses it for the READ_ARRAY command (0xff).
+     */
+    pfl->cmd = 0x00;
+    pfl->wcycle = 0;
+    memory_region_rom_device_set_romd(&pfl->mem, true);
+}
+
 /* Perform a CFI query based on the bank width of the flash.
  * If this code is called we know we have a device_width set for
  * this flash.
@@ -276,12 +288,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
     default:
         /* This should never happen : reset state & treat it as a read */
         DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
-        pfl->wcycle = 0;
-        /*
-         * The command 0x00 is not assigned by the CFI open standard,
-         * but QEMU historically uses it for the READ_ARRAY command (0xff).
-         */
-        pfl->cmd = 0x00;
+        pflash_mode_read_array(pfl);
         /* fall through to read code */
     case 0x00: /* This model reset value for READ_ARRAY (not CFI compliant) */
         /* Flash area read */
@@ -646,10 +653,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                   "\n", __func__, offset, pfl->wcycle, pfl->cmd, value);
 
  reset_flash:
-    trace_pflash_reset();
-    memory_region_rom_device_set_romd(&pfl->mem, true);
-    pfl->wcycle = 0;
-    pfl->cmd = 0x00; /* This model reset value for READ_ARRAY (not CFI) */
+    pflash_mode_read_array(pfl);
 }
 
 
@@ -764,12 +768,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
         pfl->max_device_width = pfl->device_width;
     }
 
-    pfl->wcycle = 0;
-    /*
-     * The command 0x00 is not assigned by the CFI open standard,
-     * but QEMU historically uses it for the READ_ARRAY command (0xff).
-     */
-    pfl->cmd = 0x00;
+    pflash_mode_read_array(pfl);
     pfl->status = 0x80; /* WSM ready */
     /* Hardcoded CFI table */
     /* Standard "QRY" string */
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 13d1b21dd4..91a8a106c0 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -7,6 +7,7 @@ fdc_ioport_write(uint8_t reg, uint8_t value) "write reg 0x%02x val 0x%02x"
 # pflash_cfi02.c
 # pflash_cfi01.c
 pflash_reset(void) "reset"
+pflash_mode_read_array(void) "mode: read array"
 pflash_timer_expired(uint8_t cmd) "command 0x%02x done"
 pflash_io_read(uint64_t offset, int width, int fmt_width, uint32_t value, uint8_t cmd, uint8_t wcycle) "offset:0x%04"PRIx64" width:%d value:0x%0*x cmd:0x%02x wcycle:%u"
 pflash_io_write(uint64_t offset, int width, int fmt_width, uint32_t value, uint8_t wcycle) "offset:0x%04"PRIx64" width:%d value:0x%0*x wcycle:%u"
-- 
2.20.1