Re: [RFC PATCH] vfio: add fixup for broken PCI devices

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Wed, 2012-06-06 at 17:17 -0600, Alex Williamson wrote:
> > diff --git a/drivers/vfio/pci/vfio_pci.c
> b/drivers/vfio/pci/vfio_pci.c
> > index 1e5315c..6e7c12d 100644
> > --- a/drivers/vfio/pci/vfio_pci.c
> > +++ b/drivers/vfio/pci/vfio_pci.c
> > @@ -88,6 +88,8 @@ static void vfio_pci_disable(struct
> vfio_pci_device *vdev)
> >  {
> >       int bar;
> >  
> > +     pci_fixup_device(pci_fixup_final, vdev->pdev);
> > +
> >       pci_disable_device(vdev->pdev);
> >  
> >       vfio_pci_set_irqs_ioctl(vdev, VFIO_IRQ_SET_DATA_NONE |
> 
> Sorry, just taking a look at this again.  Do you have any idea what
> fixup it is that makes it work?  Calling a fixup at this point seems
> rather odd.  I suspect the problem is that vfio is only calling
> pci_load_and_free_saved_state if pci_reset_function reports that it
> worked.  kvm device assignment doesn't do that and I'm not sure why I
> did that.  If you unconditionally call pci_load_and_free_saved_state a
> bit further down in this function, does it solve the problem?  

No it won't do, you need device-specific "reset" fixup code for devices
where the function reset doesn't do the right thing.

My suggestion is to add a new quirk category (in addition to
early,late,... add reset) and call that here.

Then we can do one for the NEC OHCI that properly stops the controller,
among others. I would be -very- surprised if that chip ends up being the
only one causing that sort of trouble.

Also, some chips will need some "tweaks" after the reset, for example if
we do a full link reset, I know of at least one device that might
randomly fail to properly train the PCIe link, such a quirk is a perfect
spot to add the right fixup.

Cheers,
Ben.