[PATCH][2.5-AC] Forced enable/disable local APIC

[PATCH][2.5-AC] Forced enable/disable local APIC

[Zwane Mwaikambo]

The DMI stuff should be fine too.

Index: linux-2.5.45-ac1/arch/i386/kernel/apic.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.45-ac1/arch/i386/kernel/apic.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 apic.c
--- linux-2.5.45-ac1/arch/i386/kernel/apic.c	5 Nov 2002 04:47:02 -0000	1.1.1.1
+++ linux-2.5.45-ac1/arch/i386/kernel/apic.c	6 Nov 2002 00:19:38 -0000
@@ -609,11 +609,27 @@
 
 #endif	/* CONFIG_PM */
 
+int enable_local_apic_flag __initdata = 0; /* 0=probe, 1=force, 2=disable e.g. DMI */
+
+static int __init nolapic_setup(char *str)
+{
+	enable_local_apic_flag = 2;
+	return 1;
+}
+
+static int __init lapic_setup(char *str)
+{
+	enable_local_apic_flag = 1;
+	return 1;
+}
+
+__setup("nolapic", nolapic_setup);
+__setup("lapic", lapic_setup);
+
 /*
  * Detect and enable local APICs on non-SMP boards.
  * Original code written by Keir Fraser.
  */
-int dont_enable_local_apic __initdata = 0;
 
 static int __init detect_init_APIC (void)
 {
@@ -621,11 +637,14 @@
 	extern void get_cpu_vendor(struct cpuinfo_x86*);
 
 	/* Disabled by DMI scan or kernel option? */
-	if (dont_enable_local_apic)
+	if (enable_local_apic_flag == 2)
 		return -1;
 
 	/* Workaround for us being called before identify_cpu(). */
 	get_cpu_vendor(&boot_cpu_data);
+	
+	if (enable_local_apic_flag == 1)
+		goto force_apic;
 
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_AMD:
@@ -642,6 +661,7 @@
 		goto no_apic;
 	}
 
