Skip to contentSkip to conversation

Handle disabled local apic better

PATCH In review Andi Kleen 3 participants 7 messages Mar 23, 2004 – Mar 24, 2004 5 messages not in the archive ACPI

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 Handle disabled local apic better

When a kernel is compiled with local and io apic code, but the local APIC is disabled before ACPI starts ACPI gets confused. The situation can occur in a mainline kernel too, e.g. when a dmi_scan entry disables the local apic early.

The problem is that the ACPI code will do all the setup assuming there is a IO-APIC because the IO-APIC is not disabled, but the machine really runs in PIC mode. This usually leads to lost network (devices get an unreachable interrupt) or IDE hangs or other problems.

This patch adds checks for local apic enabled to all paths that need it (basically everybody who has a #ifdef CONFIG_X86_LOCAL_APIC needs this check instead)

It just handles the case there the up apic is disabled before ACPI boot, it can actually be disabled later too (e.g. when the APIC initialisation fails). This is probably still broken.

Also removes an bogus error message - this path can trigger just with software disabled APIC.

-Andi

arch/i386/kernel/acpi/boot.c renamed from arch/i386/kernel/acpi/boot.c-o +7 -0
diff -u linux/arch/i386/kernel/acpi/boot.c-o linux/arch/i386/kernel/acpi/boot.c
@@ -685,6 +685,13 @@
 #ifdef CONFIG_X86_LOCAL_APIC
 	int count, error;
 
+	/* it's still wrong when the apic is disabled later */
+	extern int enable_local_apic;
+	if (enable_local_apic < 0) { 
+		printk(KERN_INFO "ACPI: local apic disabled\n");
+		return;
+	}
+
 	count = acpi_table_parse(ACPI_APIC, acpi_parse_madt);
 	if (count == 1) {
 
arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +5 -2
diff -u linux/arch/i386/kernel/apic.c-o linux/arch/i386/kernel/apic.c
@@ -41,6 +41,8 @@
 
 #include "io_ports.h"
 
+extern int enable_local_apic;
+
 static void apic_pm_activate(void);
 
 void __init apic_intr_init(void)
@@ -190,6 +192,9 @@
 {
 	unsigned long value;
 
+	if (enable_local_apic < 0) 
+		return;
+
 	clear_local_APIC();
 
 	/*
@@ -1172,8 +1193,6 @@
 	 * Complain if the BIOS pretends there is one.
 	 */
 	if (!cpu_has_apic && APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])) {
-		printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
-			boot_cpu_physical_apicid);
 		return -1;
 	}
 
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c-o +6 -1
diff -u linux/arch/i386/kernel/setup.c-o linux/arch/i386/kernel/setup.c
@@ -50,6 +50,10 @@
 #include "setup_arch_pre.h"
 #include "mach_resources.h"
 
+#ifdef CONFIG_X86_LOCAL_APIC
+extern int enable_local_apic;
+#endif
+
 /* This value is set up by the early boot code to point to the value
    immediately after the boot time page tables.  It contains a *physical*
    address, and must not be in the .bss segment! */
@@ -917,6 +925,7 @@
 	acpi_reserve_bootmem();
 #endif
 #ifdef CONFIG_X86_FIND_SMP_CONFIG
+	if (enable_local_apic >= 0) 
 	/*
 	 * Find and reserve possible boot-time SMP configuration:
 	 */
@@ -1232,7 +1241,7 @@
 	acpi_boot_init();
 
 #ifdef CONFIG_X86_LOCAL_APIC
-	if (smp_found_config)
+	if (smp_found_config && enable_local_apic >= 0)
 		get_smp_config();
 #endif
 
arch/x86_64/kernel/apic.c renamed from arch/x86_64/kernel/apic.c-o +3 -0
diff -u linux/arch/x86_64/kernel/apic.c-o linux/arch/x86_64/kernel/apic.c
@@ -970,6 +970,7 @@
 }
 
 int disable_apic; 
