i386: move apic init in init_IRQs

arch/i386/kernel/apic.c fastcall void smp_error_interrupt(struct · line 1263

Quoted 2 lines · Eric W. Biederman

arch/i386/kernel/apic.c +0 -2

-	if (enable_local_apic < 0)
-		clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);

I think this should stay.

arch/i386/kernel/apic.c fastcall void smp_error_interrupt(struct · line 1268

Quoted 2 lines

arch/i386/kernel/apic.c +2 -0

+	if (enable_local_apic < 0) {
+		printk(KERN_INFO "Apic disabled\n");

Capitalisation. ;-)

Otherwise it seems reasonable -- provided it works for you. ;-)

lapic_disable() already does this. I am just testing the results.

So what should the capitalization be? "APIC disabled\n" ?

Quoted 1 line

So what should the capitalization be? "APIC disabled\n" ?

Obviously. Thanks for your tidy-up!

arch/i386/kernel/apic.c +1 -1

 	calibration_result = calibrate_APIC_clock();

@@ -1057,7 +1063,7 @@ void __init setup_boot_APIC_clock(void)
 	 */
 	setup_APIC_timer(calibration_result);
 
-	local_irq_enable();
+	local_irq_restore(flags);
 }
 

Hi Eric,

I had a couple of observations.

[..]

Quoted 10 lines

#ifdef CONFIG_X86_IO_APIC

