Currently, erased blocks are filled with 0xFF. However SCR Bit 55
(DATA_STAT_AFTER_ERASE) indicates that an erase produces zeros. One of
them is wrong.

As erasing to zero is more performant and allows block devices to
use optimizations, we the erase to produce 0x00.

Signed-off-by: Christian Speich <c.speich@avm.de>
---
 hw/sd/sd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 23764ed99f36cf39ee7abe02f08e51897c05e718..94ef3cc62582717ee044c4b114b7f22bd1b4a256 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1115,7 +1115,6 @@ static void sd_erase(SDState *sd)
     sd->erase_end = INVALID_ADDRESS;
     sd->csd[14] |= 0x40;
 
-    memset(sd->data, 0xff, erase_len);
     for (erase_addr = erase_start; erase_addr <= erase_end;
          erase_addr += erase_len) {
         if (sdsc) {
@@ -1127,7 +1126,8 @@ static void sd_erase(SDState *sd)
                 continue;
             }
         }
-        sd_blk_write(sd, erase_addr, erase_len);
+        blk_pwrite_zeroes(sd->blk, erase_addr + sd_part_offset(sd),
+                          erase_len, 0);
     }
 }
 

-- 
2.43.0

On 19/9/25 14:34, Christian Speich wrote:
> Currently, erased blocks are filled with 0xFF. However SCR Bit 55
> (DATA_STAT_AFTER_ERASE) indicates that an erase produces zeros. One of
> them is wrong.

You are right, we don't set DATA_STAT_AFTER_ERASE correctly.

> As erasing to zero is more performant and allows block devices to
> use optimizations, we the erase to produce 0x00.
> 
> Signed-off-by: Christian Speich <c.speich@avm.de>
> ---
>   hw/sd/sd.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 23764ed99f36cf39ee7abe02f08e51897c05e718..94ef3cc62582717ee044c4b114b7f22bd1b4a256 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1115,7 +1115,6 @@ static void sd_erase(SDState *sd)
>       sd->erase_end = INVALID_ADDRESS;
>       sd->csd[14] |= 0x40;
>   
> -    memset(sd->data, 0xff, erase_len);
>       for (erase_addr = erase_start; erase_addr <= erase_end;
>            erase_addr += erase_len) {
>           if (sdsc) {
> @@ -1127,7 +1126,8 @@ static void sd_erase(SDState *sd)
>                   continue;
>               }
>           }
> -        sd_blk_write(sd, erase_addr, erase_len);
> +        blk_pwrite_zeroes(sd->blk, erase_addr + sd_part_offset(sd),
> +                          erase_len, 0);
>       }
>   }

I'm OK with this change, but I'd rather having a device boolean property
so we can keep the old behavior for backward compatibility. Maybe
'erase-block-as-zero'? Do you mind updating this patch?

Regards,

Phil.

On Mon, Nov 24, 2025 at 05:09:03AM +0100, Philippe Mathieu-Daudé wrote:
> On 19/9/25 14:34, Christian Speich wrote:
> > Currently, erased blocks are filled with 0xFF. However SCR Bit 55
> > (DATA_STAT_AFTER_ERASE) indicates that an erase produces zeros. One of
> > them is wrong.
> 
> You are right, we don't set DATA_STAT_AFTER_ERASE correctly.
> 
> > As erasing to zero is more performant and allows block devices to
> > use optimizations, we the erase to produce 0x00.
> > 
> > Signed-off-by: Christian Speich <c.speich@avm.de>
> > ---
> >   hw/sd/sd.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index 23764ed99f36cf39ee7abe02f08e51897c05e718..94ef3cc62582717ee044c4b114b7f22bd1b4a256 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -1115,7 +1115,6 @@ static void sd_erase(SDState *sd)
> >       sd->erase_end = INVALID_ADDRESS;
> >       sd->csd[14] |= 0x40;
> > -    memset(sd->data, 0xff, erase_len);
> >       for (erase_addr = erase_start; erase_addr <= erase_end;
> >            erase_addr += erase_len) {
> >           if (sdsc) {
> > @@ -1127,7 +1126,8 @@ static void sd_erase(SDState *sd)
> >                   continue;
> >               }
> >           }
> > -        sd_blk_write(sd, erase_addr, erase_len);
> > +        blk_pwrite_zeroes(sd->blk, erase_addr + sd_part_offset(sd),
> > +                          erase_len, 0);
> >       }
> >   }
> 
> I'm OK with this change, but I'd rather having a device boolean property
> so we can keep the old behavior for backward compatibility. Maybe
> 'erase-block-as-zero'? Do you mind updating this patch?

I've refrained from making this a user-configruable property to keep the
code simple and as erasing to zero can be better optimized (performance
and disk usage, e.g. the following commit). Additionally, it may be less
confusing to users as new disks created by qemu-img will be filled with
zeros.

I see that this is a breaking change though, so I'll look into adding
a property.

I assume you want erase-block-as-zero to default to false? (I'd prefer
true, but thats just my use-case).

Greetings,
Christian


> 
> Regards,
> 
> Phil.
>