From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47110) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agj5y-0008WH-Kb for qemu-devel@nongnu.org; Thu, 17 Mar 2016 21:20:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agj5u-0006AB-Es for qemu-devel@nongnu.org; Thu, 17 Mar 2016 21:20:54 -0400 Received: from [59.151.112.132] (port=3063 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agj5t-00066O-PP for qemu-devel@nongnu.org; Thu, 17 Mar 2016 21:20:50 -0400 Message-ID: <56EB56AA.1090602@cn.fujitsu.com> Date: Fri, 18 Mar 2016 09:15:22 +0800 From: Chen Fan MIME-Version: 1.0 References: <1458005749-4878-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1458005749-4878-8-git-send-email-caoj.fnst@cn.fujitsu.com> <20160315143841.6df652c7@t450s.home> In-Reply-To: <20160315143841.6df652c7@t450s.home> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [patch v3 7/9] 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: Alex Williamson , Cao jin Cc: izumi.taku@jp.fujitsu.com, qemu-devel@nongnu.org, mst@redhat.com On 03/16/2016 04:38 AM, Alex Williamson wrote: > On Tue, 15 Mar 2016 09:35:47 +0800 > Cao jin wrote: > >> From: Chen Fan >> >> Signed-off-by: Chen Fan >> --- >> hw/pci/pci.c | 2 ++ >> hw/vfio/pci.c | 14 ++++++++++++++ >> hw/vfio/pci.h | 1 + >> include/hw/pci/pci_bus.h | 2 ++ >> 4 files changed, 19 insertions(+) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index e67664d..953745d 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -276,6 +276,8 @@ static void pcibus_reset(BusState *qbus) >> for (i = 0; i < bus->nirq; i++) { >> assert(bus->irq_count[i] == 0); >> } >> + >> + bus->is_bus_rst = false; >> } >> >> static void pci_host_bus_register(DeviceState *host) >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index 223c0ee..b944d0b 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -1901,6 +1901,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; >> @@ -2593,6 +2595,10 @@ static void vfio_err_notifier_handler(void *opaque) >> return; >> } >> >> + if ((vdev->features & VFIO_FEATURE_ENABLE_AER)) { >> + vdev->pdev.bus->is_bus_rst = true; >> + } >> + > So we're *assuming* that the next reset will be a bus reset because we > took an AER fault... what if that particular error got handled in > another way, maybe a device specific handler that doesn't do a bus > reset? The asymmetry of setting a value here and clearing it in PCI > code is pretty undesirable as well. Can we detect that the bus is in > reset on our own given the current set of configuration restrictions? > Seems pretty easy to test for an AER device with the parent bridge > having PCI_BRIDGE_CTL_BUS_RESET set and wait for function #0 to do a > hot reset. Thanks, Indeed. Thanks for your help. Chen > > Alex > >> /* >> * TBD. Retrieve the error details and decide what action >> * needs to be taken. One of the actions could be to pass >> @@ -3060,6 +3066,14 @@ static void vfio_pci_reset(DeviceState *dev) >> >> trace_vfio_pci_reset(vdev->vbasedev.name); >> >> + if (pdev->bus->is_bus_rst) { >> + /* simply voting the function 0 to do hot bus reset */ >> + if (pci_get_function_0(pdev) == pdev) { >> + 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 aff46c2..32bd31f 100644 >> --- a/hw/vfio/pci.h >> +++ b/hw/vfio/pci.h >> @@ -143,6 +143,7 @@ typedef struct VFIOPCIDevice { >> bool no_kvm_intx; >> bool no_kvm_msi; >> bool no_kvm_msix; >> + bool single_depend_dev; >> } 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..6bcd334 100644 >> --- a/include/hw/pci/pci_bus.h >> +++ b/include/hw/pci/pci_bus.h >> @@ -39,6 +39,8 @@ struct PCIBus { >> Keep a count of the number of devices with raised IRQs. */ >> int nirq; >> int *irq_count; >> + >> + bool is_bus_rst; >> }; >> >> typedef struct PCIBridgeWindows PCIBridgeWindows; > > > . >