On Thu, 2012-06-07 at 12:52 +1000, Benjamin Herrenschmidt wrote:
> 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.
In so far as vfio should only have to call pci_reset_function and device
quirks take care of everything else, I agree with you, but that doesn't
answer any of my questions. Sure, we may want pre- and post-reset fixup
quirks and a pony, but what quirk is actually necessary for this device?
Does it fit into the existing pci_dev_specific_reset quirking?
Reloading config space isn't a good generic solution, but it might at
least shed some light on whether the reset function is doing anything
and if a simple config space change fixes it. VFIO needs to do this
anyway. Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
