From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50412) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGSKT-0005T8-2R for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:30:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VGSKO-0005xK-3S for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:29:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11706) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGSKN-0005xG-RY for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:29:52 -0400 Date: Mon, 2 Sep 2013 14:31:52 +0300 From: "Michael S. Tsirkin" Message-ID: <20130902113152.GA20911@redhat.com> References: <1375057621-19961-1-git-send-email-afaerber@suse.de> <1375057621-19961-7-git-send-email-afaerber@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1375057621-19961-7-git-send-email-afaerber@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC qom-next for-next v2 6/6] pci: Move VMSTATE_MSIX() into vmstate_pci_device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Anthony Liguori , qemu-devel@nongnu.org, Gerd Hoffmann On Mon, Jul 29, 2013 at 02:27:01AM +0200, Andreas F=E4rber wrote: > Use it conditional on msix_present() and drop msix_{save,load}() calls > following pci_device_{save,load}(). >=20 > This reorders the msix_save() and msix_unuse_all_vectors() calls for > virtio-pci, but they seem independent of each other. No, that's a bug. msix_unuse_all_vectors clears pending state if any, it will lose state if called before load. >=20 > Signed-off-by: Andreas F=E4rber > --- > hw/misc/ivshmem.c | 7 ++----- > hw/net/vmxnet3.c | 27 +++------------------------ > hw/pci/pci.c | 8 ++++++++ > hw/usb/hcd-xhci.c | 1 - > hw/virtio/virtio-pci.c | 19 +++++++++++-------- > include/hw/pci/msix.h | 7 ++++--- > 6 files changed, 28 insertions(+), 41 deletions(-) >=20 > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > index 4a74856..de997cd 100644 > --- a/hw/misc/ivshmem.c > +++ b/hw/misc/ivshmem.c > @@ -599,9 +599,7 @@ static void ivshmem_save(QEMUFile* f, void *opaque) > IVSHMEM_DPRINTF("ivshmem_save\n"); > pci_device_save(pci_dev, f); > =20 > - if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) { > - msix_save(pci_dev, f); > - } else { > + if (!ivshmem_has_feature(proxy, IVSHMEM_MSI)) { > qemu_put_be32(f, proxy->intrstatus); > qemu_put_be32(f, proxy->intrmask); > } > @@ -631,8 +629,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, = int version_id) > } > =20 > if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) { > - msix_load(pci_dev, f); > - ivshmem_use_msix(proxy); > + ivshmem_use_msix(proxy); > } else { > proxy->intrstatus =3D qemu_get_be32(f); > proxy->intrmask =3D qemu_get_be32(f); > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 3bad83c..471ca24 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -2031,21 +2031,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s) > } > } > =20 > -static void > -vmxnet3_msix_save(QEMUFile *f, void *opaque) > -{ > - PCIDevice *d =3D PCI_DEVICE(opaque); > - msix_save(d, f); > -} > - > -static int > -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id) > -{ > - PCIDevice *d =3D PCI_DEVICE(opaque); > - msix_load(d, f); > - return 0; > -} > - > static const MemoryRegionOps b0_ops =3D { > .read =3D vmxnet3_io_bar0_read, > .write =3D vmxnet3_io_bar0_write, > @@ -2103,9 +2088,6 @@ static int vmxnet3_pci_init(PCIDevice *pci_dev) > =20 > vmxnet3_net_init(s); > =20 > - register_savevm(dev, "vmxnet3-msix", -1, 1, > - vmxnet3_msix_save, vmxnet3_msix_load, s); > - > add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0"); > =20 > return 0; > @@ -2114,13 +2096,10 @@ static int vmxnet3_pci_init(PCIDevice *pci_dev) > =20 > static void vmxnet3_pci_uninit(PCIDevice *pci_dev) > { > - DeviceState *dev =3D DEVICE(pci_dev); > VMXNET3State *s =3D VMXNET3(pci_dev); > =20 > VMW_CBPRN("Starting uninit..."); > =20 > - unregister_savevm(dev, "vmxnet3-msix", s); > - > vmxnet3_net_uninit(s); > =20 > vmxnet3_cleanup_msix(s); > @@ -2372,9 +2351,9 @@ const VMStateInfo int_state_info =3D { > =20 > static const VMStateDescription vmstate_vmxnet3 =3D { > .name =3D "vmxnet3", > - .version_id =3D 1, > - .minimum_version_id =3D 1, > - .minimum_version_id_old =3D 1, > + .version_id =3D 2, > + .minimum_version_id =3D 2, > + .minimum_version_id_old =3D 2, > .pre_save =3D vmxnet3_pre_save, > .post_load =3D vmxnet3_post_load, > .fields =3D (VMStateField[]) { > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index b790d98..bd6078b 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -481,6 +481,13 @@ static bool pci_device_aer_needed(void *opaque, in= t version_id) > return pci_is_express(s) && s->exp.aer_log.log !=3D NULL; > } > =20 > +static bool pci_device_msix_needed(void *opaque, int version_id) > +{ > + PCIDevice *s =3D opaque; > + > + return msix_present(s); > +} > + > const VMStateDescription vmstate_pci_device =3D { > .name =3D "PCIDevice", > .version_id =3D 2, > @@ -499,6 +506,7 @@ const VMStateDescription vmstate_pci_device =3D { > VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2, > vmstate_info_pci_irq_state, > PCI_NUM_PINS * sizeof(int32_t)), > + VMSTATE_MSIX_TEST(pci_device_msix_needed), > VMSTATE_STRUCT_TEST(exp.aer_log, PCIDevice, pci_device_aer_nee= ded, 0, > vmstate_pcie_aer_log, PCIEAERLog), > VMSTATE_END_OF_LIST() > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c > index 167b58d..ca7b3cd 100644 > --- a/hw/usb/hcd-xhci.c > +++ b/hw/usb/hcd-xhci.c > @@ -3545,7 +3545,6 @@ static const VMStateDescription vmstate_xhci =3D = { > .post_load =3D usb_xhci_post_load, > .fields =3D (VMStateField[]) { > VMSTATE_PCI_DEVICE(), > - VMSTATE_MSIX(parent_obj, XHCIState), > =20 > VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1, > vmstate_xhci_port, XHCIPort), > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index c38cfd1..8e2789d 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -121,10 +121,12 @@ static void virtio_pci_notify(DeviceState *d, uin= t16_t vector) > static void virtio_pci_save_config(DeviceState *d, QEMUFile *f) > { > VirtIOPCIProxy *proxy =3D to_virtio_pci_proxy(d); > - pci_device_save(&proxy->pci_dev, f); > - msix_save(&proxy->pci_dev, f); > - if (msix_present(&proxy->pci_dev)) > + PCIDevice *pci_dev =3D PCI_DEVICE(proxy); > + > + pci_device_save(pci_dev, f); > + if (msix_present(pci_dev)) { > qemu_put_be16(f, proxy->vdev->config_vector); > + } > } > =20 > static void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f) > @@ -137,20 +139,21 @@ static void virtio_pci_save_queue(DeviceState *d,= int n, QEMUFile *f) > static int virtio_pci_load_config(DeviceState *d, QEMUFile *f) > { > VirtIOPCIProxy *proxy =3D to_virtio_pci_proxy(d); > + PCIDevice *pci_dev =3D PCI_DEVICE(proxy); > int ret; > - ret =3D pci_device_load(&proxy->pci_dev, f); > + > + ret =3D pci_device_load(pci_dev, f); > if (ret) { > return ret; > } > - msix_unuse_all_vectors(&proxy->pci_dev); > - msix_load(&proxy->pci_dev, f); > - if (msix_present(&proxy->pci_dev)) { > + msix_unuse_all_vectors(pci_dev); > + if (msix_present(pci_dev)) { > qemu_get_be16s(f, &proxy->vdev->config_vector); > } else { > proxy->vdev->config_vector =3D VIRTIO_NO_VECTOR; > } > if (proxy->vdev->config_vector !=3D VIRTIO_NO_VECTOR) { > - return msix_vector_use(&proxy->pci_dev, proxy->vdev->config_ve= ctor); > + return msix_vector_use(pci_dev, proxy->vdev->config_vector); > } > return 0; > } > diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h > index 954d82b..b1b8874 100644 > --- a/include/hw/pci/msix.h > +++ b/include/hw/pci/msix.h > @@ -46,12 +46,13 @@ void msix_unset_vector_notifiers(PCIDevice *dev); > =20 > extern const VMStateDescription vmstate_msix; > =20 > -#define VMSTATE_MSIX(_field, _state) { \ > - .name =3D (stringify(_field)), = \ > +#define VMSTATE_MSIX_TEST(_test) { \ > + .name =3D "MSI-X", = \ > .size =3D sizeof(PCIDevice), = \ > .vmsd =3D &vmstate_msix, = \ > .flags =3D VMS_STRUCT, = \ > - .offset =3D vmstate_offset_value(_state, _field, PCIDevice), = \ > + .offset =3D 0, = \ > + .field_exists =3D (_test), = \ > } > =20 > #endif > --=20 > 1.8.1.4