arch/i386/kernel/apic.c +1 -0

 	{

@@ -1046,9 +1050,11 @@ static unsigned int calibration_result;
 
 void __init setup_boot_APIC_clock(void)
 {
+	unsigned long flags;
 	apic_printk(APIC_VERBOSE, "Using local APIC timer interrupts.\n");
 	using_apic_timer = 1;
 

arch/i386/kernel/apic.c static unsigned int calibration_result; · line 1059

Quoted 3 lines

arch/i386/kernel/apic.c +1 -0

+	local_irq_save(flags);
 	local_irq_disable();
 

Should the local_irq_disable() call go away onece local_irq_save() got introduced.

Quoted 4 lines

arch/i386/kernel/apic.c +4 -0

+	/*
+	 * Should not be necessary because the MP table should list the boot
+	 * CPU too, but we do it for the sake of robustness anyway.
+	 * Makes no sense to do this check in clustered apic mode, so skip it

arch/i386/kernel/apic.c fastcall void smp_error_interrupt(struct · line 1301

Quoted 4 lines

arch/i386/kernel/apic.c +4 -0

+	 */
+	if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
+		printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
+				boot_cpu_physical_apicid);

I am testing kdump on i386 and I am hitting this message while second kernel is booting. I am doing testing with 2.6.14-rc4-mm1. Logs are pasted below.

Also kdump testing fails almost 50% of the time on my machine with 2.6.14-rc4-mm1. It works fine with 2.6.14-rc4 though.

Second kernel is unable to come up. earlyprintk on serial console showed a kernel BUG in setup_local_APIC(). Details are included in the logs below.

Second kernel boot log.

# SysRq : Trigger a crashdump
I'm in purgatory
Linux version 2.6.14-rc4-mm1-16M ([email protected]) (gcc version 3.4.3 20041212 (Red Hat 3.4.3-9.EL4)) #1 PREEMPT Wed Oct 19 13:55:24 IST 2005 BIOS-provided physical RAM map:
BIOS-e820: 0000000000000100 - 000000000009d000 (usable)
BIOS-e820: 000000000009d000 - 00000000000a0000 (reserved)
BIOS-e820: 0000000000100000 - 000000002fffa480 (usable)
BIOS-e820: 000000002fffa480 - 0000000030000000 (ACPI data)
BIOS-e820: 00000000fec00000 - 0000000100000000 (reserved)
user-defined physical RAM map:
user: 0000000000000000 - 00000000000a0000 (usable)
user: 0000000001000000 - 000000000142d000 (usable)
user: 00000000014cd400 - 0000000004000000 (usable)
0MB HIGHMEM available.
64MB LOWMEM available.
found SMP MP-table at 0009e140
early console enabled
DMI 2.1 present.
ACPI: LAPIC (acpi_id[0x00] lapic_id[0x03] enabled)
Processor #3 6:10 APIC version 17
ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
Processor #0 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
Processor #1 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x03] lapic_id[0x02] enabled)
Processor #2 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: IOAPIC (id[0x0e] address[0xfec00000] gsi_base[0]) IOAPIC[0]: apic_id 14, version 17, address 0xfec00000, GSI 0-15
ACPI: IOAPIC (id[0x0d] address[0xfec01000] gsi_base[16]) IOAPIC[1]: apic_id 13, version 17, address 0xfec01000, GSI 16-31
ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
Enabling APIC mode: Flat. Using 2 I/O APICs
Using ACPI (MADT) for SMP configuration information
Allocating PCI resources starting at 10000000 (gap: 04000000:fc000000) Built 1 zonelists
Initializing CPU#0
Kernel command line: ro root=/dev/sda7 rhgb console=ttyS0,38400 irqpoll init 3 earlyprintk=ttyS0,38400 memmap=exactmap memmap=640K@0K memmap=4276K@16384K memmap=44235K@21301K elfcorehdr=21300K
Misrouted IRQ fixup and polling support enabled
This may significantly impact system performance
weird, boot CPU (#1) not listed by the BIOS.

------------[ cut here ]------------
kernel BUG at ÿÿÿÿ:1479!
invalid operand: 0000 [#1]
PREEMPT
last sysfs file:
Modules linked in:
CPU:    0
EIP:    0060:[<c1012b17>]    Not tainted VLI
EFLAGS: 00010046   (2.6.14-rc4-mm1-16M)

EIP is at setup_local_APIC+0x26/0x18c
eax: 00000000 ebx: 00040011 ecx: 00000c06 edx: 00000000
esi: 00000011 edi: c13a9800 ebp: 01445007 esp: c13b5fbc
ds: 007b es: 007b ss: 0068
Process swapper (pid: 0, threadinfo=c13b4000 task=c133faa0)

Stack: c13a9800 01445007 c101ac30 00000000 01429900 c13c1c49 c12e8a4c 00000001
       00000003 c13b66cf c12e5d5d c13eddc0 c133b9fc 00000078 c13b6342 c13eddc0
       c1000199
Call Trace:
 [<c101ac30>] printk+0x17/0x1b
 [<c13c1c49>] APIC_init+0x5a/0xf6
 [<c13b66cf>] start_kernel+0xb3/0x1cd
 [<c13b6342>] unknown_bootoption+0x0/0x1b6
Code: e4 f7 0f 30 c3 56 53 83 ec 0c 8b 1d 30 d0 ff ff a1 20 d0 ff ff c1 e8 18 0f

b6 f3 83 e0 0f 0f a3 05 e0 03 3f c1 19 c0 85 c0 75 02 <0f> 0b c7 05 e0 d0 ff ff ff ff ff ff 8b 0d c4 03 3f c1 a1 d0 d0
<0>Kernel panic - not syncing: Attempted to kill the idle task!

Thanks
Vivek

Quoted 1 line

Should the local_irq_disable() call go away onece

local_irq_save() got

Quoted 1 line

introduced.

Nope. The irqs need to be disabled. The save just allows this
to be called in a context where irqs start out disabled. It is
just a save.

Quoted 9 lines

Quoted 1 line

Should the local_irq_disable() call go away onece

local_irq_save() got

Quoted 1 line

introduced.

Nope. The irqs need to be disabled. The save just allows this
to be called in a context where irqs start out disabled. It is
just a save.

local_irq_save() also disables interrupts.

Vivek Goyal <[email protected]> writes:

Quoted 4 lines

Quoted 4 lines

(quoted diff) +4 -0

+	/*
+	 * Should not be necessary because the MP table should list the boot
+	 * CPU too, but we do it for the sake of robustness anyway.
+	 * Makes no sense to do this check in clustered apic mode, so skip it

Quoted 8 lines

Review

Quoted 4 lines

(quoted diff) +4 -0

+	 */
+	if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
+		printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
+				boot_cpu_physical_apicid);

I am testing kdump on i386 and I am hitting this message while second kernel is booting. I am doing testing with 2.6.14-rc4-mm1. Logs are pasted below.

The check has been there for a while. All it is saying is that
our boot cpu has apicid #1. So I suspect you are either on
an Opteron system or a hyperthreaded Xeon system.

Quoted 2 lines

Also kdump testing fails almost 50% of the time on my machine with 2.6.14-rc4-mm1. It works fine with 2.6.14-rc4 though.

Is the failure that happens 50% represented by the bootlog below?

The problem bootlog appears to be a glitch in the handling of apicids on the boot cpu that the BIOS does not report to the kernel.

Quoted 4 lines

Second kernel is unable to come up. earlyprintk on serial console showed a kernel BUG in setup_local_APIC(). Details are included in the logs below.

Second kernel boot log.

The BUG is weird. I don't think apic.c even goes to line 1479. Unless the BUG is inline in one of the other functions called by setup_local_APIC() .

	/*
	 * Double-check whether this APIC is really registered.
	 */
	if (!apic_id_registered())
		BUG();

apic_id_registered expands to:
static inline int apic_id_registered(void)

{
	return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

Which indicates to me that the code that, there is something wrong in the logic of:

	if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
		printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
				boot_cpu_physical_apicid);
		physid_set(hard_smp_processor_id(), phys_cpu_present_map);
	}

Currently we are refering to the boot cpus apicid with 3 different expressions one of them appears to be wrong.

Quoted 2 lines · Eric W. Biederman

Vivek Goyal <[email protected]> writes:

[..]

Quoted 13 lines

Quoted 12 lines

Review

Quoted 8 lines

(quoted diff) +8 -0

+	/*
+	 * Should not be necessary because the MP table should list the boot
+	 * CPU too, but we do it for the sake of robustness anyway.
+	 * Makes no sense to do this check in clustered apic mode, so skip it
+	 */
+	if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
+		printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
+				boot_cpu_physical_apicid);

