Re: [Qemu-devel] [RFC v7 03/11] qdev: add bus reset_notifiers callbacks for host bus reset

[Alex Williamson]

On Tue, 2015-05-19 at 12:42 +0800, Chen Fan wrote:
> Particularly, For vfio device, Once need to recovery devices
> by bus reset such as AER, we always need to reset the host bus
> to recovery the devices under the bus, so we need to add pci bus
> callbacks to specify to do host bus reset.
> 
> Signed-off-by: Chen Fan <address@hidden>
> ---
>  hw/pci/pci.c                  |  6 ++++++
>  hw/pci/pci_bridge.c           |  3 +++
>  hw/vfio/pci.c                 | 12 ++++++++++++
>  include/hw/pci/pci.h          |  2 ++
>  include/hw/pci/pci_bus.h      |  2 ++
>  include/hw/vfio/vfio-common.h |  1 +
>  6 files changed, 26 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 48f19a3..cac7769 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -74,11 +74,17 @@ static const VMStateDescription vmstate_pcibus = {
>      }
>  };
>  
> +void pci_bus_add_reset_notifier(PCIBus *bus, Notifier *notify)
> +{
> +    notifier_list_add(&bus->reset_notifiers, notify);
> +}
> +

How do we support hot-unplug without a matching remove function?

>  static void pci_bus_realize(BusState *qbus, Error **errp)
>  {
>      PCIBus *bus = PCI_BUS(qbus);
>  
>      vmstate_register(NULL, -1, &vmstate_pcibus, bus);
> +    notifier_list_init(&bus->reset_notifiers);
>  }
>  
>  static void pci_bus_unrealize(BusState *qbus, Error **errp)
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 40c97b1..88c3240 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -267,6 +267,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) {
> +        /* Particularly for vfio devices to reset host bus */
> +        notifier_list_notify(&s->sec_bus.reset_notifiers, NULL);
> +
>          /* Trigger hot reset on 0->1 transition. */
>          qbus_reset_all(&s->sec_bus.qbus);
>      }
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c

vfio changes should be done in a separate patch, the QEMU PCI-core
changes would need to be ack'd by mst.

> index 037a2c6..43869e9 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -154,6 +154,7 @@ typedef struct VFIOPCIDevice {
>      PCIHostDeviceAddress host;
>      EventNotifier err_notifier;
>      EventNotifier req_notifier;
> +    Notifier sec_bus_reset_notifier;
>      int (*resetfn)(struct VFIOPCIDevice *);
>      uint32_t features;
>  #define VFIO_FEATURE_ENABLE_VGA_BIT 0
> @@ -3524,6 +3525,14 @@ static void vfio_setup_resetfn(VFIOPCIDevice *vdev)
>      }
>  }
>  
> +static void vfio_pci_host_needs_bus_reset(Notifier *n, void *opaque)
> +{
> +    VFIOPCIDevice *vdev = container_of(n, VFIOPCIDevice, 
> sec_bus_reset_notifier);
> +    VFIODevice *vbasedev = &vdev->vbasedev;
> +
> +    vbasedev->needs_bus_reset = true;
> +}
> +
>  static int vfio_initfn(PCIDevice *pdev)
>  {
>      VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> @@ -3674,6 +3683,9 @@ static int vfio_initfn(PCIDevice *pdev)
>      vfio_register_req_notifier(vdev);
>      vfio_setup_resetfn(vdev);
>  
> +    vdev->sec_bus_reset_notifier.notify = vfio_pci_host_needs_bus_reset;
> +    pci_bus_add_reset_notifier(pdev->bus, &vdev->sec_bus_reset_notifier);
> +

The notifier obviously needs to be unregistered in the exitfn.

>      return 0;
>  
>  out_teardown:
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 5d050c8..eae236a 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -7,6 +7,7 @@
>  #include "exec/memory.h"
>  #include "sysemu/dma.h"
>  #include "qapi/error.h"
> +#include "qemu/notify.h"
>  
>  /* PCI includes legacy ISA access.  */
>  #include "hw/isa/isa.h"
> @@ -371,6 +372,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_add_reset_notifier(PCIBus *bus, Notifier *notify);
>  
>  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 fabaeee..3b551d7 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -29,6 +29,8 @@ struct PCIBus {
>         Keep a count of the number of devices with raised IRQs.  */
>      int nirq;
>      int *irq_count;
> +
> +    NotifierList reset_notifiers;
>  };
>  
>  typedef struct PCIBridgeWindows PCIBridgeWindows;
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 0d1fb80..5f5691a 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -102,6 +102,7 @@ typedef struct VFIODevice {
>      bool reset_works;
>      bool needs_reset;
>      bool allow_mmap;
> +    bool needs_bus_reset;

This is PCI specific, it should be added to VFIOPCIDevice only.

>      VFIODeviceOps *ops;
>      unsigned int num_irqs;
>      unsigned int num_regions;