Re: [Qemu-devel] [RFC v2 6/8] vfio_pci: fix a wrong check in vfio_pci_reset

[Alex Williamson <alex.williamson@redhat.com>]

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 <chen.fan.fnst@cn.fujitsu.com>
> ---
>  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?
Thanks,

Alex