Re: [PATCH v43 2/2] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

[Philippe Mathieu-Daudé]

On 3/7/24 09:39, Luc Michel wrote:
> On 18:10 Tue 02 Jul     , Philippe Mathieu-Daudé wrote:
> > "General command" (GEN_CMD, CMD56) is described as:
> >
> >    GEN_CMD is the same as the single block read or write
> >    commands (CMD24 or CMD17). The difference is that [...]
> >    the data block is not a memory payload data but has a
> >    vendor specific format and meaning.
> >
> > Thus this block must not be stored overwriting data block
> > on underlying storage drive. Keep it in a dedicated
> > 'vendor_data[]' array.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Tested-by: Cédric Le Goater <clg@redhat.com>
> > ---
> > v43: Do not re-use VMSTATE_UNUSED_V (danpb)
> > ---
> >   hw/sd/sd.c | 16 +++++++++-------
> >   1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index 808dc1cea6..418ccb14a4 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -153,6 +153,8 @@ struct SDState {
> >       uint32_t data_offset;
> >       size_t data_size;
> >       uint8_t data[512];
> > +    uint8_t vendor_data[512];
> > +
> >       qemu_irq readonly_cb;
> >       qemu_irq inserted_cb;
> >       QEMUTimer *ocr_power_timer;
> > @@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
> >       sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
> >       sd->wp_group_bits = sect;
> >       sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
> > +    memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
> >       memset(sd->function_group, 0, sizeof(sd->function_group));
> >       sd->erase_start = INVALID_ADDRESS;
> >       sd->erase_end = INVALID_ADDRESS;
> > @@ -835,6 +838,7 @@ static const VMStateDescription sd_vmstate = {
> >           VMSTATE_UINT32(data_offset, SDState),
> >           VMSTATE_UINT8_ARRAY(data, SDState, 512),
> >           VMSTATE_UNUSED_V(1, 512),
> > +        VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
>
> Don't you need to bump the VMState version then?

Indeed. I'll add a subsection which is the recommended way:
https://www.qemu.org/docs/master/devel/migration/main.html#subsections

  The most common structure change is adding new data, e.g. when
  adding a newer form of device, or adding that state that you
  previously forgot to migrate. This is best solved using a subsection.

Thanks,

Phil.