[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 20/36] hw/pflash: implement update buffer for block writes
From: |
Philippe Mathieu-Daudé |
Subject: |
[PULL 20/36] hw/pflash: implement update buffer for block writes |
Date: |
Fri, 19 Jan 2024 12:34:49 +0100 |
From: Gerd Hoffmann <kraxel@redhat.com>
Add an update buffer where all block updates are staged.
Flush or discard updates properly, so we should never see
half-completed block writes in pflash storage.
Drop a bunch of FIXME comments ;)
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240108160900.104835-4-kraxel@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/block/pflash_cfi01.c | 110 ++++++++++++++++++++++++++++++----------
hw/block/pflash_cfi02.c | 2 +-
hw/block/trace-events | 7 ++-
3 files changed, 89 insertions(+), 30 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 8434a45cab..f956f8bcf7 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -80,16 +80,39 @@ struct PFlashCFI01 {
uint16_t ident3;
uint8_t cfi_table[0x52];
uint64_t counter;
- unsigned int writeblock_size;
+ uint32_t writeblock_size;
MemoryRegion mem;
char *name;
void *storage;
VMChangeStateEntry *vmstate;
bool old_multiple_chip_handling;
+
+ /* block update buffer */
+ unsigned char *blk_bytes;
+ uint32_t blk_offset;
};
static int pflash_post_load(void *opaque, int version_id);
+static bool pflash_blk_write_state_needed(void *opaque)
+{
+ PFlashCFI01 *pfl = opaque;
+
+ return (pfl->blk_offset != -1);
+}
+
+static const VMStateDescription vmstate_pflash_blk_write = {
+ .name = "pflash_cfi01_blk_write",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = pflash_blk_write_state_needed,
+ .fields = (const VMStateField[]) {
+ VMSTATE_VBUFFER_UINT32(blk_bytes, PFlashCFI01, 0, NULL,
writeblock_size),
+ VMSTATE_UINT32(blk_offset, PFlashCFI01),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_pflash = {
.name = "pflash_cfi01",
.version_id = 1,
@@ -101,6 +124,10 @@ static const VMStateDescription vmstate_pflash = {
VMSTATE_UINT8(status, PFlashCFI01),
VMSTATE_UINT64(counter, PFlashCFI01),
VMSTATE_END_OF_LIST()
+ },
+ .subsections = (const VMStateDescription * const []) {
+ &vmstate_pflash_blk_write,
+ NULL
}
};
@@ -376,13 +403,55 @@ static void pflash_update(PFlashCFI01 *pfl, int offset,
}
}
+/* copy current flash content to block update buffer */
+static void pflash_blk_write_start(PFlashCFI01 *pfl, hwaddr offset)
+{
+ hwaddr mask = ~(pfl->writeblock_size - 1);
+
+ trace_pflash_write_block_start(pfl->name, pfl->counter);
+ pfl->blk_offset = offset & mask;
+ memcpy(pfl->blk_bytes, pfl->storage + pfl->blk_offset,
+ pfl->writeblock_size);
+}
+
+/* commit block update buffer changes */
+static void pflash_blk_write_flush(PFlashCFI01 *pfl)
+{
+ g_assert(pfl->blk_offset != -1);
+ trace_pflash_write_block_flush(pfl->name);
+ memcpy(pfl->storage + pfl->blk_offset, pfl->blk_bytes,
+ pfl->writeblock_size);
+ pflash_update(pfl, pfl->blk_offset, pfl->writeblock_size);
+ pfl->blk_offset = -1;
+}
+
+/* discard block update buffer changes */
+static void pflash_blk_write_abort(PFlashCFI01 *pfl)
+{
+ trace_pflash_write_block_abort(pfl->name);
+ pfl->blk_offset = -1;
+}
+
static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset,
uint32_t value, int width, int be)
{
uint8_t *p;
- trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
- p = pfl->storage + offset;
+ if (pfl->blk_offset != -1) {
+ /* block write: redirect writes to block update buffer */
+ if ((offset < pfl->blk_offset) ||
+ (offset + width > pfl->blk_offset + pfl->writeblock_size)) {
+ pfl->status |= 0x10; /* Programming error */
+ return;
+ }
+ trace_pflash_data_write_block(pfl->name, offset, width, value,
+ pfl->counter);
+ p = pfl->blk_bytes + (offset - pfl->blk_offset);
+ } else {
+ /* write directly to storage */
+ trace_pflash_data_write(pfl->name, offset, width, value);
+ p = pfl->storage + offset;
+ }
if (be) {
stn_be_p(p, width, value);
@@ -503,9 +572,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
} else {
value = extract32(value, 0, pfl->bank_width * 8);
}
- trace_pflash_write_block(pfl->name, value);
pfl->counter = value;
pfl->wcycle++;
+ pflash_blk_write_start(pfl, offset);
break;
case 0x60:
if (cmd == 0xd0) {
@@ -536,12 +605,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
switch (pfl->cmd) {
case 0xe8: /* Block write */
/* FIXME check @offset, @width */
- if (!pfl->ro) {
- /*
- * FIXME writing straight to memory is *wrong*. We
- * should write to a buffer, and flush it to memory
- * only on confirm command (see below).
- */
+ if (!pfl->ro && (pfl->blk_offset != -1)) {
pflash_data_write(pfl, offset, value, width, be);
} else {
pfl->status |= 0x10; /* Programming error */
@@ -550,18 +614,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
pfl->status |= 0x80;
if (!pfl->counter) {
- hwaddr mask = pfl->writeblock_size - 1;
- mask = ~mask;
-
trace_pflash_write(pfl->name, "block write finished");
pfl->wcycle++;
- if (!pfl->ro) {
- /* Flush the entire write buffer onto backing storage. */
- /* FIXME premature! */
- pflash_update(pfl, offset & mask, pfl->writeblock_size);
- } else {
- pfl->status |= 0x10; /* Programming error */
- }
}
pfl->counter--;
@@ -573,20 +627,17 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
case 3: /* Confirm mode */
switch (pfl->cmd) {
case 0xe8: /* Block write */
- if (cmd == 0xd0) {
- /* FIXME this is where we should write out the buffer */
+ if ((cmd == 0xd0) && !(pfl->status & 0x10)) {
+ pflash_blk_write_flush(pfl);
pfl->wcycle = 0;
pfl->status |= 0x80;
} else {
- qemu_log_mask(LOG_UNIMP,
- "%s: Aborting write to buffer not implemented,"
- " the data is already written to storage!\n"
- "Flash device reset into READ mode.\n",
- __func__);
+ pflash_blk_write_abort(pfl);
goto mode_read_array;
}
break;
default:
+ pflash_blk_write_abort(pfl);
goto error_flash;
}
break;
@@ -820,6 +871,9 @@ static void pflash_cfi01_realize(DeviceState *dev, Error
**errp)
pfl->cmd = 0x00;
pfl->status = 0x80; /* WSM ready */
pflash_cfi01_fill_cfi_table(pfl);
+
+ pfl->blk_bytes = g_malloc(pfl->writeblock_size);
+ pfl->blk_offset = -1;
}
static void pflash_cfi01_system_reset(DeviceState *dev)
@@ -839,6 +893,8 @@ static void pflash_cfi01_system_reset(DeviceState *dev)
* This model deliberately ignores this delay.
*/
pfl->status = 0x80;
+
+ pfl->blk_offset = -1;
}
static Property pflash_cfi01_properties[] = {
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2a99b286b0..6fa56f14c0 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -546,7 +546,7 @@ static void pflash_write(void *opaque, hwaddr offset,
uint64_t value,
}
goto reset_flash;
}
- trace_pflash_data_write(pfl->name, offset, width, value, 0);
+ trace_pflash_data_write(pfl->name, offset, width, value);
if (!pfl->ro) {
p = (uint8_t *)pfl->storage + offset;
if (pfl->be) {
diff --git a/hw/block/trace-events b/hw/block/trace-events
index bab21d3a1c..cc9a9f2460 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -12,7 +12,8 @@ fdctrl_tc_pulse(int level) "TC pulse: %u"
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_data_write(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_block(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"
@@ -32,7 +33,9 @@ pflash_unlock0_failed(const char *name, uint64_t offset,
uint8_t cmd, uint16_t a
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_start(const char *name, uint32_t value) "%s: block write
start: bytes:0x%x"
+pflash_write_block_flush(const char *name) "%s: block write flush"
+pflash_write_block_abort(const char *name) "%s: block write abort"
pflash_write_block_erase(const char *name, uint64_t offset, uint64_t len) "%s:
block erase offset:0x%" PRIx64 " bytes:0x%" PRIx64
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"
--
2.41.0
- [PULL 10/36] target/alpha: Only build sys_helper.c on system emulation, (continued)
- [PULL 10/36] target/alpha: Only build sys_helper.c on system emulation, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 11/36] system/cpu-timers: Have icount_configure() return a boolean, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 12/36] system/cpu-timers: Introduce ICountMode enumerator, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 13/36] target/arm: Ensure icount is enabled when emulating INST_RETIRED, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 14/36] util/async: Only call icount_notify_exit() if icount is enabled, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 15/36] target/sh4: Deprecate the shix machine, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 16/36] hw/block: Deprecate the TC58128 block device, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 17/36] hw/i386/pc_piix: Make piix_intx_routing_notifier_xen() more device independent, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 18/36] hw/pflash: refactor pflash_data_write(), Philippe Mathieu-Daudé, 2024/01/19
- [PULL 19/36] hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 20/36] hw/pflash: implement update buffer for block writes,
Philippe Mathieu-Daudé <=
- [PULL 21/36] system/replay: Restrict icount to system emulation, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 22/36] system/watchpoint: Move TCG specific code to accel/tcg/, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 23/36] cpus: Restrict 'start-powered-off' property to system emulation, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 24/36] accel: Rename accel_init_ops_interfaces() to include 'system', Philippe Mathieu-Daudé, 2024/01/19
- [PULL 25/36] hw/core/cpu: Rename cpu_class_init() to include 'common', Philippe Mathieu-Daudé, 2024/01/19
- [PULL 26/36] hw/s390x: Rename cpu_class_init() to include 'sclp', Philippe Mathieu-Daudé, 2024/01/19
- [PULL 27/36] target/i386: Rename tcg_cpu_FOO() to include 'x86', Philippe Mathieu-Daudé, 2024/01/19
- [PULL 28/36] target/riscv: Rename tcg_cpu_FOO() to include 'riscv', Philippe Mathieu-Daudé, 2024/01/19
- [PULL 29/36] hw/scsi/esp-pci: use correct address register for PCI DMA transfers, Philippe Mathieu-Daudé, 2024/01/19
- [PULL 30/36] hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources, Philippe Mathieu-Daudé, 2024/01/19