Details

Reviewers

aokblast
hrs
emaste

Commits

rG405cfeef615f: xhci: Parse and print extended capabilities

Summary

We need to pick up extended capabilities for xhci debug. It makes sense to me to parse out all the capabilities on the controller, printing those that we know. This will make it easier to know if debug is supported, we can tell someone to check dmesg, and it lets us gather up from dmesgd the deployment of other features.

A change like this should make it possible to know if other interesting extensions are implemented or not by vendors, virt is one which if available would be worth exploring.

Test Plan

Please patch, reboot and provide the output from the following commands:

$ dmesg | grep xhci; sysctl hw.model; pciconf -l | grep xhci

It will look something like this:

xhci0: <Intel Union Point USB 3.0 controller> mem 0xf7030000-0xf703ffff at device 20.0 on pci0
xhci0: 32 bytes context size, 64-bit DMA
xhci0: xECP capabilities <PROTO,PROTO,LEGACY,DEBUG>
usbus0 on xhci0
hw.model: Intel(R) Core(TM) i5-7600T CPU @ 2.80GHz
xhci0@pci0:0:20:0:      class=0x0c0330 rev=0x00 hdr=0x00 vendor=0x8086 device=0xa2af subvendor=0x17aa subdevice=0x3111

The pciconf line will help me spot controllers using the generic driver, if any appear I'll add them to the parse list.

Testing on systems with none pcie xhci controllers would be great too as I don't have any handy right now. The raspberry pi is supposed to support xhci debug, but I've no direct evidence

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 66753
Build 63636: arc lint + arc unit

Event Timeline

thj requested review of this revision.Jul 11 2025, 8:05 AM

thj created this revision.

I'll see if I can try this on real hardware tomorrow.

sys/dev/usb/controller/xhci.c
668	That seems like it should be an assertion, not a check.

➜  ~ dmesg | grep xhci; sysctl hw.model; pciconf -l | grep xhci
xhci0: <XHCI (generic) USB 3.0 controller> mem 0xf6700000-0xf6707fff at device 0.0 on pci10
xhci0: 32 bytes context size, 64-bit DMA
xhci0: xECP capabilities <LEGACY,PROTO,PROTO,PROTO,DEBUG>
usbus0 on xhci0
xhci1: <XHCI (generic) USB 3.0 controller> mem 0xf6300000-0xf63fffff at device 0.3 on pci13
xhci1: 64 bytes context size, 64-bit DMA
xhci1: xECP capabilities <LEGACY,PROTO,PROTO,PROTO,DEBUG>
usbus1 on xhci1
xhci2: <XHCI (generic) USB 3.0 controller> mem 0xf6200000-0xf62fffff at device 0.4 on pci13
xhci2: 64 bytes context size, 64-bit DMA
xhci2: xECP capabilities <LEGACY,PROTO,PROTO,PROTO,DEBUG>
usbus2 on xhci2
xhci3: <XHCI (generic) USB 3.0 controller> mem 0xf6a00000-0xf6afffff at device 0.0 on pci14
xhci3: 64 bytes context size, 64-bit DMA
xhci3: xECP capabilities <LEGACY,PROTO,DEBUG>
usbus3 on xhci3
hw.model: AMD Ryzen 9 7900X 12-Core Processor            
xhci0@pci0:10:0:0:	class=0x0c0330 rev=0x01 hdr=0x00 vendor=0x1022 device=0x43f7 subvendor=0x1b21 subdevice=0x1142
xhci1@pci0:13:0:3:	class=0x0c0330 rev=0x00 hdr=0x00 vendor=0x1022 device=0x15b6 subvendor=0x1043 subdevice=0x8877
xhci2@pci0:13:0:4:	class=0x0c0330 rev=0x00 hdr=0x00 vendor=0x1022 device=0x15b7 subvendor=0x1043 subdevice=0x8877
xhci3@pci0:14:0:0:	class=0x0c0330 rev=0x00 hdr=0x00 vendor=0x1022 device=0x15b8 subvendor=0x1043 subdevice=0x8877

thj added inline comments.Jul 11 2025, 10:58 AM

sys/dev/usb/controller/xhci.c
668	it can probably go all together, I've pulled this out of another patch set where sc could be null and it not be a problem. Thanks for spotting it

thj mentioned this in D51259: xhci pci: Add some amd xhci controller device ids.Jul 11 2025, 11:41 AM

In D51255#1170269, @kp wrote:

I'll see if I can try this on real hardware tomorrow.

xhci0: <Intel Lewisburg USB 3.0 controller> mem 0x92f00000-0x92f0ffff irq 16 at device 20.0 numa-domain 0 on pci0
xhci0: 32 bytes context size, 64-bit DMA
xhci0: xECP capabilities <PROTO,PROTO,LEGACY,DEBUG>
xhci_interrupt: host controller halted
xhci_interrupt: host controller halted
xhci_interrupt: host controller halted
usbus0 numa-domain 0 on xhci0
hw.model: Intel(R) Xeon(R) Silver 4210 CPU @ 2.20GHz
xhci0@pci0:0:20:0: class=0x0c0330 rev=0x09 hdr=0x00 vendor=0x8086 device=0xa1af subvendor=0x1028 subdevice=0x07e5

