linux-kernel - Re: [PATCH v27 01/32] xhci: add helper to stop endpoint and wait for completion

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: <dec459d3-fa0d-4ed0-ad6b-4a976b0f9e49@quicinc.com>
Date: Tue, 24 Sep 2024 14:47:54 -0700
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Michał Pecio <michal.pecio@...il.com>
CC: <mathias.nyman@...ux.intel.com>, <Thinh.Nguyen@...opsys.com>,
        <alsa-devel@...a-project.org>, <bgoswami@...cinc.com>,
        <broonie@...nel.org>, <conor+dt@...nel.org>, <corbet@....net>,
        <devicetree@...r.kernel.org>, <dmitry.torokhov@...il.com>,
        <gregkh@...uxfoundation.org>, <krzk+dt@...nel.org>,
        <lgirdwood@...il.com>, <linux-arm-msm@...r.kernel.org>,
        <linux-doc@...r.kernel.org>, <linux-input@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-sound@...r.kernel.org>,
        <linux-usb@...r.kernel.org>, <mathias.nyman@...el.com>,
        <perex@...ex.cz>, <pierre-louis.bossart@...ux.intel.com>,
        <robh@...nel.org>, <srinivas.kandagatla@...aro.org>, <tiwai@...e.com>
Subject: Re: [PATCH v27 01/32] xhci: add helper to stop endpoint and wait for
 completion

Hi Michal

On 9/22/2024 4:23 PM, Michał Pecio wrote:
> Hi,
>
>> So what I ended up doing was to split off the context error handling
>> into a separate helper API, which can be also called for the sync ep
>> stop API.  From there, based on say....the helper re queuing the stop
>> EP command, it would return a specific value to signify that it has
>> done so.  The sync based API will then re-wait for the completion of
>> the subsequent stop endpoint command that was queued.
> AFAIK retries are only necessary on buggy hardware. I don't see them on
> my controllers except for two old ones, both with the same buggy chip.
>
>>  In all other context error cases, it'd return the error to the caller,
>> and its up to them to handle it accordingly.
> For the record, all existing callers end up ignoring this return value.
>
> Honestly, I don't know if improving this function is worth your effort
> if it's working for you as-is. There are no users except xhci-sideband
> and probably shouldn't be - besides failing to fix stalled endpoints,
> this function also does nothing to prevent automatic restart of the EP
> when new URBs are submitted through xhci_hcd, so it is mainly relevant
> for sideband users who never submit URBs the usual way.
>
> My issue with this function is that it is simply poorly documented what
> it is or isn't expected to achieve (both here and in the calling code
> in xhci-sideband.c), and the changelog message is wrong to suggest that
> the default completion handler will run (unless somewhere there are
> patches to make it happen), making it look like this code can do things
> that it really cannot do. And this is apparently a public, exported API.

Thanks for the clarifications.  Yes, unfortunately, I can't really test any scenarios where this would be exercised in the current path, so I will leave the code out for now, and just add some comments and updates to the commit message.  Can revisit when there is some other users for utilizing secondary interrupters.

Thanks

Wesley Cheng