On Tue, Jul 16, 2019 at 3:16 PM Philippe Mathieu-Daudé <phi...@redhat.com> wrote: > > 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. > > Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> Alistair > --- > v6: new patch > --- > 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 a9529957f8..6838e8a1ab 100644 > --- a/hw/block/pflash_cfi01.c > +++ b/hw/block/pflash_cfi01.c > @@ -277,9 +277,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; > @@ -448,7 +452,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 */ > @@ -645,7 +649,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) */ > } > > > @@ -761,7 +765,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.20.1 > >