From: Yinghai Lu on
On 08/03/2010 02:15 AM, Eric W. Biederman wrote:
> Yinghai Lu <yinghai(a)kernel.org> writes:
>
>> On 08/03/2010 01:56 AM, Eric W. Biederman wrote:
>>> Yinghai Lu <yinghai(a)kernel.org> writes:
>>>
>>>> On 08/03/2010 01:00 AM, Eric W. Biederman wrote:
>>>>> Yinghai Lu <yinghai(a)kernel.org> writes:
>>>>>
>>>>>>>> Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
>>>>>>>> ===================================================================
>>>>>>>> --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
>>>>>>>> +++ linux-2.6/arch/x86/kernel/apic/io_apic.c
>>>>>>>> @@ -1029,10 +1029,7 @@ static int pin_2_irq(int idx, int apic,
>>>>>>>> } else {
>>>>>>>> u32 gsi = mp_gsi_routing[apic].gsi_base + pin;
>>>>>>>>
>>>>>>>> - if (gsi >= NR_IRQS_LEGACY)
>>>>>>>> - irq = gsi;
>>>>>>>> - else
>>>>>>>> - irq = gsi_top + gsi;
>>>>>>>> + irq = gsi_to_irq(gsi);
>>>>>>>> }
>>>>>>>>
>>>>>>>> #ifdef CONFIG_X86_32
>>>>>>
>>>>>> what is the point for making irq = gsi_top + gsi when mptable is used instead of acpi?
>>>>>
>>>>> Because it is only convention that when mptables are used that the
>>>>> first apic pins 0-15 are the ISA irqs. This thread witnessed and a
>>>>> pci irq that came in pin < 16 that was not an ISA irq. The truly rare
>>>>> and exotic case would be for the ISA irqs to be outside the first 16
>>>>> ioapic pins but the es7000 did exactly that.
>>>>
>>>> nvidia chipset if acpi is enabled, external pci device will use ioapic from 16 to 23.
>>>>
>>>> if mptable is used, external pci device will not use pin from 16 to 23..., and lot of devices will share same pin.
>>>
>>> Exactly. Pins < 16 are not necessarily ISA irqs, and can be possibly
>>> shared level triggered PCI irqs. Unfortunately there are strange
>>> boards like the es7000 where pins > 16 are ISA irqs.
>>>
>>> The other thing that is gained by having pin_2_irq always remap pins <
>>> 16 is we can get away with the numerous hard codes in the arch/x86 and elsewhere
>>> that assume irq < 16 is an ISA irq.
>>
>> how about this one ?
>
> You can't share an edge triggered ISA irq, it isn't really physically
> possible. So I don't see how this extra complexity will change anything.
>

Dave's system mptble:

