Hi Mathias,
On 1/12/2023 1:24 AM, Mathias Nyman wrote:
On 11.1.2023 5.11, Wesley Cheng wrote:
Hi Mathias,
On 1/10/2023 12:03 PM, Wesley Cheng wrote:
Hi Mathias,
On 1/10/2023 11:47 AM, Mathias Nyman wrote:
On 9.1.2023 22.24, Wesley Cheng wrote:
Hi Mathias,
On 1/2/2023 8:38 AM, Mathias Nyman wrote:
On 29.12.2022 23.14, Wesley Cheng wrote:
Hi Mathias,
On 12/28/2022 7:47 AM, Mathias Nyman wrote:
On 24.12.2022 1.31, Wesley Cheng wrote:
Implement the XHCI operations for allocating and requesting for
a secondary
interrupter. The secondary interrupter can allow for events for a
particular endpoint to be routed to a separate event ring. The
event
routing is defined when submitting a transfer descriptor to the
USB HW.
There is a specific field which denotes which interrupter ring
to route the
event to when the transfer is completed.
An example use case, such as audio packet offloading can
utilize a separate
event ring, so that these events can be routed to a different
processor
within the system. The processor would be able to
independently submit
transfers and handle its completions without intervention from
the main
processor.
Adding support for more xHCI interrupters than just the primary
one make sense for
both the offloading and virtualization cases.
xHCI support for several interrupters was probably added to
support virtualization,
to hand over usb devices to virtual machines and give them their
own event ring and
MSI/MSI-X vector.
In this offloading case you probably want to avoid xHC
interrupts from this device
completely, making sure it doesn't wake up the main CPU
unnecessarily.
So is the idea here to let xhci driver set up the new
interrupter, its event ring,
and the endpoint transfer rings. Then pass the address of the
endpoint transfer rings
and the new event ring to the separate processor.
This separate processor then both polls the event ring for new
events, sets its dequeue
pointer, clears EHB bit, and queues new TRBs on the transfer ring.
so xhci driver does not handle any events for the audio part,
and no audio data URBs
are sent to usb core?
Your entire description is correct. To clarify, the interfaces
which are non-audio will still be handled by the main processor.
For example, a USB headset can have a HID interface as well for
volume control. The HID interface will still be handled by the
main processor, and events routed to the main event ring.
How about the control part?
Is the control endpoint for this device still handled normally
by usb core/xhci?
Control transfers are always handled on the main processor. Only
audio interface's endpoints.
Good to know, that means interrupter should be chosen per
endpoint, not per device.
For the xhci parts I think we should start start by adding
generic support for several
interrupters, then add parts needed for offloading.
I can split up the patchsets to add interrupters first, then
adding the offloading APIs in a separate patch.
I started looking at supporting secondary interrupters myself.
Let me work on that part a bit first. We have a bit different end
goals.
I want to handle interrupts from a secondary interrupter, while
this audio offload
really just wants to mask some interrupts.
I was looking at how we could possibly split up the XHCI secondary
interrupter, and offloading parts. Since the XHCI secondary
interrupter is a feature that is defined in the XHCI spec (and we
aren't doing anything outside of what is defined), I was thinking
of having a separate XHCI driver (ie xhci-sec.c/h) that can be used
to define all APIs related to setting up the event ring and ring
management. (interrupt support can be added here) This aligns a
bit with what Alan suggested, and removing the APIs in the USB HCD,
since this is XHCI specific stuff. (
https://lore.kernel.org/linux-usb/Y6zwZOquZOTZfnvP@xxxxxxxxxxxxxxxxxxx/
)
Already started working on the interrupter, that part fits well into
current driver.
Code (untested, will be randomly rebased etc) can be found in my
feature_interrupters branch:
git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git
feature_interrupters
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters
Oh perfect, let me take a look. Thanks for this!
I actually tried to see if I could get our audio offloading to work
with your current series. (I understand its still work in progress)
I did have to make some changes to expose the APIs to our class
driver, but I wanted to let you know about one of the issues I saw
when developing my implementation, because I am seeing the same
behavior w/ yours. (and there's a discrepancy w/ what's stated in the
XHCI spec :))
So the reason why my initial submission did the event ring allocation
and set up before the run/stop bit was set, is that I found that when
writing to the ir_set->erst_base in this scenario (for the secondary
interrupter), it lead to a SMMU fault from the DWC3 controller. One
thing I noticed, was that the SMMU fault address was the lower 32 bits
of the segment table base address allocated. The XHCI driver utilizes
the xhci_write_64() api which first writes the lower 32 bits then the
upper 32 bits. The XHCI spec states that:
Table 5-41: Event Ring Segment Table Base Address Register Bit
Definitions (ERSTBA)
"Event Ring Segment Table Base Address Register – RW. Default = ‘0’.
This field defines the
high order bits of the start address of the Event Ring Segment Table.
Writing this register sets the Event Ring State Machine:EREP
Advancement to the Start state.
Refer to Figure 4-12 for more information.
**For Secondary Interrupters: This field may be modified at any time.**"
I'm not sure if this is an issue with the specific controller we're
using, so maybe I will wait until you can give this a try on your set
up. However, it doesn't seem to be true that we can write the ERSTBA
any time we want to. My assumption is that once I made the lower 32
bit write, the controller attempted to enable the Event Ring State
machine (Figure 4-12), and this led to a SMMU fault, since the upper
64 bits haven't been written. I also did some bit banging manually as
well (using devmem) and any time I write to the secondary ring ERSTBA
register it generates a fault. (before any offloading has started)
Tried on an Intel host and it seems to work fine.
I created a few secondary interrupters while xHC was running without
issues.
DMA mask is 64 bits.
Only created the interrupters, no events on those new event rings, and
didn't actually
check that the values written to ERSTBA were 64 bit.
Does temporarily setting DMA mask to 32 bits while allocating erst help
in your case?
Yes, that works fine. Another thing I found which works is that, the
XHCI spec mentions that we should always write the lower 32 bits first
before the upper bits for a 64 bit register. Just as an experiment, I
flipped the write order (upper first, lower second), and that seemed to
not trigger the event ring state machine, and I was able to verify that
our audio offloading was working.
At the moment, I exposed/exported the
xhci_remove_secondary_interrupter() and
xhci_create_secondary_interrupter() into a header file
(include/linux/usb/xhci-intr.h), along with some other APIs to fetch the
transfer resources. (same mechanism I have in my patchset) This allowed
me to only reference the required APIs from the USB audio offload class
driver w/o having to include all of xhci.h (where some of the things are
currently defined in your changes)
Thanks
Wesley Cheng