+int enable_local_apic = 1;
 
 /*
  * This initializes the IO-APIC and APIC hardware if this is
@@ -1009,12 +1010,14 @@
 
 static __init int setup_disableapic(char *str) 
 { 
+	enable_local_apic = -1;
 	disable_apic = 1;
 	return 0;
 } 
 
 static __init int setup_nolapic(char *str) 
 { 
+	enable_local_apic = -1;
 	disable_apic = 1;
 	return 0;
 } 

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 5 lines · Andi Kleen

When a kernel is compiled with local and io apic code, but the local APIC is disabled before ACPI starts ACPI gets confused. The situation can occur in a mainline kernel too, e.g. when a dmi_scan entry disables the local apic early.

Quoted 8 lines

The problem is that the ACPI code will do all the setup assuming there is a IO-APIC because the IO-APIC is not disabled, but the machine really runs in PIC mode. This usually leads to lost network (devices get an unreachable interrupt) or IDE hangs or other problems.

This patch adds checks for local apic enabled to all paths that need it (basically everybody who has a #ifdef CONFIG_X86_LOCAL_APIC needs this check instead)

oof, and there are lots of CONFIG_X86_LOCAL_APIC uses apparently un-guarded at run-time...

Quoted 16 lines

It just handles the case there the up apic is disabled before ACPI boot, it can actually be disabled later too (e.g. when the APIC initialisation fails). This is probably still broken.

Also removes an bogus error message - this path can trigger just with software disabled APIC.

-Andi

diff -u linux/arch/i386/kernel/acpi/boot.c-o linux/arch/i386/kernel/acpi/boot.c

arch/i386/kernel/acpi/boot.c renamed from arch/i386/kernel/acpi/boot.c-o +0 -0
@@ -685,6 +685,13 @@
 #ifdef CONFIG_X86_LOCAL_APIC
 	int count, error;

arch/i386/kernel/acpi/boot.c line 696

Quoted 10 lines
arch/i386/kernel/acpi/boot.c renamed from arch/i386/kernel/acpi/boot.c-o +7 -0
 
+	/* it's still wrong when the apic is disabled later */
+	extern int enable_local_apic;
+	if (enable_local_apic < 0) { 
+		printk(KERN_INFO "ACPI: local apic disabled\n");
+		return;
+	}
+
 	count = acpi_table_parse(ACPI_APIC, acpi_parse_madt);
 	if (count == 1) {

Looks like i386 lacks the parse_cmdline_early() check for "nolapic" to make this test effective.

(why the heck is the parse_args() so late, anyway?)

Looks like x86_64 has it, but then does it again at __setup() time -- which should probably be removed; along with the redundant variable "disable_apic".

arch/i386/kernel/apic.c line 45

Quoted 10 lines

diff -u linux/arch/i386/kernel/apic.c-o linux/arch/i386/kernel/apic.c

arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +2 -0
@@ -41,6 +41,8 @@
 
 #include "io_ports.h"
 
+extern int enable_local_apic;
+

how about declare in asm/apic.h instead?

Quoted 26 lines
arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +3 -2
 static void apic_pm_activate(void);
 
 void __init apic_intr_init(void)
@@ -190,6 +192,9 @@
 {
 	unsigned long value;
 
+	if (enable_local_apic < 0) 
+		return;
+
 	clear_local_APIC();
 
 	/*
@@ -1172,8 +1193,6 @@
 	 * Complain if the BIOS pretends there is one.
 	 */
 	if (!cpu_has_apic && APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])) {
-		printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
-			boot_cpu_physical_apicid);
 		return -1;
 	}
 

diff -u linux/arch/i386/kernel/setup.c-o linux/arch/i386/kernel/setup.c

arch/i386/kernel/setup.c line 56

Quoted 7 lines
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c-o +4 -0
@@ -50,6 +50,10 @@
 #include "setup_arch_pre.h"
 #include "mach_resources.h"
 
+#ifdef CONFIG_X86_LOCAL_APIC
+extern int enable_local_apic;
+#endif
+

asm/apic.h

arch/i386/kernel/setup.c line 931

Quoted 11 lines
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c-o +1 -0
 /* This value is set up by the early boot code to point to the value
    immediately after the boot time page tables.  It contains a *physical*
    address, and must not be in the .bss segment! */
@@ -917,6 +925,7 @@
 	acpi_reserve_bootmem();
 #endif
 #ifdef CONFIG_X86_FIND_SMP_CONFIG
+	if (enable_local_apic >= 0) 
 	/*
 	 * Find and reserve possible boot-time SMP configuration:
 	 */

Here we're disabling MPS with "nolapic" -- something that "maxcpus=0" and "nosmp" never effectively did before...

arch/i386/kernel/setup.c line 1245

Quoted 7 lines
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c-o +1 -1
@@ -1232,7 +1241,7 @@
 	acpi_boot_init();
 
 #ifdef CONFIG_X86_LOCAL_APIC
-	if (smp_found_config)
+	if (smp_found_config && enable_local_apic >= 0)
 		get_smp_config();

i guess this is for the case where DMI disables the lapic, because "nolapic" would have prevented smp_found_config from getting set in the first place. maybe the DMI routine should simply clear smp_found_config?

arch/x86_64/kernel/apic.c line 974

Quoted 11 lines
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c-o +0 -0
 #endif
 

diff -u linux/arch/x86_64/kernel/apic.c-o linux/arch/x86_64/kernel/apic.c

arch/x86_64/kernel/apic.c renamed from arch/x86_64/kernel/apic.c-o +1 -0
@@ -970,6 +970,7 @@
 }
 
 int disable_apic; 
+int enable_local_apic = 1;
 

asm/apic.h;-)

Quoted 7 lines
arch/x86_64/kernel/apic.c renamed from arch/x86_64/kernel/apic.c-o +1 -0
 /*
  * This initializes the IO-APIC and APIC hardware if this is
@@ -1009,12 +1010,14 @@
 
 static __init int setup_disableapic(char *str) 
 { 
+	enable_local_apic = -1;

arch/x86_64/kernel/apic.c line 1023

Quoted 10 lines
arch/x86_64/kernel/apic.c renamed from arch/x86_64/kernel/apic.c-o +1 -0
 	disable_apic = 1;
 	return 0;
 } 
 
 static __init int setup_nolapic(char *str) 
 { 
+	enable_local_apic = -1;
 	disable_apic = 1;
 	return 0;
 } 

these routines should be deleted, since they duplicate the earlier parse_cmdline_early(). the ambiguous "disable_lapic" should go also, since "enable_local_apic" is on the scene, yes?

thanks,
-Len

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#4 lore raw
Quoted 11 lines · On 24 Mar 2004 02:12:54 -0500 Len Brown <len.brown-ral2JQCrhuEAvxtiuMwx3w@publi…
Quoted 10 lines · Andi Kleen

When a kernel is compiled with local and io apic code, but the local APIC is disabled before ACPI starts ACPI gets confused. The situation can occur in a mainline kernel too, e.g. when a dmi_scan entry disables the local apic early.

The problem is that the ACPI code will do all the setup assuming there is a IO-APIC because the IO-APIC is not disabled, but the machine really runs in PIC mode. This usually leads to lost network (devices get an unreachable interrupt) or IDE hangs or other problems.

Quoted 6 lines
Quoted 3 lines

This patch adds checks for local apic enabled to all paths that need it (basically everybody who has a #ifdef CONFIG_X86_LOCAL_APIC needs this check instead)

oof, and there are lots of CONFIG_X86_LOCAL_APIC uses apparently un-guarded at run-time...

I thought I catched most of them?

Quoted 2 lines

Looks like i386 lacks the parse_cmdline_early() check for "nolapic" to make this test effective.

Yes. But dmi scan is done that early.

For command line and APIC failures later it probably needs more work, agreed.

BTW if that is all cleaned up I would suggest just to get rid of the CONFIGs and always use runtime defaults. This would make these parts more maintainable I think and is the right thing to do for distributions.

Quoted 5 lines

(why the heck is the parse_args() so late, anyway?)

Looks like x86_64 has it, but then does it again at __setup() time -- which should probably be removed; along with the redundant variable "disable_apic".

The x86-64 disable_apic was there first. I just had to add the new variable because of the new incestuous sharing of acpi/boot.c to avoid link errors.

Quoted 6 lines
Quoted 6 lines

diff -u linux/arch/i386/kernel/apic.c-o linux/arch/i386/kernel/apic.c

arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +0 -0
@@ -41,6 +41,8 @@
 
Quoted 6 lines

arch/i386/kernel/apic.c line 45

Quoted 4 lines
arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +2 -0
 #include "io_ports.h"
 
+extern int enable_local_apic;
+

how about declare in asm/apic.h instead?

Sure. I was just lazy.

Quoted 6 lines
Quoted 6 lines
arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +0 -0
 /* This value is set up by the early boot code to point to the value
    immediately after the boot time page tables.  It contains a *physical*
    address, and must not be in the .bss segment! */
@@ -917,6 +925,7 @@
 	acpi_reserve_bootmem();
 #endif
Quoted 8 lines

arch/i386/kernel/apic.c line 931

Quoted 5 lines
arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +1 -0
 #ifdef CONFIG_X86_FIND_SMP_CONFIG
+	if (enable_local_apic >= 0) 
 	/*
 	 * Find and reserve possible boot-time SMP configuration:
 	 */

Here we're disabling MPS with "nolapic" -- something that "maxcpus=0" and "nosmp" never effectively did before...

MPS without local APIC is useless.

Quoted 12 lines
Quoted 12 lines

