All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/11] pflash patches for 2021-03-16
@ 2021-03-15 23:35 Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 01/11] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Philippe Mathieu-Daudé, qemu-block, Max Reitz

The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339:

  Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-03-15 19:23:00 +0000)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/pflash-20210316

for you to fetch changes up to 3b6a1da064ac6ce5256f1f6f16870ea79c2422d0:

  hw/block/pflash_cfi: Replace DPRINTF with trace events (2021-03-16 00:28:33 +0100)

----------------------------------------------------------------
Parallel NOR Flash patches queue

- Code movement to ease maintainability
- Tracing improvements
----------------------------------------------------------------

David Edmondson (2):
  hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro
  hw/block/pflash_cfi: Replace DPRINTF with trace events

Philippe Mathieu-Daudé (9):
  hw/block/pflash_cfi: Fix code style for checkpatch.pl
  hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table()
  hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table()
  hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings()
  hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false)
  hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array
  hw/block/pflash_cfi02: Factor out pflash_reset_state_machine()
  hw/block/pflash_cfi02: Add DeviceReset method
  hw/block/pflash_cfi01: Clarify trace events

 hw/block/pflash_cfi01.c | 253 ++++++++++++++++----------------
 hw/block/pflash_cfi02.c | 316 ++++++++++++++++++++--------------------
 hw/block/trace-events   |  40 +++--
 3 files changed, 320 insertions(+), 289 deletions(-)

-- 
2.26.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PULL 01/11] hw/block/pflash_cfi: Fix code style for checkpatch.pl
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 02/11] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table() Philippe Mathieu-Daudé
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

We are going to move this code, fix its style first.

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210310170528.1184868-2-philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 36 ++++++++++++++++++++++++------------
 hw/block/pflash_cfi02.c |  9 ++++++---
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 526b70417de..248889c3d02 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -115,7 +115,8 @@ static const VMStateDescription vmstate_pflash = {
     }
 };
 
-/* Perform a CFI query based on the bank width of the flash.
+/*
+ * 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.
  */
@@ -125,7 +126,8 @@ static uint32_t pflash_cfi_query(PFlashCFI01 *pfl, hwaddr offset)
     uint32_t resp = 0;
     hwaddr boff;
 
-    /* Adjust incoming offset to match expected device-width
+    /*
+     * Adjust incoming offset to match expected device-width
      * addressing. CFI query addresses are always specified in terms of
      * the maximum supported width of the device.  This means that x8
      * devices and x8/x16 devices in x8 mode behave differently.  For
@@ -141,7 +143,8 @@ static uint32_t pflash_cfi_query(PFlashCFI01 *pfl, hwaddr offset)
     if (boff >= sizeof(pfl->cfi_table)) {
         return 0;
     }
-    /* Now we will construct the CFI response generated by a single
+    /*
+     * Now we will construct the CFI response generated by a single
      * device, then replicate that for all devices that make up the
      * bus.  For wide parts used in x8 mode, CFI query responses
      * are different than native byte-wide parts.
@@ -185,7 +188,8 @@ static uint32_t pflash_devid_query(PFlashCFI01 *pfl, hwaddr offset)
     uint32_t resp;
     hwaddr boff;
 
-    /* Adjust incoming offset to match expected device-width
+    /*
+     * Adjust incoming offset to match expected device-width
      * addressing. Device ID read addresses are always specified in
      * terms of the maximum supported width of the device.  This means
      * that x8 devices and x8/x16 devices in x8 mode behave
@@ -198,7 +202,8 @@ static uint32_t pflash_devid_query(PFlashCFI01 *pfl, hwaddr offset)
     boff = offset >> (ctz32(pfl->bank_width) +
                       ctz32(pfl->max_device_width) - ctz32(pfl->device_width));
 
-    /* Mask off upper bits which may be used in to query block
+    /*
+     * Mask off upper bits which may be used in to query block
      * or sector lock status at other addresses.
      * Offsets 2/3 are block lock status, is not emulated.
      */
@@ -297,7 +302,8 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
     case 0x60: /* Block /un)lock */
     case 0x70: /* Status Register */
     case 0xe8: /* Write block */
-        /* Status register read.  Return status from each device in
+        /*
+         * Status register read.  Return status from each device in
          * bank.
          */
         ret = pfl->status;
@@ -308,7 +314,8 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
                 shift += pfl->device_width * 8;
             }
         } else if (!pfl->device_width && width > 2) {
-            /* Handle 32 bit flash cases where device width is not
+            /*
+             * Handle 32 bit flash cases where device width is not
              * set. (Existing behavior before device width added.)
              */
             ret |= pfl->status << 16;
@@ -340,7 +347,8 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
                 break;
             }
         } else {
-            /* If we have a read larger than the bank_width, combine multiple
+            /*
+             * If we have a read larger than the bank_width, combine multiple
              * manufacturer/device ID queries into a single response.
              */
             int i;
@@ -367,7 +375,8 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
                 ret = 0;
             }
         } else {
-            /* If we have a read larger than the bank_width, combine multiple
+            /*
+             * If we have a read larger than the bank_width, combine multiple
              * CFI queries into a single response.
              */
             int i;
@@ -544,7 +553,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 
             break;
         case 0xe8:
-            /* Mask writeblock size based on device width, or bank width if
+            /*
+             * Mask writeblock size based on device width, or bank width if
              * device width not specified.
              */
             /* FIXME check @offset, @width */
@@ -718,7 +728,8 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
 
     total_len = pfl->sector_len * pfl->nb_blocs;
 
-    /* These are only used to expose the parameters of each device
+    /*
+     * These are only used to expose the parameters of each device
      * in the cfi_table[].
      */
     num_devices = pfl->device_width ? (pfl->bank_width / pfl->device_width) : 1;
@@ -763,7 +774,8 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
         }
     }
 
-    /* Default to devices being used at their maximum device width. This was
+    /*
+     * Default to devices being used at their maximum device width. This was
      * assumed before the device_width support was added.
      */
     if (!pfl->max_device_width) {
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 7962cff7455..fa981465e12 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -100,7 +100,8 @@ struct PFlashCFI02 {
     uint16_t unlock_addr1;
     uint8_t cfi_table[0x4d];
     QEMUTimer timer;
-    /* The device replicates the flash memory across its memory space.  Emulate
+    /*
+     * The device replicates the flash memory across its memory space.  Emulate
      * that by having a container (.mem) filled with an array of aliases
      * (.mem_mappings) pointing to the flash memory (.orig_mem).
      */
@@ -884,8 +885,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pfl->cfi_table[0x28] = 0x02;
     pfl->cfi_table[0x29] = 0x00;
     /* Max number of bytes in multi-bytes write */
-    /* XXX: disable buffered write as it's not supported */
-    //    pfl->cfi_table[0x2A] = 0x05;
+    /*
+     * XXX: disable buffered write as it's not supported
+     * pfl->cfi_table[0x2A] = 0x05;
+     */
     pfl->cfi_table[0x2A] = 0x00;
     pfl->cfi_table[0x2B] = 0x00;
     /* Number of erase block regions */
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 02/11] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table()
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 01/11] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 03/11] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table() Philippe Mathieu-Daudé
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

Fill the CFI table in out of DeviceRealize() in a new function:
pflash_cfi01_fill_cfi_table().

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210310170528.1184868-3-philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 140 +++++++++++++++++++++-------------------
 1 file changed, 73 insertions(+), 67 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 248889c3d02..779a62f3b06 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -704,30 +704,11 @@ static const MemoryRegionOps pflash_cfi01_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
