From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41244) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv0oc-00035d-0d for qemu-devel@nongnu.org; Wed, 20 May 2015 06:01:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yv0oX-0007Ho-MH for qemu-devel@nongnu.org; Wed, 20 May 2015 06:01:29 -0400 Received: from [59.151.112.132] (port=20752 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv0oW-000782-Ps for qemu-devel@nongnu.org; Wed, 20 May 2015 06:01:25 -0400 Message-ID: <555C5AF9.4010104@cn.fujitsu.com> Date: Wed, 20 May 2015 17:59:21 +0800 From: Chen Fan MIME-Version: 1.0 References: <1432064074.11375.247.camel@redhat.com> In-Reply-To: <1432064074.11375.247.camel@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v7 05/11] vfio: do hot bus reset when do virtual secondary bus reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: izumi.taku@jp.fujitsu.com, qemu-devel@nongnu.org On 05/20/2015 03:34 AM, Alex Williamson wrote: > On Tue, 2015-05-19 at 12:42 +0800, Chen Fan wrote: >> when do virtual secondary bus reset, the vfio device under >> this bus need to do host bus reset to reset the device. >> so add this case. >> >> Signed-off-by: Chen Fan >> --- >> hw/vfio/pci.c | 12 +++++++++++- >> include/hw/vfio/vfio-common.h | 1 - >> 2 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index ff639db..cc169ea 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -169,6 +169,7 @@ typedef struct VFIOPCIDevice { >> bool has_flr; >> bool has_pm_reset; >> bool has_bus_reset; >> + bool needs_bus_reset; >> bool rom_read_failed; >> } VFIOPCIDevice; >> >> @@ -3531,7 +3532,8 @@ 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; >> + vdev->needs_bus_reset = true; >> + vbasedev->needs_reset = true; > The above should have happened in patch 03. > >> } >> >> static void vfio_check_host_bus_reset(VFIODevice *vbasedev) >> @@ -3807,6 +3809,14 @@ static void vfio_pci_reset(DeviceState *dev) >> >> trace_vfio_pci_reset(vdev->vbasedev.name); >> >> + if (vdev->has_bus_reset && vdev->needs_bus_reset) { >> + vdev->needs_bus_reset = false; >> + if (vdev->vbasedev.needs_reset) { >> + vfio_pci_hot_reset(vdev, false); > So we're not even going to check to see whether that worked? How can we > know that the configuration is still the same as after machine init? An > affected device could have been hot-unplugged an now in use by the host > (in which case this function would fail). An affected device could have > been hot unplugged and added back to the VM under a different bus, now > we have an arbitrary second device affected by the bus reset from the VM > perspective. Due to we may have the arbitrary configuration manner after hotplug /unplug vfio-pci devices, the enabled aer feature probably became to disabled. so shall we consider not to expose the aer feature to user, and enable/disable it automatically? Thanks, Chen > >> + } >> + return; >> + } >> + >> vfio_pci_pre_reset(vdev); >> >> if (vdev->resetfn && !vdev->resetfn(vdev)) { >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >> index 5f5691a..0d1fb80 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -102,7 +102,6 @@ typedef struct VFIODevice { >> bool reset_works; >> bool needs_reset; >> bool allow_mmap; >> - bool needs_bus_reset; >> VFIODeviceOps *ops; >> unsigned int num_irqs; >> unsigned int num_regions; > > > . >