Re: [Qemu-devel] [PATCH v16 10/14] pci: introduce pci bus pre reset

[Alex Williamson]

On Tue, 2016-01-12 at 10:43 +0800, Cao jin wrote:
> From: Chen Fan <address@hidden>
> 
> 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 <address@hidden>
> ---
>  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;
>  };
>