qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Alexey Kardashevskiy
Subject: Re: [Qemu-devel] [RFC PATCH] vfio: add fixup for broken PCI devices
Date: Sat, 18 Aug 2012 00:28:49 +1000



On Fri, Jun 22, 2012 at 6:16 PM, Alexey Kardashevskiy <address@hidden> 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 <address@hidden>
>> ---
>>  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

[Prev in Thread] Current Thread [Next in Thread]