[Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[Philippe Mathieu-Daudé]

The pflash device lacks a reset() function.
When a machine is resetted, the flash might be in an
inconsistent state, leading to unexpected behavior:
https://bugzilla.redhat.com/show_bug.cgi?id=1678713
Resolve this issue by adding a DeviceReset() handler.

Fix also two minor issues, and clean a bit the codebase.

Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00962.html
- addressed Laszlo review comments

Maintainers spam list from:
./scripts/get_maintainer.pl -f $(git grep -El '(pflash_cfi01_register|TYPE_PFLASH_CFI01)')

Regards,

Phil.

Philippe Mathieu-Daudé (9):
  hw/block/pflash_cfi01: Removed an unused timer
  hw/block/pflash_cfi01: Use the correct READ_ARRAY value
  hw/block/pflash_cfi01: Extract pflash_mode_read_array()
  hw/block/pflash_cfi01: Start state machine as READY to accept commands
  hw/block/pflash_cfi01: Add the DeviceReset() handler
  hw/block/pflash_cfi01: Simplify CFI_QUERY processing
  hw/block/pflash_cfi01: Improve command comments
  hw/block/pflash_cfi01: Replace DPRINTF by qemu_log_mask(GUEST_ERROR)
  hw/block/pflash_cfi01: Hold the PRI table offset in a variable

 hw/block/pflash_cfi01.c | 140 +++++++++++++++++++++-------------------
 hw/block/trace-events   |   1 +
 2 files changed, 74 insertions(+), 67 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v2 1/9] hw/block/pflash_cfi01: Removed an unused timer

[Philippe Mathieu-Daudé]

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>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Fixed commit description (Laszlo)
---
 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 35080d915f..dcc9885bf0 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.
@@ -775,7 +761,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 = 0;
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 1/9] hw/block/pflash_cfi01: Removed an unused timer

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:13 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> v2: Fixed commit description (Laszlo)
> ---
>  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 35080d915f..dcc9885bf0 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.
> @@ -775,7 +761,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 = 0;
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2 2/9] hw/block/pflash_cfi01: Use the correct READ_ARRAY value

[Philippe Mathieu-Daudé]

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

[*] "Common Flash Interface (CFI) and Command Sets"
    (Intel Application Note 646)
    Appendix B "Basic Command Set"

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index dcc9885bf0..743b5d5794 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -280,10 +280,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 */
@@ -449,8 +448,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__);
@@ -527,7 +524,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;
@@ -554,7 +551,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__);
@@ -646,7 +643,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;
 }
 
 
@@ -762,7 +759,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 */
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 2/9] hw/block/pflash_cfi01: Use the correct READ_ARRAY value

[John Snow]

On 7/1/19 8:12 PM, 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
> 
> [*] "Common Flash Interface (CFI) and Command Sets"
>     (Intel Application Note 646)
>     Appendix B "Basic Command Set"
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

I'm assuming you tested a guest and saw it sending 0xFF; and based
entirely on the assumption that you tested this and it is not now worse:

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  hw/block/pflash_cfi01.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index dcc9885bf0..743b5d5794 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -280,10 +280,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 */
> @@ -449,8 +448,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__);
> @@ -527,7 +524,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;
> @@ -554,7 +551,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__);
> @@ -646,7 +643,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;
>  }
>  
>  
> @@ -762,7 +759,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 */
> 

-- 
—js

Re: [Qemu-devel] [PATCH v2 2/9] hw/block/pflash_cfi01: Use the correct READ_ARRAY value

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:14 PM 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
>
> [*] "Common Flash Interface (CFI) and Command Sets"
>     (Intel Application Note 646)
>     Appendix B "Basic Command Set"
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/block/pflash_cfi01.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index dcc9885bf0..743b5d5794 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -280,10 +280,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 */
> @@ -449,8 +448,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__);
> @@ -527,7 +524,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;
> @@ -554,7 +551,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__);
> @@ -646,7 +643,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;
>  }
>
>
> @@ -762,7 +759,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 */
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2 3/9] hw/block/pflash_cfi01: Extract pflash_mode_read_array()

[Philippe Mathieu-Daudé]

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 twice:
- 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.

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 36 ++++++++++++++++++++----------------
 hw/block/trace-events   |  1 +
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 743b5d5794..33c77f6569 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -112,6 +112,14 @@ static const VMStateDescription vmstate_pflash = {
     }
 };
 
+static void pflash_mode_read_array(PFlashCFI01 *pfl)
+{
+    trace_pflash_mode_read_array();
+    pfl->cmd = 0xff; /* Read Array */
+    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.
@@ -470,7 +478,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;
@@ -495,10 +503,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;
+            goto mode_read_array;
         case 0xff: /* Read array mode */
             DPRINTF("%s: Read array mode\n", __func__);
-            goto reset_flash;
+            goto mode_read_array;
         default:
             goto error_flash;
         }
@@ -525,7 +533,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                 pfl->wcycle = 0;
                 pfl->status |= 0x80;
             } else if (cmd == 0xff) { /* Read Array */
-                goto reset_flash;
+                goto mode_read_array;
             } else
                 goto error_flash;
 
@@ -552,15 +560,15 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                 pfl->wcycle = 0;
                 pfl->status |= 0x80;
             } else if (cmd == 0xff) { /* read array mode */
-                goto reset_flash;
+                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;
+                goto mode_read_array;
             } else {
                 DPRINTF("%s: leaving query mode\n", __func__);
             }
@@ -620,7 +628,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:
@@ -630,7 +638,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;
 
@@ -639,11 +647,8 @@ 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:
-    trace_pflash_reset();
-    memory_region_rom_device_set_romd(&pfl->mem, true);
-    pfl->wcycle = 0;
-    pfl->cmd = 0xff;
+ mode_read_array:
+    pflash_mode_read_array(pfl);
 }
 
 
@@ -758,8 +763,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
         pfl->max_device_width = pfl->device_width;
     }
 
-    pfl->wcycle = 0;
-    pfl->cmd = 0xff;
+    pflash_mode_read_array(pfl);
     pfl->status = 0;
     /* Hardcoded CFI table */
     /* Standard "QRY" string */
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 97a17838ed..d627cfc3f5 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_read(uint64_t offset, uint8_t cmd, int width, uint8_t wcycle) "offset:0x%04"PRIx64" cmd:0x%02x width:%d wcycle:%u"
 pflash_write(uint64_t offset, uint32_t value, int width, uint8_t wcycle) "offset:0x%04"PRIx64" value:0x%03x width:%d wcycle:%u"
 pflash_timer_expired(uint8_t cmd) "command 0x%02x done"
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 3/9] hw/block/pflash_cfi01: Extract pflash_mode_read_array()

[John Snow]

On 7/1/19 8:12 PM, Philippe Mathieu-Daudé 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 twice:
> - 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.
> 
> 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.
> 

The rename does make it less confusing; but as of this patch it doesn't
seem obvious why you need this as a function, it's only really used
about "1.5 times" as of yet -- the realize call as you mention doesn't
quite exactly utilize it.

I will assume this comes in handy later, so:

Reviewed-by: John Snow <jsnow@redhat.com>

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/block/pflash_cfi01.c | 36 ++++++++++++++++++++----------------
>  hw/block/trace-events   |  1 +
>  2 files changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 743b5d5794..33c77f6569 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -112,6 +112,14 @@ static const VMStateDescription vmstate_pflash = {
>      }
>  };
>  
> +static void pflash_mode_read_array(PFlashCFI01 *pfl)
> +{
> +    trace_pflash_mode_read_array();
> +    pfl->cmd = 0xff; /* Read Array */
> +    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.
> @@ -470,7 +478,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;
> @@ -495,10 +503,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;
> +            goto mode_read_array;
>          case 0xff: /* Read array mode */
>              DPRINTF("%s: Read array mode\n", __func__);
> -            goto reset_flash;
> +            goto mode_read_array;
>          default:
>              goto error_flash;
>          }
> @@ -525,7 +533,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>                  pfl->wcycle = 0;
>                  pfl->status |= 0x80;
>              } else if (cmd == 0xff) { /* Read Array */
> -                goto reset_flash;
> +                goto mode_read_array;
>              } else
>                  goto error_flash;
>  
> @@ -552,15 +560,15 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>                  pfl->wcycle = 0;
>                  pfl->status |= 0x80;
>              } else if (cmd == 0xff) { /* read array mode */
> -                goto reset_flash;
> +                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;
> +                goto mode_read_array;
>              } else {
>                  DPRINTF("%s: leaving query mode\n", __func__);
>              }
> @@ -620,7 +628,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:
> @@ -630,7 +638,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;
>  
> @@ -639,11 +647,8 @@ 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:
> -    trace_pflash_reset();
> -    memory_region_rom_device_set_romd(&pfl->mem, true);
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0xff;
> + mode_read_array:
> +    pflash_mode_read_array(pfl);
>  }
>  
>  
> @@ -758,8 +763,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>          pfl->max_device_width = pfl->device_width;
>      }
>  
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0xff;
> +    pflash_mode_read_array(pfl);
>      pfl->status = 0;
>      /* Hardcoded CFI table */
>      /* Standard "QRY" string */
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index 97a17838ed..d627cfc3f5 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_read(uint64_t offset, uint8_t cmd, int width, uint8_t wcycle) "offset:0x%04"PRIx64" cmd:0x%02x width:%d wcycle:%u"
>  pflash_write(uint64_t offset, uint32_t value, int width, uint8_t wcycle) "offset:0x%04"PRIx64" value:0x%03x width:%d wcycle:%u"
>  pflash_timer_expired(uint8_t cmd) "command 0x%02x done"
> 

