On Wed, Aug 07, 2024, Selvarasu Ganesan wrote: > > On 8/7/2024 6:08 AM, Thinh Nguyen wrote: > > On Fri, Jul 19, 2024, Selvarasu Ganesan wrote: > >> In certain scenarios, there is a chance that the CPU may not be > >> scheduled the bottom half of dwc3 interrupt. This is because the CPU > >> may hang up where any work queue lockup has happened for the same CPU > >> that is trying to schedule the dwc3 thread interrupt. In this scenario, > >> the USB can enter runtime suspend as the bus may idle for a longer time > >> , or user can reconnect the USB cable. Then, the dwc3 event interrupt > >> can be enabled when runtime resume is happening with regardless of the > >> previous event status. This can lead to a dwc3 IRQ storm due to the > >> return from the interrupt handler by checking only the evt->flags as > >> DWC3_EVENT_PENDING, where the same flag was set as DWC3_EVENT_PENDING > >> in previous work queue lockup. > >> Let's consider the following sequences in this scenario, > >> > >> Call trace of dwc3 IRQ after workqueue lockup scenario > >> ====================================================== > >> IRQ #1: > >> ->dwc3_interrupt() > >> ->dwc3_check_event_buf() > >> ->if (evt->flags & DWC3_EVENT_PENDING) > >> return IRQ_HANDLED; > >> ->evt->flags |= DWC3_EVENT_PENDING; > >> ->/* Disable interrupt by setting DWC3_GEVNTSIZ_INTMASK in > >> DWC3_GEVNTSIZ > >> ->return IRQ_WAKE_THREAD; // No workqueue scheduled for dwc3 > >> thread_fu due to workqueue lockup > >> even after return IRQ_WAKE_THREAD > >> from top-half. > >> > >> Thread #2: > >> ->dwc3_runtime_resume() > >> ->dwc3_resume_common() > >> ->dwc3_gadget_resume() > >> ->dwc3_gadget_soft_connect() > >> ->dwc3_event_buffers_setup() > >> ->/*Enable interrupt by clearing DWC3_GEVNTSIZ_INTMASK in > >> DWC3_GEVNTSIZ*/ > >> > >> Start IRQ Storming after enable dwc3 event in resume path > >> ========================================================= > >> CPU0: IRQ > >> dwc3_interrupt() > >> dwc3_check_event_buf() > >> if (evt->flags & DWC3_EVENT_PENDING) > >> return IRQ_HANDLED; > >> > >> CPU0: IRQ > >> dwc3_interrupt() > >> dwc3_check_event_buf() > >> if (evt->flags & DWC3_EVENT_PENDING) > >> return IRQ_HANDLED; > >> .. > >> .. > >> > >> To fix this issue by avoiding enabling of the dwc3 event interrupt in > >> the runtime resume path if dwc3 event processing is in progress. > >> > >> Signed-off-by: Selvarasu Ganesan <selvarasu.g@xxxxxxxxxxx> > >> --- > >> drivers/usb/dwc3/core.c | 8 ++++++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > >> index cb82557678dd..610792a70805 100644 > >> --- a/drivers/usb/dwc3/core.c > >> +++ b/drivers/usb/dwc3/core.c > >> @@ -549,8 +549,12 @@ int dwc3_event_buffers_setup(struct dwc3 *dwc) > >> lower_32_bits(evt->dma)); > >> dwc3_writel(dwc->regs, DWC3_GEVNTADRHI(0), > >> upper_32_bits(evt->dma)); > >> - dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), > >> - DWC3_GEVNTSIZ_SIZE(evt->length)); > >> + > >> + /* Skip enable dwc3 event interrupt if event is processing in middle */ > >> + if (!(evt->flags & DWC3_EVENT_PENDING)) > >> + dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), > >> + DWC3_GEVNTSIZ_SIZE(evt->length)); > >> + > >> dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 0); > >> > >> return 0; > >> -- > >> 2.17.1 > >> > > We're not waking up from a hibernation. So after a soft-reset and > > resume, the events that weren't processed are stale. They should be > > processed prior to entering suspend or be discarded before resume. > > > > The synchronize_irq() during suspend() was not sufficient to prevent > > this? What are we missing here. > > > > Thanks, > > Thinh > I don’t think the triggering of interrupt would not be stopped even if > do soft reset. It's because of event count is may be valid . Ok. I think I see what you're referring to when you say "event is processing in the middle" now. What you want to check is probably this in dwc3_event_buffers_setup(). Please confirm: if (dwc->pending_events) dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), DWC3_GEVNTSIZ_INTMASK | DWC3_GEVNTSIZ_SIZE(evt->length)); else dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), DWC3_GEVNTSIZ_SIZE(evt->length)); > Let consider the scenarios where SW is not acknowledge the event count > after getting a interrupt with disable GEVNTSIZ_INTMASK =0. It will > triggering the spurious interrupts until enable GEVNTSIZ_INTMASK=1 or > acknowledge the event count by SW. This is happening here because of We > just return from interrupt handler by checking if evt->flags as > DWC3_EVENT_PENDING. Clearing of DWC3_EVENT_PENDING flag is done in > dwc3_thread_interrupt. In some scenario it's not happening due to cpu > hangup or work queue lockup. This can be mitigated by adjusting the imod_interval (interrupt moderation). Have you tried that? Thanks, Thinh > > The same issue was reported earlier and not derived actual root cause > from USB dwc3 driver point of view, and somehow we managing fix with our > vendor kernel by using kretprobe. > > To fix this issue, We have to reset the evt->flags or stop disable > GEVNTSIZ_INTMASK=0 > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20230102050831.105499-1-jh0801.jung@xxxxxxxxxxx/__;!!A4F2R9G_pg!b0Uj7NvYzWqrvNKOrkkXaY-g4Uaso-pW520AY2fBdlqmJeudUnmdsgpnL1YxAZaw2ydyg2M-XzhetbCraKxJa5C3NBQ$ > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/1646630679-121585-1-git-send-email-jh0801.jung@xxxxxxxxxxx/__;!!A4F2R9G_pg!b0Uj7NvYzWqrvNKOrkkXaY-g4Uaso-pW520AY2fBdlqmJeudUnmdsgpnL1YxAZaw2ydyg2M-XzhetbCraKxJ2d2qDqA$ > Thanks, Thinh
