linux-kernel - Re: [PATCH v7 01/33] xhci: add support to allocate several interrupters

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

Message-ID: <2127ef61-e263-2a0e-438a-6baa125aa70d@quicinc.com>
Date:   Wed, 4 Oct 2023 11:35:06 -0700
From:   Wesley Cheng <quic_wcheng@...cinc.com>
To:     Mathias Nyman <mathias.nyman@...el.com>,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        <gregkh@...uxfoundation.org>, <lgirdwood@...il.com>,
        <broonie@...nel.org>, <perex@...ex.cz>, <tiwai@...e.com>,
        <agross@...nel.org>, <andersson@...nel.org>,
        <konrad.dybcio@...aro.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
        <srinivas.kandagatla@...aro.org>, <bgoswami@...cinc.com>,
        <Thinh.Nguyen@...opsys.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <alsa-devel@...a-project.org>, <linux-arm-msm@...r.kernel.org>,
        <devicetree@...r.kernel.org>
Subject: Re: [PATCH v7 01/33] xhci: add support to allocate several
 interrupters

Hi Mathias,

On 10/4/2023 7:02 AM, Mathias Nyman wrote:
> On 2.10.2023 23.07, Wesley Cheng wrote:
>> Hi Mathias,
>>
>> On 9/28/2023 3:31 AM, Mathias Nyman wrote:
>>> On 22.9.2023 0.48, Wesley Cheng wrote:
>>>> From: Mathias Nyman <mathias.nyman@...ux.intel.com>
>>>>
>>>> Modify the XHCI drivers to accommodate for handling multiple event 
>>>> rings in
>>>> case there are multiple interrupters.  Add the required APIs so 
>>>> clients are
>>>> able to allocate/request for an interrupter ring, and pass this 
>>>> information
>>>> back to the client driver.  This allows for users to handle the 
>>>> resource
>>>> accordingly, such as passing the event ring base address to an audio 
>>>> DSP.
>>>> There is no actual support for multiple MSI/MSI-X vectors.
>>>>
>>>> Factoring out XHCI interrupter APIs and structures done by Wesley 
>>>> Cheng, in
>>>> order to allow for USB class drivers to utilze them.
>>>>
>>>>   }
>>>> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct 
>>>> xhci_interrupter *ir)
>>>> +{
>>>> +    struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>>> +    unsigned int intr_num;
>>>> +
>>>> +    /* interrupter 0 is primary interrupter, don't touchit */
>>>> +    if (!ir || !ir->intr_num || ir->intr_num >= 
>>>> xhci->max_interrupters) {
>>>> +        xhci_dbg(xhci, "Invalid secondary interrupter, can't 
>>>> remove\n");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    /* fixme, should we check xhci->interrupter[intr_num] == ir */
>>>> +    spin_lock(&xhci->lock);
>>>
>>> Needs to be spin_lock_irq() ir spin_lock_irqsave() as xhci->lock is 
>>> used in interrupt handler.
>>>
>>>
>>>> +    intr_num = ir->intr_num;
>>>> +    xhci_free_interrupter(xhci, ir);
>>>> +    xhci->interrupters[intr_num] = NULL;
>>>> +    spin_unlock(&xhci->lock);
>>>
>>> likewise
>>>
>>
>> Let me check these again.  In general, I think I will use both the 
>> xhci->mutex and xhci->lock where needed, because I believe we'd run 
>> into sleep while atomic issues
>> while freeing the DMA memory.  Will rework this and submit in the next 
>> rev.
>>
> 
> Maybe we need to split xhci_free_interrupter() into separate remove and 
> free functions
> 

Thanks for sharing the work you've been doing.  Yes, I did something 
similar as well on my end, but will refactor in your code and re-test.

> Did some work on this, and on the sideband api in general.
> 
> Code still has a lot of FIXMEs, and it's completely untested, but to 
> avoid us
> from doing duplicate work I pushed it to my feature_interrupters branch 
> anyway
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git 
> feature_interrupters
> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters 
> 

Ok.  Initial look at it seems like it will be fine, but will integrate 
and make changes where needed.

Thanks
Wesley Cheng