+static void pflash_cfi01_fill_cfi_table(PFlashCFI01 *pfl)
 {
-    ERRP_GUARD();
-    PFlashCFI01 *pfl = PFLASH_CFI01(dev);
-    uint64_t total_len;
-    int ret;
     uint64_t blocks_per_device, sector_len_per_device, device_len;
     int num_devices;
 
-    if (pfl->sector_len == 0) {
-        error_setg(errp, "attribute \"sector-length\" not specified or zero.");
-        return;
-    }
-    if (pfl->nb_blocs == 0) {
-        error_setg(errp, "attribute \"num-blocks\" not specified or zero.");
-        return;
-    }
-    if (pfl->name == NULL) {
-        error_setg(errp, "attribute \"name\" not specified.");
-        return;
-    }
-
-    total_len = pfl->sector_len * pfl->nb_blocs;
-
     /*
      * These are only used to expose the parameters of each device
      * in the cfi_table[].
@@ -742,53 +723,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     }
     device_len = sector_len_per_device * blocks_per_device;
 
-    memory_region_init_rom_device(
-        &pfl->mem, OBJECT(dev),
-        &pflash_cfi01_ops,
-        pfl,
-        pfl->name, total_len, errp);
-    if (*errp) {
-        return;
-    }
-
-    pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
-    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
-
-    if (pfl->blk) {
-        uint64_t perm;
-        pfl->ro = !blk_supports_write_perm(pfl->blk);
-        perm = BLK_PERM_CONSISTENT_READ | (pfl->ro ? 0 : BLK_PERM_WRITE);
-        ret = blk_set_perm(pfl->blk, perm, BLK_PERM_ALL, errp);
-        if (ret < 0) {
-            return;
-        }
-    } else {
-        pfl->ro = 0;
-    }
-
-    if (pfl->blk) {
-        if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len,
-                                         errp)) {
-            vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
-            return;
-        }
-    }
-
-    /*
-     * Default to devices being used at their maximum device width. This was
-     * assumed before the device_width support was added.
-     */
-    if (!pfl->max_device_width) {
-        pfl->max_device_width = pfl->device_width;
-    }
-
-    pfl->wcycle = 0;
-    /*
-     * The command 0x00 is not assigned by the CFI open standard,
-     * but QEMU historically uses it for the READ_ARRAY command (0xff).
-     */
-    pfl->cmd = 0x00;
-    pfl->status = 0x80; /* WSM ready */
     /* Hardcoded CFI table */
     /* Standard "QRY" string */
     pfl->cfi_table[0x10] = 'Q';
@@ -876,6 +810,78 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
     pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
 }
 
+static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
+{
+    ERRP_GUARD();
+    PFlashCFI01 *pfl = PFLASH_CFI01(dev);
+    uint64_t total_len;
+    int ret;
+
+    if (pfl->sector_len == 0) {
+        error_setg(errp, "attribute \"sector-length\" not specified or zero.");
+        return;
+    }
+    if (pfl->nb_blocs == 0) {
+        error_setg(errp, "attribute \"num-blocks\" not specified or zero.");
+        return;
+    }
+    if (pfl->name == NULL) {
+        error_setg(errp, "attribute \"name\" not specified.");
+        return;
+    }
+
+    total_len = pfl->sector_len * pfl->nb_blocs;
+
+    memory_region_init_rom_device(
+        &pfl->mem, OBJECT(dev),
+        &pflash_cfi01_ops,
+        pfl,
+        pfl->name, total_len, errp);
+    if (*errp) {
+        return;
+    }
+
+    pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
+
+    if (pfl->blk) {
+        uint64_t perm;
+        pfl->ro = !blk_supports_write_perm(pfl->blk);
+        perm = BLK_PERM_CONSISTENT_READ | (pfl->ro ? 0 : BLK_PERM_WRITE);
+        ret = blk_set_perm(pfl->blk, perm, BLK_PERM_ALL, errp);
+        if (ret < 0) {
+            return;
+        }
+    } else {
+        pfl->ro = 0;
+    }
+
+    if (pfl->blk) {
+        if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len,
+                                         errp)) {
+            vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
+            return;
+        }
+    }
+
+    /*
+     * Default to devices being used at their maximum device width. This was
+     * assumed before the device_width support was added.
+     */
+    if (!pfl->max_device_width) {
+        pfl->max_device_width = pfl->device_width;
+    }
+
+    pfl->wcycle = 0;
+    /*
+     * The command 0x00 is not assigned by the CFI open standard,
+     * but QEMU historically uses it for the READ_ARRAY command (0xff).
+     */
+    pfl->cmd = 0x00;
+    pfl->status = 0x80; /* WSM ready */
+    pflash_cfi01_fill_cfi_table(pfl);
+}
+
 static void pflash_cfi01_system_reset(DeviceState *dev)
 {
     PFlashCFI01 *pfl = PFLASH_CFI01(dev);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 03/11] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table()
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 01/11] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 02/11] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table() Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 04/11] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings() Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

Fill the CFI table in out of DeviceRealize() in a new function:
pflash_cfi02_fill_cfi_table().

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210310170528.1184868-4-philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 193 +++++++++++++++++++++-------------------
 1 file changed, 99 insertions(+), 94 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index fa981465e12..845f50ed99b 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -724,6 +724,104 @@ static const MemoryRegionOps pflash_cfi02_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void pflash_cfi02_fill_cfi_table(PFlashCFI02 *pfl, int nb_regions)
+{
+    /* Hardcoded CFI table (mostly from SG29 Spansion flash) */
+    const uint16_t pri_ofs = 0x40;
+    /* Standard "QRY" string */
+    pfl->cfi_table[0x10] = 'Q';
+    pfl->cfi_table[0x11] = 'R';
+    pfl->cfi_table[0x12] = 'Y';
+    /* Command set (AMD/Fujitsu) */
+    pfl->cfi_table[0x13] = 0x02;
+    pfl->cfi_table[0x14] = 0x00;
+    /* Primary extended table address */
+    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;
+    /* Vcc min */
+    pfl->cfi_table[0x1B] = 0x27;
+    /* Vcc max */
+    pfl->cfi_table[0x1C] = 0x36;
+    /* Vpp min (no Vpp pin) */
+    pfl->cfi_table[0x1D] = 0x00;
+    /* Vpp max (no Vpp pin) */
+    pfl->cfi_table[0x1E] = 0x00;
+    /* Timeout per single byte/word write (128 ms) */
+    pfl->cfi_table[0x1F] = 0x07;
+    /* Timeout for min size buffer write (NA) */
+    pfl->cfi_table[0x20] = 0x00;
+    /* Typical timeout for block erase (512 ms) */
+    pfl->cfi_table[0x21] = 0x09;
+    /* Typical timeout for full chip erase (4096 ms) */
+    pfl->cfi_table[0x22] = 0x0C;
+    /* Reserved */
+    pfl->cfi_table[0x23] = 0x01;
+    /* Max timeout for buffer write (NA) */
+    pfl->cfi_table[0x24] = 0x00;
+    /* Max timeout for block erase */
+    pfl->cfi_table[0x25] = 0x0A;
+    /* Max timeout for chip erase */
+    pfl->cfi_table[0x26] = 0x0D;
+    /* Device size */
+    pfl->cfi_table[0x27] = ctz32(pfl->chip_len);
+    /* Flash device interface (8 & 16 bits) */
+    pfl->cfi_table[0x28] = 0x02;
+    pfl->cfi_table[0x29] = 0x00;
+    /* Max number of bytes in multi-bytes write */
+    /*
+     * XXX: disable buffered write as it's not supported
+     * pfl->cfi_table[0x2A] = 0x05;
+     */
+    pfl->cfi_table[0x2A] = 0x00;
+    pfl->cfi_table[0x2B] = 0x00;
+    /* Number of erase block regions */
+    pfl->cfi_table[0x2c] = nb_regions;
+    /* Erase block regions */
+    for (int i = 0; i < nb_regions; ++i) {
+        uint32_t sector_len_per_device = pfl->sector_len[i];
+        pfl->cfi_table[0x2d + 4 * i] = pfl->nb_blocs[i] - 1;
+        pfl->cfi_table[0x2e + 4 * i] = (pfl->nb_blocs[i] - 1) >> 8;
+        pfl->cfi_table[0x2f + 4 * i] = sector_len_per_device >> 8;
+        pfl->cfi_table[0x30 + 4 * i] = sector_len_per_device >> 16;
+    }
+    assert(0x2c + 4 * nb_regions < pri_ofs);
+
+    /* Extended */
+    pfl->cfi_table[0x00 + pri_ofs] = 'P';
+    pfl->cfi_table[0x01 + pri_ofs] = 'R';
+    pfl->cfi_table[0x02 + pri_ofs] = 'I';
+
+    /* Extended version 1.0 */
+    pfl->cfi_table[0x03 + pri_ofs] = '1';
+    pfl->cfi_table[0x04 + pri_ofs] = '0';
+
+    /* Address sensitive unlock required. */
+    pfl->cfi_table[0x05 + pri_ofs] = 0x00;
+    /* Erase suspend to read/write. */
+    pfl->cfi_table[0x06 + pri_ofs] = 0x02;
+    /* Sector protect not supported. */
+    pfl->cfi_table[0x07 + pri_ofs] = 0x00;
+    /* Temporary sector unprotect not supported. */
+    pfl->cfi_table[0x08 + pri_ofs] = 0x00;
+
+    /* Sector protect/unprotect scheme. */
+    pfl->cfi_table[0x09 + pri_ofs] = 0x00;
+
+    /* Simultaneous operation not supported. */
+    pfl->cfi_table[0x0a + pri_ofs] = 0x00;
+    /* Burst mode not supported. */
+    pfl->cfi_table[0x0b + pri_ofs] = 0x00;
+    /* Page mode not supported. */
+    pfl->cfi_table[0x0c + pri_ofs] = 0x00;
+    assert(0x0c + pri_ofs < ARRAY_SIZE(pfl->cfi_table));
+}
+
 static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 {
     ERRP_GUARD();
@@ -837,100 +935,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pfl->cmd = 0;
     pfl->status = 0;
 
-    /* Hardcoded CFI table (mostly from SG29 Spansion flash) */
-    const uint16_t pri_ofs = 0x40;
-    /* Standard "QRY" string */
-    pfl->cfi_table[0x10] = 'Q';
-    pfl->cfi_table[0x11] = 'R';
-    pfl->cfi_table[0x12] = 'Y';
-    /* Command set (AMD/Fujitsu) */
-    pfl->cfi_table[0x13] = 0x02;
-    pfl->cfi_table[0x14] = 0x00;
-    /* Primary extended table address */
-    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;
-    /* Vcc min */
-    pfl->cfi_table[0x1B] = 0x27;
-    /* Vcc max */
-    pfl->cfi_table[0x1C] = 0x36;
-    /* Vpp min (no Vpp pin) */
-    pfl->cfi_table[0x1D] = 0x00;
-    /* Vpp max (no Vpp pin) */
-    pfl->cfi_table[0x1E] = 0x00;
-    /* Timeout per single byte/word write (128 ms) */
-    pfl->cfi_table[0x1F] = 0x07;
-    /* Timeout for min size buffer write (NA) */
-    pfl->cfi_table[0x20] = 0x00;
-    /* Typical timeout for block erase (512 ms) */
-    pfl->cfi_table[0x21] = 0x09;
-    /* Typical timeout for full chip erase (4096 ms) */
-    pfl->cfi_table[0x22] = 0x0C;
-    /* Reserved */
-    pfl->cfi_table[0x23] = 0x01;
-    /* Max timeout for buffer write (NA) */
-    pfl->cfi_table[0x24] = 0x00;
-    /* Max timeout for block erase */
-    pfl->cfi_table[0x25] = 0x0A;
-    /* Max timeout for chip erase */
-    pfl->cfi_table[0x26] = 0x0D;
-    /* Device size */
-    pfl->cfi_table[0x27] = ctz32(pfl->chip_len);
-    /* Flash device interface (8 & 16 bits) */
-    pfl->cfi_table[0x28] = 0x02;
-    pfl->cfi_table[0x29] = 0x00;
-    /* Max number of bytes in multi-bytes write */
-    /*
-     * XXX: disable buffered write as it's not supported
-     * pfl->cfi_table[0x2A] = 0x05;
-     */
-    pfl->cfi_table[0x2A] = 0x00;
-    pfl->cfi_table[0x2B] = 0x00;
-    /* Number of erase block regions */
-    pfl->cfi_table[0x2c] = nb_regions;
-    /* Erase block regions */
-    for (int i = 0; i < nb_regions; ++i) {
-        uint32_t sector_len_per_device = pfl->sector_len[i];
-        pfl->cfi_table[0x2d + 4 * i] = pfl->nb_blocs[i] - 1;
-        pfl->cfi_table[0x2e + 4 * i] = (pfl->nb_blocs[i] - 1) >> 8;
-        pfl->cfi_table[0x2f + 4 * i] = sector_len_per_device >> 8;
-        pfl->cfi_table[0x30 + 4 * i] = sector_len_per_device >> 16;
-    }
-    assert(0x2c + 4 * nb_regions < pri_ofs);
-
-    /* Extended */
-    pfl->cfi_table[0x00 + pri_ofs] = 'P';
-    pfl->cfi_table[0x01 + pri_ofs] = 'R';
-    pfl->cfi_table[0x02 + pri_ofs] = 'I';
-
-    /* Extended version 1.0 */
-    pfl->cfi_table[0x03 + pri_ofs] = '1';
-    pfl->cfi_table[0x04 + pri_ofs] = '0';
-
-    /* Address sensitive unlock required. */
-    pfl->cfi_table[0x05 + pri_ofs] = 0x00;
-    /* Erase suspend to read/write. */
-    pfl->cfi_table[0x06 + pri_ofs] = 0x02;
-    /* Sector protect not supported. */
-    pfl->cfi_table[0x07 + pri_ofs] = 0x00;
-    /* Temporary sector unprotect not supported. */
-    pfl->cfi_table[0x08 + pri_ofs] = 0x00;
-
-    /* Sector protect/unprotect scheme. */
-    pfl->cfi_table[0x09 + pri_ofs] = 0x00;
-
-    /* Simultaneous operation not supported. */
-    pfl->cfi_table[0x0a + pri_ofs] = 0x00;
-    /* Burst mode not supported. */
-    pfl->cfi_table[0x0b + pri_ofs] = 0x00;
-    /* Page mode not supported. */
-    pfl->cfi_table[0x0c + pri_ofs] = 0x00;
-    assert(0x0c + pri_ofs < ARRAY_SIZE(pfl->cfi_table));
+    pflash_cfi02_fill_cfi_table(pfl, nb_regions);
 }
 
 static Property pflash_cfi02_properties[] = {
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 04/11] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings()
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 03/11] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table() Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 05/11] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false) Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

There is only one call to pflash_setup_mappings(). Convert 'rom_mode'
to boolean and set it to true directly within pflash_setup_mappings().

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Message-Id: <20210310170528.1184868-5-philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 845f50ed99b..0eb868ecd3d 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -108,7 +108,7 @@ struct PFlashCFI02 {
     MemoryRegion mem;
     MemoryRegion *mem_mappings;    /* array; one per mapping */
     MemoryRegion orig_mem;
-    int rom_mode;
+    bool rom_mode;
     int read_counter; /* used for lazy switch-back to rom mode */
     int sectors_to_erase;
     uint64_t erase_time_remaining;
@@ -181,12 +181,13 @@ static void pflash_setup_mappings(PFlashCFI02 *pfl)
                                  "pflash-alias", &pfl->orig_mem, 0, size);
         memory_region_add_subregion(&pfl->mem, i * size, &pfl->mem_mappings[i]);
     }
+    pfl->rom_mode = true;
 }
 
 static void pflash_register_memory(PFlashCFI02 *pfl, int rom_mode)
 {
     memory_region_rom_device_set_romd(&pfl->orig_mem, rom_mode);
-    pfl->rom_mode = rom_mode;
+    pfl->rom_mode = !!rom_mode;
 }
 
 static size_t pflash_regions_count(PFlashCFI02 *pfl)
@@ -927,7 +928,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pfl->sector_erase_map = bitmap_new(pfl->total_sectors);
 
     pflash_setup_mappings(pfl);
-    pfl->rom_mode = 1;
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
 
     timer_init_ns(&pfl->timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 05/11] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false)
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 04/11] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings() Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 06/11] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

There is only one call to pflash_register_memory() with
rom_mode == false. As we want to modify pflash_register_memory()
in the next patch, open-code this trivial function in place for
the 'rom_mode == false' case.

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Message-Id: <20210310170528.1184868-6-philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 0eb868ecd3d..897b7333222 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -467,8 +467,10 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
     switch (pfl->wcycle) {
     case 0:
         /* Set the device in I/O access mode if required */
-        if (pfl->rom_mode)
-            pflash_register_memory(pfl, 0);
+        if (pfl->rom_mode) {
+            pfl->rom_mode = false;
+            memory_region_rom_device_set_romd(&pfl->orig_mem, false);
+        }
         pfl->read_counter = 0;
         /* We're in read mode */
     check_unlock0:
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 06/11] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 05/11] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false) Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 07/11] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	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 three times:

- When the timer expires and not in bypass mode

- On a read access (on invalid command).

- When the device is initialized. Here the ROMD mode is hidden
  by the memory_region_init_rom_device() call.

pflash_register_memory(rom_mode=true) already sets the ROM device
in "read array" mode (from I/O device to ROM one). Explicit that
by renaming the function as pflash_mode_read_array(), adding
a trace event and resetting wcycle.

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210310170528.1184868-7-philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 18 +++++++++---------
 hw/block/trace-events   |  1 +
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 897b7333222..2ba77a0171b 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -184,10 +184,13 @@ static void pflash_setup_mappings(PFlashCFI02 *pfl)
     pfl->rom_mode = true;
 }
 
-static void pflash_register_memory(PFlashCFI02 *pfl, int rom_mode)
+static void pflash_mode_read_array(PFlashCFI02 *pfl)
 {
-    memory_region_rom_device_set_romd(&pfl->orig_mem, rom_mode);
-    pfl->rom_mode = !!rom_mode;
+    trace_pflash_mode_read_array();
+    pfl->cmd = 0x00;
+    pfl->wcycle = 0;
+    pfl->rom_mode = true;
+    memory_region_rom_device_set_romd(&pfl->orig_mem, true);
 }
 
 static size_t pflash_regions_count(PFlashCFI02 *pfl)
@@ -249,11 +252,10 @@ static void pflash_timer(void *opaque)
     toggle_dq7(pfl);
     if (pfl->bypass) {
         pfl->wcycle = 2;
+        pfl->cmd = 0;
     } else {
-        pflash_register_memory(pfl, 1);
-        pfl->wcycle = 0;
+        pflash_mode_read_array(pfl);
     }
-    pfl->cmd = 0;
 }
 
 /*
@@ -315,7 +317,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
     /* Lazy reset to ROMD mode after a certain amount of read accesses */
     if (!pfl->rom_mode && pfl->wcycle == 0 &&
         ++pfl->read_counter > PFLASH_LAZY_ROMD_THRESHOLD) {
-        pflash_register_memory(pfl, 1);
+        pflash_mode_read_array(pfl);
     }
     offset &= pfl->chip_len - 1;
     boff = offset & 0xFF;
@@ -933,8 +935,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
 
     timer_init_ns(&pfl->timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
-    pfl->wcycle = 0;
-    pfl->cmd = 0;
     pfl->status = 0;
 
     pflash_cfi02_fill_cfi_table(pfl, nb_regions);
diff --git a/hw/block/trace-events b/hw/block/trace-events
index ef06d2ea747..6d0c43f9977 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_cfi01.c
 # pflash_cfi02.c
 pflash_reset(void) "reset"
+pflash_mode_read_array(void) "mode: read array"
 pflash_timer_expired(uint8_t cmd) "command 0x%02x done"
 pflash_io_read(uint64_t offset, unsigned size, uint32_t value, uint8_t cmd, uint8_t wcycle) "offset:0x%04"PRIx64" size:%u value:0x%04x cmd:0x%02x wcycle:%u"
 pflash_io_write(uint64_t offset, unsigned size, uint32_t value, uint8_t wcycle) "offset:0x%04"PRIx64" size:%u value:0x%04x wcycle:%u"
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 07/11] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine()
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 06/11] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 08/11] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

There is multiple places resetting the internal state machine.
Factor the code out in a new pflash_reset_state_machine() method.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210310170528.1184868-8-philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2ba77a0171b..aea47a99c61 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -184,11 +184,17 @@ static void pflash_setup_mappings(PFlashCFI02 *pfl)
     pfl->rom_mode = true;
 }
 
+static void pflash_reset_state_machine(PFlashCFI02 *pfl)
+{
+    trace_pflash_reset();
+    pfl->cmd = 0x00;
+    pfl->wcycle = 0;
+}
+
 static void pflash_mode_read_array(PFlashCFI02 *pfl)
 {
     trace_pflash_mode_read_array();
-    pfl->cmd = 0x00;
-    pfl->wcycle = 0;
+    pflash_reset_state_machine(pfl);
     pfl->rom_mode = true;
     memory_region_rom_device_set_romd(&pfl->orig_mem, true);
 }
@@ -330,8 +336,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
     default:
         /* This should never happen : reset state & treat it as a read*/
         DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
-        pfl->wcycle = 0;
-        pfl->cmd = 0;
+        pflash_reset_state_machine(pfl);
         /* fall through to the read code */
     case 0x80: /* Erase (unlock) */
         /* We accept reads during second unlock sequence... */
@@ -669,8 +674,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
                 }
                 reset_dq3(pfl);
                 timer_del(&pfl->timer);
-                pfl->wcycle = 0;
-                pfl->cmd = 0;
+                pflash_reset_state_machine(pfl);
                 return;
             }
             /*
@@ -710,10 +714,8 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
 
     /* Reset flash */
  reset_flash:
-    trace_pflash_reset();
     pfl->bypass = 0;
-    pfl->wcycle = 0;
-    pfl->cmd = 0;
+    pflash_reset_state_machine(pfl);
     return;
 
  do_bypass:
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 08/11] hw/block/pflash_cfi02: Add DeviceReset method
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 07/11] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 09/11] hw/block/pflash_cfi01: Clarify trace events Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210310170528.1184868-9-philmd@redhat.com>
---
 hw/block/pflash_cfi02.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index aea47a99c61..c40febd2a41 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -942,6 +942,13 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pflash_cfi02_fill_cfi_table(pfl, nb_regions);
 }
 
+static void pflash_cfi02_reset(DeviceState *dev)
+{
+    PFlashCFI02 *pfl = PFLASH_CFI02(dev);
+
+    pflash_reset_state_machine(pfl);
+}
+
 static Property pflash_cfi02_properties[] = {
     DEFINE_PROP_DRIVE("drive", PFlashCFI02, blk),
     DEFINE_PROP_UINT32("num-blocks", PFlashCFI02, uniform_nb_blocs, 0),
@@ -979,6 +986,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = pflash_cfi02_realize;
+    dc->reset = pflash_cfi02_reset;
     dc->unrealize = pflash_cfi02_unrealize;
     device_class_set_props(dc, pflash_cfi02_properties);
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 09/11] hw/block/pflash_cfi01: Clarify trace events
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (7 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 08/11] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 10/11] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

Use the 'mode_read_array' event when we set the device in such
mode, and use the 'reset' event in DeviceReset handler.

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210310170528.1184868-10-philmd@redhat.com>
---
 hw/block/pflash_cfi01.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 779a62f3b06..c8cecb3ac8b 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -663,7 +663,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                   "\n", __func__, offset, pfl->wcycle, pfl->cmd, value);
 
  mode_read_array:
-    trace_pflash_reset();
+    trace_pflash_mode_read_array();
     memory_region_rom_device_set_romd(&pfl->mem, true);
     pfl->wcycle = 0;
     pfl->cmd = 0x00; /* This model reset value for READ_ARRAY (not CFI) */