Re: [Qemu-devel] [PATCH v2 3/9] hw/block/pflash_cfi01: Extract pflash_mode_read_array()

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:14 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 twice:
> - 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.
>
> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/block/pflash_cfi01.c | 36 ++++++++++++++++++++----------------
>  hw/block/trace-events   |  1 +
>  2 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 743b5d5794..33c77f6569 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -112,6 +112,14 @@ static const VMStateDescription vmstate_pflash = {
>      }
>  };
>
> +static void pflash_mode_read_array(PFlashCFI01 *pfl)
> +{
> +    trace_pflash_mode_read_array();
> +    pfl->cmd = 0xff; /* Read Array */
> +    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.
> @@ -470,7 +478,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;
> @@ -495,10 +503,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;
> +            goto mode_read_array;
>          case 0xff: /* Read array mode */
>              DPRINTF("%s: Read array mode\n", __func__);
> -            goto reset_flash;
> +            goto mode_read_array;
>          default:
>              goto error_flash;
>          }
> @@ -525,7 +533,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>                  pfl->wcycle = 0;
>                  pfl->status |= 0x80;
>              } else if (cmd == 0xff) { /* Read Array */
> -                goto reset_flash;
> +                goto mode_read_array;
>              } else
>                  goto error_flash;
>
> @@ -552,15 +560,15 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>                  pfl->wcycle = 0;
>                  pfl->status |= 0x80;
>              } else if (cmd == 0xff) { /* read array mode */
> -                goto reset_flash;
> +                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;
> +                goto mode_read_array;
>              } else {
>                  DPRINTF("%s: leaving query mode\n", __func__);
>              }
> @@ -620,7 +628,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:
> @@ -630,7 +638,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;
>
> @@ -639,11 +647,8 @@ 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:
> -    trace_pflash_reset();
> -    memory_region_rom_device_set_romd(&pfl->mem, true);
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0xff;
> + mode_read_array:
> +    pflash_mode_read_array(pfl);
>  }
>
>
> @@ -758,8 +763,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>          pfl->max_device_width = pfl->device_width;
>      }
>
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0xff;
> +    pflash_mode_read_array(pfl);
>      pfl->status = 0;
>      /* Hardcoded CFI table */
>      /* Standard "QRY" string */
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index 97a17838ed..d627cfc3f5 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_read(uint64_t offset, uint8_t cmd, int width, uint8_t wcycle) "offset:0x%04"PRIx64" cmd:0x%02x width:%d wcycle:%u"
>  pflash_write(uint64_t offset, uint32_t value, int width, uint8_t wcycle) "offset:0x%04"PRIx64" value:0x%03x width:%d wcycle:%u"
>  pflash_timer_expired(uint8_t cmd) "command 0x%02x done"
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2 4/9] hw/block/pflash_cfi01: Start state machine as READY to accept commands

[Philippe Mathieu-Daudé]

When the state machine is ready to accept command, the bit 7 of
the status register (SR) is set to 1.
The guest polls the status register and check this bit before
writting command to the internal 'Write State Machine' (WSM).

Set SR.7 bit to 1 when the device is created.

Reference: Read Array Flowchart
  "Common Flash Interface (CFI) and Command Sets"
   (Intel Application Note 646)
   Appendix B "Basic Command Set"

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 33c77f6569..dd1dfd266b 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -764,7 +764,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     }
 
     pflash_mode_read_array(pfl);
-    pfl->status = 0;
+    pfl->status = 0x80; /* WSM ready */
     /* Hardcoded CFI table */
     /* Standard "QRY" string */
     pfl->cfi_table[0x10] = 'Q';
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 4/9] hw/block/pflash_cfi01: Start state machine as READY to accept commands

[John Snow]

On 7/1/19 8:12 PM, Philippe Mathieu-Daudé wrote:
> When the state machine is ready to accept command, the bit 7 of
> the status register (SR) is set to 1.
> The guest polls the status register and check this bit before
> writting command to the internal 'Write State Machine' (WSM).

writing

> 
> Set SR.7 bit to 1 when the device is created.
> 

Right, ok -- it looks like we don't set this on realize otherwise.
(Or if we do, it's not obvious where or how.)

> Reference: Read Array Flowchart
>   "Common Flash Interface (CFI) and Command Sets"
>    (Intel Application Note 646)
>    Appendix B "Basic Command Set"
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  hw/block/pflash_cfi01.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 33c77f6569..dd1dfd266b 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -764,7 +764,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>      }
>  
>      pflash_mode_read_array(pfl);
> -    pfl->status = 0;
> +    pfl->status = 0x80; /* WSM ready */
>      /* Hardcoded CFI table */
>      /* Standard "QRY" string */
>      pfl->cfi_table[0x10] = 'Q';
> 

Re: [Qemu-devel] [PATCH v2 4/9] hw/block/pflash_cfi01: Start state machine as READY to accept commands

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:14 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> When the state machine is ready to accept command, the bit 7 of
> the status register (SR) is set to 1.
> The guest polls the status register and check this bit before
> writting command to the internal 'Write State Machine' (WSM).
>
> Set SR.7 bit to 1 when the device is created.
>
> Reference: Read Array Flowchart
>   "Common Flash Interface (CFI) and Command Sets"
>    (Intel Application Note 646)
>    Appendix B "Basic Command Set"
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/block/pflash_cfi01.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 33c77f6569..dd1dfd266b 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -764,7 +764,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>      }
>
>      pflash_mode_read_array(pfl);
> -    pfl->status = 0;
> +    pfl->status = 0x80; /* WSM ready */
>      /* Hardcoded CFI table */
>      /* Standard "QRY" string */
>      pfl->cfi_table[0x10] = 'Q';
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2 5/9] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Philippe Mathieu-Daudé]

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.

This pflash device is a child of TYPE_DEVICE.
The TYPE_DEVICE interface provide a DeviceReset handler which will
be called after the device is realized, and each time the machine
resets itself.

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>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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 dd1dfd266b..8d632ea941 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -763,8 +763,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';
@@ -852,6 +850,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,
@@ -896,6 +906,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

Re: [Qemu-devel] [PATCH v2 5/9] hw/block/pflash_cfi01: Add the DeviceReset() handler

[John Snow]

On 7/1/19 8:12 PM, 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.
> 
> This pflash device is a child of TYPE_DEVICE.
> The TYPE_DEVICE interface provide a DeviceReset handler which will
> be called after the device is realized, and each time the machine
> resets itself.
> 
> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Does reset always get called as part of realize, really?

Or are we just trusting that the device is probably going to get reset
by the guest during bringup?

> ---
>  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 dd1dfd266b..8d632ea941 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -763,8 +763,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';
> @@ -852,6 +850,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,
> @@ -896,6 +906,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;
> 

Re: [Qemu-devel] [PATCH v2 5/9] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Peter Maydell]

On Tue, 2 Jul 2019 at 04:16, John Snow <jsnow@redhat.com> wrote:
> Does reset always get called as part of realize, really?
>
> Or are we just trusting that the device is probably going to get reset
> by the guest during bringup?

Reset is not called "as part of realize", but it is guaranteed
to be called after realize and before we try to run the guest,
as long as the device is in the qbus tree. Things are in the
qbus tree if either:
 * they're plugged into something already in the tree (eg
   pci devices, scsi disks)
 * they're a sysbus device (which is automatically plugged into
   the 'main system bus' which is effectively the root of the
   qbus tree)

In this case TYPE_PFLASH_CFI01 is a subclass of TYPE_SYS_BUS_DEVICE,
so it will always be reset as part of system reset.

(the main things which don't get automatically reset are direct
subclasses of TYPE_DEVICE, notably CPU objects.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 5/9] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Philippe Mathieu-Daudé]

On 7/2/19 11:23 AM, Peter Maydell wrote:
> On Tue, 2 Jul 2019 at 04:16, John Snow <jsnow@redhat.com> wrote:
>> Does reset always get called as part of realize, really?
>>
>> Or are we just trusting that the device is probably going to get reset
>> by the guest during bringup?
> 
> Reset is not called "as part of realize", but it is guaranteed
> to be called after realize and before we try to run the guest,
> as long as the device is in the qbus tree. Things are in the
> qbus tree if either:
>  * they're plugged into something already in the tree (eg
>    pci devices, scsi disks)
>  * they're a sysbus device (which is automatically plugged into
>    the 'main system bus' which is effectively the root of the
>    qbus tree)
> 
> In this case TYPE_PFLASH_CFI01 is a subclass of TYPE_SYS_BUS_DEVICE,
> so it will always be reset as part of system reset.
> 
> (the main things which don't get automatically reset are direct
> subclasses of TYPE_DEVICE, notably CPU objects.)

Thanks for the clarification!

I will update the commit description paraphrasing Peter:

  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.

John, Laszlo, is that OK with you?

Thanks,

Phil.

Re: [Qemu-devel] [PATCH v2 5/9] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Laszlo Ersek]

On 07/02/19 11:37, Philippe Mathieu-Daudé wrote:
> On 7/2/19 11:23 AM, Peter Maydell wrote:
>> On Tue, 2 Jul 2019 at 04:16, John Snow <jsnow@redhat.com> wrote:
>>> Does reset always get called as part of realize, really?
>>>
>>> Or are we just trusting that the device is probably going to get reset
>>> by the guest during bringup?
>>
>> Reset is not called "as part of realize", but it is guaranteed
>> to be called after realize and before we try to run the guest,
>> as long as the device is in the qbus tree. Things are in the
>> qbus tree if either:
>>  * they're plugged into something already in the tree (eg
>>    pci devices, scsi disks)
>>  * they're a sysbus device (which is automatically plugged into
>>    the 'main system bus' which is effectively the root of the
>>    qbus tree)
>>
>> In this case TYPE_PFLASH_CFI01 is a subclass of TYPE_SYS_BUS_DEVICE,
>> so it will always be reset as part of system reset.
>>
>> (the main things which don't get automatically reset are direct
>> subclasses of TYPE_DEVICE, notably CPU objects.)
> 
> Thanks for the clarification!
> 
> I will update the commit description paraphrasing Peter:
> 
>   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.
> 
> John, Laszlo, is that OK with you?

works for me, sure.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 5/9] hw/block/pflash_cfi01: Add the DeviceReset() handler

[John Snow]

On 7/2/19 5:23 AM, Peter Maydell wrote:
> On Tue, 2 Jul 2019 at 04:16, John Snow <jsnow@redhat.com> wrote:
>> Does reset always get called as part of realize, really?
>>
>> Or are we just trusting that the device is probably going to get reset
>> by the guest during bringup?
> 
> Reset is not called "as part of realize", but it is guaranteed
> to be called after realize and before we try to run the guest,
> as long as the device is in the qbus tree. Things are in the
> qbus tree if either:
>  * they're plugged into something already in the tree (eg
>    pci devices, scsi disks)
>  * they're a sysbus device (which is automatically plugged into
>    the 'main system bus' which is effectively the root of the
>    qbus tree)
> 
> In this case TYPE_PFLASH_CFI01 is a subclass of TYPE_SYS_BUS_DEVICE,
> so it will always be reset as part of system reset.
> 
> (the main things which don't get automatically reset are direct
> subclasses of TYPE_DEVICE, notably CPU objects.)
> 
> thanks
> -- PMM
> 

Thank you for the lesson, that makes sense -- I saw the hotplug clause,
but missed the sysbus routine. Thanks to Laszlo for pointing this out in
great detail, too.

Phil, a clarified commit message is perfectly sufficient, and please
take my RB.

Re: [Qemu-devel] [PATCH v2 5/9] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Laszlo Ersek]

On 07/02/19 05:16, John Snow wrote:
> 
> 
> On 7/1/19 8:12 PM, 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.
>>
>> This pflash device is a child of TYPE_DEVICE.
>> The TYPE_DEVICE interface provide a DeviceReset handler which will
>> be called after the device is realized, and each time the machine
>> resets itself.
>>
>> 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>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Does reset always get called as part of realize, really?

I'm not an expert on this, but I believe the following happens.

(1) When a device is hotplugged, the device is reset as part of realize
in the following call stack:

device_set_realized() [hw/core/qdev.c]
  device_reset()      [hw/core/qdev.c]


(2) For when the device is cold-plugged, we have, in main()
[hw/core/qdev.c]:

    /* TODO: once all bus devices are qdevified, this should be done
     * when bus is created by qdev.c */
    qemu_register_reset(qbus_reset_all_fn, sysbus_get_default());

and then a bit later

    qemu_system_reset(SHUTDOWN_CAUSE_NONE);

AIUI it results in a recursive traversal / reset,

qbus_reset_all_fn()      [hw/core/qdev.c]
  qbus_reset_all()       [hw/core/qdev.c]
    qbus_walk_children() [hw/core/bus.c]
      qdev_reset_one()   [hw/core/qdev.c]
        device_reset()   [hw/core/qdev.c]


In other words, I think you have a point about clarifying the commit
message. It's likely not that the device is re-set (a) after it is
realized *and* (b) each time the machine is re-set. Because, pflash is
not hot-plugged, so reset-on-hotplug does not apply, and so case (a)
falls away. Instead, case (b), "each time the machine is re-set",
includes the initial launch of the machine, and that is why pflash is
ultimately re-set at startup. It does happen after realize, but not as
part of it.

Anyway, Phil can verify this easily: just set a breakpoint on
pflash_cfi01_system_reset() in gdb before launching qemu, and get a
backtrace once the breakpoint fires. :)

Thanks
Laszlo

> Or are we just trusting that the device is probably going to get reset
> by the guest during bringup?
> 
>> ---
>>  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 dd1dfd266b..8d632ea941 100644
>> --- a/hw/block/pflash_cfi01.c
>> +++ b/hw/block/pflash_cfi01.c
>> @@ -763,8 +763,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';
>> @@ -852,6 +850,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,
>> @@ -896,6 +906,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;
>>
> 

[Qemu-devel] [PATCH v2 6/9] hw/block/pflash_cfi01: Simplify CFI_QUERY processing

[Philippe Mathieu-Daudé]

The current code does:

if (write_cycle == 0)
  if (command == CFI_QUERY)
    break
  write_cycle += 1
  last_command = command

if (write_cycle == 1)
  if (last_command == CFI_QUERY)
    if (command == READ_ARRAY
      write_cycle = 0
      last_command = READ_ARRAY

Simplify by not increasing the write_cycle on CFI_QUERY,
the next command are processed as normal wcycle=0.

This matches the hardware datasheet (we do not enter the WRITE
state machine, thus no write cycle involved).

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 8d632ea941..c1b02219b2 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -492,7 +492,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             return;
         case 0x98: /* CFI query */
             DPRINTF("%s: CFI query\n", __func__);
-            break;
+            pfl->cmd = cmd;
+            return;
         case 0xe8: /* Write to buffer */
             DPRINTF("%s: Write to buffer\n", __func__);
             /* FIXME should save @offset, @width for case 1+ */
@@ -566,13 +567,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                 goto mode_read_array;
             }
             break;
-        case 0x98:
-            if (cmd == 0xff) {
-                goto mode_read_array;
-            } else {
-                DPRINTF("%s: leaving query mode\n", __func__);
-            }
-            break;
         default:
             goto error_flash;
         }
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 6/9] hw/block/pflash_cfi01: Simplify CFI_QUERY processing

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:15 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> The current code does:
>
> if (write_cycle == 0)
>   if (command == CFI_QUERY)
>     break
>   write_cycle += 1
>   last_command = command
>
> if (write_cycle == 1)
>   if (last_command == CFI_QUERY)
>     if (command == READ_ARRAY
>       write_cycle = 0
>       last_command = READ_ARRAY
>
> Simplify by not increasing the write_cycle on CFI_QUERY,
> the next command are processed as normal wcycle=0.
>
> This matches the hardware datasheet (we do not enter the WRITE
> state machine, thus no write cycle involved).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/block/pflash_cfi01.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 8d632ea941..c1b02219b2 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -492,7 +492,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>              return;
>          case 0x98: /* CFI query */
>              DPRINTF("%s: CFI query\n", __func__);
> -            break;
> +            pfl->cmd = cmd;
> +            return;
>          case 0xe8: /* Write to buffer */
>              DPRINTF("%s: Write to buffer\n", __func__);
>              /* FIXME should save @offset, @width for case 1+ */
> @@ -566,13 +567,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>                  goto mode_read_array;
>              }
>              break;
> -        case 0x98:
> -            if (cmd == 0xff) {
> -                goto mode_read_array;
> -            } else {
> -                DPRINTF("%s: leaving query mode\n", __func__);
> -            }
> -            break;
>          default:
>              goto error_flash;
>          }
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2 7/9] hw/block/pflash_cfi01: Improve command comments

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index c1b02219b2..f50d0a9d37 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -208,11 +208,11 @@ static uint32_t pflash_devid_query(PFlashCFI01 *pfl, hwaddr offset)
      * Offsets 2/3 are block lock status, is not emulated.
      */
     switch (boff & 0xFF) {
-    case 0:
+    case 0: /* Manufacturer ID */
         resp = pfl->ident0;
         trace_pflash_manufacturer_id(resp);
         break;
-    case 1:
+    case 1: /* Device ID */
         resp = pfl->ident1;
         trace_pflash_device_id(resp);
         break;
@@ -456,11 +456,11 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
     case 0:
         /* read mode */
         switch (cmd) {
-        case 0x10: /* Single Byte Program */
-        case 0x40: /* Single Byte Program */
+        case 0x10: /* Single Byte Program (setup) */
+        case 0x40: /* Single Byte Program (setup) [Intel] */
             DPRINTF("%s: Single Byte Program\n", __func__);
             break;
-        case 0x20: /* Block erase */
+        case 0x20: /* Block erase (setup) */
             p = pfl->storage;
             offset &= ~(pfl->sector_len - 1);
 
@@ -516,8 +516,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
         break;
     case 1:
         switch (pfl->cmd) {
-        case 0x10: /* Single Byte Program */
-        case 0x40: /* Single Byte Program */
+        case 0x10: /* Single Byte Program (confirm) */
+        case 0x40: /* Single Byte Program (confirm) [Intel] */
             DPRINTF("%s: Single Byte Program\n", __func__);
             if (!pfl->ro) {
                 pflash_data_write(pfl, offset, value, width, be);
@@ -528,7 +528,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             pfl->status |= 0x80; /* Ready! */
             pfl->wcycle = 0;
         break;
-        case 0x20: /* Block erase */
+        case 0x20: /* Block erase (confirm) */
         case 0x28:
             if (cmd == 0xd0) { /* confirm */
                 pfl->wcycle = 0;
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 7/9] hw/block/pflash_cfi01: Improve command comments

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:15 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/block/pflash_cfi01.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index c1b02219b2..f50d0a9d37 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -208,11 +208,11 @@ static uint32_t pflash_devid_query(PFlashCFI01 *pfl, hwaddr offset)
>       * Offsets 2/3 are block lock status, is not emulated.
>       */
>      switch (boff & 0xFF) {
> -    case 0:
> +    case 0: /* Manufacturer ID */
>          resp = pfl->ident0;
>          trace_pflash_manufacturer_id(resp);
>          break;
> -    case 1:
> +    case 1: /* Device ID */
>          resp = pfl->ident1;
>          trace_pflash_device_id(resp);
>          break;
> @@ -456,11 +456,11 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>      case 0:
>          /* read mode */
>          switch (cmd) {
> -        case 0x10: /* Single Byte Program */
> -        case 0x40: /* Single Byte Program */
> +        case 0x10: /* Single Byte Program (setup) */
> +        case 0x40: /* Single Byte Program (setup) [Intel] */
>              DPRINTF("%s: Single Byte Program\n", __func__);
>              break;
> -        case 0x20: /* Block erase */
> +        case 0x20: /* Block erase (setup) */
>              p = pfl->storage;
>              offset &= ~(pfl->sector_len - 1);
>
> @@ -516,8 +516,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>          break;
>      case 1:
>          switch (pfl->cmd) {
> -        case 0x10: /* Single Byte Program */
> -        case 0x40: /* Single Byte Program */
> +        case 0x10: /* Single Byte Program (confirm) */
> +        case 0x40: /* Single Byte Program (confirm) [Intel] */
>              DPRINTF("%s: Single Byte Program\n", __func__);
>              if (!pfl->ro) {
>                  pflash_data_write(pfl, offset, value, width, be);
> @@ -528,7 +528,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>              pfl->status |= 0x80; /* Ready! */
>              pfl->wcycle = 0;
>          break;
> -        case 0x20: /* Block erase */
> +        case 0x20: /* Block erase (confirm) */
>          case 0x28:
>              if (cmd == 0xd0) { /* confirm */
>                  pfl->wcycle = 0;
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2 8/9] hw/block/pflash_cfi01: Replace DPRINTF by qemu_log_mask(GUEST_ERROR)

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index f50d0a9d37..e891112b67 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -286,7 +286,9 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
     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);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid state, "
+                      "wcycle %d cmd 0x02%x\n",
+                      __func__, pfl->wcycle, pfl->cmd);
         pfl->wcycle = 0;
         pfl->cmd = 0xff;
         /* fall through to read code */
@@ -631,7 +633,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
         break;
     default:
         /* Should never happen */
-        DPRINTF("%s: invalid write state\n",  __func__);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid state, "
+                      "wcycle %d cmd (0x02%x -> value 0x02%x)\n",
+                      __func__, pfl->wcycle, pfl->cmd, value);
         goto mode_read_array;
     }
     return;
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 8/9] hw/block/pflash_cfi01: Replace DPRINTF by qemu_log_mask(GUEST_ERROR)

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:16 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/block/pflash_cfi01.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index f50d0a9d37..e891112b67 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -286,7 +286,9 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
>      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);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid state, "
> +                      "wcycle %d cmd 0x02%x\n",
> +                      __func__, pfl->wcycle, pfl->cmd);
>          pfl->wcycle = 0;
>          pfl->cmd = 0xff;
>          /* fall through to read code */
> @@ -631,7 +633,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>          break;
>      default:
>          /* Should never happen */
> -        DPRINTF("%s: invalid write state\n",  __func__);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid state, "
> +                      "wcycle %d cmd (0x02%x -> value 0x02%x)\n",
> +                      __func__, pfl->wcycle, pfl->cmd, value);
>          goto mode_read_array;
>      }
>      return;
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2 9/9] hw/block/pflash_cfi01: Hold the PRI table offset in a variable

