Skip to contentSkip to conversation

x86/mrst: correct pin to irq mapping

PATCH In review Jacob Pan 2 participants 5 messages Apr 8, 2011 – Apr 12, 2011 x86

lore
Share & tools
lore

Jacob Pan

author
#0 lore raw

PATCH v2 1/1 x86/mrst: correct pin to irq mapping

Moorestown platform does not have ISA bus nor legacy IRQs. It reuses the range of legacy IRQs for regular device interrupts. The routing information of early system device IRQs (timers) are obtained from firmware provided SFI tables. We reuse/fake MP configuration table to facilitate IRQ setup with IOAPIC.

Maintaining a 1:1 mapping of IOAPIC pin (RTE entry) and IRQ# makes routing information clean and easy to understand on Moorestown. Though optional.

This patch allows SFI timer and vRTC IRQ to be treated as ISA IRQ so that pin2irq mapping will be 1:1.

Also fixed MP table type and use macros to clearly set MP IRQ entries. As a result, apbt timer and RTC interrupts on Moorestown are within legacy IRQ range.

sh-3.2# cat /proc/interrupts
CPU0 CPU1

  0:      11249          0   IO-APIC-edge      apbt0
  1:          0      12271   IO-APIC-edge      apbt1
  8:        887          0   IO-APIC-fasteoi   dw_spi
 13:          0          0   IO-APIC-fasteoi   INTEL_MID_DMAC2
 14:          0          0   IO-APIC-fasteoi   rtc0

Refer to discussion in:
https://lkml.org/lkml/2010/6/10/70

---
 arch/x86/platform/mrst/mrst.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
arch/x86/platform/mrst/mrst.c +5 -5
index 5c0207b..275dbc1 100644
@@ -97,11 +97,11 @@ static int __init sfi_parse_mtmr(struct sfi_table_header *table)
 			pentry->freq_hz, pentry->irq);
 			if (!pentry->irq)
 				continue;
-			mp_irq.type = MP_IOAPIC;
+			mp_irq.type = MP_INTSRC;
 			mp_irq.irqtype = mp_INT;
 /* triggering mode edge bit 2-3, active high polarity bit 0-1 */
 			mp_irq.irqflag = 5;
-			mp_irq.srcbus = 0;
+			mp_irq.srcbus = MP_BUS_ISA;
 			mp_irq.srcbusirq = pentry->irq;	/* IRQ */
 			mp_irq.dstapic = MP_APIC_ALL;
 			mp_irq.dstirq = pentry->irq;