@@ -886,6 +886,7 @@ static void pflash_cfi01_system_reset(DeviceState *dev)
 {
     PFlashCFI01 *pfl = PFLASH_CFI01(dev);
 
+    trace_pflash_reset();
     /*
      * The command 0x00 is not assigned by the CFI open standard,
      * but QEMU historically uses it for the READ_ARRAY command (0xff).
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 10/11] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 09/11] hw/block/pflash_cfi01: Clarify trace events Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-15 23:35 ` [PULL 11/11] hw/block/pflash_cfi: Replace DPRINTF with trace events Philippe Mathieu-Daudé
  2021-03-17 13:37 ` [PULL 00/11] pflash patches for 2021-03-16 Peter Maydell
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

From: David Edmondson <david.edmondson@oracle.com>

PFlashCFI01.ro is a bool, declare it as such.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210216142721.1985543-3-david.edmondson@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 hw/block/pflash_cfi01.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index c8cecb3ac8b..07910356756 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -82,7 +82,7 @@ struct PFlashCFI01 {
     uint8_t max_device_width;  /* max device width in bytes */
     uint32_t features;
     uint8_t wcycle; /* if 0, the flash is read normally */
-    int ro;
+    bool ro;
     uint8_t cmd;
     uint8_t status;
     uint16_t ident0;
@@ -853,7 +853,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
             return;
         }
     } else {
-        pfl->ro = 0;
+        pfl->ro = false;
     }
 
     if (pfl->blk) {
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 11/11] hw/block/pflash_cfi: Replace DPRINTF with trace events
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (9 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 10/11] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro Philippe Mathieu-Daudé
@ 2021-03-15 23:35 ` Philippe Mathieu-Daudé
  2021-03-17 13:37 ` [PULL 00/11] pflash patches for 2021-03-16 Peter Maydell
  11 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Max Reitz, David Edmondson, Bin Meng,
	Philippe Mathieu-Daudé

From: David Edmondson <david.edmondson@oracle.com>

Rather than having a device specific debug implementation in
pflash_cfi01.c and pflash_cfi02.c, use the standard tracing facility.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210216142721.1985543-2-david.edmondson@oracle.com>
[PMD: Rebased]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 hw/block/pflash_cfi01.c | 80 +++++++++++++++++------------------------
 hw/block/pflash_cfi02.c | 78 ++++++++++++++++------------------------
 hw/block/trace-events   | 41 +++++++++++++++------
 3 files changed, 95 insertions(+), 104 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 07910356756..81f9f971d88 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -56,16 +56,6 @@
 #include "sysemu/runstate.h"
 #include "trace.h"
 
-/* #define PFLASH_DEBUG */
-#ifdef PFLASH_DEBUG
-#define DPRINTF(fmt, ...)                                   \
-do {                                                        \
-    fprintf(stderr, "PFLASH: " fmt , ## __VA_ARGS__);       \
-} while (0)
-#else
-#define DPRINTF(fmt, ...) do { } while (0)
-#endif
-
 #define PFLASH_BE          0
 #define PFLASH_SECURE      1
 
@@ -155,10 +145,8 @@ static uint32_t pflash_cfi_query(PFlashCFI01 *pfl, hwaddr offset)
          * wider part.
          */
         if (pfl->device_width != 1 || pfl->bank_width > 4) {
-            DPRINTF("%s: Unsupported device configuration: "
-                    "device_width=%d, max_device_width=%d\n",
-                    __func__, pfl->device_width,
-                    pfl->max_device_width);
+            trace_pflash_unsupported_device_configuration(pfl->name,
+                                    pfl->device_width, pfl->max_device_width);
             return 0;
         }
         /* CFI query data is repeated, rather than zero padded for
@@ -210,14 +198,14 @@ static uint32_t pflash_devid_query(PFlashCFI01 *pfl, hwaddr offset)
     switch (boff & 0xFF) {
     case 0:
         resp = pfl->ident0;
-        trace_pflash_manufacturer_id(resp);
+        trace_pflash_manufacturer_id(pfl->name, resp);
         break;
     case 1:
         resp = pfl->ident1;
-        trace_pflash_device_id(resp);
+        trace_pflash_device_id(pfl->name, resp);
         break;
     default:
-        trace_pflash_device_info(offset);
+        trace_pflash_device_info(pfl->name, offset);
         return 0;
     }
     /* Replicate responses for each device in bank. */
@@ -265,10 +253,9 @@ static uint32_t pflash_data_read(PFlashCFI01 *pfl, hwaddr offset,
         }
         break;
     default:
-        DPRINTF("BUG in %s\n", __func__);
         abort();
     }
-    trace_pflash_data_read(offset, width, ret);
+    trace_pflash_data_read(pfl->name, offset, width, ret);
     return ret;
 }
 
@@ -282,7 +269,7 @@ 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);
+        trace_pflash_read_unknown_state(pfl->name, pfl->cmd);
         pfl->wcycle = 0;
         /*
          * The command 0x00 is not assigned by the CFI open standard,
@@ -320,7 +307,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
              */
             ret |= pfl->status << 16;
         }
-        DPRINTF("%s: status %x\n", __func__, ret);
+        trace_pflash_read_status(pfl->name, ret);
         break;
     case 0x90:
         if (!pfl->device_width) {
@@ -335,14 +322,14 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
             switch (boff) {
             case 0:
                 ret = pfl->ident0 << 8 | pfl->ident1;
-                trace_pflash_manufacturer_id(ret);
+                trace_pflash_manufacturer_id(pfl->name, ret);
                 break;
             case 1:
                 ret = pfl->ident2 << 8 | pfl->ident3;
-                trace_pflash_device_id(ret);
+                trace_pflash_device_id(pfl->name, ret);
                 break;
             default:
-                trace_pflash_device_info(boff);
+                trace_pflash_device_info(pfl->name, boff);
                 ret = 0;
                 break;
             }
@@ -389,7 +376,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
 
         break;
     }
-    trace_pflash_io_read(offset, width, ret, pfl->cmd, pfl->wcycle);
+    trace_pflash_io_read(pfl->name, offset, width, ret, pfl->cmd, pfl->wcycle);
 
     return ret;
 }
