Re: [PATCH] acpi: add support for extended IRQ to PCI link

From: Andy Shevchenko
Date: Sun Nov 08 2015 - 15:35:40 EST


On Sun, Nov 8, 2015 at 6:07 PM, Sinan Kaya <okaya@xxxxxxxxxxxxxx> wrote:
> The ACPI compiler uses the extended format when
> used interrupt numbers are greater than 256.
> The PCI link code currently only supports simple
> interrupt format. The IRQ numbers are represented
> using 32 bits when extended IRQ syntax. This patch
> changes the interrupt number type to 32 bits and
> places an upper limit of 1020 as possible interrupt
> id. Additional checks have been placed to prevent
> out of bounds writes.

In commit messages and in comments I see this narrow lines, any reason
to make them short, except increasing number of lines?

> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -1,6 +1,7 @@
> /*
> * pci_link.c - ACPI PCI Interrupt Link Device Driver ($Revision: 34 $)
> *
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> * Copyright (C) 2001, 2002 Andy Grover <andrew.grover@xxxxxxxxx>
> * Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@xxxxxxxxx>
> * Copyright (C) 2002 Dominik Brodowski <devel@xxxxxxxx>
> @@ -67,12 +68,12 @@ static struct acpi_scan_handler pci_link_handler = {
> * later even the link is disable. Instead, we just repick the active irq
> */
> struct acpi_pci_link_irq {
> - u8 active; /* Current IRQ */
> + u32 active; /* Current IRQ */
> u8 triggering; /* All IRQs */
> u8 polarity; /* All IRQs */
> u8 resource_type;
> u8 possible_count;
> - u8 possible[ACPI_PCI_LINK_MAX_POSSIBLE];
> + u32 possible[ACPI_PCI_LINK_MAX_POSSIBLE];
> u8 initialized:1;
> u8 reserved:7;
> };
> @@ -437,7 +438,7 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
> * enabled system.
> */
>
> -#define ACPI_MAX_IRQS 256
> +#define ACPI_MAX_IRQS 1020
> #define ACPI_MAX_ISA_IRQ 16
>
> #define PIRQ_PENALTY_PCI_AVAILABLE (0)
> @@ -493,7 +494,8 @@ int __init acpi_irq_penalty_init(void)
> penalty;
> }
>
> - } else if (link->irq.active) {
> + } else if (link->irq.active &&
> + (link->irq.active < ACPI_MAX_IRQS)) {
> acpi_irq_penalty[link->irq.active] +=
> PIRQ_PENALTY_PCI_POSSIBLE;
> }
> @@ -542,14 +544,19 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link)
> irq = link->irq.possible[link->irq.possible_count - 1];
>
> if (acpi_irq_balance || !link->irq.active) {
> - /*
> - * Select the best IRQ. This is done in reverse to promote
> - * the use of IRQs 9, 10, 11, and >15.
> - */
> - for (i = (link->irq.possible_count - 1); i >= 0; i--) {
> - if (acpi_irq_penalty[irq] >
> - acpi_irq_penalty[link->irq.possible[i]])
> - irq = link->irq.possible[i];
> +
> + if (irq < ACPI_MAX_IRQS) {
> + /*
> + * Select the best IRQ. This is done in reverse to
> + * promote the use of IRQs 9, 10, 11, and >15.
> + */
> + for (i = (link->irq.possible_count - 1); i >= 0;
> + i--) {

Why not

if ((acpi_irq_balance || !link->irq.active) && irq < ACPI_MAX_IRQS) {
int i = link->irq.possible_count;

while (--i) {
â
}
}

> + if ((link->irq.possible[i] < ACPI_MAX_IRQS) &&
> + (acpi_irq_penalty[irq] >
> + acpi_irq_penalty[link->irq.possible[i]]))
> + irq = link->irq.possible[i];
> + }
> }
> }
> if (acpi_irq_penalty[irq] >= PIRQ_PENALTY_ISA_ALWAYS) {

--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/