[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Bug 1859359] Re: xHCI and event ring handling
From: |
Benjamin David Lunt |
Subject: |
[Bug 1859359] Re: xHCI and event ring handling |
Date: |
Tue, 14 Jan 2020 00:44:18 -0000 |
Hi Hector, guys,
I think I have found my/the error:
xHCI, version 1.0, Page 136:
"Note: The detection of a Cycle bit mismatch in an Event TRB processed
by software indicates the location of the xHC Event Ring Enqueue Pointer
and that the Event Ring is empty. Software shall write the ERDP with the
address of this TRB to indicate that it has processed all Events in the
ring."
It does not state to advance the Consumer's internal Dequeue pointer.
Just the register is mentioned.
This is my error. I thought that it implied to advance the Consumer's
internal Dequeue Pointer as well. (Implied being the big word here)
Sorry for the bother. It was my error. It took me a bit of (re)reading
and a little more work to find that it was/is indeed my error.
Sorry and thank you for your time,
Ben
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1859359
Title:
xHCI and event ring handling
Status in QEMU:
Invalid
Bug description:
I believe that the Event Ring handling in QEMU is not correct. For
example, an Event Ring may have multiple segments. However, the code
in xhci_write_event()
(https://git.qemu.org/?p=qemu.git;a=blob;f=hw/usb/hcd-
xhci.c;hb=HEAD#l645), starting with line 668, seems to only support a
single segment.
Also, QEMU is sending a spurious interrupt after the Guest writes to
the ERDP register due to the fact that the address written does not
match the current index. This is because the index is incremented
after sending the event. The xHCI specification states in section
5.5.2.3.3 "When software finishes processing an Event TRB, it will
write the address of that Event TRB to the ERDP."
Since xhci_write_event() has already incremented the index pointer
(intr->er_ep_idx), the check at line 3098
(https://git.qemu.org/?p=qemu.git;a=blob;f=hw/usb/hcd-
xhci.c;hb=HEAD#l3090) no longer is valid.
I have not studied QEMU's code enough yet to offer a patch. However,
this should be a simple fix.
intr->er_ep_idx++;
if (intr->er_ep_idx >= intr->er_table[intr->er_segment].er_size) {
intr->er_ep_idx = 0;
intr->er_segment++;
if (intr->er_segment >= intr->er_table_size) {
intr->er_segment = 0;
intr->er_pcs = !intr->er_pcs;
}
}
Being sure to incorporate this new segment member into the above code
(possibly as shown) as well as change the lines at 665 to use the new
segment member of the structure, and of course in the initialization
portion of the event ring.
As for the spurious interrupt at line 3101, a new member will need to
be added to the code to keep track of the last inserted ED (TRB) into
the Event Ring and then of course checking against this new member,
not the now newly incremented member.
I have sent an email to the author listed at the top of the file as
well, not sure if this is proper bug reporting etiquette or not.
Thank you.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1859359/+subscriptions
- [Bug 1859359] [NEW] xHCI and event ring handling, Benjamin David Lunt, 2020/01/12
- [Bug 1859359] Re: xHCI and event ring handling, Benjamin David Lunt, 2020/01/12
- [Bug 1859359] Re: xHCI and event ring handling, Benjamin David Lunt, 2020/01/12
- [Bug 1859359] Re: xHCI and event ring handling, Benjamin David Lunt, 2020/01/12
- [Bug 1859359] Re: xHCI and event ring handling, Gerd Hoffmann, 2020/01/13
- [Bug 1859359] Re: xHCI and event ring handling, Benjamin David Lunt, 2020/01/13
- [Bug 1859359] Re: xHCI and event ring handling, Hector Martin, 2020/01/13
- [Bug 1859359] Re: xHCI and event ring handling,
Benjamin David Lunt <=