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 <kra...@redhat.com>
---
hw/block/pflash_cfi01.c | 104 ++++++++++++++++++++++++++++++----------
1 file changed, 78 insertions(+), 26 deletions(-)
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 67f1c9773ab3..78ac95b3291e 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
}
};
@@ -400,13 +427,50 @@ 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);
+
+ 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);
+ 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)
+{
+ 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;
+ }
+ p = pfl->blk_bytes + (offset - pfl->blk_offset);
+ } else {
+ /* write directly to storage */
+ trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter);
+ p = pfl->storage + offset;
+ }
switch (width) {
case 1:
@@ -553,6 +617,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
trace_pflash_write_block(pfl->name, value);
pfl->counter = value;
pfl->wcycle++;
+ pflash_blk_write_start(pfl, offset);
break;
case 0x60:
if (cmd == 0xd0) {
@@ -583,12 +648,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 */
@@ -597,18 +657,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--;
@@ -620,20 +670,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;
@@ -867,6 +914,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)
@@ -886,6 +936,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[] = {
--
2.43.0
