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

[Protasevich, Natalie]

> <6>IOAPIC[64]: apic_id 127, version 32, address 0xfec49000, GSI
> 1728-1751
> 
> wow, big box!

Yea, it's decent :) I asked for the big one since this way we can test
IRQs "wrapping  around".

> 
> >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.

> Also, I would prefer that this code be in 
> ioapic_renumber_irq(), as I think it is unnecessarily complex 
> to re-number, and then re-number again.
> (gsi_irq_sharing() is a separate discussion)

Sure - for i386, but Len, we would have to bring it into x86_64 and make
this thing above (or something like this) to be a default handler, yes. 

> 
> I think this should be model-specific, but if you feel that 
> handling (gsi > NR_IRQS) here is important in the generic 
> case, then I'm fine with this being a default 
> ioapic_renumber_irq() handler that a platform can augment/override.
>

OK.. besides note that only pretty large system will wrap around to get
unused IRQs, the rest of the world is getting pretty much default
behavior, pre-compression time so to speak :)
 
> Re: returning error
> At one point acpi_register_gsi() was able to return an error.
> However, that was broken when acpi_gsi_to_irq() was created, 
> and then broken worse when that routine was modified with 
> gsi_irq_sharing().  if you BUG() on i > NR_IRQS, that would 
> be consistent wth gsi_irq_sharing().

Ok, sounds good. I will put something together according to the above
and hopefully test it soon.

Thanks,
--Natalie
 
-
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/