@@ -168,10 +168,10 @@ int __init sfi_parse_mrtc(struct sfi_table_header *table)
 	for (totallen = 0; totallen < sfi_mrtc_num; totallen++, pentry++) {
 		pr_debug("RTC[%d]: paddr = 0x%08x, irq = %d\n",
 			totallen, (u32)pentry->phys_addr, pentry->irq);
-		mp_irq.type = MP_IOAPIC;
+		mp_irq.type = MP_INTSRC;
 		mp_irq.irqtype = mp_INT;
 		mp_irq.irqflag = 0xf;	/* level trigger and active low */
-		mp_irq.srcbus = 0;
+		mp_irq.srcbus = MP_BUS_ISA;
 		mp_irq.srcbusirq = pentry->irq;	/* IRQ */
 		mp_irq.dstapic = MP_APIC_ALL;
 		mp_irq.dstirq = pentry->irq;
@@ -282,7 +282,7 @@ void __init x86_mrst_early_setup(void)
 	/* Avoid searching for BIOS MP tables */
 	x86_init.mpparse.find_smp_config = x86_init_noop;
 	x86_init.mpparse.get_smp_config = x86_init_uint_noop;
-
+	set_bit(MP_BUS_ISA, mp_bus_not_pci);
 }
 
 /*
Signature · 1 line
1.7.1
#1 lore raw
Quoted 10 lines · Jacob Pan

As a result, apbt timer and RTC interrupts on Moorestown are within legacy IRQ range.

sh-3.2# cat /proc/interrupts
CPU0 CPU1

  0:      11249          0   IO-APIC-edge      apbt0
  1:          0      12271   IO-APIC-edge      apbt1
  8:        887          0   IO-APIC-fasteoi   dw_spi
 13:          0          0   IO-APIC-fasteoi   INTEL_MID_DMAC2
 14:          0          0   IO-APIC-fasteoi   rtc0

Is there a 'before the patch' /proc/interrupts output as well? Supposedly it's different and seeing that different output is *way* more descriptive to the casual commit log reader than a textual explanation only.

Also, i guess this would be .40 material, right?

Thanks,

Ingo

author
#2 lore raw
Quoted 14 lines · On Sun, 10 Apr 2011 08:01:54 +0200 Ingo Molnar <[email protected]> wrote:
Quoted 10 lines · Jacob Pan

As a result, apbt timer and RTC interrupts on Moorestown are within legacy IRQ range.

sh-3.2# cat /proc/interrupts
CPU0 CPU1

  0:      11249          0   IO-APIC-edge      apbt0
  1:          0      12271   IO-APIC-edge      apbt1
  8:        887          0   IO-APIC-fasteoi   dw_spi
 13:          0          0   IO-APIC-fasteoi   INTEL_MID_DMAC2
 14:          0          0   IO-APIC-fasteoi   rtc0
Quoted 5 lines

Is there a 'before the patch' /proc/interrupts output as well? Supposedly it's different and seeing that different output is *way* more descriptive to the casual commit log reader than a textual explanation only.

If v1 patch is used, /proc/interrupts look like below on Moorestown. Notice that apbt1 interrupts is moved to gsi_top + 1 = 39.

bash-3.2# cat /proc/interrupts
CPU0 CPU1

  0:     793303          0   IO-APIC-edge      apbt0
  8:      84124          0   IO-APIC-fasteoi   dw_spi
 13:          0          0   IO-APIC-fasteoi   INTEL_MID_DMAC2
 15:    1851630          0   IO-APIC-fasteoi   mrstouch
 27:         83          0   IO-APIC-fasteoi   mmc0l_scu_ipc
 28:          0          0   IO-APIC-fasteoi   mmc1
 30:          0          0   IO-APIC-fasteoi   INTEL_MID_DMAC1
 33:         23          0   IO-APIC-fasteoi   ehci_hcd:usb1
 34:         52          0   IO-APIC-fasteoi   mmc2
 39:          0     812273   IO-APIC-edge      apbt1
Quoted 1 line

Also, i guess this would be .40 material, right?

Without the patch, kernel will crash during boot on Moorestown since the secondary CPU clockevent (apbt1) will fail to request irq#1, which does not have ioapic chip in its irq_desc[] entry. So, I would think this is an urgent bug fix for 39.

Signature · 4 lines
Thanks

Jacob
(from Linux laptop)
#3 lore raw
Quoted 4 lines · jacob pan

Without the patch, kernel will crash during boot on Moorestown since the secondary CPU clockevent (apbt1) will fail to request irq#1, which does not have ioapic chip in its irq_desc[] entry. So, I would think this is an urgent bug fix for 39.

Ok, just for future reference, this information is like 10x more important than *any* of the other information you presented in various changelog versions of this patch! I do not have this hardware so i had no idea how severe the problem was in practice - whether it's cosmetic, performance related or stability related.

I'll push your urgent fix to Linus via tip:x86/urgent.

Thanks,

Ingo

#4 lore raw

[tip:x86/urgent] x86/mrst: Fix boot crash caused by incorrect pin to irq mapping

In reply to Jacob Pan
Commit-ID:  9d90e49da57fe73a2f35334fdd2fb60dbf3933ed
Gitweb:     http://git.kernel.org/tip/9d90e49da57fe73a2f35334fdd2fb60dbf3933ed
Author:     Jacob Pan <[email protected]>
AuthorDate: Fri, 8 Apr 2011 11:23:00 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 12 Apr 2011 08:38:52 +0200

x86/mrst: Fix boot crash caused by incorrect pin to irq mapping

Moorestown systems crash on boot because the secondary CPU clockevent (apbt1) will fail to request irq#1, which does not have ioapic chip in its irq_desc[] entry.

Background:

Moorestown platform does not have ISA bus nor legacy IRQs. It reuses the range of legacy IRQs for regular device interrupts. The routing information of early system device IRQs (timers) are obtained from firmware provided SFI tables. We reuse/fake MP configuration table to facilitate IRQ setup with IOAPIC.

Maintaining a 1:1 mapping of IOAPIC pin (RTE entry) and IRQ# makes routing information clean and easy to understand on Moorestown. Though optional.

This patch allows SFI timer and vRTC IRQ to be treated as ISA IRQ so that pin2irq mapping will be 1:1.

Also fixed MP table type and use macros to clearly set MP IRQ entries. As a result, apbt timer and RTC interrupts on Moorestown are within legacy IRQ range:

 # cat /proc/interrupts
            CPU0       CPU1
   0:      11249          0   IO-APIC-edge      apbt0
   1:          0      12271   IO-APIC-edge      apbt1
   8:        887          0   IO-APIC-fasteoi   dw_spi
  13:          0          0   IO-APIC-fasteoi   INTEL_MID_DMAC2
  14:          0          0   IO-APIC-fasteoi   rtc0

Further discussion of this patch can be found at:

https://lkml.org/lkml/2010/6/10/70

---
 arch/x86/platform/mrst/mrst.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
arch/x86/platform/mrst/mrst.c +5 -5
index 5c0207b..275dbc1 100644
@@ -97,11 +97,11 @@ static int __init sfi_parse_mtmr(struct sfi_table_header *table)
 			pentry->freq_hz, pentry->irq);
 			if (!pentry->irq)
 				continue;
-			mp_irq.type = MP_IOAPIC;
+			mp_irq.type = MP_INTSRC;
 			mp_irq.irqtype = mp_INT;
 /* triggering mode edge bit 2-3, active high polarity bit 0-1 */
 			mp_irq.irqflag = 5;
