Re: [EXT] [PATCH] serial: 8250_pci: Avoid irq sharing for MSI(-X) interrupts.

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Jul 29, 2021 at 06:25:54PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 29, 2021 at 11:37 AM Ralf Ramsauer
> <ralf.ramsauer@oth-regensburg.de> wrote:
> > On 29/07/2021 06:33, Mario Kleiner wrote:
> > > This attempts to fix a bug found with a serial port card which uses
> > > an MCS9922 chip, one of the 4 models for which MSI-X interrupts are
> > > currently supported. I don't possess such a card, and i'm not
> > > experienced with the serial subsystem, so this patch is based on what
> > > i think i found as a likely reason for failure, based on walking the
> > > user who actually owns the card through some diagnostic.
> >
> > As there's currently some (stuck) discussion on how to generally handle
> > MSI capable serial cards, and this is issue related to some degree, let
> > me Cc Jiri and Andy.
> 
> Thanks, Ralf!
> 
> > > The user who reported the problem finds the following in his dmesg
> > > output for the relevant ttyS4 and ttyS5:
> > >
> > > [    0.580425] serial 0000:02:00.0: enabling device (0000 -> 0003)
> > > [    0.601448] 0000:02:00.0: ttyS4 at I/O 0x3010 (irq = 125, base_baud = 115200) is a ST16650V2
> > > [    0.603089] serial 0000:02:00.1: enabling device (0000 -> 0003)
> > > [    0.624119] 0000:02:00.1: ttyS5 at I/O 0x3000 (irq = 126, base_baud = 115200) is a ST16650V2
> > > ...
> > > [    6.323784] genirq: Flags mismatch irq 128. 00000080 (ttyS5) vs. 00000000 (xhci_hcd)
> > > [    6.324128] genirq: Flags mismatch irq 128. 00000080 (ttyS5) vs. 00000000 (xhci_hcd)
> > > ...
> > >
> > > Output of setserial -a:
> > >
> > > /dev/ttyS4, Line 4, UART: 16650V2, Port: 0x3010, IRQ: 127
> > >       Baud_base: 115200, close_delay: 50, divisor: 0
> > >       closing_wait: 3000
> > >       Flags: spd_normal skip_test
> > >
> > > This suggests to me that the serial driver wants to register and share a
> > > MSI/MSI-X irq 128 with the xhci_hcd driver, whereas the xhci driver does
> > > not want to share the irq, as flags 0x00000080 (== IRQF_SHARED) from the
> > > serial port driver means to share the irq, and this mismatch ends in some
> > > failed irq init?
> > >
> > > With this setup, data reception works very unreliable, with dropped data,
> > > already at a transmission rate of only a 16 Bytes chunk every 1/120th of
> > > a second, ie. 1920 Bytes/sec, presumably due to rx fifo overflow due to
> > > mishandled or not used at all rx irq's?
> > >
> > > See full discussion thread with attempted diagnosis at:
> > >
> > > https://psychtoolbox.discourse.group/t/issues-with-iscan-serial-port-recording/3886
> > >
> > > Disabling the use of MSI interrupts for the serial port pci card did
> > > fix the reliability problems. The user executed the following sequence
> > > of commands to achieve this:
> > >
> > > # Disable PCI serial port driver, shut down card:
> > > echo 0000:02:00.0 | sudo tee /sys/bus/pci/drivers/serial/unbind
> > > echo 0000:02:00.1 | sudo tee /sys/bus/pci/drivers/serial/unbind
> > >
> > > # Disallow use of MSI/MSI-X interrupts on pci serial port card:
> > > echo 0 | sudo tee /sys/bus/pci/devices/0000:02:00.0/msi_bus
> > > echo 0 | sudo tee /sys/bus/pci/devices/0000:02:00.1/msi_bus
> > >
> > > # Restart driver, reinitialize card, hopefully without MSI irqs now:
> > > echo 0000:02:00.0 | sudo tee /sys/bus/pci/drivers/serial/bind
> > > echo 0000:02:00.1 | sudo tee /sys/bus/pci/drivers/serial/bind
> > >
> > > This resulted in the following log output:
> > >
> > > [   82.179021] pci 0000:02:00.0: MSI/MSI-X disallowed for future drivers
> > > [   87.003031] pci 0000:02:00.1: MSI/MSI-X disallowed for future drivers
> > > [   98.537010] 0000:02:00.0: ttyS4 at I/O 0x3010 (irq = 17, base_baud = 115200) is a ST16650V2
> > > [  103.648124] 0000:02:00.1: ttyS5 at I/O 0x3000 (irq = 18, base_baud = 115200) is a ST16650V2
> > >
> > > This patch attempts to fix the problem by disabling irq sharing when
> > > using MSI irq's. Note that all i know for sure is that disabling MSI
> 
> In general the shared MSI interrupts are weird things that can be done
> with IRQs.
> 
> > > irq's fixed the problem for the user, so this patch could be wrong and
> > > is untested. Please review with caution, keeping this in mind.
> 
> I think it's a good idea in general. I have no objections.

So is that a "reviewed-by" for this patch?

thanks,

greg k-h