RE: [(repost) git Patch 1/1] avoid IRQ0 ioapic pin collision

[Brown, Len]

>> >I have tested this algorithm and it worked just fine for 
>> >me... I used 
>> >the following compression code in mp_register_gsi():
>> >
>> >
>> >int     irqs_used = 0;
>> >int     gsi_to_irq[NR_IRQS] = { [0 ... NR_IRQS-1] = -1 };
>> >...
>> >
>> >        if (triggering == ACPI_LEVEL_SENSITIVE) {
>> >                if (gsi > NR_IRQS) {
>> >                        int i;
>> >                        printk("NBP: looking for unused IRQ\n");
>> >                        for (i = nr_ioapic_registers[0]; i 
>< NR_IRQS;
>> i++) {
>> >                                if (gsi_to_irq[i] == -1) {
>> >                                        gsi_to_irq[i] = gsi;
>> >                                        gsi = i;
>> >                                        break;
>> >                                }
>> >                        }
>> >                        if (i >= NR_IRQS) {
>> >                                printk(KERN_ERR "GSI %u is 
>> too high\n",
>> ...
>> >                                return gsi;
>> >                        }
>> >                } else
>> >                        gsi_to_irq[gsi] = gsi;
>> >        }
>> 
>> the problem with this code as it stands is that 
>> acpi/mp_register_gsi() can be called with gsi in any order.  
>> So it is possible for the compression code above to select 
>> gsi_to_irq[n] and later for the non-compression path to 
>> over-write gsi_to_irq[n].
>> 
>
>It should always find the entry it's done the first one around 
>actually, the first thing in mp_register_gsi() will check for it I
think.

No, the top of mp_register_gsi() is based on the real GSI
(before re-numbering) and the real IOAPIC pin number.
It prevents re-programming the real IOAPIC pin (a dubious optimization,
IMHO).
Yes, if it finds a 2nd call results in the same pin, it returns
gsi_to_irq[i],
but that isn't the failure case here.

a failure case is like so:
say the 1st IOAPIC has 24 pins, so the 0th RTE of the 2nd APIC is #24,
and this happens:

mp_register_gsi(NR_IRQS+1);
	gsi_to_irq[24] is free, so it will get claimed
	by the IRQ that wants to talk to GSI NR_IRQS+1

mp_register_gsi(24);
	gsi_to_irq[24] was not -1 here, it was NR_IRQS+1,
	but that will get over-written to be 24.

So now you have multiple independent interrupt sources that think
they own IRQ 24.

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