[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5644AA63.2070205@codeaurora.org>
Date: Thu, 12 Nov 2015 10:04:03 -0500
From: Sinan Kaya <okaya@...eaurora.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
Timur Tabi <timur@...eaurora.org>, cov@...eaurora.org,
jcm@...hat.com, "Rafael J. Wysocki" <rjw@...ysocki.net>,
Len Brown <lenb@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] acpi: add support for extended IRQ to PCI link
On 11/12/2015 4:56 AM, Andy Shevchenko wrote:
> On Thu, Nov 12, 2015 at 8:14 AM, Sinan Kaya <okaya@...eaurora.org> 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.
>>
>> 1020 is the maximum interrupt ID that can be assigned to
>> an ARM SPI interrupt according to ARM architecture.
>
> I think it worth to put these lines in the code as well.
>
ok
>>
>> Additional checks have been placed to prevent out of bounds
>> writes.
>>
>> Signed-off-by: Sinan Kaya <okaya@...eaurora.org>
>> ---
>> drivers/acpi/pci_link.c | 28 +++++++++++++++++-----------
>> 1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
>> index 7c8408b..a2becab 100644
>> --- 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@...el.com>
>> * Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@...el.com>
>> * Copyright (C) 2002 Dominik Brodowski <devel@...do.de>
>> @@ -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;
>> }
>> @@ -541,14 +543,16 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link)
>> else
>> irq = link->irq.possible[link->irq.possible_count - 1];
>>
>> - if (acpi_irq_balance || !link->irq.active) {
>> + if ((acpi_irq_balance || !link->irq.active) && (irq < ACPI_MAX_IRQS)) {
>> /*
>
>> - * Select the best IRQ. This is done in reverse to promote
>> - * the use of IRQs 9, 10, 11, and >15.
>> + * Select the best IRQ. This is done in reverse to
>> + * promote the use of IRQs 9, 10, 11, and >15.
>
> What was changed here?
See your comments here.
https://lkml.org/lkml/2015/11/8/231
>
>> */
>> - for (i = (link->irq.possible_count - 1); i >= 0; i--) {
>> - if (acpi_irq_penalty[irq] >
>> - acpi_irq_penalty[link->irq.possible[i]])
>> + 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];
>> }
>> }
>> @@ -568,7 +572,9 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link)
>> acpi_device_bid(link->device));
>> return -ENODEV;
>> } else {
>> - acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
>> + if (link->irq.active < ACPI_MAX_IRQS)
>> + acpi_irq_penalty[link->irq.active] +=
>> + PIRQ_PENALTY_PCI_USING;
>> printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
>> acpi_device_name(link->device),
>> acpi_device_bid(link->device), link->irq.active);
>
>
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists