On 13.1.2025 12.16, Michał Pecio wrote:
Hi,
On Fri, 10 Jan 2025 14:35:50 +0200, Mathias Nyman wrote:
On 8.1.2025 16.28, Niklas Neronin wrote:
+ page_shift = readl(&xhci->op_regs->page_size) &
XHCI_PAGE_SIZE_MASK;
Should we check that page_shift value makes sense here?
Maybe it would make sense to validate it. Interpreting PAGESIZE wrong
is potentially dangerous, because the xHC will assume that scratchpad
buffers are of this size and it can write to them whatever it wants.
Before the buggy ffs() patch 81720ec5320c, the driver used to pick the
lowest set bit or warn if all are zero, but then it still ignored the
calculated size and used 4K.
I would probably be safer to use the highest bit, or just reject the
xHC if it sets multiple bits (5.4.3 says: "the supported page size",
not "a bitmask of supported sizes").
Checking that one, and only one bit is set sounds good. If so then use
that. Otherwise print a warning and use 4k page size.
This to avoid regression. I don't know why the page size was hardcoded
to 4k originally even if we first do all the gymnastics to read it from
hardware. But it was done for some reason.
0xffffffff looks like a brain dead chip and not going to work anyway.
Reading 0xffffffff from a PCI device mmio registers is possible
if host is suddenly PCI hotplug removed (some Intel Alpine Ridge xHC), or
if I remember correctly also in PCI D3Cold power state.
We used to hardcode page_size to 4k, and don't really know if all xHC
vendors have a sane op_regs->page_size value.
FWIW, all of mine report 4K as per debugfs:
/sys/kernel/debug/usb/xhci/0000:00:10.0/reg-op:PAGESIZE = 0x00000001
/sys/kernel/debug/usb/xhci/0000:02:00.0/reg-op:PAGESIZE = 0x00000001
...
00:10.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI Controller (rev 20)
02:00.0 USB controller: Advanced Micro Devices, Inc. [AMD] 300 Series Chipset USB 3.1 xHCI Controller (rev 02)
06:00.0 USB controller: ASMedia Technology Inc. ASM1142 USB 3.1 Host Controller
09:00.0 USB controller: Renesas Electronics Corp. uPD720202 USB 3.0 Host Controller (rev 02)
0a:00.0 USB controller: Etron Technology, Inc. EJ168 USB 3.0 Host Controller (rev 01)
0b:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 03)
0c:00.0 USB controller: VIA Technologies, Inc. VL805/806 xHCI USB 3.0 Controller (rev 01)
Also ASM1042 and ASM3142.
And I have an NVIDIA Tegra board which runs some antique kernel and
doesn't warn, so PAGESIZE must at least be non-zero there.
Thanks, good to know these hosts work fine.
Thanks
Mathias
