Skip to content

Commit

Permalink
hw/pflash: implement update buffer for block writes
Browse files Browse the repository at this point in the history 
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>
(cherry picked from commit 284a7ee)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(Mjt: drop const in hw/block/pflash_cfi01.c for before
 v8.2.0-220-g7d5dc0a367 "hw/block: Constify VMState")
  • Loading branch information
@kraxel
kraxel authored and Michael Tokarev committed Jan 20, 2024
1 parent d56cc9b commit 16f6a65
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 30 deletions.
110 changes: 83 additions & 27 deletions hw/block/pflash_cfi01.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,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,
Expand All @@ -102,6 +125,10 @@ static const VMStateDescription vmstate_pflash = {
VMSTATE_UINT8(status, PFlashCFI01),
VMSTATE_UINT64(counter, PFlashCFI01),
VMSTATE_END_OF_LIST()
},
.subsections = (const VMStateDescription * []) {
&vmstate_pflash_blk_write,
NULL
}
};

Expand Down Expand Up @@ -377,13 +404,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);
Expand Down Expand Up @@ -504,9 +573,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) {
Expand Down Expand Up @@ -537,12 +606,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 */
Expand All @@ -551,18 +615,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--;
Expand All @@ -574,20 +628,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;
Expand Down Expand Up @@ -821,6 +872,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)
Expand All @@ -840,6 +894,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[] = {
Expand Down
2 changes: 1 addition & 1 deletion hw/block/pflash_cfi02.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions hw/block/trace-events
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down

0 comments on commit 16f6a65

Please sign in to comment.