[PATCH] x86_64 write apic id fix

[PATCH] x86_64 write apic id fix

[Vivek Goyal]

o Apic id is in most significant 8 bits of APIC_ID register. Current code
  is trying to write apic id to least significant 8 bits. This patch fixes
  it.

o This fix enables booting uni kdump capture kernel on a cpu with non-zero
  apic id.

Signed-off-by: Vivek Goyal <vgoyal@in.ibm.com>
---


diff -puN arch/x86_64/kernel/apic.c~kdump-up-kernel-non-boot-cpu-fix arch/x86_64/kernel/apic.c
--- linux-2.6.15-rc5-mm3-16M/arch/x86_64/kernel/apic.c~kdump-up-kernel-non-boot-cpu-fix	2005-12-27 10:38:09.000000000 -0800
+++ linux-2.6.15-rc5-mm3-16M-root/arch/x86_64/kernel/apic.c	2005-12-27 10:39:31.000000000 -0800
@@ -1060,7 +1060,7 @@ int __init APIC_init_uniprocessor (void)
 	connect_bsp_APIC();
 
 	phys_cpu_present_map = physid_mask_of_physid(boot_cpu_id);
-	apic_write_around(APIC_ID, boot_cpu_id);
+	apic_write_around(APIC_ID, SET_APIC_ID(boot_cpu_id));
 
 	setup_local_APIC();
 
diff -puN include/asm-x86_64/apicdef.h~kdump-up-kernel-non-boot-cpu-fix include/asm-x86_64/apicdef.h
--- linux-2.6.15-rc5-mm3-16M/include/asm-x86_64/apicdef.h~kdump-up-kernel-non-boot-cpu-fix	2005-12-27 10:38:25.000000000 -0800
+++ linux-2.6.15-rc5-mm3-16M-root/include/asm-x86_64/apicdef.h	2005-12-27 10:39:02.000000000 -0800
@@ -13,6 +13,7 @@
 #define		APIC_ID		0x20
 #define			APIC_ID_MASK		(0xFFu<<24)
 #define			GET_APIC_ID(x)		(((x)>>24)&0xFFu)
+#define			SET_APIC_ID(x)		(((x)<<24))
 #define		APIC_LVR	0x30
 #define			APIC_LVR_MASK		0xFF00FF
 #define			GET_APIC_VERSION(x)	((x)&0xFFu)
_

Re: [PATCH] x86_64 write apic id fix

[Andi Kleen]

On Thu, Dec 29, 2005 at 01:57:09PM +0530, Vivek Goyal wrote:
> 
> o Apic id is in most significant 8 bits of APIC_ID register. Current code
>   is trying to write apic id to least significant 8 bits. This patch fixes
>   it.
> 
> o This fix enables booting uni kdump capture kernel on a cpu with non-zero
>   apic id.

Thanks merged.
-Andi

Re: [PATCH] x86_64 write apic id fix

[Mika Penttilä]

Vivek Goyal wrote:

>o Apic id is in most significant 8 bits of APIC_ID register. Current code
>  is trying to write apic id to least significant 8 bits. This patch fixes
>  it.
>
>o This fix enables booting uni kdump capture kernel on a cpu with non-zero
>  apic id.
>
>Signed-off-by: Vivek Goyal <vgoyal@in.ibm.com>
>---
>
>  
>
What difference does it make in the first place? In 
APIC_init_uniprocessor() you write back the apic id read before from 
processor, not from mp table (because you wouldn't be in 
APIC_init_uniprocessor() otherwise).

--Mika

Re: [PATCH] x86_64 write apic id fix

[Vivek Goyal]

On Thu, Dec 29, 2005 at 05:57:11PM +0200, Mika Penttilä wrote:
> Vivek Goyal wrote:
> 
> >o Apic id is in most significant 8 bits of APIC_ID register. Current code
> > is trying to write apic id to least significant 8 bits. This patch fixes
> > it.
> >
> >o This fix enables booting uni kdump capture kernel on a cpu with non-zero
> > apic id.
> >
> >Signed-off-by: Vivek Goyal <vgoyal@in.ibm.com>
> >---
> >
> > 
> >
> What difference does it make in the first place? In 
> APIC_init_uniprocessor() you write back the apic id read before from 
> processor, not from mp table (because you wouldn't be in 
> APIC_init_uniprocessor() otherwise).
> 

In this case APIC_init_uniprocessor() is being called from smp_init() as
I have compiled this as a uni kernel. And smp_found_config is 1. So this
can not be guaranteed that boot_cpu_id has been read from APIC.

But at the same time I see two functions (detect_init_APIC() and
init_apic_mappings()) which overwrite the boot_cpu_id unconditionally. Former
sets it to zero by default and later always reads it from APIC. This 
basically means that we just don't trust the MP tables in all the cases.

For kdump case, this becomes further tricky, as we boot into a second
kernel without going through the BIOS and we might be booting on a cpu
with non-zero apic id. But ACPI and MP tables will continue to report
boot cpu id as 0 and that's not correct. 

To come back to your question, I have just fixed one problem and that is
if you are writing boot_cpu_id to APIC_ID register, write it in a proper
manner. Otherwise what happens in this case is that boot_cpu_id is 1 and
if I use apic_write_around(apic, boot_cpu_id) then it writes 0 to APIC_ID
register instead of 1 and definitely that's not the requirement.

Thanks
Vivek