+force_apic:
 	if (!cpu_has_apic) {
 		/*
 		 * Some BIOSes disable the local APIC in the
Index: linux-2.5.45-ac1/arch/i386/kernel/dmi_scan.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.45-ac1/arch/i386/kernel/dmi_scan.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 dmi_scan.c
--- linux-2.5.45-ac1/arch/i386/kernel/dmi_scan.c	5 Nov 2002 04:47:02 -0000	1.1.1.1
+++ linux-2.5.45-ac1/arch/i386/kernel/dmi_scan.c	6 Nov 2002 00:22:02 -0000
@@ -314,9 +314,9 @@
 static int __init local_apic_kills_bios(struct dmi_blacklist *d)
 {
 #ifdef CONFIG_X86_LOCAL_APIC
-	extern int dont_enable_local_apic;
-	if (!dont_enable_local_apic) {
-		dont_enable_local_apic = 1;
+	extern int enable_local_apic_flag;
+	if (!enable_local_apic_flag) {
+		enable_local_apic_flag = 2;
 		printk(KERN_WARNING "%s with broken BIOS detected. "
 		       "Refusing to enable the local APIC.\n",
 		       d->ident);
@@ -333,9 +333,9 @@
 static int __init apm_kills_local_apic(struct dmi_blacklist *d)
 {
 #ifdef CONFIG_X86_LOCAL_APIC
-	extern int dont_enable_local_apic;
-	if (apm_info.bios.version && !dont_enable_local_apic) {
-		dont_enable_local_apic = 1;
+	extern int enable_local_apic_flag;
+	if (apm_info.bios.version && !enable_local_apic_flag) {
+		enable_local_apic_flag = 2;
 		printk(KERN_WARNING "%s with broken BIOS detected. "
 		       "Refusing to enable the local APIC.\n",
 		       d->ident);

-- 
function.linuxpower.ca

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Maciej W. Rozycki]

On Tue, 5 Nov 2002, Zwane Mwaikambo wrote:

> The DMI stuff should be fine too.

 The patch looks fine for me.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Mikael Pettersson]

Zwane Mwaikambo writes:
 > +int enable_local_apic_flag __initdata = 0; /* 0=probe, 1=force, 2=disable e.g. DMI */
...
 > +	if (enable_local_apic_flag == 1)
 > +		goto force_apic;
 >  
 >  	switch (boot_cpu_data.x86_vendor) {
 >  	case X86_VENDOR_AMD:
 > @@ -642,6 +661,7 @@
 >  		goto no_apic;
 >  	}
 >  
 > +force_apic:
 >  	if (!cpu_has_apic) {
 >  		/*
 >  		 * Some BIOSes disable the local APIC in the

Of what use is the force case? If someone boots with "lapic" on a CPU
where the APIC feature bit is off, then the code will rdmsr/wrmsr on
APICBASE, even though we (the kernel) haven't verified that the CPU
actually has that MSR. This is doubleplusungood.

I have no problem with the disable option.

/Mikael

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Maciej W. Rozycki]

On Thu, 7 Nov 2002, Mikael Pettersson wrote:

> Zwane Mwaikambo writes:
>  > +int enable_local_apic_flag __initdata = 0; /* 0=probe, 1=force, 2=disable e.g. DMI */
> ...
>  > +	if (enable_local_apic_flag == 1)
>  > +		goto force_apic;
>  >  
>  >  	switch (boot_cpu_data.x86_vendor) {
>  >  	case X86_VENDOR_AMD:
>  > @@ -642,6 +661,7 @@
>  >  		goto no_apic;
>  >  	}
>  >  
>  > +force_apic:
>  >  	if (!cpu_has_apic) {
>  >  		/*
>  >  		 * Some BIOSes disable the local APIC in the
> 
> Of what use is the force case? If someone boots with "lapic" on a CPU
> where the APIC feature bit is off, then the code will rdmsr/wrmsr on
> APICBASE, even though we (the kernel) haven't verified that the CPU
> actually has that MSR. This is doubleplusungood.

 The assumption is a user knows better what he wants to achieve.  But I'm
not sure we really need it here.  A DMI override with the APICBASE
availability test in place should suffice -- I'd remove the "goto". 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Zwane Mwaikambo]

On Thu, 7 Nov 2002, Mikael Pettersson wrote:

> Zwane Mwaikambo writes:
>  > +int enable_local_apic_flag __initdata = 0; /* 0=probe, 1=force, 2=disable e.g. DMI */
> ...
>  > +	if (enable_local_apic_flag == 1)
>  > +		goto force_apic;
>  >  
>  >  	switch (boot_cpu_data.x86_vendor) {
>  >  	case X86_VENDOR_AMD:
>  > @@ -642,6 +661,7 @@
>  >  		goto no_apic;
>  >  	}
>  >  
>  > +force_apic:
>  >  	if (!cpu_has_apic) {
>  >  		/*
>  >  		 * Some BIOSes disable the local APIC in the
> 
> Of what use is the force case? If someone boots with "lapic" on a CPU
> where the APIC feature bit is off, then the code will rdmsr/wrmsr on
> APICBASE, even though we (the kernel) haven't verified that the CPU
> actually has that MSR. This is doubleplusungood.

We still honour the APIC feature bit, its just that we bypass the cpuid 
checks. Looks sane no?

	Zwane
-- 
function.linuxpower.ca

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Mikael Pettersson]

Zwane Mwaikambo writes:
 > On Thu, 7 Nov 2002, Mikael Pettersson wrote:
 > 
 > > Zwane Mwaikambo writes:
 > >  > +int enable_local_apic_flag __initdata = 0; /* 0=probe, 1=force, 2=disable e.g. DMI */
 > > ...
 > >  > +	if (enable_local_apic_flag == 1)
 > >  > +		goto force_apic;
 > >  >  
 > >  >  	switch (boot_cpu_data.x86_vendor) {
 > >  >  	case X86_VENDOR_AMD:
 > >  > @@ -642,6 +661,7 @@
 > >  >  		goto no_apic;
 > >  >  	}
 > >  >  
 > >  > +force_apic:
 > >  >  	if (!cpu_has_apic) {
 > >  >  		/*
 > >  >  		 * Some BIOSes disable the local APIC in the
 > > 
 > > Of what use is the force case? If someone boots with "lapic" on a CPU
 > > where the APIC feature bit is off, then the code will rdmsr/wrmsr on
 > > APICBASE, even though we (the kernel) haven't verified that the CPU
 > > actually has that MSR. This is doubleplusungood.
 > 
 > We still honour the APIC feature bit, its just that we bypass the cpuid 
 > checks. Looks sane no?

No. Read what I wrote: if cpu_has_apic is false, the code drops into
the "try the hard way by messing with the APICBASE MSR". Your "force"
goto bypasses the CPU checks, which are there to ensure that the CPU
actually _has_ an APICBASE MSR.

I still see no reason at all for the force.

/Mikael

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Zwane Mwaikambo]

On Thu, 7 Nov 2002, Mikael Pettersson wrote:

> No. Read what I wrote: if cpu_has_apic is false, the code drops into
> the "try the hard way by messing with the APICBASE MSR". Your "force"
> goto bypasses the CPU checks, which are there to ensure that the CPU
> actually _has_ an APICBASE MSR.

My mistake, i misread.
 
> I still see no reason at all for the force.

I agree, in which case the first patch should make everyone happy. If Alan 
doesn't take it for his next release i'll resend.

	Zwane
-- 
function.linuxpower.ca

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Maciej W. Rozycki]

On Thu, 7 Nov 2002, Zwane Mwaikambo wrote:

> > No. Read what I wrote: if cpu_has_apic is false, the code drops into
> > the "try the hard way by messing with the APICBASE MSR". Your "force"
> > goto bypasses the CPU checks, which are there to ensure that the CPU
> > actually _has_ an APICBASE MSR.
> 
> My mistake, i misread.
>  
> > I still see no reason at all for the force.
> 
> I agree, in which case the first patch should make everyone happy. If Alan 
> doesn't take it for his next release i'll resend.

 Well, the "lapic" option should override the DMI setting, not the
APICBASE availability check.  Anyway, I don't insist on this that much --
while I think consistency is good, none of the "*apic" options are
actually a necessity for me.  If one needs the option, one may still cook
an appropriate patch oneself. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Zwane Mwaikambo]

On Fri, 8 Nov 2002, Maciej W. Rozycki wrote:

>  Well, the "lapic" option should override the DMI setting, not the
> APICBASE availability check.  Anyway, I don't insist on this that much --
> while I think consistency is good, none of the "*apic" options are
> actually a necessity for me.  If one needs the option, one may still cook
> an appropriate patch oneself. 

I think the nolapic is definitely needed because i've come across a number 
of bug reports where the simplest solution would be to just disable the 
local apic.

	Zwane
-- 
function.linuxpower.ca

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Mikael Pettersson]

Zwane Mwaikambo writes:
 > On Fri, 8 Nov 2002, Maciej W. Rozycki wrote:
 > 
 > >  Well, the "lapic" option should override the DMI setting, not the
 > > APICBASE availability check.  Anyway, I don't insist on this that much --
 > > while I think consistency is good, none of the "*apic" options are
 > > actually a necessity for me.  If one needs the option, one may still cook
 > > an appropriate patch oneself. 
 > 
 > I think the nolapic is definitely needed because i've come across a number 
 > of bug reports where the simplest solution would be to just disable the 
 > local apic.

People with broken boxes should send their DMI data to me so I can add
their boxes to the local APIC blacklist in dmi_scan.c. "nolapic" is
simply a workaround for the absence of this DMI data.

Notice how silent the Inspiron 8k users are now that the DMI black
list is implemented...

/Mikael

Re: [PATCH][2.5-AC] Forced enable/disable local APIC

[Brian J. Murrell]

On Fri, 08 Nov 2002 14:22:59 +0100, Mikael Pettersson wrote:
> 
> People with broken boxes should send their DMI data to me so I can add
> their boxes to the local APIC blacklist in dmi_scan.c.

That was the approach I wanted to use in the first place to deal with the
broken local apic in vmware 2.0.4.  See the thread (which is unfortunately
broken -- the followup must not have included a References: header back to
the my original message):

http://www.ussg.iu.edu/hypermail/linux/kernel/0305.3/0907.html

The thread can be picked up again in this message: 

http://www.ussg.iu.edu/hypermail/linux/kernel/0305.3/0995.html

> "nolapic" is
> simply a workaround for the absence of this DMI data.

Unfortunately, VMware 2.0.4 does not have any DMI data to identify it by. 
the search for _DMI_ between 0xF0000 and 0xFFFFF finds nothing.  I dunno
if this is valid for proving or disproving the presence of DMI data, but:

# dmidecode
# dmidecode 1.8
BIOS32 Service Directory present.
        Calling Interface Address: 0x000FD8C0
PNP 1.0 present.
        Event Notification: Not Supported
        Real Mode Code Address: F000:AEA5
        Real Mode Data Address: 0040:0000
        Protected Mode Code Address: 0x000FAEC3
        Protected Mode Data Address: 0x00000400
PCI Interrupt Routing 1.0 present.
        Table Size: 0 bytes
        Router ID: ff:1f.7
        Exclusive IRQs: None

> Notice how silent the Inspiron 8k users are now that the DMI black
> list is implemented...

:-)  I like the approach, it just doesn't always apply, thus the need for
kernel commandline args.

b.