[Qemu-devel] [PULL v2 0/2] pflash-next patches for v4.1.0-rc2

[Qemu-devel] [PULL v2 0/2] pflash-next patches for v4.1.0-rc2

[Philippe Mathieu-Daudé]

The following changes since commit 23da9e297b4120ca9702cabec91599a44255fe96:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190722' into staging (2019-07-22 15:16:48 +0100)

are available in the Git repository at:

  https://gitlab.com/philmd/qemu.git tags/pflash-next-20190723

for you to fetch changes up to 124e4cfaa42bb5a14eec33ea47d3502b5f46eb33:

  hw/block/pflash_cfi02: Rewrite a fall through comment (2019-07-23 11:31:07 +0200)

----------------------------------------------------------------
One bugfix and silent a fallthru warning.

----------------------------------------------------------------
v2: Replace Laszlo's Regression-tested-by tag

Philippe Mathieu-Daudé (2):
  hw/block/pflash_cfi01: Add missing DeviceReset() handler
  hw/block/pflash_cfi02: Rewrite a fall through comment

 hw/block/pflash_cfi01.c | 19 +++++++++++++++++++
 hw/block/pflash_cfi02.c |  2 +-
 2 files changed, 20 insertions(+), 1 deletion(-)

-- 
2.20.1

[Qemu-devel] [PULL v2 1/2] hw/block/pflash_cfi01: Add missing DeviceReset() handler

[Philippe Mathieu-Daudé]

To avoid incoherent states when the machine resets (see bug report
below), add the device reset callback.

A "system reset" sets the device state machine in READ_ARRAY mode
and, after some delay, set the SR.7 READY bit.

Since we do not model timings, we set the SR.7 bit directly.

Fixes: 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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
[Laszlo Ersek: Regression tested EDK2 OVMF IA32X64, ArmVirtQemu Aarch64
 https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04373.html]
Message-Id: <20190718104837.13905-2-philmd@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 435be1e35c..a1ec1faae5 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -865,6 +865,24 @@ 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);
+
+    /*
+     * 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);
+    /*
+     * 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,
@@ -909,6 +927,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

[Qemu-devel] [PULL v2 2/2] hw/block/pflash_cfi02: Rewrite a fall through comment

[Philippe Mathieu-Daudé]

GCC9 is confused by this comment when building with CFLAG
-Wimplicit-fallthrough=2:

  hw/block/pflash_cfi02.c: In function ‘pflash_write’:
  hw/block/pflash_cfi02.c:574:16: error: this statement may fall through [-Werror=implicit-fallthrough=]
    574 |             if (boff == 0x55 && cmd == 0x98) {
        |                ^
  hw/block/pflash_cfi02.c:581:9: note: here
    581 |         default:
        |         ^~~~~~~
  cc1: all warnings being treated as errors

Rewrite the comment using 'fall through' which is recognized by
GCC and static analyzers.

Reported-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20190719131425.10835-4-philmd@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f68837a449..42886f6af5 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -577,7 +577,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
                 pfl->cmd = 0x98;
                 return;
             }
-            /* No break here */
+            /* fall through */
         default:
             DPRINTF("%s: invalid write for command %02x\n",
                     __func__, pfl->cmd);
-- 
2.20.1

Re: [Qemu-devel] [PULL v2 0/2] pflash-next patches for v4.1.0-rc2

[Peter Maydell]

On Tue, 23 Jul 2019 at 10:36, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> The following changes since commit 23da9e297b4120ca9702cabec91599a44255fe96:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190722' into staging (2019-07-22 15:16:48 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/philmd/qemu.git tags/pflash-next-20190723
>
> for you to fetch changes up to 124e4cfaa42bb5a14eec33ea47d3502b5f46eb33:
>
>   hw/block/pflash_cfi02: Rewrite a fall through comment (2019-07-23 11:31:07 +0200)
>
> ----------------------------------------------------------------
> One bugfix and silent a fallthru warning.
>
> ----------------------------------------------------------------
> v2: Replace Laszlo's Regression-tested-by tag
>
> Philippe Mathieu-Daudé (2):
>   hw/block/pflash_cfi01: Add missing DeviceReset() handler
>   hw/block/pflash_cfi02: Rewrite a fall through comment
>
>  hw/block/pflash_cfi01.c | 19 +++++++++++++++++++
>  hw/block/pflash_cfi02.c |  2 +-
>  2 files changed, 20 insertions(+), 1 deletion(-)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PULL v2 1/2] hw/block/pflash_cfi01: Add missing DeviceReset() handler

[Laszlo Ersek]

On 07/23/19 11:35, Philippe Mathieu-Daudé wrote:
> To avoid incoherent states when the machine resets (see bug report
> below), add the device reset callback.
> 
> A "system reset" sets the device state machine in READ_ARRAY mode
> and, after some delay, set the SR.7 READY bit.
> 
> Since we do not model timings, we set the SR.7 bit directly.
> 
> Fixes: 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>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> [Laszlo Ersek: Regression tested EDK2 OVMF IA32X64, ArmVirtQemu Aarch64
>  https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04373.html]

Nice, thanks! :)
Laszlo

> Message-Id: <20190718104837.13905-2-philmd@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/block/pflash_cfi01.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 435be1e35c..a1ec1faae5 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -865,6 +865,24 @@ 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);
> +
> +    /*
> +     * 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);
> +    /*
> +     * 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,
> @@ -909,6 +927,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;
>