# my crappy xps13 laptop
dmesg |grep xhci ; sysctl hw.model; pciconf -l |grep xhci
xhci0: <Intel Sunrise Point-LP USB 3.0 controller> mem 0xdc310000-0xdc31ffff at device 20.0 on pci0
xhci0: 32 bytes context size, 64-bit DMA
xhci0: xECP capabilities <PROTO,PROTO,LEGACY,DEBUG>
usbus0 on xhci0
xhci0: <Intel Sunrise Point-LP USB 3.0 controller> mem 0xdc310000-0xdc31ffff at device 20.0 on pci0
xhci0: 32 bytes context size, 64-bit DMA
xhci0: xECP capabilities <PROTO,PROTO,LEGACY,DEBUG>
usbus0 on xhci0
hw.model: Intel(R) Core(TM) i7-7560U CPU @ 2.40GHz
xhci0@pci0:0:20:0:      class=0x0c0330 rev=0x21 hdr=0x00 vendor=0x8086 device=0x9d2f subvendor=0x1028 subdevice=0x075b

remove null check for sc

Herald added a subscriber: imp. · View Herald TranscriptJul 14 2025, 8:10 AM

Harbormaster completed remote builds in B65407: Diff 158453.Jul 14 2025, 8:10 AM

thj added reviewers: aokblast, hrs, emaste.Jul 14 2025, 8:10 AM

Looks fine to me, although I haven't looked at the doc in detail yet.

PROTO,PROTO,LEGACY,DEBUG

In a future commit we should decode the PROTO entries more.

sys/dev/usb/controller/xhci.c
684	Probably worth a more extensive comment

aokblast added inline comments.Aug 8 2025, 9:00 AM

sys/dev/usb/controller/xhci.c
668	the hccp1 is for capabilities register. Can we rename XHCI_HCSPARAMS0 to XHCI_HCCPARAMS1 to follow the intel's manual? Of course, it can be a separated patch.
693	According to the xhci rev 1.2. Reserved Capabilities code is from 19. 18 is for USB3 Tunnelling Support.

I have no objection to adding the xECP in the kernel message, but I would like to see them in a sysctl node like dev.xhci.N.xecp, too.

thj added inline comments.Aug 12 2025, 8:49 AM

sys/dev/usb/controller/xhci.c
693	I think we are reading different documents. eXtensible Host Controller Interface for Universal Serial Bus May 2019 Revision 1.2 Table 7-2: xHCI Extended Capability Codes, has codes 18-191 as reserved https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/extensible-host-controler-interface-usb-xhci.pdf

aokblast added inline comments.Aug 12 2025, 8:52 AM

sys/dev/usb/controller/xhci.c
693	Ah, yes. I am reading xHCI 1.2b not 1.2. 1.2b is from 2023. Do you need to latest docs or just let it go?
736	IMHO. The branch here is unnecessary?

thj added inline comments.Aug 12 2025, 12:49 PM

sys/dev/usb/controller/xhci.c
668	I've created a patch, and will post it for review once I fix my system enough that I can build things

hrs added inline comments.Aug 12 2025, 2:11 PM

sys/dev/usb/controller/xhci.c
694	How about simply adding `RES()` and `VEND()` even if no `bootverbose` flag? In my experience, these values are rarely found, and the flags line will not be chatty in most cases. If we are going to use this for diagnostic purposes, it would be good to have all values unconditionally.

Reference latest standard
Remove XXX comment, this isn't that unclear
Don't hide reserved and vendor print behind print verbose
Add extended MSI and usb tunnel capabilities

Harbormaster completed remote builds in B66211: Diff 160295.Aug 13 2025, 2:24 PM

thj added inline comments.Aug 13 2025, 3:51 PM

sys/dev/usb/controller/xhci.c
693	Ah ha! Do you have a link to a public version of the spec? I have found one which isn't distributable, but I would like a reference to update wikipedia
736	It simplifies the flow rather than setting first to false in 3 places. Do you have an alternative suggestion?

aokblast added inline comments.Aug 13 2025, 4:02 PM

sys/dev/usb/controller/xhci.c
736	I means the if (first) can be removed since it only needs to be true in the first time. We can always let first = false without the if statement when the code reaches here. It may saves some time on branch prediction.

thj marked an inline comment as done.Aug 14 2025, 8:32 AM

don't check first in loop

Harbormaster completed remote builds in B66227: Diff 160327.Aug 14 2025, 8:34 AM

LGTM now!

This revision is now accepted and ready to land.Aug 14 2025, 8:35 AM

remove null check for sc
Reference latest standard
Remove XXX comment, this isn't that unclear
Don't hide reserved and vendor print behind print verbose
Add extended MSI and usb tunnel capabilities
don't check first in loop
spell tun as usb3_tun (and correctly)

This revision now requires review to proceed.Sep 2 2025, 9:33 AM

Harbormaster completed remote builds in B66753: Diff 161362.Sep 2 2025, 9:33 AM

This revision was not accepted when it landed; it landed in state Needs Review.Sep 2 2025, 9:38 AM

Closed by commit rG405cfeef615f: xhci: Parse and print extended capabilities (authored by thj). · Explain Why

This revision was automatically updated to reflect the committed changes.

thj added a commit: rG405cfeef615f: xhci: Parse and print extended capabilities.

xhci: Parse and print extended capabilities (CFT)
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 161362

sys/dev/usb/controller/xhci.c

sys/dev/usb/controller/xhcireg.h

xhci: Parse and print extended capabilities (CFT)ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 161362

sys/dev/usb/controller/xhci.c

sys/dev/usb/controller/xhcireg.h

xhci: Parse and print extended capabilities (CFT)
ClosedPublic
Actions

Revision Contents
Changeset List