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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.

  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
> 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.
> 
> Fixes: 8428413b1d14 ("serial: 8250_pci: Implement MSI(-X) support")
> Signed-off-by: Mario Kleiner <mario.kleiner.de@xxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Ralf Ramsauer <ralf.ramsauer@xxxxxxxxxxxxxxxxx>
> ---
>  drivers/tty/serial/8250/8250_pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 780cc99732b6..35fd5c4e831a 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -3964,6 +3964,7 @@ pciserial_init_ports(struct pci_dev *dev, const struct pciserial_board *board)
>  		if (pci_match_id(pci_use_msi, dev)) {
>  			dev_dbg(&dev->dev, "Using MSI(-X) interrupts\n");
>  			pci_set_master(dev);
> +			uart.port.flags &= ~UPF_SHARE_IRQ;
>  			rc = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
>  		} else {
>  			dev_dbg(&dev->dev, "Using legacy interrupts\n");
> 



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux