* [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count [not found] <20260727092258.1121-1-liujiazi@amazon.com> @ 2026-08-12 8:21 ` Jiazi Liu 2026-08-12 9:48 ` Krishna Kurapati 2026-08-28 2:36 ` Thinh Nguyen 0 siblings, 2 replies; 5+ messages in thread From: Jiazi Liu @ 2026-08-12 8:21 UTC (permalink / raw) To: Thinh.Nguyen, gregkh Cc: linux-usb, linux-kernel, stable, lkp, oe-kbuild-all, Jiazi Liu, Jiazi Liu From: Jiazi Liu <jiazi.liu1984@gmail.com> When dwc3_check_event_buf() reads a GEVNTCOUNT value exceeding the event buffer length, commit 63ccd26cd1f6 ("usb: dwc3: gadget: check that event count does not exceed event buffer length") returns IRQ_NONE without writing back GEVNTCOUNT. Since the DWC3 interrupt is level-triggered, the uncleared IRQ source keeps the line asserted, causing a tight IRQ storm that accumulates 99,900 unhandled interrupts and triggers spurious.c:184 BUG -> kernel panic. The resulting call stack: __report_bad_irq+0xac/0xc8 note_interrupt+0x340/0x468 handle_irq_event+0xac/0xc0 handle_fasteoi_irq+0x120/0x228 gic_handle_irq+0x68/0x108 ... kernel BUG at kernel/irq/spurious.c:184 To reproduce, write a bogus value exceeding the event buffer length directly to the GEVNTCOUNT register: devmem <DWC3_BASE + 0xc40c> 4 0x1004 Write the bogus count back to GEVNTCOUNT to clear the IRQ source, consistent with the stale event clearing pattern in dwc3_event_buffers_setup(), and schedule error recovery to reinitialize the controller. Fixes: 63ccd26cd1f6 ("usb: dwc3: gadget: check that event count does not exceed event buffer length") Cc: stable@vger.kernel.org Signed-off-by: Jiazi Liu <liujiazi@amazon.com> --- Changes in v2: - Rename softcon_work to err_recovery_work - Use dev_err instead of dev_err_ratelimited - Update comment to document driver/controller out-of-sync fatal error condition rather than describing IRQ storm behavior - Remove dev_err from work handler; error message belongs at the call site where the specific failure is known - Follow dwc3_gadget_suspend/resume logic in recovery work handler - Add err_dying state to reject gadget driver requests during recovery - Add err_recovery_count counter and stop scheduling recovery after DWC3_ERR_RECOVERY_MAX attempts to prevent infinite retry loops - Add synchronize_irq() before soft_disconnect to ensure IRQ handler has completed before reinitializing the controller - Define DWC3_ERR_RECOVERY_MAX in core.h alongside other global constants - Fix sparse warning: use dwc as first argument to dwc3_writel, not dwc->regs --- drivers/usb/dwc3/core.h | 7 +++++++ drivers/usb/dwc3/gadget.c | 41 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 608daeb7ef10..b1cd888eca77 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -49,6 +49,7 @@ #define DWC3_ENDPOINTS_NUM 32 #define DWC3_XHCI_RESOURCES_NUM 2 #define DWC3_ISOC_MAX_RETRIES 5 +#define DWC3_ERR_RECOVERY_MAX 3 #define DWC3_SCRATCHBUF_SIZE 4096 /* each buffer is assumed to be 4KiB */ #define DWC3_EVENT_BUFFERS_SIZE 4096 @@ -1004,6 +1005,7 @@ struct dwc3_glue_ops { /** * struct dwc3 - representation of our controller * @drd_work: workqueue used for role swapping + * @err_recovery_work: workqueue used for controller error recovery * @ep0_trb: trb which is used for the ctrl_req * @bounce: address of bounce buffer * @setup_buf: used while precessing STD USB requests @@ -1171,6 +1173,7 @@ struct dwc3_glue_ops { * @wakeup_configured: set if the device is configured for remote wakeup. * @suspended: set to track suspend event due to U3/L2. * @susphy_state: state of DWC3_GUSB2PHYCFG_SUSPHY + DWC3_GUSB3PIPECTL_SUSPHY + * @err_dying: true when controller is in error recovery, reject all requests * before PM suspend. * @imod_interval: set the interrupt moderation interval in 250ns * increments or 0 to disable. @@ -1186,9 +1189,11 @@ struct dwc3_glue_ops { * @wakeup_pending_funcs: Indicates whether any interface has requested for * function wakeup in bitmap format where bit position * represents interface_id. + * @err_recovery_count: number of consecutive error recovery attempts */ struct dwc3 { struct work_struct drd_work; + struct work_struct err_recovery_work; struct dwc3_trb *ep0_trb; void *bounce; u8 *setup_buf; @@ -1420,6 +1425,7 @@ struct dwc3 { unsigned wakeup_configured:1; unsigned suspended:1; unsigned susphy_state:1; + unsigned err_dying:1; u16 imod_interval; @@ -1429,6 +1435,7 @@ struct dwc3 { struct dentry *debug_root; u32 gsbuscfg0_reqinfo; u32 wakeup_pending_funcs; + u32 err_recovery_count; }; #define INCRX_BURST_MODE 0 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index fa0f16ffafef..05f3ffab6e47 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2054,6 +2054,9 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request *request, int ret; + if (dwc->err_dying) + return -ESHUTDOWN; + spin_lock_irqsave(&dwc->lock, flags); ret = __dwc3_gadget_ep_queue(dep, req); spin_unlock_irqrestore(&dwc->lock, flags); @@ -4667,9 +4670,22 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) return IRQ_NONE; if (count > evt->length) { - dev_err_ratelimited(dwc->dev, "invalid count(%u) > evt->length(%u)\n", + dev_err(dwc->dev, "invalid count(%u) > evt->length(%u)\n", count, evt->length); - return IRQ_NONE; + /* + * This is a fatal error - the driver and controller are out of + * sync on which event has been consumed. Reinitializing the + * controller is required to recover. Write the bogus count back + * to GEVNTCOUNT to clear the IRQ source, consistent with the + * stale event clearing in dwc3_event_buffers_setup(), then + * schedule error recovery. + */ + dwc3_writel(dwc, DWC3_GEVNTCOUNT(0), count); + dwc->err_dying = true; + dwc->err_recovery_count++; + if (dwc->err_recovery_count <= DWC3_ERR_RECOVERY_MAX) + schedule_work(&dwc->err_recovery_work); + return IRQ_HANDLED; } evt->count = count; @@ -4729,6 +4745,25 @@ static void dwc_gadget_release(struct device *dev) kfree(gadget); } +static void dwc3_err_recovery_work(struct work_struct *work) +{ + struct dwc3 *dwc = container_of(work, struct dwc3, err_recovery_work); + int ret; + + synchronize_irq(dwc->irq_gadget); + + ret = dwc3_gadget_soft_disconnect(dwc); + if (ret) + return; + + dwc3_disconnect_gadget_sleepable(dwc); + + dwc->err_dying = false; + + if (dwc->softconnect) + dwc3_gadget_soft_connect(dwc); +} + /** * dwc3_gadget_init - initializes gadget related registers * @dwc: pointer to our controller context structure @@ -4772,6 +4807,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) } init_completion(&dwc->ep0_in_setup); + INIT_WORK(&dwc->err_recovery_work, dwc3_err_recovery_work); dwc->gadget = kzalloc_obj(struct usb_gadget); if (!dwc->gadget) { ret = -ENOMEM; @@ -4868,6 +4904,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc) if (!dwc->gadget) return; + cancel_work_sync(&dwc->err_recovery_work); dwc3_enable_susphy(dwc, true); usb_del_gadget(dwc->gadget); dwc3_gadget_free_endpoints(dwc); -- 2.50.1 (Apple Git-155) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count 2026-08-12 8:21 ` [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count Jiazi Liu @ 2026-08-12 9:48 ` Krishna Kurapati 2026-08-28 2:36 ` Thinh Nguyen 1 sibling, 0 replies; 5+ messages in thread From: Krishna Kurapati @ 2026-08-12 9:48 UTC (permalink / raw) To: Jiazi Liu, Thinh.Nguyen, gregkh Cc: linux-usb, linux-kernel, stable, lkp, oe-kbuild-all, Jiazi Liu On 8/12/2026 1:51 PM, Jiazi Liu wrote: > From: Jiazi Liu <jiazi.liu1984@gmail.com> > [...] > --- > drivers/usb/dwc3/core.h | 7 +++++++ > drivers/usb/dwc3/gadget.c | 41 +++++++++++++++++++++++++++++++++++++-- > 2 files changed, 46 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 608daeb7ef10..b1cd888eca77 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -49,6 +49,7 @@ > #define DWC3_ENDPOINTS_NUM 32 > #define DWC3_XHCI_RESOURCES_NUM 2 > #define DWC3_ISOC_MAX_RETRIES 5 > +#define DWC3_ERR_RECOVERY_MAX 3 > > #define DWC3_SCRATCHBUF_SIZE 4096 /* each buffer is assumed to be 4KiB */ > #define DWC3_EVENT_BUFFERS_SIZE 4096 > @@ -1004,6 +1005,7 @@ struct dwc3_glue_ops { > /** > * struct dwc3 - representation of our controller > * @drd_work: workqueue used for role swapping > + * @err_recovery_work: workqueue used for controller error recovery > * @ep0_trb: trb which is used for the ctrl_req > * @bounce: address of bounce buffer > * @setup_buf: used while precessing STD USB requests > @@ -1171,6 +1173,7 @@ struct dwc3_glue_ops { > * @wakeup_configured: set if the device is configured for remote wakeup. > * @suspended: set to track suspend event due to U3/L2. > * @susphy_state: state of DWC3_GUSB2PHYCFG_SUSPHY + DWC3_GUSB3PIPECTL_SUSPHY > + * @err_dying: true when controller is in error recovery, reject all requests > * before PM suspend. How about gadget_dying or gadget_died (similar to hc_died in xhci). The variable "err_dyring"is misleading. > * @imod_interval: set the interrupt moderation interval in 250ns > * increments or 0 to disable. > @@ -1186,9 +1189,11 @@ struct dwc3_glue_ops { > * @wakeup_pending_funcs: Indicates whether any interface has requested for > * function wakeup in bitmap format where bit position > * represents interface_id. > + * @err_recovery_count: number of consecutive error recovery attempts > */ > struct dwc3 { > struct work_struct drd_work; > + struct work_struct err_recovery_work; > struct dwc3_trb *ep0_trb; > void *bounce; > u8 *setup_buf; > @@ -1420,6 +1425,7 @@ struct dwc3 { > unsigned wakeup_configured:1; > unsigned suspended:1; > unsigned susphy_state:1; > + unsigned err_dying:1; > > u16 imod_interval; > > @@ -1429,6 +1435,7 @@ struct dwc3 { > struct dentry *debug_root; > u32 gsbuscfg0_reqinfo; > u32 wakeup_pending_funcs; > + u32 err_recovery_count; > }; > > #define INCRX_BURST_MODE 0 > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index fa0f16ffafef..05f3ffab6e47 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2054,6 +2054,9 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request *request, > > int ret; > > + if (dwc->err_dying) > + return -ESHUTDOWN; > + Not sure but should this check be added for all gadget_ops ? Regards, Krishna, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count 2026-08-12 8:21 ` [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count Jiazi Liu 2026-08-12 9:48 ` Krishna Kurapati @ 2026-08-28 2:36 ` Thinh Nguyen 2026-08-28 9:48 ` Liu Jiazi 1 sibling, 1 reply; 5+ messages in thread From: Thinh Nguyen @ 2026-08-28 2:36 UTC (permalink / raw) To: Jiazi Liu Cc: Thinh Nguyen, gregkh, linux-usb, linux-kernel, stable, lkp, oe-kbuild-all, Jiazi Liu Hi, Sorry for the delay response. On Wed, Aug 12, 2026, Jiazi Liu wrote: > From: Jiazi Liu <jiazi.liu1984@gmail.com> > > When dwc3_check_event_buf() reads a GEVNTCOUNT value exceeding the > event buffer length, commit 63ccd26cd1f6 ("usb: dwc3: gadget: check > that event count does not exceed event buffer length") returns IRQ_NONE > without writing back GEVNTCOUNT. Since the DWC3 interrupt is > level-triggered, the uncleared IRQ source keeps the line asserted, > causing a tight IRQ storm that accumulates 99,900 unhandled interrupts > and triggers spurious.c:184 BUG -> kernel panic. > > The resulting call stack: > __report_bad_irq+0xac/0xc8 > note_interrupt+0x340/0x468 > handle_irq_event+0xac/0xc0 > handle_fasteoi_irq+0x120/0x228 > gic_handle_irq+0x68/0x108 > ... > kernel BUG at kernel/irq/spurious.c:184 > > To reproduce, write a bogus value exceeding the event buffer length > directly to the GEVNTCOUNT register: > > devmem <DWC3_BASE + 0xc40c> 4 0x1004 > > Write the bogus count back to GEVNTCOUNT to clear the IRQ source, > consistent with the stale event clearing pattern in > dwc3_event_buffers_setup(), and schedule error recovery to > reinitialize the controller. > > Fixes: 63ccd26cd1f6 ("usb: dwc3: gadget: check that event count does not > exceed event buffer length") Fixes tag should be single line. > Cc: stable@vger.kernel.org > Signed-off-by: Jiazi Liu <liujiazi@amazon.com> Your email from Signed-off-by is mismatching your From: line above. > --- > Changes in v2: > - Rename softcon_work to err_recovery_work > - Use dev_err instead of dev_err_ratelimited > - Update comment to document driver/controller out-of-sync fatal error > condition rather than describing IRQ storm behavior > - Remove dev_err from work handler; error message belongs at the call > site where the specific failure is known > - Follow dwc3_gadget_suspend/resume logic in recovery work handler > - Add err_dying state to reject gadget driver requests during recovery > - Add err_recovery_count counter and stop scheduling recovery after > DWC3_ERR_RECOVERY_MAX attempts to prevent infinite retry loops > - Add synchronize_irq() before soft_disconnect to ensure IRQ handler > has completed before reinitializing the controller > - Define DWC3_ERR_RECOVERY_MAX in core.h alongside other global constants > - Fix sparse warning: use dwc as first argument to dwc3_writel, not dwc->regs > --- > drivers/usb/dwc3/core.h | 7 +++++++ > drivers/usb/dwc3/gadget.c | 41 +++++++++++++++++++++++++++++++++++++-- > 2 files changed, 46 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 608daeb7ef10..b1cd888eca77 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -49,6 +49,7 @@ > #define DWC3_ENDPOINTS_NUM 32 > #define DWC3_XHCI_RESOURCES_NUM 2 > #define DWC3_ISOC_MAX_RETRIES 5 > +#define DWC3_ERR_RECOVERY_MAX 3 > > #define DWC3_SCRATCHBUF_SIZE 4096 /* each buffer is assumed to be 4KiB */ > #define DWC3_EVENT_BUFFERS_SIZE 4096 > @@ -1004,6 +1005,7 @@ struct dwc3_glue_ops { > /** > * struct dwc3 - representation of our controller > * @drd_work: workqueue used for role swapping > + * @err_recovery_work: workqueue used for controller error recovery > * @ep0_trb: trb which is used for the ctrl_req > * @bounce: address of bounce buffer > * @setup_buf: used while precessing STD USB requests > @@ -1171,6 +1173,7 @@ struct dwc3_glue_ops { > * @wakeup_configured: set if the device is configured for remote wakeup. > * @suspended: set to track suspend event due to U3/L2. > * @susphy_state: state of DWC3_GUSB2PHYCFG_SUSPHY + DWC3_GUSB3PIPECTL_SUSPHY > + * @err_dying: true when controller is in error recovery, reject all requests > * before PM suspend. > * @imod_interval: set the interrupt moderation interval in 250ns > * increments or 0 to disable. > @@ -1186,9 +1189,11 @@ struct dwc3_glue_ops { > * @wakeup_pending_funcs: Indicates whether any interface has requested for > * function wakeup in bitmap format where bit position > * represents interface_id. > + * @err_recovery_count: number of consecutive error recovery attempts > */ > struct dwc3 { > struct work_struct drd_work; > + struct work_struct err_recovery_work; > struct dwc3_trb *ep0_trb; > void *bounce; > u8 *setup_buf; > @@ -1420,6 +1425,7 @@ struct dwc3 { > unsigned wakeup_configured:1; > unsigned suspended:1; > unsigned susphy_state:1; > + unsigned err_dying:1; Can we change this to enum dwc3_err_state err_state where enum dwc3_err_state { DWC3_ERR_NONE = 0, DWC3_ERR_RECOVERY, DWC3_ERR_UNRECOVERABLE, }; Then we can set err_state to DWC3_ERR_RECOVERY before scheduling the recovery work, and set it to DWC3_ERR_NONE when the recovery work is done. If we hit the max number of retries, we can set it to DWC3_ERR_UNRECOVERABLE. This will make it clearer what state the controller is in. > > u16 imod_interval; > > @@ -1429,6 +1435,7 @@ struct dwc3 { > struct dentry *debug_root; > u32 gsbuscfg0_reqinfo; > u32 wakeup_pending_funcs; > + u32 err_recovery_count; > }; > > #define INCRX_BURST_MODE 0 > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index fa0f16ffafef..05f3ffab6e47 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2054,6 +2054,9 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request *request, > > int ret; > > + if (dwc->err_dying) > + return -ESHUTDOWN; > + As Krishna brought up, may need to double check, but I think all the gadget and ep ops need to be guarded. > spin_lock_irqsave(&dwc->lock, flags); > ret = __dwc3_gadget_ep_queue(dep, req); > spin_unlock_irqrestore(&dwc->lock, flags); > @@ -4667,9 +4670,22 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) > return IRQ_NONE; > > if (count > evt->length) { > - dev_err_ratelimited(dwc->dev, "invalid count(%u) > evt->length(%u)\n", > + dev_err(dwc->dev, "invalid count(%u) > evt->length(%u)\n", > count, evt->length); > - return IRQ_NONE; > + /* > + * This is a fatal error - the driver and controller are out of > + * sync on which event has been consumed. Reinitializing the > + * controller is required to recover. Write the bogus count back > + * to GEVNTCOUNT to clear the IRQ source, consistent with the > + * stale event clearing in dwc3_event_buffers_setup(), then > + * schedule error recovery. > + */ > + dwc3_writel(dwc, DWC3_GEVNTCOUNT(0), count); > + dwc->err_dying = true; > + dwc->err_recovery_count++; This increment should be done in dwc3_err_recovery_work(). Reset the count to 0 after err_state transition to DWC3_ERR_NONE and on reset event interrupt. > + if (dwc->err_recovery_count <= DWC3_ERR_RECOVERY_MAX) The dwc->err_recovery_count check here should be checked in the dwc3_err_recovery_work, to check whether to perform soft connect or not. Set the err_state to unrecoverable and print a dev_err() if soft_connect or soft_disconnect fails or if recovery count exceeds the max. > + schedule_work(&dwc->err_recovery_work); > + return IRQ_HANDLED; > } > > evt->count = count; > @@ -4729,6 +4745,25 @@ static void dwc_gadget_release(struct device *dev) > kfree(gadget); > } > > +static void dwc3_err_recovery_work(struct work_struct *work) > +{ > + struct dwc3 *dwc = container_of(work, struct dwc3, err_recovery_work); > + int ret; > + > + synchronize_irq(dwc->irq_gadget); I think we should use disable_irq_nosync() before soft_disconnect and re-enabling it after soft_connect succeeds. Leave it disabled if DWC3_ERR_RECOVERY_MAX is reached or soft_connect fails or is skipped. > + > + ret = dwc3_gadget_soft_disconnect(dwc); > + if (ret) > + return; > + > + dwc3_disconnect_gadget_sleepable(dwc); > + > + dwc->err_dying = false; > + > + if (dwc->softconnect) > + dwc3_gadget_soft_connect(dwc); > +} > + > /** > * dwc3_gadget_init - initializes gadget related registers > * @dwc: pointer to our controller context structure > @@ -4772,6 +4807,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) > } > > init_completion(&dwc->ep0_in_setup); > + INIT_WORK(&dwc->err_recovery_work, dwc3_err_recovery_work); > dwc->gadget = kzalloc_obj(struct usb_gadget); > if (!dwc->gadget) { > ret = -ENOMEM; > @@ -4868,6 +4904,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc) > if (!dwc->gadget) > return; > > + cancel_work_sync(&dwc->err_recovery_work); > dwc3_enable_susphy(dwc, true); > usb_del_gadget(dwc->gadget); > dwc3_gadget_free_endpoints(dwc); > -- > 2.50.1 (Apple Git-155) > Thanks, Thinh ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count 2026-08-28 2:36 ` Thinh Nguyen @ 2026-08-28 9:48 ` Liu Jiazi 2026-08-29 0:31 ` Thinh Nguyen 0 siblings, 1 reply; 5+ messages in thread From: Liu Jiazi @ 2026-08-28 9:48 UTC (permalink / raw) To: Thinh Nguyen Cc: gregkh, linux-usb, linux-kernel, stable, lkp, oe-kbuild-all, Jiazi Liu Hi @Thinh.Nguyen@synopsys.com , Krishna, Thanks for the detailed review. For the gadget ops protection, I propose adding the check only in dwc3_gadget_ep_queue. This covers two scenarios: 1. The window between error detection and soft_disconnect completing. 2. After soft_disconnect, existing checks on connected/pullups_connected/endpoint.desc naturally block all ops. This is also consistent with how other gadget handles it. Brs Jiazi Thinh Nguyen <Thinh.Nguyen@synopsys.com> 于2026年8月28日周五 10:36写道: > > Hi, > > Sorry for the delay response. > > On Wed, Aug 12, 2026, Jiazi Liu wrote: > > From: Jiazi Liu <jiazi.liu1984@gmail.com> > > > > When dwc3_check_event_buf() reads a GEVNTCOUNT value exceeding the > > event buffer length, commit 63ccd26cd1f6 ("usb: dwc3: gadget: check > > that event count does not exceed event buffer length") returns IRQ_NONE > > without writing back GEVNTCOUNT. Since the DWC3 interrupt is > > level-triggered, the uncleared IRQ source keeps the line asserted, > > causing a tight IRQ storm that accumulates 99,900 unhandled interrupts > > and triggers spurious.c:184 BUG -> kernel panic. > > > > The resulting call stack: > > __report_bad_irq+0xac/0xc8 > > note_interrupt+0x340/0x468 > > handle_irq_event+0xac/0xc0 > > handle_fasteoi_irq+0x120/0x228 > > gic_handle_irq+0x68/0x108 > > ... > > kernel BUG at kernel/irq/spurious.c:184 > > > > To reproduce, write a bogus value exceeding the event buffer length > > directly to the GEVNTCOUNT register: > > > > devmem <DWC3_BASE + 0xc40c> 4 0x1004 > > > > Write the bogus count back to GEVNTCOUNT to clear the IRQ source, > > consistent with the stale event clearing pattern in > > dwc3_event_buffers_setup(), and schedule error recovery to > > reinitialize the controller. > > > > Fixes: 63ccd26cd1f6 ("usb: dwc3: gadget: check that event count does not > > exceed event buffer length") > > Fixes tag should be single line. > > > Cc: stable@vger.kernel.org > > Signed-off-by: Jiazi Liu <liujiazi@amazon.com> > > Your email from Signed-off-by is mismatching your From: line above. > > > --- > > Changes in v2: > > - Rename softcon_work to err_recovery_work > > - Use dev_err instead of dev_err_ratelimited > > - Update comment to document driver/controller out-of-sync fatal error > > condition rather than describing IRQ storm behavior > > - Remove dev_err from work handler; error message belongs at the call > > site where the specific failure is known > > - Follow dwc3_gadget_suspend/resume logic in recovery work handler > > - Add err_dying state to reject gadget driver requests during recovery > > - Add err_recovery_count counter and stop scheduling recovery after > > DWC3_ERR_RECOVERY_MAX attempts to prevent infinite retry loops > > - Add synchronize_irq() before soft_disconnect to ensure IRQ handler > > has completed before reinitializing the controller > > - Define DWC3_ERR_RECOVERY_MAX in core.h alongside other global constants > > - Fix sparse warning: use dwc as first argument to dwc3_writel, not dwc->regs > > --- > > drivers/usb/dwc3/core.h | 7 +++++++ > > drivers/usb/dwc3/gadget.c | 41 +++++++++++++++++++++++++++++++++++++-- > > 2 files changed, 46 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > > index 608daeb7ef10..b1cd888eca77 100644 > > --- a/drivers/usb/dwc3/core.h > > +++ b/drivers/usb/dwc3/core.h > > @@ -49,6 +49,7 @@ > > #define DWC3_ENDPOINTS_NUM 32 > > #define DWC3_XHCI_RESOURCES_NUM 2 > > #define DWC3_ISOC_MAX_RETRIES 5 > > +#define DWC3_ERR_RECOVERY_MAX 3 > > > > #define DWC3_SCRATCHBUF_SIZE 4096 /* each buffer is assumed to be 4KiB */ > > #define DWC3_EVENT_BUFFERS_SIZE 4096 > > @@ -1004,6 +1005,7 @@ struct dwc3_glue_ops { > > /** > > * struct dwc3 - representation of our controller > > * @drd_work: workqueue used for role swapping > > + * @err_recovery_work: workqueue used for controller error recovery > > * @ep0_trb: trb which is used for the ctrl_req > > * @bounce: address of bounce buffer > > * @setup_buf: used while precessing STD USB requests > > @@ -1171,6 +1173,7 @@ struct dwc3_glue_ops { > > * @wakeup_configured: set if the device is configured for remote wakeup. > > * @suspended: set to track suspend event due to U3/L2. > > * @susphy_state: state of DWC3_GUSB2PHYCFG_SUSPHY + DWC3_GUSB3PIPECTL_SUSPHY > > + * @err_dying: true when controller is in error recovery, reject all requests > > * before PM suspend. > > * @imod_interval: set the interrupt moderation interval in 250ns > > * increments or 0 to disable. > > @@ -1186,9 +1189,11 @@ struct dwc3_glue_ops { > > * @wakeup_pending_funcs: Indicates whether any interface has requested for > > * function wakeup in bitmap format where bit position > > * represents interface_id. > > + * @err_recovery_count: number of consecutive error recovery attempts > > */ > > struct dwc3 { > > struct work_struct drd_work; > > + struct work_struct err_recovery_work; > > struct dwc3_trb *ep0_trb; > > void *bounce; > > u8 *setup_buf; > > @@ -1420,6 +1425,7 @@ struct dwc3 { > > unsigned wakeup_configured:1; > > unsigned suspended:1; > > unsigned susphy_state:1; > > + unsigned err_dying:1; > > Can we change this to enum dwc3_err_state err_state where > > enum dwc3_err_state { > DWC3_ERR_NONE = 0, > DWC3_ERR_RECOVERY, > DWC3_ERR_UNRECOVERABLE, > }; > > Then we can set err_state to DWC3_ERR_RECOVERY before scheduling the > recovery work, and set it to DWC3_ERR_NONE when the recovery work is > done. If we hit the max number of retries, we can set it to > DWC3_ERR_UNRECOVERABLE. This will make it clearer what state the > controller is in. > > > > > u16 imod_interval; > > > > @@ -1429,6 +1435,7 @@ struct dwc3 { > > struct dentry *debug_root; > > u32 gsbuscfg0_reqinfo; > > u32 wakeup_pending_funcs; > > + u32 err_recovery_count; > > }; > > > > #define INCRX_BURST_MODE 0 > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index fa0f16ffafef..05f3ffab6e47 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -2054,6 +2054,9 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request *request, > > > > int ret; > > > > + if (dwc->err_dying) > > + return -ESHUTDOWN; > > + > > As Krishna brought up, may need to double check, but I think all the > gadget and ep ops need to be guarded. > > > spin_lock_irqsave(&dwc->lock, flags); > > ret = __dwc3_gadget_ep_queue(dep, req); > > spin_unlock_irqrestore(&dwc->lock, flags); > > @@ -4667,9 +4670,22 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) > > return IRQ_NONE; > > > > if (count > evt->length) { > > - dev_err_ratelimited(dwc->dev, "invalid count(%u) > evt->length(%u)\n", > > + dev_err(dwc->dev, "invalid count(%u) > evt->length(%u)\n", > > count, evt->length); > > - return IRQ_NONE; > > + /* > > + * This is a fatal error - the driver and controller are out of > > + * sync on which event has been consumed. Reinitializing the > > + * controller is required to recover. Write the bogus count back > > + * to GEVNTCOUNT to clear the IRQ source, consistent with the > > + * stale event clearing in dwc3_event_buffers_setup(), then > > + * schedule error recovery. > > + */ > > + dwc3_writel(dwc, DWC3_GEVNTCOUNT(0), count); > > + dwc->err_dying = true; > > + dwc->err_recovery_count++; > > This increment should be done in dwc3_err_recovery_work(). Reset the > count to 0 after err_state transition to DWC3_ERR_NONE and on reset > event interrupt. > > > + if (dwc->err_recovery_count <= DWC3_ERR_RECOVERY_MAX) > > The dwc->err_recovery_count check here should be checked in the > dwc3_err_recovery_work, to check whether to perform soft connect or not. > Set the err_state to unrecoverable and print a dev_err() if soft_connect > or soft_disconnect fails or if recovery count exceeds the max. > > > + schedule_work(&dwc->err_recovery_work); > > + return IRQ_HANDLED; > > } > > > > evt->count = count; > > @@ -4729,6 +4745,25 @@ static void dwc_gadget_release(struct device *dev) > > kfree(gadget); > > } > > > > +static void dwc3_err_recovery_work(struct work_struct *work) > > +{ > > + struct dwc3 *dwc = container_of(work, struct dwc3, err_recovery_work); > > + int ret; > > + > > + synchronize_irq(dwc->irq_gadget); > > I think we should use disable_irq_nosync() before soft_disconnect and > re-enabling it after soft_connect succeeds. Leave it disabled if > DWC3_ERR_RECOVERY_MAX is reached or soft_connect fails or is skipped. > > > + > > + ret = dwc3_gadget_soft_disconnect(dwc); > > + if (ret) > > + return; > > + > > + dwc3_disconnect_gadget_sleepable(dwc); > > + > > + dwc->err_dying = false; > > + > > + if (dwc->softconnect) > > + dwc3_gadget_soft_connect(dwc); > > +} > > + > > /** > > * dwc3_gadget_init - initializes gadget related registers > > * @dwc: pointer to our controller context structure > > @@ -4772,6 +4807,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) > > } > > > > init_completion(&dwc->ep0_in_setup); > > + INIT_WORK(&dwc->err_recovery_work, dwc3_err_recovery_work); > > dwc->gadget = kzalloc_obj(struct usb_gadget); > > if (!dwc->gadget) { > > ret = -ENOMEM; > > @@ -4868,6 +4904,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc) > > if (!dwc->gadget) > > return; > > > > + cancel_work_sync(&dwc->err_recovery_work); > > dwc3_enable_susphy(dwc, true); > > usb_del_gadget(dwc->gadget); > > dwc3_gadget_free_endpoints(dwc); > > -- > > 2.50.1 (Apple Git-155) > > > > Thanks, > Thinh ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count 2026-08-28 9:48 ` Liu Jiazi @ 2026-08-29 0:31 ` Thinh Nguyen 0 siblings, 0 replies; 5+ messages in thread From: Thinh Nguyen @ 2026-08-29 0:31 UTC (permalink / raw) To: Liu Jiazi Cc: Thinh Nguyen, gregkh, linux-usb, linux-kernel, stable, lkp, oe-kbuild-all, Jiazi Liu Hi Liu, On Fri, Aug 28, 2026, Liu Jiazi wrote: > Hi @Thinh.Nguyen@synopsys.com , Krishna, > > Thanks for the detailed review. > For the gadget ops protection, I propose adding the check only in > dwc3_gadget_ep_queue. > This covers two scenarios: > 1. The window between error detection and soft_disconnect completing. > 2. After soft_disconnect, existing checks on > connected/pullups_connected/endpoint.desc naturally block all ops. > This is also consistent with how other gadget handles it. > Looking at the change more closely, especially the gadget_ops and ep_ops. We need to consider the followings: 1) We need a mutex to prevent a race against dwc3_gadget_pullup() 2) ep_enable, ep_queue, ep_set_halt/wedge, wakeup, and function_wakeup need to be guarded. The other ones are for cleanup and should be left alone. 3) We shouldn't use disable_irq_nosync() as suggested previously because we need the event interrupt to drain the GEVENTCOUNT to allow the controller to halt after clearing the DCTL.RUN_STOP bit. But we should move the synchronize_irq() before the soft_connect. Can you try the change below (not tested). Thanks, Thinh diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 608daeb7ef10..f3dabb37f806 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -49,6 +49,7 @@ #define DWC3_ENDPOINTS_NUM 32 #define DWC3_XHCI_RESOURCES_NUM 2 #define DWC3_ISOC_MAX_RETRIES 5 +#define DWC3_ERR_RECOVERY_MAX 3 #define DWC3_SCRATCHBUF_SIZE 4096 /* each buffer is assumed to be 4KiB */ #define DWC3_EVENT_BUFFERS_SIZE 4096 @@ -841,6 +842,12 @@ enum dwc3_link_state { DWC3_LINK_STATE_MASK = 0x0f, }; +enum dwc3_err_state { + DWC3_ERR_NONE = 0, + DWC3_ERR_RECOVERY, + DWC3_ERR_UNRECOVERABLE, +}; + /* TRB Length, PCM and Status */ #define DWC3_TRB_SIZE_MASK (0x00ffffff) #define DWC3_TRB_SIZE_LENGTH(n) ((n) & DWC3_TRB_SIZE_MASK) @@ -1004,6 +1011,7 @@ struct dwc3_glue_ops { /** * struct dwc3 - representation of our controller * @drd_work: workqueue used for role swapping + * @err_recovery_work: workqueue used for controller error recovery * @ep0_trb: trb which is used for the ctrl_req * @bounce: address of bounce buffer * @setup_buf: used while precessing STD USB requests @@ -1013,6 +1021,7 @@ struct dwc3_glue_ops { * @ep0_in_setup: one control transfer is completed and enter setup phase * @lock: for synchronizing * @mutex: for mode switching + * @connect_mutex: for the pull-up and err_recovery_work * @dev: pointer to our struct device * @sysdev: pointer to the DMA-capable device * @xhci: pointer to our xHCI child @@ -1079,6 +1088,9 @@ struct dwc3_glue_ops { * @ep0_next_event: hold the next expected event * @ep0state: state of endpoint zero * @link_state: link state + * @err_state: current error recovery state. + * @err_recovery_count: number of consecutive error recovery attempts until + * confirmed healthy and reset to 0 on reset event. * @speed: device speed (super, high, full, low) * @hwparams: copy of hwparams registers * @regset: debugfs pointer to regdump file @@ -1189,6 +1201,7 @@ struct dwc3_glue_ops { */ struct dwc3 { struct work_struct drd_work; + struct work_struct err_recovery_work; struct dwc3_trb *ep0_trb; void *bounce; u8 *setup_buf; @@ -1203,6 +1216,9 @@ struct dwc3 { /* mode switching lock */ struct mutex mutex; + /* serializes pull-up run/stop vs error recovery */ + struct mutex connect_mutex; + struct device *dev; struct device *sysdev; @@ -1332,6 +1348,9 @@ struct dwc3 { enum dwc3_ep0_next ep0_next_event; enum dwc3_ep0_state ep0state; enum dwc3_link_state link_state; + enum dwc3_err_state err_state; + + u32 err_recovery_count; u16 u2sel; u16 u2pel; diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index bfe616194dfa..fa3d8b9f6f8e 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -200,6 +200,11 @@ int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request, int ret; spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + ret = -ESHUTDOWN; + goto out; + } + if (!dep->endpoint.desc || !dwc->pullups_connected || !dwc->connected) { dev_err(dwc->dev, "%s: can't queue to disabled endpoint\n", dep->name); @@ -271,6 +276,10 @@ int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value) int ret; spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + spin_unlock_irqrestore(&dwc->lock, flags); + return -ESHUTDOWN; + } ret = __dwc3_gadget_ep0_set_halt(ep, value); spin_unlock_irqrestore(&dwc->lock, flags); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index fa944856f956..87b535fb9443 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1149,6 +1149,10 @@ static int dwc3_gadget_ep_enable(struct usb_ep *ep, return 0; spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + spin_unlock_irqrestore(&dwc->lock, flags); + return -ESHUTDOWN; + } ret = __dwc3_gadget_ep_enable(dep, DWC3_DEPCFG_ACTION_INIT); spin_unlock_irqrestore(&dwc->lock, flags); @@ -2055,6 +2059,10 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request *request, int ret; spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + spin_unlock_irqrestore(&dwc->lock, flags); + return -ESHUTDOWN; + } ret = __dwc3_gadget_ep_queue(dep, req); spin_unlock_irqrestore(&dwc->lock, flags); @@ -2287,6 +2295,10 @@ static int dwc3_gadget_ep_set_halt(struct usb_ep *ep, int value) int ret; spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + spin_unlock_irqrestore(&dwc->lock, flags); + return -ESHUTDOWN; + } ret = __dwc3_gadget_ep_set_halt(dep, value, false); spin_unlock_irqrestore(&dwc->lock, flags); @@ -2301,6 +2313,10 @@ static int dwc3_gadget_ep_set_wedge(struct usb_ep *ep) int ret; spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + spin_unlock_irqrestore(&dwc->lock, flags); + return -ESHUTDOWN; + } dep->flags |= DWC3_EP_WEDGE; if (dep->number == 0 || dep->number == 1) @@ -2432,6 +2448,10 @@ static int dwc3_gadget_wakeup(struct usb_gadget *g) } spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + spin_unlock_irqrestore(&dwc->lock, flags); + return -ESHUTDOWN; + } if (!dwc->gadget->wakeup_armed) { dev_err(dwc->dev, "not armed for remote wakeup\n"); spin_unlock_irqrestore(&dwc->lock, flags); @@ -2459,6 +2479,10 @@ static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id) } spin_lock_irqsave(&dwc->lock, flags); + if (dwc->err_state != DWC3_ERR_NONE) { + spin_unlock_irqrestore(&dwc->lock, flags); + return -ESHUTDOWN; + } /* * If the link is in U3, signal for remote wakeup and wait for the * link to transition to U0 before sending device notification. @@ -2828,10 +2852,13 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) synchronize_irq(dwc->irq_gadget); + /* Serialize against dwc3_err_recovery_work() */ + mutex_lock(&dwc->connect_mutex); if (!is_on) ret = dwc3_gadget_soft_disconnect(dwc); else ret = dwc3_gadget_soft_connect(dwc); + mutex_unlock(&dwc->connect_mutex); pm_runtime_put(dwc->dev); @@ -4148,6 +4175,10 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) dwc->suspended = false; + /* The controller is recovered. */ + if (dwc->err_state == DWC3_ERR_NONE) + dwc->err_recovery_count = 0; + /* * Ideally, dwc3_reset_gadget() would trigger the function * drivers to stop any active transfers through ep disable. @@ -4635,6 +4666,12 @@ static irqreturn_t dwc3_thread_interrupt(int irq, void *_evt) return ret; } +static void dwc3_schedule_err_recovery(struct dwc3 *dwc) +{ + dwc->err_state = DWC3_ERR_RECOVERY; + schedule_work(&dwc->err_recovery_work); +} + static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) { struct dwc3 *dwc = evt->dwc; @@ -4668,9 +4705,21 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) return IRQ_NONE; if (count > evt->length) { - dev_err_ratelimited(dwc->dev, "invalid count(%u) > evt->length(%u)\n", + dev_err(dwc->dev, "invalid count(%u) > evt->length(%u)\n", count, evt->length); - return IRQ_NONE; + /* + * This is a fatal error - the driver and controller are out of + * sync on which event has been consumed. Reinitializing the + * controller is required to recover. Write the bogus count back + * to GEVNTCOUNT to clear the IRQ source, consistent with the + * stale event clearing in dwc3_event_buffers_setup(), then + * schedule error recovery. + */ + spin_lock(&dwc->lock); + dwc3_schedule_err_recovery(dwc); + spin_unlock(&dwc->lock); + dwc3_writel(dwc, DWC3_GEVNTCOUNT(0), count); + return IRQ_HANDLED; } evt->count = count; @@ -4730,6 +4779,53 @@ static void dwc_gadget_release(struct device *dev) kfree(gadget); } +static void dwc3_err_recovery_work(struct work_struct *work) +{ + struct dwc3 *dwc = container_of(work, struct dwc3, err_recovery_work); + unsigned long flags; + int ret; + + dwc->err_recovery_count++; + + /* serializes against dwc3_gadget_pullup() */ + mutex_lock(&dwc->connect_mutex); + + ret = dwc3_gadget_soft_disconnect(dwc); + if (ret) + goto err_unrecoverable; + + dwc3_disconnect_gadget_sleepable(dwc); + + if (dwc->err_recovery_count > DWC3_ERR_RECOVERY_MAX) + goto err_unrecoverable; + + if (dwc->softconnect) { + /* + * Wait irq to finish before soft_connect resets evt->lpos and + * the event buffer registers to avoid racing with + * dwc3_process_event_buf(). + */ + synchronize_irq(dwc->irq_gadget); + + ret = dwc3_gadget_soft_connect(dwc); + if (ret) + goto err_unrecoverable; + } + mutex_unlock(&dwc->connect_mutex); + + spin_lock_irqsave(&dwc->lock, flags); + dwc->err_state = DWC3_ERR_NONE; + spin_unlock_irqrestore(&dwc->lock, flags); + return; + +err_unrecoverable: + dev_err(dwc->dev, "Unable to recover the controller\n"); + mutex_unlock(&dwc->connect_mutex); + spin_lock_irqsave(&dwc->lock, flags); + dwc->err_state = DWC3_ERR_UNRECOVERABLE; + spin_unlock_irqrestore(&dwc->lock, flags); +} + /** * dwc3_gadget_init - initializes gadget related registers * @dwc: pointer to our controller context structure @@ -4773,6 +4869,8 @@ int dwc3_gadget_init(struct dwc3 *dwc) } init_completion(&dwc->ep0_in_setup); + INIT_WORK(&dwc->err_recovery_work, dwc3_err_recovery_work); + mutex_init(&dwc->connect_mutex); dwc->gadget = kzalloc_obj(struct usb_gadget); if (!dwc->gadget) { ret = -ENOMEM; @@ -4869,6 +4967,8 @@ void dwc3_gadget_exit(struct dwc3 *dwc) if (!dwc->gadget) return; + cancel_work_sync(&dwc->err_recovery_work); + mutex_destroy(&dwc->connect_mutex); dwc3_enable_susphy(dwc, true); usb_del_gadget(dwc->gadget); dwc3_gadget_free_endpoints(dwc); ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-29 0:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20260727092258.1121-1-liujiazi@amazon.com>
2026-08-12 8:21 ` [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count Jiazi Liu
2026-08-12 9:48 ` Krishna Kurapati
2026-08-28 2:36 ` Thinh Nguyen
2026-08-28 9:48 ` Liu Jiazi
2026-08-29 0:31 ` Thinh Nguyen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®