Patchwork pci: disable migration of p2p bridge

login
register
mail settings
Submitter Isaku Yamahata
Date Dec. 22, 2010, 3:13 a.m.
Message ID <ad4668c6482905c5c0f3c73bfbd9d58d449be04b.1292987590.git.yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/76355/
State New
Headers show

Comments

Isaku Yamahata - Dec. 22, 2010, 3:13 a.m.
Right now pcibus_get_dev_path() isn't migration save because
bus number/secondary bus number are set by guest OS.
So it can't be used reliably for qemu internal id.

For 0.14 release, disable p2p bridge migration at the moment.
Once pcibus_get_dev_path() is fixed, this patch should be reverted.
It will be addressed for 0.15 release.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/apb_pci.c            |    9 +++++++++
 hw/dec_pci.c            |    6 ++++++
 hw/ioh3420.c            |    4 ++++
 hw/xio3130_downstream.c |    4 ++++
 hw/xio3130_upstream.c   |    4 ++++
 5 files changed, 27 insertions(+), 0 deletions(-)
Michael S. Tsirkin - Dec. 22, 2010, 6:27 a.m.
On Wed, Dec 22, 2010 at 12:13:43PM +0900, Isaku Yamahata wrote:
> Right now pcibus_get_dev_path() isn't migration save because
> bus number/secondary bus number are set by guest OS.
> So it can't be used reliably for qemu internal id.
> 
> For 0.14 release, disable p2p bridge migration at the moment.
> Once pcibus_get_dev_path() is fixed, this patch should be reverted.
> It will be addressed for 0.15 release.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>


Hmm, haven't looked into this deeply - can we do this in one place
when the bridge is created?

> ---
>  hw/apb_pci.c            |    9 +++++++++
>  hw/dec_pci.c            |    6 ++++++
>  hw/ioh3420.c            |    4 ++++
>  hw/xio3130_downstream.c |    4 ++++
>  hw/xio3130_upstream.c   |    4 ++++
>  5 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index 84e9af7..c456d8d 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -368,6 +368,11 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
>      qdev_init_nofail(&pci_dev->qdev);
>      *bus2 = pci_bridge_get_sec_bus(br);
>  
> +    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
> +       remove this once p2p bridge migration is supported */
> +    register_device_unmigratable(&pci_dev->qdev, "pbm-bridge", &pci_dev->qdev);
> +
> +
>      pci_dev = pci_create_multifunction(d->bus, PCI_DEVFN(1, 1), true,
>                                     "pbm-bridge");
>      br = DO_UPCAST(PCIBridge, dev, pci_dev);
> @@ -376,6 +381,10 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
>      qdev_init_nofail(&pci_dev->qdev);
>      *bus3 = pci_bridge_get_sec_bus(br);
>  
> +    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
> +       remove this once p2p bridge migration is supported */
> +    register_device_unmigratable(&pci_dev->qdev, "pbm-bridge", &pci_dev->qdev);
> +
>      return d->bus;
>  }
>  
> diff --git a/hw/dec_pci.c b/hw/dec_pci.c
> index bf88f2a..3710984 100644
> --- a/hw/dec_pci.c
> +++ b/hw/dec_pci.c
> @@ -86,6 +86,12 @@ PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
>      br = DO_UPCAST(PCIBridge, dev, dev);
>      pci_bridge_map_irq(br, "DEC 21154 PCI-PCI bridge", dec_map_irq);
>      qdev_init_nofail(&dev->qdev);
> +
> +    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
> +       remove this once p2p bridge migration is supported */
> +    register_device_unmigratable(&dev->qdev, "dec-21154-p2p-bridge",
> +                                 &dev->qdev);
> +
>      return pci_bridge_get_sec_bus(br);
>  }
>  
> diff --git a/hw/ioh3420.c b/hw/ioh3420.c
> index 95adf09..b1a5c96 100644
> --- a/hw/ioh3420.c
> +++ b/hw/ioh3420.c
> @@ -188,6 +188,10 @@ PCIESlot *ioh3420_init(PCIBus *bus, int devfn, bool multifunction,
>      qdev_prop_set_uint16(qdev, "slot", slot);
>      qdev_init_nofail(qdev);
>  
> +    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
> +       remove this once p2p bridge migration is supported */
> +    register_device_unmigratable(qdev, "ioh3420", qdev);
> +
>      return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
>  }
>  
> diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
> index 1a2d258..83394ab 100644
> --- a/hw/xio3130_downstream.c
> +++ b/hw/xio3130_downstream.c
> @@ -153,6 +153,10 @@ PCIESlot *xio3130_downstream_init(PCIBus *bus, int devfn, bool multifunction,
>      qdev_prop_set_uint16(qdev, "slot", slot);
>      qdev_init_nofail(qdev);
>  
> +    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
> +       remove this once p2p bridge migration is supported */
> +    register_device_unmigratable(qdev, "x3130-downstream", qdev);
> +
>      return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
>  }
>  
> diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
> index 387bf6c..cbd126a 100644
> --- a/hw/xio3130_upstream.c
> +++ b/hw/xio3130_upstream.c
> @@ -134,6 +134,10 @@ PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
>      qdev_prop_set_uint8(qdev, "port", port);
>      qdev_init_nofail(qdev);
>  
> +    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
> +       remove this once p2p bridge migration is supported */
> +    register_device_unmigratable(qdev, "x3130-upstream", qdev);
> +
>      return DO_UPCAST(PCIEPort, br, br);
>  }
>  
> -- 
> 1.7.1.1
Isaku Yamahata - Dec. 22, 2010, 8 a.m.
On Wed, Dec 22, 2010 at 08:27:17AM +0200, Michael S. Tsirkin wrote:
> On Wed, Dec 22, 2010 at 12:13:43PM +0900, Isaku Yamahata wrote:
> > Right now pcibus_get_dev_path() isn't migration save because
> > bus number/secondary bus number are set by guest OS.
> > So it can't be used reliably for qemu internal id.
> > 
> > For 0.14 release, disable p2p bridge migration at the moment.
> > Once pcibus_get_dev_path() is fixed, this patch should be reverted.
> > It will be addressed for 0.15 release.
> > 
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Blue Swirl <blauwirbel@gmail.com>
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> 
> 
> Hmm, haven't looked into this deeply - can we do this in one place
> when the bridge is created?

