From: Jeff Vanhoof qjv001@motorola.com
arm-smmu related crashes seen after a Missed ISOC interrupt when no_interrupt=1 is used. This can happen if the hardware is still using the data associated with a TRB after the usb_request's ->complete call has been made. Instead of immediately releasing a request when a Missed ISOC interrupt has occurred, this change will add logic to cancel the request instead where it will eventually be released when the END_TRANSFER command has completed. This logic is similar to some of the cleanup done in dwc3_gadget_ep_dequeue.
Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status") Cc: stable@vger.kernel.org Signed-off-by: Jeff Vanhoof qjv001@motorola.com Co-developed-by: Dan Vacura w36195@motorola.com Signed-off-by: Dan Vacura w36195@motorola.com --- V1 -> V3: - no change, new patch in series V3 -> V4: - no change
drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/gadget.c | 38 ++++++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 8f9959ba9fd4..9b005d912241 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -943,6 +943,7 @@ struct dwc3_request { #define DWC3_REQUEST_STATUS_DEQUEUED 3 #define DWC3_REQUEST_STATUS_STALLED 4 #define DWC3_REQUEST_STATUS_COMPLETED 5 +#define DWC3_REQUEST_STATUS_MISSED_ISOC 6 #define DWC3_REQUEST_STATUS_UNKNOWN -1
u8 epnum; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 079cd333632e..411532c5c378 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2021,6 +2021,9 @@ static void dwc3_gadget_ep_cleanup_cancelled_requests(struct dwc3_ep *dep) case DWC3_REQUEST_STATUS_STALLED: dwc3_gadget_giveback(dep, req, -EPIPE); break; + case DWC3_REQUEST_STATUS_MISSED_ISOC: + dwc3_gadget_giveback(dep, req, -EXDEV); + break; default: dev_err(dwc->dev, "request cancelled with wrong reason:%d\n", req->status); dwc3_gadget_giveback(dep, req, -ECONNRESET); @@ -3402,21 +3405,32 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep, struct dwc3 *dwc = dep->dwc; bool no_started_trb = true;
- dwc3_gadget_ep_cleanup_completed_requests(dep, event, status); + if (status == -EXDEV) { + struct dwc3_request *tmp; + struct dwc3_request *req;
- if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) - goto out; + if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) + dwc3_stop_active_transfer(dep, true, true);
- if (!dep->endpoint.desc) - return no_started_trb; + list_for_each_entry_safe(req, tmp, &dep->started_list, list) + dwc3_gadget_move_cancelled_request(req, + DWC3_REQUEST_STATUS_MISSED_ISOC); + } else { + dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
- if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && - list_empty(&dep->started_list) && - (list_empty(&dep->pending_list) || status == -EXDEV)) - dwc3_stop_active_transfer(dep, true, true); - else if (dwc3_gadget_ep_should_continue(dep)) - if (__dwc3_gadget_kick_transfer(dep) == 0) - no_started_trb = false; + if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) + goto out; + + if (!dep->endpoint.desc) + return no_started_trb; + + if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && + list_empty(&dep->started_list) && list_empty(&dep->pending_list)) + dwc3_stop_active_transfer(dep, true, true); + else if (dwc3_gadget_ep_should_continue(dep)) + if (__dwc3_gadget_kick_transfer(dep) == 0) + no_started_trb = false; + }
out: /*
On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
From: Jeff Vanhoof qjv001@motorola.com
arm-smmu related crashes seen after a Missed ISOC interrupt when no_interrupt=1 is used. This can happen if the hardware is still using the data associated with a TRB after the usb_request's ->complete call has been made. Instead of immediately releasing a request when a Missed ISOC interrupt has occurred, this change will add logic to cancel the request instead where it will eventually be released when the END_TRANSFER command has completed. This logic is similar to some of the cleanup done in dwc3_gadget_ep_dequeue.
Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status") Cc: stable@vger.kernel.org Signed-off-by: Jeff Vanhoof qjv001@motorola.com Co-developed-by: Dan Vacura w36195@motorola.com Signed-off-by: Dan Vacura w36195@motorola.com
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
I need an ack from the dwc3 maintainer before I can take this one.
thanks,
greg k-h
Hi Greg,
On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
From: Jeff Vanhoof qjv001@motorola.com
arm-smmu related crashes seen after a Missed ISOC interrupt when no_interrupt=1 is used. This can happen if the hardware is still using the data associated with a TRB after the usb_request's ->complete call has been made. Instead of immediately releasing a request when a Missed ISOC interrupt has occurred, this change will add logic to cancel the request instead where it will eventually be released when the END_TRANSFER command has completed. This logic is similar to some of the cleanup done in dwc3_gadget_ep_dequeue.
Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status") Cc: stable@vger.kernel.org Signed-off-by: Jeff Vanhoof qjv001@motorola.com Co-developed-by: Dan Vacura w36195@motorola.com Signed-off-by: Dan Vacura w36195@motorola.com
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
I need an ack from the dwc3 maintainer before I can take this one.
thanks,
greg k-h
Thinh has rejected this version of the patch. He has provided an alternative implementation which has been testing well for us so far. Either Thinh or Dan will formalize this patch within the next few days. The latest proposed changes are:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index dfaf9ac24c4f..50287437d6de 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_SHORT && !chain) return 1;
+ if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain) + return 1; + if ((trb->ctrl & DWC3_TRB_CTRL_IOC) || (trb->ctrl & DWC3_TRB_CTRL_LST)) return 1; @@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, struct scatterlist *s; unsigned int num_queued = req->num_queued_sgs; unsigned int i; + bool missed_isoc = false; int ret = 0;
for_each_sg(sg, s, num_queued, i) { @@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, req->sg = sg_next(s); req->num_queued_sgs--;
+ if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC) + missed_isoc = true; + ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req, trb, event, status, true); if (ret) break; }
+ if (missed_isoc) + ret = 1; + return ret; }
Thanks, Jeff
On Sat, Oct 22, 2022, Jeff Vanhoof wrote:
Hi Greg,
On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
From: Jeff Vanhoof qjv001@motorola.com
arm-smmu related crashes seen after a Missed ISOC interrupt when no_interrupt=1 is used. This can happen if the hardware is still using the data associated with a TRB after the usb_request's ->complete call has been made. Instead of immediately releasing a request when a Missed ISOC interrupt has occurred, this change will add logic to cancel the request instead where it will eventually be released when the END_TRANSFER command has completed. This logic is similar to some of the cleanup done in dwc3_gadget_ep_dequeue.
Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status") Cc: stable@vger.kernel.org Signed-off-by: Jeff Vanhoof qjv001@motorola.com Co-developed-by: Dan Vacura w36195@motorola.com Signed-off-by: Dan Vacura w36195@motorola.com
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
I need an ack from the dwc3 maintainer before I can take this one.
thanks,
greg k-h
Thinh has rejected this version of the patch. He has provided an alternative implementation which has been testing well for us so far. Either Thinh or Dan will formalize this patch within the next few days. The latest proposed changes are:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index dfaf9ac24c4f..50287437d6de 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_SHORT && !chain) return 1;
if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)return 1;if ((trb->ctrl & DWC3_TRB_CTRL_IOC) || (trb->ctrl & DWC3_TRB_CTRL_LST)) return 1;@@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, struct scatterlist *s; unsigned int num_queued = req->num_queued_sgs; unsigned int i;
bool missed_isoc = false; int ret = 0;for_each_sg(sg, s, num_queued, i) { @@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, req->sg = sg_next(s); req->num_queued_sgs--;
if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)missed_isoc = true;ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req, trb, event, status, true); if (ret) break; }
if (missed_isoc)ret = 1;return ret;}
That's just a debug patch. I'll send out proper fix patches.
Thanks, Thinh
Hi Thinh,
On Mon, Oct 24, 2022 at 10:47:53PM +0000, Thinh Nguyen wrote:
On Sat, Oct 22, 2022, Jeff Vanhoof wrote:
Hi Greg,
On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
From: Jeff Vanhoof qjv001@motorola.com
arm-smmu related crashes seen after a Missed ISOC interrupt when no_interrupt=1 is used. This can happen if the hardware is still using the data associated with a TRB after the usb_request's ->complete call has been made. Instead of immediately releasing a request when a Missed ISOC interrupt has occurred, this change will add logic to cancel the request instead where it will eventually be released when the END_TRANSFER command has completed. This logic is similar to some of the cleanup done in dwc3_gadget_ep_dequeue.
Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status") Cc: stable@vger.kernel.org Signed-off-by: Jeff Vanhoof qjv001@motorola.com Co-developed-by: Dan Vacura w36195@motorola.com Signed-off-by: Dan Vacura w36195@motorola.com
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
I need an ack from the dwc3 maintainer before I can take this one.
thanks,
greg k-h
Thinh has rejected this version of the patch. He has provided an alternative implementation which has been testing well for us so far. Either Thinh or Dan will formalize this patch within the next few days. The latest proposed changes are:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index dfaf9ac24c4f..50287437d6de 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_SHORT && !chain) return 1;
if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)return 1;if ((trb->ctrl & DWC3_TRB_CTRL_IOC) || (trb->ctrl & DWC3_TRB_CTRL_LST)) return 1;@@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, struct scatterlist *s; unsigned int num_queued = req->num_queued_sgs; unsigned int i;
bool missed_isoc = false; int ret = 0; for_each_sg(sg, s, num_queued, i) {@@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, req->sg = sg_next(s); req->num_queued_sgs--;
if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)missed_isoc = true;ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req, trb, event, status, true); if (ret) break; }if (missed_isoc)ret = 1;return ret;}
That's just a debug patch. I'll send out proper fix patches.
Ping!
While digging out this thread, I did not find any followup patch for this suggestion. Did it hit the mailinglist anywhere?
If not, will you send one?
Regards, Michael
On Tue, Sep 19, 2023 at 11:10:55AM +0200, Michael Grzeschik wrote:
On Mon, Oct 24, 2022 at 10:47:53PM +0000, Thinh Nguyen wrote:
On Sat, Oct 22, 2022, Jeff Vanhoof wrote:
Hi Greg,
On Sat, Oct 22, 2022 at 01:31:24PM +0200, Greg Kroah-Hartman wrote:
On Tue, Oct 18, 2022 at 04:50:38PM -0500, Dan Vacura wrote:
From: Jeff Vanhoof qjv001@motorola.com
arm-smmu related crashes seen after a Missed ISOC interrupt when no_interrupt=1 is used. This can happen if the hardware is still using the data associated with a TRB after the usb_request's ->complete call has been made. Instead of immediately releasing a request when a Missed ISOC interrupt has occurred, this change will add logic to cancel the request instead where it will eventually be released when the END_TRANSFER command has completed. This logic is similar to some of the cleanup done in dwc3_gadget_ep_dequeue.
Fixes: 6d8a019614f3 ("usb: dwc3: gadget: check for Missed Isoc from event status") Cc: stable@vger.kernel.org Signed-off-by: Jeff Vanhoof qjv001@motorola.com Co-developed-by: Dan Vacura w36195@motorola.com Signed-off-by: Dan Vacura w36195@motorola.com
V1 -> V3:
- no change, new patch in series
V3 -> V4:
- no change
I need an ack from the dwc3 maintainer before I can take this one.
thanks,
greg k-h
Thinh has rejected this version of the patch. He has provided an alternative implementation which has been testing well for us so far. Either Thinh or Dan will formalize this patch within the next few days. The latest proposed changes are:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index dfaf9ac24c4f..50287437d6de 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3195,6 +3195,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_SHORT && !chain) return 1;
if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC && !chain)return 1;if ((trb->ctrl & DWC3_TRB_CTRL_IOC) || (trb->ctrl & DWC3_TRB_CTRL_LST)) return 1;@@ -3211,6 +3214,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, struct scatterlist *s; unsigned int num_queued = req->num_queued_sgs; unsigned int i;
bool missed_isoc = false; int ret = 0; for_each_sg(sg, s, num_queued, i) {@@ -3219,12 +3223,18 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep, req->sg = sg_next(s); req->num_queued_sgs--;
if (DWC3_TRB_SIZE_TRBSTS(trb->size) == DWC3_TRBSTS_MISSED_ISOC)missed_isoc = true;ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req, trb, event, status, true); if (ret) break; }if (missed_isoc)ret = 1;return ret;}
That's just a debug patch. I'll send out proper fix patches.
Ping!
While digging out this thread, I did not find any followup patch for this suggestion. Did it hit the mailinglist anywhere?
If not, will you send one?
Nevermind, I think I found the hunk in a variated version in this series.
https://lore.kernel.org/linux-usb/cover.1666735451.git.Thinh.Nguyen@synopsys...
Michael
linux-stable-mirror@lists.linaro.org
-
Dan Vacura -
Greg Kroah-Hartman -
Jeff Vanhoof -
Michael Grzeschik -
Thinh Nguyen