| Message ID | 1402302011-23642-13-git-send-email-jiang.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Mon, 9 Jun 2014, Jiang Liu wrote: > unsigned int arch_dynirq_lower_bound(unsigned int from) > { > - return from < nr_irqs_gsi ? nr_irqs_gsi : from; > + unsigned int min = gsi_top + NR_IRQS_LEGACY; Why is this gsi_top + NR_IRQ_LEGACY? The legacy interrupts are part of the gsi space, aren't they? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Thomas, This piece of code is inherited from current IOAPIC driver and I think it's a workaround for some weird platforms. For normal platforms with both 8259A and IOAPIC controllers, legacy ISA IRQs should be connected to both 8259A and IOAPIC pins (ignore timer and cascade IRQs for simplicity). According to comments in current kernel, there are some platforms on which: 1) some ISA IRQs are only connected to 8259A controllers. 2) the corresponding IOAPIC pins are connected to some non-ISA IRQs. For such platforms, IRQ0-15 are used for ISA IRQs and another 16 IRQs just above gsi_top are reserved for IOAPIC pins 0-15 which are connected to non-ISA IRQs. I have no real experience with such a platform, but just guessing possible cases according to kernel comments and "Multiple Processor Specification". Please look at these two pictures for quick reference. http://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=31#manual http://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=63#manual Thanks! Gerry On 2014/6/10 7:22, Thomas Gleixner wrote: > On Mon, 9 Jun 2014, Jiang Liu wrote: >> unsigned int arch_dynirq_lower_bound(unsigned int from) >> { >> - return from < nr_irqs_gsi ? nr_irqs_gsi : from; >> + unsigned int min = gsi_top + NR_IRQS_LEGACY; > > Why is this gsi_top + NR_IRQ_LEGACY? The legacy interrupts are part of > the gsi space, aren't they? > > Thanks, > > tglx > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 10 Jun 2014, Jiang Liu wrote: > Hi Thomas, > This piece of code is inherited from current IOAPIC driver > and I think it's a workaround for some weird platforms. > For normal platforms with both 8259A and IOAPIC controllers, > legacy ISA IRQs should be connected to both 8259A and IOAPIC pins > (ignore timer and cascade IRQs for simplicity). According to comments > in current kernel, there are some platforms on which: > 1) some ISA IRQs are only connected to 8259A controllers. > 2) the corresponding IOAPIC pins are connected to some non-ISA IRQs. > For such platforms, IRQ0-15 are used for ISA IRQs and another > 16 IRQs just above gsi_top are reserved for IOAPIC pins 0-15 which > are connected to non-ISA IRQs. > I have no real experience with such a platform, but just > guessing possible cases according to kernel comments and "Multiple > Processor Specification". Please look at these two pictures for quick > reference. > http://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=31#manual > http://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=63#manual Duh. I completely forgot about the 82489 mess. We probably want a comment somewhere why we have this gsi + legacy thing. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Thomas,
With all patches applied, we have following code and comments
for this case:
switch (type) {
case IOAPIC_DOMAIN_LEGACY:
/*
* Dynamically allocate IRQ number for non-ISA IRQs in
the first 16
* GSIs on some weird platforms.
*/
if (gsi < nr_legacy_irqs())
irq = irq_create_mapping(domain, pin);
else if (irq_create_strict_mappings(domain, gsi, pin, 1)
== 0)
irq = gsi;
break;
On 2014/6/12 18:58, Thomas Gleixner wrote:
> On Tue, 10 Jun 2014, Jiang Liu wrote:
>
>> Hi Thomas,
>> This piece of code is inherited from current IOAPIC driver
>> and I think it's a workaround for some weird platforms.
>> For normal platforms with both 8259A and IOAPIC controllers,
>> legacy ISA IRQs should be connected to both 8259A and IOAPIC pins
>> (ignore timer and cascade IRQs for simplicity). According to comments
>> in current kernel, there are some platforms on which:
>> 1) some ISA IRQs are only connected to 8259A controllers.
>> 2) the corresponding IOAPIC pins are connected to some non-ISA IRQs.
>> For such platforms, IRQ0-15 are used for ISA IRQs and another
>> 16 IRQs just above gsi_top are reserved for IOAPIC pins 0-15 which
>> are connected to non-ISA IRQs.
>> I have no real experience with such a platform, but just
>> guessing possible cases according to kernel comments and "Multiple
>> Processor Specification". Please look at these two pictures for quick
>> reference.
>> http://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=31#manual
>> http://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=63#manual
>
> Duh. I completely forgot about the 82489 mess.
>
> We probably want a comment somewhere why we have this gsi + legacy
> thing.
>
> Thanks,
>
> tglx
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 94a56c233e87..e8cd0bf0ee82 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -118,9 +118,6 @@ struct mpc_intsrc mp_irqs[MAX_IRQ_SOURCES]; /* # of MP IRQ source entries */ int mp_irq_entries; -/* GSI interrupts */ -static int nr_irqs_gsi = NR_IRQS_LEGACY; - #ifdef CONFIG_EISA int mp_bus_id_to_type[MAX_MP_BUSSES]; #endif @@ -3322,20 +3319,11 @@ static int __init io_apic_get_redir_entries(int ioapic) return reg_01.bits.entries + 1; } -static void __init probe_nr_irqs_gsi(void) -{ - int nr; - - nr = gsi_top + NR_IRQS_LEGACY; - if (nr > nr_irqs_gsi) - nr_irqs_gsi = nr; - - printk(KERN_DEBUG "nr_irqs_gsi: %d\n", nr_irqs_gsi); -} - unsigned int arch_dynirq_lower_bound(unsigned int from) { - return from < nr_irqs_gsi ? nr_irqs_gsi : from; + unsigned int min = gsi_top + NR_IRQS_LEGACY; + + return from < min ? min : from; } int __init arch_probe_nr_irqs(void) @@ -3345,12 +3333,12 @@ int __init arch_probe_nr_irqs(void) if (nr_irqs > (NR_VECTORS * nr_cpu_ids)) nr_irqs = NR_VECTORS * nr_cpu_ids; - nr = nr_irqs_gsi + 8 * nr_cpu_ids; + nr = (gsi_top + NR_IRQS_LEGACY) + 8 * nr_cpu_ids; #if defined(CONFIG_PCI_MSI) || defined(CONFIG_HT_IRQ) /* * for MSI and HT dyn irq */ - nr += nr_irqs_gsi * 16; + nr += (gsi_top + NR_IRQS_LEGACY) * 16; #endif if (nr < nr_irqs) nr_irqs = nr; @@ -3623,8 +3611,6 @@ fake_ioapic_page: ioapic_res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1; ioapic_res++; } - - probe_nr_irqs_gsi(); } void __init ioapic_insert_resources(void)
Static variable nr_irqs_gsi is used to maintain the lowest dynamic allocatable IRQ number. It may cause trouble when enabling dynamic IRQ allocation for IOAPIC, so use arch_dynirq_lower_bound() to avoid directly accessing nr_irqs_gsi and kill nr_irqs_gsi. Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> --- arch/x86/kernel/apic/io_apic.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-)