[Philippe Mathieu-Daudé]

Manufacturers are allowed to move the PRI table, this is why the
offset is queryable via fixed offsets 0x15/0x16.
Add a variable to hold the offset, so it will be easier to later
move the PRI table.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index e891112b67..f65840eb2b 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -762,6 +762,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     }
 
     /* Hardcoded CFI table */
+    const uint16_t pri_ofs = 0x31;
     /* Standard "QRY" string */
     pfl->cfi_table[0x10] = 'Q';
     pfl->cfi_table[0x11] = 'R';
@@ -770,14 +771,17 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     pfl->cfi_table[0x13] = 0x01;
     pfl->cfi_table[0x14] = 0x00;
     /* Primary extended table address (none) */
-    pfl->cfi_table[0x15] = 0x31;
-    pfl->cfi_table[0x16] = 0x00;
+    pfl->cfi_table[0x15] = pri_ofs;
+    pfl->cfi_table[0x16] = pri_ofs >> 8;
     /* Alternate command set (none) */
     pfl->cfi_table[0x17] = 0x00;
     pfl->cfi_table[0x18] = 0x00;
     /* Alternate extended table (none) */
     pfl->cfi_table[0x19] = 0x00;
     pfl->cfi_table[0x1A] = 0x00;
+
+    /* CFI: System Interface Information */
+
     /* Vcc min */
     pfl->cfi_table[0x1B] = 0x45;
     /* Vcc max */
@@ -802,6 +806,9 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     pfl->cfi_table[0x25] = 0x04;
     /* Max timeout for chip erase */
     pfl->cfi_table[0x26] = 0x00;
+
+    /* CFI: Device Geometry Definition */
+
     /* Device size */
     pfl->cfi_table[0x27] = ctz32(device_len); /* + 1; */
     /* Flash device interface (8 & 16 bits) */
@@ -826,26 +833,30 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     pfl->cfi_table[0x2E] = (blocks_per_device - 1) >> 8;
     pfl->cfi_table[0x2F] = sector_len_per_device >> 8;
     pfl->cfi_table[0x30] = sector_len_per_device >> 16;
+    assert(0x30 < pri_ofs);
+
+    /* CFI: Primary-Vendor Specific */
 
     /* Extended */
-    pfl->cfi_table[0x31] = 'P';
-    pfl->cfi_table[0x32] = 'R';
-    pfl->cfi_table[0x33] = 'I';
+    pfl->cfi_table[0x00 + pri_ofs] = 'P';
+    pfl->cfi_table[0x01 + pri_ofs] = 'R';
+    pfl->cfi_table[0x02 + pri_ofs] = 'I';
 
-    pfl->cfi_table[0x34] = '1';
-    pfl->cfi_table[0x35] = '0';
+    pfl->cfi_table[0x03 + pri_ofs] = '1';
+    pfl->cfi_table[0x04 + pri_ofs] = '0';
 
-    pfl->cfi_table[0x36] = 0x00;
-    pfl->cfi_table[0x37] = 0x00;
-    pfl->cfi_table[0x38] = 0x00;
-    pfl->cfi_table[0x39] = 0x00;
+    pfl->cfi_table[0x05 + pri_ofs] = 0x00; /* Optional features */
+    pfl->cfi_table[0x06 + pri_ofs] = 0x00;
+    pfl->cfi_table[0x07 + pri_ofs] = 0x00;
+    pfl->cfi_table[0x08 + pri_ofs] = 0x00;
 
-    pfl->cfi_table[0x3a] = 0x00;
+    pfl->cfi_table[0x09 + pri_ofs] = 0x00; /* Func. supported after suspend */
 
-    pfl->cfi_table[0x3b] = 0x00;
-    pfl->cfi_table[0x3c] = 0x00;
+    pfl->cfi_table[0x0a + pri_ofs] = 0x00; /* Block status register mask */
+    pfl->cfi_table[0x0b + pri_ofs] = 0x00;
 
-    pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
+    pfl->cfi_table[0x0e + pri_ofs] = 0x01; /* Number of protection fields */
+    assert(0x0e + pri_ofs < ARRAY_SIZE(pfl->cfi_table));
 }
 
 static void pflash_cfi01_system_reset(DeviceState *dev)
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 9/9] hw/block/pflash_cfi01: Hold the PRI table offset in a variable

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:16 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Manufacturers are allowed to move the PRI table, this is why the
> offset is queryable via fixed offsets 0x15/0x16.
> Add a variable to hold the offset, so it will be easier to later
> move the PRI table.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/block/pflash_cfi01.c | 41 ++++++++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index e891112b67..f65840eb2b 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -762,6 +762,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>      }
>
>      /* Hardcoded CFI table */
> +    const uint16_t pri_ofs = 0x31;
>      /* Standard "QRY" string */
>      pfl->cfi_table[0x10] = 'Q';
>      pfl->cfi_table[0x11] = 'R';
> @@ -770,14 +771,17 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>      pfl->cfi_table[0x13] = 0x01;
>      pfl->cfi_table[0x14] = 0x00;
>      /* Primary extended table address (none) */
> -    pfl->cfi_table[0x15] = 0x31;
> -    pfl->cfi_table[0x16] = 0x00;
> +    pfl->cfi_table[0x15] = pri_ofs;
> +    pfl->cfi_table[0x16] = pri_ofs >> 8;
>      /* Alternate command set (none) */
>      pfl->cfi_table[0x17] = 0x00;
>      pfl->cfi_table[0x18] = 0x00;
>      /* Alternate extended table (none) */
>      pfl->cfi_table[0x19] = 0x00;
>      pfl->cfi_table[0x1A] = 0x00;
> +
> +    /* CFI: System Interface Information */
> +
>      /* Vcc min */
>      pfl->cfi_table[0x1B] = 0x45;
>      /* Vcc max */
> @@ -802,6 +806,9 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>      pfl->cfi_table[0x25] = 0x04;
>      /* Max timeout for chip erase */
>      pfl->cfi_table[0x26] = 0x00;
> +
> +    /* CFI: Device Geometry Definition */
> +
>      /* Device size */
>      pfl->cfi_table[0x27] = ctz32(device_len); /* + 1; */
>      /* Flash device interface (8 & 16 bits) */
> @@ -826,26 +833,30 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
>      pfl->cfi_table[0x2E] = (blocks_per_device - 1) >> 8;
>      pfl->cfi_table[0x2F] = sector_len_per_device >> 8;
>      pfl->cfi_table[0x30] = sector_len_per_device >> 16;
> +    assert(0x30 < pri_ofs);
> +
> +    /* CFI: Primary-Vendor Specific */
>
>      /* Extended */
> -    pfl->cfi_table[0x31] = 'P';
> -    pfl->cfi_table[0x32] = 'R';
> -    pfl->cfi_table[0x33] = 'I';
> +    pfl->cfi_table[0x00 + pri_ofs] = 'P';
> +    pfl->cfi_table[0x01 + pri_ofs] = 'R';
> +    pfl->cfi_table[0x02 + pri_ofs] = 'I';
>
> -    pfl->cfi_table[0x34] = '1';
> -    pfl->cfi_table[0x35] = '0';
> +    pfl->cfi_table[0x03 + pri_ofs] = '1';
> +    pfl->cfi_table[0x04 + pri_ofs] = '0';
>
> -    pfl->cfi_table[0x36] = 0x00;
> -    pfl->cfi_table[0x37] = 0x00;
> -    pfl->cfi_table[0x38] = 0x00;
> -    pfl->cfi_table[0x39] = 0x00;
> +    pfl->cfi_table[0x05 + pri_ofs] = 0x00; /* Optional features */
> +    pfl->cfi_table[0x06 + pri_ofs] = 0x00;
> +    pfl->cfi_table[0x07 + pri_ofs] = 0x00;
> +    pfl->cfi_table[0x08 + pri_ofs] = 0x00;
>
> -    pfl->cfi_table[0x3a] = 0x00;
> +    pfl->cfi_table[0x09 + pri_ofs] = 0x00; /* Func. supported after suspend */
>
> -    pfl->cfi_table[0x3b] = 0x00;
> -    pfl->cfi_table[0x3c] = 0x00;
> +    pfl->cfi_table[0x0a + pri_ofs] = 0x00; /* Block status register mask */
> +    pfl->cfi_table[0x0b + pri_ofs] = 0x00;
>
> -    pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
> +    pfl->cfi_table[0x0e + pri_ofs] = 0x01; /* Number of protection fields */
> +    assert(0x0e + pri_ofs < ARRAY_SIZE(pfl->cfi_table));
>  }
>
>  static void pflash_cfi01_system_reset(DeviceState *dev)
> --
> 2.20.1
>