MPTABLE: OEM ID: DELL
MPTABLE: Product ID: Dell XPS710
MPTABLE: APIC at: 0xFEE00000
Processor #0 (Bootup-CPU)
Processor #1
Bus #0 is PCI
Bus #1 is PCI
Bus #2 is PCI
Bus #3 is PCI
Bus #4 is PCI
Bus #5 is PCI
Bus #6 is PCI
Bus #7 is PCI
Bus #8 is PCI
Bus #9 is PCI
Bus #10 is ISA
I/O APIC #8 Version 17 at 0xFEC00000.
IOAPIC[0]: apic_id 8, version 17, address 0xfec00000, GSI 0-23
Int: type 0, pol 0, trig 0, bus 0a, IRQ 00, APIC ID 8, APIC INT 00
Int: type 0, pol 0, trig 0, bus 0a, IRQ 01, APIC ID 8, APIC INT 01
Int: type 3, pol 1, trig 1, bus 0a, IRQ 00, APIC ID 8, APIC INT 02
Int: type 0, pol 0, trig 0, bus 0a, IRQ 03, APIC ID 8, APIC INT 03
Int: type 0, pol 0, trig 0, bus 0a, IRQ 04, APIC ID 8, APIC INT 04
Int: type 0, pol 0, trig 0, bus 0a, IRQ 05, APIC ID 8, APIC INT 05
Int: type 0, pol 0, trig 0, bus 0a, IRQ 06, APIC ID 8, APIC INT 06
Int: type 0, pol 0, trig 0, bus 0a, IRQ 07, APIC ID 8, APIC INT 07
Int: type 0, pol 0, trig 0, bus 0a, IRQ 08, APIC ID 8, APIC INT 08
Int: type 0, pol 0, trig 0, bus 0a, IRQ 09, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 0a, IRQ 0a, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 0a, IRQ 0b, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 0a, IRQ 0c, APIC ID 8, APIC INT 0c
Int: type 0, pol 0, trig 0, bus 0a, IRQ 0e, APIC ID 8, APIC INT 0e
Int: type 0, pol 0, trig 0, bus 0a, IRQ 0f, APIC ID 8, APIC INT 0f
Int: type 0, pol 0, trig 0, bus 00, IRQ 28, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 00, IRQ 2c, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 00, IRQ 2d, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 00, IRQ 34, APIC ID 8, APIC INT 0e
Int: type 0, pol 0, trig 0, bus 00, IRQ 38, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 00, IRQ 38, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 00, IRQ 38, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 00, IRQ 34, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 01, IRQ 00, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 01, IRQ 01, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 01, IRQ 02, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 03, IRQ 00, APIC ID 0, APIC INT 10
Int: type 0, pol 0, trig 0, bus 04, IRQ 00, APIC ID 0, APIC INT 10
Int: type 0, pol 0, trig 0, bus 04, IRQ 01, APIC ID 0, APIC INT 11
Int: type 0, pol 0, trig 0, bus 05, IRQ 00, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 05, IRQ 01, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 05, IRQ 03, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 06, IRQ 00, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 07, IRQ 0c, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 07, IRQ 0e, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 07, IRQ 0f, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 07, IRQ 10, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 07, IRQ 11, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 07, IRQ 12, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 07, IRQ 14, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 07, IRQ 15, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 07, IRQ 17, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 07, IRQ 28, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 08, IRQ 00, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 08, IRQ 01, APIC ID 8, APIC INT 09
Int: type 0, pol 0, trig 0, bus 08, IRQ 03, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 09, IRQ 00, APIC ID 8, APIC INT 0b
Int: type 0, pol 0, trig 0, bus 09, IRQ 01, APIC ID 8, APIC INT 0a
Int: type 0, pol 0, trig 0, bus 09, IRQ 02, APIC ID 8, APIC INT 09
Lint: type 3, pol 1, trig 1, bus 0a, IRQ 00, APIC ID ff, APIC LINT 00
Lint: type 1, pol 1, trig 1, bus 0a, IRQ 00, APIC ID ff, APIC LINT 01
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Eric W. Biederman on
Yinghai Lu <yinghai(a)kernel.org> writes:

> On 08/03/2010 02:15 AM, Eric W. Biederman wrote:
>> Yinghai Lu <yinghai(a)kernel.org> writes:
>>
>>
>> You can't share an edge triggered ISA irq, it isn't really physically
>> possible. So I don't see how this extra complexity will change anything.
>>

>
> Dave's system mptble:

Interesting he has ISA irqs on bus #10 on the same apic id and pin as
pci irqs. Blink. I had missed we had that print out.

The immediate issue are these lines:
> Int: type 0, pol 0, trig 0, bus 03, IRQ 00, APIC ID 0, APIC INT 10
> Int: type 0, pol 0, trig 0, bus 04, IRQ 00, APIC ID 0, APIC INT 10
> Int: type 0, pol 0, trig 0, bus 04, IRQ 01, APIC ID 0, APIC INT 11

Which get the apic id wrong, and thus cause us to mishandle them and
get

You are right Dave's mptable does the arguably broken:

> Int: type 0, pol 0, trig 0, bus 00, IRQ 28, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 00, IRQ 34, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 01, IRQ 02, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 05, IRQ 01, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 06, IRQ 00, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 07, IRQ 0c, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 07, IRQ 12, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 07, IRQ 15, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 07, IRQ 28, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 08, IRQ 01, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 09, IRQ 02, APIC ID 8, APIC INT 09
> Int: type 0, pol 0, trig 0, bus 0a, IRQ 09, APIC ID 8, APIC INT 09

Where busses 0,1,5,6,7,8,9 result in level triggered interrupts and
bus 0x0a results in an edge triggered interrupt.

As I read the code. First we will do a generic isa setup, marking the
interrupt ioapic table entry edge triggered. Then we will do a pci
setup for any pin we use as pci, and then we will set pin_programmed
stopping us from updating the pin any more, during setup.

For the common case I think we still do the right thing, even now, for
these broken bios tables. There is likely an uncommon case for which
something like your shared_legacy_irq deserves to be used, especially
at it preserves our well tested historical behavior.

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on
On 08/03/2010 04:08 AM, Eric W. Biederman wrote:
>
> For the common case I think we still do the right thing, even now, for
> these broken bios tables. There is likely an uncommon case for which
> something like your shared_legacy_irq deserves to be used, especially
> at it preserves our well tested historical behavior.