I am testing kdump on i386 and I am hitting this message while second kernel is booting. I am doing testing with 2.6.14-rc4-mm1. Logs are pasted below.

Quoted 4 lines

The check has been there for a while. All it is saying is that
our boot cpu has apicid #1. So I suspect you are either on
an Opteron system or a hyperthreaded Xeon system.

I am using Pentium. No hyperthreading.

Quoted 5 lines

Quoted 2 lines

Also kdump testing fails almost 50% of the time on my machine with 2.6.14-rc4-mm1. It works fine with 2.6.14-rc4 though.

Is the failure that happens 50% represented by the bootlog below?

Yes. But this problem is not happening all the time. Now in 4 trials I got it once again. The message in all the failures remains the same.

Quoted 8 lines

apic_id_registered expands to:
static inline int apic_id_registered(void)
{
return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map); }

Which indicates to me that the code that, there is something wrong in the logic of:

Quoted 9 lines

	if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
		printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
				boot_cpu_physical_apicid);
		physid_set(hard_smp_processor_id(), phys_cpu_present_map);
	}

Currently we are refering to the boot cpus apicid with 3 different expressions one of them appears to be wrong.

Looks like apic_id_registered() is failing. I had put two debug printk() statements and to my surprise hard_smp_processor_id() is returning different value then GET_APIC_ID(apic_read(APIC_ID)).

source code of hard_smp_processor_id() shows that it is also reading APIC_ID register only. Then how can two values be different. (Until and unless somebody modified the value in between two reads).

I am pasting another failure log with my debug messages(prefixed with "Debug:"). My debug patch is also attached with the mail.

Vivek Goyal <[email protected]> writes:

Quoted 20 lines

Quoted 10 lines

apic_id_registered expands to:
static inline int apic_id_registered(void)
{
return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map); }

Which indicates to me that the code that, there is something wrong in the logic of:

Quoted 9 lines

	if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
		printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
				boot_cpu_physical_apicid);
		physid_set(hard_smp_processor_id(), phys_cpu_present_map);
	}

Currently we are refering to the boot cpus apicid with 3 different expressions one of them appears to be wrong.

Quoted 7 lines

Looks like apic_id_registered() is failing. I had put two debug printk() statements and to my surprise hard_smp_processor_id() is returning different value then GET_APIC_ID(apic_read(APIC_ID)).

source code of hard_smp_processor_id() shows that it is also reading APIC_ID register only. Then how can two values be different. (Until and unless somebody modified the value in between two reads).

