Re: [PATCH v2 6/8] hw/sd: sd: Actually perform the erase operation

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 2/16/21 4:02 PM, 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>
> 
> ---
> 
> Changes in v2:
> - honor the write protection bits for SDSC cards
> 
>  hw/sd/sd.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index f1f98bdec3..b386f16fcb 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -766,6 +766,9 @@ static void sd_erase(SDState *sd)
>      uint64_t erase_start = sd->erase_start;
>      uint64_t erase_end = sd->erase_end;
>      bool sdsc = true;
> +    uint64_t wpnum;
> +    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
> @@ -794,17 +797,20 @@ static void sd_erase(SDState *sd)
>      sd->erase_end = INVALID_ADDRESS;
>      sd->csd[14] |= 0x40;
>  
> -    /* Only SDSC cards support write protect groups */
> -    if (sdsc) {
> -        erase_start = sd_addr_to_wpnum(erase_start);
> -        erase_end = sd_addr_to_wpnum(erase_end);
> -
> -        for (i = erase_start; i <= erase_end; i++) {
> -            assert(i < sd->wpgrps_size);
> -            if (test_bit(i, sd->wp_groups)) {
> +    memset(sd->data, 0xff, erase_len);
> +    erase_addr = erase_start;
> +    for (i = 0; i <= (erase_end - erase_start) / erase_len; i++) {
> +        if (sdsc) {
> +            /* Only SDSC cards support write protect groups */
> +            wpnum = sd_addr_to_wpnum(erase_addr);
> +            assert(wpnum < sd->wpgrps_size);
> +            if (test_bit(wpnum, sd->wp_groups)) {
>                  sd->card_status |= WP_ERASE_SKIP;
> +                continue;

So if a group is protected, you skip it but don't increase erase_addr.
If G#4 is protected and G#5 isn't, when you check G#5 you end erasing
G#4.

>              }
>          }
> +        BLK_WRITE_BLOCK(erase_addr, erase_len);
> +        erase_addr += erase_len;
>      }
>  }
>  
>