| Message ID | 20220616080933.1238309-1-xiehongyu1@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,-next] usb: xhci: disable irq during initialization | expand |
On Thu, Jun 16, 2022 at 04:09:33PM +0800, 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> > Reported-by: kernel test robot <lkp@intel.com> the test robot did not find this original problem, it only found problems with your original submission, which is different.
Hi greg, On 2022/6/16 16:12, Greg KH wrote: > On Thu, Jun 16, 2022 at 04:09:33PM +0800, 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> >> Reported-by: kernel test robot <lkp@intel.com> > > the test robot did not find this original problem, it only found > problems with your original submission, which is different. > "Reported-by" shouldn't had been put here. Quote from the last email test robot sent to me， "If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com>" Where should I put this? Or just ignore it?
On Thu, Jun 16, 2022 at 04:24:08PM +0800, 谢泓宇 wrote: > Hi greg, > > On 2022/6/16 16:12, Greg KH wrote: > > On Thu, Jun 16, 2022 at 04:09:33PM +0800, 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> > > > Reported-by: kernel test robot <lkp@intel.com> > > > > the test robot did not find this original problem, it only found > > problems with your original submission, which is different. > > > "Reported-by" shouldn't had been put here. > Quote from the last email test robot sent to me， > "If you fix the issue, kindly add following tag where applicable > Reported-by: kernel test robot <lkp@intel.com>" > Where should I put this? Or just ignore it? > Yes, you can ignore it but you properly referenced it in your changelog area.
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> Reported-by: kernel test robot <lkp@intel.com> --- v4: add definition for flags 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(-)