/*
* This initializes the IO-APIC and APIC hardware if this is

arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +1 -0
@@ -1009,12 +1010,14 @@
 
 static __init int setup_disableapic(char *str) 
 { 
+	enable_local_apic = -1;
 	disable_apic = 1;
 	return 0;
 } 
 
Quoted 10 lines

arch/i386/kernel/apic.c line 1023

Quoted 6 lines
arch/i386/kernel/apic.c renamed from arch/i386/kernel/apic.c-o +1 -0
 static __init int setup_nolapic(char *str) 
 { 
+	enable_local_apic = -1;
 	disable_apic = 1;
 	return 0;
 } 

these routines should be deleted, since they duplicate the earlier parse_cmdline_early(). the ambiguous "disable_lapic" should go also, since "enable_local_apic" is on the scene, yes?

No, even when you parse in parse_cmdline_early() you need a __setup, otherwise the normal __setup parser will complain about unknown options.

In 2.7 i hope this all will get up cleared up with a __early_setup()

-Andi

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 4 lines

BTW if that is all cleaned up I would suggest just to get rid of the CONFIGs and always use runtime defaults. This would make these parts more maintainable I think and is the right thing to do for distributions.

tempting.
I gues if nobody builds with !CONFIG_X86_LOCAL_APIC anyway, then run-time check is the way to go.

Quoted 4 lines
Quoted 2 lines

Here we're disabling MPS with "nolapic" -- something that "maxcpus=0" and "nosmp" never effectively did before...

MPS without local APIC is useless.

I agree, my point is that "nosmp" and "maxcpus=0" are broken in that they didn't disable MPS this early.

Quoted 22 lines
Quoted 11 lines
Quoted 11 lines

/*
* This initializes the IO-APIC and APIC hardware if this is

arch/x86_64/kernel/apic.c renamed from arch/x86_64/kernel/apic.c-o +1 -0
@@ -1009,12 +1010,14 @@
 
 static __init int setup_disableapic(char *str) 
 { 
+	enable_local_apic = -1;
 	disable_apic = 1;
 	return 0;
 } 
 
Quoted 10 lines

arch/x86_64/kernel/apic.c line 1023

Quoted 6 lines
arch/x86_64/kernel/apic.c renamed from arch/x86_64/kernel/apic.c-o +1 -0
 static __init int setup_nolapic(char *str) 
 { 
+	enable_local_apic = -1;
 	disable_apic = 1;
 	return 0;
 } 

these routines should be deleted, since they duplicate the earlier parse_cmdline_early(). the ambiguous "disable_lapic" should go also, since "enable_local_apic" is on the scene, yes?

Quoted 3 lines

No, even when you parse in parse_cmdline_early() you need a __setup, otherwise the normal __setup parser will complain about unknown options.

Huh? So if I boot a kernel with an unknown parameter, say "len=gone_fishing", where would I expect to see the complaint -- I don't see it in my dmesg except where the entire commmand line is dumped out.

thanks,
-Len

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#8 lore raw
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Here we're disabling MPS with "nolapic" -- something that "maxcpus=0" and "nosmp" never effectively did before...

MPS without local APIC is useless.

I agree, my point is that "nosmp" and "maxcpus=0" are broken in that they didn't disable MPS this early.

Sure, it's broken. Also when the APIC initialization fails and probably some other cases.

Quoted 7 lines
Quoted 3 lines

No, even when you parse in parse_cmdline_early() you need a __setup, otherwise the normal __setup parser will complain about unknown options.

Huh? So if I boot a kernel with an unknown parameter, say "len=gone_fishing", where would I expect to see the complaint -- I don't see it in my dmesg except where the entire commmand line is dumped out.

That will result in a environment variable len in init's environment with value gone_fishing and is not unknown. But nolapic doesn't have a =

-Andi

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw
Quoted 10 lines · Andi Kleen
Quoted 7 lines
Quoted 3 lines

No, even when you parse in parse_cmdline_early() you need a __setup, otherwise the normal __setup parser will complain about unknown options.

Huh? So if I boot a kernel with an unknown parameter, say "len=gone_fishing", where would I expect to see the complaint -- I don't see it in my dmesg except where the entire commmand line is dumped out.

That will result in a environment variable len in init's environment with value gone_fishing and is not unknown. But nolapic doesn't have a =

ah, so parse_args() extracts cmdline params with '=' and what it doesn't recognize make it to init's environment. looks like i recently added some potential environment pollution to init's environment then...

thanks,
-Len

-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

#11 lore raw
In reply to a message not in the archive

Thus wrote Len Brown:

Quoted 2 lines

Looks like i386 lacks the parse_cmdline_early() check for "nolapic" to make this test effective.

Since we're at it, it seems to ignore "lapic" too (or at least it is supposed to force-enable LAPIC disabled by DMI blacklist; it did at some point, but currently doesn't). I didn't have time to identify te cause, so just a quick remark for now.
Best regards,

Signature · 9 lines
Karol 'sziwan' Kozimor
sziwan-DETuoxkZsSqrDJvtcaxF/[email protected]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click