| Message ID | 20220616083852.1264261-1-xiehongyu1@kylinos.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,-next] usb: xhci: disable irq during initialization | expand |
Hi On 16.6.2022 11.38, Hongyu Xie wrote: > irq is disabled in xhci_quiesce(called by xhci_halt, with bit:2 cleared > in USBCMD register), but xhci_run(called by usb_add_hcd) re-enable it. > It's possible that you will receive thousands of interrupt requests > after initialization for 2.0 roothub. And you will get a lot of > warning like, "xHCI dying, ignoring interrupt. Shouldn't IRQs be > disabled?". This amount of interrupt requests will cause the entire > system to freeze. > This problem was first found on a device with ASM2142 host controller > on it. > > Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn> > -- Thanks, added. Did some non-functional cleanups, mostly to the old debug messages this patch moves around: https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-linus&id=2d1fa9b0b08bb841fde9136c249f632eb04a1e00 Let me know if you disagree with the changes -Mathias
Hi, It looks fine. Thanks for all the advices and cleanups. On 2022/6/21 16:25, Mathias Nyman wrote: > Hi > > On 16.6.2022 11.38, Hongyu Xie wrote: >> irq is disabled in xhci_quiesce(called by xhci_halt, with bit:2 cleared >> in USBCMD register), but xhci_run(called by usb_add_hcd) re-enable it. >> It's possible that you will receive thousands of interrupt requests >> after initialization for 2.0 roothub. And you will get a lot of >> warning like, "xHCI dying, ignoring interrupt. Shouldn't IRQs be >> disabled?". This amount of interrupt requests will cause the entire >> system to freeze. >> This problem was first found on a device with ASM2142 host controller >> on it. >> >> Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn> >> -- > Thanks, added. > Did some non-functional cleanups, mostly to the old debug messages this patch > moves around: > > https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-linus&id=2d1fa9b0b08bb841fde9136c249f632eb04a1e00 > > Let me know if you disagree with the changes > > -Mathias -Hongyu
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 9ac56e9ffc64..1344cc566338 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -611,15 +611,41 @@ static int xhci_init(struct usb_hcd *hcd) static int xhci_run_finished(struct xhci_hcd *xhci) { + unsigned long flags; + u32 temp; + + /* Prevent receiving irqs in the small window between enabling interrupt + * and setting Run/Stop bit + */ + spin_lock_irqsave(&xhci->lock, flags); + + /* Enable interrupt right before setting Run/Stop bit according to spec + * 4.2 + */ + /* Set the HCD state before we enable the irqs */ + temp = readl(&xhci->op_regs->command); + temp |= (CMD_EIE); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, + "// Enable interrupts, cmd = 0x%x.", temp); + writel(temp, &xhci->op_regs->command); + + temp = readl(&xhci->ir_set->irq_pending); + xhci_dbg_trace(xhci, trace_xhci_dbg_init, + "// %s %p by writing 0x%x %s", + "Enabling event ring interrupter", + "to irq_pending", xhci->ir_set, + (unsigned int) ER_IRQ_ENABLE(temp)); + writel(ER_IRQ_ENABLE(temp), &xhci->ir_set->irq_pending); if (xhci_start(xhci)) { xhci_halt(xhci); + spin_unlock_irqrestore(&xhci->lock, flags); return -ENODEV; } xhci->cmd_ring_state = CMD_RING_STATE_RUNNING; if (xhci->quirks & XHCI_NEC_HOST) xhci_ring_cmd_db(xhci); - + spin_unlock_irqrestore(&xhci->lock, flags); return 0; } @@ -668,19 +694,6 @@ int xhci_run(struct usb_hcd *hcd) temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK; writel(temp, &xhci->ir_set->irq_control); - /* Set the HCD state before we enable the irqs */ - temp = readl(&xhci->op_regs->command); - temp |= (CMD_EIE); - xhci_dbg_trace(xhci, trace_xhci_dbg_init, - "// Enable interrupts, cmd = 0x%x.", temp); - writel(temp, &xhci->op_regs->command); - - temp = readl(&xhci->ir_set->irq_pending); - xhci_dbg_trace(xhci, trace_xhci_dbg_init, - "// Enabling event ring interrupter %p by writing 0x%x to irq_pending", - xhci->ir_set, (unsigned int) ER_IRQ_ENABLE(temp)); - writel(ER_IRQ_ENABLE(temp), &xhci->ir_set->irq_pending); - if (xhci->quirks & XHCI_NEC_HOST) { struct xhci_command *command;
irq is disabled in xhci_quiesce(called by xhci_halt, with bit:2 cleared in USBCMD register), but xhci_run(called by usb_add_hcd) re-enable it. It's possible that you will receive thousands of interrupt requests after initialization for 2.0 roothub. And you will get a lot of warning like, "xHCI dying, ignoring interrupt. Shouldn't IRQs be disabled?". This amount of interrupt requests will cause the entire system to freeze. This problem was first found on a device with ASM2142 host controller on it. Signed-off-by: Hongyu Xie <xiehongyu1@kylinos.cn> --- v5: remove reported by test robot v4: fix compile err reported by test robot v3: - enabling interrupt right before setting Run/Stop bit - spin_lock_irqsave to prevent receiving irqs in the small window according to Mathias's suggestion v2: fix compile error drivers/usb/host/xhci.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-)