| Message ID | 20220510155824.1779789-1-marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] ns16550: reject IRQ above nr_irqs_gsi | expand |
Subject line needs to be updated :). On Tue, May 10, 2022 at 05:58:23PM +0200, Marek Marczykowski-Górecki wrote: > Intel LPSS has INTERRUPT_LINE set to 0xff by default, that is declared > by the PCI Local Bus Specification Revision 3.0 (from 2004) as > "unknown"/"no connection". Fallback to poll mode in this case. > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > Changes in v3: > - change back to checking 0xff explicitly > - adjust commit message, include spec reference > - change warning to match the above > Changes in v2: > - add log message > - extend commit message > - code style fix > --- > xen/drivers/char/ns16550.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index fb75cee4a13a..b4434ad815e1 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -1238,6 +1238,15 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx) > pci_conf_read8(PCI_SBDF(0, b, d, f), > PCI_INTERRUPT_LINE) : 0; > > + if ( uart->irq == 0xff ) > + { > + printk(XENLOG_WARNING XENLOG_INFO would be better, IMO this configuration is no reason to warn the user. > + "ns16550: %02x:%02x.%u has no legacy IRQ %d, " > + "falling back to a poll mode\n", Could you use %pp and then pass the parameter using &PCI_SBDF(0, b, d, f)? Also we try to avoid splitting printk format strings, what about using: "ns16550: %02x:%02x.%u no legacy IRQ, using poll mode\n" TBH, we don't print a similar message if INTERRUPT_{PIN,LINE} is 0 (which also results in the console running in poll mode), so I wonder if we should extend the printing of the message also to ->irq == 0 for consistency. Thanks, Roger.
On Wed, May 11, 2022 at 09:20:46AM +0200, Roger Pau Monné wrote: > Subject line needs to be updated :). > > On Tue, May 10, 2022 at 05:58:23PM +0200, Marek Marczykowski-Górecki wrote: > > Intel LPSS has INTERRUPT_LINE set to 0xff by default, that is declared > > by the PCI Local Bus Specification Revision 3.0 (from 2004) as > > "unknown"/"no connection". Fallback to poll mode in this case. Forgot to comment: you should also mention that this 0xff special handling is for x86 only, other arches can use other meanings for the INTERRUPT_LINE register. Likely this also implies that the 0xff check should be protected by CONFIG_X86 (albeit I think that code is already only reachable from x86 as Arm doesn't yet enable CONFIG_PCI). Thanks, Roger.
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index fb75cee4a13a..b4434ad815e1 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1238,6 +1238,15 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx) pci_conf_read8(PCI_SBDF(0, b, d, f), PCI_INTERRUPT_LINE) : 0; + if ( uart->irq == 0xff ) + { + printk(XENLOG_WARNING + "ns16550: %02x:%02x.%u has no legacy IRQ %d, " + "falling back to a poll mode\n", + b, d, f, uart->irq); + uart->irq = 0; + } + return 0; } }
Intel LPSS has INTERRUPT_LINE set to 0xff by default, that is declared by the PCI Local Bus Specification Revision 3.0 (from 2004) as "unknown"/"no connection". Fallback to poll mode in this case. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v3: - change back to checking 0xff explicitly - adjust commit message, include spec reference - change warning to match the above Changes in v2: - add log message - extend commit message - code style fix --- xen/drivers/char/ns16550.c | 9 +++++++++ 1 file changed, 9 insertions(+)