On 09:00 Fri 28 Jun , Philippe Mathieu-Daudé wrote: > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > "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é <phi...@linaro.org> > Tested-by: Cédric Le Goater <c...@redhat.com> > --- > RFC: Is it safe to reuse VMSTATE_UNUSED_V() (which happens > to be the same size)? > > Cc: Peter Xu <pet...@redhat.com> > Cc: Fabiano Rosas <faro...@suse.de>
I'm not sure about this migration question. But IMHO you can simplify your implementation to avoid having to store and migrate this vendor_data array. After some research on this command, I came to the conclusion that it's used by manufacturers to return device health related vendor-specific data. (E.g., https://images-na.ssl-images-amazon.com/images/I/91tTtUMDM3L.pdf Section 1.6.1). So I guess you can simply discard writes and return 0s on reads (or "QEMU" in ASCII or... :)). > --- > hw/sd/sd.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 464576751a..1f3eea6e84 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -142,6 +142,8 @@ struct SDState { > uint64_t data_start; > uint32_t data_offset; > uint8_t data[512]; > + uint8_t vendor_data[512]; > + > qemu_irq readonly_cb; > qemu_irq inserted_cb; > QEMUTimer *ocr_power_timer; > @@ -656,6 +658,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; > @@ -771,7 +774,7 @@ static const VMStateDescription sd_vmstate = { > VMSTATE_UINT64(data_start, SDState), > VMSTATE_UINT32(data_offset, SDState), > VMSTATE_UINT8_ARRAY(data, SDState, 512), > - VMSTATE_UNUSED_V(1, 512), > + VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512), > VMSTATE_BOOL(enable, SDState), > VMSTATE_END_OF_LIST() > }, > @@ -2029,9 +2032,8 @@ void sd_write_byte(SDState *sd, uint8_t value) > break; > > case 56: /* CMD56: GEN_CMD */ > - sd->data[sd->data_offset ++] = value; > - if (sd->data_offset >= sd->blk_len) { > - APP_WRITE_BLOCK(sd->data_start, sd->data_offset); > + sd->vendor_data[sd->data_offset ++] = value; > + if (sd->data_offset >= sizeof(sd->vendor_data)) { > sd->state = sd_transfer_state; > } > break; > @@ -2165,12 +2167,11 @@ uint8_t sd_read_byte(SDState *sd) > break; > > case 56: /* CMD56: GEN_CMD */ > - if (sd->data_offset == 0) > - APP_READ_BLOCK(sd->data_start, sd->blk_len); > - ret = sd->data[sd->data_offset ++]; > + ret = sd->vendor_data[sd->data_offset ++]; > > - if (sd->data_offset >= sd->blk_len) > + if (sd->data_offset >= sizeof(sd->vendor_data)) { > sd->state = sd_transfer_state; > + } > break; > > default: > -- > 2.41.0 > > --
