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