Re: [Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190702001301.4768-1-philmd@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 check-qlit /qlit/equal_qobject
PASS 2 check-qlit /qlit/qobject_from_qlit
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qobject-output-visitor -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qobject-output-visitor" 
==7795==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-coroutine" 
==7839==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==7839==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffebcfa2000; bottom 0x7f75214f8000; size: 0x00899baaa000 (591022170112)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
==7855==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 28 test-aio /aio-gsource/timer/schedule
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-aio-multithread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==7862==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" 
PASS 2 test-aio-multithread /aio/multi/schedule
==7879==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==7890==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==7901==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==7907==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==7918==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
==7929==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-throttle" 
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
---
PASS 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
PASS 8 test-throttle /throttle/accounting
==7936==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 test-throttle /throttle/groups
PASS 10 test-throttle /throttle/config/enabled
PASS 11 test-throttle /throttle/config/conflicting
---
PASS 15 test-throttle /throttle/config/iops_size
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-thread-pool" 
==7943==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==7941==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 ide-test /x86_64/ide/bmdma/long_prdt
==8015==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8015==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffee4038000; bottom 0x7f65bd5fe000; size: 0x009926a3a000 (657778253824)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 5 test-thread-pool /thread-pool/cancel
PASS 8 ide-test /x86_64/ide/bmdma/no_busmaster
PASS 9 ide-test /x86_64/ide/flush/nodev
PASS 6 test-thread-pool /thread-pool/cancel-async
==8026==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-hbitmap" 
PASS 10 ide-test /x86_64/ide/flush/empty_drive
PASS 1 test-hbitmap /hbitmap/granularity
PASS 2 test-hbitmap /hbitmap/size/0
PASS 3 test-hbitmap /hbitmap/size/unaligned
PASS 4 test-hbitmap /hbitmap/iter/empty
==8037==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-hbitmap /hbitmap/iter/partial
PASS 6 test-hbitmap /hbitmap/iter/granularity
PASS 7 test-hbitmap /hbitmap/iter/iter_and_reset
---
PASS 12 test-hbitmap /hbitmap/set/two-elem
PASS 13 test-hbitmap /hbitmap/set/general
PASS 14 test-hbitmap /hbitmap/set/twice
==8043==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 test-hbitmap /hbitmap/set/overlap
PASS 16 test-hbitmap /hbitmap/reset/empty
PASS 12 ide-test /x86_64/ide/flush/retry_isa
---
PASS 28 test-hbitmap /hbitmap/truncate/shrink/medium
PASS 29 test-hbitmap /hbitmap/truncate/shrink/large
PASS 30 test-hbitmap /hbitmap/meta/zero
==8049==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 ide-test /x86_64/ide/cdrom/pio
==8055==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 ide-test /x86_64/ide/cdrom/pio_large
==8061==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 ide-test /x86_64/ide/cdrom/dma
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ahci-test" 
PASS 31 test-hbitmap /hbitmap/meta/one
PASS 32 test-hbitmap /hbitmap/meta/byte
PASS 33 test-hbitmap /hbitmap/meta/word
==8075==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 34 test-hbitmap /hbitmap/meta/sector
PASS 35 test-hbitmap /hbitmap/serialize/align
PASS 1 ahci-test /x86_64/ahci/sanity
==8081==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 ahci-test /x86_64/ahci/pci_spec
PASS 36 test-hbitmap /hbitmap/serialize/basic
PASS 37 test-hbitmap /hbitmap/serialize/part
---
PASS 42 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_1
PASS 43 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_4
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bdrv-drain -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-drain" 
==8087==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8090==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-drain /bdrv-drain/nested
PASS 2 test-bdrv-drain /bdrv-drain/multiparent
PASS 3 test-bdrv-drain /bdrv-drain/set_aio_context
---
PASS 16 test-bdrv-drain /bdrv-drain/graph-change/drain_subtree
PASS 17 test-bdrv-drain /bdrv-drain/graph-change/drain_all
=================================================================
==8090==ERROR: AddressSanitizer: heap-use-after-free on address 0x61200002c1f0 at pc 0x559638e7e006 bp 0x7f974eab8680 sp 0x7f974eab8678
WRITE of size 1 at 0x61200002c1f0 thread T5
PASS 3 ahci-test /x86_64/ahci/pci_enable
    #0 0x559638e7e005 in aio_notify /tmp/qemu-test/src/util/async.c:351:9
---
  Right alloca redzone:    cb
  Shadow gap:              cc
==8090==ABORTING
ERROR - too few tests run (expected 39, got 17)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:899: check-unit] Error 1
make: *** Waiting for unfinished jobs....
==8103==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 ahci-test /x86_64/ahci/hba_spec
==8109==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 ahci-test /x86_64/ahci/hba_enable
==8115==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 ahci-test /x86_64/ahci/identify
==8121==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 ahci-test /x86_64/ahci/max
==8127==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 ahci-test /x86_64/ahci/reset
==8133==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8133==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff77275000; bottom 0x7fb8fabfe000; size: 0x00467c677000 (302734864384)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ahci-test /x86_64/ahci/io/pio/lba28/simple/zero
==8139==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8139==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff83f3a000; bottom 0x7f6cf29fe000; size: 0x00929153c000 (629503410176)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 10 ahci-test /x86_64/ahci/io/pio/lba28/simple/low
==8145==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8145==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff77704000; bottom 0x7fdea05fe000; size: 0x0020d7106000 (141047128064)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 11 ahci-test /x86_64/ahci/io/pio/lba28/simple/high
==8151==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8151==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff0fbb6000; bottom 0x7f49a29fe000; size: 0x00b56d1b8000 (779219599360)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 12 ahci-test /x86_64/ahci/io/pio/lba28/double/zero
==8157==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8157==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcfd2ea000; bottom 0x7f4c64dfe000; size: 0x00b0984ec000 (758469541888)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 13 ahci-test /x86_64/ahci/io/pio/lba28/double/low
==8163==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8163==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffb8526000; bottom 0x7fd664dfe000; size: 0x002953728000 (177493671936)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 14 ahci-test /x86_64/ahci/io/pio/lba28/double/high
==8169==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8169==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffe54eb000; bottom 0x7f6c6af24000; size: 0x00937a5c7000 (633413070848)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 15 ahci-test /x86_64/ahci/io/pio/lba28/long/zero
==8175==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8175==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc2dbce000; bottom 0x7f12bf9fe000; size: 0x00e96e1d0000 (1002574774272)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 16 ahci-test /x86_64/ahci/io/pio/lba28/long/low
==8181==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8181==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff4bc34000; bottom 0x7efcb837c000; size: 0x0102938b8000 (1110576955392)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 17 ahci-test /x86_64/ahci/io/pio/lba28/long/high
==8187==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 18 ahci-test /x86_64/ahci/io/pio/lba28/short/zero
==8193==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 19 ahci-test /x86_64/ahci/io/pio/lba28/short/low
==8199==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 20 ahci-test /x86_64/ahci/io/pio/lba28/short/high
==8205==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8205==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcec3df000; bottom 0x7f556a9fe000; size: 0x00a7819e1000 (719434158080)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 21 ahci-test /x86_64/ahci/io/pio/lba48/simple/zero
==8211==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8211==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc1c278000; bottom 0x7f3350ffe000; size: 0x00c8cb27a000 (862401830912)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 22 ahci-test /x86_64/ahci/io/pio/lba48/simple/low
==8217==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8217==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffdbc7f000; bottom 0x7fa8397fe000; size: 0x0057a2481000 (376384786432)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 23 ahci-test /x86_64/ahci/io/pio/lba48/simple/high
==8223==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8223==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff494d4000; bottom 0x7fe9b09fe000; size: 0x001598ad6000 (92755812352)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 24 ahci-test /x86_64/ahci/io/pio/lba48/double/zero
==8229==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8229==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffffa16e000; bottom 0x7f953d3fe000; size: 0x006abcd70000 (458434740224)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 25 ahci-test /x86_64/ahci/io/pio/lba48/double/low
==8235==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8235==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcb3a63000; bottom 0x7fdf093fe000; size: 0x001daa665000 (127412883456)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 26 ahci-test /x86_64/ahci/io/pio/lba48/double/high
==8241==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8241==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe9ddf4000; bottom 0x7f0950bfe000; size: 0x00f54d1f6000 (1053560889344)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 27 ahci-test /x86_64/ahci/io/pio/lba48/long/zero
==8247==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8247==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffc0df3000; bottom 0x7f305d524000; size: 0x00cf638cf000 (890728411136)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 28 ahci-test /x86_64/ahci/io/pio/lba48/long/low
==8253==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8253==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffde2748000; bottom 0x7f348b97c000; size: 0x00c956dcc000 (864745734144)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 29 ahci-test /x86_64/ahci/io/pio/lba48/long/high
==8259==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 30 ahci-test /x86_64/ahci/io/pio/lba48/short/zero
==8265==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 31 ahci-test /x86_64/ahci/io/pio/lba48/short/low
==8271==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 32 ahci-test /x86_64/ahci/io/pio/lba48/short/high
==8277==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 33 ahci-test /x86_64/ahci/io/dma/lba28/fragmented
==8283==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 34 ahci-test /x86_64/ahci/io/dma/lba28/retry
==8289==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 35 ahci-test /x86_64/ahci/io/dma/lba28/simple/zero
==8295==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 36 ahci-test /x86_64/ahci/io/dma/lba28/simple/low
==8301==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 37 ahci-test /x86_64/ahci/io/dma/lba28/simple/high
==8307==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 38 ahci-test /x86_64/ahci/io/dma/lba28/double/zero
==8313==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 39 ahci-test /x86_64/ahci/io/dma/lba28/double/low
==8319==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 40 ahci-test /x86_64/ahci/io/dma/lba28/double/high
==8325==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 41 ahci-test /x86_64/ahci/io/dma/lba28/long/zero
==8331==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 42 ahci-test /x86_64/ahci/io/dma/lba28/long/low
==8337==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 43 ahci-test /x86_64/ahci/io/dma/lba28/long/high
==8343==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 44 ahci-test /x86_64/ahci/io/dma/lba28/short/zero
==8349==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 45 ahci-test /x86_64/ahci/io/dma/lba28/short/low
==8355==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 46 ahci-test /x86_64/ahci/io/dma/lba28/short/high
==8361==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 47 ahci-test /x86_64/ahci/io/dma/lba48/simple/zero
==8367==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 48 ahci-test /x86_64/ahci/io/dma/lba48/simple/low
==8373==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 49 ahci-test /x86_64/ahci/io/dma/lba48/simple/high
==8379==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 50 ahci-test /x86_64/ahci/io/dma/lba48/double/zero
==8385==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 51 ahci-test /x86_64/ahci/io/dma/lba48/double/low
==8391==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 52 ahci-test /x86_64/ahci/io/dma/lba48/double/high
==8397==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 53 ahci-test /x86_64/ahci/io/dma/lba48/long/zero
==8404==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 54 ahci-test /x86_64/ahci/io/dma/lba48/long/low
==8410==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 55 ahci-test /x86_64/ahci/io/dma/lba48/long/high
==8416==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 56 ahci-test /x86_64/ahci/io/dma/lba48/short/zero
==8422==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 57 ahci-test /x86_64/ahci/io/dma/lba48/short/low
==8428==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 58 ahci-test /x86_64/ahci/io/dma/lba48/short/high
==8434==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 59 ahci-test /x86_64/ahci/io/ncq/simple
==8440==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 60 ahci-test /x86_64/ahci/io/ncq/retry
==8446==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 61 ahci-test /x86_64/ahci/flush/simple
==8452==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 62 ahci-test /x86_64/ahci/flush/retry
==8458==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8463==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 63 ahci-test /x86_64/ahci/flush/migrate
==8472==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8477==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 64 ahci-test /x86_64/ahci/migrate/sanity
==8486==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8491==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 65 ahci-test /x86_64/ahci/migrate/dma/simple
==8501==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8506==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 66 ahci-test /x86_64/ahci/migrate/dma/halted
==8515==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8520==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 67 ahci-test /x86_64/ahci/migrate/ncq/simple
==8529==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8534==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 68 ahci-test /x86_64/ahci/migrate/ncq/halted
==8543==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 69 ahci-test /x86_64/ahci/cdrom/eject
==8548==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 70 ahci-test /x86_64/ahci/cdrom/dma/single
==8554==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 71 ahci-test /x86_64/ahci/cdrom/dma/multi
==8560==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 72 ahci-test /x86_64/ahci/cdrom/pio/single
==8566==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8566==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffce8706000; bottom 0x7fc8ebd9a000; size: 0x0033fc96c000 (223281070080)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 73 ahci-test /x86_64/ahci/cdrom/pio/multi
==8572==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 ahci-test /x86_64/ahci/cdrom/pio/bcl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/hd-geo-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="hd-geo-test" 
PASS 1 hd-geo-test /x86_64/hd-geo/ide/none
==8586==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 hd-geo-test /x86_64/hd-geo/ide/drive/cd_0
==8592==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/blank
==8598==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/lba
==8604==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/chs
==8610==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 hd-geo-test /x86_64/hd-geo/ide/device/mbr/blank
==8616==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 hd-geo-test /x86_64/hd-geo/ide/device/mbr/lba
==8622==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 hd-geo-test /x86_64/hd-geo/ide/device/mbr/chs
==8628==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 hd-geo-test /x86_64/hd-geo/ide/device/user/chs
==8633==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 hd-geo-test /x86_64/hd-geo/ide/device/user/chst
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-order-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-order-test" 
PASS 1 boot-order-test /x86_64/boot-order/pc
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8701==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 bios-tables-test /x86_64/acpi/piix4
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8707==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 bios-tables-test /x86_64/acpi/q35
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8713==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 bios-tables-test /x86_64/acpi/piix4/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8719==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 bios-tables-test /x86_64/acpi/piix4/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8725==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 bios-tables-test /x86_64/acpi/piix4/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8732==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 bios-tables-test /x86_64/acpi/piix4/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8738==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 bios-tables-test /x86_64/acpi/piix4/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8744==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 bios-tables-test /x86_64/acpi/piix4/dimmpxm
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8753==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 bios-tables-test /x86_64/acpi/q35/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8759==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 bios-tables-test /x86_64/acpi/q35/mmio64
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8765==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 bios-tables-test /x86_64/acpi/q35/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8771==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 bios-tables-test /x86_64/acpi/q35/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8778==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 bios-tables-test /x86_64/acpi/q35/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8784==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 bios-tables-test /x86_64/acpi/q35/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==8790==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 bios-tables-test /x86_64/acpi/q35/dimmpxm
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-serial-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-serial-test" 
PASS 1 boot-serial-test /x86_64/boot-serial/isapc
---
PASS 1 i440fx-test /x86_64/i440fx/defaults
PASS 2 i440fx-test /x86_64/i440fx/pam
PASS 3 i440fx-test /x86_64/i440fx/firmware/bios
==8874==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 i440fx-test /x86_64/i440fx/firmware/pflash
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/fw_cfg-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="fw_cfg-test" 
PASS 1 fw_cfg-test /x86_64/fw_cfg/signature
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/drive_del-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="drive_del-test" 
PASS 1 drive_del-test /x86_64/drive_del/without-dev
PASS 2 drive_del-test /x86_64/drive_del/after_failed_device_add
==8962==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 drive_del-test /x86_64/blockdev/drive_del_device_del
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/wdt_ib700-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="wdt_ib700-test" 
PASS 1 wdt_ib700-test /x86_64/wdt_ib700/pause
---
PASS 1 usb-hcd-uhci-test /x86_64/uhci/pci/init
PASS 2 usb-hcd-uhci-test /x86_64/uhci/pci/port1
PASS 3 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug
==9157==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug/usb-storage
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/usb-hcd-xhci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="usb-hcd-xhci-test" 
PASS 1 usb-hcd-xhci-test /x86_64/xhci/pci/init
PASS 2 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug
==9166==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-uas
PASS 4 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-ccid
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/cpu-plug-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="cpu-plug-test" 
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9272==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9278==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid-auto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9284==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 vmgenid-test /x86_64/vmgenid/vmgenid/query-monitor
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/tpm-crb-swtpm-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="tpm-crb-swtpm-test" 
SKIP 1 tpm-crb-swtpm-test /x86_64/tpm/crb-swtpm/test # SKIP swtpm not in PATH or missing --tpm2 support
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9389==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9394==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 migration-test /x86_64/migration/fd_proto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9402==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9407==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 migration-test /x86_64/migration/postcopy/unix
PASS 5 migration-test /x86_64/migration/postcopy/recovery
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9437==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9442==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 migration-test /x86_64/migration/precopy/unix
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9451==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9456==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 migration-test /x86_64/migration/precopy/tcp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9465==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9470==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 migration-test /x86_64/migration/xbzrle/unix
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/test-x86-cpuid-compat -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid-compat" 
PASS 1 test-x86-cpuid-compat /x86/cpuid/parsing-plus-minus
---
PASS 6 numa-test /x86_64/numa/pc/dynamic/cpu
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qmp-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="qmp-test" 
PASS 1 qmp-test /x86_64/qmp/protocol
==9799==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 qmp-test /x86_64/qmp/oob
PASS 3 qmp-test /x86_64/qmp/preconfig
PASS 4 qmp-test /x86_64/qmp/missing-any-arg
---
PASS 6 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/sdhci-pci/sdhci/sdhci-tests/registers
PASS 7 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/tpci200/ipack/ipoctal232/ipoctal232-tests/nop
PASS 8 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/tpci200/pci-device/pci-device-tests/nop
==10208==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/pci-device/pci-device-tests/nop
PASS 10 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio/virtio-tests/nop
PASS 11 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/config
---
PASS 20 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/fs/flush/ignored
PASS 21 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-balloon-pci/pci-device/pci-device-tests/nop
PASS 22 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-balloon-pci/virtio/virtio-tests/nop
==10221==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 23 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/indirect
==10228==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 24 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/config
==10235==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 25 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/basic
==10242==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 26 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/resize
==10249==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 27 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/msix
==10256==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 28 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/idx
==10263==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 29 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/nxvirtq
==10270==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 30 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/hotplug
PASS 31 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/basic
PASS 32 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/rx_stop_cont
---
PASS 40 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-rng-pci/pci-device/pci-device-tests/nop
PASS 41 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-rng-pci/virtio/virtio-tests/nop
PASS 42 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-rng-pci/virtio-rng-pci-tests/hotplug
==10381==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 43 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-scsi-pci/pci-device/pci-device-tests/nop
==10387==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 44 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-scsi-pci/virtio-scsi/virtio-scsi-tests/hotplug
==10393==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 45 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-scsi-pci/virtio-scsi/virtio-scsi-tests/unaligned-write-same
==10399==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 46 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-scsi-pci/virtio-scsi-pci-tests/iothread-attach-node
PASS 47 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-serial-pci/pci-device/pci-device-tests/nop
PASS 48 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-serial-pci/virtio/virtio-tests/nop
---
PASS 67 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/i82562/pci-device/pci-device-tests/nop
PASS 68 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/i82801/pci-device/pci-device-tests/nop
PASS 69 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/ES1370/pci-device/pci-device-tests/nop
==10544==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 70 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/megasas/pci-device/pci-device-tests/nop
PASS 71 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/megasas/megasas-tests/dcmd/pd-get-info/fuzz
PASS 72 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/ne2k_pci/pci-device/pci-device-tests/nop
PASS 73 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/nvme/pci-device/pci-device-tests/nop
==10556==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/nvme/nvme-tests/oob-cmb-access
==10562==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 75 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/pcnet/pci-device/pci-device-tests/nop
PASS 76 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/pci-ohci/pci-device/pci-device-tests/nop
PASS 77 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/pci-ohci/pci-ohci-tests/ohci_pci-test-hotplug