@@ -419,7 +406,7 @@ static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset,
 {
     uint8_t *p = pfl->storage;
 
-    trace_pflash_data_write(offset, width, value, pfl->counter);
+    trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
     switch (width) {
     case 1:
         p[offset] = value;
@@ -458,7 +445,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 
     cmd = value;
 
-    trace_pflash_io_write(offset, width, value, pfl->wcycle);
+    trace_pflash_io_write(pfl->name, offset, width, value, pfl->wcycle);
     if (!pfl->wcycle) {
         /* Set the device in I/O access mode */
         memory_region_rom_device_set_romd(&pfl->mem, false);
@@ -472,14 +459,13 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             goto mode_read_array;
         case 0x10: /* Single Byte Program */
         case 0x40: /* Single Byte Program */
-            DPRINTF("%s: Single Byte Program\n", __func__);
+            trace_pflash_write(pfl->name, "single byte program (0)");
             break;
         case 0x20: /* Block erase */
             p = pfl->storage;
             offset &= ~(pfl->sector_len - 1);
 
-            DPRINTF("%s: block erase at " TARGET_FMT_plx " bytes %x\n",
-                    __func__, offset, (unsigned)pfl->sector_len);
+            trace_pflash_write_block_erase(pfl->name, offset, pfl->sector_len);
 
             if (!pfl->ro) {
                 memset(p + offset, 0xff, pfl->sector_len);
@@ -490,25 +476,25 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             pfl->status |= 0x80; /* Ready! */
             break;
         case 0x50: /* Clear status bits */
-            DPRINTF("%s: Clear status bits\n", __func__);
+            trace_pflash_write(pfl->name, "clear status bits");
             pfl->status = 0x0;
             goto mode_read_array;
         case 0x60: /* Block (un)lock */
-            DPRINTF("%s: Block unlock\n", __func__);
+            trace_pflash_write(pfl->name, "block unlock");
             break;
         case 0x70: /* Status Register */
-            DPRINTF("%s: Read status register\n", __func__);
+            trace_pflash_write(pfl->name, "read status register");
             pfl->cmd = cmd;
             return;
         case 0x90: /* Read Device ID */
-            DPRINTF("%s: Read Device information\n", __func__);
+            trace_pflash_write(pfl->name, "read device information");
             pfl->cmd = cmd;
             return;
         case 0x98: /* CFI query */
-            DPRINTF("%s: CFI query\n", __func__);
+            trace_pflash_write(pfl->name, "CFI query");
             break;
         case 0xe8: /* Write to buffer */
-            DPRINTF("%s: Write to buffer\n", __func__);
+            trace_pflash_write(pfl->name, "write to buffer");
             /* FIXME should save @offset, @width for case 1+ */
             qemu_log_mask(LOG_UNIMP,
                           "%s: Write to buffer emulation is flawed\n",
@@ -516,10 +502,10 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             pfl->status |= 0x80; /* Ready! */
             break;
         case 0xf0: /* Probe for AMD flash */
-            DPRINTF("%s: Probe for AMD flash\n", __func__);
+            trace_pflash_write(pfl->name, "probe for AMD flash");
             goto mode_read_array;
         case 0xff: /* Read Array */
-            DPRINTF("%s: Read array mode\n", __func__);
+            trace_pflash_write(pfl->name, "read array mode");
             goto mode_read_array;
         default:
             goto error_flash;
@@ -531,7 +517,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
         switch (pfl->cmd) {
         case 0x10: /* Single Byte Program */
         case 0x40: /* Single Byte Program */
-            DPRINTF("%s: Single Byte Program\n", __func__);
+            trace_pflash_write(pfl->name, "single byte program (1)");
             if (!pfl->ro) {
                 pflash_data_write(pfl, offset, value, width, be);
                 pflash_update(pfl, offset, width);
@@ -563,7 +549,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             } else {
                 value = extract32(value, 0, pfl->bank_width * 8);
             }
-            DPRINTF("%s: block write of %x bytes\n", __func__, value);
+            trace_pflash_write_block(pfl->name, value);
             pfl->counter = value;
             pfl->wcycle++;
             break;
@@ -577,7 +563,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             } else if (cmd == 0xff) { /* Read Array */
                 goto mode_read_array;
             } else {
-                DPRINTF("%s: Unknown (un)locking command\n", __func__);
+                trace_pflash_write(pfl->name, "unknown (un)locking command");
                 goto mode_read_array;
             }
             break;
@@ -585,7 +571,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
             if (cmd == 0xff) { /* Read Array */
                 goto mode_read_array;
             } else {
-                DPRINTF("%s: leaving query mode\n", __func__);
+                trace_pflash_write(pfl->name, "leaving query mode");
             }
             break;
         default:
@@ -613,7 +599,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                 hwaddr mask = pfl->writeblock_size - 1;
                 mask = ~mask;
 
-                DPRINTF("%s: block write finished\n", __func__);
+                trace_pflash_write(pfl->name, "block write finished");
                 pfl->wcycle++;
                 if (!pfl->ro) {
                     /* Flush the entire write buffer onto backing storage.  */
@@ -652,7 +638,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
         break;
     default:
         /* Should never happen */
-        DPRINTF("%s: invalid write state\n",  __func__);
+        trace_pflash_write(pfl->name, "invalid write state");
         goto mode_read_array;
     }
     return;
@@ -663,7 +649,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                   "\n", __func__, offset, pfl->wcycle, pfl->cmd, value);
 
  mode_read_array:
-    trace_pflash_mode_read_array();
+    trace_pflash_mode_read_array(pfl->name);
     memory_region_rom_device_set_romd(&pfl->mem, true);
     pfl->wcycle = 0;
     pfl->cmd = 0x00; /* This model reset value for READ_ARRAY (not CFI) */
@@ -886,7 +872,7 @@ static void pflash_cfi01_system_reset(DeviceState *dev)
 {
     PFlashCFI01 *pfl = PFLASH_CFI01(dev);
 
-    trace_pflash_reset();
+    trace_pflash_reset(pfl->name);
     /*
      * The command 0x00 is not assigned by the CFI open standard,
      * but QEMU historically uses it for the READ_ARRAY command (0xff).
@@ -1041,7 +1027,7 @@ static void postload_update_cb(void *opaque, bool running, RunState state)
     qemu_del_vm_change_state_handler(pfl->vmstate);
     pfl->vmstate = NULL;
 
-    DPRINTF("%s: updating bdrv for %s\n", __func__, pfl->name);
+    trace_pflash_postload_cb(pfl->name);
     pflash_update(pfl, 0, pfl->sector_len * pfl->nb_blocs);
 }
 
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index c40febd2a41..25c053693ce 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -48,14 +48,6 @@
 #include "migration/vmstate.h"
 #include "trace.h"
 
-#define PFLASH_DEBUG false
-#define DPRINTF(fmt, ...)                                  \
-do {                                                       \
-    if (PFLASH_DEBUG) {                                    \
-        fprintf(stderr, "PFLASH: " fmt, ## __VA_ARGS__);   \
-    }                                                      \
-} while (0)
-
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
 /*
@@ -186,14 +178,14 @@ static void pflash_setup_mappings(PFlashCFI02 *pfl)
 
 static void pflash_reset_state_machine(PFlashCFI02 *pfl)
 {
-    trace_pflash_reset();
+    trace_pflash_reset(pfl->name);
     pfl->cmd = 0x00;
     pfl->wcycle = 0;
 }
 
 static void pflash_mode_read_array(PFlashCFI02 *pfl)
 {
-    trace_pflash_mode_read_array();
+    trace_pflash_mode_read_array(pfl->name);
     pflash_reset_state_machine(pfl);
     pfl->rom_mode = true;
     memory_region_rom_device_set_romd(&pfl->orig_mem, true);
@@ -231,7 +223,7 @@ static void pflash_timer(void *opaque)
 {
     PFlashCFI02 *pfl = opaque;
 
-    trace_pflash_timer_expired(pfl->cmd);
+    trace_pflash_timer_expired(pfl->name, pfl->cmd);
     if (pfl->cmd == 0x30) {
         /*
          * Sector erase. If DQ3 is 0 when the timer expires, then the 50
@@ -244,11 +236,10 @@ static void pflash_timer(void *opaque)
             uint64_t timeout = pflash_erase_time(pfl);
             timer_mod(&pfl->timer,
                       qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + timeout);
-            DPRINTF("%s: erase timeout fired; erasing %d sectors\n",
-                    __func__, pfl->sectors_to_erase);
+            trace_pflash_erase_timeout(pfl->name, pfl->sectors_to_erase);
             return;
         }
-        DPRINTF("%s: sector erase complete\n", __func__);
+        trace_pflash_erase_complete(pfl->name);
         bitmap_zero(pfl->sector_erase_map, pfl->total_sectors);
         pfl->sectors_to_erase = 0;
         reset_dq3(pfl);
@@ -272,7 +263,7 @@ static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr offset,
 {
     uint8_t *p = (uint8_t *)pfl->storage + offset;
     uint64_t ret = pfl->be ? ldn_be_p(p, width) : ldn_le_p(p, width);
-    trace_pflash_data_read(offset, width, ret);
+    trace_pflash_data_read(pfl->name, offset, width, ret);
     return ret;
 }
 
@@ -335,7 +326,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
     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);
+        trace_pflash_read_unknown_state(pfl->name, pfl->cmd);
         pflash_reset_state_machine(pfl);
         /* fall through to the read code */
     case 0x80: /* Erase (unlock) */
@@ -347,7 +338,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
             toggle_dq2(pfl);
             /* Status register read */
             ret = pfl->status;
-            DPRINTF("%s: status %" PRIx64 "\n", __func__, ret);
+            trace_pflash_read_status(pfl->name, ret);
             break;
         }
         /* Flash area read */
@@ -372,7 +363,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
         default:
             ret = pflash_data_read(pfl, offset, width);
         }
-        DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx64 "\n", __func__, boff, ret);
+        trace_pflash_read_done(pfl->name, boff, ret);
         break;
     case 0x10: /* Chip Erase */
     case 0x30: /* Sector Erase */
@@ -384,7 +375,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
         toggle_dq6(pfl);
         /* Status register read */
         ret = pfl->status;
-        DPRINTF("%s: status %" PRIx64 "\n", __func__, ret);
+        trace_pflash_read_status(pfl->name, ret);
         break;
     case 0x98:
         /* CFI query mode */
@@ -395,7 +386,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
         }
         break;
     }
