* [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 2026-09-04 9:49 ` [PATCH v3] " Jiazi Liu 1 sibling, 2 replies; 9+ 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 related [flat|nested] 9+ 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; 9+ 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] 9+ 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; 9+ messages in thread From: Thinh Nguyen @ 2026-08-28 2:36 UTC (permalink / raw) To: Jiazi Liu Cc: Thinh Nguyen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, lkp@intel.com, oe-kbuild-all@lists.linux.dev, 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] 9+ 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; 9+ messages in thread From: Liu Jiazi @ 2026-08-28 9:48 UTC (permalink / raw) To: Thinh Nguyen Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, lkp@intel.com, oe-kbuild-all@lists.linux.dev, 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] 9+ 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 2026-09-02 4:14 ` Liu Jiazi 0 siblings, 1 reply; 9+ messages in thread From: Thinh Nguyen @ 2026-08-29 0:31 UTC (permalink / raw) To: Liu Jiazi Cc: Thinh Nguyen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, lkp@intel.com, oe-kbuild-all@lists.linux.dev, 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 related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count 2026-08-29 0:31 ` Thinh Nguyen @ 2026-09-02 4:14 ` Liu Jiazi 2026-09-04 1:56 ` Thinh Nguyen 0 siblings, 1 reply; 9+ messages in thread From: Liu Jiazi @ 2026-09-02 4:14 UTC (permalink / raw) To: Thinh Nguyen Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, lkp@intel.com, oe-kbuild-all@lists.linux.dev, Jiazi Liu Hi @Thinh Nguyen Tested on our device with the patch you provided. Trigger the error by writing a bogus value to GEVNTCOUNT and the controller recovers successfully USB re-enumrates and ADB comes back. No kernel panic observed. uart print: [ 1892.781261][ C0] dwc3 b1800000.usb: invalid count(61436) > evt->length(4096) [ 1892.834341][ T6793] dwc3 b1800000.usb: request ffffff81475856c0 was not queued to ep0out [ 1895.042752][ T9114] ffs_data_put(): freeing [ 1896.206669][ T9142] read descriptors [ 1896.207465][ T9142] bcdVersion must be 0x0100, stored in Little Endian order. Userspace driver should be fixed, accepting 0x0001 for compatibility. [ 1896.209638][ T9142] bcdVersion must be 0x0100, stored in Little Endian order. Userspace driver should be fixed, accepting 0x0001 for compatibility. [ 1896.211754][ T9142] read strings [ 1897.244130][ T9123] FFS endpoints ready, proceeding with UDC bind [ 1897.258493][ T9123] bcdVersion must be 0x0100, stored in Little Endian order. Userspace driver should be fixed, accepting 0x0001 for compatibility. [ 1897.260970][ T9123] bcdVersion must be 0x0100, stored in Little Endian order. Userspace driver should be fixed, accepting 0x0001 for compatibility. Brs Jiazi Thinh Nguyen <Thinh.Nguyen@synopsys.com> 于2026年8月29日周六 08:31写道: > > 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] 9+ messages in thread
* Re: [PATCH v2 1/1] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count 2026-09-02 4:14 ` Liu Jiazi @ 2026-09-04 1:56 ` Thinh Nguyen 0 siblings, 0 replies; 9+ messages in thread From: Thinh Nguyen @ 2026-09-04 1:56 UTC (permalink / raw) To: Liu Jiazi Cc: Thinh Nguyen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, lkp@intel.com, oe-kbuild-all@lists.linux.dev, Jiazi Liu On Wed, Sep 02, 2026, Liu Jiazi wrote: > Hi @Thinh Nguyen > > Tested on our device with the patch you provided. > Trigger the error by writing a bogus value to GEVNTCOUNT and the > controller recovers successfully > USB re-enumrates and ADB comes back. No kernel panic observed. > > uart print: > [ 1892.781261][ C0] dwc3 b1800000.usb: invalid count(61436) > > evt->length(4096) > [ 1892.834341][ T6793] dwc3 b1800000.usb: request ffffff81475856c0 was > not queued to ep0out > [ 1895.042752][ T9114] ffs_data_put(): freeing > [ 1896.206669][ T9142] read descriptors > [ 1896.207465][ T9142] bcdVersion must be 0x0100, stored in Little > Endian order. Userspace driver should be > fixed, accepting 0x0001 for compatibility. > [ 1896.209638][ T9142] bcdVersion must be 0x0100, stored in Little > Endian order. Userspace driver should be > fixed, accepting 0x0001 for compatibility. > [ 1896.211754][ T9142] read strings > [ 1897.244130][ T9123] FFS endpoints ready, proceeding with UDC bind > [ 1897.258493][ T9123] bcdVersion must be 0x0100, stored in Little > Endian order. Userspace driver should be > fixed, accepting 0x0001 for compatibility. > [ 1897.260970][ T9123] bcdVersion must be 0x0100, stored in Little > Endian order. Userspace driver should be > fixed, accepting 0x0001 for compatibility. > Thanks for testing and for verifying the fix. If there are no remaining concerns, could you please submit the patch? Otherwise, let me know if you notice anything else that needs to be addressed. Thanks, Thinh ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count [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-09-04 9:49 ` Jiazi Liu 2026-09-04 23:55 ` Thinh Nguyen 1 sibling, 1 reply; 9+ messages in thread From: Jiazi Liu @ 2026-09-04 9:49 UTC (permalink / raw) To: Thinh.Nguyen, gregkh Cc: frode, linux-usb, linux-kernel, stable, 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 Tested-by: Jiazi Liu <liujiazi@amazon.com> Signed-off-by: Jiazi Liu <liujiazi@amazon.com> --- drivers/usb/dwc3/core.h | 19 +++++++ drivers/usb/dwc3/ep0.c | 9 ++++ drivers/usb/dwc3/gadget.c | 104 +++++++++++++++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 2 deletions(-) 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 fa0f16ffafef..dbfff5e691e6 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); @@ -4147,6 +4174,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. @@ -4634,6 +4665,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; @@ -4667,9 +4704,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; @@ -4729,6 +4778,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 @@ -4772,6 +4868,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; @@ -4868,6 +4966,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); -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count 2026-09-04 9:49 ` [PATCH v3] " Jiazi Liu @ 2026-09-04 23:55 ` Thinh Nguyen 0 siblings, 0 replies; 9+ messages in thread From: Thinh Nguyen @ 2026-09-04 23:55 UTC (permalink / raw) To: Jiazi Liu Cc: Thinh Nguyen, gregkh@linuxfoundation.org, frode@meta.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Jiazi Liu Hi, On Fri, Sep 04, 2026, Jiazi Liu wrote: > From: Jiazi Liu <jiazi.liu1984@gmail.com> Can you fix your email to match the From and Signed-off-by tag? > > 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 > Tested-by: Jiazi Liu <liujiazi@amazon.com> Remove Tested-by tag as it should be implied by the Signed-off-by tag. > Signed-off-by: Jiazi Liu <liujiazi@amazon.com> > --- > drivers/usb/dwc3/core.h | 19 +++++++ > drivers/usb/dwc3/ep0.c | 9 ++++ > drivers/usb/dwc3/gadget.c | 104 +++++++++++++++++++++++++++++++++++++- > 3 files changed, 130 insertions(+), 2 deletions(-) > > 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 fa0f16ffafef..dbfff5e691e6 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); > > @@ -4147,6 +4174,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. > @@ -4634,6 +4665,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; > @@ -4667,9 +4704,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; > @@ -4729,6 +4778,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); Can we also call dwc3_disconnect_gadget_sleepable() in the unrecoverable err to ensure the gadget driver is notified of the disconnect? ret = dwc3_gadget_soft_disconnect(dwc); dwc3_disconnect_gadget_sleepable(dwc); if (ret) goto err_unrecoverable; > + > + 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); > +} > + Thanks, Thinh ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-04 23:55 UTC | newest]
Thread overview: 9+ 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
2026-09-02 4:14 ` Liu Jiazi
2026-09-04 1:56 ` Thinh Nguyen
2026-09-04 9:49 ` [PATCH v3] " Jiazi Liu
2026-09-04 23:55 ` Thinh Nguyen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox