* [PATCH V4 0/3] ACPI,PCI,IRQ: revert penalty calculation for ISA and SCI interrupts @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: linux-pci, agross, linux-arm-msm, linux-arm-kernel, wim, Sinan Kaya By the time ACPI gets initialized, this code tries to determine an IRQ number based on penalty values in this array. It will try to locate the IRQ with the least penalty assignment so that interrupt sharing is avoided if possible. A couple of notes about the external APIs: 1. These API can be called before the ACPI is started. Therefore, one cannot assume that the PCI link objects are initialized for calculating penalties. 2. The polarity and trigger information passed via the acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem is reporting as the call might have been placed before the IRQ is registered by the interrupt subsystem. The reverted changes were in the direction to remove these external API and try to calculate the penalties at runtime for the ISA, SCI as well as PCI IRQS. This didn't work out well with the existing platforms. V4: * Drop ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts * A new patch to isolate early boot ISA penalty calculations from dynamic penalty calculation by directly modifying the array members in ("ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages") * Now that we isolated both SCI and ISA interrupts, revert commit ("Revert "ACPI,PCI,IRQ: separate ISA penalty calculation"") and commit ("487cf917ed0d (Revert "ACPI, PCI, IRQ: remove redundant code in acpi_irq_penalty_init()") to share code between ISA and PCI penalties as originally intended. V3: http://www.spinics.net/lists/arm-kernel/msg536208.html * drop patch #1 as discussed with Bjorn * add patch #3 to track SCI irq and penalty separately V2: https://lkml.org/lkml/2016/10/1/106 * Commit message updates V1: http://lists-archives.com/linux-kernel/28673954-revert-acpi-pci-irq-reduce-static-irq-array-size-to-16.html * initial implementation Sinan Kaya (3): ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages Revert "ACPI,PCI,IRQ: remove SCI penalize function" Revert "ACPI,PCI,IRQ: separate ISA penalty calculation" arch/x86/kernel/acpi/boot.c | 1 + arch/x86/pci/acpi.c | 1 - drivers/acpi/pci_link.c | 72 ++++++++++++--------------------------------- include/acpi/acpi_drivers.h | 1 - include/linux/acpi.h | 1 + 5 files changed, 21 insertions(+), 55 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 0/3] ACPI, PCI, IRQ: revert penalty calculation for ISA and SCI interrupts @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-arm-kernel By the time ACPI gets initialized, this code tries to determine an IRQ number based on penalty values in this array. It will try to locate the IRQ with the least penalty assignment so that interrupt sharing is avoided if possible. A couple of notes about the external APIs: 1. These API can be called before the ACPI is started. Therefore, one cannot assume that the PCI link objects are initialized for calculating penalties. 2. The polarity and trigger information passed via the acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem is reporting as the call might have been placed before the IRQ is registered by the interrupt subsystem. The reverted changes were in the direction to remove these external API and try to calculate the penalties at runtime for the ISA, SCI as well as PCI IRQS. This didn't work out well with the existing platforms. V4: * Drop ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts * A new patch to isolate early boot ISA penalty calculations from dynamic penalty calculation by directly modifying the array members in ("ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages") * Now that we isolated both SCI and ISA interrupts, revert commit ("Revert "ACPI,PCI,IRQ: separate ISA penalty calculation"") and commit ("487cf917ed0d (Revert "ACPI, PCI, IRQ: remove redundant code in acpi_irq_penalty_init()") to share code between ISA and PCI penalties as originally intended. V3: http://www.spinics.net/lists/arm-kernel/msg536208.html * drop patch #1 as discussed with Bjorn * add patch #3 to track SCI irq and penalty separately V2: https://lkml.org/lkml/2016/10/1/106 * Commit message updates V1: http://lists-archives.com/linux-kernel/28673954-revert-acpi-pci-irq-reduce-static-irq-array-size-to-16.html * initial implementation Sinan Kaya (3): ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages Revert "ACPI,PCI,IRQ: remove SCI penalize function" Revert "ACPI,PCI,IRQ: separate ISA penalty calculation" arch/x86/kernel/acpi/boot.c | 1 + arch/x86/pci/acpi.c | 1 - drivers/acpi/pci_link.c | 72 ++++++++++++--------------------------------- include/acpi/acpi_drivers.h | 1 - include/linux/acpi.h | 1 + 5 files changed, 21 insertions(+), 55 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 0/3] ACPI, PCI, IRQ: revert penalty calculation for ISA and SCI interrupts @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: linux-pci, agross, Sinan Kaya, linux-arm-msm, wim, linux-arm-kernel By the time ACPI gets initialized, this code tries to determine an IRQ number based on penalty values in this array. It will try to locate the IRQ with the least penalty assignment so that interrupt sharing is avoided if possible. A couple of notes about the external APIs: 1. These API can be called before the ACPI is started. Therefore, one cannot assume that the PCI link objects are initialized for calculating penalties. 2. The polarity and trigger information passed via the acpi_penalize_sci_irq from the BIOS may not match what the IRQ subsystem is reporting as the call might have been placed before the IRQ is registered by the interrupt subsystem. The reverted changes were in the direction to remove these external API and try to calculate the penalties at runtime for the ISA, SCI as well as PCI IRQS. This didn't work out well with the existing platforms. V4: * Drop ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts * A new patch to isolate early boot ISA penalty calculations from dynamic penalty calculation by directly modifying the array members in ("ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages") * Now that we isolated both SCI and ISA interrupts, revert commit ("Revert "ACPI,PCI,IRQ: separate ISA penalty calculation"") and commit ("487cf917ed0d (Revert "ACPI, PCI, IRQ: remove redundant code in acpi_irq_penalty_init()") to share code between ISA and PCI penalties as originally intended. V3: http://www.spinics.net/lists/arm-kernel/msg536208.html * drop patch #1 as discussed with Bjorn * add patch #3 to track SCI irq and penalty separately V2: https://lkml.org/lkml/2016/10/1/106 * Commit message updates V1: http://lists-archives.com/linux-kernel/28673954-revert-acpi-pci-irq-reduce-static-irq-array-size-to-16.html * initial implementation Sinan Kaya (3): ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages Revert "ACPI,PCI,IRQ: remove SCI penalize function" Revert "ACPI,PCI,IRQ: separate ISA penalty calculation" arch/x86/kernel/acpi/boot.c | 1 + arch/x86/pci/acpi.c | 1 - drivers/acpi/pci_link.c | 72 ++++++++++++--------------------------------- include/acpi/acpi_drivers.h | 1 - include/linux/acpi.h | 1 + 5 files changed, 21 insertions(+), 55 deletions(-) -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-19 22:21 ` Sinan Kaya (?) @ 2016-10-19 22:21 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: linux-pci, agross, linux-arm-msm, linux-arm-kernel, wim, Sinan Kaya, Len Brown, linux-kernel The penalty determination of ISA IRQ goes through 4 paths. 1. assign PCI_USING during power up via acpi_irq_penalty_init. 2. update the penalty with acpi_penalize_isa_irq function based on the active parameter. 3. kernel command line penalty update via acpi_irq_penalty_update function. 4. increment the penalty as USING right after the IRQ is assign to PCI. acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called before the ACPI subsystem is started. These API need to bypass the acpi_irq_get_penalty function. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/acpi/pci_link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index c983bf7..4f37938 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) continue; if (used) - new_penalty = acpi_irq_get_penalty(irq) + + new_penalty = acpi_isa_irq_penalty[irq] + PIRQ_PENALTY_ISA_USED; else new_penalty = 0; @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) void acpi_penalize_isa_irq(int irq, int active) { if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-arm-kernel The penalty determination of ISA IRQ goes through 4 paths. 1. assign PCI_USING during power up via acpi_irq_penalty_init. 2. update the penalty with acpi_penalize_isa_irq function based on the active parameter. 3. kernel command line penalty update via acpi_irq_penalty_update function. 4. increment the penalty as USING right after the IRQ is assign to PCI. acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called before the ACPI subsystem is started. These API need to bypass the acpi_irq_get_penalty function. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/acpi/pci_link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index c983bf7..4f37938 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) continue; if (used) - new_penalty = acpi_irq_get_penalty(irq) + + new_penalty = acpi_isa_irq_penalty[irq] + PIRQ_PENALTY_ISA_USED; else new_penalty = 0; @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) void acpi_penalize_isa_irq(int irq, int active) { if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: linux-pci, agross, linux-kernel, Sinan Kaya, linux-arm-msm, wim, linux-arm-kernel, Len Brown The penalty determination of ISA IRQ goes through 4 paths. 1. assign PCI_USING during power up via acpi_irq_penalty_init. 2. update the penalty with acpi_penalize_isa_irq function based on the active parameter. 3. kernel command line penalty update via acpi_irq_penalty_update function. 4. increment the penalty as USING right after the IRQ is assign to PCI. acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called before the ACPI subsystem is started. These API need to bypass the acpi_irq_get_penalty function. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/acpi/pci_link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index c983bf7..4f37938 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) continue; if (used) - new_penalty = acpi_irq_get_penalty(irq) + + new_penalty = acpi_isa_irq_penalty[irq] + PIRQ_PENALTY_ISA_USED; else new_penalty = 0; @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) void acpi_penalize_isa_irq(int irq, int active) { if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); } -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 56+ messages in thread
* Re: [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-19 22:21 ` Sinan Kaya (?) @ 2016-10-20 21:39 ` Rafael J. Wysocki -1 siblings, 0 replies; 56+ messages in thread From: Rafael J. Wysocki @ 2016-10-20 21:39 UTC (permalink / raw) To: Sinan Kaya Cc: ACPI Devel Maling List, Rafael J. Wysocki, Bjorn Helgaas, ravikanth.nalla, Ondrej Zary, Timur Tabi, Christopher Covington, Jon Masters, Alex Williamson, Linux PCI, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, Linux Kernel Mailing List On Thu, Oct 20, 2016 at 12:21 AM, Sinan Kaya <okaya@codeaurora.org> wrote: > The penalty determination of ISA IRQ goes through 4 paths. > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > 2. update the penalty with acpi_penalize_isa_irq function based on the > active parameter. > 3. kernel command line penalty update via acpi_irq_penalty_update function. > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > before the ACPI subsystem is started. > > These API need to bypass the acpi_irq_get_penalty function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/acpi/pci_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index c983bf7..4f37938 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > continue; > > if (used) > - new_penalty = acpi_irq_get_penalty(irq) + > + new_penalty = acpi_isa_irq_penalty[irq] + > PIRQ_PENALTY_ISA_USED; > else > new_penalty = 0; > @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > void acpi_penalize_isa_irq(int irq, int active) > { > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + This looks slightly odd. What about + acpi_isa_irq_penalty[irq] += > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); > } > Thanks, Rafael ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-20 21:39 ` Rafael J. Wysocki 0 siblings, 0 replies; 56+ messages in thread From: Rafael J. Wysocki @ 2016-10-20 21:39 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 20, 2016 at 12:21 AM, Sinan Kaya <okaya@codeaurora.org> wrote: > The penalty determination of ISA IRQ goes through 4 paths. > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > 2. update the penalty with acpi_penalize_isa_irq function based on the > active parameter. > 3. kernel command line penalty update via acpi_irq_penalty_update function. > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > before the ACPI subsystem is started. > > These API need to bypass the acpi_irq_get_penalty function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/acpi/pci_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index c983bf7..4f37938 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > continue; > > if (used) > - new_penalty = acpi_irq_get_penalty(irq) + > + new_penalty = acpi_isa_irq_penalty[irq] + > PIRQ_PENALTY_ISA_USED; > else > new_penalty = 0; > @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > void acpi_penalize_isa_irq(int irq, int active) > { > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + This looks slightly odd. What about + acpi_isa_irq_penalty[irq] += > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); > } > Thanks, Rafael ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-20 21:39 ` Rafael J. Wysocki 0 siblings, 0 replies; 56+ messages in thread From: Rafael J. Wysocki @ 2016-10-20 21:39 UTC (permalink / raw) To: Sinan Kaya Cc: ACPI Devel Maling List, Rafael J. Wysocki, Bjorn Helgaas, ravikanth.nalla, Ondrej Zary, Timur Tabi, Christopher Covington, Jon Masters, Alex Williamson, Linux PCI, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, Linux Kernel Mailing List On Thu, Oct 20, 2016 at 12:21 AM, Sinan Kaya <okaya@codeaurora.org> wrote: > The penalty determination of ISA IRQ goes through 4 paths. > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > 2. update the penalty with acpi_penalize_isa_irq function based on the > active parameter. > 3. kernel command line penalty update via acpi_irq_penalty_update function. > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > before the ACPI subsystem is started. > > These API need to bypass the acpi_irq_get_penalty function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/acpi/pci_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index c983bf7..4f37938 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > continue; > > if (used) > - new_penalty = acpi_irq_get_penalty(irq) + > + new_penalty = acpi_isa_irq_penalty[irq] + > PIRQ_PENALTY_ISA_USED; > else > new_penalty = 0; > @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > void acpi_penalize_isa_irq(int irq, int active) > { > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + This looks slightly odd. What about + acpi_isa_irq_penalty[irq] += > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); > } > Thanks, Rafael ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-20 21:39 ` Rafael J. Wysocki (?) @ 2016-10-24 3:48 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:48 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, Rafael J. Wysocki, Bjorn Helgaas, ravikanth.nalla, Ondrej Zary, Timur Tabi, Christopher Covington, Jon Masters, Alex Williamson, Linux PCI, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, Linux Kernel Mailing List On 10/20/2016 5:39 PM, Rafael J. Wysocki wrote: >> @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) >> > void acpi_penalize_isa_irq(int irq, int active) >> > { >> > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) >> > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + >> > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + > This looks slightly odd. What about > > + acpi_isa_irq_penalty[irq] += > >> > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); >> > } Makes sense. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-24 3:48 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:48 UTC (permalink / raw) To: linux-arm-kernel On 10/20/2016 5:39 PM, Rafael J. Wysocki wrote: >> @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) >> > void acpi_penalize_isa_irq(int irq, int active) >> > { >> > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) >> > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + >> > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + > This looks slightly odd. What about > > + acpi_isa_irq_penalty[irq] += > >> > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); >> > } Makes sense. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-24 3:48 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:48 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, Rafael J. Wysocki, Bjorn Helgaas, ravikanth.nalla, Ondrej Zary, Timur Tabi, Christopher Covington, Jon Masters, Alex Williamson, Linux PCI, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, Linux Kernel Mailing List On 10/20/2016 5:39 PM, Rafael J. Wysocki wrote: >> @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) >> > void acpi_penalize_isa_irq(int irq, int active) >> > { >> > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) >> > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + >> > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + > This looks slightly odd. What about > > + acpi_isa_irq_penalty[irq] += > >> > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); >> > } Makes sense. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-19 22:21 ` Sinan Kaya @ 2016-10-21 1:39 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 1:39 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, agross, linux-kernel, linux-arm-msm, wim, linux-arm-kernel, Len Brown On Wed, Oct 19, 2016 at 06:21:02PM -0400, Sinan Kaya wrote: > The penalty determination of ISA IRQ goes through 4 paths. > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > 2. update the penalty with acpi_penalize_isa_irq function based on the > active parameter. > 3. kernel command line penalty update via acpi_irq_penalty_update function. > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > before the ACPI subsystem is started. > > These API need to bypass the acpi_irq_get_penalty function. I don't mind this patch, but the changelog doesn't tell me what's broken and why we need this fix. Apparently acpi_irq_get_penalty() doesn't work before ACPI is initialized, but I don't see *why* it wouldn't work. However, I see one bug it *does* fix: we do not store the SCI penalty in the acpi_isa_irq_penalty[] table because acpi_isa_irq_penalty[] only holds ISA IRQ penalties, and there's no guarantee that the SCI is an ISA IRQ. But prior to this patch, we added in the SCI penalty to the acpi_isa_irq_penalty[] entry when the SCI was an ISA IRQ, which makes acpi_irq_get_penalty() return the wrong thing. Consider: Initially acpi_isa_irq_penalty[9] = 0. Assume sci_interrupt = 9. Then acpi_irq_get_penalty(9) returns X. If we call acpi_penalize_isa_irq(9, 1), it sets acpi_isa_irq_penalty[9] = X, and now acpi_irq_get_penalty(9) returns X + X. I'd propose a changelog like this: We do not want to store the SCI penalty in the acpi_isa_irq_penalty[] table because acpi_isa_irq_penalty[] only holds ISA IRQ penalties and there's no guarantee that the SCI is an ISA IRQ. We add in the SCI penalty as a special case in acpi_irq_get_penalty(). But if we called acpi_penalize_isa_irq() or acpi_irq_penalty_update() for an SCI that happened to be an ISA IRQ, they stored the SCI penalty (part of the acpi_irq_get_penalty() return value) in acpi_isa_irq_penalty[]. Subsequent calls to acpi_irq_get_penalty() returned a penalty that included *two* SCI penalties. If this actually fixes a worse problem related to ACPI initialization, of course you should detail that. Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/acpi/pci_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index c983bf7..4f37938 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > continue; > > if (used) > - new_penalty = acpi_irq_get_penalty(irq) + > + new_penalty = acpi_isa_irq_penalty[irq] + > PIRQ_PENALTY_ISA_USED; > else > new_penalty = 0; > @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > void acpi_penalize_isa_irq(int irq, int active) > { > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); > } > > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-21 1:39 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 1:39 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 19, 2016 at 06:21:02PM -0400, Sinan Kaya wrote: > The penalty determination of ISA IRQ goes through 4 paths. > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > 2. update the penalty with acpi_penalize_isa_irq function based on the > active parameter. > 3. kernel command line penalty update via acpi_irq_penalty_update function. > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > before the ACPI subsystem is started. > > These API need to bypass the acpi_irq_get_penalty function. I don't mind this patch, but the changelog doesn't tell me what's broken and why we need this fix. Apparently acpi_irq_get_penalty() doesn't work before ACPI is initialized, but I don't see *why* it wouldn't work. However, I see one bug it *does* fix: we do not store the SCI penalty in the acpi_isa_irq_penalty[] table because acpi_isa_irq_penalty[] only holds ISA IRQ penalties, and there's no guarantee that the SCI is an ISA IRQ. But prior to this patch, we added in the SCI penalty to the acpi_isa_irq_penalty[] entry when the SCI was an ISA IRQ, which makes acpi_irq_get_penalty() return the wrong thing. Consider: Initially acpi_isa_irq_penalty[9] = 0. Assume sci_interrupt = 9. Then acpi_irq_get_penalty(9) returns X. If we call acpi_penalize_isa_irq(9, 1), it sets acpi_isa_irq_penalty[9] = X, and now acpi_irq_get_penalty(9) returns X + X. I'd propose a changelog like this: We do not want to store the SCI penalty in the acpi_isa_irq_penalty[] table because acpi_isa_irq_penalty[] only holds ISA IRQ penalties and there's no guarantee that the SCI is an ISA IRQ. We add in the SCI penalty as a special case in acpi_irq_get_penalty(). But if we called acpi_penalize_isa_irq() or acpi_irq_penalty_update() for an SCI that happened to be an ISA IRQ, they stored the SCI penalty (part of the acpi_irq_get_penalty() return value) in acpi_isa_irq_penalty[]. Subsequent calls to acpi_irq_get_penalty() returned a penalty that included *two* SCI penalties. If this actually fixes a worse problem related to ACPI initialization, of course you should detail that. Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/acpi/pci_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index c983bf7..4f37938 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > continue; > > if (used) > - new_penalty = acpi_irq_get_penalty(irq) + > + new_penalty = acpi_isa_irq_penalty[irq] + > PIRQ_PENALTY_ISA_USED; > else > new_penalty = 0; > @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > void acpi_penalize_isa_irq(int irq, int active) > { > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); > } > > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-21 1:39 ` Bjorn Helgaas @ 2016-10-21 14:07 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 14:07 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, agross, linux-kernel, linux-arm-msm, wim, linux-arm-kernel, Len Brown On Thu, Oct 20, 2016 at 08:39:30PM -0500, Bjorn Helgaas wrote: > On Wed, Oct 19, 2016 at 06:21:02PM -0400, Sinan Kaya wrote: > > The penalty determination of ISA IRQ goes through 4 paths. > > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > > 2. update the penalty with acpi_penalize_isa_irq function based on the > > active parameter. > > 3. kernel command line penalty update via acpi_irq_penalty_update function. > > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > > before the ACPI subsystem is started. > > > > These API need to bypass the acpi_irq_get_penalty function. > > I don't mind this patch, but the changelog doesn't tell me what's > broken and why we need this fix. Apparently acpi_irq_get_penalty() > doesn't work before ACPI is initialized, but I don't see *why* it > wouldn't work. > > However, I see one bug it *does* fix: we do not store the SCI penalty > in the acpi_isa_irq_penalty[] table because acpi_isa_irq_penalty[] > only holds ISA IRQ penalties, and there's no guarantee that the SCI is > an ISA IRQ. But prior to this patch, we added in the SCI penalty to > the acpi_isa_irq_penalty[] entry when the SCI was an ISA IRQ, which > makes acpi_irq_get_penalty() return the wrong thing. Consider: > > Initially acpi_isa_irq_penalty[9] = 0. > Assume sci_interrupt = 9. > Then acpi_irq_get_penalty(9) returns X. > If we call acpi_penalize_isa_irq(9, 1), > it sets acpi_isa_irq_penalty[9] = X, > and now acpi_irq_get_penalty(9) returns X + X. Oops, I forgot the penalty we *intended* to add with acpi_penalize_isa_irq(). It's really like this, where X is the SCI penalty and Y is the part added by acpi_penalize_isa_irq(): Initially acpi_isa_irq_penalty[9] = 0. Assume sci_interrupt = 9. Then acpi_irq_get_penalty(9) returns X. If we call acpi_penalize_isa_irq(9, 1), it sets acpi_isa_irq_penalty[9] = X + Y, and now acpi_irq_get_penalty(9) returns X + X + Y. At the end, acpi_irq_get_penalty(9) *should* return X + Y, but instead it returns X + X + Y, i.e., the SCI penalty is included twice. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-21 14:07 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 14:07 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 20, 2016 at 08:39:30PM -0500, Bjorn Helgaas wrote: > On Wed, Oct 19, 2016 at 06:21:02PM -0400, Sinan Kaya wrote: > > The penalty determination of ISA IRQ goes through 4 paths. > > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > > 2. update the penalty with acpi_penalize_isa_irq function based on the > > active parameter. > > 3. kernel command line penalty update via acpi_irq_penalty_update function. > > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > > before the ACPI subsystem is started. > > > > These API need to bypass the acpi_irq_get_penalty function. > > I don't mind this patch, but the changelog doesn't tell me what's > broken and why we need this fix. Apparently acpi_irq_get_penalty() > doesn't work before ACPI is initialized, but I don't see *why* it > wouldn't work. > > However, I see one bug it *does* fix: we do not store the SCI penalty > in the acpi_isa_irq_penalty[] table because acpi_isa_irq_penalty[] > only holds ISA IRQ penalties, and there's no guarantee that the SCI is > an ISA IRQ. But prior to this patch, we added in the SCI penalty to > the acpi_isa_irq_penalty[] entry when the SCI was an ISA IRQ, which > makes acpi_irq_get_penalty() return the wrong thing. Consider: > > Initially acpi_isa_irq_penalty[9] = 0. > Assume sci_interrupt = 9. > Then acpi_irq_get_penalty(9) returns X. > If we call acpi_penalize_isa_irq(9, 1), > it sets acpi_isa_irq_penalty[9] = X, > and now acpi_irq_get_penalty(9) returns X + X. Oops, I forgot the penalty we *intended* to add with acpi_penalize_isa_irq(). It's really like this, where X is the SCI penalty and Y is the part added by acpi_penalize_isa_irq(): Initially acpi_isa_irq_penalty[9] = 0. Assume sci_interrupt = 9. Then acpi_irq_get_penalty(9) returns X. If we call acpi_penalize_isa_irq(9, 1), it sets acpi_isa_irq_penalty[9] = X + Y, and now acpi_irq_get_penalty(9) returns X + X + Y. At the end, acpi_irq_get_penalty(9) *should* return X + Y, but instead it returns X + X + Y, i.e., the SCI penalty is included twice. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-21 1:39 ` Bjorn Helgaas @ 2016-10-24 3:22 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:22 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, agross, linux-kernel, linux-arm-msm, wim, linux-arm-kernel, Len Brown On 10/20/2016 9:39 PM, Bjorn Helgaas wrote: >> These API need to bypass the acpi_irq_get_penalty function. > I don't mind this patch, but the changelog doesn't tell me what's > broken and why we need this fix. Apparently acpi_irq_get_penalty() > doesn't work before ACPI is initialized, but I don't see *why* it > wouldn't work. I'll update the commit message as you suggested. The code doesn't work if we apply PATCH V4 2/3 + PATCH V4 3/3 without PATCH V4 1/3 since the caller is going to end up calling get_penalty function while ACPI is not initialized. This happened during the debug of this regression. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-24 3:22 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:22 UTC (permalink / raw) To: linux-arm-kernel On 10/20/2016 9:39 PM, Bjorn Helgaas wrote: >> These API need to bypass the acpi_irq_get_penalty function. > I don't mind this patch, but the changelog doesn't tell me what's > broken and why we need this fix. Apparently acpi_irq_get_penalty() > doesn't work before ACPI is initialized, but I don't see *why* it > wouldn't work. I'll update the commit message as you suggested. The code doesn't work if we apply PATCH V4 2/3 + PATCH V4 3/3 without PATCH V4 1/3 since the caller is going to end up calling get_penalty function while ACPI is not initialized. This happened during the debug of this regression. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [V4, 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-19 22:21 ` Sinan Kaya @ 2016-10-23 3:48 ` Jonathan Liu -1 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-23 3:48 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, Bjorn Helgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, linux-kernel On 20 October 2016 at 09:21, Sinan Kaya <okaya@codeaurora.org> wrote: > The penalty determination of ISA IRQ goes through 4 paths. > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > 2. update the penalty with acpi_penalize_isa_irq function based on the > active parameter. > 3. kernel command line penalty update via acpi_irq_penalty_update function. > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > before the ACPI subsystem is started. > > These API need to bypass the acpi_irq_get_penalty function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/acpi/pci_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index c983bf7..4f37938 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > continue; > > if (used) > - new_penalty = acpi_irq_get_penalty(irq) + > + new_penalty = acpi_isa_irq_penalty[irq] + > PIRQ_PENALTY_ISA_USED; > else > new_penalty = 0; > @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > void acpi_penalize_isa_irq(int irq, int active) > { > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); > } > This series fixes one or more network adapters not working in Linux 32-bit x86 guest running inside VirtualBox if I have 4 network adapters enabled. The following message no longer appears in the kernel log: ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off Tested-by: Jonathan Liu <net147@gmail.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
* [V4, 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-23 3:48 ` Jonathan Liu 0 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-23 3:48 UTC (permalink / raw) To: linux-arm-kernel On 20 October 2016 at 09:21, Sinan Kaya <okaya@codeaurora.org> wrote: > The penalty determination of ISA IRQ goes through 4 paths. > 1. assign PCI_USING during power up via acpi_irq_penalty_init. > 2. update the penalty with acpi_penalize_isa_irq function based on the > active parameter. > 3. kernel command line penalty update via acpi_irq_penalty_update function. > 4. increment the penalty as USING right after the IRQ is assign to PCI. > > acpi_penalize_isa_irq and acpi_irq_penalty_update functions get called > before the ACPI subsystem is started. > > These API need to bypass the acpi_irq_get_penalty function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/acpi/pci_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index c983bf7..4f37938 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -849,7 +849,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > continue; > > if (used) > - new_penalty = acpi_irq_get_penalty(irq) + > + new_penalty = acpi_isa_irq_penalty[irq] + > PIRQ_PENALTY_ISA_USED; > else > new_penalty = 0; > @@ -871,7 +871,7 @@ static int __init acpi_irq_penalty_update(char *str, int used) > void acpi_penalize_isa_irq(int irq, int active) > { > if ((irq >= 0) && (irq < ARRAY_SIZE(acpi_isa_irq_penalty))) > - acpi_isa_irq_penalty[irq] = acpi_irq_get_penalty(irq) + > + acpi_isa_irq_penalty[irq] = acpi_isa_irq_penalty[irq] + > (active ? PIRQ_PENALTY_ISA_USED : PIRQ_PENALTY_PCI_USING); > } > This series fixes one or more network adapters not working in Linux 32-bit x86 guest running inside VirtualBox if I have 4 network adapters enabled. The following message no longer appears in the kernel log: ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off Tested-by: Jonathan Liu <net147@gmail.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [V4, 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-23 3:48 ` Jonathan Liu @ 2016-10-24 4:17 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 4:17 UTC (permalink / raw) To: Jonathan Liu Cc: linux-acpi, rjw, Bjorn Helgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, linux-kernel Thanks, On 10/22/2016 11:48 PM, Jonathan Liu wrote: > This series fixes one or more network adapters not working in Linux > 32-bit x86 guest running inside VirtualBox if I have 4 network > adapters enabled. The following message no longer appears in the > kernel log: > ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off > > Tested-by: Jonathan Liu <net147@gmail.com> I'm hoping that you can retest V5 so that Rafael can pull in your tested-by into the commit message. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [V4, 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-24 4:17 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 4:17 UTC (permalink / raw) To: linux-arm-kernel Thanks, On 10/22/2016 11:48 PM, Jonathan Liu wrote: > This series fixes one or more network adapters not working in Linux > 32-bit x86 guest running inside VirtualBox if I have 4 network > adapters enabled. The following message no longer appears in the > kernel log: > ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off > > Tested-by: Jonathan Liu <net147@gmail.com> I'm hoping that you can retest V5 so that Rafael can pull in your tested-by into the commit message. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [V4, 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages 2016-10-24 4:17 ` Sinan Kaya @ 2016-10-24 4:21 ` Jonathan Liu -1 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-24 4:21 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, Bjorn Helgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, linux-kernel On 24 October 2016 at 15:17, Sinan Kaya <okaya@codeaurora.org> wrote: > Thanks, > > On 10/22/2016 11:48 PM, Jonathan Liu wrote: >> This series fixes one or more network adapters not working in Linux >> 32-bit x86 guest running inside VirtualBox if I have 4 network >> adapters enabled. The following message no longer appears in the >> kernel log: >> ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off >> >> Tested-by: Jonathan Liu <net147@gmail.com> > > I'm hoping that you can retest V5 so that Rafael can pull in your tested-by into > the commit message. > > -- > Sinan Kaya > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. > Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. Sure. Please CC me when you submit V5. Regards, Jonathan ^ permalink raw reply [flat|nested] 56+ messages in thread
* [V4, 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages @ 2016-10-24 4:21 ` Jonathan Liu 0 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-24 4:21 UTC (permalink / raw) To: linux-arm-kernel On 24 October 2016 at 15:17, Sinan Kaya <okaya@codeaurora.org> wrote: > Thanks, > > On 10/22/2016 11:48 PM, Jonathan Liu wrote: >> This series fixes one or more network adapters not working in Linux >> 32-bit x86 guest running inside VirtualBox if I have 4 network >> adapters enabled. The following message no longer appears in the >> kernel log: >> ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off >> >> Tested-by: Jonathan Liu <net147@gmail.com> > > I'm hoping that you can retest V5 so that Rafael can pull in your tested-by into > the commit message. > > -- > Sinan Kaya > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. > Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. Sure. Please CC me when you submit V5. Regards, Jonathan ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-19 22:21 ` Sinan Kaya (?) @ 2016-10-19 22:21 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: linux-pci, agross, linux-arm-msm, linux-arm-kernel, wim, Sinan Kaya, Len Brown, Pavel Machek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm, linux-kernel The SCI penalize function was removed in two steps (first refactor and then remove) and these changes are reverted here in one go. The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") refactored the original code so that SCI penalty is calculated dynamically by the time get_penalty function is called. That change is partially reverted here, specifically for the SCI IRQ alone. The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI penalty API with penalty calculation carried out dynamically and based on the acpi_gbl_FADT.sci_interrupt value. However, that new algorithm relies on the accurate setting of IRQ types and that doesn't happen early enough on some platforms which leads to incorrect penalty assignments for PCI IRQs. In those cases, irq_get_trigger_type() returns incorrect values for the IRQs in question, because they have not been registered yet by the time the penalties are calculated. To fix this problem, we only need to fix the penalty for the SCI interrupt. It seems better to add a single "sci_penalty" variable, set it to PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS otherwise, and add "sci_penalty" in when appropriate. That should fix it for *any* SCI IRQ, not just those less than 256, and we don't have to add these extra penalty table entries that are all unused (except possibly for one entry if we have an SCI in the 16-255 range). For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") completely to restore the correct behavior. Link: https://lkml.org/lkml/2016/10/4/283 Signed-off-by: Sinan Kaya <okaya@codeaurora.org> Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") --- arch/x86/kernel/acpi/boot.c | 1 + drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- include/linux/acpi.h | 1 + 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 90d84c3..0ffd26e 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); + acpi_penalize_sci_irq(bus_irq, trigger, polarity); /* * stash over-ride to indicate we've been here diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 4f37938..294b190 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -87,6 +87,7 @@ struct acpi_pci_link { static LIST_HEAD(acpi_link_list); static DEFINE_MUTEX(acpi_link_lock); +static int sci_irq = -1, sci_penalty; /* -------------------------------------------------------------------------- PCI Link Device Management @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) { int penalty = 0; - /* - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be - * use for PCI IRQs. - */ - if (irq == acpi_gbl_FADT.sci_interrupt) { - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; - - if (type != IRQ_TYPE_LEVEL_LOW) - penalty += PIRQ_PENALTY_ISA_ALWAYS; - else - penalty += PIRQ_PENALTY_PCI_USING; - } + if (irq == sci_irq) + penalty += sci_penalty; if (irq < ACPI_MAX_ISA_IRQS) return penalty + acpi_isa_irq_penalty[irq]; - penalty += acpi_irq_pci_sharing_penalty(irq); - return penalty; + return penalty + acpi_irq_pci_sharing_penalty(irq); } int __init acpi_irq_penalty_init(void) @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); } +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) +{ + sci_irq = irq; + + if (trigger == ACPI_MADT_TRIGGER_LEVEL && + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) + sci_penalty = PIRQ_PENALTY_PCI_USING; + else + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; +} + /* * Over-ride default table to reserve additional IRQs for use by ISA * e.g. acpi_irq_isa=5 diff --git a/include/linux/acpi.h b/include/linux/acpi.h index c5eaf2f..67d1d3e 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -318,6 +318,7 @@ struct pci_dev; int acpi_pci_irq_enable (struct pci_dev *dev); void acpi_penalize_isa_irq(int irq, int active); bool acpi_isa_irq_available(int irq); +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); void acpi_pci_irq_disable (struct pci_dev *dev); extern int ec_read(u8 addr, u8 *val); -- 1.9.1 ^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-arm-kernel The SCI penalize function was removed in two steps (first refactor and then remove) and these changes are reverted here in one go. The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") refactored the original code so that SCI penalty is calculated dynamically by the time get_penalty function is called. That change is partially reverted here, specifically for the SCI IRQ alone. The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI penalty API with penalty calculation carried out dynamically and based on the acpi_gbl_FADT.sci_interrupt value. However, that new algorithm relies on the accurate setting of IRQ types and that doesn't happen early enough on some platforms which leads to incorrect penalty assignments for PCI IRQs. In those cases, irq_get_trigger_type() returns incorrect values for the IRQs in question, because they have not been registered yet by the time the penalties are calculated. To fix this problem, we only need to fix the penalty for the SCI interrupt. It seems better to add a single "sci_penalty" variable, set it to PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS otherwise, and add "sci_penalty" in when appropriate. That should fix it for *any* SCI IRQ, not just those less than 256, and we don't have to add these extra penalty table entries that are all unused (except possibly for one entry if we have an SCI in the 16-255 range). For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") completely to restore the correct behavior. Link: https://lkml.org/lkml/2016/10/4/283 Signed-off-by: Sinan Kaya <okaya@codeaurora.org> Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") --- arch/x86/kernel/acpi/boot.c | 1 + drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- include/linux/acpi.h | 1 + 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 90d84c3..0ffd26e 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); + acpi_penalize_sci_irq(bus_irq, trigger, polarity); /* * stash over-ride to indicate we've been here diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 4f37938..294b190 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -87,6 +87,7 @@ struct acpi_pci_link { static LIST_HEAD(acpi_link_list); static DEFINE_MUTEX(acpi_link_lock); +static int sci_irq = -1, sci_penalty; /* -------------------------------------------------------------------------- PCI Link Device Management @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) { int penalty = 0; - /* - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be - * use for PCI IRQs. - */ - if (irq == acpi_gbl_FADT.sci_interrupt) { - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; - - if (type != IRQ_TYPE_LEVEL_LOW) - penalty += PIRQ_PENALTY_ISA_ALWAYS; - else - penalty += PIRQ_PENALTY_PCI_USING; - } + if (irq == sci_irq) + penalty += sci_penalty; if (irq < ACPI_MAX_ISA_IRQS) return penalty + acpi_isa_irq_penalty[irq]; - penalty += acpi_irq_pci_sharing_penalty(irq); - return penalty; + return penalty + acpi_irq_pci_sharing_penalty(irq); } int __init acpi_irq_penalty_init(void) @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); } +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) +{ + sci_irq = irq; + + if (trigger == ACPI_MADT_TRIGGER_LEVEL && + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) + sci_penalty = PIRQ_PENALTY_PCI_USING; + else + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; +} + /* * Over-ride default table to reserve additional IRQs for use by ISA * e.g. acpi_irq_isa=5 diff --git a/include/linux/acpi.h b/include/linux/acpi.h index c5eaf2f..67d1d3e 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -318,6 +318,7 @@ struct pci_dev; int acpi_pci_irq_enable (struct pci_dev *dev); void acpi_penalize_isa_irq(int irq, int active); bool acpi_isa_irq_available(int irq); +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); void acpi_pci_irq_disable (struct pci_dev *dev); extern int ec_read(u8 addr, u8 *val); -- 1.9.1 ^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Sinan Kaya, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel The SCI penalize function was removed in two steps (first refactor and then remove) and these changes are reverted here in one go. The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") refactored the original code so that SCI penalty is calculated dynamically by the time get_penalty function is called. That change is partially reverted here, specifically for the SCI IRQ alone. The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI penalty API with penalty calculation carried out dynamically and based on the acpi_gbl_FADT.sci_interrupt value. However, that new algorithm relies on the accurate setting of IRQ types and that doesn't happen early enough on some platforms which leads to incorrect penalty assignments for PCI IRQs. In those cases, irq_get_trigger_type() returns incorrect values for the IRQs in question, because they have not been registered yet by the time the penalties are calculated. To fix this problem, we only need to fix the penalty for the SCI interrupt. It seems better to add a single "sci_penalty" variable, set it to PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS otherwise, and add "sci_penalty" in when appropriate. That should fix it for *any* SCI IRQ, not just those less than 256, and we don't have to add these extra penalty table entries that are all unused (except possibly for one entry if we have an SCI in the 16-255 range). For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") completely to restore the correct behavior. Link: https://lkml.org/lkml/2016/10/4/283 Signed-off-by: Sinan Kaya <okaya@codeaurora.org> Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") --- arch/x86/kernel/acpi/boot.c | 1 + drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- include/linux/acpi.h | 1 + 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 90d84c3..0ffd26e 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); + acpi_penalize_sci_irq(bus_irq, trigger, polarity); /* * stash over-ride to indicate we've been here diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 4f37938..294b190 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -87,6 +87,7 @@ struct acpi_pci_link { static LIST_HEAD(acpi_link_list); static DEFINE_MUTEX(acpi_link_lock); +static int sci_irq = -1, sci_penalty; /* -------------------------------------------------------------------------- PCI Link Device Management @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) { int penalty = 0; - /* - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be - * use for PCI IRQs. - */ - if (irq == acpi_gbl_FADT.sci_interrupt) { - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; - - if (type != IRQ_TYPE_LEVEL_LOW) - penalty += PIRQ_PENALTY_ISA_ALWAYS; - else - penalty += PIRQ_PENALTY_PCI_USING; - } + if (irq == sci_irq) + penalty += sci_penalty; if (irq < ACPI_MAX_ISA_IRQS) return penalty + acpi_isa_irq_penalty[irq]; - penalty += acpi_irq_pci_sharing_penalty(irq); - return penalty; + return penalty + acpi_irq_pci_sharing_penalty(irq); } int __init acpi_irq_penalty_init(void) @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); } +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) +{ + sci_irq = irq; + + if (trigger == ACPI_MADT_TRIGGER_LEVEL && + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) + sci_penalty = PIRQ_PENALTY_PCI_USING; + else + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; +} + /* * Over-ride default table to reserve additional IRQs for use by ISA * e.g. acpi_irq_isa=5 diff --git a/include/linux/acpi.h b/include/linux/acpi.h index c5eaf2f..67d1d3e 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -318,6 +318,7 @@ struct pci_dev; int acpi_pci_irq_enable (struct pci_dev *dev); void acpi_penalize_isa_irq(int irq, int active); bool acpi_isa_irq_available(int irq); +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); void acpi_pci_irq_disable (struct pci_dev *dev); extern int ec_read(u8 addr, u8 *val); -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 56+ messages in thread
* Re: [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-19 22:21 ` Sinan Kaya @ 2016-10-21 1:58 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 1:58 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel On Wed, Oct 19, 2016 at 06:21:03PM -0400, Sinan Kaya wrote: > The SCI penalize function was removed in two steps (first refactor > and then remove) and these changes are reverted here in one go. > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > refactored the original code so that SCI penalty is calculated dynamically > by the time get_penalty function is called. That change is partially > reverted here, specifically for the SCI IRQ alone. > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > penalty API with penalty calculation carried out dynamically and based > on the acpi_gbl_FADT.sci_interrupt value. > > However, that new algorithm relies on the accurate setting of IRQ > types and that doesn't happen early enough on some platforms which > leads to incorrect penalty assignments for PCI IRQs. In those cases, > irq_get_trigger_type() returns incorrect values for the IRQs in > question, because they have not been registered yet by the time the > penalties are calculated. > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > It seems better to add a single "sci_penalty" variable, set it to > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > otherwise, and add "sci_penalty" in when appropriate. That should fix it > for *any* SCI IRQ, not just those less than 256, and we don't have to add > these extra penalty table entries that are all unused (except possibly for > one entry if we have an SCI in the 16-255 range). > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > penalize function") completely to restore the correct behavior. I like this patch fine, except for the changelog. I don't think it's useful to describe this as a revert and give all the historical details. I think the important part is something like this: We previously used irq_get_trigger_type(irq) to help compute the penalty for the SCI, but that depends on the SCI having been registered already. Add acpi_penalize_sci_irq() so platforms can tell us the SCI IRQ, trigger, and polarity so we can compute the penalty even before the SCI has been registered. Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Link: https://lkml.org/lkml/2016/10/4/283 > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") "commit" is redundant; it's sufficient to say: Fixes: 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") In fact, I don't think you really need to include "commit" in the reference to 9e5ed6d1fb87 above either. > --- > arch/x86/kernel/acpi/boot.c | 1 + > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > include/linux/acpi.h | 1 + > 3 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index 90d84c3..0ffd26e 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > /* > * stash over-ride to indicate we've been here > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 4f37938..294b190 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > static LIST_HEAD(acpi_link_list); > static DEFINE_MUTEX(acpi_link_lock); > +static int sci_irq = -1, sci_penalty; > > /* -------------------------------------------------------------------------- > PCI Link Device Management > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > { > int penalty = 0; > > - /* > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > - * use for PCI IRQs. > - */ > - if (irq == acpi_gbl_FADT.sci_interrupt) { > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > - > - if (type != IRQ_TYPE_LEVEL_LOW) > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > - else > - penalty += PIRQ_PENALTY_PCI_USING; > - } > + if (irq == sci_irq) > + penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > return penalty + acpi_isa_irq_penalty[irq]; > > - penalty += acpi_irq_pci_sharing_penalty(irq); > - return penalty; > + return penalty + acpi_irq_pci_sharing_penalty(irq); > } > > int __init acpi_irq_penalty_init(void) > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > } > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > +{ > + sci_irq = irq; > + > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > + sci_penalty = PIRQ_PENALTY_PCI_USING; > + else > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > +} > + > /* > * Over-ride default table to reserve additional IRQs for use by ISA > * e.g. acpi_irq_isa=5 > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index c5eaf2f..67d1d3e 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -318,6 +318,7 @@ struct pci_dev; > int acpi_pci_irq_enable (struct pci_dev *dev); > void acpi_penalize_isa_irq(int irq, int active); > bool acpi_isa_irq_available(int irq); > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > void acpi_pci_irq_disable (struct pci_dev *dev); > > extern int ec_read(u8 addr, u8 *val); > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-21 1:58 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 1:58 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 19, 2016 at 06:21:03PM -0400, Sinan Kaya wrote: > The SCI penalize function was removed in two steps (first refactor > and then remove) and these changes are reverted here in one go. > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > refactored the original code so that SCI penalty is calculated dynamically > by the time get_penalty function is called. That change is partially > reverted here, specifically for the SCI IRQ alone. > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > penalty API with penalty calculation carried out dynamically and based > on the acpi_gbl_FADT.sci_interrupt value. > > However, that new algorithm relies on the accurate setting of IRQ > types and that doesn't happen early enough on some platforms which > leads to incorrect penalty assignments for PCI IRQs. In those cases, > irq_get_trigger_type() returns incorrect values for the IRQs in > question, because they have not been registered yet by the time the > penalties are calculated. > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > It seems better to add a single "sci_penalty" variable, set it to > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > otherwise, and add "sci_penalty" in when appropriate. That should fix it > for *any* SCI IRQ, not just those less than 256, and we don't have to add > these extra penalty table entries that are all unused (except possibly for > one entry if we have an SCI in the 16-255 range). > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > penalize function") completely to restore the correct behavior. I like this patch fine, except for the changelog. I don't think it's useful to describe this as a revert and give all the historical details. I think the important part is something like this: We previously used irq_get_trigger_type(irq) to help compute the penalty for the SCI, but that depends on the SCI having been registered already. Add acpi_penalize_sci_irq() so platforms can tell us the SCI IRQ, trigger, and polarity so we can compute the penalty even before the SCI has been registered. Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Link: https://lkml.org/lkml/2016/10/4/283 > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") "commit" is redundant; it's sufficient to say: Fixes: 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") In fact, I don't think you really need to include "commit" in the reference to 9e5ed6d1fb87 above either. > --- > arch/x86/kernel/acpi/boot.c | 1 + > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > include/linux/acpi.h | 1 + > 3 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index 90d84c3..0ffd26e 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > /* > * stash over-ride to indicate we've been here > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 4f37938..294b190 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > static LIST_HEAD(acpi_link_list); > static DEFINE_MUTEX(acpi_link_lock); > +static int sci_irq = -1, sci_penalty; > > /* -------------------------------------------------------------------------- > PCI Link Device Management > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > { > int penalty = 0; > > - /* > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > - * use for PCI IRQs. > - */ > - if (irq == acpi_gbl_FADT.sci_interrupt) { > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > - > - if (type != IRQ_TYPE_LEVEL_LOW) > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > - else > - penalty += PIRQ_PENALTY_PCI_USING; > - } > + if (irq == sci_irq) > + penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > return penalty + acpi_isa_irq_penalty[irq]; > > - penalty += acpi_irq_pci_sharing_penalty(irq); > - return penalty; > + return penalty + acpi_irq_pci_sharing_penalty(irq); > } > > int __init acpi_irq_penalty_init(void) > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > } > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > +{ > + sci_irq = irq; > + > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > + sci_penalty = PIRQ_PENALTY_PCI_USING; > + else > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > +} > + > /* > * Over-ride default table to reserve additional IRQs for use by ISA > * e.g. acpi_irq_isa=5 > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index c5eaf2f..67d1d3e 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -318,6 +318,7 @@ struct pci_dev; > int acpi_pci_irq_enable (struct pci_dev *dev); > void acpi_penalize_isa_irq(int irq, int active); > bool acpi_isa_irq_available(int irq); > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > void acpi_pci_irq_disable (struct pci_dev *dev); > > extern int ec_read(u8 addr, u8 *val); > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-21 1:58 ` Bjorn Helgaas @ 2016-10-21 14:45 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 14:45 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel On Thu, Oct 20, 2016 at 08:58:14PM -0500, Bjorn Helgaas wrote: > On Wed, Oct 19, 2016 at 06:21:03PM -0400, Sinan Kaya wrote: > > The SCI penalize function was removed in two steps (first refactor > > and then remove) and these changes are reverted here in one go. > > > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > refactored the original code so that SCI penalty is calculated dynamically > > by the time get_penalty function is called. That change is partially > > reverted here, specifically for the SCI IRQ alone. > > > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > > penalty API with penalty calculation carried out dynamically and based > > on the acpi_gbl_FADT.sci_interrupt value. > > > > However, that new algorithm relies on the accurate setting of IRQ > > types and that doesn't happen early enough on some platforms which > > leads to incorrect penalty assignments for PCI IRQs. In those cases, > > irq_get_trigger_type() returns incorrect values for the IRQs in > > question, because they have not been registered yet by the time the > > penalties are calculated. > > > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > > It seems better to add a single "sci_penalty" variable, set it to > > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > > otherwise, and add "sci_penalty" in when appropriate. That should fix it > > for *any* SCI IRQ, not just those less than 256, and we don't have to add > > these extra penalty table entries that are all unused (except possibly for > > one entry if we have an SCI in the 16-255 range). > > > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > > penalize function") completely to restore the correct behavior. > > I like this patch fine, except for the changelog. I don't think it's > useful to describe this as a revert and give all the historical > details. I think the important part is something like this: > > We previously used irq_get_trigger_type(irq) to help compute the > penalty for the SCI, but that depends on the SCI having been > registered already. Add acpi_penalize_sci_irq() so platforms can > tell us the SCI IRQ, trigger, and polarity so we can compute the > penalty even before the SCI has been registered. Thanks for the pointer [1] to the issues around acpi_penalize_sci_irq() being called before kmalloc() is available. That begs the question of exactly *why* we need acpi_penalize_sci_irq() so early -- I doubt we actually need to handle SCIs that early, and obviously we don't need to look up penalties that early (because we only look them up when enabling interrupt links, which happens much later). [1] http://marc.info/?l=linux-acpi&m=145580159209240&w=2) > Acked-by: Bjorn Helgaas <bhelgaas@google.com> Wait a minute, I still have a question here: what about other ACPI arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() somewhere? > > --- > > arch/x86/kernel/acpi/boot.c | 1 + > > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > > include/linux/acpi.h | 1 + > > 3 files changed, 17 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > > index 90d84c3..0ffd26e 100644 > > --- a/arch/x86/kernel/acpi/boot.c > > +++ b/arch/x86/kernel/acpi/boot.c > > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > > > /* > > * stash over-ride to indicate we've been here > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > > index 4f37938..294b190 100644 > > --- a/drivers/acpi/pci_link.c > > +++ b/drivers/acpi/pci_link.c > > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > > > static LIST_HEAD(acpi_link_list); > > static DEFINE_MUTEX(acpi_link_lock); > > +static int sci_irq = -1, sci_penalty; > > > > /* -------------------------------------------------------------------------- > > PCI Link Device Management > > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > > { > > int penalty = 0; > > > > - /* > > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > > - * use for PCI IRQs. > > - */ > > - if (irq == acpi_gbl_FADT.sci_interrupt) { > > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > > - > > - if (type != IRQ_TYPE_LEVEL_LOW) > > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > > - else > > - penalty += PIRQ_PENALTY_PCI_USING; > > - } > > + if (irq == sci_irq) > > + penalty += sci_penalty; > > > > if (irq < ACPI_MAX_ISA_IRQS) > > return penalty + acpi_isa_irq_penalty[irq]; > > > > - penalty += acpi_irq_pci_sharing_penalty(irq); > > - return penalty; > > + return penalty + acpi_irq_pci_sharing_penalty(irq); > > } > > > > int __init acpi_irq_penalty_init(void) > > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > > } > > > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > > +{ > > + sci_irq = irq; > > + > > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > > + sci_penalty = PIRQ_PENALTY_PCI_USING; > > + else > > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > > +} > > + > > /* > > * Over-ride default table to reserve additional IRQs for use by ISA > > * e.g. acpi_irq_isa=5 > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index c5eaf2f..67d1d3e 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -318,6 +318,7 @@ struct pci_dev; > > int acpi_pci_irq_enable (struct pci_dev *dev); > > void acpi_penalize_isa_irq(int irq, int active); > > bool acpi_isa_irq_available(int irq); > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > > void acpi_pci_irq_disable (struct pci_dev *dev); > > > > extern int ec_read(u8 addr, u8 *val); > > -- > > 1.9.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-21 14:45 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 14:45 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 20, 2016 at 08:58:14PM -0500, Bjorn Helgaas wrote: > On Wed, Oct 19, 2016 at 06:21:03PM -0400, Sinan Kaya wrote: > > The SCI penalize function was removed in two steps (first refactor > > and then remove) and these changes are reverted here in one go. > > > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > refactored the original code so that SCI penalty is calculated dynamically > > by the time get_penalty function is called. That change is partially > > reverted here, specifically for the SCI IRQ alone. > > > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > > penalty API with penalty calculation carried out dynamically and based > > on the acpi_gbl_FADT.sci_interrupt value. > > > > However, that new algorithm relies on the accurate setting of IRQ > > types and that doesn't happen early enough on some platforms which > > leads to incorrect penalty assignments for PCI IRQs. In those cases, > > irq_get_trigger_type() returns incorrect values for the IRQs in > > question, because they have not been registered yet by the time the > > penalties are calculated. > > > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > > It seems better to add a single "sci_penalty" variable, set it to > > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > > otherwise, and add "sci_penalty" in when appropriate. That should fix it > > for *any* SCI IRQ, not just those less than 256, and we don't have to add > > these extra penalty table entries that are all unused (except possibly for > > one entry if we have an SCI in the 16-255 range). > > > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > > penalize function") completely to restore the correct behavior. > > I like this patch fine, except for the changelog. I don't think it's > useful to describe this as a revert and give all the historical > details. I think the important part is something like this: > > We previously used irq_get_trigger_type(irq) to help compute the > penalty for the SCI, but that depends on the SCI having been > registered already. Add acpi_penalize_sci_irq() so platforms can > tell us the SCI IRQ, trigger, and polarity so we can compute the > penalty even before the SCI has been registered. Thanks for the pointer [1] to the issues around acpi_penalize_sci_irq() being called before kmalloc() is available. That begs the question of exactly *why* we need acpi_penalize_sci_irq() so early -- I doubt we actually need to handle SCIs that early, and obviously we don't need to look up penalties that early (because we only look them up when enabling interrupt links, which happens much later). [1] http://marc.info/?l=linux-acpi&m=145580159209240&w=2) > Acked-by: Bjorn Helgaas <bhelgaas@google.com> Wait a minute, I still have a question here: what about other ACPI arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() somewhere? > > --- > > arch/x86/kernel/acpi/boot.c | 1 + > > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > > include/linux/acpi.h | 1 + > > 3 files changed, 17 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > > index 90d84c3..0ffd26e 100644 > > --- a/arch/x86/kernel/acpi/boot.c > > +++ b/arch/x86/kernel/acpi/boot.c > > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > > > /* > > * stash over-ride to indicate we've been here > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > > index 4f37938..294b190 100644 > > --- a/drivers/acpi/pci_link.c > > +++ b/drivers/acpi/pci_link.c > > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > > > static LIST_HEAD(acpi_link_list); > > static DEFINE_MUTEX(acpi_link_lock); > > +static int sci_irq = -1, sci_penalty; > > > > /* -------------------------------------------------------------------------- > > PCI Link Device Management > > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > > { > > int penalty = 0; > > > > - /* > > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > > - * use for PCI IRQs. > > - */ > > - if (irq == acpi_gbl_FADT.sci_interrupt) { > > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > > - > > - if (type != IRQ_TYPE_LEVEL_LOW) > > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > > - else > > - penalty += PIRQ_PENALTY_PCI_USING; > > - } > > + if (irq == sci_irq) > > + penalty += sci_penalty; > > > > if (irq < ACPI_MAX_ISA_IRQS) > > return penalty + acpi_isa_irq_penalty[irq]; > > > > - penalty += acpi_irq_pci_sharing_penalty(irq); > > - return penalty; > > + return penalty + acpi_irq_pci_sharing_penalty(irq); > > } > > > > int __init acpi_irq_penalty_init(void) > > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > > } > > > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > > +{ > > + sci_irq = irq; > > + > > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > > + sci_penalty = PIRQ_PENALTY_PCI_USING; > > + else > > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > > +} > > + > > /* > > * Over-ride default table to reserve additional IRQs for use by ISA > > * e.g. acpi_irq_isa=5 > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index c5eaf2f..67d1d3e 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -318,6 +318,7 @@ struct pci_dev; > > int acpi_pci_irq_enable (struct pci_dev *dev); > > void acpi_penalize_isa_irq(int irq, int active); > > bool acpi_isa_irq_available(int irq); > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > > void acpi_pci_irq_disable (struct pci_dev *dev); > > > > extern int ec_read(u8 addr, u8 *val); > > -- > > 1.9.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-21 14:45 ` Bjorn Helgaas @ 2016-10-21 16:13 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-21 16:13 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel On 10/21/2016 7:45 AM, Bjorn Helgaas wrote: > [1] http://marc.info/?l=linux-acpi&m=145580159209240&w=2) > >> > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Wait a minute, I still have a question here: what about other ACPI > arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() > somewhere? > ACPI ARM64 architecture implements reduced ACPI profile which doesn't have GED object. Instead, ARM64 architecture uses onchip peripherals for similar functionality. If there is a need to signal interrupts, this is done by ACPI Notify in ASL or if absolutely needed using ACPI Generic Event Device (GED). -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-21 16:13 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-21 16:13 UTC (permalink / raw) To: linux-arm-kernel On 10/21/2016 7:45 AM, Bjorn Helgaas wrote: > [1] http://marc.info/?l=linux-acpi&m=145580159209240&w=2) > >> > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Wait a minute, I still have a question here: what about other ACPI > arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() > somewhere? > ACPI ARM64 architecture implements reduced ACPI profile which doesn't have GED object. Instead, ARM64 architecture uses onchip peripherals for similar functionality. If there is a need to signal interrupts, this is done by ACPI Notify in ASL or if absolutely needed using ACPI Generic Event Device (GED). -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-21 16:13 ` Sinan Kaya @ 2016-10-22 14:57 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-22 14:57 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel On Fri, Oct 21, 2016 at 09:13:06AM -0700, Sinan Kaya wrote: > On 10/21/2016 7:45 AM, Bjorn Helgaas wrote: > > [1] http://marc.info/?l=linux-acpi&m=145580159209240&w=2) > > > >> > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > Wait a minute, I still have a question here: what about other ACPI > > arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() > > somewhere? > > > > ACPI ARM64 architecture implements reduced ACPI profile which doesn't > have GED object. Instead, ARM64 architecture uses onchip peripherals > for similar functionality. If there is a need to signal interrupts, > this is done by ACPI Notify in ASL or if absolutely needed using > ACPI Generic Event Device (GED). OK. I guess ia64 never did call acpi_penalize_sci_irq(), so while it could be added someday to unify things, we don't need to add it now. Same for arm64. So I'd like it if you updated the changelog, but I'm OK with the patch: Acked-by: Bjorn Helgaas <bhelgaas@google.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-22 14:57 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-22 14:57 UTC (permalink / raw) To: linux-arm-kernel On Fri, Oct 21, 2016 at 09:13:06AM -0700, Sinan Kaya wrote: > On 10/21/2016 7:45 AM, Bjorn Helgaas wrote: > > [1] http://marc.info/?l=linux-acpi&m=145580159209240&w=2) > > > >> > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > Wait a minute, I still have a question here: what about other ACPI > > arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() > > somewhere? > > > > ACPI ARM64 architecture implements reduced ACPI profile which doesn't > have GED object. Instead, ARM64 architecture uses onchip peripherals > for similar functionality. If there is a need to signal interrupts, > this is done by ACPI Notify in ASL or if absolutely needed using > ACPI Generic Event Device (GED). OK. I guess ia64 never did call acpi_penalize_sci_irq(), so while it could be added someday to unify things, we don't need to add it now. Same for arm64. So I'd like it if you updated the changelog, but I'm OK with the patch: Acked-by: Bjorn Helgaas <bhelgaas@google.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-21 16:13 ` Sinan Kaya @ 2016-10-24 3:48 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:48 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel On 10/21/2016 12:13 PM, Sinan Kaya wrote: >> Wait a minute, I still have a question here: what about other ACPI >> > arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() >> > somewhere? >> > > ACPI ARM64 architecture implements reduced ACPI profile which doesn't > have GED object. I actually wanted to mean that ARM64 architecture doesn't have *GPE* object implemented on ACPI reduced profile. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-24 3:48 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:48 UTC (permalink / raw) To: linux-arm-kernel On 10/21/2016 12:13 PM, Sinan Kaya wrote: >> Wait a minute, I still have a question here: what about other ACPI >> > arches (ia64, arm64)? Don't they need to call acpi_penalize_sci_irq() >> > somewhere? >> > > ACPI ARM64 architecture implements reduced ACPI profile which doesn't > have GED object. I actually wanted to mean that ARM64 architecture doesn't have *GPE* object implemented on ACPI reduced profile. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-21 1:58 ` Bjorn Helgaas @ 2016-10-22 23:28 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-22 23:28 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel On Thu, Oct 20, 2016 at 08:58:14PM -0500, Bjorn Helgaas wrote: > On Wed, Oct 19, 2016 at 06:21:03PM -0400, Sinan Kaya wrote: > > The SCI penalize function was removed in two steps (first refactor > > and then remove) and these changes are reverted here in one go. > > > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > refactored the original code so that SCI penalty is calculated dynamically > > by the time get_penalty function is called. That change is partially > > reverted here, specifically for the SCI IRQ alone. > > > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > > penalty API with penalty calculation carried out dynamically and based > > on the acpi_gbl_FADT.sci_interrupt value. > > > > However, that new algorithm relies on the accurate setting of IRQ > > types and that doesn't happen early enough on some platforms which > > leads to incorrect penalty assignments for PCI IRQs. In those cases, > > irq_get_trigger_type() returns incorrect values for the IRQs in > > question, because they have not been registered yet by the time the > > penalties are calculated. > > > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > > It seems better to add a single "sci_penalty" variable, set it to > > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > > otherwise, and add "sci_penalty" in when appropriate. That should fix it > > for *any* SCI IRQ, not just those less than 256, and we don't have to add > > these extra penalty table entries that are all unused (except possibly for > > one entry if we have an SCI in the 16-255 range). > > > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > > penalize function") completely to restore the correct behavior. > > I like this patch fine, except for the changelog. I don't think it's > useful to describe this as a revert and give all the historical > details. I think the important part is something like this: > > We previously used irq_get_trigger_type(irq) to help compute the > penalty for the SCI, but that depends on the SCI having been > registered already. Add acpi_penalize_sci_irq() so platforms can > tell us the SCI IRQ, trigger, and polarity so we can compute the > penalty even before the SCI has been registered. I think this is actually the critical patch that fixes the regression reported by Ondrej, so the changelog really should include details about the regression, e.g., something like this: ACPI: pci_link: Penalize SCI correctly Ondrej reported that IRQs stopped working in v4.7 on several platforms. A typical scenario, from Ondrej's VT82C694X/694X, is: ACPI: Using PIC for interrupt routing ACPI: PCI Interrupt Link [LNKA] (IRQs 1 3 4 5 6 7 10 *11 12 14 15) ACPI: No IRQ available for PCI Interrupt Link [LNKA] 8139too 0000:00:0f.0: PCI INT A: no GSI We're using PIC routing, so acpi_irq_balance == 0, and LNKA is already active at IRQ 11. In that case, acpi_pci_link_allocate() only tries to use the active IRQ (IRQ 11) which also happens to be the SCI. We should penalize the SCI by PIRQ_PENALTY_PCI_USING, but irq_get_trigger_type(11) returns something other than IRQ_TYPE_LEVEL_LOW, so we penalize it by PIRQ_PENALTY_ISA_ALWAYS instead, which makes acpi_pci_link_allocate() assume the IRQ isn't available and give up. Add acpi_penalize_sci_irq() so platforms can tell us the SCI IRQ, trigger, and polarity directly and we don't have to depend on irq_get_trigger_type(). Link: http://lkml.kernel.org/r/201609251512.05657.linux@rainbow-software.org Reported-by: Ondrej Zary <linux@rainbow-software.org> I don't understand what's broken about using irq_get_trigger_type(). The SCI is registered in the following path, which is certainly before we try to enable LNKA for the 8139too device: acpi_init # subsys_initcall acpi_bus_init acpi_enable_subsystem acpi_ev_install_xrupt_handlers acpi_ev_install_sci_handler acpi_os_install_interrupt_handler(sci_interrupt, ...) acpi_gsi_to_irq acpi_register_gsi irq_create_fwspec_mapping irqd_set_trigger_type In any event, I think this patch to add acpi_penalize_sci_irq() is fine. > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > Link: https://lkml.org/lkml/2016/10/4/283 > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") > > "commit" is redundant; it's sufficient to say: > > Fixes: 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > In fact, I don't think you really need to include "commit" in the > reference to 9e5ed6d1fb87 above either. > > > --- > > arch/x86/kernel/acpi/boot.c | 1 + > > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > > include/linux/acpi.h | 1 + > > 3 files changed, 17 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > > index 90d84c3..0ffd26e 100644 > > --- a/arch/x86/kernel/acpi/boot.c > > +++ b/arch/x86/kernel/acpi/boot.c > > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > > > /* > > * stash over-ride to indicate we've been here > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > > index 4f37938..294b190 100644 > > --- a/drivers/acpi/pci_link.c > > +++ b/drivers/acpi/pci_link.c > > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > > > static LIST_HEAD(acpi_link_list); > > static DEFINE_MUTEX(acpi_link_lock); > > +static int sci_irq = -1, sci_penalty; > > > > /* -------------------------------------------------------------------------- > > PCI Link Device Management > > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > > { > > int penalty = 0; > > > > - /* > > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > > - * use for PCI IRQs. > > - */ > > - if (irq == acpi_gbl_FADT.sci_interrupt) { > > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > > - > > - if (type != IRQ_TYPE_LEVEL_LOW) > > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > > - else > > - penalty += PIRQ_PENALTY_PCI_USING; > > - } > > + if (irq == sci_irq) > > + penalty += sci_penalty; > > > > if (irq < ACPI_MAX_ISA_IRQS) > > return penalty + acpi_isa_irq_penalty[irq]; > > > > - penalty += acpi_irq_pci_sharing_penalty(irq); > > - return penalty; > > + return penalty + acpi_irq_pci_sharing_penalty(irq); > > } > > > > int __init acpi_irq_penalty_init(void) > > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > > } > > > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > > +{ > > + sci_irq = irq; > > + > > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > > + sci_penalty = PIRQ_PENALTY_PCI_USING; > > + else > > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > > +} > > + > > /* > > * Over-ride default table to reserve additional IRQs for use by ISA > > * e.g. acpi_irq_isa=5 > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index c5eaf2f..67d1d3e 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -318,6 +318,7 @@ struct pci_dev; > > int acpi_pci_irq_enable (struct pci_dev *dev); > > void acpi_penalize_isa_irq(int irq, int active); > > bool acpi_isa_irq_available(int irq); > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > > void acpi_pci_irq_disable (struct pci_dev *dev); > > > > extern int ec_read(u8 addr, u8 *val); > > -- > > 1.9.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-22 23:28 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-22 23:28 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 20, 2016 at 08:58:14PM -0500, Bjorn Helgaas wrote: > On Wed, Oct 19, 2016 at 06:21:03PM -0400, Sinan Kaya wrote: > > The SCI penalize function was removed in two steps (first refactor > > and then remove) and these changes are reverted here in one go. > > > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > refactored the original code so that SCI penalty is calculated dynamically > > by the time get_penalty function is called. That change is partially > > reverted here, specifically for the SCI IRQ alone. > > > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > > penalty API with penalty calculation carried out dynamically and based > > on the acpi_gbl_FADT.sci_interrupt value. > > > > However, that new algorithm relies on the accurate setting of IRQ > > types and that doesn't happen early enough on some platforms which > > leads to incorrect penalty assignments for PCI IRQs. In those cases, > > irq_get_trigger_type() returns incorrect values for the IRQs in > > question, because they have not been registered yet by the time the > > penalties are calculated. > > > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > > It seems better to add a single "sci_penalty" variable, set it to > > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > > otherwise, and add "sci_penalty" in when appropriate. That should fix it > > for *any* SCI IRQ, not just those less than 256, and we don't have to add > > these extra penalty table entries that are all unused (except possibly for > > one entry if we have an SCI in the 16-255 range). > > > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > > penalize function") completely to restore the correct behavior. > > I like this patch fine, except for the changelog. I don't think it's > useful to describe this as a revert and give all the historical > details. I think the important part is something like this: > > We previously used irq_get_trigger_type(irq) to help compute the > penalty for the SCI, but that depends on the SCI having been > registered already. Add acpi_penalize_sci_irq() so platforms can > tell us the SCI IRQ, trigger, and polarity so we can compute the > penalty even before the SCI has been registered. I think this is actually the critical patch that fixes the regression reported by Ondrej, so the changelog really should include details about the regression, e.g., something like this: ACPI: pci_link: Penalize SCI correctly Ondrej reported that IRQs stopped working in v4.7 on several platforms. A typical scenario, from Ondrej's VT82C694X/694X, is: ACPI: Using PIC for interrupt routing ACPI: PCI Interrupt Link [LNKA] (IRQs 1 3 4 5 6 7 10 *11 12 14 15) ACPI: No IRQ available for PCI Interrupt Link [LNKA] 8139too 0000:00:0f.0: PCI INT A: no GSI We're using PIC routing, so acpi_irq_balance == 0, and LNKA is already active at IRQ 11. In that case, acpi_pci_link_allocate() only tries to use the active IRQ (IRQ 11) which also happens to be the SCI. We should penalize the SCI by PIRQ_PENALTY_PCI_USING, but irq_get_trigger_type(11) returns something other than IRQ_TYPE_LEVEL_LOW, so we penalize it by PIRQ_PENALTY_ISA_ALWAYS instead, which makes acpi_pci_link_allocate() assume the IRQ isn't available and give up. Add acpi_penalize_sci_irq() so platforms can tell us the SCI IRQ, trigger, and polarity directly and we don't have to depend on irq_get_trigger_type(). Link: http://lkml.kernel.org/r/201609251512.05657.linux at rainbow-software.org Reported-by: Ondrej Zary <linux@rainbow-software.org> I don't understand what's broken about using irq_get_trigger_type(). The SCI is registered in the following path, which is certainly before we try to enable LNKA for the 8139too device: acpi_init # subsys_initcall acpi_bus_init acpi_enable_subsystem acpi_ev_install_xrupt_handlers acpi_ev_install_sci_handler acpi_os_install_interrupt_handler(sci_interrupt, ...) acpi_gsi_to_irq acpi_register_gsi irq_create_fwspec_mapping irqd_set_trigger_type In any event, I think this patch to add acpi_penalize_sci_irq() is fine. > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > > Link: https://lkml.org/lkml/2016/10/4/283 > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") > > "commit" is redundant; it's sufficient to say: > > Fixes: 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > In fact, I don't think you really need to include "commit" in the > reference to 9e5ed6d1fb87 above either. > > > --- > > arch/x86/kernel/acpi/boot.c | 1 + > > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > > include/linux/acpi.h | 1 + > > 3 files changed, 17 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > > index 90d84c3..0ffd26e 100644 > > --- a/arch/x86/kernel/acpi/boot.c > > +++ b/arch/x86/kernel/acpi/boot.c > > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > > > /* > > * stash over-ride to indicate we've been here > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > > index 4f37938..294b190 100644 > > --- a/drivers/acpi/pci_link.c > > +++ b/drivers/acpi/pci_link.c > > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > > > static LIST_HEAD(acpi_link_list); > > static DEFINE_MUTEX(acpi_link_lock); > > +static int sci_irq = -1, sci_penalty; > > > > /* -------------------------------------------------------------------------- > > PCI Link Device Management > > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > > { > > int penalty = 0; > > > > - /* > > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > > - * use for PCI IRQs. > > - */ > > - if (irq == acpi_gbl_FADT.sci_interrupt) { > > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > > - > > - if (type != IRQ_TYPE_LEVEL_LOW) > > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > > - else > > - penalty += PIRQ_PENALTY_PCI_USING; > > - } > > + if (irq == sci_irq) > > + penalty += sci_penalty; > > > > if (irq < ACPI_MAX_ISA_IRQS) > > return penalty + acpi_isa_irq_penalty[irq]; > > > > - penalty += acpi_irq_pci_sharing_penalty(irq); > > - return penalty; > > + return penalty + acpi_irq_pci_sharing_penalty(irq); > > } > > > > int __init acpi_irq_penalty_init(void) > > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > > } > > > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > > +{ > > + sci_irq = irq; > > + > > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > > + sci_penalty = PIRQ_PENALTY_PCI_USING; > > + else > > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > > +} > > + > > /* > > * Over-ride default table to reserve additional IRQs for use by ISA > > * e.g. acpi_irq_isa=5 > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index c5eaf2f..67d1d3e 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -318,6 +318,7 @@ struct pci_dev; > > int acpi_pci_irq_enable (struct pci_dev *dev); > > void acpi_penalize_isa_irq(int irq, int active); > > bool acpi_isa_irq_available(int irq); > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > > void acpi_pci_irq_disable (struct pci_dev *dev); > > > > extern int ec_read(u8 addr, u8 *val); > > -- > > 1.9.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-21 1:58 ` Bjorn Helgaas @ 2016-10-24 3:48 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:48 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, Len Brown, x86, H. Peter Anvin, linux-pm, linux-pci, agross, linux-kernel, Ingo Molnar, Pavel Machek, linux-arm-msm, wim, Thomas Gleixner, linux-arm-kernel On 10/20/2016 9:58 PM, Bjorn Helgaas wrote: > I like this patch fine, except for the changelog. I don't think it's > useful to describe this as a revert and give all the historical > details. I think the important part is something like this: > > We previously used irq_get_trigger_type(irq) to help compute the > penalty for the SCI, but that depends on the SCI having been > registered already. Add acpi_penalize_sci_irq() so platforms can > tell us the SCI IRQ, trigger, and polarity so we can compute the > penalty even before the SCI has been registered. OK, will replace with this and also change the commit summary as "ACPI,PCI,IRQ: save SCI IRQ details for runtime penalty calculation" > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> > Link: https://lkml.org/lkml/2016/10/4/283 >> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >> > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") >> > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") > "commit" is redundant; it's sufficient to say: OK. I have been fighting with checkpatch. Checkpatch doesn't like a commit summary without "commit 12 char SHA" > > Fixes: 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > In fact, I don't think you really need to include "commit" in the > reference to 9e5ed6d1fb87 above either. > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-24 3:48 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 3:48 UTC (permalink / raw) To: linux-arm-kernel On 10/20/2016 9:58 PM, Bjorn Helgaas wrote: > I like this patch fine, except for the changelog. I don't think it's > useful to describe this as a revert and give all the historical > details. I think the important part is something like this: > > We previously used irq_get_trigger_type(irq) to help compute the > penalty for the SCI, but that depends on the SCI having been > registered already. Add acpi_penalize_sci_irq() so platforms can > tell us the SCI IRQ, trigger, and polarity so we can compute the > penalty even before the SCI has been registered. OK, will replace with this and also change the commit summary as "ACPI,PCI,IRQ: save SCI IRQ details for runtime penalty calculation" > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> > Link: https://lkml.org/lkml/2016/10/4/283 >> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >> > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") >> > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") > "commit" is redundant; it's sufficient to say: OK. I have been fighting with checkpatch. Checkpatch doesn't like a commit summary without "commit 12 char SHA" > > Fixes: 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > > In fact, I don't think you really need to include "commit" in the > reference to 9e5ed6d1fb87 above either. > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [V4,2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" 2016-10-19 22:21 ` Sinan Kaya @ 2016-10-23 3:49 ` Jonathan Liu -1 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-23 3:49 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, Bjorn Helgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Len Brown, Pavel Machek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm, linux-kernel On 20 October 2016 at 09:21, Sinan Kaya <okaya@codeaurora.org> wrote: > The SCI penalize function was removed in two steps (first refactor > and then remove) and these changes are reverted here in one go. > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > refactored the original code so that SCI penalty is calculated dynamically > by the time get_penalty function is called. That change is partially > reverted here, specifically for the SCI IRQ alone. > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > penalty API with penalty calculation carried out dynamically and based > on the acpi_gbl_FADT.sci_interrupt value. > > However, that new algorithm relies on the accurate setting of IRQ > types and that doesn't happen early enough on some platforms which > leads to incorrect penalty assignments for PCI IRQs. In those cases, > irq_get_trigger_type() returns incorrect values for the IRQs in > question, because they have not been registered yet by the time the > penalties are calculated. > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > It seems better to add a single "sci_penalty" variable, set it to > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > otherwise, and add "sci_penalty" in when appropriate. That should fix it > for *any* SCI IRQ, not just those less than 256, and we don't have to add > these extra penalty table entries that are all unused (except possibly for > one entry if we have an SCI in the 16-255 range). > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > penalize function") completely to restore the correct behavior. > > Link: https://lkml.org/lkml/2016/10/4/283 > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") > --- > arch/x86/kernel/acpi/boot.c | 1 + > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > include/linux/acpi.h | 1 + > 3 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index 90d84c3..0ffd26e 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > /* > * stash over-ride to indicate we've been here > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 4f37938..294b190 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > static LIST_HEAD(acpi_link_list); > static DEFINE_MUTEX(acpi_link_lock); > +static int sci_irq = -1, sci_penalty; > > /* -------------------------------------------------------------------------- > PCI Link Device Management > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > { > int penalty = 0; > > - /* > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > - * use for PCI IRQs. > - */ > - if (irq == acpi_gbl_FADT.sci_interrupt) { > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > - > - if (type != IRQ_TYPE_LEVEL_LOW) > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > - else > - penalty += PIRQ_PENALTY_PCI_USING; > - } > + if (irq == sci_irq) > + penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > return penalty + acpi_isa_irq_penalty[irq]; > > - penalty += acpi_irq_pci_sharing_penalty(irq); > - return penalty; > + return penalty + acpi_irq_pci_sharing_penalty(irq); > } > > int __init acpi_irq_penalty_init(void) > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > } > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > +{ > + sci_irq = irq; > + > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > + sci_penalty = PIRQ_PENALTY_PCI_USING; > + else > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > +} > + > /* > * Over-ride default table to reserve additional IRQs for use by ISA > * e.g. acpi_irq_isa=5 > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index c5eaf2f..67d1d3e 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -318,6 +318,7 @@ struct pci_dev; > int acpi_pci_irq_enable (struct pci_dev *dev); > void acpi_penalize_isa_irq(int irq, int active); > bool acpi_isa_irq_available(int irq); > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > void acpi_pci_irq_disable (struct pci_dev *dev); > > extern int ec_read(u8 addr, u8 *val); This series fixes one or more network adapters not working in Linux 32-bit x86 guest running inside VirtualBox if I have 4 network adapters enabled. The following message no longer appears in the kernel log: ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off Tested-by: Jonathan Liu <net147@gmail.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
* [V4,2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" @ 2016-10-23 3:49 ` Jonathan Liu 0 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-23 3:49 UTC (permalink / raw) To: linux-arm-kernel On 20 October 2016 at 09:21, Sinan Kaya <okaya@codeaurora.org> wrote: > The SCI penalize function was removed in two steps (first refactor > and then remove) and these changes are reverted here in one go. > > The commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > refactored the original code so that SCI penalty is calculated dynamically > by the time get_penalty function is called. That change is partially > reverted here, specifically for the SCI IRQ alone. > > The SCI penalize function was finally dropped by commit 9e5ed6d1fb87 > ("ACPI,PCI,IRQ: remove SCI penalize function") that replaced the old SCI > penalty API with penalty calculation carried out dynamically and based > on the acpi_gbl_FADT.sci_interrupt value. > > However, that new algorithm relies on the accurate setting of IRQ > types and that doesn't happen early enough on some platforms which > leads to incorrect penalty assignments for PCI IRQs. In those cases, > irq_get_trigger_type() returns incorrect values for the IRQs in > question, because they have not been registered yet by the time the > penalties are calculated. > > To fix this problem, we only need to fix the penalty for the SCI interrupt. > It seems better to add a single "sci_penalty" variable, set it to > PIRQ_PENALTY_PCI_USING if it's level/low or PIRQ_PENALTY_ISA_ALWAYS > otherwise, and add "sci_penalty" in when appropriate. That should fix it > for *any* SCI IRQ, not just those less than 256, and we don't have to add > these extra penalty table entries that are all unused (except possibly for > one entry if we have an SCI in the 16-255 range). > > For this reason, revert commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI > penalize function") completely to restore the correct behavior. > > Link: https://lkml.org/lkml/2016/10/4/283 > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Fixes: commit 103544d86976 ("ACPI,PCI,IRQ: reduce resource requirements") > Fixes: commit 9e5ed6d1fb87 ("ACPI,PCI,IRQ: remove SCI penalize function") > --- > arch/x86/kernel/acpi/boot.c | 1 + > drivers/acpi/pci_link.c | 30 +++++++++++++++--------------- > include/linux/acpi.h | 1 + > 3 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index 90d84c3..0ffd26e 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -453,6 +453,7 @@ static void __init acpi_sci_ioapic_setup(u8 bus_irq, u16 polarity, u16 trigger, > polarity = acpi_sci_flags & ACPI_MADT_POLARITY_MASK; > > mp_override_legacy_irq(bus_irq, polarity, trigger, gsi); > + acpi_penalize_sci_irq(bus_irq, trigger, polarity); > > /* > * stash over-ride to indicate we've been here > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 4f37938..294b190 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -87,6 +87,7 @@ struct acpi_pci_link { > > static LIST_HEAD(acpi_link_list); > static DEFINE_MUTEX(acpi_link_lock); > +static int sci_irq = -1, sci_penalty; > > /* -------------------------------------------------------------------------- > PCI Link Device Management > @@ -496,25 +497,13 @@ static int acpi_irq_get_penalty(int irq) > { > int penalty = 0; > > - /* > - * Penalize IRQ used by ACPI SCI. If ACPI SCI pin attributes conflict > - * with PCI IRQ attributes, mark ACPI SCI as ISA_ALWAYS so it won't be > - * use for PCI IRQs. > - */ > - if (irq == acpi_gbl_FADT.sci_interrupt) { > - u32 type = irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK; > - > - if (type != IRQ_TYPE_LEVEL_LOW) > - penalty += PIRQ_PENALTY_ISA_ALWAYS; > - else > - penalty += PIRQ_PENALTY_PCI_USING; > - } > + if (irq == sci_irq) > + penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > return penalty + acpi_isa_irq_penalty[irq]; > > - penalty += acpi_irq_pci_sharing_penalty(irq); > - return penalty; > + return penalty + acpi_irq_pci_sharing_penalty(irq); > } > > int __init acpi_irq_penalty_init(void) > @@ -881,6 +870,17 @@ bool acpi_isa_irq_available(int irq) > acpi_irq_get_penalty(irq) < PIRQ_PENALTY_ISA_ALWAYS); > } > > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > +{ > + sci_irq = irq; > + > + if (trigger == ACPI_MADT_TRIGGER_LEVEL && > + polarity == ACPI_MADT_POLARITY_ACTIVE_LOW) > + sci_penalty = PIRQ_PENALTY_PCI_USING; > + else > + sci_penalty = PIRQ_PENALTY_ISA_ALWAYS; > +} > + > /* > * Over-ride default table to reserve additional IRQs for use by ISA > * e.g. acpi_irq_isa=5 > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index c5eaf2f..67d1d3e 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -318,6 +318,7 @@ struct pci_dev; > int acpi_pci_irq_enable (struct pci_dev *dev); > void acpi_penalize_isa_irq(int irq, int active); > bool acpi_isa_irq_available(int irq); > +void acpi_penalize_sci_irq(int irq, int trigger, int polarity); > void acpi_pci_irq_disable (struct pci_dev *dev); > > extern int ec_read(u8 addr, u8 *val); This series fixes one or more network adapters not working in Linux 32-bit x86 guest running inside VirtualBox if I have 4 network adapters enabled. The following message no longer appears in the kernel log: ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off Tested-by: Jonathan Liu <net147@gmail.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 3/3] Revert "ACPI,PCI,IRQ: separate ISA penalty calculation" 2016-10-19 22:21 ` Sinan Kaya (?) @ 2016-10-19 22:21 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: linux-pci, agross, linux-arm-msm, linux-arm-kernel, wim, Sinan Kaya, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Len Brown, Robert Moore, Lv Zheng, linux-kernel, devel This reverts commit f7eca374f000 ("ACPI,PCI,IRQ: separate ISA penalty calculation") and commit 487cf917ed0d ("revert "ACPI, PCI, IRQ: remove redundant code in acpi_irq_penalty_init()""). Now that we understand the real issue (SCI and ISA penalty getting calculated before ACPI start), there is no need for special handling for ISA interrupts. Let's try to simplify the code one more time to share code. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- arch/x86/pci/acpi.c | 1 - drivers/acpi/pci_link.c | 44 +++++--------------------------------------- include/acpi/acpi_drivers.h | 1 - 3 files changed, 5 insertions(+), 41 deletions(-) diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index 3cd6983..b2a4e2a 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -396,7 +396,6 @@ int __init pci_acpi_init(void) return -ENODEV; printk(KERN_INFO "PCI: Using ACPI for IRQ routing\n"); - acpi_irq_penalty_init(); pcibios_enable_irq = acpi_pci_irq_enable; pcibios_disable_irq = acpi_pci_irq_disable; x86_init.pci.init_irq = x86_init_noop; diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 294b190..dd14d78 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -478,7 +478,8 @@ static int acpi_irq_pci_sharing_penalty(int irq) * If a link is active, penalize its IRQ heavily * so we try to choose a different IRQ. */ - if (link->irq.active && link->irq.active == irq) + if ((link->irq.active && link->irq.active == irq) && + (link->irq.initialized == 1)) penalty += PIRQ_PENALTY_PCI_USING; /* @@ -501,45 +502,10 @@ static int acpi_irq_get_penalty(int irq) penalty += sci_penalty; if (irq < ACPI_MAX_ISA_IRQS) - return penalty + acpi_isa_irq_penalty[irq]; + penalty += acpi_isa_irq_penalty[irq]; - return penalty + acpi_irq_pci_sharing_penalty(irq); -} - -int __init acpi_irq_penalty_init(void) -{ - struct acpi_pci_link *link; - int i; - - /* - * Update penalties to facilitate IRQ balancing. - */ - list_for_each_entry(link, &acpi_link_list, list) { - - /* - * reflect the possible and active irqs in the penalty table -- - * useful for breaking ties. - */ - if (link->irq.possible_count) { - int penalty = - PIRQ_PENALTY_PCI_POSSIBLE / - link->irq.possible_count; - - for (i = 0; i < link->irq.possible_count; i++) { - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS) - acpi_isa_irq_penalty[link->irq. - possible[i]] += - penalty; - } - - } else if (link->irq.active && - (link->irq.active < ACPI_MAX_ISA_IRQS)) { - acpi_isa_irq_penalty[link->irq.active] += - PIRQ_PENALTY_PCI_POSSIBLE; - } - } - - return 0; + penalty += acpi_irq_pci_sharing_penalty(irq); + return penalty; } static int acpi_irq_balance = -1; /* 0: static, 1: balance */ diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index 29c6912..797ae2e 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -78,7 +78,6 @@ /* ACPI PCI Interrupt Link (pci_link.c) */ -int acpi_irq_penalty_init(void); int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering, int *polarity, char **name); int acpi_pci_link_free_irq(acpi_handle handle); -- 1.9.1 ^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-arm-kernel This reverts commit f7eca374f000 ("ACPI,PCI,IRQ: separate ISA penalty calculation") and commit 487cf917ed0d ("revert "ACPI, PCI, IRQ: remove redundant code in acpi_irq_penalty_init()""). Now that we understand the real issue (SCI and ISA penalty getting calculated before ACPI start), there is no need for special handling for ISA interrupts. Let's try to simplify the code one more time to share code. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- arch/x86/pci/acpi.c | 1 - drivers/acpi/pci_link.c | 44 +++++--------------------------------------- include/acpi/acpi_drivers.h | 1 - 3 files changed, 5 insertions(+), 41 deletions(-) diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index 3cd6983..b2a4e2a 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -396,7 +396,6 @@ int __init pci_acpi_init(void) return -ENODEV; printk(KERN_INFO "PCI: Using ACPI for IRQ routing\n"); - acpi_irq_penalty_init(); pcibios_enable_irq = acpi_pci_irq_enable; pcibios_disable_irq = acpi_pci_irq_disable; x86_init.pci.init_irq = x86_init_noop; diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 294b190..dd14d78 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -478,7 +478,8 @@ static int acpi_irq_pci_sharing_penalty(int irq) * If a link is active, penalize its IRQ heavily * so we try to choose a different IRQ. */ - if (link->irq.active && link->irq.active == irq) + if ((link->irq.active && link->irq.active == irq) && + (link->irq.initialized == 1)) penalty += PIRQ_PENALTY_PCI_USING; /* @@ -501,45 +502,10 @@ static int acpi_irq_get_penalty(int irq) penalty += sci_penalty; if (irq < ACPI_MAX_ISA_IRQS) - return penalty + acpi_isa_irq_penalty[irq]; + penalty += acpi_isa_irq_penalty[irq]; - return penalty + acpi_irq_pci_sharing_penalty(irq); -} - -int __init acpi_irq_penalty_init(void) -{ - struct acpi_pci_link *link; - int i; - - /* - * Update penalties to facilitate IRQ balancing. - */ - list_for_each_entry(link, &acpi_link_list, list) { - - /* - * reflect the possible and active irqs in the penalty table -- - * useful for breaking ties. - */ - if (link->irq.possible_count) { - int penalty = - PIRQ_PENALTY_PCI_POSSIBLE / - link->irq.possible_count; - - for (i = 0; i < link->irq.possible_count; i++) { - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS) - acpi_isa_irq_penalty[link->irq. - possible[i]] += - penalty; - } - - } else if (link->irq.active && - (link->irq.active < ACPI_MAX_ISA_IRQS)) { - acpi_isa_irq_penalty[link->irq.active] += - PIRQ_PENALTY_PCI_POSSIBLE; - } - } - - return 0; + penalty += acpi_irq_pci_sharing_penalty(irq); + return penalty; } static int acpi_irq_balance = -1; /* 0: static, 1: balance */ diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index 29c6912..797ae2e 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -78,7 +78,6 @@ /* ACPI PCI Interrupt Link (pci_link.c) */ -int acpi_irq_penalty_init(void); int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering, int *polarity, char **name); int acpi_pci_link_free_irq(acpi_handle handle); -- 1.9.1 ^ permalink raw reply related [flat|nested] 56+ messages in thread
* [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" @ 2016-10-19 22:21 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-19 22:21 UTC (permalink / raw) To: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson Cc: x86, H. Peter Anvin, linux-pci, agross, Robert Moore, linux-kernel, Sinan Kaya, Ingo Molnar, Lv Zheng, linux-arm-msm, wim, devel, Thomas Gleixner, linux-arm-kernel, Len Brown This reverts commit f7eca374f000 ("ACPI,PCI,IRQ: separate ISA penalty calculation") and commit 487cf917ed0d ("revert "ACPI, PCI, IRQ: remove redundant code in acpi_irq_penalty_init()""). Now that we understand the real issue (SCI and ISA penalty getting calculated before ACPI start), there is no need for special handling for ISA interrupts. Let's try to simplify the code one more time to share code. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- arch/x86/pci/acpi.c | 1 - drivers/acpi/pci_link.c | 44 +++++--------------------------------------- include/acpi/acpi_drivers.h | 1 - 3 files changed, 5 insertions(+), 41 deletions(-) diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index 3cd6983..b2a4e2a 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -396,7 +396,6 @@ int __init pci_acpi_init(void) return -ENODEV; printk(KERN_INFO "PCI: Using ACPI for IRQ routing\n"); - acpi_irq_penalty_init(); pcibios_enable_irq = acpi_pci_irq_enable; pcibios_disable_irq = acpi_pci_irq_disable; x86_init.pci.init_irq = x86_init_noop; diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 294b190..dd14d78 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -478,7 +478,8 @@ static int acpi_irq_pci_sharing_penalty(int irq) * If a link is active, penalize its IRQ heavily * so we try to choose a different IRQ. */ - if (link->irq.active && link->irq.active == irq) + if ((link->irq.active && link->irq.active == irq) && + (link->irq.initialized == 1)) penalty += PIRQ_PENALTY_PCI_USING; /* @@ -501,45 +502,10 @@ static int acpi_irq_get_penalty(int irq) penalty += sci_penalty; if (irq < ACPI_MAX_ISA_IRQS) - return penalty + acpi_isa_irq_penalty[irq]; + penalty += acpi_isa_irq_penalty[irq]; - return penalty + acpi_irq_pci_sharing_penalty(irq); -} - -int __init acpi_irq_penalty_init(void) -{ - struct acpi_pci_link *link; - int i; - - /* - * Update penalties to facilitate IRQ balancing. - */ - list_for_each_entry(link, &acpi_link_list, list) { - - /* - * reflect the possible and active irqs in the penalty table -- - * useful for breaking ties. - */ - if (link->irq.possible_count) { - int penalty = - PIRQ_PENALTY_PCI_POSSIBLE / - link->irq.possible_count; - - for (i = 0; i < link->irq.possible_count; i++) { - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS) - acpi_isa_irq_penalty[link->irq. - possible[i]] += - penalty; - } - - } else if (link->irq.active && - (link->irq.active < ACPI_MAX_ISA_IRQS)) { - acpi_isa_irq_penalty[link->irq.active] += - PIRQ_PENALTY_PCI_POSSIBLE; - } - } - - return 0; + penalty += acpi_irq_pci_sharing_penalty(irq); + return penalty; } static int acpi_irq_balance = -1; /* 0: static, 1: balance */ diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index 29c6912..797ae2e 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -78,7 +78,6 @@ /* ACPI PCI Interrupt Link (pci_link.c) */ -int acpi_irq_penalty_init(void); int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering, int *polarity, char **name); int acpi_pci_link_free_irq(acpi_handle handle); -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 56+ messages in thread
* Re: [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" 2016-10-19 22:21 ` Sinan Kaya @ 2016-10-21 2:31 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 2:31 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, x86, H. Peter Anvin, linux-pci, agross, Robert Moore, linux-kernel, Ingo Molnar, Lv Zheng, linux-arm-msm, wim, devel, Thomas Gleixner, linux-arm-kernel, Len Brown On Wed, Oct 19, 2016 at 06:21:04PM -0400, Sinan Kaya wrote: > This reverts commit f7eca374f000 ("ACPI,PCI,IRQ: separate ISA penalty > calculation") and commit 487cf917ed0d ("revert "ACPI, PCI, IRQ: remove > redundant code in acpi_irq_penalty_init()""). > > Now that we understand the real issue (SCI and ISA penalty getting > calculated before ACPI start), there is no need for special handling > for ISA interrupts. > > Let's try to simplify the code one more time to share code. I'm sort of OK with this, but it's not exactly a revert of the above (the commits you mention don't check "link->irq.initialized == 1". Previously acpi_irq_penalty_init() looked at _PRS info ("possible" IRQs), but now we won't. Maybe that's good; I dunno. But it should be mentioned. And I don't think it fixes a user-visible problem, so it doesn't need to be applied immediately. I'm not sure this is worth doing by itself; maybe it should wait until we can do more cleanup and think about all these issues together? > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > arch/x86/pci/acpi.c | 1 - > drivers/acpi/pci_link.c | 44 +++++--------------------------------------- > include/acpi/acpi_drivers.h | 1 - > 3 files changed, 5 insertions(+), 41 deletions(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index 3cd6983..b2a4e2a 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -396,7 +396,6 @@ int __init pci_acpi_init(void) > return -ENODEV; > > printk(KERN_INFO "PCI: Using ACPI for IRQ routing\n"); > - acpi_irq_penalty_init(); > pcibios_enable_irq = acpi_pci_irq_enable; > pcibios_disable_irq = acpi_pci_irq_disable; > x86_init.pci.init_irq = x86_init_noop; > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 294b190..dd14d78 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -478,7 +478,8 @@ static int acpi_irq_pci_sharing_penalty(int irq) > * If a link is active, penalize its IRQ heavily > * so we try to choose a different IRQ. > */ > - if (link->irq.active && link->irq.active == irq) > + if ((link->irq.active && link->irq.active == irq) && > + (link->irq.initialized == 1)) > penalty += PIRQ_PENALTY_PCI_USING; > > /* > @@ -501,45 +502,10 @@ static int acpi_irq_get_penalty(int irq) > penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > - return penalty + acpi_isa_irq_penalty[irq]; > + penalty += acpi_isa_irq_penalty[irq]; > > - return penalty + acpi_irq_pci_sharing_penalty(irq); > -} > - > -int __init acpi_irq_penalty_init(void) > -{ > - struct acpi_pci_link *link; > - int i; > - > - /* > - * Update penalties to facilitate IRQ balancing. > - */ > - list_for_each_entry(link, &acpi_link_list, list) { > - > - /* > - * reflect the possible and active irqs in the penalty table -- > - * useful for breaking ties. > - */ > - if (link->irq.possible_count) { > - int penalty = > - PIRQ_PENALTY_PCI_POSSIBLE / > - link->irq.possible_count; > - > - for (i = 0; i < link->irq.possible_count; i++) { > - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS) > - acpi_isa_irq_penalty[link->irq. > - possible[i]] += > - penalty; > - } > - > - } else if (link->irq.active && > - (link->irq.active < ACPI_MAX_ISA_IRQS)) { > - acpi_isa_irq_penalty[link->irq.active] += > - PIRQ_PENALTY_PCI_POSSIBLE; > - } > - } > - > - return 0; > + penalty += acpi_irq_pci_sharing_penalty(irq); > + return penalty; > } > > static int acpi_irq_balance = -1; /* 0: static, 1: balance */ > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > index 29c6912..797ae2e 100644 > --- a/include/acpi/acpi_drivers.h > +++ b/include/acpi/acpi_drivers.h > @@ -78,7 +78,6 @@ > > /* ACPI PCI Interrupt Link (pci_link.c) */ > > -int acpi_irq_penalty_init(void); > int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering, > int *polarity, char **name); > int acpi_pci_link_free_irq(acpi_handle handle); > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" @ 2016-10-21 2:31 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-21 2:31 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 19, 2016 at 06:21:04PM -0400, Sinan Kaya wrote: > This reverts commit f7eca374f000 ("ACPI,PCI,IRQ: separate ISA penalty > calculation") and commit 487cf917ed0d ("revert "ACPI, PCI, IRQ: remove > redundant code in acpi_irq_penalty_init()""). > > Now that we understand the real issue (SCI and ISA penalty getting > calculated before ACPI start), there is no need for special handling > for ISA interrupts. > > Let's try to simplify the code one more time to share code. I'm sort of OK with this, but it's not exactly a revert of the above (the commits you mention don't check "link->irq.initialized == 1". Previously acpi_irq_penalty_init() looked at _PRS info ("possible" IRQs), but now we won't. Maybe that's good; I dunno. But it should be mentioned. And I don't think it fixes a user-visible problem, so it doesn't need to be applied immediately. I'm not sure this is worth doing by itself; maybe it should wait until we can do more cleanup and think about all these issues together? > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > arch/x86/pci/acpi.c | 1 - > drivers/acpi/pci_link.c | 44 +++++--------------------------------------- > include/acpi/acpi_drivers.h | 1 - > 3 files changed, 5 insertions(+), 41 deletions(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index 3cd6983..b2a4e2a 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -396,7 +396,6 @@ int __init pci_acpi_init(void) > return -ENODEV; > > printk(KERN_INFO "PCI: Using ACPI for IRQ routing\n"); > - acpi_irq_penalty_init(); > pcibios_enable_irq = acpi_pci_irq_enable; > pcibios_disable_irq = acpi_pci_irq_disable; > x86_init.pci.init_irq = x86_init_noop; > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 294b190..dd14d78 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -478,7 +478,8 @@ static int acpi_irq_pci_sharing_penalty(int irq) > * If a link is active, penalize its IRQ heavily > * so we try to choose a different IRQ. > */ > - if (link->irq.active && link->irq.active == irq) > + if ((link->irq.active && link->irq.active == irq) && > + (link->irq.initialized == 1)) > penalty += PIRQ_PENALTY_PCI_USING; > > /* > @@ -501,45 +502,10 @@ static int acpi_irq_get_penalty(int irq) > penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > - return penalty + acpi_isa_irq_penalty[irq]; > + penalty += acpi_isa_irq_penalty[irq]; > > - return penalty + acpi_irq_pci_sharing_penalty(irq); > -} > - > -int __init acpi_irq_penalty_init(void) > -{ > - struct acpi_pci_link *link; > - int i; > - > - /* > - * Update penalties to facilitate IRQ balancing. > - */ > - list_for_each_entry(link, &acpi_link_list, list) { > - > - /* > - * reflect the possible and active irqs in the penalty table -- > - * useful for breaking ties. > - */ > - if (link->irq.possible_count) { > - int penalty = > - PIRQ_PENALTY_PCI_POSSIBLE / > - link->irq.possible_count; > - > - for (i = 0; i < link->irq.possible_count; i++) { > - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS) > - acpi_isa_irq_penalty[link->irq. > - possible[i]] += > - penalty; > - } > - > - } else if (link->irq.active && > - (link->irq.active < ACPI_MAX_ISA_IRQS)) { > - acpi_isa_irq_penalty[link->irq.active] += > - PIRQ_PENALTY_PCI_POSSIBLE; > - } > - } > - > - return 0; > + penalty += acpi_irq_pci_sharing_penalty(irq); > + return penalty; > } > > static int acpi_irq_balance = -1; /* 0: static, 1: balance */ > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > index 29c6912..797ae2e 100644 > --- a/include/acpi/acpi_drivers.h > +++ b/include/acpi/acpi_drivers.h > @@ -78,7 +78,6 @@ > > /* ACPI PCI Interrupt Link (pci_link.c) */ > > -int acpi_irq_penalty_init(void); > int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering, > int *polarity, char **name); > int acpi_pci_link_free_irq(acpi_handle handle); > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" 2016-10-21 2:31 ` Bjorn Helgaas @ 2016-10-21 2:58 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-21 2:58 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, x86, H. Peter Anvin, linux-pci, agross, Robert Moore, linux-kernel, Ingo Molnar, Lv Zheng, linux-arm-msm, wim, devel, Thomas Gleixner, linux-arm-kernel, Len Brown On 10/20/2016 7:31 PM, Bjorn Helgaas wrote: >> Let's try to simplify the code one more time to share code. > I'm sort of OK with this, but it's not exactly a revert of the above > (the commits you mention don't check "link->irq.initialized == 1". I can split the initialized bit. If I remove it from this commit, it can break the git bisect. That's why, I folded it into this review. I briefly mentioned about it in the cover letter. It might not be quiet clear. > > Previously acpi_irq_penalty_init() looked at _PRS info ("possible" > IRQs), but now we won't. Maybe that's good; I dunno. But it should > be mentioned. I'm directing all IRQs to acpi_irq_pci_sharing_penalty function. acpi_irq_pci_sharing_penalty checks for the possible values here from _PRS. /* * penalize the IRQs PCI might use, but not as severely. */ for (i = 0; i < link->irq.possible_count; i++) if (link->irq.possible[i] == irq) penalty += PIRQ_PENALTY_PCI_POSSIBLE / link->irq.possible_count; > > And I don't think it fixes a user-visible problem, so it doesn't need > to be applied immediately. I'm not sure this is worth doing by > itself; maybe it should wait until we can do more cleanup and think > about all these issues together? > It does fix the PCI_USING penalty assignment. if (link->irq.active && link->irq.active == irq) penalty += PIRQ_PENALTY_PCI_USING; If we drop this patch, then we need [PATCH V3 1/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts http://www.gossamer-threads.com/lists/linux/kernel/2547605 as somebody needs to increment the penalty with PCI_USING when IRQ is assigned for a given ISA IRQ. We might as well take [PATCH V4 1/3], [PATCH V4 2/3] and [PATCH V3 1/3] for this regression. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" @ 2016-10-21 2:58 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-21 2:58 UTC (permalink / raw) To: linux-arm-kernel On 10/20/2016 7:31 PM, Bjorn Helgaas wrote: >> Let's try to simplify the code one more time to share code. > I'm sort of OK with this, but it's not exactly a revert of the above > (the commits you mention don't check "link->irq.initialized == 1". I can split the initialized bit. If I remove it from this commit, it can break the git bisect. That's why, I folded it into this review. I briefly mentioned about it in the cover letter. It might not be quiet clear. > > Previously acpi_irq_penalty_init() looked at _PRS info ("possible" > IRQs), but now we won't. Maybe that's good; I dunno. But it should > be mentioned. I'm directing all IRQs to acpi_irq_pci_sharing_penalty function. acpi_irq_pci_sharing_penalty checks for the possible values here from _PRS. /* * penalize the IRQs PCI might use, but not as severely. */ for (i = 0; i < link->irq.possible_count; i++) if (link->irq.possible[i] == irq) penalty += PIRQ_PENALTY_PCI_POSSIBLE / link->irq.possible_count; > > And I don't think it fixes a user-visible problem, so it doesn't need > to be applied immediately. I'm not sure this is worth doing by > itself; maybe it should wait until we can do more cleanup and think > about all these issues together? > It does fix the PCI_USING penalty assignment. if (link->irq.active && link->irq.active == irq) penalty += PIRQ_PENALTY_PCI_USING; If we drop this patch, then we need [PATCH V3 1/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts http://www.gossamer-threads.com/lists/linux/kernel/2547605 as somebody needs to increment the penalty with PCI_USING when IRQ is assigned for a given ISA IRQ. We might as well take [PATCH V4 1/3], [PATCH V4 2/3] and [PATCH V3 1/3] for this regression. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" 2016-10-21 2:58 ` Sinan Kaya @ 2016-10-22 23:59 ` Bjorn Helgaas -1 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-22 23:59 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, x86, H. Peter Anvin, linux-pci, agross, Robert Moore, linux-kernel, Ingo Molnar, Lv Zheng, linux-arm-msm, wim, devel, Thomas Gleixner, linux-arm-kernel, Len Brown On Thu, Oct 20, 2016 at 07:58:57PM -0700, Sinan Kaya wrote: > On 10/20/2016 7:31 PM, Bjorn Helgaas wrote: > ... > > And I don't think it fixes a user-visible problem, so it doesn't need > > to be applied immediately. I'm not sure this is worth doing by > > itself; maybe it should wait until we can do more cleanup and think > > about all these issues together? > > It does fix the PCI_USING penalty assignment. > > if (link->irq.active && link->irq.active == irq) > penalty += PIRQ_PENALTY_PCI_USING; > > > If we drop this patch, then we need > [PATCH V3 1/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts > > http://www.gossamer-threads.com/lists/linux/kernel/2547605 > > as somebody needs to increment the penalty with PCI_USING when IRQ is assigned > for a given ISA IRQ. > > We might as well take [PATCH V4 1/3], [PATCH V4 2/3] and [PATCH V3 1/3] > for this regression. It sounds like either V3 1/3 or V4 3/3 will fix the regression. The V3 1/3 patch is much smaller and essentially makes this piece look like it did in v4.6. The V4 3/3 patch removes acpi_irq_penalty_init() and compensates by using acpi_irq_pci_sharing_penalty() for ISA IRQs again. But acpi_irq_penalty_init() added PIRQ_PENALTY_PCI_POSSIBLE for _CRS, and only if there was no _PRS, while acpi_irq_pci_sharing_penalty() always adds PIRQ_PENALTY_PCI_USING for _CRS, regardless of whether _PRS exists. Since V4 3/3 is so much bigger and makes this quite subtle change in how _CRS is handled, I like V3 1/3 better. Are we all set to go now? I think I've acked the patches you mentioned. Bjorn ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" @ 2016-10-22 23:59 ` Bjorn Helgaas 0 siblings, 0 replies; 56+ messages in thread From: Bjorn Helgaas @ 2016-10-22 23:59 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 20, 2016 at 07:58:57PM -0700, Sinan Kaya wrote: > On 10/20/2016 7:31 PM, Bjorn Helgaas wrote: > ... > > And I don't think it fixes a user-visible problem, so it doesn't need > > to be applied immediately. I'm not sure this is worth doing by > > itself; maybe it should wait until we can do more cleanup and think > > about all these issues together? > > It does fix the PCI_USING penalty assignment. > > if (link->irq.active && link->irq.active == irq) > penalty += PIRQ_PENALTY_PCI_USING; > > > If we drop this patch, then we need > [PATCH V3 1/3] ACPI, PCI IRQ: add PCI_USING penalty for ISA interrupts > > http://www.gossamer-threads.com/lists/linux/kernel/2547605 > > as somebody needs to increment the penalty with PCI_USING when IRQ is assigned > for a given ISA IRQ. > > We might as well take [PATCH V4 1/3], [PATCH V4 2/3] and [PATCH V3 1/3] > for this regression. It sounds like either V3 1/3 or V4 3/3 will fix the regression. The V3 1/3 patch is much smaller and essentially makes this piece look like it did in v4.6. The V4 3/3 patch removes acpi_irq_penalty_init() and compensates by using acpi_irq_pci_sharing_penalty() for ISA IRQs again. But acpi_irq_penalty_init() added PIRQ_PENALTY_PCI_POSSIBLE for _CRS, and only if there was no _PRS, while acpi_irq_pci_sharing_penalty() always adds PIRQ_PENALTY_PCI_USING for _CRS, regardless of whether _PRS exists. Since V4 3/3 is so much bigger and makes this quite subtle change in how _CRS is handled, I like V3 1/3 better. Are we all set to go now? I think I've acked the patches you mentioned. Bjorn ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" 2016-10-22 23:59 ` Bjorn Helgaas @ 2016-10-24 4:16 ` Sinan Kaya -1 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 4:16 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-acpi, rjw, bhelgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, x86, H. Peter Anvin, linux-pci, agross, Robert Moore, linux-kernel, Ingo Molnar, Lv Zheng, linux-arm-msm, wim, devel, Thomas Gleixner, linux-arm-kernel, Len Brown On 10/22/2016 7:59 PM, Bjorn Helgaas wrote: > Since V4 3/3 is so much bigger and makes this quite subtle change in > how _CRS is handled, I like V3 1/3 better. > OK > Are we all set to go now? I think I've acked the patches you > mentioned. Yes, I'll post a follow up with your recommendations. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: separate ISA penalty calculation" @ 2016-10-24 4:16 ` Sinan Kaya 0 siblings, 0 replies; 56+ messages in thread From: Sinan Kaya @ 2016-10-24 4:16 UTC (permalink / raw) To: linux-arm-kernel On 10/22/2016 7:59 PM, Bjorn Helgaas wrote: > Since V4 3/3 is so much bigger and makes this quite subtle change in > how _CRS is handled, I like V3 1/3 better. > OK > Are we all set to go now? I think I've acked the patches you > mentioned. Yes, I'll post a follow up with your recommendations. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [V4,3/3] Revert "ACPI,PCI,IRQ: separate ISA penalty calculation" 2016-10-19 22:21 ` Sinan Kaya @ 2016-10-23 3:49 ` Jonathan Liu -1 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-23 3:49 UTC (permalink / raw) To: Sinan Kaya Cc: linux-acpi, rjw, Bjorn Helgaas, ravikanth.nalla, linux, timur, cov, jcm, alex.williamson, linux-pci, Andy Gross, linux-arm-msm, linux-arm-kernel, wim, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Len Brown, Robert Moore, Lv Zheng, linux-kernel, devel On 20 October 2016 at 09:21, Sinan Kaya <okaya@codeaurora.org> wrote: > This reverts commit f7eca374f000 ("ACPI,PCI,IRQ: separate ISA penalty > calculation") and commit 487cf917ed0d ("revert "ACPI, PCI, IRQ: remove > redundant code in acpi_irq_penalty_init()""). > > Now that we understand the real issue (SCI and ISA penalty getting > calculated before ACPI start), there is no need for special handling > for ISA interrupts. > > Let's try to simplify the code one more time to share code. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > arch/x86/pci/acpi.c | 1 - > drivers/acpi/pci_link.c | 44 +++++--------------------------------------- > include/acpi/acpi_drivers.h | 1 - > 3 files changed, 5 insertions(+), 41 deletions(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index 3cd6983..b2a4e2a 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -396,7 +396,6 @@ int __init pci_acpi_init(void) > return -ENODEV; > > printk(KERN_INFO "PCI: Using ACPI for IRQ routing\n"); > - acpi_irq_penalty_init(); > pcibios_enable_irq = acpi_pci_irq_enable; > pcibios_disable_irq = acpi_pci_irq_disable; > x86_init.pci.init_irq = x86_init_noop; > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 294b190..dd14d78 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -478,7 +478,8 @@ static int acpi_irq_pci_sharing_penalty(int irq) > * If a link is active, penalize its IRQ heavily > * so we try to choose a different IRQ. > */ > - if (link->irq.active && link->irq.active == irq) > + if ((link->irq.active && link->irq.active == irq) && > + (link->irq.initialized == 1)) > penalty += PIRQ_PENALTY_PCI_USING; > > /* > @@ -501,45 +502,10 @@ static int acpi_irq_get_penalty(int irq) > penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > - return penalty + acpi_isa_irq_penalty[irq]; > + penalty += acpi_isa_irq_penalty[irq]; > > - return penalty + acpi_irq_pci_sharing_penalty(irq); > -} > - > -int __init acpi_irq_penalty_init(void) > -{ > - struct acpi_pci_link *link; > - int i; > - > - /* > - * Update penalties to facilitate IRQ balancing. > - */ > - list_for_each_entry(link, &acpi_link_list, list) { > - > - /* > - * reflect the possible and active irqs in the penalty table -- > - * useful for breaking ties. > - */ > - if (link->irq.possible_count) { > - int penalty = > - PIRQ_PENALTY_PCI_POSSIBLE / > - link->irq.possible_count; > - > - for (i = 0; i < link->irq.possible_count; i++) { > - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS) > - acpi_isa_irq_penalty[link->irq. > - possible[i]] += > - penalty; > - } > - > - } else if (link->irq.active && > - (link->irq.active < ACPI_MAX_ISA_IRQS)) { > - acpi_isa_irq_penalty[link->irq.active] += > - PIRQ_PENALTY_PCI_POSSIBLE; > - } > - } > - > - return 0; > + penalty += acpi_irq_pci_sharing_penalty(irq); > + return penalty; > } > > static int acpi_irq_balance = -1; /* 0: static, 1: balance */ > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > index 29c6912..797ae2e 100644 > --- a/include/acpi/acpi_drivers.h > +++ b/include/acpi/acpi_drivers.h > @@ -78,7 +78,6 @@ > > /* ACPI PCI Interrupt Link (pci_link.c) */ > > -int acpi_irq_penalty_init(void); > int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering, > int *polarity, char **name); > int acpi_pci_link_free_irq(acpi_handle handle); This series fixes one or more network adapters not working in Linux 32-bit x86 guest running inside VirtualBox if I have 4 network adapters enabled. The following message no longer appears in the kernel log: ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off Tested-by: Jonathan Liu <net147@gmail.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
* [V4,3/3] Revert "ACPI,PCI,IRQ: separate ISA penalty calculation" @ 2016-10-23 3:49 ` Jonathan Liu 0 siblings, 0 replies; 56+ messages in thread From: Jonathan Liu @ 2016-10-23 3:49 UTC (permalink / raw) To: linux-arm-kernel On 20 October 2016 at 09:21, Sinan Kaya <okaya@codeaurora.org> wrote: > This reverts commit f7eca374f000 ("ACPI,PCI,IRQ: separate ISA penalty > calculation") and commit 487cf917ed0d ("revert "ACPI, PCI, IRQ: remove > redundant code in acpi_irq_penalty_init()""). > > Now that we understand the real issue (SCI and ISA penalty getting > calculated before ACPI start), there is no need for special handling > for ISA interrupts. > > Let's try to simplify the code one more time to share code. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > arch/x86/pci/acpi.c | 1 - > drivers/acpi/pci_link.c | 44 +++++--------------------------------------- > include/acpi/acpi_drivers.h | 1 - > 3 files changed, 5 insertions(+), 41 deletions(-) > > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index 3cd6983..b2a4e2a 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -396,7 +396,6 @@ int __init pci_acpi_init(void) > return -ENODEV; > > printk(KERN_INFO "PCI: Using ACPI for IRQ routing\n"); > - acpi_irq_penalty_init(); > pcibios_enable_irq = acpi_pci_irq_enable; > pcibios_disable_irq = acpi_pci_irq_disable; > x86_init.pci.init_irq = x86_init_noop; > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 294b190..dd14d78 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -478,7 +478,8 @@ static int acpi_irq_pci_sharing_penalty(int irq) > * If a link is active, penalize its IRQ heavily > * so we try to choose a different IRQ. > */ > - if (link->irq.active && link->irq.active == irq) > + if ((link->irq.active && link->irq.active == irq) && > + (link->irq.initialized == 1)) > penalty += PIRQ_PENALTY_PCI_USING; > > /* > @@ -501,45 +502,10 @@ static int acpi_irq_get_penalty(int irq) > penalty += sci_penalty; > > if (irq < ACPI_MAX_ISA_IRQS) > - return penalty + acpi_isa_irq_penalty[irq]; > + penalty += acpi_isa_irq_penalty[irq]; > > - return penalty + acpi_irq_pci_sharing_penalty(irq); > -} > - > -int __init acpi_irq_penalty_init(void) > -{ > - struct acpi_pci_link *link; > - int i; > - > - /* > - * Update penalties to facilitate IRQ balancing. > - */ > - list_for_each_entry(link, &acpi_link_list, list) { > - > - /* > - * reflect the possible and active irqs in the penalty table -- > - * useful for breaking ties. > - */ > - if (link->irq.possible_count) { > - int penalty = > - PIRQ_PENALTY_PCI_POSSIBLE / > - link->irq.possible_count; > - > - for (i = 0; i < link->irq.possible_count; i++) { > - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQS) > - acpi_isa_irq_penalty[link->irq. > - possible[i]] += > - penalty; > - } > - > - } else if (link->irq.active && > - (link->irq.active < ACPI_MAX_ISA_IRQS)) { > - acpi_isa_irq_penalty[link->irq.active] += > - PIRQ_PENALTY_PCI_POSSIBLE; > - } > - } > - > - return 0; > + penalty += acpi_irq_pci_sharing_penalty(irq); > + return penalty; > } > > static int acpi_irq_balance = -1; /* 0: static, 1: balance */ > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h > index 29c6912..797ae2e 100644 > --- a/include/acpi/acpi_drivers.h > +++ b/include/acpi/acpi_drivers.h > @@ -78,7 +78,6 @@ > > /* ACPI PCI Interrupt Link (pci_link.c) */ > > -int acpi_irq_penalty_init(void); > int acpi_pci_link_allocate_irq(acpi_handle handle, int index, int *triggering, > int *polarity, char **name); > int acpi_pci_link_free_irq(acpi_handle handle); This series fixes one or more network adapters not working in Linux 32-bit x86 guest running inside VirtualBox if I have 4 network adapters enabled. The following message no longer appears in the kernel log: ACPI: No IRQ available for PCI Interrupt Link [LNKD]. Try pci=noacpi or acpi=off Tested-by: Jonathan Liu <net147@gmail.com> ^ permalink raw reply [flat|nested] 56+ messages in thread
end of thread, other threads:[~2016-10-24 4:21 UTC | newest] Thread overview: 56+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-19 22:21 [PATCH V4 0/3] ACPI,PCI,IRQ: revert penalty calculation for ISA and SCI interrupts Sinan Kaya 2016-10-19 22:21 ` [PATCH V4 0/3] ACPI, PCI, IRQ: " Sinan Kaya 2016-10-19 22:21 ` Sinan Kaya 2016-10-19 22:21 ` [PATCH V4 1/3] ACPI, PCI, IRQ: assign ISA IRQ directly during early boot stages Sinan Kaya 2016-10-19 22:21 ` Sinan Kaya 2016-10-19 22:21 ` Sinan Kaya 2016-10-20 21:39 ` Rafael J. Wysocki 2016-10-20 21:39 ` Rafael J. Wysocki 2016-10-20 21:39 ` Rafael J. Wysocki 2016-10-24 3:48 ` Sinan Kaya 2016-10-24 3:48 ` Sinan Kaya 2016-10-24 3:48 ` Sinan Kaya 2016-10-21 1:39 ` Bjorn Helgaas 2016-10-21 1:39 ` Bjorn Helgaas 2016-10-21 14:07 ` Bjorn Helgaas 2016-10-21 14:07 ` Bjorn Helgaas 2016-10-24 3:22 ` Sinan Kaya 2016-10-24 3:22 ` Sinan Kaya 2016-10-23 3:48 ` [V4, " Jonathan Liu 2016-10-23 3:48 ` Jonathan Liu 2016-10-24 4:17 ` Sinan Kaya 2016-10-24 4:17 ` Sinan Kaya 2016-10-24 4:21 ` Jonathan Liu 2016-10-24 4:21 ` Jonathan Liu 2016-10-19 22:21 ` [PATCH V4 2/3] Revert "ACPI,PCI,IRQ: remove SCI penalize function" Sinan Kaya 2016-10-19 22:21 ` Sinan Kaya 2016-10-19 22:21 ` Sinan Kaya 2016-10-21 1:58 ` Bjorn Helgaas 2016-10-21 1:58 ` Bjorn Helgaas 2016-10-21 14:45 ` Bjorn Helgaas 2016-10-21 14:45 ` Bjorn Helgaas 2016-10-21 16:13 ` Sinan Kaya 2016-10-21 16:13 ` Sinan Kaya 2016-10-22 14:57 ` Bjorn Helgaas 2016-10-22 14:57 ` Bjorn Helgaas 2016-10-24 3:48 ` Sinan Kaya 2016-10-24 3:48 ` Sinan Kaya 2016-10-22 23:28 ` Bjorn Helgaas 2016-10-22 23:28 ` Bjorn Helgaas 2016-10-24 3:48 ` Sinan Kaya 2016-10-24 3:48 ` Sinan Kaya 2016-10-23 3:49 ` [V4,2/3] " Jonathan Liu 2016-10-23 3:49 ` Jonathan Liu 2016-10-19 22:21 ` [PATCH V4 3/3] Revert "ACPI,PCI,IRQ: separate ISA penalty calculation" Sinan Kaya 2016-10-19 22:21 ` [PATCH V4 3/3] Revert "ACPI, PCI, IRQ: " Sinan Kaya 2016-10-19 22:21 ` Sinan Kaya 2016-10-21 2:31 ` Bjorn Helgaas 2016-10-21 2:31 ` Bjorn Helgaas 2016-10-21 2:58 ` Sinan Kaya 2016-10-21 2:58 ` Sinan Kaya 2016-10-22 23:59 ` Bjorn Helgaas 2016-10-22 23:59 ` Bjorn Helgaas 2016-10-24 4:16 ` Sinan Kaya 2016-10-24 4:16 ` Sinan Kaya 2016-10-23 3:49 ` [V4,3/3] Revert "ACPI,PCI,IRQ: " Jonathan Liu 2016-10-23 3:49 ` Jonathan Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.