[PATCH V2] usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI

[PATCH V2] usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI

[David Jeffery]

When MSI is used by the ehci-hcd driver, it can cause lost interrupts which
results in EHCI only continuing to work due to a polling fallback. But the
reliance of polling drastically reduces performance of any I/O through EHCI.

Interrupts are lost as the EHCI interrupt handler does not safely handle
edge-triggered interrupts. It fails to ensure all interrupt status bits are
cleared, which works with level-triggered interrupts but not the
edge-triggered interrupts typical from using MSI.

To fix this problem, check if the driver may have raced with the hardware
setting additional interrupt status bits and clear status until it is in a
stable state.

Signed-off-by: David Jeffery <djeffery@redhat.com>
Tested-by: Laurence Oberman <loberman@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Fixes: 306c54d0edb6 usb: hcd: Try MSI interrupts on PCI devices
---

 ehci-hcd.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

This is an alternative to reverting 306c54d0edb6ba94d39877524dddebaad7770cf2
which is the patch which allowed MSI to be used with EHCI and was tested
against latest 5.14. It should have all the requested cleanups from v1.

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 36f5bf6a0752..10b0365f3439 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -703,24 +703,28 @@ EXPORT_SYMBOL_GPL(ehci_setup);
 static irqreturn_t ehci_irq (struct usb_hcd *hcd)
 {
 	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
-	u32			status, masked_status, pcd_status = 0, cmd;
+	u32			status, current_status, masked_status, pcd_status = 0;
+	u32			cmd;
 	int			bh;
 
 	spin_lock(&ehci->lock);
 
-	status = ehci_readl(ehci, &ehci->regs->status);
+	status = 0;
+	current_status = ehci_readl(ehci, &ehci->regs->status);
+restart:
 
 	/* e.g. cardbus physical eject */
-	if (status == ~(u32) 0) {
+	if (current_status == ~(u32) 0) {
 		ehci_dbg (ehci, "device removed\n");
 		goto dead;
 	}
+	status |= current_status;
 
 	/*
 	 * We don't use STS_FLR, but some controllers don't like it to
 	 * remain on, so mask it out along with the other status bits.
 	 */
-	masked_status = status & (INTR_MASK | STS_FLR);
+	masked_status = current_status & (INTR_MASK | STS_FLR);
 
 	/* Shared IRQ? */
 	if (!masked_status || unlikely(ehci->rh_state == EHCI_RH_HALTED)) {
@@ -730,6 +734,12 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
 
 	/* clear (just) interrupts */
 	ehci_writel(ehci, masked_status, &ehci->regs->status);
+
+	/* For edge interrupts, don't race with an interrupt bit being raised */
+	current_status = ehci_readl(ehci, &ehci->regs->status);
+	if (current_status & INTR_MASK)
+		goto restart;
+
 	cmd = ehci_readl(ehci, &ehci->regs->command);
 	bh = 0;
 

Re: [PATCH V2] usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI

[Andy Shevchenko]

On Thu, Jul 15, 2021 at 05:37:44PM -0400, David Jeffery wrote:
> When MSI is used by the ehci-hcd driver, it can cause lost interrupts which
> results in EHCI only continuing to work due to a polling fallback. But the
> reliance of polling drastically reduces performance of any I/O through EHCI.
> 
> Interrupts are lost as the EHCI interrupt handler does not safely handle
> edge-triggered interrupts. It fails to ensure all interrupt status bits are
> cleared, which works with level-triggered interrupts but not the
> edge-triggered interrupts typical from using MSI.
> 
> To fix this problem, check if the driver may have raced with the hardware
> setting additional interrupt status bits and clear status until it is in a
> stable state.
> 
> Signed-off-by: David Jeffery <djeffery@redhat.com>
> Tested-by: Laurence Oberman <loberman@redhat.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

> Fixes: 306c54d0edb6 usb: hcd: Try MSI interrupts on PCI devices

Improper format.

Add to your .gitconfig the following:

[alias]
	one = show -s --pretty='format:%h (\"%s\")'
	c = show -s --pretty='format:https://git.kernel.org/torvalds/c/%h'

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V2] usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI

[Andy Shevchenko]

On Fri, Jul 16, 2021 at 04:36:18PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 15, 2021 at 05:37:44PM -0400, David Jeffery wrote:
> > When MSI is used by the ehci-hcd driver, it can cause lost interrupts which
> > results in EHCI only continuing to work due to a polling fallback. But the
> > reliance of polling drastically reduces performance of any I/O through EHCI.
> > 
> > Interrupts are lost as the EHCI interrupt handler does not safely handle
> > edge-triggered interrupts. It fails to ensure all interrupt status bits are
> > cleared, which works with level-triggered interrupts but not the
> > edge-triggered interrupts typical from using MSI.
> > 
> > To fix this problem, check if the driver may have raced with the hardware
> > setting additional interrupt status bits and clear status until it is in a
> > stable state.
> > 
> > Signed-off-by: David Jeffery <djeffery@redhat.com>
> > Tested-by: Laurence Oberman <loberman@redhat.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> 
> > Fixes: 306c54d0edb6 usb: hcd: Try MSI interrupts on PCI devices
> 
> Improper format.
> 
> Add to your .gitconfig the following:
> 
> [alias]
> 	one = show -s --pretty='format:%h (\"%s\")'
> 	c = show -s --pretty='format:https://git.kernel.org/torvalds/c/%h'

And this

[core]
	abbrev = 12

`git one $COMMIT` will give you a nice representation.

-- 
With Best Regards,
Andy Shevchenko