Hello.
Thanks for patch. I tested it.
Good news, does not assert() :-)
Bad news, not all works as expected (trace + pcap for usbredir attached),
maybe loosing some "interrupts".
------------------------------------------- EVENT description
@1486115814.426976
- guest (win7+renesas v2.1.39) boots ok (monitoring over RDP)
@1486115984.087097
- usbredir attach keyboard
- guest start recognize device - "usb compound" and 3x "usb hid" but NOT HID
keyboard
@address@hidden
- recognitions stops/hungs *** 1 ***
- I try to "press" key but no activity on network "usbredir" channel or
"xhci" (10 minutes)
@1486116641.063441
- disconnect USB keyboard
@1486116654.757221
- reconnect USB keyboard
- guest start recognize device - "usb compound" and 4x "usb hid" and HID
keyboard !
@1486116786.752275 (frame 143-145 of pcap)
- press + release key "enter"
- BUT there is no corresponding activity in xhci driver (no trace) *** 2 ***
@1486116990.347841
- shutdown guest
-------------------------------------------
I see two problems/questions:
*** 1 *** - Why recognition stopped ? Need some "hw" reset ? (no problem with
UHCI/EHCI emulation)
Sometimes (1:5) it attaches ok.
*** 2 *** - Recognized successfully but does not work. Problem with INTR
transfers ?
Mouse works with the same problems.
I try attach USB-flash disk, all seems to work ok (read files, write files)
and 2x faster then UHCI/EHCI emulation (and probably reached limit of
RaspberryPI).
Any hint to debug (maybe qemu/usbredir - usbredir/libusb bindings) ?
Thanks, Martin Cerveny
On Fri, 3 Feb 2017, Gerd Hoffmann wrote:
The qemu xhci emulation doesn't handle the ERDP_EHB flag correctly.
When the host adapter queues a new event the ERDP_EHB flag is set. The
flag is cleared (via w1c) by the guest when it updates the ERDP (event
ring dequeue pointer) register to notify the host adapter which events
it has fetched.
An IRQ must raised in case the ERDP_EHB flag flips from clear to set.
If the flag is set already (which implies there are events queued up
which are not yet processed by the guest) xhci must *not* raise a IRQ.
Qemu got that wrong and raised an IRQ on every event, thereby generating
suspious interrupts in case we've queued events faster than the guest
processed them. This patch fixes that.
With that change in place we also have to check ERDP updates, to see
whenever the guest has fetched all queued events. In case there are
still pending events set ERDP_EHB and raise an IRQ again, to make sure
the events don't linger unseen forever.
The linux kernel driver and the microsoft windows driver (shipped with
win8+) can deal with the suspious interrupts without problems. The
renesas windows driver (v2.1.39) which can be used on older windows
versions is quite upset though. It does suspious ERDP updates now and
then (not every time, seems we must hit a race window for this to
happen), which in turn makes the qemu xhci emulation think the event
ring is full. Things go south from here ...
tl;dr: This is the "fix xhci on win7" patch.
Cc: address@hidden
Cc: address@hidden
Signed-off-by: Gerd Hoffmann <address@hidden>
---
hw/usb/hcd-xhci.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index df75907..4f05747 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -789,11 +789,15 @@ static void xhci_msix_update(XHCIState *xhci, int v)
static void xhci_intr_raise(XHCIState *xhci, int v)
{
PCIDevice *pci_dev = PCI_DEVICE(xhci);
+ bool pending = (xhci->intr[v].erdp_low & ERDP_EHB);
xhci->intr[v].erdp_low |= ERDP_EHB;
xhci->intr[v].iman |= IMAN_IP;
xhci->usbsts |= USBSTS_EINT;
+ if (pending) {
+ return;
+ }
if (!(xhci->intr[v].iman & IMAN_IE)) {
return;
}
@@ -3352,6 +3356,15 @@ static void xhci_runtime_write(void *ptr, hwaddr
reg,
intr->erdp_low &= ~ERDP_EHB;
}
intr->erdp_low = (val & ~ERDP_EHB) | (intr->erdp_low & ERDP_EHB);
+ if (val & ERDP_EHB) {
+ dma_addr_t erdp = xhci_addr64(intr->erdp_low,
intr->erdp_high);
+ unsigned int dp_idx = (erdp - intr->er_start) / TRB_SIZE;
+ if (erdp >= intr->er_start &&
+ erdp < (intr->er_start + TRB_SIZE * intr->er_size) &&
+ dp_idx != intr->er_ep_idx) {
+ xhci_intr_raise(xhci, v);
+ }
+ }
break;
case 0x1c: /* ERDP high */
intr->erdp_high = val;
--
1.8.3.1