RE: [PATCH v7 3/7] usb/hcd-xhci: Split pci wrapper for xhci base model

[Sai Pavan Boddu]

Hi Gerd,

Thanks, I will update this in V8.

Regards,
Sai Pavan
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Thursday, September 17, 2020 5:14 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>; Markus Armbruster
> <armbru@redhat.com>; 'Marc-André Lureau'
> <marcandre.lureau@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>;
> Edgar Iglesias <edgari@xilinx.com>; Francisco Eduardo Iglesias
> <figlesia@xilinx.com>; qemu-devel@nongnu.org; Alistair Francis
> <alistair.francis@wdc.com>; Eduardo Habkost <ehabkost@redhat.com>;
> Ying Fang <fangying1@huawei.com>; 'Philippe Mathieu-Daudé'
> <philmd@redhat.com>; Vikram Garhwal <fnuv@xilinx.com>; Paul
> Zimmerman <pauldzim@gmail.com>; Sai Pavan Boddu
> <saipava@xilinx.com>
> Subject: Re: [PATCH v7 3/7] usb/hcd-xhci: Split pci wrapper for xhci base
> model
> 
>   Hi,
> 
> > +static const VMStateDescription vmstate_xhci_pci = {
> > +    .name = "xhci-pci",
> > +    .version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_PCI_DEVICE(parent_obj, XHCIPciState),
> > +        VMSTATE_MSIX(parent_obj, XHCIPciState),
> > +        VMSTATE_UINT8_ARRAY(msix_used, XHCIPciState, MAXINTRS),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> 
> > @@ -3634,7 +3527,6 @@ static const VMStateDescription
> vmstate_xhci_intr = {
> >          VMSTATE_UINT32(erdp_high,     XHCIInterrupter),
> >
> >          /* state */
> > -        VMSTATE_BOOL(msix_used,       XHCIInterrupter),
> >          VMSTATE_BOOL(er_pcs,          XHCIInterrupter),
> >          VMSTATE_UINT64(er_start,      XHCIInterrupter),
> >          VMSTATE_UINT32(er_size,       XHCIInterrupter),
> > @@ -3657,9 +3549,6 @@ static const VMStateDescription vmstate_xhci = {
> >      .version_id = 1,
> >      .post_load = usb_xhci_post_load,
> >      .fields = (VMStateField[]) {
> > -        VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
> > -        VMSTATE_MSIX(parent_obj, XHCIState),
> > -
> >          VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
> >                                       vmstate_xhci_port, XHCIPort),
> >          VMSTATE_STRUCT_VARRAY_UINT32(slots, XHCIState, numslots, 1,
> 
> Uh oh, that'll break cross-version live migration (i.e. save state with 5.1, 
> load
> with 5.2).  Sorry for not noticing this earlier.
> 
> You can't remove the msix_used field.  I guess easiest is to just leave that 
> in
> XHCIInterrupter even if only xhci-pci actually uses it.
> 
> The vmstate_xhci + vmstate_xhci_pci reorganization should work with a few
> changes:
>  * First it must be one vmstate which keeps the fields in order.
>    I think you can do that by not registering vmstate_xhci directly, but
>    using an VMSTATE_STRUCT() entry in vmstate_xhci_pci.
>  * Second the name must not change, so vmstate_xhci_pci must continue to
>    use the "xhci" name.
>  * I think for vmstates referenced by VMSTATE_STRUCT the name doesn't
>    matter, so you could rename vmstate_xhci to -- for example --
>    "xhci-core" so vmstate_xhci and vmstate_xhci_pci have different
>    names.
> 
> take care,
>   Gerd