mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Jiazi Liu <jiazi.liu1984@gmail.com>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"frode@meta.com" <frode@meta.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	Jiazi Liu <liujiazi@amazon.com>
Subject: Re: [PATCH v3] usb: dwc3: gadget: fix IRQ storm on invalid event buffer count
Date: Fri, 4 Sep 2026 23:55:31 +0000	[thread overview]
Message-ID: <aptYnB2TuvPIdPhG@vbox> (raw)
In-Reply-To: <20260904094934.12341-1-liujiazi@amazon.com>

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

      reply	other threads:[~2026-09-04 23:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260727092258.1121-1-liujiazi@amazon.com>
2026-08-12  8:21 ` [PATCH v2 1/1] " 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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aptYnB2TuvPIdPhG@vbox \
    --to=thinh.nguyen@synopsys.com \
    --cc=frode@meta.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiazi.liu1984@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=liujiazi@amazon.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®