[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 1/2] ns16550: reject IRQ above nr_irqs
On 10.03.2022 17:21, Julien Grall wrote: > On 10/03/2022 16:12, Roger Pau Monné wrote: >> On Thu, Mar 10, 2022 at 05:08:07PM +0100, Jan Beulich wrote: >>> On 10.03.2022 16:47, Roger Pau Monné wrote: >>>> On Thu, Mar 10, 2022 at 04:23:00PM +0100, Jan Beulich wrote: >>>>> On 10.03.2022 15:34, Marek Marczykowski-Górecki wrote: >>>>>> --- a/xen/drivers/char/ns16550.c >>>>>> +++ b/xen/drivers/char/ns16550.c >>>>>> @@ -1221,6 +1221,9 @@ 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 >= nr_irqs) >>>>>> + uart->irq = 0; >>>>> >>>>> Don't you mean nr_irqs_gsi here? Also (nit) please add the missing blanks >>>>> immediately inside the parentheses. >>>> >>>> If we use nr_irqs_gsi we will need to make the check x86 only AFAICT. >>> >>> Down the road (when Arm wants to select HAS_PCI) - yes. Not necessarily >>> right away. After all Arm wants to have an equivalent check here then, >>> not merely checking against nr_irqs instead. So putting a conditional >>> here right away would hide the need for putting in place an Arm-specific >>> alternative. >> >> Oh, I always forget Arm doesn't have CONFIG_HAS_PCI enabled just yet. > The PCI code in ns16550.c is gated by CONFIG_HAS_PCI and CONFIG_X86. I > am not sure we will ever see a support for PCI UART card in Xen on Arm. > > However, if it evers happens then neither nr_irqs or nr_irqs_gsi would > help here because from the interrupt controller PoV 0xff may be a valid > (GICv2 supports up to 1024 interrupts). > > Is there any reason we can't explicitely check 0xff? FF isn't called out by the spec as having a special meaning. Unlike I think Andrew did say somewhere, FF does not indicate "none". That's instead indicated by PIN returning zero. That's my reading of the spec, at least. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |