In the "Read Array Flowchart" the command has a value of 0xFF.
In the document [*] the "Read Array Flowchart", the READ_ARRAY
command has a value of 0xff.
Use the correct value in the pflash model.
There is no change of behavior in the guest, because:
- when the guest were sending 0xFF, the reset_flash label
was setting the command value as 0x00
- 0x00 was used internally for READ_ARRAY
To keep migration behaving correctly, we have to increase
the VMState version. When migrating from an older version,
we use the correct command value.
[*] "Common Flash Interface (CFI) and Command Sets"
(Intel Application Note 646)
Appendix B "Basic Command Set"
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v3: Handle migrating the 'cmd' field.
Since Laszlo stated he did not test migration [*], I'm keeping his
test tag, because the change with v2 has no impact in the tests
he ran.
Likewise I'm keeping John and Alistair tags, but I'd like an extra
review for the migration change, thanks!
[*] https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00679.html
---
hw/block/pflash_cfi01.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 9e34fd4e82..58cbef0588 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -100,7 +100,7 @@ static int pflash_post_load(void *opaque, int version_id);
static const VMStateDescription vmstate_pflash = {
.name = "pflash_cfi01",
- .version_id = 1,
+ .version_id = 2,
.minimum_version_id = 1,
.post_load = pflash_post_load,
.fields = (VMStateField[]) {
@@ -277,10 +277,9 @@ 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;
+ pfl->cmd = 0xff;
/* fall through to read code */
- case 0x00:
- /* Flash area read */
+ case 0xff: /* Read Array */
ret = pflash_data_read(pfl, offset, width, be);
break;
case 0x10: /* Single byte program */
@@ -448,8 +447,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
case 0:
/* read mode */
switch (cmd) {
- case 0x00: /* ??? */
- goto reset_flash;
case 0x10: /* Single Byte Program */
case 0x40: /* Single Byte Program */
DPRINTF("%s: Single Byte Program\n", __func__);
@@ -526,7 +523,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
if (cmd == 0xd0) { /* confirm */
pfl->wcycle = 0;
pfl->status |= 0x80;
- } else if (cmd == 0xff) { /* read array mode */
+ } else if (cmd == 0xff) { /* Read Array */
goto reset_flash;
} else
goto error_flash;
@@ -553,7 +550,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
} else if (cmd == 0x01) {
pfl->wcycle = 0;
pfl->status |= 0x80;
- } else if (cmd == 0xff) {
+ } else if (cmd == 0xff) { /* read array mode */
goto reset_flash;
} else {
DPRINTF("%s: Unknown (un)locking command\n", __func__);
@@ -645,7 +642,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 = 0xff;
}
@@ -761,7 +758,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
}
pfl->wcycle = 0;
- pfl->cmd = 0;
+ pfl->cmd = 0xff;
pfl->status = 0;
/* Hardcoded CFI table */
/* Standard "QRY" string */
@@ -1001,5 +998,11 @@ static int pflash_post_load(void *opaque, int version_id)
pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
pfl);
}
+ if (version_id < 2) {
+ /* v1 used incorrect value of 0x00 for the READ_ARRAY command. */
+ if (pfl->cmd == 0x00) {
+ pfl->cmd = 0xff;
+ }
+ }
return 0;
}
--
2.20.1
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> In the "Read Array Flowchart" the command has a value of 0xFF.
>
> In the document [*] the "Read Array Flowchart", the READ_ARRAY
> command has a value of 0xff.
>
> Use the correct value in the pflash model.
>
> There is no change of behavior in the guest, because:
> - when the guest were sending 0xFF, the reset_flash label
> was setting the command value as 0x00
> - 0x00 was used internally for READ_ARRAY
>
> To keep migration behaving correctly, we have to increase
> the VMState version. When migrating from an older version,
> we use the correct command value.
The problem is that incrementing the version will break backwards
compatibility; so you won't be able to migrate this back to an older
QEMU version; so for example a q35/uefi with this won't be able
to migrate backwards to a 4.0.0 or older qemu.
So instead of bumping the version_id you probably need to wire
the behaviour to a machine type and then on your new type
wire a subsection containing a flag; the reception of that subsection
tells you to use the new/correct semantics.
Dave
> [*] "Common Flash Interface (CFI) and Command Sets"
> (Intel Application Note 646)
> Appendix B "Basic Command Set"
>
> Reviewed-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v3: Handle migrating the 'cmd' field.
>
> Since Laszlo stated he did not test migration [*], I'm keeping his
> test tag, because the change with v2 has no impact in the tests
> he ran.
>
> Likewise I'm keeping John and Alistair tags, but I'd like an extra
> review for the migration change, thanks!
>
> [*] https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00679.html
> ---
> hw/block/pflash_cfi01.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 9e34fd4e82..58cbef0588 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -100,7 +100,7 @@ static int pflash_post_load(void *opaque, int version_id);
>
> static const VMStateDescription vmstate_pflash = {
> .name = "pflash_cfi01",
> - .version_id = 1,
> + .version_id = 2,
> .minimum_version_id = 1,
> .post_load = pflash_post_load,
> .fields = (VMStateField[]) {
> @@ -277,10 +277,9 @@ 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;
> + pfl->cmd = 0xff;
> /* fall through to read code */
> - case 0x00:
> - /* Flash area read */
> + case 0xff: /* Read Array */
> ret = pflash_data_read(pfl, offset, width, be);
> break;
> case 0x10: /* Single byte program */
> @@ -448,8 +447,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
> case 0:
> /* read mode */
> switch (cmd) {
> - case 0x00: /* ??? */
> - goto reset_flash;
> case 0x10: /* Single Byte Program */
> case 0x40: /* Single Byte Program */
> DPRINTF("%s: Single Byte Program\n", __func__);
> @@ -526,7 +523,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
> if (cmd == 0xd0) { /* confirm */
> pfl->wcycle = 0;
> pfl->status |= 0x80;
> - } else if (cmd == 0xff) { /* read array mode */
> + } else if (cmd == 0xff) { /* Read Array */
> goto reset_flash;
> } else
> goto error_flash;
> @@ -553,7 +550,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
> } else if (cmd == 0x01) {
> pfl->wcycle = 0;
> pfl->status |= 0x80;
> - } else if (cmd == 0xff) {
> + } else if (cmd == 0xff) { /* read array mode */
> goto reset_flash;
> } else {
> DPRINTF("%s: Unknown (un)locking command\n", __func__);
> @@ -645,7 +642,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 = 0xff;
> }
>
>
> @@ -761,7 +758,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
> }
>
> pfl->wcycle = 0;
> - pfl->cmd = 0;
> + pfl->cmd = 0xff;
> pfl->status = 0;
> /* Hardcoded CFI table */
> /* Standard "QRY" string */
> @@ -1001,5 +998,11 @@ static int pflash_post_load(void *opaque, int version_id)
> pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
> pfl);
> }
> + if (version_id < 2) {
> + /* v1 used incorrect value of 0x00 for the READ_ARRAY command. */
> + if (pfl->cmd == 0x00) {
> + pfl->cmd = 0xff;
> + }
> + }
> return 0;
> }
> --
> 2.20.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Hi David,
On 7/9/19 12:30 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> In the "Read Array Flowchart" the command has a value of 0xFF.
>>
>> In the document [*] the "Read Array Flowchart", the READ_ARRAY
>> command has a value of 0xff.
>>
>> Use the correct value in the pflash model.
>>
>> There is no change of behavior in the guest, because:
>> - when the guest were sending 0xFF, the reset_flash label
>> was setting the command value as 0x00
>> - 0x00 was used internally for READ_ARRAY
>>
>> To keep migration behaving correctly, we have to increase
>> the VMState version. When migrating from an older version,
>> we use the correct command value.
>
> The problem is that incrementing the version will break backwards
> compatibility; so you won't be able to migrate this back to an older
> QEMU version; so for example a q35/uefi with this won't be able
> to migrate backwards to a 4.0.0 or older qemu.
>
> So instead of bumping the version_id you probably need to wire
> the behaviour to a machine type and then on your new type
> wire a subsection containing a flag; the reception of that subsection
> tells you to use the new/correct semantics.
I'm starting to understand VMState subsections, but it might be overkill
for this change...
Subsections
-----------
The most common structure change is adding new data, e.g. when adding
a newer form of device, or adding that state that you previously
forgot to migrate. This is best solved using a subsection.
This is not the case here, the field is already present and migrated.
It seems I can use a simple pre_save hook, always migrating the
READ_ARRAY using the incorrect value:
-- >8 --
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -97,12 +97,29 @@ struct PFlashCFI01 {
bool incorrect_read_array_command;
};
+static int pflash_pre_save(void *opaque)
+{
+ PFlashCFI01 *s = opaque;
+
+ /*
+ * Previous to QEMU v4.1 an incorrect value of 0x00 was used for the
+ * READ_ARRAY command. To preserve migrating to these older version,
+ * always migrate the READ_ARRAY command as 0x00.
+ */
+ if (s->cmd == 0xff) {
+ s->cmd = 0x00;
+ }
+
+ return 0;
+}
+
static int pflash_post_load(void *opaque, int version_id);
static const VMStateDescription vmstate_pflash = {
.name = "pflash_cfi01",
.version_id = 1,
.minimum_version_id = 1,
+ .pre_save = pflash_pre_save,
.post_load = pflash_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINT8(wcycle, PFlashCFI01),
@@ -1001,5 +1018,14 @@ static int pflash_post_load(void *opaque, int
version_id)
pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
pfl);
}
+
+ /*
+ * Previous to QEMU v4.1 an incorrect value of 0x00 was used for the
+ * READ_ARRAY command.
+ */
+ if (pfl->cmd == 0x00) {
+ pfl->cmd = 0xff;
+ }
+
return 0;
}
---
Being simpler and less intrusive (no new property in hw/core/machine.c),
is this acceptable?
Thanks,
Phil.
[...]
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Hi David,
>
> On 7/9/19 12:30 PM, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> >> In the "Read Array Flowchart" the command has a value of 0xFF.
> >>
> >> In the document [*] the "Read Array Flowchart", the READ_ARRAY
> >> command has a value of 0xff.
> >>
> >> Use the correct value in the pflash model.
> >>
> >> There is no change of behavior in the guest, because:
> >> - when the guest were sending 0xFF, the reset_flash label
> >> was setting the command value as 0x00
> >> - 0x00 was used internally for READ_ARRAY
> >>
> >> To keep migration behaving correctly, we have to increase
> >> the VMState version. When migrating from an older version,
> >> we use the correct command value.
> >
> > The problem is that incrementing the version will break backwards
> > compatibility; so you won't be able to migrate this back to an older
> > QEMU version; so for example a q35/uefi with this won't be able
> > to migrate backwards to a 4.0.0 or older qemu.
> >
> > So instead of bumping the version_id you probably need to wire
> > the behaviour to a machine type and then on your new type
> > wire a subsection containing a flag; the reception of that subsection
> > tells you to use the new/correct semantics.
>
> I'm starting to understand VMState subsections, but it might be overkill
> for this change...
>
> Subsections
> -----------
>
> The most common structure change is adding new data, e.g. when adding
> a newer form of device, or adding that state that you previously
> forgot to migrate. This is best solved using a subsection.
>
> This is not the case here, the field is already present and migrated.
>
> It seems I can use a simple pre_save hook, always migrating the
> READ_ARRAY using the incorrect value:
>
> -- >8 --
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -97,12 +97,29 @@ struct PFlashCFI01 {
> bool incorrect_read_array_command;
> };
>
> +static int pflash_pre_save(void *opaque)
> +{
> + PFlashCFI01 *s = opaque;
> +
> + /*
> + * Previous to QEMU v4.1 an incorrect value of 0x00 was used for the
> + * READ_ARRAY command. To preserve migrating to these older version,
> + * always migrate the READ_ARRAY command as 0x00.
> + */
> + if (s->cmd == 0xff) {
> + s->cmd = 0x00;
> + }
> +
> + return 0;
> +}
Be careful what happens if migration fails and you continue on the
source - is that OK - or are you going to have to flip that back somehow
(in a post_save).
Another way to do the same is to have a dummy field; tmp_cmd, and the
tmp_cmd is the thing that's actually migrated and filled by pre_save
(or use VMSTATE_WITH_TMP )
> static int pflash_post_load(void *opaque, int version_id);
>
> static const VMStateDescription vmstate_pflash = {
> .name = "pflash_cfi01",
> .version_id = 1,
> .minimum_version_id = 1,
> + .pre_save = pflash_pre_save,
> .post_load = pflash_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT8(wcycle, PFlashCFI01),
> @@ -1001,5 +1018,14 @@ static int pflash_post_load(void *opaque, int
> version_id)
> pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
> pfl);
> }
> +
> + /*
> + * Previous to QEMU v4.1 an incorrect value of 0x00 was used for the
> + * READ_ARRAY command.
> + */
> + if (pfl->cmd == 0x00) {
> + pfl->cmd = 0xff;
> + }
> +
> return 0;
> }
> ---
>
> Being simpler and less intrusive (no new property in hw/core/machine.c),
> is this acceptable?
From the migration point of view yes; I don't know enough about pflash
to say if it makes sense; for example could there ever be a 00 command
really used and then you'd have to distinguish that somehow?
> Thanks,
>
> Phil.
>
> [...]
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 7/9/19 7:10 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> Hi David,
>>
>> On 7/9/19 12:30 PM, Dr. David Alan Gilbert wrote:
>>> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>>>> In the "Read Array Flowchart" the command has a value of 0xFF.
>>>>
>>>> In the document [*] the "Read Array Flowchart", the READ_ARRAY
>>>> command has a value of 0xff.
>>>>
>>>> Use the correct value in the pflash model.
>>>>
>>>> There is no change of behavior in the guest, because:
>>>> - when the guest were sending 0xFF, the reset_flash label
>>>> was setting the command value as 0x00
>>>> - 0x00 was used internally for READ_ARRAY
>>>>
>>>> To keep migration behaving correctly, we have to increase
>>>> the VMState version. When migrating from an older version,
>>>> we use the correct command value.
>>>
>>> The problem is that incrementing the version will break backwards
>>> compatibility; so you won't be able to migrate this back to an older
>>> QEMU version; so for example a q35/uefi with this won't be able
>>> to migrate backwards to a 4.0.0 or older qemu.
>>>
>>> So instead of bumping the version_id you probably need to wire
>>> the behaviour to a machine type and then on your new type
>>> wire a subsection containing a flag; the reception of that subsection
>>> tells you to use the new/correct semantics.
>>
>> I'm starting to understand VMState subsections, but it might be overkill
>> for this change...
>>
>> Subsections
>> -----------
>>
>> The most common structure change is adding new data, e.g. when adding
>> a newer form of device, or adding that state that you previously
>> forgot to migrate. This is best solved using a subsection.
>>
>> This is not the case here, the field is already present and migrated.
>>
>> It seems I can use a simple pre_save hook, always migrating the
>> READ_ARRAY using the incorrect value:
>>
>> -- >8 --
>> --- a/hw/block/pflash_cfi01.c
>> +++ b/hw/block/pflash_cfi01.c
>> @@ -97,12 +97,29 @@ struct PFlashCFI01 {
>> bool incorrect_read_array_command;
>> };
>>
>> +static int pflash_pre_save(void *opaque)
>> +{
>> + PFlashCFI01 *s = opaque;
>> +
>> + /*
>> + * Previous to QEMU v4.1 an incorrect value of 0x00 was used for the
>> + * READ_ARRAY command. To preserve migrating to these older version,
>> + * always migrate the READ_ARRAY command as 0x00.
>> + */
>> + if (s->cmd == 0xff) {
>> + s->cmd = 0x00;
>> + }
>> +
>> + return 0;
>> +}
>
> Be careful what happens if migration fails and you continue on the
> source - is that OK - or are you going to have to flip that back somehow
> (in a post_save).
Hmm OK...
>
> Another way to do the same is to have a dummy field; tmp_cmd, and the
> tmp_cmd is the thing that's actually migrated and filled by pre_save
> (or use VMSTATE_WITH_TMP )
>
>
>> static int pflash_post_load(void *opaque, int version_id);
>>
>> static const VMStateDescription vmstate_pflash = {
>> .name = "pflash_cfi01",
>> .version_id = 1,
>> .minimum_version_id = 1,
>> + .pre_save = pflash_pre_save,
>> .post_load = pflash_post_load,
>> .fields = (VMStateField[]) {
>> VMSTATE_UINT8(wcycle, PFlashCFI01),
>> @@ -1001,5 +1018,14 @@ static int pflash_post_load(void *opaque, int
>> version_id)
>> pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
>> pfl);
>> }
>> +
>> + /*
>> + * Previous to QEMU v4.1 an incorrect value of 0x00 was used for the
>> + * READ_ARRAY command.
>> + */
>> + if (pfl->cmd == 0x00) {
>> + pfl->cmd = 0xff;
>> + }
>> +
>> return 0;
>> }
>> ---
>>
>> Being simpler and less intrusive (no new property in hw/core/machine.c),
>> is this acceptable?
>
> From the migration point of view yes; I don't know enough about pflash
> to say if it makes sense; for example could there ever be a 00 command
> really used and then you'd have to distinguish that somehow?
Well, I think this change is simpler than it looks.
Currently the code is (what will run on older guest):
static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
int width, int be)
{
switch (pfl->cmd) {
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;
pfl->cmd = 0;
/* fall through to read code */
case 0x00:
/* Flash area read */
ret = pflash_data_read(pfl, offset, width, be);
break;
and:
static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
uint32_t value, int width, int be)
{
switch (pfl->wcycle) {
case 0:
/* read mode */
switch (cmd) {
case 0x00: /* ??? */
goto reset_flash;
case 0xff: /* Read array mode */
DPRINTF("%s: Read array mode\n", __func__);
goto reset_flash;
default:
goto error_flash;
}
So current (incorrect) 0x00 will be then 0xff, and will be backprocessed
correctly.
What I want is to get ride of this 0x00 processing that is confusing,
the spec and the guests use 0xff for READ_ARRAY.
So if I increase version I can not back-migrate, but luckily it seems I
can simply update 0x00 -> 0xff without even increasing the version :)
(I'm reluctant to skip this patch because I'd rather avoid Laszlo to
re-run his tests).
Regards,
Phil.
On 07/05/19 17:46, Philippe Mathieu-Daudé wrote:
> In the "Read Array Flowchart" the command has a value of 0xFF.
>
> In the document [*] the "Read Array Flowchart", the READ_ARRAY
> command has a value of 0xff.
>
> Use the correct value in the pflash model.
>
> There is no change of behavior in the guest, because:
> - when the guest were sending 0xFF, the reset_flash label
> was setting the command value as 0x00
> - 0x00 was used internally for READ_ARRAY
>
> To keep migration behaving correctly, we have to increase
> the VMState version. When migrating from an older version,
> we use the correct command value.
>
> [*] "Common Flash Interface (CFI) and Command Sets"
> (Intel Application Note 646)
> Appendix B "Basic Command Set"
>
> Reviewed-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v3: Handle migrating the 'cmd' field.
>
> Since Laszlo stated he did not test migration [*], I'm keeping his
> test tag, because the change with v2 has no impact in the tests
> he ran.
My thinking exactly. Thanks!
Laszlo
>
> Likewise I'm keeping John and Alistair tags, but I'd like an extra
> review for the migration change, thanks!
>
> [*] https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00679.html
> ---
> hw/block/pflash_cfi01.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 9e34fd4e82..58cbef0588 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -100,7 +100,7 @@ static int pflash_post_load(void *opaque, int version_id);
>
> static const VMStateDescription vmstate_pflash = {
> .name = "pflash_cfi01",
> - .version_id = 1,
> + .version_id = 2,
> .minimum_version_id = 1,
> .post_load = pflash_post_load,
> .fields = (VMStateField[]) {
> @@ -277,10 +277,9 @@ 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;
> + pfl->cmd = 0xff;
> /* fall through to read code */
> - case 0x00:
> - /* Flash area read */
> + case 0xff: /* Read Array */
> ret = pflash_data_read(pfl, offset, width, be);
> break;
> case 0x10: /* Single byte program */
> @@ -448,8 +447,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
> case 0:
> /* read mode */
> switch (cmd) {
> - case 0x00: /* ??? */
> - goto reset_flash;
> case 0x10: /* Single Byte Program */
> case 0x40: /* Single Byte Program */
> DPRINTF("%s: Single Byte Program\n", __func__);
> @@ -526,7 +523,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
> if (cmd == 0xd0) { /* confirm */
> pfl->wcycle = 0;
> pfl->status |= 0x80;
> - } else if (cmd == 0xff) { /* read array mode */
> + } else if (cmd == 0xff) { /* Read Array */
> goto reset_flash;
> } else
> goto error_flash;
> @@ -553,7 +550,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
> } else if (cmd == 0x01) {
> pfl->wcycle = 0;
> pfl->status |= 0x80;
> - } else if (cmd == 0xff) {
> + } else if (cmd == 0xff) { /* read array mode */
> goto reset_flash;
> } else {
> DPRINTF("%s: Unknown (un)locking command\n", __func__);
> @@ -645,7 +642,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 = 0xff;
> }
>
>
> @@ -761,7 +758,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
> }
>
> pfl->wcycle = 0;
> - pfl->cmd = 0;
> + pfl->cmd = 0xff;
> pfl->status = 0;
> /* Hardcoded CFI table */
> /* Standard "QRY" string */
> @@ -1001,5 +998,11 @@ static int pflash_post_load(void *opaque, int version_id)
> pfl->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
> pfl);
> }
> + if (version_id < 2) {
> + /* v1 used incorrect value of 0x00 for the READ_ARRAY command. */
> + if (pfl->cmd == 0x00) {
> + pfl->cmd = 0xff;
> + }
> + }
> return 0;
> }
>
© 2016 - 2026 Red Hat, Inc.