Re: [PATCH v2 1/2] ns16550: reject IRQ above nr_irqs_gsi

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Tue, May 10, 2022 at 05:46:12PM +0200, Roger Pau Monné wrote:
> On Tue, May 10, 2022 at 02:30:41PM +0000, Andrew Cooper wrote:
> > On 10/05/2022 12:55, Marek Marczykowski-Górecki wrote:
> > > Intel LPSS has INTERRUPT_LINE set to 0xff by default, that can't
> > > possibly work. While a proper IRQ configuration may be useful,
> > > validating value retrieved from the hardware is still necessary. If it
> > > fails, use the device in poll mode, instead of crashing down the line
> > > (at smp_initr_init()). Currently it's
> > > x86-specific, as the surrounding code is guarded with CONFIG_X86 anyway.
> > >
> > > Signed-off-by: Marek Marczykowski-Górecki 
> > > <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > 
> > This isn't invalid per say.  It explicitly means unknown/no connection
> > and is used in practice to mean "never generate line interrupts, even
> > when MSI is disabled".  It's part of PCI 3.0 if the internet is to be
> > believed, but ISTR is mandatory for SRIOV devices where the use of line
> > interrupts is prohibited by the spec.
> > 
> > Also, there are systems where nr_irq_gsi is greater than 0xff.
> > 
> > I'd recommend handling 0xff specially as "no legacy irq", and not
> > involving nr_irq_gsi at all.
> 
> I've finally found the reference for it in (one) PCI specification.
> It's in the PCI Local Bus Specification Revision 3.0 (from 2004) as a
> footnote, so for the reference I'm going to paste it here:
> 
> Interrupt Line
> 
> The Interrupt Line register is an eight-bit register used to
> communicate interrupt line routing information. The register is
> read/write and must be implemented by any device (or device function)
> that uses an interrupt pin. POST software will write the routing
> information into this register as it initializes and configures the
> system.  The value in this register tells which input of the system
> interrupt controller(s) the device's interrupt pin is connected to.
> The device itself does not use this value, rather it is used by device
> drivers and operating systems. Device drivers and operating systems
> can use this information to determine priority and vector information.
> Values in this register are system architecture specific. [43]
> 
> [...]
> 
> [43] For x86 based PCs, the values in this register correspond to IRQ
> numbers (0-15) of the standard dual 8259 configuration. The value 255
> is defined as meaning "unknown" or "no connection" to the interrupt
> controller. Values between 15 and 254 are reserved.
> 
> That note however is completely missed on the newer PCI Express
> specifications.
> 
> Marek, can you please adjust the code as suggested by Andrew and add
> the reference to the commit message?

Sure.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature