Re: [patch] x86: 2.6.31-rc7 crash due to buggy flat_phys_pkg_id

[Ravikiran G Thirumalai <kiran@scalex86.org>]

On Mon, Aug 24, 2009 at 04:53:45PM -0700, Yinghai Lu wrote:
>Ravikiran G Thirumalai wrote:
>> 
>> Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
>> Cc: Yinghai Lu <yinghai@kernel.org>
>> 
>> Index: linux-2.6.31-rc6/arch/x86/kernel/apic/apic_flat_64.c
>> ===================================================================
>> --- linux-2.6.31-rc6.orig/arch/x86/kernel/apic/apic_flat_64.c	2009-08-21 12:42:16.000000000 -0700
>> +++ linux-2.6.31-rc6/arch/x86/kernel/apic/apic_flat_64.c	2009-08-21 14:12:21.654837472 -0700
>> @@ -161,7 +161,8 @@ static int flat_apic_id_registered(void)
>>  
>>  static int flat_phys_pkg_id(int initial_apic_id, int index_msb)
>>  {
>> -	return initial_apic_id >> index_msb;
>> +	return cpu_has_apic ? hard_smp_processor_id() >> index_msb :
>> +	    initial_apic_id >> index_msb;
>>  }
>
>it seems we should use initial apic id here.
>

Why?  The specs seem to indicate otherwise unless I am mistaken --
Intel  systems programming guide, Vol 3A Part1, chapter 7 section
7.5.5 - Identifying Logical Processors in a MP system:
<quote>
After the BIOS has completed the MP initialization protocol, each logical
processor can be uniquely identified by its local APIC ID. Software can
access these APIC IDs in either of the following ways
</quote>
phys_pkg_id() indicates that the logical package id is being looked up,
so local apic id should be used here no?
What am I missing?


>can you check which calling for vsmp cause the problem?
>arch/x86/kernel/cpu/addon_cpuid_features.c:     c->cpu_core_id = apic->phys_pkg_id(c->initial_apicid, ht_mask_width)
>arch/x86/kernel/cpu/addon_cpuid_features.c:     c->phys_proc_id = apic->phys_pkg_id(c->initial_apicid, core_plus_mask_width
>);
>arch/x86/kernel/cpu/addon_cpuid_features.c:     c->apicid = apic->phys_pkg_id(c->initial_apicid, 0);

The above are definitely problematic.  Anyplace that uses phys_pkg_id to get
the unique pkg id of a logical processor will have problems.