Message ID | 1372863856-19860-1-git-send-email-dkoch@terremark.com |
---|---|
State | New |
Headers | show |
Am 03.07.2013 17:04, schrieb Don Koch: > From: Don Koch <dkoch@verizon.com> > > Update mappings for PCI bridge after live migration. > > Signed-off-by: Don Koch <dkoch@verizon.com> > --- > This fixes bug 1187529: devices on a PCI bridge stop working after migration. Feel free to reference this as LP#1187529 in the commit message if this is from QEMU's Launchpad bug tracker. > > hw/pci-bridge/pci_bridge_dev.c | 9 +++++++++ > hw/pci/pci_bridge.c | 2 +- > include/hw/pci/pci_bridge.h | 1 + > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > index 971b432..9e5062e 100644 > --- a/hw/pci-bridge/pci_bridge_dev.c > +++ b/hw/pci-bridge/pci_bridge_dev.c > @@ -110,6 +110,14 @@ static void qdev_pci_bridge_dev_reset(DeviceState *qdev) > shpc_reset(dev); > } > > +static int pci_bridge_dev_post_load(void *opaque, int ver) { > + PCIDevice *d = opaque; > + PCIBridge *s = container_of(d, PCIBridge, dev); Casting should no longer be done that way. It seems there is no PCI_BRIDGE() cast macro in upstream yet, but it doesn't seem necessary either. You can just use: PCIBridge *s = opaque; for simplicity. Functional change looks sensible to me. Regards, Andreas > + > + pci_bridge_update_mappings(s); > + return 0; > +} > + > static Property pci_bridge_dev_properties[] = { > /* Note: 0 is not a legal chassis number. */ > DEFINE_PROP_UINT8("chassis_nr", PCIBridgeDev, chassis_nr, 0), > @@ -119,6 +127,7 @@ static Property pci_bridge_dev_properties[] = { > > static const VMStateDescription pci_bridge_dev_vmstate = { > .name = "pci_bridge", > + .post_load = pci_bridge_dev_post_load, > .fields = (VMStateField[]) { > VMSTATE_PCI_DEVICE(bridge.dev, PCIBridgeDev), > SHPC_VMSTATE(bridge.dev.shpc, PCIBridgeDev), > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > index 24be6c5..3897bd8 100644 > --- a/hw/pci/pci_bridge.c > +++ b/hw/pci/pci_bridge.c > @@ -224,7 +224,7 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w) > g_free(w); > } > > -static void pci_bridge_update_mappings(PCIBridge *br) > +void pci_bridge_update_mappings(PCIBridge *br) > { > PCIBridgeWindows *w = br->windows; > > diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h > index 1868f7a..1d8f997 100644 > --- a/include/hw/pci/pci_bridge.h > +++ b/include/hw/pci/pci_bridge.h > @@ -37,6 +37,7 @@ PCIBus *pci_bridge_get_sec_bus(PCIBridge *br); > pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type); > pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type); > > +void pci_bridge_update_mappings(PCIBridge *br); > void pci_bridge_write_config(PCIDevice *d, > uint32_t address, uint32_t val, int len); > void pci_bridge_disable_base_limit(PCIDevice *dev); >
On Wed, Jul 03, 2013 at 11:04:16AM -0400, Don Koch wrote: > From: Don Koch <dkoch@verizon.com> > > Update mappings for PCI bridge after live migration. > > Signed-off-by: Don Koch <dkoch@verizon.com> > --- > This fixes bug 1187529: devices on a PCI bridge stop working after migration. Thanks, this looks good, but any bridge device would need this fix, won't it? Could we call this from get_pci_config_device instead? This way all bridge devices would be fixed. > hw/pci-bridge/pci_bridge_dev.c | 9 +++++++++ > hw/pci/pci_bridge.c | 2 +- > include/hw/pci/pci_bridge.h | 1 + > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c > index 971b432..9e5062e 100644 > --- a/hw/pci-bridge/pci_bridge_dev.c > +++ b/hw/pci-bridge/pci_bridge_dev.c > @@ -110,6 +110,14 @@ static void qdev_pci_bridge_dev_reset(DeviceState *qdev) > shpc_reset(dev); > } > > +static int pci_bridge_dev_post_load(void *opaque, int ver) { > + PCIDevice *d = opaque; > + PCIBridge *s = container_of(d, PCIBridge, dev); > + > + pci_bridge_update_mappings(s); > + return 0; > +} > + > static Property pci_bridge_dev_properties[] = { > /* Note: 0 is not a legal chassis number. */ > DEFINE_PROP_UINT8("chassis_nr", PCIBridgeDev, chassis_nr, 0), > @@ -119,6 +127,7 @@ static Property pci_bridge_dev_properties[] = { > > static const VMStateDescription pci_bridge_dev_vmstate = { > .name = "pci_bridge", > + .post_load = pci_bridge_dev_post_load, > .fields = (VMStateField[]) { > VMSTATE_PCI_DEVICE(bridge.dev, PCIBridgeDev), > SHPC_VMSTATE(bridge.dev.shpc, PCIBridgeDev), > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > index 24be6c5..3897bd8 100644 > --- a/hw/pci/pci_bridge.c > +++ b/hw/pci/pci_bridge.c > @@ -224,7 +224,7 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w) > g_free(w); > } > > -static void pci_bridge_update_mappings(PCIBridge *br) > +void pci_bridge_update_mappings(PCIBridge *br) > { > PCIBridgeWindows *w = br->windows; > > diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h > index 1868f7a..1d8f997 100644 > --- a/include/hw/pci/pci_bridge.h > +++ b/include/hw/pci/pci_bridge.h > @@ -37,6 +37,7 @@ PCIBus *pci_bridge_get_sec_bus(PCIBridge *br); > pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type); > pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type); > > +void pci_bridge_update_mappings(PCIBridge *br); > void pci_bridge_write_config(PCIDevice *d, > uint32_t address, uint32_t val, int len); > void pci_bridge_disable_base_limit(PCIDevice *dev); > -- > 1.7.11.7
On 07/03/2013 12:15 PM, Andreas Färber wrote: > Am 03.07.2013 17:04, schrieb Don Koch: >> From: Don Koch <dkoch@verizon.com> >> >> Update mappings for PCI bridge after live migration. >> >> Signed-off-by: Don Koch <dkoch@verizon.com> >> --- >> This fixes bug 1187529: devices on a PCI bridge stop working after migration. > > Feel free to reference this as LP#1187529 in the commit message if this > is from QEMU's Launchpad bug tracker. OK, will do. (Wasn't aware of this convention.) >> >> hw/pci-bridge/pci_bridge_dev.c | 9 +++++++++ >> hw/pci/pci_bridge.c | 2 +- >> include/hw/pci/pci_bridge.h | 1 + >> 3 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c >> index 971b432..9e5062e 100644 >> --- a/hw/pci-bridge/pci_bridge_dev.c >> +++ b/hw/pci-bridge/pci_bridge_dev.c >> @@ -110,6 +110,14 @@ static void qdev_pci_bridge_dev_reset(DeviceState *qdev) >> shpc_reset(dev); >> } >> >> +static int pci_bridge_dev_post_load(void *opaque, int ver) { >> + PCIDevice *d = opaque; >> + PCIBridge *s = container_of(d, PCIBridge, dev); > > Casting should no longer be done that way. It seems there is no > PCI_BRIDGE() cast macro in upstream yet, but it doesn't seem necessary > either. You can just use: > > PCIBridge *s = opaque; > > for simplicity. Might I suggest submitting the patch as is (possibly modulo Michael Tsirkin's suggestions) and later submitting a PCI_BRIDGE macro patch to "standardize" the casting? > > Functional change looks sensible to me. > > Regards, > Andreas > >> + >> + pci_bridge_update_mappings(s); >> + return 0; >> +} >> + >> static Property pci_bridge_dev_properties[] = { >> /* Note: 0 is not a legal chassis number. */ >> DEFINE_PROP_UINT8("chassis_nr", PCIBridgeDev, chassis_nr, 0), >> @@ -119,6 +127,7 @@ static Property pci_bridge_dev_properties[] = { >> >> static const VMStateDescription pci_bridge_dev_vmstate = { >> .name = "pci_bridge", >> + .post_load = pci_bridge_dev_post_load, >> .fields = (VMStateField[]) { >> VMSTATE_PCI_DEVICE(bridge.dev, PCIBridgeDev), >> SHPC_VMSTATE(bridge.dev.shpc, PCIBridgeDev), >> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c >> index 24be6c5..3897bd8 100644 >> --- a/hw/pci/pci_bridge.c >> +++ b/hw/pci/pci_bridge.c >> @@ -224,7 +224,7 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w) >> g_free(w); >> } >> >> -static void pci_bridge_update_mappings(PCIBridge *br) >> +void pci_bridge_update_mappings(PCIBridge *br) >> { >> PCIBridgeWindows *w = br->windows; >> >> diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h >> index 1868f7a..1d8f997 100644 >> --- a/include/hw/pci/pci_bridge.h >> +++ b/include/hw/pci/pci_bridge.h >> @@ -37,6 +37,7 @@ PCIBus *pci_bridge_get_sec_bus(PCIBridge *br); >> pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type); >> pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type); >> >> +void pci_bridge_update_mappings(PCIBridge *br); >> void pci_bridge_write_config(PCIDevice *d, >> uint32_t address, uint32_t val, int len); >> void pci_bridge_disable_base_limit(PCIDevice *dev); >> > >
On 07/04/2013 04:57 AM, Michael S. Tsirkin wrote: > On Wed, Jul 03, 2013 at 11:04:16AM -0400, Don Koch wrote: >> From: Don Koch <dkoch@verizon.com> >> >> Update mappings for PCI bridge after live migration. >> >> Signed-off-by: Don Koch <dkoch@verizon.com> >> --- >> This fixes bug 1187529: devices on a PCI bridge stop working after migration. > Thanks, this looks good, but any bridge device would > need this fix, won't it? One would think so. > Could we call this from get_pci_config_device instead? Seems a bit odd, but I can't find a better spot (there is no pci_bridge equivalent). I'll look into a patch for that (but can't guarantee total testing). > This way all bridge devices would be fixed. Well, all that support migration (e.g., not the i82801b11). ;) > >> hw/pci-bridge/pci_bridge_dev.c | 9 +++++++++ >> hw/pci/pci_bridge.c | 2 +- >> include/hw/pci/pci_bridge.h | 1 + >> 3 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c >> index 971b432..9e5062e 100644 >> --- a/hw/pci-bridge/pci_bridge_dev.c >> +++ b/hw/pci-bridge/pci_bridge_dev.c >> @@ -110,6 +110,14 @@ static void qdev_pci_bridge_dev_reset(DeviceState *qdev) >> shpc_reset(dev); >> } >> >> +static int pci_bridge_dev_post_load(void *opaque, int ver) { >> + PCIDevice *d = opaque; >> + PCIBridge *s = container_of(d, PCIBridge, dev); >> + >> + pci_bridge_update_mappings(s); >> + return 0; >> +} >> + >> static Property pci_bridge_dev_properties[] = { >> /* Note: 0 is not a legal chassis number. */ >> DEFINE_PROP_UINT8("chassis_nr", PCIBridgeDev, chassis_nr, 0), >> @@ -119,6 +127,7 @@ static Property pci_bridge_dev_properties[] = { >> >> static const VMStateDescription pci_bridge_dev_vmstate = { >> .name = "pci_bridge", >> + .post_load = pci_bridge_dev_post_load, >> .fields = (VMStateField[]) { >> VMSTATE_PCI_DEVICE(bridge.dev, PCIBridgeDev), >> SHPC_VMSTATE(bridge.dev.shpc, PCIBridgeDev), >> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c >> index 24be6c5..3897bd8 100644 >> --- a/hw/pci/pci_bridge.c >> +++ b/hw/pci/pci_bridge.c >> @@ -224,7 +224,7 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w) >> g_free(w); >> } >> >> -static void pci_bridge_update_mappings(PCIBridge *br) >> +void pci_bridge_update_mappings(PCIBridge *br) >> { >> PCIBridgeWindows *w = br->windows; >> >> diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h >> index 1868f7a..1d8f997 100644 >> --- a/include/hw/pci/pci_bridge.h >> +++ b/include/hw/pci/pci_bridge.h >> @@ -37,6 +37,7 @@ PCIBus *pci_bridge_get_sec_bus(PCIBridge *br); >> pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type); >> pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type); >> >> +void pci_bridge_update_mappings(PCIBridge *br); >> void pci_bridge_write_config(PCIDevice *d, >> uint32_t address, uint32_t val, int len); >> void pci_bridge_disable_base_limit(PCIDevice *dev); >> -- >> 1.7.11.7
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c index 971b432..9e5062e 100644 --- a/hw/pci-bridge/pci_bridge_dev.c +++ b/hw/pci-bridge/pci_bridge_dev.c @@ -110,6 +110,14 @@ static void qdev_pci_bridge_dev_reset(DeviceState *qdev) shpc_reset(dev); } +static int pci_bridge_dev_post_load(void *opaque, int ver) { + PCIDevice *d = opaque; + PCIBridge *s = container_of(d, PCIBridge, dev); + + pci_bridge_update_mappings(s); + return 0; +} + static Property pci_bridge_dev_properties[] = { /* Note: 0 is not a legal chassis number. */ DEFINE_PROP_UINT8("chassis_nr", PCIBridgeDev, chassis_nr, 0), @@ -119,6 +127,7 @@ static Property pci_bridge_dev_properties[] = { static const VMStateDescription pci_bridge_dev_vmstate = { .name = "pci_bridge", + .post_load = pci_bridge_dev_post_load, .fields = (VMStateField[]) { VMSTATE_PCI_DEVICE(bridge.dev, PCIBridgeDev), SHPC_VMSTATE(bridge.dev.shpc, PCIBridgeDev), diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 24be6c5..3897bd8 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -224,7 +224,7 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w) g_free(w); } -static void pci_bridge_update_mappings(PCIBridge *br) +void pci_bridge_update_mappings(PCIBridge *br) { PCIBridgeWindows *w = br->windows; diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h index 1868f7a..1d8f997 100644 --- a/include/hw/pci/pci_bridge.h +++ b/include/hw/pci/pci_bridge.h @@ -37,6 +37,7 @@ PCIBus *pci_bridge_get_sec_bus(PCIBridge *br); pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type); pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type); +void pci_bridge_update_mappings(PCIBridge *br); void pci_bridge_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len); void pci_bridge_disable_base_limit(PCIDevice *dev);