* [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode
@ 2021-03-10 17:05 Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 01/12] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
` (12 more replies)
0 siblings, 13 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, Philippe Mathieu-Daudé
I remembered this almost 2 years old series while reviewing
David Edmondson's patches... (which are included at the end).
Basically we move things around to make the code easier to maintain.
Since v1:
- Addressed David/Bin comments
- Added R-b
Missing review: patches 7 & 8
Regards,
Phil.
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é (10):
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: Extract pflash_mode_read_array()
hw/block/pflash_cfi01.c | 278 ++++++++++++++++++-----------------
hw/block/pflash_cfi02.c | 316 ++++++++++++++++++++--------------------
hw/block/trace-events | 40 +++--
3 files changed, 329 insertions(+), 305 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 01/12] hw/block/pflash_cfi: Fix code style for checkpatch.pl
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 02/12] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table() Philippe Mathieu-Daudé
` (11 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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 22287a1522e..b6919bbe474 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] 23+ messages in thread
* [PATCH v2 02/12] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table()
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 01/12] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 03/12] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table() Philippe Mathieu-Daudé
` (10 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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 b6919bbe474..03472ea5b64 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] 23+ messages in thread
* [PATCH v2 03/12] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table()
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 01/12] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 02/12] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table() Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 04/12] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings() Philippe Mathieu-Daudé
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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] 23+ messages in thread
* [PATCH v2 04/12] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings()
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 03/12] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table() Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:30 ` David Edmondson
2021-03-10 17:05 ` [PATCH v2 05/12] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false) Philippe Mathieu-Daudé
` (8 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
v2: Convert to bool in pflash_register_memory (David)
---
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] 23+ messages in thread
* [PATCH v2 05/12] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false)
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 04/12] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings() Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
2021-03-10 17:05 ` [PATCH v2 06/12] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array Philippe Mathieu-Daudé
` (7 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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] 23+ messages in thread
* [PATCH v2 06/12] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 05/12] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false) Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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 d32475c3989..f16d6e90cfd 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] 23+ messages in thread
* [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine()
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 06/12] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
` (5 subsequent siblings)
12 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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] 23+ messages in thread
* [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 09/12] hw/block/pflash_cfi01: Clarify trace events Philippe Mathieu-Daudé
` (4 subsequent siblings)
12 siblings, 2 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <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] 23+ messages in thread
* [PATCH v2 09/12] hw/block/pflash_cfi01: Clarify trace events
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 10/12] hw/block/pflash_cfi01: Extract pflash_mode_read_array() Philippe Mathieu-Daudé
` (3 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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 03472ea5b64..2618e00926d 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] 23+ messages in thread
* [PATCH v2 10/12] hw/block/pflash_cfi01: Extract pflash_mode_read_array()
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 09/12] hw/block/pflash_cfi01: Clarify trace events Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 11/12] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro Philippe Mathieu-Daudé
` (2 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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:
- On a read access (on invalid command).
- 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.
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/block/pflash_cfi01.c | 40 +++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 2618e00926d..32c9b289715 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -115,6 +115,19 @@ static const VMStateDescription vmstate_pflash = {
}
};
+static void pflash_mode_read_array(PFlashCFI01 *pfl)
+{
+ trace_pflash_mode_read_array();
+ /*
+ * The command 0x00 is not assigned by the CFI open standard,
+ * but QEMU historically uses it for the READ_ARRAY command (0xff).
+ */
+ trace_pflash_mode_read_array();
+ pfl->cmd = 0x00;
+ pfl->wcycle = 0;
+ memory_region_rom_device_set_romd(&pfl->mem, true);
+}
+
/*
* Perform a CFI query based on the bank width of the flash.
* If this code is called we know we have a device_width set for
@@ -283,12 +296,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
default:
/* This should never happen : reset state & treat it as a read */
DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
- pfl->wcycle = 0;
- /*
- * The command 0x00 is not assigned by the CFI open standard,
- * but QEMU historically uses it for the READ_ARRAY command (0xff).
- */
- pfl->cmd = 0x00;
+ pflash_mode_read_array(pfl);
/* fall through to read code */
case 0x00: /* This model reset value for READ_ARRAY (not CFI compliant) */
/* Flash area read */
@@ -663,10 +671,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();
- memory_region_rom_device_set_romd(&pfl->mem, true);
- pfl->wcycle = 0;
- pfl->cmd = 0x00; /* This model reset value for READ_ARRAY (not CFI) */
+ pflash_mode_read_array(pfl);
}
@@ -872,13 +877,8 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
pfl->max_device_width = pfl->device_width;
}
- pfl->wcycle = 0;
- /*
- * The command 0x00 is not assigned by the CFI open standard,
- * but QEMU historically uses it for the READ_ARRAY command (0xff).
- */
- pfl->cmd = 0x00;
pfl->status = 0x80; /* WSM ready */
+ pflash_mode_read_array(pfl);
pflash_cfi01_fill_cfi_table(pfl);
}
@@ -887,13 +887,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).
- */
- pfl->cmd = 0x00;
- pfl->wcycle = 0;
- memory_region_rom_device_set_romd(&pfl->mem, true);
+ pflash_mode_read_array(pfl);
/*
* The WSM ready timer occurs at most 150ns after system reset.
* This model deliberately ignores this delay.
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 11/12] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 10/12] hw/block/pflash_cfi01: Extract pflash_mode_read_array() Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 12/12] hw/block/pflash_cfi: Replace DPRINTF with trace events Philippe Mathieu-Daudé
2021-03-15 23:24 ` [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
12 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
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 32c9b289715..787466b249f 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;
@@ -858,7 +858,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] 23+ messages in thread
* [PATCH v2 12/12] hw/block/pflash_cfi: Replace DPRINTF with trace events
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 11/12] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro Philippe Mathieu-Daudé
@ 2021-03-10 17:05 ` Philippe Mathieu-Daudé
2021-03-11 3:05 ` Bin Meng
2021-03-15 23:24 ` [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
12 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-10 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis, 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>
---
hw/block/pflash_cfi01.c | 81 +++++++++++++++++------------------------
hw/block/pflash_cfi02.c | 78 ++++++++++++++++-----------------------
hw/block/trace-events | 41 ++++++++++++++++-----
3 files changed, 95 insertions(+), 105 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 787466b249f..108425402b3 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
@@ -117,12 +107,11 @@ static const VMStateDescription vmstate_pflash = {
static void pflash_mode_read_array(PFlashCFI01 *pfl)
{
- trace_pflash_mode_read_array();
+ trace_pflash_mode_read_array(pfl->name);
/*
* The command 0x00 is not assigned by the CFI open standard,
* but QEMU historically uses it for the READ_ARRAY command (0xff).
*/
- trace_pflash_mode_read_array();
pfl->cmd = 0x00;
pfl->wcycle = 0;
memory_region_rom_device_set_romd(&pfl->mem, true);
@@ -168,10 +157,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
@@ -223,14 +210,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. */
@@ -278,10 +265,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;
}
@@ -295,7 +281,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);
pflash_mode_read_array(pfl);
/* fall through to read code */
case 0x00: /* This model reset value for READ_ARRAY (not CFI compliant) */
@@ -328,7 +314,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) {
@@ -343,14 +329,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;
}
@@ -397,7 +383,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;
}
@@ -427,7 +413,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;
@@ -466,7 +452,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);
@@ -480,14 +466,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);
@@ -498,25 +483,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",
@@ -524,10 +509,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;
@@ -539,7 +524,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);
@@ -571,7 +556,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;
@@ -585,7 +570,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;
@@ -593,7 +578,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:
@@ -621,7 +606,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. */
@@ -660,7 +645,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;
@@ -886,7 +871,7 @@ static void pflash_cfi01_system_reset(DeviceState *dev)
{
PFlashCFI01 *pfl = PFLASH_CFI01(dev);
- trace_pflash_reset();
+ trace_pflash_reset(pfl->name);
pflash_mode_read_array(pfl);
/*
* The WSM ready timer occurs at most 150ns after system reset.
@@ -1035,7 +1020,7 @@ static void postload_update_cb(void *opaque, int 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 f16d6e90cfd..f655f997597 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] 23+ messages in thread
* Re: [PATCH v2 04/12] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings()
2021-03-10 17:05 ` [PATCH v2 04/12] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings() Philippe Mathieu-Daudé
@ 2021-03-10 17:30 ` David Edmondson
0 siblings, 0 replies; 23+ messages in thread
From: David Edmondson @ 2021-03-10 17:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
Alistair Francis, Bin Meng, Philippe Mathieu-Daudé
On Wednesday, 2021-03-10 at 18:05:20 +01, Philippe Mathieu-Daudé wrote:
> 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>
> ---
> v2: Convert to bool in pflash_register_memory (David)
> ---
> 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
dme.
--
Everyone I know, goes away in the end.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 05/12] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false)
2021-03-10 17:05 ` [PATCH v2 05/12] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false) Philippe Mathieu-Daudé
@ 2021-03-10 17:31 ` David Edmondson
0 siblings, 0 replies; 23+ messages in thread
From: David Edmondson @ 2021-03-10 17:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
Alistair Francis, Bin Meng, Philippe Mathieu-Daudé
On Wednesday, 2021-03-10 at 18:05:21 +01, Philippe Mathieu-Daudé wrote:
> 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>
> ---
> 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
dme.
--
And you're standing here beside me, I love the passing of time.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine()
2021-03-10 17:05 ` [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
@ 2021-03-10 17:31 ` David Edmondson
2021-03-11 3:05 ` Bin Meng
1 sibling, 0 replies; 23+ messages in thread
From: David Edmondson @ 2021-03-10 17:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
Alistair Francis, Philippe Mathieu-Daudé
On Wednesday, 2021-03-10 at 18:05:23 +01, Philippe Mathieu-Daudé wrote:
> 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>
> ---
> 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
dme.
--
"Can I take you out to the pictures, Joan?"
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method
2021-03-10 17:05 ` [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
@ 2021-03-10 17:31 ` David Edmondson
2021-03-11 3:05 ` Bin Meng
1 sibling, 0 replies; 23+ messages in thread
From: David Edmondson @ 2021-03-10 17:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
Alistair Francis, Philippe Mathieu-Daudé
On Wednesday, 2021-03-10 at 18:05:24 +01, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.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
dme.
--
Stop the music and go home.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine()
2021-03-10 17:05 ` [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
@ 2021-03-11 3:05 ` Bin Meng
1 sibling, 0 replies; 23+ messages in thread
From: Bin Meng @ 2021-03-11 3:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Stephen Checkoway, Qemu-block,
qemu-devel@nongnu.org Developers, Max Reitz, David Edmondson,
Alistair Francis
On Thu, Mar 11, 2021 at 1:36 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> 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>
> ---
> hw/block/pflash_cfi02.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method
2021-03-10 17:05 ` [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
@ 2021-03-11 3:05 ` Bin Meng
1 sibling, 0 replies; 23+ messages in thread
From: Bin Meng @ 2021-03-11 3:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Stephen Checkoway, Qemu-block,
qemu-devel@nongnu.org Developers, Max Reitz, David Edmondson,
Alistair Francis
On Thu, Mar 11, 2021 at 1:22 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/pflash_cfi02.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 10/12] hw/block/pflash_cfi01: Extract pflash_mode_read_array()
2021-03-10 17:05 ` [PATCH v2 10/12] hw/block/pflash_cfi01: Extract pflash_mode_read_array() Philippe Mathieu-Daudé
@ 2021-03-11 3:05 ` Bin Meng
0 siblings, 0 replies; 23+ messages in thread
From: Bin Meng @ 2021-03-11 3:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Stephen Checkoway, Qemu-block,
qemu-devel@nongnu.org Developers, Max Reitz, David Edmondson,
Alistair Francis
On Thu, Mar 11, 2021 at 1:37 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> The same pattern is used when setting the flash in READ_ARRAY mode:
> - Set the state machine command to READ_ARRAY
> - Reset the write_cycle counter
> - Reset the memory region in ROMD
>
> Refactor the current code by extracting this pattern.
> It is used three times:
>
> - On a read access (on invalid command).
>
> - 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.
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: David Edmondson <david.edmondson@oracle.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/pflash_cfi01.c | 40 +++++++++++++++++-----------------------
> 1 file changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 2618e00926d..32c9b289715 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -115,6 +115,19 @@ static const VMStateDescription vmstate_pflash = {
> }
> };
>
> +static void pflash_mode_read_array(PFlashCFI01 *pfl)
> +{
> + trace_pflash_mode_read_array();
Duplicated trace calls
> + /*
> + * The command 0x00 is not assigned by the CFI open standard,
> + * but QEMU historically uses it for the READ_ARRAY command (0xff).
> + */
> + trace_pflash_mode_read_array();
> + pfl->cmd = 0x00;
> + pfl->wcycle = 0;
> + memory_region_rom_device_set_romd(&pfl->mem, true);
> +}
> +
> /*
> * Perform a CFI query based on the bank width of the flash.
> * If this code is called we know we have a device_width set for
> @@ -283,12 +296,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
> default:
> /* This should never happen : reset state & treat it as a read */
> DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
> - pfl->wcycle = 0;
> - /*
> - * The command 0x00 is not assigned by the CFI open standard,
> - * but QEMU historically uses it for the READ_ARRAY command (0xff).
> - */
> - pfl->cmd = 0x00;
> + pflash_mode_read_array(pfl);
This adds a memory_region_rom_device_set_romd() call compared to the
original codes. Please double check if this is correct.
> /* fall through to read code */
> case 0x00: /* This model reset value for READ_ARRAY (not CFI compliant) */
> /* Flash area read */
> @@ -663,10 +671,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();
> - memory_region_rom_device_set_romd(&pfl->mem, true);
> - pfl->wcycle = 0;
> - pfl->cmd = 0x00; /* This model reset value for READ_ARRAY (not CFI) */
> + pflash_mode_read_array(pfl);
> }
>
>
> @@ -872,13 +877,8 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
> pfl->max_device_width = pfl->device_width;
> }
>
> - pfl->wcycle = 0;
> - /*
> - * The command 0x00 is not assigned by the CFI open standard,
> - * but QEMU historically uses it for the READ_ARRAY command (0xff).
> - */
> - pfl->cmd = 0x00;
> pfl->status = 0x80; /* WSM ready */
> + pflash_mode_read_array(pfl);
ditto
> pflash_cfi01_fill_cfi_table(pfl);
> }
>
> @@ -887,13 +887,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).
> - */
> - pfl->cmd = 0x00;
> - pfl->wcycle = 0;
> - memory_region_rom_device_set_romd(&pfl->mem, true);
> + pflash_mode_read_array(pfl);
> /*
> * The WSM ready timer occurs at most 150ns after system reset.
> * This model deliberately ignores this delay.
Regards,
Bin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 11/12] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro
2021-03-10 17:05 ` [PATCH v2 11/12] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro Philippe Mathieu-Daudé
@ 2021-03-11 3:05 ` Bin Meng
0 siblings, 0 replies; 23+ messages in thread
From: Bin Meng @ 2021-03-11 3:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Stephen Checkoway, Qemu-block,
qemu-devel@nongnu.org Developers, Max Reitz, David Edmondson,
Alistair Francis
On Thu, Mar 11, 2021 at 1:28 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> 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>
> ---
> hw/block/pflash_cfi01.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 12/12] hw/block/pflash_cfi: Replace DPRINTF with trace events
2021-03-10 17:05 ` [PATCH v2 12/12] hw/block/pflash_cfi: Replace DPRINTF with trace events Philippe Mathieu-Daudé
@ 2021-03-11 3:05 ` Bin Meng
0 siblings, 0 replies; 23+ messages in thread
From: Bin Meng @ 2021-03-11 3:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Stephen Checkoway, Qemu-block,
qemu-devel@nongnu.org Developers, Max Reitz, David Edmondson,
Alistair Francis
On Thu, Mar 11, 2021 at 1:43 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> 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>
> ---
> hw/block/pflash_cfi01.c | 81 +++++++++++++++++------------------------
> hw/block/pflash_cfi02.c | 78 ++++++++++++++++-----------------------
> hw/block/trace-events | 41 ++++++++++++++++-----
> 3 files changed, 95 insertions(+), 105 deletions(-)
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2021-03-10 17:05 ` [PATCH v2 12/12] hw/block/pflash_cfi: Replace DPRINTF with trace events Philippe Mathieu-Daudé
@ 2021-03-15 23:24 ` Philippe Mathieu-Daudé
12 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-15 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Stephen Checkoway, qemu-block, Max Reitz,
David Edmondson, Alistair Francis
On 3/10/21 6:05 PM, Philippe Mathieu-Daudé wrote:
> I remembered this almost 2 years old series while reviewing
> David Edmondson's patches... (which are included at the end).
>
> Basically we move things around to make the code easier to maintain.
>
> 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é (10):
> 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: Extract pflash_mode_read_array()
Thanks, patches 1-10 & 12 queued to pflash-next.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2021-03-15 23:25 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 17:05 [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 01/12] hw/block/pflash_cfi: Fix code style for checkpatch.pl Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 02/12] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table() Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 03/12] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table() Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 04/12] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings() Philippe Mathieu-Daudé
2021-03-10 17:30 ` David Edmondson
2021-03-10 17:05 ` [PATCH v2 05/12] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false) Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
2021-03-10 17:05 ` [PATCH v2 06/12] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 07/12] hw/block/pflash_cfi02: Factor out pflash_reset_state_machine() Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 08/12] hw/block/pflash_cfi02: Add DeviceReset method Philippe Mathieu-Daudé
2021-03-10 17:31 ` David Edmondson
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 09/12] hw/block/pflash_cfi01: Clarify trace events Philippe Mathieu-Daudé
2021-03-10 17:05 ` [PATCH v2 10/12] hw/block/pflash_cfi01: Extract pflash_mode_read_array() Philippe Mathieu-Daudé
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 11/12] hw/block/pflash_cfi01: Correct the type of PFlashCFI01.ro Philippe Mathieu-Daudé
2021-03-11 3:05 ` Bin Meng
2021-03-10 17:05 ` [PATCH v2 12/12] hw/block/pflash_cfi: Replace DPRINTF with trace events Philippe Mathieu-Daudé
2021-03-11 3:05 ` Bin Meng
2021-03-15 23:24 ` [PATCH v2 00/12] hw/block/pflash: Refactors around setting the device in read-array mode 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.