* [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled
@ 2020-12-22 6:45 Bin Meng
2020-12-22 6:45 ` [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes Bin Meng
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Bin Meng @ 2020-12-22 6:45 UTC (permalink / raw)
To: Alistair Francis, Francisco Iglesias, Kevin Wolf, Max Reitz,
qemu-block, qemu-devel
Cc: Bin Meng
From: Bin Meng <bin.meng@windriver.com>
When write is disabled, the write to flash should be avoided
in flash_write8().
Fixes: 82a2499011a7 ("m25p80: Initial implementation of SPI flash device")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
(no changes since v2)
Changes in v2:
- new patch: honor write enable flag in flash write
hw/block/m25p80.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 483925f..236e1b4 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -594,6 +594,7 @@ void flash_write8(Flash *s, uint32_t addr, uint8_t data)
if (!s->write_enable) {
qemu_log_mask(LOG_GUEST_ERROR, "M25P80: write with write protect!\n");
+ return;
}
if ((prev ^ data) & data) {
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes
2020-12-22 6:45 [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Bin Meng
@ 2020-12-22 6:45 ` Bin Meng
2020-12-22 15:43 ` Francisco Iglesias
2020-12-22 14:25 ` [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Philippe Mathieu-Daudé
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2020-12-22 6:45 UTC (permalink / raw)
To: Alistair Francis, Francisco Iglesias, Kevin Wolf, Max Reitz,
qemu-block, qemu-devel
Cc: Xuzhou Cheng, Bin Meng
From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Auto Address Increment (AAI) Word-Program is a special command of
SST flashes. AAI-WP allows multiple bytes of data to be programmed
without re-issuing the next sequential address location.
Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
Changes in v4:
- simplify is_valid_aai_cmd()
- use a subsection for s->aai_enable vm state
Changes in v3:
- initialize aai_enable to false in reset_memory()
Changes in v2:
- add aai_enable into the vmstate
- validate AAI command before decoding a new command
- log guest errors during AAI_WP command handling
- report AAI status in the status register
- abort AAI programming when address is wrapped
- make sure AAI programming starts from the even address
hw/block/m25p80.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 236e1b4..d37b4d8 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -359,6 +359,7 @@ typedef enum {
QPP_4 = 0x34,
RDID_90 = 0x90,
RDID_AB = 0xab,
+ AAI_WP = 0xad,
ERASE_4K = 0x20,
ERASE4_4K = 0x21,
@@ -449,6 +450,7 @@ struct Flash {
bool four_bytes_address_mode;
bool reset_enable;
bool quad_enable;
+ bool aai_enable;
uint8_t ear;
int64_t dirty_page;
@@ -664,6 +666,11 @@ static void complete_collecting_data(Flash *s)
case PP4_4:
s->state = STATE_PAGE_PROGRAM;
break;
+ case AAI_WP:
+ /* AAI programming starts from the even address */
+ s->cur_addr &= ~BIT(0);
+ s->state = STATE_PAGE_PROGRAM;
+ break;
case READ:
case READ4:
case FAST_READ:
@@ -762,6 +769,7 @@ static void reset_memory(Flash *s)
s->write_enable = false;
s->reset_enable = false;
s->quad_enable = false;
+ s->aai_enable = false;
switch (get_man(s)) {
case MAN_NUMONYX:
@@ -932,6 +940,11 @@ static void decode_qio_read_cmd(Flash *s)
s->state = STATE_COLLECTING_DATA;
}
+static bool is_valid_aai_cmd(uint32_t cmd)
+{
+ return cmd == AAI_WP || cmd == WRDI || cmd == RDSR;
+}
+
static void decode_new_cmd(Flash *s, uint32_t value)
{
int i;
@@ -943,6 +956,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
s->reset_enable = false;
}
+ if (get_man(s) == MAN_SST && s->aai_enable && !is_valid_aai_cmd(value)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Invalid cmd within AAI programming sequence");
+ }
+
switch (value) {
case ERASE_4K:
@@ -1008,6 +1026,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case WRDI:
s->write_enable = false;
+ if (get_man(s) == MAN_SST) {
+ s->aai_enable = false;
+ }
break;
case WREN:
s->write_enable = true;
@@ -1018,6 +1039,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
if (get_man(s) == MAN_MACRONIX) {
s->data[0] |= (!!s->quad_enable) << 6;
}
+ if (get_man(s) == MAN_SST) {
+ s->data[0] |= (!!s->aai_enable) << 6;
+ }
+
s->pos = 0;
s->len = 1;
s->data_read_loop = true;
@@ -1160,6 +1185,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case RSTQIO:
s->quad_enable = false;
break;
+ case AAI_WP:
+ if (get_man(s) == MAN_SST) {
+ if (s->write_enable) {
+ if (s->aai_enable) {
+ s->state = STATE_PAGE_PROGRAM;
+ } else {
+ s->aai_enable = true;
+ s->needed_bytes = get_addr_length(s);
+ s->state = STATE_COLLECTING_DATA;
+ }
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: AAI_WP with write protect\n");
+ }
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
+ }
+ break;
default:
s->pos = 0;
s->len = 1;
@@ -1205,6 +1248,19 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
trace_m25p80_page_program(s, s->cur_addr, (uint8_t)tx);
flash_write8(s, s->cur_addr, (uint8_t)tx);
s->cur_addr = (s->cur_addr + 1) & (s->size - 1);
+
+ if (get_man(s) == MAN_SST && s->aai_enable && s->cur_addr == 0) {
+ /*
+ * There is no wrap mode during AAI programming once the highest
+ * unprotected memory address is reached. The Write-Enable-Latch
+ * bit is automatically reset, and AAI programming mode aborts.
+ */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: AAI highest memory address reached");
+ s->write_enable = false;
+ s->aai_enable = false;
+ }
+
break;
case STATE_READ:
@@ -1350,6 +1406,24 @@ static const VMStateDescription vmstate_m25p80_data_read_loop = {
}
};
+static bool m25p80_aai_enable_needed(void *opaque)
+{
+ Flash *s = (Flash *)opaque;
+
+ return get_man(s) == MAN_SST;
+}
+
+static const VMStateDescription vmstate_m25p80_aai_enable = {
+ .name = "m25p80/aai_enable",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = m25p80_aai_enable_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_BOOL(aai_enable, Flash),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_m25p80 = {
.name = "m25p80",
.version_id = 0,
@@ -1380,6 +1454,7 @@ static const VMStateDescription vmstate_m25p80 = {
},
.subsections = (const VMStateDescription * []) {
&vmstate_m25p80_data_read_loop,
+ &vmstate_m25p80_aai_enable,
NULL
}
};
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled
2020-12-22 6:45 [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Bin Meng
2020-12-22 6:45 ` [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes Bin Meng
@ 2020-12-22 14:25 ` Philippe Mathieu-Daudé
2020-12-22 15:55 ` Francisco Iglesias
2021-01-05 3:46 ` Bin Meng
3 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-22 14:25 UTC (permalink / raw)
To: Bin Meng, Alistair Francis, Francisco Iglesias, Kevin Wolf,
Max Reitz, qemu-block, qemu-devel
Cc: Bin Meng
On 12/22/20 7:45 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
>
> When write is disabled, the write to flash should be avoided
> in flash_write8().
>
> Fixes: 82a2499011a7 ("m25p80: Initial implementation of SPI flash device")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - new patch: honor write enable flag in flash write
>
> hw/block/m25p80.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 483925f..236e1b4 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -594,6 +594,7 @@ void flash_write8(Flash *s, uint32_t addr, uint8_t data)
>
> if (!s->write_enable) {
> qemu_log_mask(LOG_GUEST_ERROR, "M25P80: write with write protect!\n");
> + return;
> }
>
> if ((prev ^ data) & data) {
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes
2020-12-22 6:45 ` [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes Bin Meng
@ 2020-12-22 15:43 ` Francisco Iglesias
2020-12-22 15:51 ` Bin Meng
0 siblings, 1 reply; 8+ messages in thread
From: Francisco Iglesias @ 2020-12-22 15:43 UTC (permalink / raw)
To: Bin Meng
Cc: Kevin Wolf, qemu-block, Xuzhou Cheng, Bin Meng, qemu-devel,
Max Reitz, Alistair Francis
Hi Bin,
A couple of minor comments only!
On [2020 Dec 22] Tue 14:45:20, Bin Meng wrote:
> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>
> Auto Address Increment (AAI) Word-Program is a special command of
> SST flashes. AAI-WP allows multiple bytes of data to be programmed
> without re-issuing the next sequential address location.
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v4:
> - simplify is_valid_aai_cmd()
> - use a subsection for s->aai_enable vm state
>
> Changes in v3:
> - initialize aai_enable to false in reset_memory()
>
> Changes in v2:
> - add aai_enable into the vmstate
> - validate AAI command before decoding a new command
> - log guest errors during AAI_WP command handling
> - report AAI status in the status register
> - abort AAI programming when address is wrapped
> - make sure AAI programming starts from the even address
>
> hw/block/m25p80.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 236e1b4..d37b4d8 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -359,6 +359,7 @@ typedef enum {
> QPP_4 = 0x34,
> RDID_90 = 0x90,
> RDID_AB = 0xab,
> + AAI_WP = 0xad,
>
> ERASE_4K = 0x20,
> ERASE4_4K = 0x21,
> @@ -449,6 +450,7 @@ struct Flash {
> bool four_bytes_address_mode;
> bool reset_enable;
> bool quad_enable;
> + bool aai_enable;
> uint8_t ear;
>
> int64_t dirty_page;
> @@ -664,6 +666,11 @@ static void complete_collecting_data(Flash *s)
> case PP4_4:
> s->state = STATE_PAGE_PROGRAM;
> break;
> + case AAI_WP:
> + /* AAI programming starts from the even address */
> + s->cur_addr &= ~BIT(0);
> + s->state = STATE_PAGE_PROGRAM;
> + break;
> case READ:
> case READ4:
> case FAST_READ:
> @@ -762,6 +769,7 @@ static void reset_memory(Flash *s)
> s->write_enable = false;
> s->reset_enable = false;
> s->quad_enable = false;
> + s->aai_enable = false;
>
> switch (get_man(s)) {
> case MAN_NUMONYX:
> @@ -932,6 +940,11 @@ static void decode_qio_read_cmd(Flash *s)
> s->state = STATE_COLLECTING_DATA;
> }
>
> +static bool is_valid_aai_cmd(uint32_t cmd)
> +{
> + return cmd == AAI_WP || cmd == WRDI || cmd == RDSR;
> +}
> +
> static void decode_new_cmd(Flash *s, uint32_t value)
> {
> int i;
> @@ -943,6 +956,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> s->reset_enable = false;
> }
>
> + if (get_man(s) == MAN_SST && s->aai_enable && !is_valid_aai_cmd(value)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "M25P80: Invalid cmd within AAI programming sequence");
> + }
> +
> switch (value) {
>
> case ERASE_4K:
> @@ -1008,6 +1026,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>
> case WRDI:
> s->write_enable = false;
> + if (get_man(s) == MAN_SST) {
> + s->aai_enable = false;
> + }
> break;
> case WREN:
> s->write_enable = true;
> @@ -1018,6 +1039,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> if (get_man(s) == MAN_MACRONIX) {
> s->data[0] |= (!!s->quad_enable) << 6;
> }
> + if (get_man(s) == MAN_SST) {
> + s->data[0] |= (!!s->aai_enable) << 6;
> + }
> +
> s->pos = 0;
> s->len = 1;
> s->data_read_loop = true;
> @@ -1160,6 +1185,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> case RSTQIO:
> s->quad_enable = false;
> break;
> + case AAI_WP:
> + if (get_man(s) == MAN_SST) {
> + if (s->write_enable) {
> + if (s->aai_enable) {
> + s->state = STATE_PAGE_PROGRAM;
> + } else {
> + s->aai_enable = true;
> + s->needed_bytes = get_addr_length(s);
> + s->state = STATE_COLLECTING_DATA;
> + }
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "M25P80: AAI_WP with write protect\n");
> + }
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
> + }
> + break;
> default:
> s->pos = 0;
> s->len = 1;
> @@ -1205,6 +1248,19 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
> trace_m25p80_page_program(s, s->cur_addr, (uint8_t)tx);
> flash_write8(s, s->cur_addr, (uint8_t)tx);
> s->cur_addr = (s->cur_addr + 1) & (s->size - 1);
> +
> + if (get_man(s) == MAN_SST && s->aai_enable && s->cur_addr == 0) {
> + /*
> + * There is no wrap mode during AAI programming once the highest
> + * unprotected memory address is reached. The Write-Enable-Latch
> + * bit is automatically reset, and AAI programming mode aborts.
> + */
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "M25P80: AAI highest memory address reached");
Above message will be printed after writing the highest addressed byte but
before trying to write a byte after wrapping. Since it wouldn't be a guest
error (to write the final byte) perhaps we should remove it?
(An option is to swap it to a trace event instead also)
> + s->write_enable = false;
> + s->aai_enable = false;
> + }
> +
> break;
>
> case STATE_READ:
> @@ -1350,6 +1406,24 @@ static const VMStateDescription vmstate_m25p80_data_read_loop = {
> }
> };
>
> +static bool m25p80_aai_enable_needed(void *opaque)
> +{
> + Flash *s = (Flash *)opaque;
> +
> + return get_man(s) == MAN_SST;
For only using the subsection if really needed (and restricting further)
we can swap above to:
return s->aai_enable;
Best regards,
Francisco Iglesias
> +}
> +
> +static const VMStateDescription vmstate_m25p80_aai_enable = {
> + .name = "m25p80/aai_enable",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = m25p80_aai_enable_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_BOOL(aai_enable, Flash),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_m25p80 = {
> .name = "m25p80",
> .version_id = 0,
> @@ -1380,6 +1454,7 @@ static const VMStateDescription vmstate_m25p80 = {
> },
> .subsections = (const VMStateDescription * []) {
> &vmstate_m25p80_data_read_loop,
> + &vmstate_m25p80_aai_enable,
> NULL
> }
> };
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes
2020-12-22 15:43 ` Francisco Iglesias
@ 2020-12-22 15:51 ` Bin Meng
0 siblings, 0 replies; 8+ messages in thread
From: Bin Meng @ 2020-12-22 15:51 UTC (permalink / raw)
To: Francisco Iglesias
Cc: Kevin Wolf, Qemu-block, Xuzhou Cheng, Bin Meng,
qemu-devel@nongnu.org Developers, Max Reitz, Alistair Francis
Hi Francisco,
On Tue, Dec 22, 2020 at 11:43 PM Francisco Iglesias
<frasse.iglesias@gmail.com> wrote:
>
> Hi Bin,
>
> A couple of minor comments only!
>
> On [2020 Dec 22] Tue 14:45:20, Bin Meng wrote:
> > From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> >
> > Auto Address Increment (AAI) Word-Program is a special command of
> > SST flashes. AAI-WP allows multiple bytes of data to be programmed
> > without re-issuing the next sequential address location.
> >
> > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > Changes in v4:
> > - simplify is_valid_aai_cmd()
> > - use a subsection for s->aai_enable vm state
> >
> > Changes in v3:
> > - initialize aai_enable to false in reset_memory()
> >
> > Changes in v2:
> > - add aai_enable into the vmstate
> > - validate AAI command before decoding a new command
> > - log guest errors during AAI_WP command handling
> > - report AAI status in the status register
> > - abort AAI programming when address is wrapped
> > - make sure AAI programming starts from the even address
> >
> > hw/block/m25p80.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 75 insertions(+)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> > index 236e1b4..d37b4d8 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -359,6 +359,7 @@ typedef enum {
> > QPP_4 = 0x34,
> > RDID_90 = 0x90,
> > RDID_AB = 0xab,
> > + AAI_WP = 0xad,
> >
> > ERASE_4K = 0x20,
> > ERASE4_4K = 0x21,
> > @@ -449,6 +450,7 @@ struct Flash {
> > bool four_bytes_address_mode;
> > bool reset_enable;
> > bool quad_enable;
> > + bool aai_enable;
> > uint8_t ear;
> >
> > int64_t dirty_page;
> > @@ -664,6 +666,11 @@ static void complete_collecting_data(Flash *s)
> > case PP4_4:
> > s->state = STATE_PAGE_PROGRAM;
> > break;
> > + case AAI_WP:
> > + /* AAI programming starts from the even address */
> > + s->cur_addr &= ~BIT(0);
> > + s->state = STATE_PAGE_PROGRAM;
> > + break;
> > case READ:
> > case READ4:
> > case FAST_READ:
> > @@ -762,6 +769,7 @@ static void reset_memory(Flash *s)
> > s->write_enable = false;
> > s->reset_enable = false;
> > s->quad_enable = false;
> > + s->aai_enable = false;
> >
> > switch (get_man(s)) {
> > case MAN_NUMONYX:
> > @@ -932,6 +940,11 @@ static void decode_qio_read_cmd(Flash *s)
> > s->state = STATE_COLLECTING_DATA;
> > }
> >
> > +static bool is_valid_aai_cmd(uint32_t cmd)
> > +{
> > + return cmd == AAI_WP || cmd == WRDI || cmd == RDSR;
> > +}
> > +
> > static void decode_new_cmd(Flash *s, uint32_t value)
> > {
> > int i;
> > @@ -943,6 +956,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> > s->reset_enable = false;
> > }
> >
> > + if (get_man(s) == MAN_SST && s->aai_enable && !is_valid_aai_cmd(value)) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "M25P80: Invalid cmd within AAI programming sequence");
> > + }
> > +
> > switch (value) {
> >
> > case ERASE_4K:
> > @@ -1008,6 +1026,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> >
> > case WRDI:
> > s->write_enable = false;
> > + if (get_man(s) == MAN_SST) {
> > + s->aai_enable = false;
> > + }
> > break;
> > case WREN:
> > s->write_enable = true;
> > @@ -1018,6 +1039,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> > if (get_man(s) == MAN_MACRONIX) {
> > s->data[0] |= (!!s->quad_enable) << 6;
> > }
> > + if (get_man(s) == MAN_SST) {
> > + s->data[0] |= (!!s->aai_enable) << 6;
> > + }
> > +
> > s->pos = 0;
> > s->len = 1;
> > s->data_read_loop = true;
> > @@ -1160,6 +1185,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> > case RSTQIO:
> > s->quad_enable = false;
> > break;
> > + case AAI_WP:
> > + if (get_man(s) == MAN_SST) {
> > + if (s->write_enable) {
> > + if (s->aai_enable) {
> > + s->state = STATE_PAGE_PROGRAM;
> > + } else {
> > + s->aai_enable = true;
> > + s->needed_bytes = get_addr_length(s);
> > + s->state = STATE_COLLECTING_DATA;
> > + }
> > + } else {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "M25P80: AAI_WP with write protect\n");
> > + }
> > + } else {
> > + qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
> > + }
> > + break;
> > default:
> > s->pos = 0;
> > s->len = 1;
> > @@ -1205,6 +1248,19 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
> > trace_m25p80_page_program(s, s->cur_addr, (uint8_t)tx);
> > flash_write8(s, s->cur_addr, (uint8_t)tx);
> > s->cur_addr = (s->cur_addr + 1) & (s->size - 1);
> > +
> > + if (get_man(s) == MAN_SST && s->aai_enable && s->cur_addr == 0) {
> > + /*
> > + * There is no wrap mode during AAI programming once the highest
> > + * unprotected memory address is reached. The Write-Enable-Latch
> > + * bit is automatically reset, and AAI programming mode aborts.
> > + */
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "M25P80: AAI highest memory address reached");
>
> Above message will be printed after writing the highest addressed byte but
> before trying to write a byte after wrapping. Since it wouldn't be a guest
> error (to write the final byte) perhaps we should remove it?
> (An option is to swap it to a trace event instead also)
Agree. This is not an error. Will update in the next version.
>
> > + s->write_enable = false;
> > + s->aai_enable = false;
> > + }
> > +
> > break;
> >
> > case STATE_READ:
> > @@ -1350,6 +1406,24 @@ static const VMStateDescription vmstate_m25p80_data_read_loop = {
> > }
> > };
> >
> > +static bool m25p80_aai_enable_needed(void *opaque)
> > +{
> > + Flash *s = (Flash *)opaque;
> > +
> > + return get_man(s) == MAN_SST;
>
> For only using the subsection if really needed (and restricting further)
> we can swap above to:
>
> return s->aai_enable;
Will do. Thanks!
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled
2020-12-22 6:45 [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Bin Meng
2020-12-22 6:45 ` [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes Bin Meng
2020-12-22 14:25 ` [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Philippe Mathieu-Daudé
@ 2020-12-22 15:55 ` Francisco Iglesias
2021-01-05 3:46 ` Bin Meng
3 siblings, 0 replies; 8+ messages in thread
From: Francisco Iglesias @ 2020-12-22 15:55 UTC (permalink / raw)
To: Bin Meng
Cc: Kevin Wolf, qemu-block, Bin Meng, qemu-devel, Max Reitz,
Alistair Francis
On [2020 Dec 22] Tue 14:45:19, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
>
> When write is disabled, the write to flash should be avoided
> in flash_write8().
>
> Fixes: 82a2499011a7 ("m25p80: Initial implementation of SPI flash device")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - new patch: honor write enable flag in flash write
>
> hw/block/m25p80.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 483925f..236e1b4 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -594,6 +594,7 @@ void flash_write8(Flash *s, uint32_t addr, uint8_t data)
>
> if (!s->write_enable) {
> qemu_log_mask(LOG_GUEST_ERROR, "M25P80: write with write protect!\n");
> + return;
> }
>
> if ((prev ^ data) & data) {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled
2020-12-22 6:45 [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Bin Meng
` (2 preceding siblings ...)
2020-12-22 15:55 ` Francisco Iglesias
@ 2021-01-05 3:46 ` Bin Meng
2021-01-05 3:49 ` Bin Meng
3 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2021-01-05 3:46 UTC (permalink / raw)
To: Alistair Francis, Francisco Iglesias, Kevin Wolf, Max Reitz,
Qemu-block, qemu-devel@nongnu.org Developers, Peter Maydell
Cc: Bin Meng
Hello,
On Tue, Dec 22, 2020 at 2:45 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> When write is disabled, the write to flash should be avoided
> in flash_write8().
>
> Fixes: 82a2499011a7 ("m25p80: Initial implementation of SPI flash device")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - new patch: honor write enable flag in flash write
>
> hw/block/m25p80.c | 1 +
> 1 file changed, 1 insertion(+)
>
Who is going to pick up this series? Is it Alistair, or Peter?
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled
2021-01-05 3:46 ` Bin Meng
@ 2021-01-05 3:49 ` Bin Meng
0 siblings, 0 replies; 8+ messages in thread
From: Bin Meng @ 2021-01-05 3:49 UTC (permalink / raw)
To: Alistair Francis, Francisco Iglesias, Kevin Wolf, Max Reitz,
Qemu-block, qemu-devel@nongnu.org Developers, Peter Maydell
Cc: Bin Meng
On Tue, Jan 5, 2021 at 11:46 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hello,
>
> On Tue, Dec 22, 2020 at 2:45 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > When write is disabled, the write to flash should be avoided
> > in flash_write8().
> >
> > Fixes: 82a2499011a7 ("m25p80: Initial implementation of SPI flash device")
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - new patch: honor write enable flag in flash write
> >
> > hw/block/m25p80.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
>
> Who is going to pick up this series? Is it Alistair, or Peter?
>
Oops, I replied to the wrong thread. There is a v5 series that should
be applied.
Regards,
Bin
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-05 3:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-22 6:45 [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Bin Meng
2020-12-22 6:45 ` [PATCH v4 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes Bin Meng
2020-12-22 15:43 ` Francisco Iglesias
2020-12-22 15:51 ` Bin Meng
2020-12-22 14:25 ` [PATCH v4 1/2] hw/block: m25p80: Don't write to flash if write is disabled Philippe Mathieu-Daudé
2020-12-22 15:55 ` Francisco Iglesias
2021-01-05 3:46 ` Bin Meng
2021-01-05 3:49 ` Bin Meng
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.