Re: [PATCH] usb: dwc3: Do not process request if HWO is set for its TRB

[Sriharsha Allenki <sallenki@codeaurora.org>]

Hi Felipe,

On 12/3/2019 6:00 PM, Felipe Balbi wrote:
> Hi,
>
> Sriharsha Allenki <sallenki@codeaurora.org> writes:
>
>> This case occurs only after the first TRB of the chain is processed,
>> which we arechecking in the patch. Although, this piece of code has
>> been no-op after introducingthe function
>> "dwc3_gadget_ep_reclaim_trb_sg".This function checks for the HWO and
>> does notcall the "dwc3_gadget_ep_reclaim_completed_trb" if it is
>> set.Hence this condition mostly likely will never hit.
> You're missing one important detail: If we have e.g. 200 TRBs in a
> single SG-list and we receive a short packet on TRB 10, we will have 190
> TRBs with HWO bit left set and your patch prevents the driver from
> clearing that bit. Yes, you are regressing a very special case.

Iam checking only the first TRB of the chain and not the TRB pointed
by the current dequeue pointer.
>
>>> what problem you actually found? Preferrably with tracepoint data
>>> showing the fault.
>> Test case here involves f_fs driver in AIO mode and we see ~8 TRBs in
>> the queue with HWO set and UPDATE_XFER done. In the failure case I see
>> thatas part of processingthe interrupt generated by the core for the
>> completion of the first TRB, the driver isgoing ahead and giving
> we shouldn't get completion interrupt for the first TRB, only the
> last. Care to share tracepoint data?

We have seen the issue only once and we do not have any tracepoint
data for it. But with the internal logging we have in our downstream code,
I see a race between dequeue from the function driver, and the giveback
as part of the completion (XferInProgress).

A request (say Request-1) is dequeued before we could notify it's
completion to the gadget driver. Because of this, as part of handling
the completion event for the Request-1 we gaveback the next
request(Request-2) in the queue which is yet to be processed by the
core leading to the mentioned SMMU fault.

Normally, the core should not process the TRBs once a request
has been dequeued because of the stop_active_transfer as part of
dequeue, but I see a timeout when issuing the end transfer command
during dequeue because of which core is still processing the TRBs
in the queue.

Regards,
Sriharsha

>
>> backthe requests of all theother queued TRBs, whichinvolves removing
>> the SMMU mapping of the buffers associated with the requests.  But
>> these are still active and when core processesthese TRBs and their
>> correspondingun-mapped buffers, I see a translationfaultraised by the
>> SMMU.
>>
>> I hope I have answered your queries, please let me know if I am still 
>> missing something here.
> yes, tracepoint data showing the problem. Thank you
>