Unfortunately it's not easy. It requires revising
register_device_unmigratable(). I have to admit this patch is ugly. 

This patch is temporal work around and should be reverted eventually.
So I think it is better to address the original issue (allowing migration
of p2p bridge) instead of addressing register_device_unmigratable().

thanks

Patch

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 84e9af7..c456d8d 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -368,6 +368,11 @@  PCIBus *pci_apb_init(target_phys_addr_t special_base,
     qdev_init_nofail(&pci_dev->qdev);
     *bus2 = pci_bridge_get_sec_bus(br);
 
+    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
+       remove this once p2p bridge migration is supported */
+    register_device_unmigratable(&pci_dev->qdev, "pbm-bridge", &pci_dev->qdev);
+
+
     pci_dev = pci_create_multifunction(d->bus, PCI_DEVFN(1, 1), true,
                                    "pbm-bridge");
     br = DO_UPCAST(PCIBridge, dev, pci_dev);
@@ -376,6 +381,10 @@  PCIBus *pci_apb_init(target_phys_addr_t special_base,
     qdev_init_nofail(&pci_dev->qdev);
     *bus3 = pci_bridge_get_sec_bus(br);
 
+    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
+       remove this once p2p bridge migration is supported */
+    register_device_unmigratable(&pci_dev->qdev, "pbm-bridge", &pci_dev->qdev);
+
     return d->bus;
 }
 
diff --git a/hw/dec_pci.c b/hw/dec_pci.c
index bf88f2a..3710984 100644
--- a/hw/dec_pci.c
+++ b/hw/dec_pci.c
@@ -86,6 +86,12 @@  PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn)
     br = DO_UPCAST(PCIBridge, dev, dev);
     pci_bridge_map_irq(br, "DEC 21154 PCI-PCI bridge", dec_map_irq);
     qdev_init_nofail(&dev->qdev);
+
+    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
+       remove this once p2p bridge migration is supported */
+    register_device_unmigratable(&dev->qdev, "dec-21154-p2p-bridge",
+                                 &dev->qdev);
+
     return pci_bridge_get_sec_bus(br);
 }
 
diff --git a/hw/ioh3420.c b/hw/ioh3420.c
index 95adf09..b1a5c96 100644
--- a/hw/ioh3420.c
+++ b/hw/ioh3420.c
@@ -188,6 +188,10 @@  PCIESlot *ioh3420_init(PCIBus *bus, int devfn, bool multifunction,
     qdev_prop_set_uint16(qdev, "slot", slot);
     qdev_init_nofail(qdev);
 
+    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
+       remove this once p2p bridge migration is supported */
+    register_device_unmigratable(qdev, "ioh3420", qdev);
+
     return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
 }
 
diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
index 1a2d258..83394ab 100644
--- a/hw/xio3130_downstream.c
+++ b/hw/xio3130_downstream.c
@@ -153,6 +153,10 @@  PCIESlot *xio3130_downstream_init(PCIBus *bus, int devfn, bool multifunction,
     qdev_prop_set_uint16(qdev, "slot", slot);
     qdev_init_nofail(qdev);
 
+    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
+       remove this once p2p bridge migration is supported */
+    register_device_unmigratable(qdev, "x3130-downstream", qdev);
+
     return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
 }
 
diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
index 387bf6c..cbd126a 100644
--- a/hw/xio3130_upstream.c
+++ b/hw/xio3130_upstream.c
@@ -134,6 +134,10 @@  PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
     qdev_prop_set_uint8(qdev, "port", port);
     qdev_init_nofail(qdev);
 
+    /* TODO: p2p bridge migration. pcibus_get_dev_path() isn't migration-safe.
+       remove this once p2p bridge migration is supported */
+    register_device_unmigratable(qdev, "x3130-upstream", qdev);
+
     return DO_UPCAST(PCIEPort, br, br);
 }