* Re: drivers/pci/quirks.c: irq_reroute_variant is dead code?
2009-08-11 8:06 ` Ingo Molnar
@ 2009-08-11 12:36 ` Olaf Dabrunz
2009-08-11 13:48 ` Stefan Assmann
2009-09-07 16:09 ` Stefan Assmann
2009-09-07 16:09 ` Stefan Assmann
2 siblings, 1 reply; 10+ messages in thread
From: Olaf Dabrunz @ 2009-08-11 12:36 UTC (permalink / raw)
To: Ingo Molnar
Cc: Stefan Assmann, Len Brown, Jesse Barnes, Jeremy Fitzhardinge,
Linux Kernel Mailing List, Ingo Molnar, the arch/x86 maintainers,
Olaf Dabrunz
On 11-Aug-09, Ingo Molnar wrote:
>
> * Stefan Assmann <sassmann@redhat.com> wrote:
>
> > On 10.08.2009 18:32, Jesse Barnes wrote:
> >> [Corrected Stefan's address.]
> >
> > Thanks Jesse! CCing Olaf as well.
> >
> >> On Sat, 8 Aug 2009 16:45:20 +0200
> >> Ingo Molnar<mingo@elte.hu> wrote:
> >>
> >>> * Jesse Barnes<jbarnes@virtuousgeek.org> wrote:
> >>>
> >>>> On Tue, 28 Jul 2009 12:50:08 -0700
> >>>> Jeremy Fitzhardinge<jeremy@goop.org> wrote:
> >>>>
> >>>>> quirk_reroute_to_boot_interrupts_intel() goes to some effort to
> >>>>> scan for a bunch of Intel devices and set
> >>>>> dev->irq_reroute_variant = INTEL_IRQ_REROUTE_VARIANT. However,
> >>>>> as far as I can see that's the only reference to
> >>>>> dev->irq_reroute_variant and INTEL_IRQ_REROUTE_VARIANT, so the
> >>>>> whole thing appears to have no effect.
> >>>>>
> >>>>> Is it all dead code? Is there something else which should be
> >>>>> there missing?
> >>>> Hm my quick look couldn't find a user either. The initial patch
> >>>> to add that stuff (e1d3a90846b40ad3160bf4b648d36c6badad39ac) had
> >>>> an ACPI component, but it looks like that may have vanished
> >>>> somehow (maybe some of the IOAPIC code got shuffled around and
> >>>> this bit was missed?).
> >>>>
> >>>> Stefan or Ingo?
> >>> I dont remember ioapic interactions. Stefan?
> >>>
> >>> Ingo
> >>>
> >
> > Actually the ACPI part of the commit got trashed by some major
> > ACPI rewrite. [...]
>
> Yeah, i remember some larger reject resolved by Len i think, about
> two cycles ago, in this area. So we probably lost those bits there -
> i've Cc:-ed Len.
>
> There's no urgency in terms of v2.6.31 (since we never really
> exposed this new functionality in a new release), but it would be
> nice to get to the bottom of this.
Yep, that would be nice.
I was made aware of that problem early this year when the ACPI code
change was made. Since then I heard that someone already wrote some
patch for the boot IRQ quirks. But obviously that referred to some other
part of the quirk. Stefan?
As I am currently unemployed, demotivated and trying to get my work on
fctrace going again (as well as to get the rest of my life in order),
it seems I have no time to fix this. I would love to, really. We need to
get these quirks in, and I would like to discuss how to do this The Right
Way with the rewritten ACPI code (and the existing MP code).
Regards,
--
Olaf Dabrunz (Olaf.Dabrunz <at> gmx.net)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: drivers/pci/quirks.c: irq_reroute_variant is dead code?
2009-08-11 12:36 ` Olaf Dabrunz
@ 2009-08-11 13:48 ` Stefan Assmann
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Assmann @ 2009-08-11 13:48 UTC (permalink / raw)
To: Olaf Dabrunz
Cc: Ingo Molnar, Len Brown, Jesse Barnes, Jeremy Fitzhardinge,
Linux Kernel Mailing List, the arch/x86 maintainers
On 11.08.2009 14:36, Olaf Dabrunz wrote:
> On 11-Aug-09, Ingo Molnar wrote:
>> * Stefan Assmann<sassmann@redhat.com> wrote:
>>
>>> On 10.08.2009 18:32, Jesse Barnes wrote:
>>>> [Corrected Stefan's address.]
>>> Thanks Jesse! CCing Olaf as well.
>>>
>>>> On Sat, 8 Aug 2009 16:45:20 +0200
>>>> Ingo Molnar<mingo@elte.hu> wrote:
>>>>
>>>>> * Jesse Barnes<jbarnes@virtuousgeek.org> wrote:
>>>>>
>>>>>> On Tue, 28 Jul 2009 12:50:08 -0700
>>>>>> Jeremy Fitzhardinge<jeremy@goop.org> wrote:
>>>>>>
>>>>>>> quirk_reroute_to_boot_interrupts_intel() goes to some effort to
>>>>>>> scan for a bunch of Intel devices and set
>>>>>>> dev->irq_reroute_variant = INTEL_IRQ_REROUTE_VARIANT. However,
>>>>>>> as far as I can see that's the only reference to
>>>>>>> dev->irq_reroute_variant and INTEL_IRQ_REROUTE_VARIANT, so the
>>>>>>> whole thing appears to have no effect.
>>>>>>>
>>>>>>> Is it all dead code? Is there something else which should be
>>>>>>> there missing?
>>>>>> Hm my quick look couldn't find a user either. The initial patch
>>>>>> to add that stuff (e1d3a90846b40ad3160bf4b648d36c6badad39ac) had
>>>>>> an ACPI component, but it looks like that may have vanished
>>>>>> somehow (maybe some of the IOAPIC code got shuffled around and
>>>>>> this bit was missed?).
>>>>>>
>>>>>> Stefan or Ingo?
>>>>> I dont remember ioapic interactions. Stefan?
>>>>>
>>>>> Ingo
>>>>>
>>> Actually the ACPI part of the commit got trashed by some major
>>> ACPI rewrite. [...]
>> Yeah, i remember some larger reject resolved by Len i think, about
>> two cycles ago, in this area. So we probably lost those bits there -
>> i've Cc:-ed Len.
>>
>> There's no urgency in terms of v2.6.31 (since we never really
>> exposed this new functionality in a new release), but it would be
>> nice to get to the bottom of this.
>
> Yep, that would be nice.
>
> I was made aware of that problem early this year when the ACPI code
> change was made. Since then I heard that someone already wrote some
> patch for the boot IRQ quirks. But obviously that referred to some other
> part of the quirk. Stefan?
That's a whole different story (other chipset). ;-)
> As I am currently unemployed, demotivated and trying to get my work on
> fctrace going again (as well as to get the rest of my life in order),
> it seems I have no time to fix this. I would love to, really. We need to
> get these quirks in, and I would like to discuss how to do this The Right
> Way with the rewritten ACPI code (and the existing MP code).
Well, best of luck!
Stefan
--
Stefan Assmann | Red Hat GmbH
Software Engineer | Otto-Hahn-Strasse 20, 85609 Dornach
| HR: Amtsgericht Muenchen HRB 153243
| GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com | Michael Cunningham, Charles Cachera
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: drivers/pci/quirks.c: irq_reroute_variant is dead code?
2009-08-11 8:06 ` Ingo Molnar
2009-08-11 12:36 ` Olaf Dabrunz
@ 2009-09-07 16:09 ` Stefan Assmann
2009-09-07 16:09 ` Stefan Assmann
2 siblings, 0 replies; 10+ messages in thread
From: Stefan Assmann @ 2009-09-07 16:09 UTC (permalink / raw)
To: Ingo Molnar
Cc: Len Brown, Jesse Barnes, Jeremy Fitzhardinge,
Linux Kernel Mailing List, Ingo Molnar, the arch/x86 maintainers,
Olaf Dabrunz, linux-acpi
On 11.08.2009 10:06, Ingo Molnar wrote:
[snip]
>>>>> On Tue, 28 Jul 2009 12:50:08 -0700
>>>>> Jeremy Fitzhardinge<jeremy@goop.org> wrote:
>>>>>
>>>>>> quirk_reroute_to_boot_interrupts_intel() goes to some effort to
>>>>>> scan for a bunch of Intel devices and set
>>>>>> dev->irq_reroute_variant = INTEL_IRQ_REROUTE_VARIANT. However,
>>>>>> as far as I can see that's the only reference to
>>>>>> dev->irq_reroute_variant and INTEL_IRQ_REROUTE_VARIANT, so the
>>>>>> whole thing appears to have no effect.
>>>>>>
>>>>>> Is it all dead code? Is there something else which should be
>>>>>> there missing?
>>>>> Hm my quick look couldn't find a user either. The initial patch
>>>>> to add that stuff (e1d3a90846b40ad3160bf4b648d36c6badad39ac) had
>>>>> an ACPI component, but it looks like that may have vanished
>>>>> somehow (maybe some of the IOAPIC code got shuffled around and
>>>>> this bit was missed?).
>>>>>
>>>>> Stefan or Ingo?
>>>> I dont remember ioapic interactions. Stefan?
>>>>
>>>> Ingo
>>>>
>>
>> Actually the ACPI part of the commit got trashed by some major
>> ACPI rewrite. [...]
>
> Yeah, i remember some larger reject resolved by Len i think, about
> two cycles ago, in this area. So we probably lost those bits there -
> i've Cc:-ed Len.
>
> There's no urgency in terms of v2.6.31 (since we never really
> exposed this new functionality in a new release), but it would be
> nice to get to the bottom of this.
sorry it took me a while to come up with a patch. Here's some first
attempt of how we could turn CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS
into something useful again.
Ccing linux-acpi@vger.kernel.org as this changes ACPI code.
I'm not really sure if the ACPI GSI is always matches the final IRQ
number. From looking at the code it looks like it is but there might
be exceptions. So please let me know of any troubles that this patch
might cause.
I've tested this patch on a machine with 6700PXH PCI Express-to-PCI
Bridge and the machine booted fine. With the patch enabled and
CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y set the NICs IRQ was
successfully moved from IRQ
48: 0 0 16163 195 IO-APIC-fasteoi eth0
to
16: 0 0 4715 237 IO-APIC-fasteoi uhci_hcd:usb2, uhci_hcd:usb5, eth0
Len, I know that you weren't that pleased by this whole "altering ACPI
information" approach the last time. Please consider it anyway.
Stefan
---
drivers/acpi/pci_irq.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -301,6 +301,61 @@ void acpi_pci_irq_del_prt(struct pci_bus
/* --------------------------------------------------------------------------
PCI Interrupt Routing Support
-------------------------------------------------------------------------- */
+#ifdef CONFIG_X86_IO_APIC
+extern int noioapicquirk;
+extern int noioapicreroute;
+
+static int bridge_has_boot_interrupt_variant(struct pci_bus *bus)
+{
+ struct pci_bus *bus_it;
+
+ for (bus_it = bus ; bus_it ; bus_it = bus_it->parent) {
+ if (!bus_it->self)
+ return 0;
+ if (bus_it->self->irq_reroute_variant)
+ return bus_it->self->irq_reroute_variant;
+ }
+ return 0;
+}
+
+/*
+ * Some chipsets (e.g. Intel 6700PXH) generate a legacy INTx when the IRQ
+ * entry in the chipset's IO-APIC is masked (as, e.g. the RT kernel does
+ * during interrupt handling). When this INTx generation cannot be disabled,
+ * we reroute these interrupts to their legacy equivalent to get rid of
+ * spurious interrupts.
+ */
+static int acpi_reroute_boot_interrupt(struct pci_dev *dev,
+ struct acpi_prt_entry *entry)
+{
+ if (noioapicquirk || noioapicreroute) {
+ return 0;
+ } else {
+ switch (bridge_has_boot_interrupt_variant(dev->bus)) {
+ case 0:
+ /* no rerouting necessary */
+ return 0;
+ case INTEL_IRQ_REROUTE_VARIANT:
+ /*
+ * Remap according to INTx routing table in 6700PXH
+ * specs, intel order number 302628-002, section
+ * 2.15.2. Other chipsets (80332, ...) have the same
+ * mapping and are handled here as well.
+ */
+ dev_info(&dev->dev, "PCI IRQ %d -> rerouted to legacy "
+ "IRQ %d\n", entry->index,
+ (entry->index % 4) + 16);
+ entry->index = (entry->index % 4) + 16;
+ return 1;
+ default:
+ dev_warn(&dev->dev, "Cannot reroute IRQ %d to legacy "
+ "IRQ: unknown mapping\n", entry->index);
+ return -1;
+ }
+ }
+}
+#endif /* CONFIG_X86_IO_APIC */
+
static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
{
struct acpi_prt_entry *entry;
@@ -309,6 +364,9 @@ static struct acpi_prt_entry *acpi_pci_i
entry = acpi_pci_irq_find_prt_entry(dev, pin);
if (entry) {
+#ifdef CONFIG_X86_IO_APIC
+ acpi_reroute_boot_interrupt(dev, entry);
+#endif /* CONFIG_X86_IO_APIC */
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %s[%c] _PRT entry\n",
pci_name(dev), pin_name(pin)));
return entry;
--
Stefan Assmann | Red Hat GmbH
Software Engineer | Otto-Hahn-Strasse 20, 85609 Dornach
| HR: Amtsgericht Muenchen HRB 153243
| GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com | Michael Cunningham, Charles Cachera
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: drivers/pci/quirks.c: irq_reroute_variant is dead code?
2009-08-11 8:06 ` Ingo Molnar
2009-08-11 12:36 ` Olaf Dabrunz
2009-09-07 16:09 ` Stefan Assmann
@ 2009-09-07 16:09 ` Stefan Assmann
2 siblings, 0 replies; 10+ messages in thread
From: Stefan Assmann @ 2009-09-07 16:09 UTC (permalink / raw)
To: Ingo Molnar
Cc: Len Brown, Jesse Barnes, Jeremy Fitzhardinge,
Linux Kernel Mailing List, Ingo Molnar, the arch/x86 maintainers,
Olaf Dabrunz, linux-acpi
On 11.08.2009 10:06, Ingo Molnar wrote:
[snip]
>>>>> On Tue, 28 Jul 2009 12:50:08 -0700
>>>>> Jeremy Fitzhardinge<jeremy@goop.org> wrote:
>>>>>
>>>>>> quirk_reroute_to_boot_interrupts_intel() goes to some effort to
>>>>>> scan for a bunch of Intel devices and set
>>>>>> dev->irq_reroute_variant = INTEL_IRQ_REROUTE_VARIANT. However,
>>>>>> as far as I can see that's the only reference to
>>>>>> dev->irq_reroute_variant and INTEL_IRQ_REROUTE_VARIANT, so the
>>>>>> whole thing appears to have no effect.
>>>>>>
>>>>>> Is it all dead code? Is there something else which should be
>>>>>> there missing?
>>>>> Hm my quick look couldn't find a user either. The initial patch
>>>>> to add that stuff (e1d3a90846b40ad3160bf4b648d36c6badad39ac) had
>>>>> an ACPI component, but it looks like that may have vanished
>>>>> somehow (maybe some of the IOAPIC code got shuffled around and
>>>>> this bit was missed?).
>>>>>
>>>>> Stefan or Ingo?
>>>> I dont remember ioapic interactions. Stefan?
>>>>
>>>> Ingo
>>>>
>>
>> Actually the ACPI part of the commit got trashed by some major
>> ACPI rewrite. [...]
>
> Yeah, i remember some larger reject resolved by Len i think, about
> two cycles ago, in this area. So we probably lost those bits there -
> i've Cc:-ed Len.
>
> There's no urgency in terms of v2.6.31 (since we never really
> exposed this new functionality in a new release), but it would be
> nice to get to the bottom of this.
sorry it took me a while to come up with a patch. Here's some first
attempt of how we could turn CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS
into something useful again.
Ccing linux-acpi@vger.kernel.org as this changes ACPI code.
I'm not really sure if the ACPI GSI is always matches the final IRQ
number. From looking at the code it looks like it is but there might
be exceptions. So please let me know of any troubles that this patch
might cause.
I've tested this patch on a machine with 6700PXH PCI Express-to-PCI
Bridge and the machine booted fine. With the patch enabled and
CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y set the NICs IRQ was
successfully moved from IRQ
48: 0 0 16163 195 IO-APIC-fasteoi eth0
to
16: 0 0 4715 237 IO-APIC-fasteoi uhci_hcd:usb2, uhci_hcd:usb5, eth0
Len, I know that you weren't that pleased by this whole "altering ACPI
information" approach the last time. Please consider it anyway.
Stefan
---
drivers/acpi/pci_irq.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -301,6 +301,61 @@ void acpi_pci_irq_del_prt(struct pci_bus
/* --------------------------------------------------------------------------
PCI Interrupt Routing Support
-------------------------------------------------------------------------- */
+#ifdef CONFIG_X86_IO_APIC
+extern int noioapicquirk;
+extern int noioapicreroute;
+
+static int bridge_has_boot_interrupt_variant(struct pci_bus *bus)
+{
+ struct pci_bus *bus_it;
+
+ for (bus_it = bus ; bus_it ; bus_it = bus_it->parent) {
+ if (!bus_it->self)
+ return 0;
+ if (bus_it->self->irq_reroute_variant)
+ return bus_it->self->irq_reroute_variant;
+ }
+ return 0;
+}
+
+/*
+ * Some chipsets (e.g. Intel 6700PXH) generate a legacy INTx when the IRQ
+ * entry in the chipset's IO-APIC is masked (as, e.g. the RT kernel does
+ * during interrupt handling). When this INTx generation cannot be disabled,
+ * we reroute these interrupts to their legacy equivalent to get rid of
+ * spurious interrupts.
+ */
+static int acpi_reroute_boot_interrupt(struct pci_dev *dev,
+ struct acpi_prt_entry *entry)
+{
+ if (noioapicquirk || noioapicreroute) {
+ return 0;
+ } else {
+ switch (bridge_has_boot_interrupt_variant(dev->bus)) {
+ case 0:
+ /* no rerouting necessary */
+ return 0;
+ case INTEL_IRQ_REROUTE_VARIANT:
+ /*
+ * Remap according to INTx routing table in 6700PXH
+ * specs, intel order number 302628-002, section
+ * 2.15.2. Other chipsets (80332, ...) have the same
+ * mapping and are handled here as well.
+ */
+ dev_info(&dev->dev, "PCI IRQ %d -> rerouted to legacy "
+ "IRQ %d\n", entry->index,
+ (entry->index % 4) + 16);
+ entry->index = (entry->index % 4) + 16;
+ return 1;
+ default:
+ dev_warn(&dev->dev, "Cannot reroute IRQ %d to legacy "
+ "IRQ: unknown mapping\n", entry->index);
+ return -1;
+ }
+ }
+}
+#endif /* CONFIG_X86_IO_APIC */
+
static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
{
struct acpi_prt_entry *entry;
@@ -309,6 +364,9 @@ static struct acpi_prt_entry *acpi_pci_i
entry = acpi_pci_irq_find_prt_entry(dev, pin);
if (entry) {
+#ifdef CONFIG_X86_IO_APIC
+ acpi_reroute_boot_interrupt(dev, entry);
+#endif /* CONFIG_X86_IO_APIC */
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %s[%c] _PRT entry\n",
pci_name(dev), pin_name(pin)));
return entry;
--
Stefan Assmann | Red Hat GmbH
Software Engineer | Otto-Hahn-Strasse 20, 85609 Dornach
| HR: Amtsgericht Muenchen HRB 153243
| GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com | Michael Cunningham, Charles Cachera
^ permalink raw reply [flat|nested] 10+ messages in thread