-    trace_pflash_io_read(offset, width, ret, pfl->cmd, pfl->wcycle);
+    trace_pflash_io_read(pfl->name, offset, width, ret, pfl->cmd, pfl->wcycle);
 
     return ret;
 }
@@ -424,9 +415,8 @@ static void pflash_sector_erase(PFlashCFI02 *pfl, hwaddr offset)
     SectorInfo sector_info = pflash_sector_info(pfl, offset);
     uint64_t sector_len = sector_info.len;
     offset &= ~(sector_len - 1);
-    DPRINTF("%s: start sector erase at %0*" PRIx64 "-%0*" PRIx64 "\n",
-            __func__, pfl->width * 2, offset,
-            pfl->width * 2, offset + sector_len - 1);
+    trace_pflash_sector_erase_start(pfl->name, pfl->width * 2, offset,
+                                    pfl->width * 2, offset + sector_len - 1);
     if (!pfl->ro) {
         uint8_t *p = pfl->storage;
         memset(p + offset, 0xff, sector_len);
@@ -447,7 +437,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
     uint8_t *p;
     uint8_t cmd;
 
-    trace_pflash_io_write(offset, width, value, pfl->wcycle);
+    trace_pflash_io_write(pfl->name, offset, width, value, pfl->wcycle);
     cmd = value;
     if (pfl->cmd != 0xA0) {
         /* Reset does nothing during chip erase and sector erase. */
@@ -507,27 +497,25 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
             return;
         }
         if (boff != pfl->unlock_addr0 || cmd != 0xAA) {
-            DPRINTF("%s: unlock0 failed " TARGET_FMT_plx " %02x %04x\n",
-                    __func__, boff, cmd, pfl->unlock_addr0);
+            trace_pflash_unlock0_failed(pfl->name, boff,
+                                        cmd, pfl->unlock_addr0);
             goto reset_flash;
         }
-        DPRINTF("%s: unlock sequence started\n", __func__);
+        trace_pflash_write(pfl->name, "unlock sequence started");
         break;
     case 1:
         /* We started an unlock sequence */
     check_unlock1:
         if (boff != pfl->unlock_addr1 || cmd != 0x55) {
-            DPRINTF("%s: unlock1 failed " TARGET_FMT_plx " %02x\n", __func__,
-                    boff, cmd);
+            trace_pflash_unlock1_failed(pfl->name, boff, cmd);
             goto reset_flash;
         }
-        DPRINTF("%s: unlock sequence done\n", __func__);
+        trace_pflash_write(pfl->name, "unlock sequence done");
         break;
     case 2:
         /* We finished an unlock sequence */
         if (!pfl->bypass && boff != pfl->unlock_addr0) {
-            DPRINTF("%s: command failed " TARGET_FMT_plx " %02x\n", __func__,
-                    boff, cmd);
+            trace_pflash_write_failed(pfl->name, boff, cmd);
             goto reset_flash;
         }
         switch (cmd) {
@@ -538,10 +526,10 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
         case 0x90: /* Autoselect */
         case 0xA0: /* Program */
             pfl->cmd = cmd;
-            DPRINTF("%s: starting command %02x\n", __func__, cmd);
+            trace_pflash_write_start(pfl->name, cmd);
             break;
         default:
-            DPRINTF("%s: unknown command %02x\n", __func__, cmd);
+            trace_pflash_write_unknown(pfl->name, cmd);
             goto reset_flash;
         }
         break;
@@ -559,7 +547,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
                 }
                 goto reset_flash;
             }
-            trace_pflash_data_write(offset, width, value, 0);
+            trace_pflash_data_write(pfl->name, offset, width, value, 0);
             if (!pfl->ro) {
                 p = (uint8_t *)pfl->storage + offset;
                 if (pfl->be) {
@@ -597,8 +585,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
             }
             /* fall through */
         default:
-            DPRINTF("%s: invalid write for command %02x\n",
-                    __func__, pfl->cmd);
+            trace_pflash_write_invalid(pfl->name, pfl->cmd);
             goto reset_flash;
         }
     case 4:
@@ -611,8 +598,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
             goto check_unlock1;
         default:
             /* Should never happen */
-            DPRINTF("%s: invalid command state %02x (wc 4)\n",
-                    __func__, pfl->cmd);
+            trace_pflash_write_invalid_state(pfl->name, pfl->cmd, 5);
             goto reset_flash;
         }
         break;
@@ -624,12 +610,11 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
         switch (cmd) {
         case 0x10: /* Chip Erase */
             if (boff != pfl->unlock_addr0) {
-                DPRINTF("%s: chip erase: invalid address " TARGET_FMT_plx "\n",
-                        __func__, offset);
+                trace_pflash_chip_erase_invalid(pfl->name, offset);
                 goto reset_flash;
             }
             /* Chip erase */
-            DPRINTF("%s: start chip erase\n", __func__);
+            trace_pflash_chip_erase_start(pfl->name);
             if (!pfl->ro) {
                 memset(pfl->storage, 0xff, pfl->chip_len);
                 pflash_update(pfl, 0, pfl->chip_len);
@@ -643,7 +628,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
             pflash_sector_erase(pfl, offset);
             break;
         default:
-            DPRINTF("%s: invalid command %02x (wc 5)\n", __func__, cmd);
+            trace_pflash_write_invalid_command(pfl->name, cmd);
             goto reset_flash;
         }
         pfl->cmd = cmd;
@@ -693,19 +678,18 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
             return;
         default:
             /* Should never happen */
-            DPRINTF("%s: invalid command state %02x (wc 6)\n",
-                    __func__, pfl->cmd);
+            trace_pflash_write_invalid_state(pfl->name, pfl->cmd, 6);
             goto reset_flash;
         }
         break;
     /* Special values for CFI queries */
     case WCYCLE_CFI:
     case WCYCLE_AUTOSELECT_CFI:
-        DPRINTF("%s: invalid write in CFI query mode\n", __func__);
+        trace_pflash_write(pfl->name, "invalid write in CFI query mode");
         goto reset_flash;
     default:
         /* Should never happen */
-        DPRINTF("%s: invalid write state (wc 7)\n",  __func__);
+        trace_pflash_write(pfl->name, "invalid write state (wc 7)");
         goto reset_flash;
     }
     pfl->wcycle++;
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 6d0c43f9977..1e0d6108b9d 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -6,16 +6,37 @@ fdc_ioport_write(uint8_t reg, uint8_t value) "write reg 0x%02x val 0x%02x"
 
 # pflash_cfi01.c
 # pflash_cfi02.c
