linux-kernel - Re: [PATCH] usb: xhci: Increase timeout for HC halt

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: <c1b24d1a-497d-a303-7b54-e1d65402f08c@gmail.com>
Date:   Tue, 11 May 2021 10:42:14 +0200
From:   Maximilian Luz <luzmaximilian@...il.com>
To:     Mathias Nyman <mathias.nyman@...ux.intel.com>,
        Mathias Nyman <mathias.nyman@...el.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: xhci: Increase timeout for HC halt

On 5/11/21 9:19 AM, Mathias Nyman wrote:
> On 11.5.2021 3.29, Maximilian Luz wrote:
>> On some devices (specifically the SC8180x based Surface Pro X with
>> QCOM04A6) HC halt / xhci_halt() times out during boot. Manually binding
>> the xhci-hcd driver at some point later does not exhibit this behavior.
>> To work around this, double XHCI_MAX_HALT_USEC, which also resolves this
>> issue.
>>
>> Signed-off-by: Maximilian Luz <luzmaximilian@...il.com>
>> ---
>>   drivers/usb/host/xhci-ext-caps.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
>> index fa59b242cd51..fb591e41cd50 100644
>> --- a/drivers/usb/host/xhci-ext-caps.h
>> +++ b/drivers/usb/host/xhci-ext-caps.h
>> @@ -7,8 +7,8 @@
>>    * Author: Sarah Sharp
>>    * Some code borrowed from the Linux EHCI driver.
>>    */
>> -/* Up to 16 ms to halt an HC */
>> -#define XHCI_MAX_HALT_USEC	(16*1000)
>> +/* Up to 32 ms to halt an HC */
>> +#define XHCI_MAX_HALT_USEC	(32 * 1000)
> 
> xHCI spec has a 16ms limit stated in several places, for example section 5.4.1
> "xHC is forced to halt within 16 ms. of software clearing the R/S bit to ‘0’,
> irrespective of any queued Transfer or Command Ring activity"

Right, thanks, I wasn't aware of this.

> To make sure hosts work we could increase it to 32, but comment could be
> changed to make sure it doean't get optimized back to 16 ms later.
> 
> How about:
> /* HC should halt within 16 ms, but use 32 ms as some in reality take longer */
> 
> If that's ok I can take this and modify the comment

That makes sense, yes. Please feel free to change that comment as you wish.

Regards,
Max