Dave, Irnna, Gary:

can you check this patch on your systems?

Thanks

Yinghai

[PATCH] x86: check if apic/pin is shared with legacy one

fix system that external device that have io apic on apic0/pin(0-15)

also
for the io apic out of order system:
<6>ACPI: IOAPIC (id[0x10] address[0xfecff000] gsi_base[0])
<6>IOAPIC[0]: apic_id 16, version 0, address 0xfecff000, GSI 0-2
<6>ACPI: IOAPIC (id[0x0f] address[0xfec00000] gsi_base[3])
<6>IOAPIC[1]: apic_id 15, version 0, address 0xfec00000, GSI 3-38
<6>ACPI: IOAPIC (id[0x0e] address[0xfec01000] gsi_base[39])
<6>IOAPIC[2]: apic_id 14, version 0, address 0xfec01000, GSI 39-74
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 4 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 5 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 3 global_irq 6 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 4 global_irq 7 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 6 global_irq 9 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 7 global_irq 10 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 8 global_irq 11 low edge)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 12 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 12 global_irq 15 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 13 global_irq 16 dfl dfl)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 17 low edge)
<6>ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 18 dfl dfl)

after this patch will get

apic0, pin0, GSI 0: irq 0+75
apic0, pin1, GSI 1: irq 1+75
apic0, pin2, GSI 2: irq 2
apic1, pin0, GSI 3: irq 3+75
apic1, pin5, GSI 8: irq 8+75
apic1, pin10,GSI 13: irq 13+75
apic1, pin11,GSI 14: irq 14+75

because mp_config_acpi_legacy_irqs will put apic0, pin2, irq2 in mp_irqs...
so pin_2_irq_legacy will report 2.
irq_to_gsi will still report 2. so it is right.
gsi_to_irq will report 2.

for GSI 0, 1, 3, 8, 13, 14: still right as before.

Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>

---
arch/x86/kernel/apic/io_apic.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6/arch/x86/kernel/apic/io_apic.c
@@ -1013,6 +1013,28 @@ static inline int irq_trigger(int idx)
return MPBIOS_trigger(idx);
}

+static int pin_2_irq_leagcy(int apic, int pin)
+{
+ int i;
+
+ for (i = 0; i < mp_irq_entries; i++) {
+ int bus = mp_irqs[i].srcbus;
+
+ if (!test_bit(bus, mp_bus_not_pci))
+ continue;
+
+ if (mp_ioapics[apic].apicid != mp_irqs[i].dstapic)
+ continue;
+
+ if (mp_irqs[i].dstirq != pin)
+ continue;
+
+ return mp_irqs[i].srcbusirq;
+ }
+
+ return -1;
+}
+
static int pin_2_irq(int idx, int apic, int pin)
{
int irq;
@@ -1029,10 +1051,13 @@ static int pin_2_irq(int idx, int apic,
} else {
u32 gsi = mp_gsi_routing[apic].gsi_base + pin;

- if (gsi >= NR_IRQS_LEGACY)
+ if (gsi >= NR_IRQS_LEGACY) {
irq = gsi;
- else
- irq = gsi_top + gsi;
+ } else {
+ irq = pin_2_irq_legacy(apic, pin);
+ if (irq < 0)
+ irq = gsi_top + gsi;
+ }
}

#ifdef CONFIG_X86_32
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on
On 08/03/2010 12:45 PM, Yinghai Lu wrote:
> On 08/03/2010 04:08 AM, Eric W. Biederman wrote:
>>
>> For the common case I think we still do the right thing, even now, for
>> these broken bios tables. There is likely an uncommon case for which
>> something like your shared_legacy_irq deserves to be used, especially
>> at it preserves our well tested historical behavior.
>
> Dave, Irnna, Gary:
>
> can you check this patch on your systems?
>
> Thanks
>
> Yinghai
>
> [PATCH] x86: check if apic/pin is shared with legacy one
>
> fix system that external device that have io apic on apic0/pin(0-15)
>
> also
> for the io apic out of order system:
> <6>ACPI: IOAPIC (id[0x10] address[0xfecff000] gsi_base[0])
> <6>IOAPIC[0]: apic_id 16, version 0, address 0xfecff000, GSI 0-2
> <6>ACPI: IOAPIC (id[0x0f] address[0xfec00000] gsi_base[3])
> <6>IOAPIC[1]: apic_id 15, version 0, address 0xfec00000, GSI 3-38
> <6>ACPI: IOAPIC (id[0x0e] address[0xfec01000] gsi_base[39])
> <6>IOAPIC[2]: apic_id 14, version 0, address 0xfec01000, GSI 39-74
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 4 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 5 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 3 global_irq 6 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 4 global_irq 7 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 6 global_irq 9 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 7 global_irq 10 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 8 global_irq 11 low edge)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 12 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 12 global_irq 15 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 13 global_irq 16 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 17 low edge)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 18 dfl dfl)
>
> after this patch will get
>
> apic0, pin0, GSI 0: irq 0+75
> apic0, pin1, GSI 1: irq 1+75
> apic0, pin2, GSI 2: irq 2
> apic1, pin0, GSI 3: irq 3+75
> apic1, pin5, GSI 8: irq 8+75
> apic1, pin10,GSI 13: irq 13+75
> apic1, pin11,GSI 14: irq 14+75
>
> because mp_config_acpi_legacy_irqs will put apic0, pin2, irq2 in mp_irqs...
> so pin_2_irq_legacy will report 2.
> irq_to_gsi will still report 2. so it is right.
> gsi_to_irq will report 2.
>
> for GSI 0, 1, 3, 8, 13, 14: still right as before.
>
> Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>
>
> ---
> arch/x86/kernel/apic/io_apic.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
> +++ linux-2.6/arch/x86/kernel/apic/io_apic.c
> @@ -1013,6 +1013,28 @@ static inline int irq_trigger(int idx)
> return MPBIOS_trigger(idx);
> }
>
> +static int pin_2_irq_leagcy(int apic, int pin)

