[PULL 2/4] hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00' - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	qemu-block@nongnu.org, "Max Reitz" <mreitz@redhat.com>
Subject: [PULL 2/4] hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00'
Date: Fri, 22 May 2020 19:45:22 +0200	[thread overview]
Message-ID: <20200522174524.29152-3-philmd@redhat.com> (raw)
In-Reply-To: <20200522174524.29152-1-philmd@redhat.com>

The command 0x00 is used by this model since its origin (commit
05ee37ebf630). In this commit the command is described with a
amusing '/* ??? */' comment, probably meaning 'FIXME'.

        switch (cmd) {
        case 0x00: /* ??? */
            ...

This comment survived 12 years because the 0x00 value is indeed
not specified by the CFI open standard (as of this commit).

The 'cmd' field is transfered during migration. To keep the
migration feature working with older QEMU version, we have to
take a lot of care with migrated field. We figured out it is
too late to remove a non-specified value from this model
(this would make migration review very complex). It is however
not too late to improve the documentation.

Add few comments to remember this is a special value related
to QEMU, and we won't find information about it on the CFI
spec.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20190716221555.11145-3-philmd@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index d67f84d655..3cd483d26a 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -278,9 +278,13 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
         /* This should never happen : reset state & treat it as a read */
         DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
         pfl->wcycle = 0;
-        pfl->cmd = 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;
         /* fall through to read code */
-    case 0x00:
+    case 0x00: /* This model reset value for READ_ARRAY (not CFI compliant) */
         /* Flash area read */
         ret = pflash_data_read(pfl, offset, width, be);
         break;
@@ -449,7 +453,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
     case 0:
         /* read mode */
         switch (cmd) {
-        case 0x00: /* ??? */
+        case 0x00: /* This model reset value for READ_ARRAY (not CFI) */
             goto reset_flash;
         case 0x10: /* Single Byte Program */
         case 0x40: /* Single Byte Program */
@@ -646,7 +650,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
     trace_pflash_reset();
     memory_region_rom_device_set_romd(&pfl->mem, true);
     pfl->wcycle = 0;
-    pfl->cmd = 0;
+    pfl->cmd = 0x00; /* This model reset value for READ_ARRAY (not CFI) */
 }
 
 
@@ -762,7 +766,11 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     }
 
     pfl->wcycle = 0;
-    pfl->cmd = 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;
     pfl->status = 0x80; /* WSM ready */
     /* Hardcoded CFI table */
     /* Standard "QRY" string */
-- 
2.21.3


