* [PATCH 0/3] hw/sd: sd: erase operation fixes
@ 2021-01-28 6:43 Bin Meng
2021-01-28 6:43 ` [PATCH 1/3] hw/sd: sd: Fix address check in sd_erase() Bin Meng
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Bin Meng @ 2021-01-28 6:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-block, qemu-devel; +Cc: Bin Meng
From: Bin Meng <bin.meng@windriver.com>
This includes several fixes related to erase operation of a SD card.
Based-on:
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=226785
Bin Meng (3):
hw/sd: sd: Fix address check in sd_erase()
hw/sd: sd: Move the sd_block_{read,write} and macros ahead
hw/sd: sd: Actually perform the erase operation
hw/sd/sd.c | 53 +++++++++++++++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 22 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] hw/sd: sd: Fix address check in sd_erase() 2021-01-28 6:43 [PATCH 0/3] hw/sd: sd: erase operation fixes Bin Meng @ 2021-01-28 6:43 ` Bin Meng 2021-02-08 10:06 ` Philippe Mathieu-Daudé 2021-01-28 6:43 ` [PATCH 2/3] hw/sd: sd: Move the sd_block_{read, write} and macros ahead Bin Meng ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Bin Meng @ 2021-01-28 6:43 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-block, qemu-devel; +Cc: Bin Meng From: Bin Meng <bin.meng@windriver.com> For high capacity memory cards, the erase start address and end address are multiplied by 512, but the address check is still based on the original block number in sd->erase_{start, end}. Fixes: 1bd6fd8ed593 ("hw/sd/sdcard: Do not attempt to erase out of range addresses") Signed-off-by: Bin Meng <bin.meng@windriver.com> --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index c99c0e93bb..a6a0b3dcc6 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -760,7 +760,7 @@ static void sd_erase(SDState *sd) erase_end *= 512; } - if (sd->erase_start > sd->size || sd->erase_end > sd->size) { + if (erase_start > sd->size || erase_end > sd->size) { sd->card_status |= OUT_OF_RANGE; sd->erase_start = INVALID_ADDRESS; sd->erase_end = INVALID_ADDRESS; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] hw/sd: sd: Fix address check in sd_erase() 2021-01-28 6:43 ` [PATCH 1/3] hw/sd: sd: Fix address check in sd_erase() Bin Meng @ 2021-02-08 10:06 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2021-02-08 10:06 UTC (permalink / raw) To: Bin Meng, qemu-block, qemu-devel; +Cc: Bin Meng On 1/28/21 7:43 AM, Bin Meng wrote: > From: Bin Meng <bin.meng@windriver.com> > > For high capacity memory cards, the erase start address and end > address are multiplied by 512, but the address check is still > based on the original block number in sd->erase_{start, end}. Oops, good catch. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Fixes: 1bd6fd8ed593 ("hw/sd/sdcard: Do not attempt to erase out of range addresses") > Signed-off-by: Bin Meng <bin.meng@windriver.com> > --- > > hw/sd/sd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] hw/sd: sd: Move the sd_block_{read, write} and macros ahead 2021-01-28 6:43 [PATCH 0/3] hw/sd: sd: erase operation fixes Bin Meng 2021-01-28 6:43 ` [PATCH 1/3] hw/sd: sd: Fix address check in sd_erase() Bin Meng @ 2021-01-28 6:43 ` Bin Meng 2021-01-28 7:04 ` Cédric Le Goater 2021-01-28 6:43 ` [PATCH 3/3] hw/sd: sd: Actually perform the erase operation Bin Meng 2021-02-08 3:52 ` [PATCH 0/3] hw/sd: sd: erase operation fixes Bin Meng 3 siblings, 1 reply; 9+ messages in thread From: Bin Meng @ 2021-01-28 6:43 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-block, qemu-devel; +Cc: Bin Meng From: Bin Meng <bin.meng@windriver.com> These APIs and macros may be referenced by functions that are currently before them. Move them ahead a little bit. Signed-off-by: Bin Meng <bin.meng@windriver.com> --- hw/sd/sd.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index a6a0b3dcc6..1886d4b30b 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -739,6 +739,27 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert) qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0); } +static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) +{ + trace_sdcard_read_block(addr, len); + if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { + fprintf(stderr, "sd_blk_read: read error on host side\n"); + } +} + +static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) +{ + trace_sdcard_write_block(addr, len); + if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) { + fprintf(stderr, "sd_blk_write: write error on host side\n"); + } +} + +#define BLK_READ_BLOCK(a, len) sd_blk_read(sd, a, len) +#define BLK_WRITE_BLOCK(a, len) sd_blk_write(sd, a, len) +#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len) +#define APP_WRITE_BLOCK(a, len) + static void sd_erase(SDState *sd) { int i; @@ -1742,27 +1763,6 @@ send_response: return rsplen; } -static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) -{ - trace_sdcard_read_block(addr, len); - if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { - fprintf(stderr, "sd_blk_read: read error on host side\n"); - } -} - -static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) -{ - trace_sdcard_write_block(addr, len); - if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) { - fprintf(stderr, "sd_blk_write: write error on host side\n"); - } -} - -#define BLK_READ_BLOCK(a, len) sd_blk_read(sd, a, len) -#define BLK_WRITE_BLOCK(a, len) sd_blk_write(sd, a, len) -#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len) -#define APP_WRITE_BLOCK(a, len) - void sd_write_byte(SDState *sd, uint8_t value) { int i; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] hw/sd: sd: Move the sd_block_{read, write} and macros ahead 2021-01-28 6:43 ` [PATCH 2/3] hw/sd: sd: Move the sd_block_{read, write} and macros ahead Bin Meng @ 2021-01-28 7:04 ` Cédric Le Goater 2021-02-08 10:12 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 9+ messages in thread From: Cédric Le Goater @ 2021-01-28 7:04 UTC (permalink / raw) To: Bin Meng, Philippe Mathieu-Daudé, qemu-block, qemu-devel; +Cc: Bin Meng Hello Bin, On 1/28/21 7:43 AM, Bin Meng wrote: > From: Bin Meng <bin.meng@windriver.com> > > These APIs and macros may be referenced by functions that are > currently before them. Move them ahead a little bit. We could also change fprintf() by qemu_log_mask() Thanks, C. > Signed-off-by: Bin Meng <bin.meng@windriver.com> > --- > > hw/sd/sd.c | 42 +++++++++++++++++++++--------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index a6a0b3dcc6..1886d4b30b 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -739,6 +739,27 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert) > qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0); > } > > +static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) > +{ > + trace_sdcard_read_block(addr, len); > + if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { > + fprintf(stderr, "sd_blk_read: read error on host side\n"); > + } > +} > + > +static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) > +{ > + trace_sdcard_write_block(addr, len); > + if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) { > + fprintf(stderr, "sd_blk_write: write error on host side\n"); > + } > +} > + > +#define BLK_READ_BLOCK(a, len) sd_blk_read(sd, a, len) > +#define BLK_WRITE_BLOCK(a, len) sd_blk_write(sd, a, len) > +#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len) > +#define APP_WRITE_BLOCK(a, len) > + > static void sd_erase(SDState *sd) > { > int i; > @@ -1742,27 +1763,6 @@ send_response: > return rsplen; > } > > -static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) > -{ > - trace_sdcard_read_block(addr, len); > - if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { > - fprintf(stderr, "sd_blk_read: read error on host side\n"); > - } > -} > - > -static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) > -{ > - trace_sdcard_write_block(addr, len); > - if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) { > - fprintf(stderr, "sd_blk_write: write error on host side\n"); > - } > -} > - > -#define BLK_READ_BLOCK(a, len) sd_blk_read(sd, a, len) > -#define BLK_WRITE_BLOCK(a, len) sd_blk_write(sd, a, len) > -#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len) > -#define APP_WRITE_BLOCK(a, len) > - > void sd_write_byte(SDState *sd, uint8_t value) > { > int i; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] hw/sd: sd: Move the sd_block_{read, write} and macros ahead 2021-01-28 7:04 ` Cédric Le Goater @ 2021-02-08 10:12 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2021-02-08 10:12 UTC (permalink / raw) To: Cédric Le Goater, Bin Meng, qemu-block, qemu-devel; +Cc: Bin Meng On 1/28/21 8:04 AM, Cédric Le Goater wrote: > Hello Bin, > > On 1/28/21 7:43 AM, Bin Meng wrote: >> From: Bin Meng <bin.meng@windriver.com> >> >> These APIs and macros may be referenced by functions that are >> currently before them. Move them ahead a little bit. > > We could also change fprintf() by qemu_log_mask() Hmm in this case warn_report() maybe, as IIUC QEMU aims to support using smaller block drive (which this model currently rejects), in which case this wouldn't be a GUEST_ERROR. Can be done on top in another patch, since this is pure code movement. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] hw/sd: sd: Actually perform the erase operation 2021-01-28 6:43 [PATCH 0/3] hw/sd: sd: erase operation fixes Bin Meng 2021-01-28 6:43 ` [PATCH 1/3] hw/sd: sd: Fix address check in sd_erase() Bin Meng 2021-01-28 6:43 ` [PATCH 2/3] hw/sd: sd: Move the sd_block_{read, write} and macros ahead Bin Meng @ 2021-01-28 6:43 ` Bin Meng 2021-02-08 10:17 ` Philippe Mathieu-Daudé 2021-02-08 3:52 ` [PATCH 0/3] hw/sd: sd: erase operation fixes Bin Meng 3 siblings, 1 reply; 9+ messages in thread From: Bin Meng @ 2021-01-28 6:43 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-block, qemu-devel; +Cc: Bin Meng From: Bin Meng <bin.meng@windriver.com> At present the sd_erase() does not erase the requested range of card data to 0xFFs. Let's make the erase operation actually happen. Signed-off-by: Bin Meng <bin.meng@windriver.com> --- hw/sd/sd.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 1886d4b30b..8c397d4ad7 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -765,6 +765,8 @@ static void sd_erase(SDState *sd) int i; uint64_t erase_start = sd->erase_start; uint64_t erase_end = sd->erase_end; + uint64_t erase_addr; + int erase_len = 1 << HWBLOCK_SHIFT; trace_sdcard_erase(sd->erase_start, sd->erase_end); if (sd->erase_start == INVALID_ADDRESS @@ -788,6 +790,13 @@ static void sd_erase(SDState *sd) return; } + memset(sd->data, 0xff, erase_len); + erase_addr = erase_start; + for (i = 0; i < (erase_end - erase_start) / erase_len + 1; i++) { + BLK_WRITE_BLOCK(erase_addr, erase_len); + erase_addr += erase_len; + } + erase_start = sd_addr_to_wpnum(erase_start); erase_end = sd_addr_to_wpnum(erase_end); sd->erase_start = INVALID_ADDRESS; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] hw/sd: sd: Actually perform the erase operation 2021-01-28 6:43 ` [PATCH 3/3] hw/sd: sd: Actually perform the erase operation Bin Meng @ 2021-02-08 10:17 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2021-02-08 10:17 UTC (permalink / raw) To: Bin Meng, qemu-block, qemu-devel; +Cc: Bin Meng On 1/28/21 7:43 AM, Bin Meng wrote: > From: Bin Meng <bin.meng@windriver.com> > > At present the sd_erase() does not erase the requested range of card > data to 0xFFs. Let's make the erase operation actually happen. > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > > --- > > hw/sd/sd.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 1886d4b30b..8c397d4ad7 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -765,6 +765,8 @@ static void sd_erase(SDState *sd) > int i; > uint64_t erase_start = sd->erase_start; > uint64_t erase_end = sd->erase_end; > + uint64_t erase_addr; > + int erase_len = 1 << HWBLOCK_SHIFT; > > trace_sdcard_erase(sd->erase_start, sd->erase_end); > if (sd->erase_start == INVALID_ADDRESS > @@ -788,6 +790,13 @@ static void sd_erase(SDState *sd) > return; > } > > + memset(sd->data, 0xff, erase_len); > + erase_addr = erase_start; > + for (i = 0; i < (erase_end - erase_start) / erase_len + 1; i++) { > + BLK_WRITE_BLOCK(erase_addr, erase_len); > + erase_addr += erase_len; > + } Watch out, you are erasing eventual write-protected blocks. > erase_start = sd_addr_to_wpnum(erase_start); > erase_end = sd_addr_to_wpnum(erase_end); > sd->erase_start = INVALID_ADDRESS; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] hw/sd: sd: erase operation fixes 2021-01-28 6:43 [PATCH 0/3] hw/sd: sd: erase operation fixes Bin Meng ` (2 preceding siblings ...) 2021-01-28 6:43 ` [PATCH 3/3] hw/sd: sd: Actually perform the erase operation Bin Meng @ 2021-02-08 3:52 ` Bin Meng 3 siblings, 0 replies; 9+ messages in thread From: Bin Meng @ 2021-02-08 3:52 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Qemu-block, qemu-devel@nongnu.org Developers Cc: Bin Meng On Thu, Jan 28, 2021 at 2:43 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > From: Bin Meng <bin.meng@windriver.com> > > This includes several fixes related to erase operation of a SD card. > > Based-on: > http://patchwork.ozlabs.org/project/qemu-devel/list/?series=226785 > > > Bin Meng (3): > hw/sd: sd: Fix address check in sd_erase() > hw/sd: sd: Move the sd_block_{read,write} and macros ahead > hw/sd: sd: Actually perform the erase operation > > hw/sd/sd.c | 53 +++++++++++++++++++++++++++++++---------------------- > 1 file changed, 31 insertions(+), 22 deletions(-) Ping? ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-08 17:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28 6:43 [PATCH 0/3] hw/sd: sd: erase operation fixes Bin Meng
2021-01-28 6:43 ` [PATCH 1/3] hw/sd: sd: Fix address check in sd_erase() Bin Meng
2021-02-08 10:06 ` Philippe Mathieu-Daudé
2021-01-28 6:43 ` [PATCH 2/3] hw/sd: sd: Move the sd_block_{read, write} and macros ahead Bin Meng
2021-01-28 7:04 ` Cédric Le Goater
2021-02-08 10:12 ` Philippe Mathieu-Daudé
2021-01-28 6:43 ` [PATCH 3/3] hw/sd: sd: Actually perform the erase operation Bin Meng
2021-02-08 10:17 ` Philippe Mathieu-Daudé
2021-02-08 3:52 ` [PATCH 0/3] hw/sd: sd: erase operation fixes 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.