should be pin_2_irq_legacy here..., change the function name without compiling...

> +{
> + int i;
> +
> + for (i = 0; i < mp_irq_entries; i++) {
> + int bus = mp_irqs[i].srcbus;
> +
> + if (!test_bit(bus, mp_bus_not_pci))
> + continue;
> +
> + if (mp_ioapics[apic].apicid != mp_irqs[i].dstapic)
> + continue;
> +
> + if (mp_irqs[i].dstirq != pin)
> + continue;
> +
> + return mp_irqs[i].srcbusirq;
> + }
> +
> + return -1;
> +}
> +
> static int pin_2_irq(int idx, int apic, int pin)
> {
> int irq;
> @@ -1029,10 +1051,13 @@ static int pin_2_irq(int idx, int apic,
> } else {
> u32 gsi = mp_gsi_routing[apic].gsi_base + pin;
>
> - if (gsi >= NR_IRQS_LEGACY)
> + if (gsi >= NR_IRQS_LEGACY) {
> irq = gsi;
> - else
> - irq = gsi_top + gsi;
> + } else {
> + irq = pin_2_irq_legacy(apic, pin);
> + if (irq < 0)
> + irq = gsi_top + gsi;
> + }
> }
>
> #ifdef CONFIG_X86_32

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Eric W. Biederman on
Yinghai Lu <yinghai(a)kernel.org> writes:

> On 08/03/2010 04:08 AM, Eric W. Biederman wrote:
>>
>> For the common case I think we still do the right thing, even now, for
>> these broken bios tables. There is likely an uncommon case for which
>> something like your shared_legacy_irq deserves to be used, especially
>> at it preserves our well tested historical behavior.
>
> Dave, Irnna, Gary:
>
> can you check this patch on your systems?



>
> Thanks
>
> Yinghai
>
> [PATCH] x86: check if apic/pin is shared with legacy one
>
> fix system that external device that have io apic on apic0/pin(0-15)

Nacked-by: "Eric W. Biederman" <ebiederm(a)xmission.com>

Your patch addresses what appears to be a theoretical issue, caused by
a BIOS bug. So far you have not presented a credible scenario where
this would affect anything in real life except the user visible irq
number.

Will you please stop, think, and describe what is going on clearly
and how you expect this patch to affect anything, and please stop
selling this patch as the solution to all of the world's ills. You
are being sloppy and wasting everyone's time.

This patch definitely does not solve Dave Airlie's problem. That was
clearly caused by a (ACPI being disabled compared to the test case)
and a bad BIOS provided MP table with an invalid apic_id, and our
attempt to lookup the ioapic and failed.

This patch doesn't have much of a chance of solving the no VGA output
on boot problem. It comes much much to late affect anything there.


> also
> for the io apic out of order system:
> <6>ACPI: IOAPIC (id[0x10] address[0xfecff000] gsi_base[0])
> <6>IOAPIC[0]: apic_id 16, version 0, address 0xfecff000, GSI 0-2
> <6>ACPI: IOAPIC (id[0x0f] address[0xfec00000] gsi_base[3])
> <6>IOAPIC[1]: apic_id 15, version 0, address 0xfec00000, GSI 3-38
> <6>ACPI: IOAPIC (id[0x0e] address[0xfec01000] gsi_base[39])
> <6>IOAPIC[2]: apic_id 14, version 0, address 0xfec01000, GSI 39-74
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 4 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 5 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 3 global_irq 6 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 4 global_irq 7 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 6 global_irq 9 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 7 global_irq 10 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 8 global_irq 11 low edge)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 12 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 12 global_irq 15 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 13 global_irq 16 dfl dfl)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 17 low edge)
> <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 18 dfl dfl)
>
> after this patch will get
>
> apic0, pin0, GSI 0: irq 0+75
> apic0, pin1, GSI 1: irq 1+75
> apic0, pin2, GSI 2: irq 2
> apic1, pin0, GSI 3: irq 3+75
> apic1, pin5, GSI 8: irq 8+75
> apic1, pin10,GSI 13: irq 13+75
> apic1, pin11,GSI 14: irq 14+75

