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

[Chen Fan]

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 <address@hidden>
> > ---
> >   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
>
> .