The full log is available at
http://patchew.org/logs/20190702001301.4768-1-philmd@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[Philippe Mathieu-Daudé]

Cc'ing Marc-André,

On 7/2/19 8:15 AM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190702001301.4768-1-philmd@redhat.com/
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> make docker-image-fedora V=1 NETWORK=1
> time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
> === TEST SCRIPT END ===

I am not sure how the error reported is related to this series:

MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-bdrv-drain -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-bdrv-drain"
==8090==WARNING: ASan doesn't fully support makecontext/swapcontext
functions and may produce false positives in some cases!
PASS 1 test-bdrv-drain /bdrv-drain/nested
PASS 2 test-bdrv-drain /bdrv-drain/multiparent
PASS 3 test-bdrv-drain /bdrv-drain/set_aio_context
PASS 4 test-bdrv-drain /bdrv-drain/driver-cb/drain_all
PASS 5 test-bdrv-drain /bdrv-drain/driver-cb/drain
PASS 6 test-bdrv-drain /bdrv-drain/driver-cb/drain_subtree
PASS 7 test-bdrv-drain /bdrv-drain/driver-cb/co/drain_all
PASS 8 test-bdrv-drain /bdrv-drain/driver-cb/co/drain
PASS 9 test-bdrv-drain /bdrv-drain/driver-cb/co/drain_subtree
PASS 10 test-bdrv-drain /bdrv-drain/quiesce/drain_all
PASS 11 test-bdrv-drain /bdrv-drain/quiesce/drain
PASS 12 test-bdrv-drain /bdrv-drain/quiesce/drain_subtree
PASS 13 test-bdrv-drain /bdrv-drain/quiesce/co/drain_all
PASS 14 test-bdrv-drain /bdrv-drain/quiesce/co/drain
PASS 15 test-bdrv-drain /bdrv-drain/quiesce/co/drain_subtree
PASS 16 test-bdrv-drain /bdrv-drain/graph-change/drain_subtree
PASS 17 test-bdrv-drain /bdrv-drain/graph-change/drain_all
=================================================================
==8090==ERROR: AddressSanitizer: heap-use-after-free on address
0x61200002c1f0 at pc 0x559638e7e006 bp 0x7f974eab8680 sp 0x7f974eab8678
WRITE of size 1 at 0x61200002c1f0 thread T5
PASS 3 ahci-test /x86_64/ahci/pci_enable
    #0 0x559638e7e005 in aio_notify /tmp/qemu-test/src/util/async.c:351:9
    #1 0x559638e7fc3b in qemu_bh_schedule
