Re: Patch "usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI" has been added to the 5.10-stable tree

[Greg KH]

On Mon, Jul 26, 2021 at 12:17:57PM +0200, Greg KH wrote:
> On Mon, Jul 26, 2021 at 01:05:04PM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 26, 2021 at 10:21:06AM +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> > > 
> > > This is a note to let you know that I've just added the patch titled
> > > 
> > >     usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI
> > > 
> > > to the 5.10-stable tree which can be found at:
> > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > 
> > > The filename of the patch is:
> > >      usb-ehci-prevent-missed-ehci-interrupts-with-edge-triggered-msi.patch
> > > and it can be found in the queue-5.10 subdirectory.
> > > 
> > > If you, or anyone else, feels it should not be added to the stable tree,
> > > please let <stable@xxxxxxxxxxxxxxx> know about it.
> > > 
> > > 
> > > From 0b60557230adfdeb8164e0b342ac9cd469a75759 Mon Sep 17 00:00:00 2001
> > > From: David Jeffery <djeffery@xxxxxxxxxx>
> > > Date: Thu, 15 Jul 2021 17:37:44 -0400
> > > Subject: usb: ehci: Prevent missed ehci interrupts with edge-triggered MSI
> > > 
> > > From: David Jeffery <djeffery@xxxxxxxxxx>
> > > 
> > > commit 0b60557230adfdeb8164e0b342ac9cd469a75759 upstream.
> > > 
> > > 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.
> > > 
> > > Fixes: 306c54d0edb6 ("usb: hcd: Try MSI interrupts on PCI devices")
> > > Tested-by: Laurence Oberman <loberman@xxxxxxxxxx>
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > > Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: David Jeffery <djeffery@xxxxxxxxxx>
> > > Link: https://lore.kernel.org/r/20210715213744.GA44506@redhat
> > > Cc: stable <stable@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > ---
> > >  drivers/usb/host/ehci-hcd.c |   17 +++++++++++++----
> > >  1 file changed, 13 insertions(+), 4 deletions(-)
> > > 
> > > --- a/drivers/usb/host/ehci-hcd.c
> > > +++ b/drivers/usb/host/ehci-hcd.c
> > > @@ -703,7 +703,8 @@ 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;
> > >  	unsigned long		flags;
> > >  
> > > @@ -715,10 +716,12 @@ static irqreturn_t ehci_irq (struct usb_
> > >  	 */
> > >  	spin_lock_irqsave(&ehci->lock, flags);
> > >  
> > > -	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;
> > >  	}
> > 
> > I'm wondering if we should do the same here as in the original patch, i.e.
> > 
> >     status |= current_status;
> 
> Ugh, my fault, I missed this in the backport, thanks!  I'll go fix that
> up...

Should now be fixed up, thanks for catching this.

greg k-h