If your description of what this patch will do is correct we very
definitely do not want this patch. For the previously mentioned
acpi tables we want GSI17 -> apic1, pin 14 -> irq 14.

Not that your description is correct, but a buggy description is an
equally valid reason to reject this incarnation of the patch.

> because mp_config_acpi_legacy_irqs will put apic0, pin2, irq2 in mp_irqs...
> so pin_2_irq_legacy will report 2.
> irq_to_gsi will still report 2. so it is right.
> gsi_to_irq will report 2.
>
> for GSI 0, 1, 3, 8, 13, 14: still right as before.

YH you have a point that a certain class of buggy firmware exists,
that reports some ioapic pins as both edge triggered ISA irqs
and level triggered PCI irqs, and assigning different irqs to the
same ioapic pin is confusing and suboptimal.

In practice I don't see that behavior affecting how we program that
pin, especially since I don't expect any bios that exports that setup
to actually use the ISA irq. So we should just have an unused irq
number.

A clean solution would be to scrub the input from the MP table before
we attempt to use of it, instead of scrubbing the data as we use in
code paths like pin_2_irq. Just touching pin_2_irq is certainly an
incomplete solution because you have not resolved if the pins should
be edge or level triggered, and what polarity we should be sampling
them at.

Until I see a plausible scenario where not handling buggy MP tables
exactly as we have done in the past I don't see hacks like you
are proposing making much sense at all.

Eric


> Signed-off-by: Yinghai Lu <yinghai(a)kernel.org>
>
> ---
> arch/x86/kernel/apic/io_apic.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
> +++ linux-2.6/arch/x86/kernel/apic/io_apic.c
> @@ -1013,6 +1013,28 @@ static inline int irq_trigger(int idx)
> return MPBIOS_trigger(idx);
> }
>
> +static int pin_2_irq_leagcy(int apic, int pin)
> +{
> + int i;
> +
> + for (i = 0; i < mp_irq_entries; i++) {
> + int bus = mp_irqs[i].srcbus;
> +
> + if (!test_bit(bus, mp_bus_not_pci))
> + continue;
> +
> + if (mp_ioapics[apic].apicid != mp_irqs[i].dstapic)
> + continue;
> +
> + if (mp_irqs[i].dstirq != pin)
> + continue;
> +
> + return mp_irqs[i].srcbusirq;
> + }
> +
> + return -1;
> +}
> +
> static int pin_2_irq(int idx, int apic, int pin)
> {
> int irq;
> @@ -1029,10 +1051,13 @@ static int pin_2_irq(int idx, int apic,
> } else {
> u32 gsi = mp_gsi_routing[apic].gsi_base + pin;
>
> - if (gsi >= NR_IRQS_LEGACY)
> + if (gsi >= NR_IRQS_LEGACY) {
> irq = gsi;
> - else
> - irq = gsi_top + gsi;
> + } else {
> + irq = pin_2_irq_legacy(apic, pin);
> + if (irq < 0)
> + irq = gsi_top + gsi;
> + }
> }
>
> #ifdef CONFIG_X86_32
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/