| Message ID | abef1937a0761cc91c18949b3657f2fe40aaa803.1452564770.git.chen.fan.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 2016-01-12 at 10:43 +0800, Cao jin wrote: > From: Chen Fan <chen.fan.fnst@cn.fujitsu.com> > > avoid repeat bus reset, here introduce a sequence ID for each time > bus hot reset, so each vfio device could know whether they've already > been reset for that sequence ID. > > Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> > --- > hw/pci/pci.c | 13 +++++++++++++ > hw/pci/pci_bridge.c | 3 +++ > include/hw/pci/pci.h | 1 + > include/hw/pci/pci_bus.h | 3 +++ > 4 files changed, 20 insertions(+) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index f6ca6ef..ceb72d5 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -91,6 +91,18 @@ static void pci_bus_unrealize(BusState *qbus, > Error **errp) > vmstate_unregister(NULL, &vmstate_pcibus, bus); > } > > +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid) > +{ > + PCIBus *sec; > + > + bus->in_reset = true; > + bus->reset_seqid = seqid; > + > + QLIST_FOREACH(sec, &bus->child, sibling) { > + pci_bus_pre_reset(sec, seqid); > + } > +} > + > static bool pcibus_is_root(PCIBus *bus) > { > return !bus->parent_dev; > @@ -276,6 +288,7 @@ static void pcibus_reset(BusState *qbus) > for (i = 0; i < bus->nirq; i++) { > assert(bus->irq_count[i] == 0); > } > + bus->in_reset = false; > } > > static void pci_host_bus_register(PCIBus *bus, DeviceState *parent) > diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c > index 40c97b1..c7f15a1 100644 > --- a/hw/pci/pci_bridge.c > +++ b/hw/pci/pci_bridge.c > @@ -268,6 +268,9 @@ void pci_bridge_write_config(PCIDevice *d, > newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL); > if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) { > /* Trigger hot reset on 0->1 transition. */ > + uint32_t seqid = s->sec_bus.reset_seqid++; Doesn't this need to come from a global sequence ID? Imagine the case of a nested bus, the leaf bus is reset incrementing the sequence ID. The devices on that bus store that sequence ID as they're reset. The parent bus is then reset, but all the devices on the leaf bus have already been reset for that sequence ID and ignore the reset. > + > + pci_bus_pre_reset(&s->sec_bus, seqid ? seqid : 1); Does this work? Seems like this would make devices ignore the second bus reset after the VM is instantiated. ie. the first bus reset seqid is 0, so we call pre_reset with 1, the second time we call it with 1 again. > qbus_reset_all(&s->sec_bus.qbus); I'd be tempted to call qbus_walk_children() directly, it already has a pre_busfn callback hook. > } > } > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index 379b6e1..b811279 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -381,6 +381,7 @@ void pci_bus_fire_intx_routing_notifier(PCIBus > *bus); > void pci_device_set_intx_routing_notifier(PCIDevice *dev, > PCIINTxRoutingNotifier > notifier); > void pci_device_reset(PCIDevice *dev); > +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid); > > PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, > const char *default_model, > diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h > index 7812fa9..dd6aaf1 100644 > --- a/include/hw/pci/pci_bus.h > +++ b/include/hw/pci/pci_bus.h > @@ -40,6 +40,9 @@ struct PCIBus { > int nirq; > int *irq_count; > > + bool in_reset; > + uint32_t reset_seqid; > + > NotifierWithReturnList hotplug_notifiers; > }; >
On 01/15/2016 04:36 AM, Alex Williamson wrote: > On Tue, 2016-01-12 at 10:43 +0800, Cao jin wrote: >> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com> >> >> avoid repeat bus reset, here introduce a sequence ID for each time >> bus hot reset, so each vfio device could know whether they've already >> been reset for that sequence ID. >> >> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> >> --- >> hw/pci/pci.c | 13 +++++++++++++ >> hw/pci/pci_bridge.c | 3 +++ >> include/hw/pci/pci.h | 1 + >> include/hw/pci/pci_bus.h | 3 +++ >> 4 files changed, 20 insertions(+) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index f6ca6ef..ceb72d5 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -91,6 +91,18 @@ static void pci_bus_unrealize(BusState *qbus, >> Error **errp) >> vmstate_unregister(NULL, &vmstate_pcibus, bus); >> } >> >> +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid) >> +{ >> + PCIBus *sec; >> + >> + bus->in_reset = true; >> + bus->reset_seqid = seqid; >> + >> + QLIST_FOREACH(sec, &bus->child, sibling) { >> + pci_bus_pre_reset(sec, seqid); >> + } >> +} >> + >> static bool pcibus_is_root(PCIBus *bus) >> { >> return !bus->parent_dev; >> @@ -276,6 +288,7 @@ static void pcibus_reset(BusState *qbus) >> for (i = 0; i < bus->nirq; i++) { >> assert(bus->irq_count[i] == 0); >> } >> + bus->in_reset = false; >> } >> >> static void pci_host_bus_register(PCIBus *bus, DeviceState *parent) >> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c >> index 40c97b1..c7f15a1 100644 >> --- a/hw/pci/pci_bridge.c >> +++ b/hw/pci/pci_bridge.c >> @@ -268,6 +268,9 @@ void pci_bridge_write_config(PCIDevice *d, >> newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL); >> if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) { >> /* Trigger hot reset on 0->1 transition. */ >> + uint32_t seqid = s->sec_bus.reset_seqid++; > Doesn't this need to come from a global sequence ID? Imagine the case > of a nested bus, the leaf bus is reset incrementing the sequence ID. > The devices on that bus store that sequence ID as they're reset. The > parent bus is then reset, but all the devices on the leaf bus have > already been reset for that sequence ID and ignore the reset. > >> + >> + pci_bus_pre_reset(&s->sec_bus, seqid ? seqid : 1); > Does this work? Seems like this would make devices ignore the second > bus reset after the VM is instantiated. ie. the first bus reset seqid > is 0, so we call pre_reset with 1, the second time we call it with 1 > again. > >> qbus_reset_all(&s->sec_bus.qbus); > I'd be tempted to call qbus_walk_children() directly, it already has a > pre_busfn callback hook. Hi Alex, this looks like need to change much pci core code, as Michael suggested in 00/14, maybe we should simply the aer implementation. what do you think of that? Thanks, Chen > >> } >> } >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index 379b6e1..b811279 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -381,6 +381,7 @@ void pci_bus_fire_intx_routing_notifier(PCIBus >> *bus); >> void pci_device_set_intx_routing_notifier(PCIDevice *dev, >> PCIINTxRoutingNotifier >> notifier); >> void pci_device_reset(PCIDevice *dev); >> +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid); >> >> PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, >> const char *default_model, >> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h >> index 7812fa9..dd6aaf1 100644 >> --- a/include/hw/pci/pci_bus.h >> +++ b/include/hw/pci/pci_bus.h >> @@ -40,6 +40,9 @@ struct PCIBus { >> int nirq; >> int *irq_count; >> >> + bool in_reset; >> + uint32_t reset_seqid; >> + >> NotifierWithReturnList hotplug_notifiers; >> }; >> > > > . >
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index f6ca6ef..ceb72d5 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -91,6 +91,18 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp) vmstate_unregister(NULL, &vmstate_pcibus, bus); } +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid) +{ + PCIBus *sec; + + bus->in_reset = true; + bus->reset_seqid = seqid; + + QLIST_FOREACH(sec, &bus->child, sibling) { + pci_bus_pre_reset(sec, seqid); + } +} + static bool pcibus_is_root(PCIBus *bus) { return !bus->parent_dev; @@ -276,6 +288,7 @@ static void pcibus_reset(BusState *qbus) for (i = 0; i < bus->nirq; i++) { assert(bus->irq_count[i] == 0); } + bus->in_reset = false; } static void pci_host_bus_register(PCIBus *bus, DeviceState *parent) diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 40c97b1..c7f15a1 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -268,6 +268,9 @@ void pci_bridge_write_config(PCIDevice *d, newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL); if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) { /* Trigger hot reset on 0->1 transition. */ + uint32_t seqid = s->sec_bus.reset_seqid++; + + pci_bus_pre_reset(&s->sec_bus, seqid ? seqid : 1); qbus_reset_all(&s->sec_bus.qbus); } } diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 379b6e1..b811279 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -381,6 +381,7 @@ void pci_bus_fire_intx_routing_notifier(PCIBus *bus); void pci_device_set_intx_routing_notifier(PCIDevice *dev, PCIINTxRoutingNotifier notifier); void pci_device_reset(PCIDevice *dev); +void pci_bus_pre_reset(PCIBus *bus, uint32_t seqid); PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, const char *default_model, diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h index 7812fa9..dd6aaf1 100644 --- a/include/hw/pci/pci_bus.h +++ b/include/hw/pci/pci_bus.h @@ -40,6 +40,9 @@ struct PCIBus { int nirq; int *irq_count; + bool in_reset; + uint32_t reset_seqid; + NotifierWithReturnList hotplug_notifiers; };