/tmp/qemu-test/src/util/async.c:167:9
    #2 0x559638e82e40 in aio_co_schedule
/tmp/qemu-test/src/util/async.c:464:5
    #3 0x559638e83109 in aio_co_enter /tmp/qemu-test/src/util/async.c:483:9
    #4 0x559638e8308d in aio_co_wake /tmp/qemu-test/src/util/async.c:477:5
    #5 0x55963876b3d4 in co_reenter_bh
/tmp/qemu-test/src/tests/test-bdrv-drain.c:63:5
    #6 0x559638e7e8aa in aio_bh_call /tmp/qemu-test/src/util/async.c:89:5
    #7 0x559638e7efc2 in aio_bh_poll /tmp/qemu-test/src/util/async.c:117:13
    #8 0x559638ea4a73 in aio_poll /tmp/qemu-test/src/util/aio-posix.c:728:17
    #9 0x559638d48628 in iothread_run
/tmp/qemu-test/src/tests/iothread.c:51:9
    #10 0x559638eb8612 in qemu_thread_start
/tmp/qemu-test/src/util/qemu-thread-posix.c:502:9
    #11 0x7f976074a5a1 in start_thread (/lib64/libpthread.so.0+0x85a1)
    #12 0x7f9760657022 in __GI___clone (/lib64/libc.so.6+0xfb022)

0x61200002c1f0 is located 176 bytes inside of 312-byte region
[0x61200002c140,0x61200002c278)
freed by thread T0 here:
    #0 0x55963872475f in free
(/tmp/qemu-test/build/tests/test-bdrv-drain+0x53375f)
    #1 0x7f9760bc5d8c in g_free (/lib64/libglib-2.0.so.0+0x55d8c)

previously allocated by thread T4 here:
    #0 0x559638724d9e in calloc
(/tmp/qemu-test/build/tests/test-bdrv-drain+0x533d9e)
    #1 0x7f9760bc5cf0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55cf0)

Thread T5 created by T0 here:
    #0 0x559638659f16 in __interceptor_pthread_create
(/tmp/qemu-test/build/tests/test-bdrv-drain+0x468f16)
    #1 0x559638eb7f19 in qemu_thread_create
/tmp/qemu-test/src/util/qemu-thread-posix.c:539:11
    #2 0x559638d47cce in iothread_new
/tmp/qemu-test/src/tests/iothread.c:75:5
    #3 0x55963876c412 in test_iothread_common
/tmp/qemu-test/src/tests/test-bdrv-drain.c:664:19
    #4 0x55963876724e in test_iothread_drain_all
/tmp/qemu-test/src/tests/test-bdrv-drain.c:758:5
    #5 0x7f9760be7f9d  (/lib64/libglib-2.0.so.0+0x77f9d)

Thread T4 created by T0 here:
    #0 0x559638659f16 in __interceptor_pthread_create
(/tmp/qemu-test/build/tests/test-bdrv-drain+0x468f16)
    #1 0x559638eb7f19 in qemu_thread_create
/tmp/qemu-test/src/util/qemu-thread-posix.c:539:11
    #2 0x559638d47cce in iothread_new
/tmp/qemu-test/src/tests/iothread.c:75:5
    #3 0x55963876c406 in test_iothread_common
/tmp/qemu-test/src/tests/test-bdrv-drain.c:663:19
    #4 0x55963876724e in test_iothread_drain_all
/tmp/qemu-test/src/tests/test-bdrv-drain.c:758:5
    #5 0x7f9760be7f9d  (/lib64/libglib-2.0.so.0+0x77f9d)

SUMMARY: AddressSanitizer: heap-use-after-free
/tmp/qemu-test/src/util/async.c:351:9 in aio_notify
Shadow bytes around the buggy address:
  0x0c247fffd7e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fffd7f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c247fffd800: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c247fffd810: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c247fffd820: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
=>0x0c247fffd830: fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd
  0x0c247fffd840: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c247fffd850: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fffd860: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fffd870: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa
  0x0c247fffd880: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==8090==ABORTING
ERROR - too few tests run (expected 39, got 17)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:899: check-unit]
Error 1
make: *** Waiting for unfinished jobs....

