From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37111) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwqPy-0003dW-3p for qemu-devel@nongnu.org; Thu, 12 Nov 2015 06:51:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwqPt-0001EU-FN for qemu-devel@nongnu.org; Thu, 12 Nov 2015 06:51:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43008) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwqPt-0001EL-7e for qemu-devel@nongnu.org; Thu, 12 Nov 2015 06:51:49 -0500 Date: Thu, 12 Nov 2015 13:51:46 +0200 From: "Michael S. Tsirkin" Message-ID: <20151112134713-mutt-send-email-mst@redhat.com> References: <7e222e2840fe58fe26d3bd73f626c1da029ca981.1447231392.git.chen.fan.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7e222e2840fe58fe26d3bd73f626c1da029ca981.1447231392.git.chen.fan.fnst@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH v13 09/13] add check reset mechanism when hotplug vfio device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin Cc: Chen Fan , alex.williamson@redhat.com, qemu-devel@nongnu.org On Wed, Nov 11, 2015 at 06:34:27PM +0800, Cao jin wrote: > From: Chen Fan > > Since we support multi-function hotplug. the function 0 indicate > the closure of the slot, so we have the chance to do the check. > > Signed-off-by: Chen Fan > --- > hw/pci/pci.c | 29 +++++++++++++++++++++++++++++ > hw/vfio/pci.c | 19 +++++++++++++++++++ > hw/vfio/pci.h | 2 ++ > include/hw/pci/pci_bus.h | 5 +++++ > 4 files changed, 55 insertions(+) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 168b9cc..f6ca6ef 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -81,6 +81,7 @@ static void pci_bus_realize(BusState *qbus, Error **errp) > PCIBus *bus = PCI_BUS(qbus); > > vmstate_register(NULL, -1, &vmstate_pcibus, bus); > + notifier_with_return_list_init(&bus->hotplug_notifiers); > } > > static void pci_bus_unrealize(BusState *qbus, Error **errp) > @@ -1835,6 +1836,22 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn) > return bus->devices[devfn]; > } > > +void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify) > +{ > + notifier_with_return_list_add(&bus->hotplug_notifiers, notify); > +} > + > +void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notifier) > +{ > + notifier_with_return_remove(notifier); > +} > + > +static int pci_bus_hotplug_notifier(PCIBus *bus, void *opaque) > +{ > + return notifier_with_return_list_notify(&bus->hotplug_notifiers, > + opaque); > +} > + > static void pci_qdev_realize(DeviceState *qdev, Error **errp) > { > PCIDevice *pci_dev = (PCIDevice *)qdev; > @@ -1877,6 +1894,18 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) > pci_qdev_unrealize(DEVICE(pci_dev), NULL); > return; > } > + > + /* > + * If the function is func 0, indicate the closure of the slot. > + * signal the callback. > + */ > + if (DEVICE(pci_dev)->hotplugged && > + pci_get_function_0(pci_dev) == pci_dev && > + pci_bus_hotplug_notifier(bus, pci_dev)) { > + error_setg(errp, "failed to hotplug function 0"); > + pci_qdev_unrealize(DEVICE(pci_dev), NULL); > + return; > + } I don't understand why this is required in pci core. PCI Device is already constructed anyway. Just do the checks and call unrealize in vfio. I also don't see why you are tying this to hotplug. I would check each function as it's added. But that's a vfio thing, if both you and Alex think it's a good idea, fine by me. > } > > static void pci_default_realize(PCIDevice *dev, Error **errp) > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 31ffd44..e619998 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1990,6 +1990,19 @@ static int vfio_check_devices_host_bus_reset(void) > return 0; > } > > +static int vfio_check_bus_reset(NotifierWithReturn *n, void *opaque) > +{ > + VFIOPCIDevice *vdev = container_of(n, VFIOPCIDevice, hotplug_notifier); > + PCIDevice *pci_dev = PCI_DEVICE(vdev); > + PCIDevice *pci_func0 = opaque; > + > + if (pci_get_function_0(pci_dev) != pci_func0) { > + return 0; > + } > + > + return vfio_check_host_bus_reset(vdev); > +} > + > static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, > int pos, uint16_t size) > { > @@ -2044,6 +2057,9 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver, > return ret; > } > > + vdev->hotplug_notifier.notify = vfio_check_bus_reset; > + pci_bus_add_hotplug_notifier(pdev->bus, &vdev->hotplug_notifier); > + > return 0; > > error: > @@ -2919,6 +2935,9 @@ static void vfio_exitfn(PCIDevice *pdev) > vfio_unregister_req_notifier(vdev); > vfio_unregister_err_notifier(vdev); > pci_device_set_intx_routing_notifier(&vdev->pdev, NULL); > + if (vdev->features & VFIO_FEATURE_ENABLE_AER) { > + pci_bus_remove_hotplug_notifier(&vdev->hotplug_notifier); > + } > vfio_disable_interrupts(vdev); > if (vdev->intx.mmap_timer) { > timer_free(vdev->intx.mmap_timer); > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h > index 59ae194..b385f07 100644 > --- a/hw/vfio/pci.h > +++ b/hw/vfio/pci.h > @@ -142,6 +142,8 @@ typedef struct VFIOPCIDevice { > bool no_kvm_intx; > bool no_kvm_msi; > bool no_kvm_msix; > + > + NotifierWithReturn hotplug_notifier; > } VFIOPCIDevice; > > uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len); > diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h > index 403fec6..7812fa9 100644 > --- a/include/hw/pci/pci_bus.h > +++ b/include/hw/pci/pci_bus.h > @@ -39,8 +39,13 @@ struct PCIBus { > Keep a count of the number of devices with raised IRQs. */ > int nirq; > int *irq_count; > + > + NotifierWithReturnList hotplug_notifiers; > }; > > +void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify); > +void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notify); > + > typedef struct PCIBridgeWindows PCIBridgeWindows; > > /* > -- > 1.9.3