From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34018) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIwmC-0000dn-Bo for qemu-devel@nongnu.org; Wed, 04 Feb 2015 05:01:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YIwm8-0003Xh-5U for qemu-devel@nongnu.org; Wed, 04 Feb 2015 05:01:40 -0500 Received: from [59.151.112.132] (port=22593 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIwm7-0003X9-QR for qemu-devel@nongnu.org; Wed, 04 Feb 2015 05:01:36 -0500 Message-ID: <54D1EC5F.2020200@cn.fujitsu.com> Date: Wed, 4 Feb 2015 17:54:39 +0800 From: Chen Fan MIME-Version: 1.0 References: <87dd603bfecae702fc24207300b047937933618b.1422433767.git.chen.fan.fnst@cn.fujitsu.com> <1422908169.22865.420.camel@redhat.com> In-Reply-To: <1422908169.22865.420.camel@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v2 6/8] vfio_pci: fix a wrong check in vfio_pci_reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: marcel@redhat.com, izumi.taku@jp.fujitsu.com, qemu-devel@nongnu.org On 02/03/2015 04:16 AM, Alex Williamson wrote: > On Wed, 2015-01-28 at 16:37 +0800, Chen Fan wrote: >> when vfio device support FLR, then when device reset, >> we call VFIO_DEVICE_RESET ioctl to reset the device first, >> at kernel side, we also can see the order of reset: >> 3330 rc = pcie_flr(dev, probe); >> 3331 if (rc != -ENOTTY) >> 3332 goto done; >> 3333 >> 3334 rc = pci_af_flr(dev, probe); >> 3335 if (rc != -ENOTTY) >> 3336 goto done; >> 3337 >> 3338 rc = pci_pm_reset(dev, probe); >> 3339 if (rc != -ENOTTY) >> 3340 goto done; >> >> so when vfio has FLR, reset it directly. >> >> Signed-off-by: Chen Fan >> --- >> hw/vfio/pci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index 8c81bb3..54eb6b4 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -3455,7 +3455,7 @@ static void vfio_pci_reset(DeviceState *dev) >> vfio_pci_pre_reset(vdev); >> >> if (vdev->vbasedev.reset_works && >> - (vdev->has_flr || !vdev->has_pm_reset) && >> + vdev->has_flr && >> !ioctl(vdev->vbasedev.fd, VFIO_DEVICE_RESET)) { >> trace_vfio_pci_reset_flr(vdev->vbasedev.name); >> goto post_reset; > Does this actually fix anything? QEMU shouldn't rely on a specific > behavior of the kernel. This test is de-prioritizing a PM reset because > they're often non-effective. If the device supports FLR, the second > part of the OR is unreached, so what's the point of this change? For this change, when I tested the code on my own machine. I found the vfio device has neither flr nor pm reset (e.g. NoSoftRst+). this also trigger ioctl VFIO_DEVICE_RESET, is it right? Thanks, Chen > Thanks, > > Alex > > . >