It appears the buggy expression is hard_smp_processor_id. Quite possibly because it doesn't call apic_read() and instead open codes it.

boot_cpu_physical_apicid also returns apicid #1, before we have a problem.

So either we want to change hard_smp_processor_id to use apic_read() or we can just use boot_cpu_physical_apicid when fixing the apicid present bitmap.

Quoted 2 lines

I am pasting another failure log with my debug messages(prefixed with "Debug:"). My debug patch is also attached with the mail.

See above but I am pretty certain we know enough to get farther. For testing you may want to hard code your first kernel to use the second cpu.

The fact that hard_smp_processor_id gets the wrong value makes me wonder if your kernel will boot all of the way once we get past this problem.

I suspect if you disassemble the code for hard_smp_processor_id we will see the compiler doing the wrong thing.

You are right. hard_smp_processor_id() is hard-coded to zero in case of a non SMP kernel (include/linux/smp.h) and that's why the problem is happening. I am booting a non-SMP capture kernel. In case of kexec on panic, we can very well boot on a cpu whose id is not zero.

I have attached a patch with the mail which is now using boot_cpu_physical_apicid to hard set presence of boot cpu instead of hard_smp_processor_id(). But the interesting questoin remains why BIOS is not reporting the boot cpu.

I have attached a patch with the mail which is now using boot_cpu_physical_apicid to hard set presence of boot cpu instead of hard_smp_processor_id(). But the interesting questoin remains why BIOS is not reporting the boot cpu.

From your earlier boot log.

ACPI: LAPIC (acpi_id[0x00] lapic_id[0x03] enabled) Processor #3 6:10 APIC version 17
ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled) Processor #0 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled) Processor #1 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x03] lapic_id[0x02] enabled) Processor #2 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.

After staring at mpparse.c for a little longer I noticed that when we hit our limit of num_processors we are filtering out information about other processors that we can still store.

This patch just reorders the code so we store everything we can.

This should avoid the incorrect warning about our boot CPU not being listed by the BIOS that we are now getting in the kexec on panic case, and it should allow us to detect all apicid conflicts even when our physical number of cpus exceeds maxcpus.

Signed-off-by Eric W. Biederman <[email protected]>

---

 arch/i386/kernel/mpparse.c |   35 +++++++++++++++++++----------------
 1 files changed, 19 insertions(+), 16 deletions(-)

applies-to: cf16f96fe9347e42dd2fc6b305005a52783195d4 192f11c9442be11c6535b38d371aa3771fd9513e

arch/i386/kernel/mpparse.c +19 -16

index 27aabfc..07555a4 100644
@@ -182,17 +182,6 @@ static void __init MP_processor_info (st
 		boot_cpu_physical_apicid = m->mpc_apicid;
 	}
 
-	if (num_processors >= NR_CPUS) {
-		printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
-			"  Processor ignored.\n", NR_CPUS); 
-		return;
-	}
-
-	if (num_processors >= maxcpus) {
-		printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
-			" Processor ignored.\n", maxcpus); 
-		return;
-	}
 	ver = m->mpc_apicver;
 
 	if (!MP_valid_apicid(apicid, ver)) {

@@ -201,11 +190,6 @@ static void __init MP_processor_info (st
 		return;
 	}
 
-	cpu_set(num_processors, cpu_possible_map);
-	num_processors++;
-	phys_cpu = apicid_to_cpu_present(apicid);
-	physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
-
 	/*
 	 * Validate version
 	 */

@@ -216,6 +200,25 @@ static void __init MP_processor_info (st
 		ver = 0x10;
 	}
 	apic_version[m->mpc_apicid] = ver;
+
+	phys_cpu = apicid_to_cpu_present(apicid);
+	physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
+
+	if (num_processors >= NR_CPUS) {
+		printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
+			"  Processor ignored.\n", NR_CPUS); 
+		return;
+	}
+
+	if (num_processors >= maxcpus) {
+		printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
+			" Processor ignored.\n", maxcpus); 
+		return;
+	}
+
+	cpu_set(num_processors, cpu_possible_map);
+	num_processors++;
+
 	if ((num_processors > 8) &&
 	    APIC_XAPIC(ver) &&
 	    (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL))