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