xhci: Parse and print extended capabilities (CFT)

[thj]

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

[kp]

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.

[me_jamesrm.com]

➜  ~ 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]

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

[kp]

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

[dch]

# 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

[thj]

• remove null check for sc

[emaste]

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]

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.

[hrs]

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]

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]

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]

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]

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.

[thj]

• 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

[thj]

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]

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]

• don't check first in loop

[aokblast]

LGTM now!

[thj]

• 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)