linux-kernel - Re: [PATCH 0/2] Allow disabling USB3 ports in xHCI/DWC3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f3342e07-4d35-7714-ad85-5c5c9ed9b25b@linux.intel.com>
Date: Mon, 18 Dec 2023 17:40:20 +0200
From: Mathias Nyman <mathias.nyman@...ux.intel.com>
To: Sam Edwards <cfsworks@...il.com>, Mathias Nyman
 <mathias.nyman@...el.com>, Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Heiko Stuebner <heiko@...ech.de>, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/2] Allow disabling USB3 ports in xHCI/DWC3

On 15.12.2023 23.59, Sam Edwards wrote:
> Hi Mathias,
> 
> On 12/14/23 04:05, Mathias Nyman wrote:
>> I don't think this will work as a generic xhci driver feature.
>>
>> Even if we ignore all USB3 ports in software they will for most xHC hosts be powered
>> and enabled in hardware by default after controller reset.
>>
>> This means they perform link training, generate all kinds of events with interrupts
>> (connect, over-current etc) that driver now can't handle.
> 
> By this do you mean that having the xHCI driver ignore the USB3 ports isn't enough to ensure that PP=0 (and the driver would need to do a little bit more to make sure that the "parking brake" is on: e.g. initialize, but not use, the ports) or that the xHC's PP=0 signal isn't sufficient to keep the PHYs from trying to bring the link up and generating those interrupts (PP=0 really isn't enough, and there is no general "parking brake" to be found here)?
> 

Yes, in most cases PP==1 after xHC reset, here's some old debug output during boot:

[    2.571057] xhci_hcd 0000:00:0d.0: new USB bus registered, assigned bus number 2
[    2.571061] xhci_hcd 0000:00:0d.0: Host supports USB 3.2 Enhanced SuperSpeed
[    2.571065] xhci_hcd 0000:00:0d.0: // Turn on HC, cmd = 0x5.
[    2.571067] xhci_hcd 0000:00:0d.0: Finished xhci_run for USB3 roothub
[    2.571093] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 5.15
[    2.571095] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[    2.571097] usb usb2: Product: xHCI Host Controller
[    2.571098] usb usb2: Manufacturer: Linux 5.15.57-06982-gf7339f7585d8-dirty xhci-hcd
[    2.571099] usb usb2: SerialNumber: 0000:00:0d.0
[    2.571197] xHCI xhci_add_endpoint called for root hub
[    2.571199] xHCI xhci_check_bandwidth called for root hub
[    2.571224] hub 2-0:1.0: USB hub found
[    2.571230] hub 2-0:1.0: 2 ports detected
[    2.571279] xhci_hcd 0000:00:0d.0: set port power 2-1 ON, portsc: 0x2a0

Note that portsc: 0x2a0 entry above has PP=1, and it shows the portsc register value
_before_  port power is set to 1 (ON).

Port Status: 0x2a0
	Disconnected
	Disabled
	Link: Rx Detect
	Powered
	Unknown port speed

Forcing PP=0 could possibly prevent any events from those ports.

>> Sound like the setup you are using has a very specific issue, and it would need
>> a narrow targeted quirk to solve it.
> 
> I infer from this that you're against having a DT property added to xHCI? What if the property were to be narrowed in scope to "ignore the USB3 PHYs, they're disabled/absent" vs. this iteration's "disable the USB3 ports" meaning?
> 
> If this quirk ends up landing in the dwc3 driver (since, arguably, DWC3 is the real misbehaving hw block in these circumstances), what would be your preferred mechanism of signaling to the xHCI layer "the USB3 PHYs have been disabled; please ignore"?

I don't have a good solution in mind for this so I'll just throw some ideas:

What happends if you in some RK3588 platform code disable USB  ports via those
syscon registers, but let the xhci driver be, and USB3 roothub enumerates normally?

Or if this is about a misbehaving USB3 PHY, how about adding the USB3 PHY driver that
describes reality and fails when usb_phy_roothub_init() or usb_phy_roothub_set_mode()
are called by for USB3 hcd during usb_add_hcd(USB3).
xhci driver could then continue without the USB3 hcd. turning off USB3 ports.
      
Adding the 'maximum-speed = "high-speed"' entry could also be one option.

Thanks
Mathias