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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 18 Jan 2022 14:50:43 +0800
From:   Puma Hsu <pumahsu@...gle.com>
To:     Mathias Nyman <mathias.nyman@...ux.intel.com>
Cc:     Greg KH <gregkh@...uxfoundation.org>, 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

On Mon, Jan 17, 2022 at 8:20 PM Mathias Nyman
<mathias.nyman@...ux.intel.com> wrote:
>
>
> >>> 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.

I will print a debug message in next patch version.

> 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

Maybe I can try to make this in the future. Thank you for advising.

> -Mathias