trivial comments:
On 02/03/17 07:51, 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.
s/must raised/must be raised/
> 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
s/suspious/spurious/ (3 occurrences in total)
Thanks
Laszlo
> 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: m.cerv...@computer.org
> Cc: 1373...@bugs.launchpad.net
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> 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;
>
