* [Qemu-devel] [PATCH-for-4.1 v6 1/5] hw/block/pflash_cfi01: Removed an unused timer
2019-07-16 22:15 [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler Philippe Mathieu-Daudé
@ 2019-07-16 22:15 ` Philippe Mathieu-Daudé
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 2/5] hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00' Philippe Mathieu-Daudé
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-16 22:15 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Laszlo Ersek, qemu-block,
Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Alistair Francis, Wei Yang,
John Snow, Dr . David Alan Gilbert
The 'CFI02' NOR flash was introduced in commit 29133e9a0fff, with
timing modelled. One year later, the CFI01 model was introduced
(commit 05ee37ebf630) based on the CFI02 model. As noted in the
header, "It does not support timings". 12 years later, we never
had to model the device timings. Time to remove the unused timer,
we can still add it back if required.
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Laszlo Ersek <lersek@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>
---
hw/block/pflash_cfi01.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 435be1e35c..a9529957f8 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -42,7 +42,6 @@
#include "hw/block/flash.h"
#include "sysemu/block-backend.h"
#include "qapi/error.h"
-#include "qemu/timer.h"
#include "qemu/bitops.h"
#include "qemu/error-report.h"
#include "qemu/host-utils.h"
@@ -90,7 +89,6 @@ struct PFlashCFI01 {
uint8_t cfi_table[0x52];
uint64_t counter;
unsigned int writeblock_size;
- QEMUTimer *timer;
MemoryRegion mem;
char *name;
void *storage;
@@ -114,18 +112,6 @@ static const VMStateDescription vmstate_pflash = {
}
};
-static void pflash_timer (void *opaque)
-{
- PFlashCFI01 *pfl = opaque;
-
- trace_pflash_timer_expired(pfl->cmd);
- /* Reset flash */
- pfl->status ^= 0x80;
- memory_region_rom_device_set_romd(&pfl->mem, true);
- pfl->wcycle = 0;
- pfl->cmd = 0;
-}
-
/* 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.
@@ -774,7 +760,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
pfl->max_device_width = pfl->device_width;
}
- pfl->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
pfl->wcycle = 0;
pfl->cmd = 0;
pfl->status = 0x80; /* WSM ready */
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH-for-4.1 v6 2/5] hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00'
2019-07-16 22:15 [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler Philippe Mathieu-Daudé
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 1/5] hw/block/pflash_cfi01: Removed an unused timer Philippe Mathieu-Daudé
@ 2019-07-16 22:15 ` Philippe Mathieu-Daudé
2019-07-16 22:24 ` Alistair Francis
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 3/5] hw/block/pflash_cfi01: Extract pflash_mode_read_array() Philippe Mathieu-Daudé
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-16 22:15 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Laszlo Ersek, qemu-block,
Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Alistair Francis, John Snow,
Dr . David Alan Gilbert
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é <philmd@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH-for-4.1 v6 2/5] hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00'
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 2/5] hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00' Philippe Mathieu-Daudé
@ 2019-07-16 22:24 ` Alistair Francis
0 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2019-07-16 22:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Qemu-block, John Snow,
qemu-devel@nongnu.org Developers, Markus Armbruster,
Alistair Francis, Max Reitz, Laszlo Ersek,
Dr . David Alan Gilbert
On Tue, Jul 16, 2019 at 3:16 PM Philippe Mathieu-Daudé
<philmd@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é <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@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
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH-for-4.1 v6 3/5] hw/block/pflash_cfi01: Extract pflash_mode_read_array()
2019-07-16 22:15 [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler Philippe Mathieu-Daudé
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 1/5] hw/block/pflash_cfi01: Removed an unused timer Philippe Mathieu-Daudé
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 2/5] hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00' Philippe Mathieu-Daudé
@ 2019-07-16 22:15 ` Philippe Mathieu-Daudé
2019-07-16 22:53 ` Alistair Francis
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 4/5] hw/block/pflash_cfi01: Rename 'reset_flash' label as 'mode_read_array' Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-16 22:15 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Laszlo Ersek, qemu-block,
Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Alistair Francis, John Snow,
Dr . David Alan Gilbert
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH-for-4.1 v6 3/5] hw/block/pflash_cfi01: Extract pflash_mode_read_array()
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 3/5] hw/block/pflash_cfi01: Extract pflash_mode_read_array() Philippe Mathieu-Daudé
@ 2019-07-16 22:53 ` Alistair Francis
0 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2019-07-16 22:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Qemu-block, John Snow,
qemu-devel@nongnu.org Developers, Markus Armbruster,
Alistair Francis, Max Reitz, Laszlo Ersek,
Dr . David Alan Gilbert
On Tue, Jul 16, 2019 at 3:16 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> 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>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> 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
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH-for-4.1 v6 4/5] hw/block/pflash_cfi01: Rename 'reset_flash' label as 'mode_read_array'
2019-07-16 22:15 [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 3/5] hw/block/pflash_cfi01: Extract pflash_mode_read_array() Philippe Mathieu-Daudé
@ 2019-07-16 22:15 ` Philippe Mathieu-Daudé
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 5/5] hw/block/pflash_cfi01: Add the DeviceReset() handler Philippe Mathieu-Daudé
2019-07-17 12:24 ` [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add " Laszlo Ersek
5 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-16 22:15 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Laszlo Ersek, qemu-block,
Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Alistair Francis, John Snow,
Dr . David Alan Gilbert
Rename the 'reset_flash' as 'mode_read_array' to make explicit we
do not reset the device, we simply set its internal state machine
in the READ_ARRAY mode. We do not reset the status register error
bits, as a device reset would do.
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v6: split of from previous patch to ease review
dropped Laszlo's Regression-tested-by tag
---
hw/block/pflash_cfi01.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index a28d0f8cc7..65afdbf3a7 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -460,7 +460,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
/* read mode */
switch (cmd) {
case 0x00: /* This model reset value for READ_ARRAY (not CFI) */
- goto reset_flash;
+ goto mode_read_array;
case 0x10: /* Single Byte Program */
case 0x40: /* Single Byte Program */
DPRINTF("%s: Single Byte Program\n", __func__);
@@ -483,7 +483,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
case 0x50: /* Clear status bits */
DPRINTF("%s: Clear status bits\n", __func__);
pfl->status = 0x0;
- goto reset_flash;
+ goto mode_read_array;
case 0x60: /* Block (un)lock */
DPRINTF("%s: Block unlock\n", __func__);
break;
@@ -508,10 +508,10 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
break;
case 0xf0: /* Probe for AMD flash */
DPRINTF("%s: Probe for AMD flash\n", __func__);
- goto reset_flash;
- case 0xff: /* Read array mode */
+ goto mode_read_array;
+ case 0xff: /* Read Array */
DPRINTF("%s: Read array mode\n", __func__);
- goto reset_flash;
+ goto mode_read_array;
default:
goto error_flash;
}
@@ -537,8 +537,8 @@ 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 */
- goto reset_flash;
+ } else if (cmd == 0xff) { /* Read Array */
+ goto mode_read_array;
} else
goto error_flash;
@@ -564,16 +564,16 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
} else if (cmd == 0x01) {
pfl->wcycle = 0;
pfl->status |= 0x80;
- } else if (cmd == 0xff) {
- goto reset_flash;
+ } else if (cmd == 0xff) { /* Read Array */
+ goto mode_read_array;
} else {
DPRINTF("%s: Unknown (un)locking command\n", __func__);
- goto reset_flash;
+ goto mode_read_array;
}
break;
case 0x98:
- if (cmd == 0xff) {
- goto reset_flash;
+ if (cmd == 0xff) { /* Read Array */
+ goto mode_read_array;
} else {
DPRINTF("%s: leaving query mode\n", __func__);
}
@@ -633,7 +633,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
" the data is already written to storage!\n"
"Flash device reset into READ mode.\n",
__func__);
- goto reset_flash;
+ goto mode_read_array;
}
break;
default:
@@ -643,7 +643,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
default:
/* Should never happen */
DPRINTF("%s: invalid write state\n", __func__);
- goto reset_flash;
+ goto mode_read_array;
}
return;
@@ -652,7 +652,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
"(offset " TARGET_FMT_plx ", wcycle 0x%x cmd 0x%x value 0x%x)"
"\n", __func__, offset, pfl->wcycle, pfl->cmd, value);
- reset_flash:
+ mode_read_array:
pflash_mode_read_array(pfl);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH-for-4.1 v6 5/5] hw/block/pflash_cfi01: Add the DeviceReset() handler
2019-07-16 22:15 [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 4/5] hw/block/pflash_cfi01: Rename 'reset_flash' label as 'mode_read_array' Philippe Mathieu-Daudé
@ 2019-07-16 22:15 ` Philippe Mathieu-Daudé
2019-07-17 6:43 ` Philippe Mathieu-Daudé
2019-07-17 12:24 ` [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add " Laszlo Ersek
5 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-16 22:15 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Laszlo Ersek, qemu-block,
Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz, Alistair Francis, John Snow,
Dr . David Alan Gilbert
A "system reset" sets the device state machine in READ_ARRAY mode
and, after some delay, set the SR.7 READY bit.
We do not model timings, so we set the SR.7 bit directly.
The TYPE_DEVICE interface provides a DeviceReset handler.
This pflash device is a subclass of TYPE_SYS_BUS_DEVICE (which
is a subclass of TYPE_DEVICE).
SYS_BUS devices are automatically plugged into the 'main system
bus', which is the root of the qbus tree.
Devices in the qbus tree are guaranteed to have their reset()
handler called after realize() and before we try to run the guest.
To avoid incoherent states when the machine resets (see but report
below), factor out the reset code into pflash_cfi01_system_reset,
and register the method as a device reset callback.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713
Reported-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v6: dropped Laszlo's Regression-tested-by tag
---
hw/block/pflash_cfi01.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 65afdbf3a7..ee0ed70242 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -768,8 +768,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
pfl->max_device_width = pfl->device_width;
}
- pflash_mode_read_array(pfl);
- pfl->status = 0x80; /* WSM ready */
/* Hardcoded CFI table */
/* Standard "QRY" string */
pfl->cfi_table[0x10] = 'Q';
@@ -857,6 +855,18 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
}
+static void pflash_cfi01_system_reset(DeviceState *dev)
+{
+ PFlashCFI01 *pfl = PFLASH_CFI01(dev);
+
+ pflash_mode_read_array(pfl);
+ /*
+ * The WSM ready timer occurs at most 150ns after system reset.
+ * This model deliberately ignores this delay.
+ */
+ pfl->status = 0x80;
+}
+
static Property pflash_cfi01_properties[] = {
DEFINE_PROP_DRIVE("drive", PFlashCFI01, blk),
/* num-blocks is the number of blocks actually visible to the guest,
@@ -901,6 +911,7 @@ static void pflash_cfi01_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+ dc->reset = pflash_cfi01_system_reset;
dc->realize = pflash_cfi01_realize;
dc->props = pflash_cfi01_properties;
dc->vmsd = &vmstate_pflash;
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH-for-4.1 v6 5/5] hw/block/pflash_cfi01: Add the DeviceReset() handler
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 5/5] hw/block/pflash_cfi01: Add the DeviceReset() handler Philippe Mathieu-Daudé
@ 2019-07-17 6:43 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-17 6:43 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, qemu-block, Laszlo Ersek,
Markus Armbruster, Max Reitz, Alistair Francis, John Snow,
Dr . David Alan Gilbert
On 7/17/19 12:15 AM, Philippe Mathieu-Daudé wrote:
> A "system reset" sets the device state machine in READ_ARRAY mode
> and, after some delay, set the SR.7 READY bit.
>
> We do not model timings, so we set the SR.7 bit directly.
>
> The TYPE_DEVICE interface provides a DeviceReset handler.
> This pflash device is a subclass of TYPE_SYS_BUS_DEVICE (which
> is a subclass of TYPE_DEVICE).
> SYS_BUS devices are automatically plugged into the 'main system
> bus', which is the root of the qbus tree.
> Devices in the qbus tree are guaranteed to have their reset()
> handler called after realize() and before we try to run the guest.
>
> To avoid incoherent states when the machine resets (see but report
"bug report"
> below), factor out the reset code into pflash_cfi01_system_reset,
> and register the method as a device reset callback.
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v6: dropped Laszlo's Regression-tested-by tag
> ---
> hw/block/pflash_cfi01.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 65afdbf3a7..ee0ed70242 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -768,8 +768,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
> pfl->max_device_width = pfl->device_width;
> }
>
> - pflash_mode_read_array(pfl);
> - pfl->status = 0x80; /* WSM ready */
> /* Hardcoded CFI table */
> /* Standard "QRY" string */
> pfl->cfi_table[0x10] = 'Q';
> @@ -857,6 +855,18 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
> pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
> }
>
> +static void pflash_cfi01_system_reset(DeviceState *dev)
> +{
> + PFlashCFI01 *pfl = PFLASH_CFI01(dev);
> +
> + pflash_mode_read_array(pfl);
> + /*
> + * The WSM ready timer occurs at most 150ns after system reset.
> + * This model deliberately ignores this delay.
> + */
> + pfl->status = 0x80;
> +}
> +
> static Property pflash_cfi01_properties[] = {
> DEFINE_PROP_DRIVE("drive", PFlashCFI01, blk),
> /* num-blocks is the number of blocks actually visible to the guest,
> @@ -901,6 +911,7 @@ static void pflash_cfi01_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> + dc->reset = pflash_cfi01_system_reset;
> dc->realize = pflash_cfi01_realize;
> dc->props = pflash_cfi01_properties;
> dc->vmsd = &vmstate_pflash;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler
2019-07-16 22:15 [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2019-07-16 22:15 ` [Qemu-devel] [PATCH-for-4.1 v6 5/5] hw/block/pflash_cfi01: Add the DeviceReset() handler Philippe Mathieu-Daudé
@ 2019-07-17 12:24 ` Laszlo Ersek
2019-07-17 14:00 ` Philippe Mathieu-Daudé
5 siblings, 1 reply; 11+ messages in thread
From: Laszlo Ersek @ 2019-07-17 12:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Peter Maydell, qemu-block, Markus Armbruster,
Max Reitz, Alistair Francis, John Snow, Dr . David Alan Gilbert
Hi Phil,
On 07/17/19 00:15, Philippe Mathieu-Daudé wrote:
> Hello it's me again, insisting with this series because there are at
> least 2 different report of guests bricked on reset due to the bug
> fixed by patch #5:
> https://bugzilla.redhat.com/show_bug.cgi?id=1678713
> https://bugzilla.redhat.com/show_bug.cgi?id=1704584
>
> Patches missing review: 2 and 3
>
> The pflash device lacks a reset() function.
> When a machine is resetted, the flash might be in an
> inconsistent state, leading to unexpected behavior.
> Resolve this issue by adding a DeviceReset() handler.
>
> Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00962.html
> - addressed Laszlo review comments
>
> Since v2: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00395.html
> - consider migration (Laszlo, Peter)
>
> Since v3: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01668.html
> - more reliable migration (Dave)
> - dropped patches 6-9 not required for next release
>
> Since v4: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg02785.html
> - document why using READ_ARRAY value 0x00 for migration is safe
>
> Since v5: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03366.html
> - avoid trying to be spec-compliant and messing with migration. KISS.
> review/test tags reset, sorry.
I have a number of questions / requests.
(1) The last I recall from the v5 discussion is Markus asking about some
risky cases (corner cases?) related to migration.
So what is the new avenue taken in v6? What does "continue ignoring spec
compliance" mean, with regard to migration?
My vague understanding is that we're not trying to answer Markus's
questions; instead, we're side-stepping them, by doing something else.
That works for me, but can we please summarize in a bit more detail?
Like, "in v6, we're not mapping 0x00 vs. 0xff across migration because..."
Yes, I could inter-diff v5 vs. v6, but I know way too little about
pflash. I'd miss how our *dropping* of the special 0xff mapping impacts
our conformance to the data sheet.
I'm not asking for commit message updates, just a bit more explanation
in this free-form discussion. (I looked for it under v5, and couldn't
find it.)
(2) Has someone regression-tested v6 for migration specifically?
Or, is v6 not "risky" wrt. migration any longer?
(3) I'm fine regression testing v6 too (without migration, again).
Please ping me separately once the reviews have converged and the series
is otherwise ready for merging.
Thanks!
Laszlo
> $ git backport-diff -u v5
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/5:[----] [-C] 'hw/block/pflash_cfi01: Removed an unused timer'
> 002/5:[down] 'hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00''
> 003/5:[0031] [FC] 'hw/block/pflash_cfi01: Extract pflash_mode_read_array()'
> 004/5:[down] 'hw/block/pflash_cfi01: Rename 'reset_flash' label as 'mode_read_array''
> 005/5:[----] [--] 'hw/block/pflash_cfi01: Add the DeviceReset() handler'
>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (5):
> hw/block/pflash_cfi01: Removed an unused timer
> hw/block/pflash_cfi01: Document use of non-CFI compliant command
> '0x00'
> hw/block/pflash_cfi01: Extract pflash_mode_read_array()
> hw/block/pflash_cfi01: Rename 'reset_flash' label as 'mode_read_array'
> hw/block/pflash_cfi01: Add the DeviceReset() handler
>
> hw/block/pflash_cfi01.c | 77 +++++++++++++++++++++--------------------
> hw/block/trace-events | 1 +
> 2 files changed, 41 insertions(+), 37 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add DeviceReset() handler
2019-07-17 12:24 ` [Qemu-devel] [PATCH-for-4.1 v6 0/5] hw/block/pflash_cfi01: Add " Laszlo Ersek
@ 2019-07-17 14:00 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-17 14:00 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel
Cc: Kevin Wolf, Peter Maydell, qemu-block, Markus Armbruster,
Max Reitz, Alistair Francis, John Snow, Dr . David Alan Gilbert
Hi Laszlo,
On 7/17/19 2:24 PM, Laszlo Ersek wrote:
> Hi Phil,
>
> On 07/17/19 00:15, Philippe Mathieu-Daudé wrote:
>> Hello it's me again, insisting with this series because there are at
>> least 2 different report of guests bricked on reset due to the bug
>> fixed by patch #5:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1678713
>> https://bugzilla.redhat.com/show_bug.cgi?id=1704584
>>
>> Patches missing review: 2 and 3
>>
>> The pflash device lacks a reset() function.
>> When a machine is resetted, the flash might be in an
>> inconsistent state, leading to unexpected behavior.
>> Resolve this issue by adding a DeviceReset() handler.
>>
>> Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00962.html
>> - addressed Laszlo review comments
>>
>> Since v2: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg00395.html
>> - consider migration (Laszlo, Peter)
>>
>> Since v3: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01668.html
>> - more reliable migration (Dave)
>> - dropped patches 6-9 not required for next release
>>
>> Since v4: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg02785.html
>> - document why using READ_ARRAY value 0x00 for migration is safe
>>
>> Since v5: https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03366.html
>> - avoid trying to be spec-compliant and messing with migration. KISS.
>> review/test tags reset, sorry.
>
> I have a number of questions / requests.
>
>
> (1) The last I recall from the v5 discussion is Markus asking about some
> risky cases (corner cases?) related to migration.
>
> So what is the new avenue taken in v6? What does "continue ignoring spec
> compliance" mean, with regard to migration?
>
> My vague understanding is that we're not trying to answer Markus's
> questions; instead, we're side-stepping them, by doing something else.
I'd love to keep the v5 series and address Markus issues, but as shown
by the quantity of respins, I don't understand the migration feature
enough to fix it in time for the next release. I plan to address them
(and other issues reported by Markus in other reviews) during the next
dev cycle.
> That works for me, but can we please summarize in a bit more detail?
> Like, "in v6, we're not mapping 0x00 vs. 0xff across migration because..."
Since it is very late in the release process and this series intend to
fix a guest corruption bug worthwhile for release, the approch taken by
v6 is "try to change the strict minimum regarding to migration, do not
worry about spec issues". I even tried to make no migration change at
all, but as explained in patch 6 "Extract pflash_mode_read_array" there
is still one.
I could make no migration change, and in patch 6 not call
memory_region_rom_device_set_romd() in pflash_mode_read_array() [and
call it in other places instead], but then we still have the undefined
behavior described in the patch. We always lived with this UNDEF, so...
I could work on a simpler v7.
> Yes, I could inter-diff v5 vs. v6, but I know way too little about
> pflash. I'd miss how our *dropping* of the special 0xff mapping impacts
> our conformance to the data sheet.
To reassure you, it never worked well but nobody cared, I noticed while
converting DPRINTF to trace-events and adding more, then follow the
model state machine. While it's probably not worth fixing, it makes
debugging slighly harder when looking at the CFI spec workflow. Now I'm
used to it.
> I'm not asking for commit message updates, just a bit more explanation
> in this free-form discussion. (I looked for it under v5, and couldn't
> find it.)
I tried to be verbose in the patch description, so for reference:
0xff on v5:
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03368.html
not 0xff on v6:
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03931.html
migration impact on v6 [1]:
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03932.html
> (2) Has someone regression-tested v6 for migration specifically?
No, neither were tested the previous versions.
>
> Or, is v6 not "risky" wrt. migration any longer?
v6 should be way less risky than previous versions (still one issue
noted in [1]).
> (3) I'm fine regression testing v6 too (without migration, again).
> Please ping me separately once the reviews have converged and the series
> is otherwise ready for merging.
Yes, I know your testing takes very long, so let's wait first.
Thanks for having a look.
Phil.
>
> Thanks!
> Laszlo
>
>
>> $ git backport-diff -u v5
>> Key:
>> [----] : patches are identical
>> [####] : number of functional differences between upstream/downstream patch
>> [down] : patch is downstream-only
>> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>>
>> 001/5:[----] [-C] 'hw/block/pflash_cfi01: Removed an unused timer'
>> 002/5:[down] 'hw/block/pflash_cfi01: Document use of non-CFI compliant command '0x00''
>> 003/5:[0031] [FC] 'hw/block/pflash_cfi01: Extract pflash_mode_read_array()'
>> 004/5:[down] 'hw/block/pflash_cfi01: Rename 'reset_flash' label as 'mode_read_array''
>> 005/5:[----] [--] 'hw/block/pflash_cfi01: Add the DeviceReset() handler'
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (5):
>> hw/block/pflash_cfi01: Removed an unused timer
>> hw/block/pflash_cfi01: Document use of non-CFI compliant command
>> '0x00'
>> hw/block/pflash_cfi01: Extract pflash_mode_read_array()
>> hw/block/pflash_cfi01: Rename 'reset_flash' label as 'mode_read_array'
>> hw/block/pflash_cfi01: Add the DeviceReset() handler
>>
>> hw/block/pflash_cfi01.c | 77 +++++++++++++++++++++--------------------
>> hw/block/trace-events | 1 +
>> 2 files changed, 41 insertions(+), 37 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread