linux-kernel - Re: [PATCH v12 04/41] usb: host: xhci-mem: Cleanup pending secondary event ring events

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: <ae64ce69-dc1b-1534-7950-0a35c4a56f58@quicinc.com>
Date: Tue, 16 Jan 2024 12:24:08 -0800
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Mathias Nyman <mathias.nyman@...ux.intel.com>,
        <srinivas.kandagatla@...aro.org>, <mathias.nyman@...el.com>,
        <perex@...ex.cz>, <conor+dt@...nel.org>, <corbet@....net>,
        <gregkh@...uxfoundation.org>, <lgirdwood@...il.com>,
        <andersson@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
        <konrad.dybcio@...aro.org>, <Thinh.Nguyen@...opsys.com>,
        <broonie@...nel.org>, <bgoswami@...cinc.com>, <tiwai@...e.com>,
        <robh+dt@...nel.org>, <agross@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-sound@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <alsa-devel@...a-project.org>
Subject: Re: [PATCH v12 04/41] usb: host: xhci-mem: Cleanup pending secondary
 event ring events

Hi Mathias,

On 1/15/2024 6:01 AM, Mathias Nyman wrote:
> On 10.1.2024 1.42, Wesley Cheng wrote:
>> Hi Mathias,
>>
>> On 1/8/2024 12:51 PM, Wesley Cheng wrote:
>>> Hi Mathias,
>>>
>>> On 1/4/2024 6:48 AM, Mathias Nyman wrote:
>>>> On 2.1.2024 23.45, Wesley Cheng wrote:
>>>>> As part of xHCI bus suspend, the XHCI is halted.  However, if there 
>>>>> are
>>>>> pending events in the secondary event ring, it is observed that the 
>>>>> xHCI
>>>>> controller stops responding to further commands upon host or device
>>>>> initiated bus resume.  Iterate through all pending events and 
>>>>> update the
>>>>> dequeue pointer to the beginning of the event ring.
>>>>>
>>>>> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
>>>> ...
>>>>> +/*
>>>>> + * Move the event ring dequeue pointer to skip events kept in the 
>>>>> secondary
>>>>> + * event ring.  This is used to ensure that pending events in the 
>>>>> ring are
>>>>> + * acknowledged, so the XHCI HCD can properly enter 
>>>>> suspend/resume. The
>>>>> + * secondary ring is typically maintained by an external component.
>>>>> + */
>>>>> +void xhci_skip_sec_intr_events(struct xhci_hcd *xhci,
>>>>> +    struct xhci_ring *ring,    struct xhci_interrupter *ir)
>>>>> +{
>>>>> +    union xhci_trb *erdp_trb, *current_trb;
>>>>> +    u64 erdp_reg;
>>>>> +    u32 iman_reg;
>>>>> +    dma_addr_t deq;
>>>>> +
>>>>> +    /* disable irq, ack pending interrupt and ack all pending 
>>>>> events */
>>>>> +    xhci_disable_interrupter(ir);
>>>>> +    iman_reg = readl_relaxed(&ir->ir_set->irq_pending);
>>>>> +    if (iman_reg & IMAN_IP)
>>>>> +        writel_relaxed(iman_reg, &ir->ir_set->irq_pending);
>>>>> +
>>>>> +    /* last acked event trb is in erdp reg  */
>>>>> +    erdp_reg = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
>>>>> +    deq = (dma_addr_t)(erdp_reg & ERST_PTR_MASK);
>>>>> +    if (!deq) {
>>>>> +        xhci_err(xhci, "event ring handling not required\n");
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    erdp_trb = current_trb = ir->event_ring->dequeue;
>>>>> +    /* read cycle state of the last acked trb to find out CCS */
>>>>> +    ring->cycle_state = le32_to_cpu(current_trb->event_cmd.flags) 
>>>>> & TRB_CYCLE;
>>>>> +
>>>>> +    while (1) {
>>>>> +        inc_deq(xhci, ir->event_ring);
>>>>> +        erdp_trb = ir->event_ring->dequeue;
>>>>> +        /* cycle state transition */
>>>>> +        if ((le32_to_cpu(erdp_trb->event_cmd.flags) & TRB_CYCLE) !=
>>>>> +            ring->cycle_state)
>>>>> +            break;
>>>>> +    }
>>>>> +
>>>>> +    xhci_update_erst_dequeue(xhci, ir, current_trb, true);
>>>>> +}
>>>>
>>>> Code above is very similar to the existing event ring processing 
>>>> parts of xhci_irq()
>>>> and xhci_handle_event()
>>>>
>>>> I'll see if I can refactor the existing event ring processing, 
>>>> decouple it from
>>>> event handling so that it could be used by primary and secondary 
>>>> interrupters with
>>>> handlers, and this case where we just want to clear the event ring.
>>>>
>>>
>>> Thanks, that makes sense.  Will take a look as well.
>>>
>>
>> How about something like the below?  Tested this on my set up and 
>> everything looks to be working fine.  Had to add another param to 
>> struct xhci_interrupters to tell the XHCI interrupt handler to say if 
>> that particular interrupter wants to skip_events (handling).  This 
>> way, its something that the class driver utilizing the interrupter 
>> will have to tell XHCI sideband.  It would allow the user to determine 
>> if they want to use the interrupter to actually handle events or not 
>> on the proc running Linux.
>>
> 
> Yes, I have something similar.
> I'll share it soon, just need to
> clean it up a bit fist.
> 

Sure, no worries.  Will test it when its available.  Thanks!

Thanks
Wesley Cheng