* [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
@ 2016-12-14 19:58 Dr. David Alan Gilbert (git)
2016-12-15 23:18 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2016-12-14 19:58 UTC (permalink / raw)
To: qemu-devel, mst, marcel, quintela, amit.shah
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
The vmstate_pci_device and vmstate_pcie_devices differ
just in the size of one buffer; combine the two using a _TEST
macro.
I think this is safe as long as everywhere which currently
uses either of these two uses the right type.
One thing that concerns me is that some places use pci_device_load/save
which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
macro - how are they getting the same irq mangling?
This passes a smoke test migrate of:
./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
./littlefed20.img -device e1000e -device virtio-net -device
e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
ioh3420 -device nec-usb-xhci
to an unmodified qemu.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/net/e1000e.c | 2 +-
hw/net/vmxnet3.c | 2 +-
hw/pci-bridge/ioh3420.c | 2 +-
hw/pci-bridge/xio3130_downstream.c | 2 +-
hw/pci-bridge/xio3130_upstream.c | 2 +-
hw/pci/pci.c | 41 +++++++++++++++++---------------------
hw/scsi/megasas.c | 2 +-
hw/scsi/vmw_pvscsi.c | 2 +-
hw/usb/hcd-xhci.c | 2 +-
include/hw/pci/pcie.h | 10 ----------
10 files changed, 26 insertions(+), 41 deletions(-)
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 4994e1c..463ac9b 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -592,7 +592,7 @@ static const VMStateDescription e1000e_vmstate = {
.pre_save = e1000e_pre_save,
.post_load = e1000e_post_load,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
+ VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
VMSTATE_MSIX(parent_obj, E1000EState),
VMSTATE_UINT32(ioaddr, E1000EState),
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index bbb898b..86cded9 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2538,7 +2538,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
.minimum_version_id = 1,
.needed = vmxnet3_vmstate_need_pcie_device,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
+ VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
VMSTATE_END_OF_LIST()
}
};
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index c8b5ac4..98114e1 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -178,7 +178,7 @@ static const VMStateDescription vmstate_ioh3420 = {
.minimum_version_id = 1,
.post_load = pcie_cap_slot_post_load,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
+ VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
VMSTATE_END_OF_LIST()
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index cef6e13..4c54301 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -164,7 +164,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
.minimum_version_id = 1,
.post_load = pcie_cap_slot_post_load,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
+ VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
VMSTATE_END_OF_LIST()
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index 4ad0440..45dbd1c 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -136,7 +136,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
+ VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
vmstate_pcie_aer_log, PCIEAERLog),
VMSTATE_END_OF_LIST()
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 16df188..8e4226b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
.put = put_pci_irq_state,
};
+static bool migrate_is_pcie(void *opaque, int version_id)
+{
+ return pci_is_express((PCIDevice *)opaque);
+}
+
+static bool migrate_is_not_pcie(void *opaque, int version_id)
+{
+ return !pci_is_express((PCIDevice *)opaque);
+}
+
const VMStateDescription vmstate_pci_device = {
.name = "PCIDevice",
.version_id = 2,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
- VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
- vmstate_info_pci_config,
+ VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
+ migrate_is_not_pcie,
+ 0, vmstate_info_pci_config,
PCI_CONFIG_SPACE_SIZE),
- VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
- vmstate_info_pci_irq_state,
- PCI_NUM_PINS * sizeof(int32_t)),
- VMSTATE_END_OF_LIST()
- }
-};
-
-const VMStateDescription vmstate_pcie_device = {
- .name = "PCIEDevice",
- .version_id = 2,
- .minimum_version_id = 1,
- .fields = (VMStateField[]) {
- VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
- VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
- vmstate_info_pci_config,
+ VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
+ migrate_is_pcie,
+ 0, vmstate_info_pci_config,
PCIE_CONFIG_SPACE_SIZE),
VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
vmstate_info_pci_irq_state,
@@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
}
};
-static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
-{
- return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
-}
void pci_device_save(PCIDevice *s, QEMUFile *f)
{
@@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
* This makes us compatible with old devices
* which never set or clear this bit. */
s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
- vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
+ vmstate_save_state(f, &vmstate_pci_device, s, NULL);
/* Restore the interrupt status bit. */
pci_update_irq_status(s);
}
@@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
int pci_device_load(PCIDevice *s, QEMUFile *f)
{
int ret;
- ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
+ ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
/* Restore the interrupt status bit. */
pci_update_irq_status(s);
return ret;
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 67fc1e7..adc0c4b 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2288,7 +2288,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
.minimum_version_id = 0,
.minimum_version_id_old = 0,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
+ VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
VMSTATE_MSIX(parent_obj, MegasasState),
VMSTATE_INT32(fw_state, MegasasState),
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index a5ce7de..7557546 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
.name = "pvscsi/pcie",
.needed = pvscsi_vmstate_need_pcie_device,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
+ VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
VMSTATE_END_OF_LIST()
}
};
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4acf0c6..e0b5169 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
.version_id = 1,
.post_load = usb_xhci_post_load,
.fields = (VMStateField[]) {
- VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
+ VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
VMSTATE_MSIX(parent_obj, XHCIState),
VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 056d25e..894262e 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -121,16 +121,6 @@ void pcie_add_capability(PCIDevice *dev,
void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
-extern const VMStateDescription vmstate_pcie_device;
-
-#define VMSTATE_PCIE_DEVICE(_field, _state) { \
- .name = (stringify(_field)), \
- .size = sizeof(PCIDevice), \
- .vmsd = &vmstate_pcie_device, \
- .flags = VMS_STRUCT, \
- .offset = vmstate_offset_value(_state, _field, PCIDevice), \
-}
-
void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp);
void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
2016-12-14 19:58 [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device Dr. David Alan Gilbert (git)
@ 2016-12-15 23:18 ` Michael S. Tsirkin
2016-12-16 9:35 ` Dr. David Alan Gilbert
2017-01-24 12:10 ` Dr. David Alan Gilbert
0 siblings, 2 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2016-12-15 23:18 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, marcel, quintela, amit.shah
On Wed, Dec 14, 2016 at 07:58:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The vmstate_pci_device and vmstate_pcie_devices differ
> just in the size of one buffer; combine the two using a _TEST
> macro.
>
> I think this is safe as long as everywhere which currently
> uses either of these two uses the right type.
>
> One thing that concerns me is that some places use pci_device_load/save
> which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
> macro - how are they getting the same irq mangling?
>
> This passes a smoke test migrate of:
> ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
> ./littlefed20.img -device e1000e -device virtio-net -device
> e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
> ioh3420 -device nec-usb-xhci
>
> to an unmodified qemu.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
feel free to merge through migration tree.
> ---
> hw/net/e1000e.c | 2 +-
> hw/net/vmxnet3.c | 2 +-
> hw/pci-bridge/ioh3420.c | 2 +-
> hw/pci-bridge/xio3130_downstream.c | 2 +-
> hw/pci-bridge/xio3130_upstream.c | 2 +-
> hw/pci/pci.c | 41 +++++++++++++++++---------------------
> hw/scsi/megasas.c | 2 +-
> hw/scsi/vmw_pvscsi.c | 2 +-
> hw/usb/hcd-xhci.c | 2 +-
> include/hw/pci/pcie.h | 10 ----------
> 10 files changed, 26 insertions(+), 41 deletions(-)
>
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 4994e1c..463ac9b 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -592,7 +592,7 @@ static const VMStateDescription e1000e_vmstate = {
> .pre_save = e1000e_pre_save,
> .post_load = e1000e_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
> + VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
> VMSTATE_MSIX(parent_obj, E1000EState),
>
> VMSTATE_UINT32(ioaddr, E1000EState),
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index bbb898b..86cded9 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2538,7 +2538,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> .minimum_version_id = 1,
> .needed = vmxnet3_vmstate_need_pcie_device,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> VMSTATE_END_OF_LIST()
> }
> };
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index c8b5ac4..98114e1 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -178,7 +178,7 @@ static const VMStateDescription vmstate_ioh3420 = {
> .minimum_version_id = 1,
> .post_load = pcie_cap_slot_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> VMSTATE_END_OF_LIST()
> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> index cef6e13..4c54301 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -164,7 +164,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
> .minimum_version_id = 1,
> .post_load = pcie_cap_slot_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> VMSTATE_END_OF_LIST()
> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> index 4ad0440..45dbd1c 100644
> --- a/hw/pci-bridge/xio3130_upstream.c
> +++ b/hw/pci-bridge/xio3130_upstream.c
> @@ -136,7 +136,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
> .version_id = 1,
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
> + VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
> VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
> vmstate_pcie_aer_log, PCIEAERLog),
> VMSTATE_END_OF_LIST()
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 16df188..8e4226b 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
> .put = put_pci_irq_state,
> };
>
> +static bool migrate_is_pcie(void *opaque, int version_id)
> +{
> + return pci_is_express((PCIDevice *)opaque);
> +}
> +
> +static bool migrate_is_not_pcie(void *opaque, int version_id)
> +{
> + return !pci_is_express((PCIDevice *)opaque);
> +}
> +
> const VMStateDescription vmstate_pci_device = {
> .name = "PCIDevice",
> .version_id = 2,
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> - vmstate_info_pci_config,
> + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> + migrate_is_not_pcie,
> + 0, vmstate_info_pci_config,
> PCI_CONFIG_SPACE_SIZE),
> - VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> - vmstate_info_pci_irq_state,
> - PCI_NUM_PINS * sizeof(int32_t)),
> - VMSTATE_END_OF_LIST()
> - }
> -};
> -
> -const VMStateDescription vmstate_pcie_device = {
> - .name = "PCIEDevice",
> - .version_id = 2,
> - .minimum_version_id = 1,
> - .fields = (VMStateField[]) {
> - VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> - vmstate_info_pci_config,
> + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> + migrate_is_pcie,
> + 0, vmstate_info_pci_config,
> PCIE_CONFIG_SPACE_SIZE),
> VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> vmstate_info_pci_irq_state,
> @@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
> }
> };
>
> -static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
> -{
> - return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
> -}
>
> void pci_device_save(PCIDevice *s, QEMUFile *f)
> {
> @@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> * This makes us compatible with old devices
> * which never set or clear this bit. */
> s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> - vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
> + vmstate_save_state(f, &vmstate_pci_device, s, NULL);
> /* Restore the interrupt status bit. */
> pci_update_irq_status(s);
> }
> @@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> int pci_device_load(PCIDevice *s, QEMUFile *f)
> {
> int ret;
> - ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
> + ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
> /* Restore the interrupt status bit. */
> pci_update_irq_status(s);
> return ret;
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index 67fc1e7..adc0c4b 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2288,7 +2288,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
> .minimum_version_id = 0,
> .minimum_version_id_old = 0,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> + VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
> VMSTATE_MSIX(parent_obj, MegasasState),
>
> VMSTATE_INT32(fw_state, MegasasState),
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index a5ce7de..7557546 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
> .name = "pvscsi/pcie",
> .needed = pvscsi_vmstate_need_pcie_device,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
> + VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
> VMSTATE_END_OF_LIST()
> }
> };
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 4acf0c6..e0b5169 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
> .version_id = 1,
> .post_load = usb_xhci_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
> + VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
> VMSTATE_MSIX(parent_obj, XHCIState),
>
> VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index 056d25e..894262e 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -121,16 +121,6 @@ void pcie_add_capability(PCIDevice *dev,
> void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
>
> -extern const VMStateDescription vmstate_pcie_device;
> -
> -#define VMSTATE_PCIE_DEVICE(_field, _state) { \
> - .name = (stringify(_field)), \
> - .size = sizeof(PCIDevice), \
> - .vmsd = &vmstate_pcie_device, \
> - .flags = VMS_STRUCT, \
> - .offset = vmstate_offset_value(_state, _field, PCIDevice), \
> -}
> -
> void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp);
> void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> --
> 2.9.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
2016-12-15 23:18 ` Michael S. Tsirkin
@ 2016-12-16 9:35 ` Dr. David Alan Gilbert
2017-01-10 3:38 ` Michael S. Tsirkin
2017-01-24 12:10 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2016-12-16 9:35 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, marcel, quintela, amit.shah
* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Dec 14, 2016 at 07:58:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > The vmstate_pci_device and vmstate_pcie_devices differ
> > just in the size of one buffer; combine the two using a _TEST
> > macro.
> >
> > I think this is safe as long as everywhere which currently
> > uses either of these two uses the right type.
> >
> > One thing that concerns me is that some places use pci_device_load/save
> > which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
> > macro - how are they getting the same irq mangling?
> >
> > This passes a smoke test migrate of:
> > ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
> > ./littlefed20.img -device e1000e -device virtio-net -device
> > e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
> > ioh3420 -device nec-usb-xhci
> >
> > to an unmodified qemu.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> feel free to merge through migration tree.
Thanks!
What about my related question about the difference between pci_device_load/save
with respect to the irq mangling it does?
Dave
> > ---
> > hw/net/e1000e.c | 2 +-
> > hw/net/vmxnet3.c | 2 +-
> > hw/pci-bridge/ioh3420.c | 2 +-
> > hw/pci-bridge/xio3130_downstream.c | 2 +-
> > hw/pci-bridge/xio3130_upstream.c | 2 +-
> > hw/pci/pci.c | 41 +++++++++++++++++---------------------
> > hw/scsi/megasas.c | 2 +-
> > hw/scsi/vmw_pvscsi.c | 2 +-
> > hw/usb/hcd-xhci.c | 2 +-
> > include/hw/pci/pcie.h | 10 ----------
> > 10 files changed, 26 insertions(+), 41 deletions(-)
> >
> > diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> > index 4994e1c..463ac9b 100644
> > --- a/hw/net/e1000e.c
> > +++ b/hw/net/e1000e.c
> > @@ -592,7 +592,7 @@ static const VMStateDescription e1000e_vmstate = {
> > .pre_save = e1000e_pre_save,
> > .post_load = e1000e_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
> > + VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
> > VMSTATE_MSIX(parent_obj, E1000EState),
> >
> > VMSTATE_UINT32(ioaddr, E1000EState),
> > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > index bbb898b..86cded9 100644
> > --- a/hw/net/vmxnet3.c
> > +++ b/hw/net/vmxnet3.c
> > @@ -2538,7 +2538,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > .minimum_version_id = 1,
> > .needed = vmxnet3_vmstate_need_pcie_device,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
> > + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > VMSTATE_END_OF_LIST()
> > }
> > };
> > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> > index c8b5ac4..98114e1 100644
> > --- a/hw/pci-bridge/ioh3420.c
> > +++ b/hw/pci-bridge/ioh3420.c
> > @@ -178,7 +178,7 @@ static const VMStateDescription vmstate_ioh3420 = {
> > .minimum_version_id = 1,
> > .post_load = pcie_cap_slot_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > VMSTATE_END_OF_LIST()
> > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> > index cef6e13..4c54301 100644
> > --- a/hw/pci-bridge/xio3130_downstream.c
> > +++ b/hw/pci-bridge/xio3130_downstream.c
> > @@ -164,7 +164,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
> > .minimum_version_id = 1,
> > .post_load = pcie_cap_slot_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > VMSTATE_END_OF_LIST()
> > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> > index 4ad0440..45dbd1c 100644
> > --- a/hw/pci-bridge/xio3130_upstream.c
> > +++ b/hw/pci-bridge/xio3130_upstream.c
> > @@ -136,7 +136,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
> > .version_id = 1,
> > .minimum_version_id = 1,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
> > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
> > VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
> > vmstate_pcie_aer_log, PCIEAERLog),
> > VMSTATE_END_OF_LIST()
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 16df188..8e4226b 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
> > .put = put_pci_irq_state,
> > };
> >
> > +static bool migrate_is_pcie(void *opaque, int version_id)
> > +{
> > + return pci_is_express((PCIDevice *)opaque);
> > +}
> > +
> > +static bool migrate_is_not_pcie(void *opaque, int version_id)
> > +{
> > + return !pci_is_express((PCIDevice *)opaque);
> > +}
> > +
> > const VMStateDescription vmstate_pci_device = {
> > .name = "PCIDevice",
> > .version_id = 2,
> > .minimum_version_id = 1,
> > .fields = (VMStateField[]) {
> > VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > - vmstate_info_pci_config,
> > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > + migrate_is_not_pcie,
> > + 0, vmstate_info_pci_config,
> > PCI_CONFIG_SPACE_SIZE),
> > - VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > - vmstate_info_pci_irq_state,
> > - PCI_NUM_PINS * sizeof(int32_t)),
> > - VMSTATE_END_OF_LIST()
> > - }
> > -};
> > -
> > -const VMStateDescription vmstate_pcie_device = {
> > - .name = "PCIEDevice",
> > - .version_id = 2,
> > - .minimum_version_id = 1,
> > - .fields = (VMStateField[]) {
> > - VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > - vmstate_info_pci_config,
> > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > + migrate_is_pcie,
> > + 0, vmstate_info_pci_config,
> > PCIE_CONFIG_SPACE_SIZE),
> > VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > vmstate_info_pci_irq_state,
> > @@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
> > }
> > };
> >
> > -static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
> > -{
> > - return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
> > -}
> >
> > void pci_device_save(PCIDevice *s, QEMUFile *f)
> > {
> > @@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > * This makes us compatible with old devices
> > * which never set or clear this bit. */
> > s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> > - vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
> > + vmstate_save_state(f, &vmstate_pci_device, s, NULL);
> > /* Restore the interrupt status bit. */
> > pci_update_irq_status(s);
> > }
> > @@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > int pci_device_load(PCIDevice *s, QEMUFile *f)
> > {
> > int ret;
> > - ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
> > + ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
> > /* Restore the interrupt status bit. */
> > pci_update_irq_status(s);
> > return ret;
> > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> > index 67fc1e7..adc0c4b 100644
> > --- a/hw/scsi/megasas.c
> > +++ b/hw/scsi/megasas.c
> > @@ -2288,7 +2288,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
> > .minimum_version_id = 0,
> > .minimum_version_id_old = 0,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> > + VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
> > VMSTATE_MSIX(parent_obj, MegasasState),
> >
> > VMSTATE_INT32(fw_state, MegasasState),
> > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > index a5ce7de..7557546 100644
> > --- a/hw/scsi/vmw_pvscsi.c
> > +++ b/hw/scsi/vmw_pvscsi.c
> > @@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
> > .name = "pvscsi/pcie",
> > .needed = pvscsi_vmstate_need_pcie_device,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
> > + VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
> > VMSTATE_END_OF_LIST()
> > }
> > };
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index 4acf0c6..e0b5169 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
> > .version_id = 1,
> > .post_load = usb_xhci_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
> > + VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
> > VMSTATE_MSIX(parent_obj, XHCIState),
> >
> > VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
> > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > index 056d25e..894262e 100644
> > --- a/include/hw/pci/pcie.h
> > +++ b/include/hw/pci/pcie.h
> > @@ -121,16 +121,6 @@ void pcie_add_capability(PCIDevice *dev,
> > void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> > void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> >
> > -extern const VMStateDescription vmstate_pcie_device;
> > -
> > -#define VMSTATE_PCIE_DEVICE(_field, _state) { \
> > - .name = (stringify(_field)), \
> > - .size = sizeof(PCIDevice), \
> > - .vmsd = &vmstate_pcie_device, \
> > - .flags = VMS_STRUCT, \
> > - .offset = vmstate_offset_value(_state, _field, PCIDevice), \
> > -}
> > -
> > void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> > Error **errp);
> > void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> > --
> > 2.9.3
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
2016-12-16 9:35 ` Dr. David Alan Gilbert
@ 2017-01-10 3:38 ` Michael S. Tsirkin
2017-01-12 17:27 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2017-01-10 3:38 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-devel, marcel, quintela, amit.shah
On Fri, Dec 16, 2016 at 09:35:28AM +0000, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Dec 14, 2016 at 07:58:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > >
> > > The vmstate_pci_device and vmstate_pcie_devices differ
> > > just in the size of one buffer; combine the two using a _TEST
> > > macro.
> > >
> > > I think this is safe as long as everywhere which currently
> > > uses either of these two uses the right type.
> > >
> > > One thing that concerns me is that some places use pci_device_load/save
> > > which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
> > > macro - how are they getting the same irq mangling?
> > >
> > > This passes a smoke test migrate of:
> > > ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
> > > ./littlefed20.img -device e1000e -device virtio-net -device
> > > e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
> > > ioh3420 -device nec-usb-xhci
> > >
> > > to an unmodified qemu.
> > >
> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > feel free to merge through migration tree.
>
> Thanks!
> What about my related question about the difference between pci_device_load/save
> with respect to the irq mangling it does?
>
> Dave
Do you mean pci_update_irq_status?
All this saving/clearing is for compatibility with
old code which didn't set/clear it properly.
I would not be too surprised if that compatibility
just got broken at some point ...
> > > ---
> > > hw/net/e1000e.c | 2 +-
> > > hw/net/vmxnet3.c | 2 +-
> > > hw/pci-bridge/ioh3420.c | 2 +-
> > > hw/pci-bridge/xio3130_downstream.c | 2 +-
> > > hw/pci-bridge/xio3130_upstream.c | 2 +-
> > > hw/pci/pci.c | 41 +++++++++++++++++---------------------
> > > hw/scsi/megasas.c | 2 +-
> > > hw/scsi/vmw_pvscsi.c | 2 +-
> > > hw/usb/hcd-xhci.c | 2 +-
> > > include/hw/pci/pcie.h | 10 ----------
> > > 10 files changed, 26 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> > > index 4994e1c..463ac9b 100644
> > > --- a/hw/net/e1000e.c
> > > +++ b/hw/net/e1000e.c
> > > @@ -592,7 +592,7 @@ static const VMStateDescription e1000e_vmstate = {
> > > .pre_save = e1000e_pre_save,
> > > .post_load = e1000e_post_load,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
> > > + VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
> > > VMSTATE_MSIX(parent_obj, E1000EState),
> > >
> > > VMSTATE_UINT32(ioaddr, E1000EState),
> > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > > index bbb898b..86cded9 100644
> > > --- a/hw/net/vmxnet3.c
> > > +++ b/hw/net/vmxnet3.c
> > > @@ -2538,7 +2538,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > > .minimum_version_id = 1,
> > > .needed = vmxnet3_vmstate_need_pcie_device,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
> > > + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > > VMSTATE_END_OF_LIST()
> > > }
> > > };
> > > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> > > index c8b5ac4..98114e1 100644
> > > --- a/hw/pci-bridge/ioh3420.c
> > > +++ b/hw/pci-bridge/ioh3420.c
> > > @@ -178,7 +178,7 @@ static const VMStateDescription vmstate_ioh3420 = {
> > > .minimum_version_id = 1,
> > > .post_load = pcie_cap_slot_post_load,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > > VMSTATE_END_OF_LIST()
> > > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> > > index cef6e13..4c54301 100644
> > > --- a/hw/pci-bridge/xio3130_downstream.c
> > > +++ b/hw/pci-bridge/xio3130_downstream.c
> > > @@ -164,7 +164,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
> > > .minimum_version_id = 1,
> > > .post_load = pcie_cap_slot_post_load,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > > VMSTATE_END_OF_LIST()
> > > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> > > index 4ad0440..45dbd1c 100644
> > > --- a/hw/pci-bridge/xio3130_upstream.c
> > > +++ b/hw/pci-bridge/xio3130_upstream.c
> > > @@ -136,7 +136,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
> > > .version_id = 1,
> > > .minimum_version_id = 1,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
> > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
> > > VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
> > > vmstate_pcie_aer_log, PCIEAERLog),
> > > VMSTATE_END_OF_LIST()
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index 16df188..8e4226b 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
> > > .put = put_pci_irq_state,
> > > };
> > >
> > > +static bool migrate_is_pcie(void *opaque, int version_id)
> > > +{
> > > + return pci_is_express((PCIDevice *)opaque);
> > > +}
> > > +
> > > +static bool migrate_is_not_pcie(void *opaque, int version_id)
> > > +{
> > > + return !pci_is_express((PCIDevice *)opaque);
> > > +}
> > > +
> > > const VMStateDescription vmstate_pci_device = {
> > > .name = "PCIDevice",
> > > .version_id = 2,
> > > .minimum_version_id = 1,
> > > .fields = (VMStateField[]) {
> > > VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > > - vmstate_info_pci_config,
> > > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > > + migrate_is_not_pcie,
> > > + 0, vmstate_info_pci_config,
> > > PCI_CONFIG_SPACE_SIZE),
> > > - VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > > - vmstate_info_pci_irq_state,
> > > - PCI_NUM_PINS * sizeof(int32_t)),
> > > - VMSTATE_END_OF_LIST()
> > > - }
> > > -};
> > > -
> > > -const VMStateDescription vmstate_pcie_device = {
> > > - .name = "PCIEDevice",
> > > - .version_id = 2,
> > > - .minimum_version_id = 1,
> > > - .fields = (VMStateField[]) {
> > > - VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > > - vmstate_info_pci_config,
> > > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > > + migrate_is_pcie,
> > > + 0, vmstate_info_pci_config,
> > > PCIE_CONFIG_SPACE_SIZE),
> > > VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > > vmstate_info_pci_irq_state,
> > > @@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
> > > }
> > > };
> > >
> > > -static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
> > > -{
> > > - return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
> > > -}
> > >
> > > void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > {
> > > @@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > * This makes us compatible with old devices
> > > * which never set or clear this bit. */
> > > s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> > > - vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
> > > + vmstate_save_state(f, &vmstate_pci_device, s, NULL);
> > > /* Restore the interrupt status bit. */
> > > pci_update_irq_status(s);
> > > }
> > > @@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > int pci_device_load(PCIDevice *s, QEMUFile *f)
> > > {
> > > int ret;
> > > - ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
> > > + ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
> > > /* Restore the interrupt status bit. */
> > > pci_update_irq_status(s);
> > > return ret;
> > > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> > > index 67fc1e7..adc0c4b 100644
> > > --- a/hw/scsi/megasas.c
> > > +++ b/hw/scsi/megasas.c
> > > @@ -2288,7 +2288,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
> > > .minimum_version_id = 0,
> > > .minimum_version_id_old = 0,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> > > + VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
> > > VMSTATE_MSIX(parent_obj, MegasasState),
> > >
> > > VMSTATE_INT32(fw_state, MegasasState),
> > > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > > index a5ce7de..7557546 100644
> > > --- a/hw/scsi/vmw_pvscsi.c
> > > +++ b/hw/scsi/vmw_pvscsi.c
> > > @@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
> > > .name = "pvscsi/pcie",
> > > .needed = pvscsi_vmstate_need_pcie_device,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
> > > + VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
> > > VMSTATE_END_OF_LIST()
> > > }
> > > };
> > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > > index 4acf0c6..e0b5169 100644
> > > --- a/hw/usb/hcd-xhci.c
> > > +++ b/hw/usb/hcd-xhci.c
> > > @@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
> > > .version_id = 1,
> > > .post_load = usb_xhci_post_load,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
> > > + VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
> > > VMSTATE_MSIX(parent_obj, XHCIState),
> > >
> > > VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
> > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > > index 056d25e..894262e 100644
> > > --- a/include/hw/pci/pcie.h
> > > +++ b/include/hw/pci/pcie.h
> > > @@ -121,16 +121,6 @@ void pcie_add_capability(PCIDevice *dev,
> > > void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> > > void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> > >
> > > -extern const VMStateDescription vmstate_pcie_device;
> > > -
> > > -#define VMSTATE_PCIE_DEVICE(_field, _state) { \
> > > - .name = (stringify(_field)), \
> > > - .size = sizeof(PCIDevice), \
> > > - .vmsd = &vmstate_pcie_device, \
> > > - .flags = VMS_STRUCT, \
> > > - .offset = vmstate_offset_value(_state, _field, PCIDevice), \
> > > -}
> > > -
> > > void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> > > Error **errp);
> > > void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> > > --
> > > 2.9.3
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
2017-01-10 3:38 ` Michael S. Tsirkin
@ 2017-01-12 17:27 ` Dr. David Alan Gilbert
2017-01-12 20:13 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-12 17:27 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, marcel, quintela, amit.shah
* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Fri, Dec 16, 2016 at 09:35:28AM +0000, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Wed, Dec 14, 2016 at 07:58:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > >
> > > > The vmstate_pci_device and vmstate_pcie_devices differ
> > > > just in the size of one buffer; combine the two using a _TEST
> > > > macro.
> > > >
> > > > I think this is safe as long as everywhere which currently
> > > > uses either of these two uses the right type.
> > > >
> > > > One thing that concerns me is that some places use pci_device_load/save
> > > > which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
> > > > macro - how are they getting the same irq mangling?
> > > >
> > > > This passes a smoke test migrate of:
> > > > ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
> > > > ./littlefed20.img -device e1000e -device virtio-net -device
> > > > e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
> > > > ioh3420 -device nec-usb-xhci
> > > >
> > > > to an unmodified qemu.
> > > >
> > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > >
> > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > >
> > > feel free to merge through migration tree.
> >
> > Thanks!
> > What about my related question about the difference between pci_device_load/save
> > with respect to the irq mangling it does?
> >
> > Dave
>
> Do you mean pci_update_irq_status?
>
> All this saving/clearing is for compatibility with
> old code which didn't set/clear it properly.
> I would not be too surprised if that compatibility
> just got broken at some point ...
Yes, there's pci_update_irq_status in both pci_device_save and pci_device_load,
but also the commented code in pci_device_save that does:
s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
when is it safe to replace a pci_device_load/save by the use
of the VMSTATE_PCI_DEVICE macro?
I see the save only in virtio-pci.c,
all the other files that use it only use it in compatibility code.
Dave
>
>
> > > > ---
> > > > hw/net/e1000e.c | 2 +-
> > > > hw/net/vmxnet3.c | 2 +-
> > > > hw/pci-bridge/ioh3420.c | 2 +-
> > > > hw/pci-bridge/xio3130_downstream.c | 2 +-
> > > > hw/pci-bridge/xio3130_upstream.c | 2 +-
> > > > hw/pci/pci.c | 41 +++++++++++++++++---------------------
> > > > hw/scsi/megasas.c | 2 +-
> > > > hw/scsi/vmw_pvscsi.c | 2 +-
> > > > hw/usb/hcd-xhci.c | 2 +-
> > > > include/hw/pci/pcie.h | 10 ----------
> > > > 10 files changed, 26 insertions(+), 41 deletions(-)
> > > >
> > > > diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> > > > index 4994e1c..463ac9b 100644
> > > > --- a/hw/net/e1000e.c
> > > > +++ b/hw/net/e1000e.c
> > > > @@ -592,7 +592,7 @@ static const VMStateDescription e1000e_vmstate = {
> > > > .pre_save = e1000e_pre_save,
> > > > .post_load = e1000e_post_load,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
> > > > + VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
> > > > VMSTATE_MSIX(parent_obj, E1000EState),
> > > >
> > > > VMSTATE_UINT32(ioaddr, E1000EState),
> > > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > > > index bbb898b..86cded9 100644
> > > > --- a/hw/net/vmxnet3.c
> > > > +++ b/hw/net/vmxnet3.c
> > > > @@ -2538,7 +2538,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > > > .minimum_version_id = 1,
> > > > .needed = vmxnet3_vmstate_need_pcie_device,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
> > > > + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > > > VMSTATE_END_OF_LIST()
> > > > }
> > > > };
> > > > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> > > > index c8b5ac4..98114e1 100644
> > > > --- a/hw/pci-bridge/ioh3420.c
> > > > +++ b/hw/pci-bridge/ioh3420.c
> > > > @@ -178,7 +178,7 @@ static const VMStateDescription vmstate_ioh3420 = {
> > > > .minimum_version_id = 1,
> > > > .post_load = pcie_cap_slot_post_load,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > > > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > > > VMSTATE_END_OF_LIST()
> > > > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> > > > index cef6e13..4c54301 100644
> > > > --- a/hw/pci-bridge/xio3130_downstream.c
> > > > +++ b/hw/pci-bridge/xio3130_downstream.c
> > > > @@ -164,7 +164,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
> > > > .minimum_version_id = 1,
> > > > .post_load = pcie_cap_slot_post_load,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > > > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > > > VMSTATE_END_OF_LIST()
> > > > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> > > > index 4ad0440..45dbd1c 100644
> > > > --- a/hw/pci-bridge/xio3130_upstream.c
> > > > +++ b/hw/pci-bridge/xio3130_upstream.c
> > > > @@ -136,7 +136,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
> > > > .version_id = 1,
> > > > .minimum_version_id = 1,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
> > > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
> > > > VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
> > > > vmstate_pcie_aer_log, PCIEAERLog),
> > > > VMSTATE_END_OF_LIST()
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index 16df188..8e4226b 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
> > > > .put = put_pci_irq_state,
> > > > };
> > > >
> > > > +static bool migrate_is_pcie(void *opaque, int version_id)
> > > > +{
> > > > + return pci_is_express((PCIDevice *)opaque);
> > > > +}
> > > > +
> > > > +static bool migrate_is_not_pcie(void *opaque, int version_id)
> > > > +{
> > > > + return !pci_is_express((PCIDevice *)opaque);
> > > > +}
> > > > +
> > > > const VMStateDescription vmstate_pci_device = {
> > > > .name = "PCIDevice",
> > > > .version_id = 2,
> > > > .minimum_version_id = 1,
> > > > .fields = (VMStateField[]) {
> > > > VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > > > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > > > - vmstate_info_pci_config,
> > > > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > > > + migrate_is_not_pcie,
> > > > + 0, vmstate_info_pci_config,
> > > > PCI_CONFIG_SPACE_SIZE),
> > > > - VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > > > - vmstate_info_pci_irq_state,
> > > > - PCI_NUM_PINS * sizeof(int32_t)),
> > > > - VMSTATE_END_OF_LIST()
> > > > - }
> > > > -};
> > > > -
> > > > -const VMStateDescription vmstate_pcie_device = {
> > > > - .name = "PCIEDevice",
> > > > - .version_id = 2,
> > > > - .minimum_version_id = 1,
> > > > - .fields = (VMStateField[]) {
> > > > - VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > > > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > > > - vmstate_info_pci_config,
> > > > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > > > + migrate_is_pcie,
> > > > + 0, vmstate_info_pci_config,
> > > > PCIE_CONFIG_SPACE_SIZE),
> > > > VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > > > vmstate_info_pci_irq_state,
> > > > @@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
> > > > }
> > > > };
> > > >
> > > > -static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
> > > > -{
> > > > - return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
> > > > -}
> > > >
> > > > void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > > {
> > > > @@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > > * This makes us compatible with old devices
> > > > * which never set or clear this bit. */
> > > > s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> > > > - vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
> > > > + vmstate_save_state(f, &vmstate_pci_device, s, NULL);
> > > > /* Restore the interrupt status bit. */
> > > > pci_update_irq_status(s);
> > > > }
> > > > @@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > > int pci_device_load(PCIDevice *s, QEMUFile *f)
> > > > {
> > > > int ret;
> > > > - ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
> > > > + ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
> > > > /* Restore the interrupt status bit. */
> > > > pci_update_irq_status(s);
> > > > return ret;
> > > > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> > > > index 67fc1e7..adc0c4b 100644
> > > > --- a/hw/scsi/megasas.c
> > > > +++ b/hw/scsi/megasas.c
> > > > @@ -2288,7 +2288,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
> > > > .minimum_version_id = 0,
> > > > .minimum_version_id_old = 0,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> > > > + VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
> > > > VMSTATE_MSIX(parent_obj, MegasasState),
> > > >
> > > > VMSTATE_INT32(fw_state, MegasasState),
> > > > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > > > index a5ce7de..7557546 100644
> > > > --- a/hw/scsi/vmw_pvscsi.c
> > > > +++ b/hw/scsi/vmw_pvscsi.c
> > > > @@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
> > > > .name = "pvscsi/pcie",
> > > > .needed = pvscsi_vmstate_need_pcie_device,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
> > > > + VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
> > > > VMSTATE_END_OF_LIST()
> > > > }
> > > > };
> > > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > > > index 4acf0c6..e0b5169 100644
> > > > --- a/hw/usb/hcd-xhci.c
> > > > +++ b/hw/usb/hcd-xhci.c
> > > > @@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
> > > > .version_id = 1,
> > > > .post_load = usb_xhci_post_load,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
> > > > + VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
> > > > VMSTATE_MSIX(parent_obj, XHCIState),
> > > >
> > > > VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
> > > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > > > index 056d25e..894262e 100644
> > > > --- a/include/hw/pci/pcie.h
> > > > +++ b/include/hw/pci/pcie.h
> > > > @@ -121,16 +121,6 @@ void pcie_add_capability(PCIDevice *dev,
> > > > void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> > > > void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> > > >
> > > > -extern const VMStateDescription vmstate_pcie_device;
> > > > -
> > > > -#define VMSTATE_PCIE_DEVICE(_field, _state) { \
> > > > - .name = (stringify(_field)), \
> > > > - .size = sizeof(PCIDevice), \
> > > > - .vmsd = &vmstate_pcie_device, \
> > > > - .flags = VMS_STRUCT, \
> > > > - .offset = vmstate_offset_value(_state, _field, PCIDevice), \
> > > > -}
> > > > -
> > > > void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> > > > Error **errp);
> > > > void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> > > > --
> > > > 2.9.3
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
2017-01-12 17:27 ` Dr. David Alan Gilbert
@ 2017-01-12 20:13 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2017-01-12 20:13 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-devel, marcel, quintela, amit.shah
On Thu, Jan 12, 2017 at 05:27:40PM +0000, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Fri, Dec 16, 2016 at 09:35:28AM +0000, Dr. David Alan Gilbert wrote:
> > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > On Wed, Dec 14, 2016 at 07:58:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > >
> > > > > The vmstate_pci_device and vmstate_pcie_devices differ
> > > > > just in the size of one buffer; combine the two using a _TEST
> > > > > macro.
> > > > >
> > > > > I think this is safe as long as everywhere which currently
> > > > > uses either of these two uses the right type.
> > > > >
> > > > > One thing that concerns me is that some places use pci_device_load/save
> > > > > which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
> > > > > macro - how are they getting the same irq mangling?
> > > > >
> > > > > This passes a smoke test migrate of:
> > > > > ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
> > > > > ./littlefed20.img -device e1000e -device virtio-net -device
> > > > > e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
> > > > > ioh3420 -device nec-usb-xhci
> > > > >
> > > > > to an unmodified qemu.
> > > > >
> > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > >
> > > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > > >
> > > > feel free to merge through migration tree.
> > >
> > > Thanks!
> > > What about my related question about the difference between pci_device_load/save
> > > with respect to the irq mangling it does?
> > >
> > > Dave
> >
> > Do you mean pci_update_irq_status?
> >
> > All this saving/clearing is for compatibility with
> > old code which didn't set/clear it properly.
> > I would not be too surprised if that compatibility
> > just got broken at some point ...
>
> Yes, there's pci_update_irq_status in both pci_device_save and pci_device_load,
> but also the commented code in pci_device_save that does:
> s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
>
> when is it safe to replace a pci_device_load/save by the use
> of the VMSTATE_PCI_DEVICE macro?
> I see the save only in virtio-pci.c,
> all the other files that use it only use it in compatibility code.
>
> Dave
Basically if you don't care about compatilibity with
qemu before f9bf77dd1f838b0061172fe41709b221956da2f5
(that is 0.11 and 0.10) then you can use it.
You need pci_update_irq_status on load and
s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT on save
for 0.11 and 0.10 machine types.
Maybe just teach VMSTATE_PCI_DEVICE about these old
machine types?
> >
> >
> > > > > ---
> > > > > hw/net/e1000e.c | 2 +-
> > > > > hw/net/vmxnet3.c | 2 +-
> > > > > hw/pci-bridge/ioh3420.c | 2 +-
> > > > > hw/pci-bridge/xio3130_downstream.c | 2 +-
> > > > > hw/pci-bridge/xio3130_upstream.c | 2 +-
> > > > > hw/pci/pci.c | 41 +++++++++++++++++---------------------
> > > > > hw/scsi/megasas.c | 2 +-
> > > > > hw/scsi/vmw_pvscsi.c | 2 +-
> > > > > hw/usb/hcd-xhci.c | 2 +-
> > > > > include/hw/pci/pcie.h | 10 ----------
> > > > > 10 files changed, 26 insertions(+), 41 deletions(-)
> > > > >
> > > > > diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> > > > > index 4994e1c..463ac9b 100644
> > > > > --- a/hw/net/e1000e.c
> > > > > +++ b/hw/net/e1000e.c
> > > > > @@ -592,7 +592,7 @@ static const VMStateDescription e1000e_vmstate = {
> > > > > .pre_save = e1000e_pre_save,
> > > > > .post_load = e1000e_post_load,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
> > > > > VMSTATE_MSIX(parent_obj, E1000EState),
> > > > >
> > > > > VMSTATE_UINT32(ioaddr, E1000EState),
> > > > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > > > > index bbb898b..86cded9 100644
> > > > > --- a/hw/net/vmxnet3.c
> > > > > +++ b/hw/net/vmxnet3.c
> > > > > @@ -2538,7 +2538,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > > > > .minimum_version_id = 1,
> > > > > .needed = vmxnet3_vmstate_need_pcie_device,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > > > > VMSTATE_END_OF_LIST()
> > > > > }
> > > > > };
> > > > > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> > > > > index c8b5ac4..98114e1 100644
> > > > > --- a/hw/pci-bridge/ioh3420.c
> > > > > +++ b/hw/pci-bridge/ioh3420.c
> > > > > @@ -178,7 +178,7 @@ static const VMStateDescription vmstate_ioh3420 = {
> > > > > .minimum_version_id = 1,
> > > > > .post_load = pcie_cap_slot_post_load,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > > > > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > > > > VMSTATE_END_OF_LIST()
> > > > > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> > > > > index cef6e13..4c54301 100644
> > > > > --- a/hw/pci-bridge/xio3130_downstream.c
> > > > > +++ b/hw/pci-bridge/xio3130_downstream.c
> > > > > @@ -164,7 +164,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
> > > > > .minimum_version_id = 1,
> > > > > .post_load = pcie_cap_slot_post_load,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > > > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > > > > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > > > > VMSTATE_END_OF_LIST()
> > > > > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> > > > > index 4ad0440..45dbd1c 100644
> > > > > --- a/hw/pci-bridge/xio3130_upstream.c
> > > > > +++ b/hw/pci-bridge/xio3130_upstream.c
> > > > > @@ -136,7 +136,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
> > > > > .version_id = 1,
> > > > > .minimum_version_id = 1,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
> > > > > VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
> > > > > vmstate_pcie_aer_log, PCIEAERLog),
> > > > > VMSTATE_END_OF_LIST()
> > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > > index 16df188..8e4226b 100644
> > > > > --- a/hw/pci/pci.c
> > > > > +++ b/hw/pci/pci.c
> > > > > @@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
> > > > > .put = put_pci_irq_state,
> > > > > };
> > > > >
> > > > > +static bool migrate_is_pcie(void *opaque, int version_id)
> > > > > +{
> > > > > + return pci_is_express((PCIDevice *)opaque);
> > > > > +}
> > > > > +
> > > > > +static bool migrate_is_not_pcie(void *opaque, int version_id)
> > > > > +{
> > > > > + return !pci_is_express((PCIDevice *)opaque);
> > > > > +}
> > > > > +
> > > > > const VMStateDescription vmstate_pci_device = {
> > > > > .name = "PCIDevice",
> > > > > .version_id = 2,
> > > > > .minimum_version_id = 1,
> > > > > .fields = (VMStateField[]) {
> > > > > VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > > > > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > > > > - vmstate_info_pci_config,
> > > > > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > > > > + migrate_is_not_pcie,
> > > > > + 0, vmstate_info_pci_config,
> > > > > PCI_CONFIG_SPACE_SIZE),
> > > > > - VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > > > > - vmstate_info_pci_irq_state,
> > > > > - PCI_NUM_PINS * sizeof(int32_t)),
> > > > > - VMSTATE_END_OF_LIST()
> > > > > - }
> > > > > -};
> > > > > -
> > > > > -const VMStateDescription vmstate_pcie_device = {
> > > > > - .name = "PCIEDevice",
> > > > > - .version_id = 2,
> > > > > - .minimum_version_id = 1,
> > > > > - .fields = (VMStateField[]) {
> > > > > - VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > > > > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > > > > - vmstate_info_pci_config,
> > > > > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > > > > + migrate_is_pcie,
> > > > > + 0, vmstate_info_pci_config,
> > > > > PCIE_CONFIG_SPACE_SIZE),
> > > > > VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > > > > vmstate_info_pci_irq_state,
> > > > > @@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
> > > > > }
> > > > > };
> > > > >
> > > > > -static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
> > > > > -{
> > > > > - return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
> > > > > -}
> > > > >
> > > > > void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > > > {
> > > > > @@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > > > * This makes us compatible with old devices
> > > > > * which never set or clear this bit. */
> > > > > s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> > > > > - vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
> > > > > + vmstate_save_state(f, &vmstate_pci_device, s, NULL);
> > > > > /* Restore the interrupt status bit. */
> > > > > pci_update_irq_status(s);
> > > > > }
> > > > > @@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > > > int pci_device_load(PCIDevice *s, QEMUFile *f)
> > > > > {
> > > > > int ret;
> > > > > - ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
> > > > > + ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
> > > > > /* Restore the interrupt status bit. */
> > > > > pci_update_irq_status(s);
> > > > > return ret;
> > > > > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> > > > > index 67fc1e7..adc0c4b 100644
> > > > > --- a/hw/scsi/megasas.c
> > > > > +++ b/hw/scsi/megasas.c
> > > > > @@ -2288,7 +2288,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
> > > > > .minimum_version_id = 0,
> > > > > .minimum_version_id_old = 0,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
> > > > > VMSTATE_MSIX(parent_obj, MegasasState),
> > > > >
> > > > > VMSTATE_INT32(fw_state, MegasasState),
> > > > > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > > > > index a5ce7de..7557546 100644
> > > > > --- a/hw/scsi/vmw_pvscsi.c
> > > > > +++ b/hw/scsi/vmw_pvscsi.c
> > > > > @@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
> > > > > .name = "pvscsi/pcie",
> > > > > .needed = pvscsi_vmstate_need_pcie_device,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
> > > > > VMSTATE_END_OF_LIST()
> > > > > }
> > > > > };
> > > > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > > > > index 4acf0c6..e0b5169 100644
> > > > > --- a/hw/usb/hcd-xhci.c
> > > > > +++ b/hw/usb/hcd-xhci.c
> > > > > @@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
> > > > > .version_id = 1,
> > > > > .post_load = usb_xhci_post_load,
> > > > > .fields = (VMStateField[]) {
> > > > > - VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
> > > > > + VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
> > > > > VMSTATE_MSIX(parent_obj, XHCIState),
> > > > >
> > > > > VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
> > > > > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > > > > index 056d25e..894262e 100644
> > > > > --- a/include/hw/pci/pcie.h
> > > > > +++ b/include/hw/pci/pcie.h
> > > > > @@ -121,16 +121,6 @@ void pcie_add_capability(PCIDevice *dev,
> > > > > void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> > > > > void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> > > > >
> > > > > -extern const VMStateDescription vmstate_pcie_device;
> > > > > -
> > > > > -#define VMSTATE_PCIE_DEVICE(_field, _state) { \
> > > > > - .name = (stringify(_field)), \
> > > > > - .size = sizeof(PCIDevice), \
> > > > > - .vmsd = &vmstate_pcie_device, \
> > > > > - .flags = VMS_STRUCT, \
> > > > > - .offset = vmstate_offset_value(_state, _field, PCIDevice), \
> > > > > -}
> > > > > -
> > > > > void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> > > > > Error **errp);
> > > > > void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> > > > > --
> > > > > 2.9.3
> > > --
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
2016-12-15 23:18 ` Michael S. Tsirkin
2016-12-16 9:35 ` Dr. David Alan Gilbert
@ 2017-01-24 12:10 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-24 12:10 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, marcel, quintela, amit.shah
* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Dec 14, 2016 at 07:58:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > The vmstate_pci_device and vmstate_pcie_devices differ
> > just in the size of one buffer; combine the two using a _TEST
> > macro.
> >
> > I think this is safe as long as everywhere which currently
> > uses either of these two uses the right type.
> >
> > One thing that concerns me is that some places use pci_device_load/save
> > which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
> > macro - how are they getting the same irq mangling?
> >
> > This passes a smoke test migrate of:
> > ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
> > ./littlefed20.img -device e1000e -device virtio-net -device
> > e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
> > ioh3420 -device nec-usb-xhci
> >
> > to an unmodified qemu.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> feel free to merge through migration tree.
Queued.
Dave
>
> > ---
> > hw/net/e1000e.c | 2 +-
> > hw/net/vmxnet3.c | 2 +-
> > hw/pci-bridge/ioh3420.c | 2 +-
> > hw/pci-bridge/xio3130_downstream.c | 2 +-
> > hw/pci-bridge/xio3130_upstream.c | 2 +-
> > hw/pci/pci.c | 41 +++++++++++++++++---------------------
> > hw/scsi/megasas.c | 2 +-
> > hw/scsi/vmw_pvscsi.c | 2 +-
> > hw/usb/hcd-xhci.c | 2 +-
> > include/hw/pci/pcie.h | 10 ----------
> > 10 files changed, 26 insertions(+), 41 deletions(-)
> >
> > diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> > index 4994e1c..463ac9b 100644
> > --- a/hw/net/e1000e.c
> > +++ b/hw/net/e1000e.c
> > @@ -592,7 +592,7 @@ static const VMStateDescription e1000e_vmstate = {
> > .pre_save = e1000e_pre_save,
> > .post_load = e1000e_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
> > + VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
> > VMSTATE_MSIX(parent_obj, E1000EState),
> >
> > VMSTATE_UINT32(ioaddr, E1000EState),
> > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > index bbb898b..86cded9 100644
> > --- a/hw/net/vmxnet3.c
> > +++ b/hw/net/vmxnet3.c
> > @@ -2538,7 +2538,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > .minimum_version_id = 1,
> > .needed = vmxnet3_vmstate_need_pcie_device,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
> > + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > VMSTATE_END_OF_LIST()
> > }
> > };
> > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> > index c8b5ac4..98114e1 100644
> > --- a/hw/pci-bridge/ioh3420.c
> > +++ b/hw/pci-bridge/ioh3420.c
> > @@ -178,7 +178,7 @@ static const VMStateDescription vmstate_ioh3420 = {
> > .minimum_version_id = 1,
> > .post_load = pcie_cap_slot_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > VMSTATE_END_OF_LIST()
> > diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> > index cef6e13..4c54301 100644
> > --- a/hw/pci-bridge/xio3130_downstream.c
> > +++ b/hw/pci-bridge/xio3130_downstream.c
> > @@ -164,7 +164,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
> > .minimum_version_id = 1,
> > .post_load = pcie_cap_slot_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > VMSTATE_END_OF_LIST()
> > diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> > index 4ad0440..45dbd1c 100644
> > --- a/hw/pci-bridge/xio3130_upstream.c
> > +++ b/hw/pci-bridge/xio3130_upstream.c
> > @@ -136,7 +136,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
> > .version_id = 1,
> > .minimum_version_id = 1,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
> > + VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
> > VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
> > vmstate_pcie_aer_log, PCIEAERLog),
> > VMSTATE_END_OF_LIST()
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 16df188..8e4226b 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
> > .put = put_pci_irq_state,
> > };
> >
> > +static bool migrate_is_pcie(void *opaque, int version_id)
> > +{
> > + return pci_is_express((PCIDevice *)opaque);
> > +}
> > +
> > +static bool migrate_is_not_pcie(void *opaque, int version_id)
> > +{
> > + return !pci_is_express((PCIDevice *)opaque);
> > +}
> > +
> > const VMStateDescription vmstate_pci_device = {
> > .name = "PCIDevice",
> > .version_id = 2,
> > .minimum_version_id = 1,
> > .fields = (VMStateField[]) {
> > VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > - vmstate_info_pci_config,
> > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > + migrate_is_not_pcie,
> > + 0, vmstate_info_pci_config,
> > PCI_CONFIG_SPACE_SIZE),
> > - VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > - vmstate_info_pci_irq_state,
> > - PCI_NUM_PINS * sizeof(int32_t)),
> > - VMSTATE_END_OF_LIST()
> > - }
> > -};
> > -
> > -const VMStateDescription vmstate_pcie_device = {
> > - .name = "PCIEDevice",
> > - .version_id = 2,
> > - .minimum_version_id = 1,
> > - .fields = (VMStateField[]) {
> > - VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
> > - VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
> > - vmstate_info_pci_config,
> > + VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
> > + migrate_is_pcie,
> > + 0, vmstate_info_pci_config,
> > PCIE_CONFIG_SPACE_SIZE),
> > VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
> > vmstate_info_pci_irq_state,
> > @@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
> > }
> > };
> >
> > -static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
> > -{
> > - return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
> > -}
> >
> > void pci_device_save(PCIDevice *s, QEMUFile *f)
> > {
> > @@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > * This makes us compatible with old devices
> > * which never set or clear this bit. */
> > s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> > - vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
> > + vmstate_save_state(f, &vmstate_pci_device, s, NULL);
> > /* Restore the interrupt status bit. */
> > pci_update_irq_status(s);
> > }
> > @@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > int pci_device_load(PCIDevice *s, QEMUFile *f)
> > {
> > int ret;
> > - ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
> > + ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
> > /* Restore the interrupt status bit. */
> > pci_update_irq_status(s);
> > return ret;
> > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> > index 67fc1e7..adc0c4b 100644
> > --- a/hw/scsi/megasas.c
> > +++ b/hw/scsi/megasas.c
> > @@ -2288,7 +2288,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
> > .minimum_version_id = 0,
> > .minimum_version_id_old = 0,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> > + VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
> > VMSTATE_MSIX(parent_obj, MegasasState),
> >
> > VMSTATE_INT32(fw_state, MegasasState),
> > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > index a5ce7de..7557546 100644
> > --- a/hw/scsi/vmw_pvscsi.c
> > +++ b/hw/scsi/vmw_pvscsi.c
> > @@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
> > .name = "pvscsi/pcie",
> > .needed = pvscsi_vmstate_need_pcie_device,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
> > + VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
> > VMSTATE_END_OF_LIST()
> > }
> > };
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index 4acf0c6..e0b5169 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
> > .version_id = 1,
> > .post_load = usb_xhci_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
> > + VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
> > VMSTATE_MSIX(parent_obj, XHCIState),
> >
> > VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
> > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> > index 056d25e..894262e 100644
> > --- a/include/hw/pci/pcie.h
> > +++ b/include/hw/pci/pcie.h
> > @@ -121,16 +121,6 @@ void pcie_add_capability(PCIDevice *dev,
> > void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
> > void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
> >
> > -extern const VMStateDescription vmstate_pcie_device;
> > -
> > -#define VMSTATE_PCIE_DEVICE(_field, _state) { \
> > - .name = (stringify(_field)), \
> > - .size = sizeof(PCIDevice), \
> > - .vmsd = &vmstate_pcie_device, \
> > - .flags = VMS_STRUCT, \
> > - .offset = vmstate_offset_value(_state, _field, PCIDevice), \
> > -}
> > -
> > void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> > Error **errp);
> > void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> > --
> > 2.9.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-24 12:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-14 19:58 [Qemu-devel] [RFC] PCI/migration merge vmstate_pci_device and vmstate_pcie_device Dr. David Alan Gilbert (git)
2016-12-15 23:18 ` Michael S. Tsirkin
2016-12-16 9:35 ` Dr. David Alan Gilbert
2017-01-10 3:38 ` Michael S. Tsirkin
2017-01-12 17:27 ` Dr. David Alan Gilbert
2017-01-12 20:13 ` Michael S. Tsirkin
2017-01-24 12:10 ` Dr. David Alan Gilbert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).