-			mp_irq.srcbus = 0;
+			mp_irq.srcbus = MP_BUS_ISA;
 			mp_irq.srcbusirq = pentry->irq;	/* IRQ */
 			mp_irq.dstapic = MP_APIC_ALL;
 			mp_irq.dstirq = pentry->irq;
@@ -168,10 +168,10 @@ int __init sfi_parse_mrtc(struct sfi_table_header *table)
 	for (totallen = 0; totallen < sfi_mrtc_num; totallen++, pentry++) {
 		pr_debug("RTC[%d]: paddr = 0x%08x, irq = %d\n",
 			totallen, (u32)pentry->phys_addr, pentry->irq);
-		mp_irq.type = MP_IOAPIC;
+		mp_irq.type = MP_INTSRC;
 		mp_irq.irqtype = mp_INT;
 		mp_irq.irqflag = 0xf;	/* level trigger and active low */
-		mp_irq.srcbus = 0;
+		mp_irq.srcbus = MP_BUS_ISA;
 		mp_irq.srcbusirq = pentry->irq;	/* IRQ */
 		mp_irq.dstapic = MP_APIC_ALL;
 		mp_irq.dstirq = pentry->irq;
@@ -282,7 +282,7 @@ void __init x86_mrst_early_setup(void)
 	/* Avoid searching for BIOS MP tables */
 	x86_init.mpparse.find_smp_config = x86_init_noop;
 	x86_init.mpparse.get_smp_config = x86_init_uint_noop;
-
+	set_bit(MP_BUS_ISA, mp_bus_not_pci);
 }
 
 /*