-pflash_reset(void) "reset"
-pflash_mode_read_array(void) "mode: read array"
-pflash_timer_expired(uint8_t cmd) "command 0x%02x done"
-pflash_io_read(uint64_t offset, unsigned size, uint32_t value, uint8_t cmd, uint8_t wcycle) "offset:0x%04"PRIx64" size:%u value:0x%04x cmd:0x%02x wcycle:%u"
-pflash_io_write(uint64_t offset, unsigned size, uint32_t value, uint8_t wcycle) "offset:0x%04"PRIx64" size:%u value:0x%04x wcycle:%u"
-pflash_data_read(uint64_t offset, unsigned size, uint32_t value) "data offset:0x%04"PRIx64" size:%u value:0x%04x"
-pflash_data_write(uint64_t offset, unsigned size, uint32_t value, uint64_t counter) "data offset:0x%04"PRIx64" size:%u value:0x%04x counter:0x%016"PRIx64
-pflash_manufacturer_id(uint16_t id) "Read Manufacturer ID: 0x%04x"
-pflash_device_id(uint16_t id) "Read Device ID: 0x%04x"
-pflash_device_info(uint64_t offset) "Read Device Information offset:0x%04"PRIx64
+pflash_chip_erase_invalid(const char *name, uint64_t offset) "%s: chip erase: invalid address 0x%" PRIx64
+pflash_chip_erase_start(const char *name) "%s: start chip erase"
+pflash_data_read(const char *name, uint64_t offset, unsigned size, uint32_t value) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x"
+pflash_data_write(const char *name, uint64_t offset, unsigned size, uint32_t value, uint64_t counter) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x counter:0x%016"PRIx64
+pflash_device_id(const char *name, uint16_t id) "%s: read device ID: 0x%04x"
+pflash_device_info(const char *name, uint64_t offset) "%s: read device information offset:0x%04" PRIx64
+pflash_erase_complete(const char *name) "%s: sector erase complete"
+pflash_erase_timeout(const char *name, int count) "%s: erase timeout fired; erasing %d sectors"
+pflash_io_read(const char *name, uint64_t offset, unsigned int size, uint32_t value, uint8_t cmd, uint8_t wcycle) "%s: offset:0x%04" PRIx64 " size:%u value:0x%04x cmd:0x%02x wcycle:%u"
+pflash_io_write(const char *name, uint64_t offset, unsigned int size, uint32_t value, uint8_t wcycle) "%s: offset:0x%04"PRIx64" size:%u value:0x%04x wcycle:%u"
+pflash_manufacturer_id(const char *name, uint16_t id) "%s: read manufacturer ID: 0x%04x"
+pflash_mode_read_array(const char *name) "%s: read array mode"
+pflash_postload_cb(const char *name)  "%s: updating bdrv"
+pflash_read_done(const char *name, uint64_t offset, uint64_t ret) "%s: ID:0x%" PRIx64 " ret:0x%" PRIx64
+pflash_read_status(const char *name, uint32_t ret) "%s: status:0x%x"
+pflash_read_unknown_state(const char *name, uint8_t cmd) "%s: unknown command state:0x%x"
+pflash_reset(const char *name) "%s: reset"
+pflash_sector_erase_start(const char *name, int width1, uint64_t start, int width2, uint64_t end) "%s: start sector erase at: 0x%0*" PRIx64 "-0x%0*" PRIx64
+pflash_timer_expired(const char *name, uint8_t cmd) "%s: command 0x%02x done"
+pflash_unlock0_failed(const char *name, uint64_t offset, uint8_t cmd, uint16_t addr0) "%s: unlock0 failed 0x%" PRIx64 " 0x%02x 0x%04x"
+pflash_unlock1_failed(const char *name, uint64_t offset, uint8_t cmd) "%s: unlock0 failed 0x%" PRIx64 " 0x%02x"
+pflash_unsupported_device_configuration(const char *name, uint8_t width, uint8_t max) "%s: unsupported device configuration: device_width:%d max_device_width:%d"
+pflash_write(const char *name, const char *str) "%s: %s"
+pflash_write_block(const char *name, uint32_t value) "%s: block write: bytes:0x%x"
+pflash_write_block_erase(const char *name, uint64_t offset, uint64_t len) "%s: block erase offset:0x%" PRIx64 " bytes:0x%lx"
+pflash_write_failed(const char *name, uint64_t offset, uint8_t cmd) "%s: command failed 0x%" PRIx64 " 0x%02x"
+pflash_write_invalid(const char *name, uint8_t cmd) "%s: invalid write for command 0x%02x"
+pflash_write_invalid_command(const char *name, uint8_t cmd) "%s: invalid command 0x%02x (wc 5)"
+pflash_write_invalid_state(const char *name, uint8_t cmd, int wc) "%s: invalid command state 0x%02x (wc %d)"
+pflash_write_start(const char *name, uint8_t cmd) "%s: starting command 0x%02x"
+pflash_write_unknown(const char *name, uint8_t cmd) "%s: unknown command 0x%02x"
 
 # virtio-blk.c
 virtio_blk_req_complete(void *vdev, void *req, int status) "vdev %p req %p status %d"
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PULL 00/11] pflash patches for 2021-03-16
  2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
                   ` (10 preceding siblings ...)
  2021-03-15 23:35 ` [PULL 11/11] hw/block/pflash_cfi: Replace DPRINTF with trace events Philippe Mathieu-Daudé
@ 2021-03-17 13:37 ` Peter Maydell
  2021-03-17 14:22   ` Philippe Mathieu-Daudé
  11 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2021-03-17 13:37 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Kevin Wolf, QEMU Developers, Qemu-block, Max Reitz

On Mon, 15 Mar 2021 at 23:37, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339:
>
>   Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-03-15 19:23:00 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/pflash-20210316
>
> for you to fetch changes up to 3b6a1da064ac6ce5256f1f6f16870ea79c2422d0:
>
>   hw/block/pflash_cfi: Replace DPRINTF with trace events (2021-03-16 00:28:33 +0100)
>
> ----------------------------------------------------------------
> Parallel NOR Flash patches queue
>
> - Code movement to ease maintainability
> - Tracing improvements
> ----------------------------------------------------------------

Fails to build on 32-bit and OSX due to format string issues:

In file included from trace/trace-hw_block.c:5:
/Users/pm215/src/qemu-for-merges/hw/block/trace-events:36:38: error:
format specifies type 'unsigned long' but the argument has type
'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
                     , name, offset, len);
                                     ^~~
/Users/pm215/src/qemu-for-merges/hw/block/trace-events:33:121: error:
format specifies type 'unsigned long' but the argument has type
'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
            qemu_log("pflash_write_block_erase " "%s: block erase
offset:0x%" PRIx64 " bytes:0x%lx" "\n", name, offset, len);

                        ~~~                      ^~~

                        %llx


thanks
-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PULL 00/11] pflash patches for 2021-03-16
  2021-03-17 13:37 ` [PULL 00/11] pflash patches for 2021-03-16 Peter Maydell
@ 2021-03-17 14:22   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-17 14:22 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Kevin Wolf, QEMU Developers, Qemu-block, Max Reitz

On 3/17/21 2:37 PM, Peter Maydell wrote:
> On Mon, 15 Mar 2021 at 23:37, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> The following changes since commit 2615a5e433aeb812c300d3a48e1a88e1303e2339:
>>
>>   Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-03-15 19:23:00 +0000)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/philmd/qemu.git tags/pflash-20210316
>>
>> for you to fetch changes up to 3b6a1da064ac6ce5256f1f6f16870ea79c2422d0:
>>
>>   hw/block/pflash_cfi: Replace DPRINTF with trace events (2021-03-16 00:28:33 +0100)
>>
>> ----------------------------------------------------------------
>> Parallel NOR Flash patches queue
>>
>> - Code movement to ease maintainability
>> - Tracing improvements
>> ----------------------------------------------------------------
> 
> Fails to build on 32-bit and OSX due to format string issues:
> 
> In file included from trace/trace-hw_block.c:5:
> /Users/pm215/src/qemu-for-merges/hw/block/trace-events:36:38: error:
> format specifies type 'unsigned long' but the argument has type
> 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
>                      , name, offset, len);
>                                      ^~~
> /Users/pm215/src/qemu-for-merges/hw/block/trace-events:33:121: error:
> format specifies type 'unsigned long' but the argument has type
> 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
>             qemu_log("pflash_write_block_erase " "%s: block erase
> offset:0x%" PRIx64 " bytes:0x%lx" "\n", name, offset, len);
> 
>                         ~~~                      ^~~
> 
>                         %llx

Oops. I lost access to Gitlab [*] so couldn't run the test suite.
I tried to run all of them manually but forgot the win32 cross
compilation job which usually show these problems.

[*] Which made me realize we put all your eggs in one basket.



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-03-17 14:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 23:35 [PULL 00/11] pflash patches for 2021-03-16 Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 01/11] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 02/11] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table() Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 03/11] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table() Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 04/11] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings() Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 05/11] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false) Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 06/11] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 07/11] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 08/11] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 09/11] hw/block/pflash_cfi01: Clarify trace events Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 10/11] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro Philippe Mathieu-Daudé
2021-03-15 23:35 ` [PULL 11/11] hw/block/pflash_cfi: Replace DPRINTF with trace events Philippe Mathieu-Daudé
2021-03-17 13:37 ` [PULL 00/11] pflash patches for 2021-03-16 Peter Maydell
2021-03-17 14:22   ` Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.