From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41148) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai7Yq-0002o0-8h for qemu-devel@nongnu.org; Mon, 21 Mar 2016 17:40:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ai7Ym-0008Iq-PC for qemu-devel@nongnu.org; Mon, 21 Mar 2016 17:40:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50881) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai7Ym-0008Im-Hn for qemu-devel@nongnu.org; Mon, 21 Mar 2016 17:40:24 -0400 Date: Mon, 21 Mar 2016 15:40:20 -0600 From: Alex Williamson Message-ID: <20160321154020.1743d1fb@t450s.home> In-Reply-To: <1458554926-7844-9-git-send-email-caoj.fnst@cn.fujitsu.com> References: <1458554926-7844-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1458554926-7844-9-git-send-email-caoj.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 08/10] vfio: vote the function 0 to do host bus reset when aer occurred List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin Cc: chen.fan.fnst@cn.fujitsu.com, izumi.taku@jp.fujitsu.com, qemu-devel@nongnu.org, mst@redhat.com On Mon, 21 Mar 2016 18:08:44 +0800 Cao jin wrote: > From: Chen Fan > > Due to all devices assigned to VM on the same way as host if enable > aer, so we can easily do the hot reset by selecting the function #0 > to do the hot reset. > > Signed-off-by: Chen Fan > --- > hw/vfio/pci.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/vfio/pci.h | 2 ++ > 2 files changed, 52 insertions(+) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 9902c87..718cde7 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1900,6 +1900,8 @@ static void vfio_check_hot_bus_reset(VFIOPCIDevice *vdev, Error **errp) > /* List all affected devices by bus reset */ > devices = &info->devices[0]; > > + vdev->single_depend_dev = (info->count == 1); > + > /* Verify that we have all the groups required */ > for (i = 0; i < info->count; i++) { > PCIHostDeviceAddress host; > @@ -2608,11 +2610,36 @@ static void vfio_put_device(VFIOPCIDevice *vdev) > static void vfio_err_notifier_handler(void *opaque) > { > VFIOPCIDevice *vdev = opaque; > + PCIDevice *pdev = &vdev->pdev; > > if (!event_notifier_test_and_clear(&vdev->err_notifier)) { > return; > } > > + if (vdev->features & VFIO_FEATURE_ENABLE_AER) { > + VFIOPCIDevice *tmp; > + PCIDevice *dev; > + int devfn; > + > + /* > + * If one device has aer capability on a bus, when aer occurred, > + * we should notify all devices on the bus there was an aer arrived, > + * then we are able to vote the device #0 to do host bus reset. > + */ > + for (devfn = 0; devfn < 8; devfn++) { ARI? > + dev = pci_find_device(pdev->bus, pci_bus_num(pdev->bus), > + PCI_DEVFN(PCI_SLOT(pdev->devfn), devfn)); > + if (!dev) { > + continue; > + } > + if (!object_dynamic_cast(OBJECT(dev), "vfio-pci")) { > + continue; > + } > + tmp = DO_UPCAST(VFIOPCIDevice, pdev, dev); > + tmp->aer_occurred = true; > + } > + } > + > /* > * TBD. Retrieve the error details and decide what action > * needs to be taken. One of the actions could be to pass > @@ -3075,6 +3102,29 @@ static void vfio_pci_reset(DeviceState *dev) > > trace_vfio_pci_reset(vdev->vbasedev.name); > > + if (vdev->aer_occurred) { > + PCIDevice *br = pci_bridge_get_device(pdev->bus); > + > + if (br && > + (pci_get_word(br->config + PCI_BRIDGE_CONTROL) & > + PCI_BRIDGE_CTL_BUS_RESET)) { > + /* simply voting the function 0 to do hot bus reset */ > + if (pci_get_function_0(pdev) == pdev) { > + if (vdev->features & VFIO_FEATURE_ENABLE_AER) { > + vfio_pci_hot_reset(vdev, vdev->single_depend_dev); > + } else { > + /* if this device has not AER capability, code > + * coming here indicates there is another function > + * on the bus has AER capability. > + * */ This shouldn't be possible, right? > + vfio_pci_hot_reset(vdev, false); > + } > + } > + vdev->aer_occurred = false; > + return; > + } > + } Why do we care than an AER occurred now? Can't we simply test: if (vdev->features & VFIO_FEATURE_ENABLE_AER && pci_get_function_0(pdev) == pdev) { PCIDevice *br = pci_bridge_get_device(pdev->bus); if (pci_get_word(br->config + PCI_BRIDGE_CONTROL) & PCI_BRIDGE_CTL_BUS_RESET)) { vfio_pci_hot_reset(vdev, vdev->single_depend_dev); return; } } > + > vfio_pci_pre_reset(vdev); > > if (vdev->resetfn && !vdev->resetfn(vdev)) { > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h > index db7c6d5..17c75b8 100644 > --- a/hw/vfio/pci.h > +++ b/hw/vfio/pci.h > @@ -143,6 +143,8 @@ typedef struct VFIOPCIDevice { > bool no_kvm_intx; > bool no_kvm_msi; > bool no_kvm_msix; > + bool aer_occurred; > + bool single_depend_dev; > } VFIOPCIDevice; > > uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);