Linux USB
 help / color / mirror / Atom feed
* [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