Re: [Qemu-devel] EHCI USB regression in 1.2.0 - ehci_state_fetchqtd() asserting

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10/08/2012 04:49 PM, Johannes Stezenbach wrote:
> Hi,
>
> On Mon, Oct 08, 2012 at 03:51:08PM +0200, Hans de Goede wrote:
>> On 10/08/2012 03:01 PM, Johannes Stezenbach wrote:
>>>
>>> There will always be a race between the call to USBDEVFS_DISCARDURB
>>> and the URB completing.  IMHO the handling in usb_host_stop_n_free_iso()
>>> is buggy.  How about dropping the "killed" and "free" variables and
>>> calling async_complete() and g_free() unconditionally?
>>
>> This race is well known already handled correctly,
>
> You mean the message about "leaking iso urbs" is wrong?
> (since it will be freed later in async_completem right?)
>

Hmm, ah this is about iso urbs, not about regular async urbs. Sorry, then I'm
not sure if the code is correct. I'm not that familiar with the host-linux
code, as I work on and use the usb-redir code mostly.

>> the real problem is the
>> "ehci warning: guest updated active QH" message, which most likely indicates
>> that the guest has hit the doorbell (IAAD) in the EHCI controller, and then
>> has not gotten an IAA interrupt within
>> a certain amount of time triggering its IAAD watchdog (some real EHCI
>> hardware is broken wrt delivering IAA interrupt) causing us to not see
>> an unlinked qh as unlinked, and then later on triggering the
>> "warning: guest updated active QH" message.
>>
>> This is unavoidable when we get too large latencies, the ehci hardware
>> simple was not designed to be virtualized, anything but actually.
>
> OK, thanks for this explanation.
> I haven't much clue about qemu but isn't the issue that qemu
> delivers timer irqs to the guest (for EHCI_HRTIMER_IAA_WATCHDOG) while
> failing to handle the IAAD -> IAA interrupt generation?
> (via qemu_bh_schedule -> ehci_advance_async_state -> ehci_raise_irq,
> why does ehci_raise_irq() not call ehci_update_irq() for USBSTS_IAA?)

We need to throttle the interrupt generation inside ehci both per
the spec, and because otherwise we may trigger:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=361aabf395e4a23cf554cf4ec0c0c6963b8beb01

Which is present in all but the very latest linux kernels.

> If that cannot be fixed, have you tried talking to the Linux
> EHCI driver maintainer if the EHCI_HRTIMER_IAA_WATCHDOG
> timeout (10ms) can be increased or skipped entirely for non-broken hw?
> (Linux commit 26f953fd884ea4879 suggests it's only for VIA chips)

AFAIK Shawn is using a windows guest, so the fact that he is seeing
the "ehci warning: guest updated active QH" warning, suggests that
windows is using a watchdog too, also the messages about cancelling
the isoc stream suggest that windows is actively stopping the stream
from the webcam, at which point there is little the redir code
can do to recover. This could be caused by timeouts caused by the
latency spikes from the debug kernel...

We do our best to make the whole usb-redir code and ehci emulation as
proof as possible against latency spikes, and some of the patches
from my latest patchset may help there, but in the end, esp. for
isoc devices, the code will always be sensitive to too large latencies.

Regards,

Hans