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