mbox series

[v3,0/2] usb: dwc3: gadget: Fix isoc interrupt check

Message ID cover.1666735451.git.Thinh.Nguyen@synopsys.com
Headers show
Series usb: dwc3: gadget: Fix isoc interrupt check | expand

Message

Thinh Nguyen Oct. 25, 2022, 10:10 p.m. UTC 
Fix issues where usb_request->no_interrupt is set for isoc transfers.
* Make sure no interrupt is asserted if no_interrupt is set
* Make sure to stop reclaiming TRBs when the driver needs to stop


Thinh Nguyen (2):
  usb: dwc3: gadget: Stop processing more requests on IMI
  usb: dwc3: gadget: Don't set IMI for no_interrupt

 drivers/usb/dwc3/gadget.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Changes in v3:
* Only set IMI to the last TRB of the chained TRBs

Changes in v2:
* Fix missing cover letter subject
* No need to check for the last chained TRB

base-commit: fb8f60dd1b67520e0e0d7978ef17d015690acfc1

Comments

Jeff Vanhoof Oct. 26, 2022, 8:24 a.m. UTC | #1 
On Tue, Oct 25, 2022 at 03:10:14PM -0700, Thinh Nguyen wrote:
> When servicing a transfer completion event, the dwc3 driver will reclaim
> TRBs of started requests up to the request associated with the interrupt
> event. Currently we don't check for interrupt due to missed isoc, and
> the driver may attempt to reclaim TRBs beyond the associated event. This
> causes invalid memory access when the hardware still owns the TRB. If
> there's a missed isoc TRB with IMI (interrupt on missed isoc), make sure
> to stop servicing further.
> 
> Note that only the last TRB of chained TRBs has its status updated with
> missed isoc.
> 
> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
> Cc: stable@vger.kernel.org
> Reported-by: Jeff Vanhoof <jdv1029@gmail.com>
> Reported-by: Dan Vacura <w36195@motorola.com>
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
>  Changes in v3:
>  - None
>  Changes in v2:
>  - No need to check for CHN=0 since only the last TRB has its status
>  updated to missed isoc
> 
> 
>  drivers/usb/dwc3/gadget.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index dd8ecbe61bec..230b3c660054 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3248,6 +3248,10 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>  	if (event->status & DEPEVT_STATUS_SHORT && !chain)
>  		return 1;
>  
> +	if ((trb->ctrl & DWC3_TRB_CTRL_ISP_IMI) &&
> +	    DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)
> +		return 1;
> +
>  	if ((trb->ctrl & DWC3_TRB_CTRL_IOC) ||
>  	    (trb->ctrl & DWC3_TRB_CTRL_LST))
>  		return 1;
> -- 
> 2.28.0
>

No new issues seen with these changes. Changes look good to me.

Reviewed-by: Jeff Vanhoof <jdv1029@gmail.com>
Tested-by: Jeff Vanhoof <jdv1029@gmail.com> 

Regards,
Jeff