Re: [Qemu-devel] [RFC v10 13/19] pci: add bus reset_notifiers callbacks for host bus reset

[Michael S. Tsirkin]

On Wed, Jun 17, 2015 at 09:41:40AM +0800, Chen Fan wrote:
> 
> On 06/16/2015 06:20 PM, Michael S. Tsirkin wrote:
> >On Tue, Jun 16, 2015 at 04:10:57PM +0800, Chen Fan wrote:
> >>Particularly, For vfio devices, 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>
> >Interesting. What prevents guest from triggering reset at an arbitrary
> >time, killing all VFs for all guests?
> If the VF device was assigned to VM, with enable aer, we check the
> VF affected devices e.g. the other VFs and PF is belonged to VM or not.
> so It can't to affect other VMs for this. if disable aer, there will no
> affection.
> 
> Thanks,
> Chen

So fundamentally this assumes PF is assigned to the VM?
Why even bother with this aer on VFs then?
Do this for PFs only.

> >
> >>---
> >>  hw/pci/pci.c             | 16 ++++++++++++++++
> >>  hw/pci/pci_bridge.c      |  6 ++++++
> >>  include/hw/pci/pci.h     |  4 ++++
> >>  include/hw/pci/pci_bus.h |  2 ++
> >>  4 files changed, 28 insertions(+)
> >>
> >>diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >>index 3423c3a..3bd954e 100644
> >>--- a/hw/pci/pci.c
> >>+++ b/hw/pci/pci.c
> >>@@ -74,11 +74,27 @@ static const VMStateDescription vmstate_pcibus = {
> >>      }
> >>  };
> >>+void pci_bus_add_reset_notifier(PCIBus *bus, Notifier *notify)
> >>+{
> >>+    notifier_list_add(&bus->reset_notifiers, notify);
> >>+}
> >>+
> >>+void pci_bus_remove_reset_notifier(Notifier *notify)
> >>+{
> >>+    notifier_remove(notify);
> >>+}
> >>+
> >>+void pci_bus_run_reset_notifier(PCIBus *bus, void *opaque)
> >>+{
> >>+    notifier_list_notify(&bus->reset_notifiers, opaque);
> >>+}
> >>+
> >>  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..a8e3f57 100644
> >>--- a/hw/pci/pci_bridge.c
> >>+++ b/hw/pci/pci_bridge.c
> >>@@ -267,6 +267,12 @@ void pci_bridge_write_config(PCIDevice *d,
> >>      newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
> >>      if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
> >>+        /*
> >>+         * Notify all vfio-pci devices under the bus
> >>+         * to do physical bus reset.
> >>+         */
> >>+        pci_for_each_bus(&s->sec_bus,
> >>+                pci_bus_run_reset_notifier, d);
> >>          /* Trigger hot reset on 0->1 transition. */
> >>          qbus_reset_all(&s->sec_bus.qbus);
> >>      }
> >>diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> >>index 6c2af0d..d353c9d 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"
> >>@@ -377,6 +378,9 @@ 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);
> >>+void pci_bus_remove_reset_notifier(Notifier *notify);
> >>+void pci_bus_run_reset_notifier(PCIBus *bus, void *opaque);
> >>  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;
> >>-- 
> >>1.9.3
> >>
> >.
> >