On Fri, Jun 22, 2012 at 6:16 PM, Alexey Kardashevskiy <a...@ozlabs.ru> wrote:

> On 07/06/12 09:17, Alex Williamson wrote:
> > On Fri, 2012-05-25 at 17:35 +1000, Alexey Kardashevskiy wrote:
> >> Some adapters (like NEC PCI USB controller) do not flush their config
> >> on a sioftware reset and remember DMA config, etc.
> >>
> >> If we use such an adapter with QEMU, then crash QEMU (stop it with
> >> ctrl-A ctrl-X), and try to use it in QEMU again, it may start working
> >> immediately with previous config when pci_enable_device() is called
> >> on that PCI function.
> >>
> >> To eliminate such effect, some quirk should be called. The proposed
> >> pci_fixup_final does its job well for mentioned NEC PCI USB but not
> >> sure if it is 100% correct.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> >> ---
> >>  drivers/vfio/pci/vfio_pci.c |    2 ++
> >>  1 files changed, 2 insertions(+), 0 deletions(-)
> >>
> >> 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?  Thanks,
>
>
> Checked again.
>
> Seems to be a false alarm, cannot reproduce the bad behavior anymore,
> looks like it was caused by
> another issue which Alex fixed.
>
> So although the problem may arise again, there is nothing urgent to do at
> the moment.
>
>
While doing cleanups in my SPAPR IOMMU driver for VFIO,
I found that I have not unmapped all the pages on module_exit.
Heh. My bad. So I implemented that and got a lot of strange accidental
crashes of the host kernel when I tried to debug
"USB Controller: NEC Corporation USB (rev 43)".
I applied the quoted patch and it has gone.

You asked what fixup exactly does but honestly I do not know.
>From the code, it enables a device, does some tricks in
quirk_usb_handoff_ohci()/quirk_usb_disable_ehci() and
disables the device back. The comments of these quirks
say that they basically disable interrupts and do shutdown/reset
via OHCI/EHCI specific registers as pci_disable_device() does
not do the job like it does not reset the device's DMA config so when
it gets enabled back, it continues DMA transfer to/from addresses
which were actual at the moment of QEMU shutdown/group release.

So do we add a new class of quirks?


-- 
Alexey

Reply via email to