Re: [Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[Laszlo Ersek]

Hi Phil,

On 07/02/19 02:12, Philippe Mathieu-Daudé wrote:
> The pflash device lacks a reset() function.
> When a machine is resetted, the flash might be in an
> inconsistent state, leading to unexpected behavior:
> https://bugzilla.redhat.com/show_bug.cgi?id=1678713
> Resolve this issue by adding a DeviceReset() handler.
> 
> Fix also two minor issues, and clean a bit the codebase.
> 
> Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00962.html
> - addressed Laszlo review comments
> 
> Maintainers spam list from:
> ./scripts/get_maintainer.pl -f $(git grep -El '(pflash_cfi01_register|TYPE_PFLASH_CFI01)')
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (9):
>   hw/block/pflash_cfi01: Removed an unused timer
>   hw/block/pflash_cfi01: Use the correct READ_ARRAY value
>   hw/block/pflash_cfi01: Extract pflash_mode_read_array()
>   hw/block/pflash_cfi01: Start state machine as READY to accept commands
>   hw/block/pflash_cfi01: Add the DeviceReset() handler
>   hw/block/pflash_cfi01: Simplify CFI_QUERY processing
>   hw/block/pflash_cfi01: Improve command comments
>   hw/block/pflash_cfi01: Replace DPRINTF by qemu_log_mask(GUEST_ERROR)
>   hw/block/pflash_cfi01: Hold the PRI table offset in a variable
> 
>  hw/block/pflash_cfi01.c | 140 +++++++++++++++++++++-------------------
>  hw/block/trace-events   |   1 +
>  2 files changed, 74 insertions(+), 67 deletions(-)
> 

I'll do some regression-tests with this, using OVMF and ArmVirtQemu.

I don't think I can usefully review the patches without getting lost in
the related spec(s), and I don't have capacity for that.

Until I have regression test results, one question: are the changes to
the device model transparent with regard to migration? (You are not
introducing any compat properties.)

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[Laszlo Ersek]

On 07/02/19 13:52, Laszlo Ersek wrote:
> Hi Phil,
> 
> On 07/02/19 02:12, Philippe Mathieu-Daudé wrote:
>> The pflash device lacks a reset() function.
>> When a machine is resetted, the flash might be in an
>> inconsistent state, leading to unexpected behavior:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1678713
>> Resolve this issue by adding a DeviceReset() handler.
>>
>> Fix also two minor issues, and clean a bit the codebase.
>>
>> Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00962.html
>> - addressed Laszlo review comments
>>
>> Maintainers spam list from:
>> ./scripts/get_maintainer.pl -f $(git grep -El '(pflash_cfi01_register|TYPE_PFLASH_CFI01)')
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (9):
>>   hw/block/pflash_cfi01: Removed an unused timer
>>   hw/block/pflash_cfi01: Use the correct READ_ARRAY value
>>   hw/block/pflash_cfi01: Extract pflash_mode_read_array()
>>   hw/block/pflash_cfi01: Start state machine as READY to accept commands
>>   hw/block/pflash_cfi01: Add the DeviceReset() handler
>>   hw/block/pflash_cfi01: Simplify CFI_QUERY processing
>>   hw/block/pflash_cfi01: Improve command comments
>>   hw/block/pflash_cfi01: Replace DPRINTF by qemu_log_mask(GUEST_ERROR)
>>   hw/block/pflash_cfi01: Hold the PRI table offset in a variable
>>
>>  hw/block/pflash_cfi01.c | 140 +++++++++++++++++++++-------------------
>>  hw/block/trace-events   |   1 +
>>  2 files changed, 74 insertions(+), 67 deletions(-)
>>
> 
> I'll do some regression-tests with this, using OVMF and ArmVirtQemu.
> 
> I don't think I can usefully review the patches without getting lost in
> the related spec(s), and I don't have capacity for that.
> 
> Until I have regression test results, one question: are the changes to
> the device model transparent with regard to migration? (You are not
> introducing any compat properties.)

I didn't test migration.

With OVMF, I performed my usual Linux guest tests (partly described at
<https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#tests-to-perform-in-the-installed-guest-fedora-26-guest>).
I found no problems / discrepancies, in either guest behavior or
firmware logs.

With ArmVirtQemu, I meant to test on KVM (pflash used to be really
sensitive to KVM<->TCG differences), but my aarch64 hardware is
apparently dying, and I wouldn't like to spend a day just to provision a
remote aarch64 box. So, no test results on aarch64.

With those caveats:

Regression-tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[Laszlo Ersek]

On 07/02/19 15:41, Laszlo Ersek wrote:
> On 07/02/19 13:52, Laszlo Ersek wrote:
>> Hi Phil,
>>
>> On 07/02/19 02:12, Philippe Mathieu-Daudé wrote:
>>> The pflash device lacks a reset() function.
>>> When a machine is resetted, the flash might be in an
>>> inconsistent state, leading to unexpected behavior:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1678713
>>> Resolve this issue by adding a DeviceReset() handler.
>>>
>>> Fix also two minor issues, and clean a bit the codebase.
>>>
>>> Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00962.html
>>> - addressed Laszlo review comments
>>>
>>> Maintainers spam list from:
>>> ./scripts/get_maintainer.pl -f $(git grep -El '(pflash_cfi01_register|TYPE_PFLASH_CFI01)')
>>>
>>> Regards,
>>>
>>> Phil.
>>>
>>> Philippe Mathieu-Daudé (9):
>>>   hw/block/pflash_cfi01: Removed an unused timer
>>>   hw/block/pflash_cfi01: Use the correct READ_ARRAY value
>>>   hw/block/pflash_cfi01: Extract pflash_mode_read_array()
>>>   hw/block/pflash_cfi01: Start state machine as READY to accept commands
>>>   hw/block/pflash_cfi01: Add the DeviceReset() handler
>>>   hw/block/pflash_cfi01: Simplify CFI_QUERY processing
>>>   hw/block/pflash_cfi01: Improve command comments
>>>   hw/block/pflash_cfi01: Replace DPRINTF by qemu_log_mask(GUEST_ERROR)
>>>   hw/block/pflash_cfi01: Hold the PRI table offset in a variable
>>>
>>>  hw/block/pflash_cfi01.c | 140 +++++++++++++++++++++-------------------
>>>  hw/block/trace-events   |   1 +
>>>  2 files changed, 74 insertions(+), 67 deletions(-)
>>>
>>
>> I'll do some regression-tests with this, using OVMF and ArmVirtQemu.
>>
>> I don't think I can usefully review the patches without getting lost in
>> the related spec(s), and I don't have capacity for that.
>>
>> Until I have regression test results, one question: are the changes to
>> the device model transparent with regard to migration? (You are not
>> introducing any compat properties.)
> 
> I didn't test migration.
> 
> With OVMF, I performed my usual Linux guest tests (partly described at
> <https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#tests-to-perform-in-the-installed-guest-fedora-26-guest>).
> I found no problems / discrepancies, in either guest behavior or
> firmware logs.
> 
> With ArmVirtQemu, I meant to test on KVM (pflash used to be really
> sensitive to KVM<->TCG differences), but my aarch64 hardware is
> apparently dying, and I wouldn't like to spend a day just to provision a
> remote aarch64 box. So, no test results on aarch64.

Managed to run a light regression-test on aarch64 KVM too, using the
ArmVirtQemu fw and an F28 guest.  (boot+reboot, without the patches and
with the patches; logs compared, behavior compared.) Everything seems fine.

Thanks
Laszlo

> With those caveats:
> 
> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks
> Laszlo
> 

Re: [Qemu-devel] [PATCH v2 0/9] hw/block/pflash_cfi01: Add DeviceReset() handler

[Philippe Mathieu-Daudé]

On 7/2/19 1:52 PM, Laszlo Ersek wrote:
> Hi Phil,
> 
> On 07/02/19 02:12, Philippe Mathieu-Daudé wrote:
>> The pflash device lacks a reset() function.
>> When a machine is resetted, the flash might be in an
>> inconsistent state, leading to unexpected behavior:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1678713
>> Resolve this issue by adding a DeviceReset() handler.
>>
>> Fix also two minor issues, and clean a bit the codebase.
>>
>> Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00962.html
>> - addressed Laszlo review comments
>>
>> Maintainers spam list from:
>> ./scripts/get_maintainer.pl -f $(git grep -El '(pflash_cfi01_register|TYPE_PFLASH_CFI01)')
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (9):
>>   hw/block/pflash_cfi01: Removed an unused timer
>>   hw/block/pflash_cfi01: Use the correct READ_ARRAY value
>>   hw/block/pflash_cfi01: Extract pflash_mode_read_array()
>>   hw/block/pflash_cfi01: Start state machine as READY to accept commands
>>   hw/block/pflash_cfi01: Add the DeviceReset() handler
>>   hw/block/pflash_cfi01: Simplify CFI_QUERY processing
>>   hw/block/pflash_cfi01: Improve command comments
>>   hw/block/pflash_cfi01: Replace DPRINTF by qemu_log_mask(GUEST_ERROR)
>>   hw/block/pflash_cfi01: Hold the PRI table offset in a variable
>>
>>  hw/block/pflash_cfi01.c | 140 +++++++++++++++++++++-------------------
>>  hw/block/trace-events   |   1 +
>>  2 files changed, 74 insertions(+), 67 deletions(-)
>>
> 
> I'll do some regression-tests with this, using OVMF and ArmVirtQemu.

My local tree is larger with many tracing improvments, but I stripped
that part to keep this series short and bugfix oriented.

I used the trace logs to analyse the guest accesses.

My test setup is not yet automatized but I'm working on passing all the
one working with TCG as Avocado tests.

Tests machines (firmwares):

- ARM Verdex with (U-Boot)
- MIPS Malta with (Linux)
- LM32 Milkymist One (RTEMS)
- X86 (EDK2 OVMF)

Each guest has different driver, thus use this device quite differently.

What I want to add to this list is:

- AARCH64 Virt (EDK2 ArmVirtQemu)

Because it uses another driver (not the OVMF one).

> I don't think I can usefully review the patches without getting lost in
> the related spec(s), and I don't have capacity for that.
> 
> Until I have regression test results, one question: are the changes to
> the device model transparent with regard to migration? (You are not
> introducing any compat properties.)

Good point.

Two fields updated are migrated:

static const VMStateDescription vmstate_pflash = {
    .name = "pflash_cfi01",
    .version_id = 1,
    .minimum_version_id = 1,
    .post_load = pflash_post_load,
    .fields = (VMStateField[]) {
        ...
        VMSTATE_UINT8(cmd, PFlashCFI01),
        VMSTATE_UINT8(status, PFlashCFI01),
        ...
        VMSTATE_END_OF_LIST()
    }
};

- status: should not break anything

- cmd:

  If migrated during cmd == READ_ARRAY state:

  Guest is saved with older QEMU having cmd=READ_ARRAY=0x00

  Guest restored to newer QEMU expecting READ_ARRAY=0xff

  - If the guest do a READ access, we are [currently] fine:
  the 0x00 command hits the default case and falls through
  0xff.

    ('currently' because I have a local patch that would
     clean up this impossible case, but I made it possible).

  - If guest does WRITE access, again the 0x00 is processed
  by the default case where we jump to the error_flash label.
  There we log a warning and call pflash_mode_read_array().

  So this is not clean, but safe.

  To be clean I should document 0x00 was previously used and
  even not matching the specs, we have to live with handling
  it forever.



  A cleaner way to keep this safe and the code simple is to
  increment vmstate_pflash.version_id, and add a new case in
  the post_load handler:

static int pflash_post_load(void *opaque, int version_id)
{
    PFlashCFI01 *pfl = opaque;

    ... // current handler code

    if (version_id < 2) {
        /* version_id used 0x00 for READ_ARRAY */
        if (s->cmd == 0x00) {
            s->cmd =  0xff;
        }
    }
    return 0;
}

Since I'm new with migration, I Cc'd Dave to check :)

Thanks!

Phil.