linux-kernel - Re: [PATCH v3] xhci: re-initialize the HC during resume if HCE was set

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <fe411488-c67c-62ab-4709-98621b9f199b@linux.intel.com>
Date:   Mon, 17 Jan 2022 14:22:10 +0200
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Puma Hsu <pumahsu@...gle.com>, Greg KH <gregkh@...uxfoundation.org>
Cc:     mathias.nyman@...el.com, Sergey Shtylyov <s.shtylyov@....ru>,
        Albert Wang <albertccwang@...gle.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH v3] xhci: re-initialize the HC during resume if HCE was
 set

>>> This seems like a big hammer for when the host controller throws an
>>>> error.  Why is this the only place that it should be checked for?  What
>>>> caused the error that can now allow it to be fixed?
>>>
>>> I believe this is not the only place that the host controller may set
>>> HCE, the host controller may set HCE anytime it sees an error in my
>>> opinion, not only in suspend or resume.
>>
>> Then where else should it be checked?  Where else will your silicon set
>> this bit as part of the normal operating process?
> 
> We observed this flag while resume in our silicon so far. According to the XHCI
> specification 4.24.1, “Software should implement an algorithm for checking the
> HCE flag if the xHC is not responding.”, so maybe it would be better
> to implement
> a new API to recover host controller whenever the driver side finds no response
> from host controller in the future.
> 

As all the code to reset the host during resume already exists, and is well tried
due to issues in resume being so common, I think it makes sense to add the HCE case 
here as well. It's a simple fix that makes the life of users better.

That said we shouldn't hide the reason for reset like this.
Print a debug message telling about the HCE so that everybody working with xHCI
can see it and start fixing the rootcause.

Another HCE check could be added to command timeout code, but just to show a
warning for now.
Reset might not always clear HCE, and we don't want to be stuck in a reset loop.
This check  could be a separate patch

-Mathias