Messages in this thread | | | Date | Sat, 27 Apr 2024 15:18:31 -0700 | Subject | Re: [PATCH v2] usb: ohci: Prevent missed ohci interrupts | From | Guenter Roeck <> |
| |
On 4/27/24 14:00, David Laight wrote: > From: Guenter Roeck >> Sent: 24 April 2024 21:00 >> >> Testing ohci functionality with qemu's pci-ohci emulation often results >> in ohci interface stalls, resulting in hung task timeouts. >> >> The problem is caused by lost interrupts between the emulation and the >> Linux kernel code. Additional interrupts raised while the ohci interrupt >> handler in Linux is running and before the handler clears the interrupt >> status are not handled. The fix for a similar problem in ehci suggests >> that the problem is likely caused by edge-triggered MSI interrupts. See >> commit 0b60557230ad ("usb: ehci: Prevent missed ehci interrupts with >> edge-triggered MSI") for details. >> >> Ensure that the ohci interrupt code handles all pending interrupts before >> returning to solve the problem. >> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Fixes: 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI devices") >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> v2: Only repeat if the interface is still active >> >> drivers/usb/host/ohci-hcd.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c >> index 4f9982ecfb58..bb6b50b4a356 100644 >> --- a/drivers/usb/host/ohci-hcd.c >> +++ b/drivers/usb/host/ohci-hcd.c >> @@ -888,6 +888,7 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd) >> /* Check for an all 1's result which is a typical consequence >> * of dead, unclocked, or unplugged (CardBus...) devices >> */ >> +again: >> if (ints == ~(u32)0) { >> ohci->rh_state = OHCI_RH_HALTED; >> ohci_dbg (ohci, "device removed!\n"); >> @@ -982,6 +983,13 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd) >> } >> spin_unlock(&ohci->lock); >> >> + /* repeat until all enabled interrupts are handled */ >> + if (ohci->rh_state != OHCI_RH_HALTED) { >> + ints = ohci_readl(ohci, ®s->intrstatus); >> + if (ints & ohci_readl(ohci, ®s->intrenable)) > > Doesn't the driver know which interrupts are enabled? > So it should be able to avoid doing two (likely) slow io reads? > (PCIe reads are pretty much guaranteed to be high latency.) >
No, the driver does not cache intrenable.
Guenter
| |