[PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type (Re: [PATCH 03/10] ARM: hotplug cpu: Keep processor information, startup code & __lookup_processor_type)

[PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type (Re: [PATCH 03/10] ARM: hotplug cpu: Keep processor information, startup code & __lookup_processor_type)

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [101004 10:18]:
> When hotplug CPU is enabled, we need to keep the list of supported CPUs,
> their setup functions, and __lookup_processor_type in place so that we
> can find and initialize secondary CPUs.  Move these into the __CPUINIT
> section.

> +	__CPUINIT
> +__lookup_processor_type:
> +	adr	r3, __lookup_processor_type_data
> +	ldmia	r3, {r4 - r6}
> +	sub	r3, r3, r4			@ get offset between virt&phys
> +	add	r5, r5, r3			@ convert virt addresses to
> +	add	r6, r6, r3			@ physical address space
> +1:	ldmia	r5, {r3, r4}			@ value, mask
> +	and	r4, r4, r9			@ mask wanted bits
> +	teq	r3, r4
> +	beq	2f
> +	add	r5, r5, #PROC_INFO_SZ		@ sizeof(proc_info_list)
> +	cmp	r5, r6
> +	blo	1b
> +	mov	r5, #0				@ unknown processor
> +2:	mov	pc, lr
> +ENDPROC(__lookup_processor_type)

The ldmia r5 above can cause a data abort depending on the compiler
pixies. This can happen if r5 address is unaligned. Here's a fix
for that.

Regards,

Tony


From: Tony Lindgren <tony@atomide.com>
Date: Thu, 21 Oct 2010 19:00:41 -0700
Subject: [PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type

Commit 5085f3ff458521045f7e43da62b8c30ea7df2e82 added better support for
CONFIG_HOTPLUG_CPU by keeping proc_info around. However, depending on
the Kconfig options selected, this can make the booting fail mysteriously
early on.

Turns out a data abort can happen in __lookup_processor in ldmia r5 {r3, r4}.
When it happens the address loaded to r5 is not aligned. Fix the problem by
aligning proc_info.

Reported-by: Anand Gadiyar <gadiyar@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 1953e3d..a58b91d 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -114,6 +114,7 @@ SECTIONS
 			*(.glue_7)
 			*(.glue_7t)
 		*(.got)			/* Global offset table		*/
+			. = ALIGN(4);
 			ARM_CPU_KEEP(PROC_INFO)
 	}
 

Re: [PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type (Re: [PATCH 03/10] ARM: hotplug cpu: Keep processor information, startup code & __lookup_processor_type)

[Anand Gadiyar]

On 10/22/2010 2:51 PM, Tony Lindgren wrote:
> * Russell King - ARM Linux<linux@arm.linux.org.uk>  [101004 10:18]:
>> When hotplug CPU is enabled, we need to keep the list of supported CPUs,
>> their setup functions, and __lookup_processor_type in place so that we
>> can find and initialize secondary CPUs.  Move these into the __CPUINIT
>> section.
>
>> +	__CPUINIT
>> +__lookup_processor_type:
>> +	adr	r3, __lookup_processor_type_data
>> +	ldmia	r3, {r4 - r6}
>> +	sub	r3, r3, r4			@ get offset between virt&phys
>> +	add	r5, r5, r3			@ convert virt addresses to
>> +	add	r6, r6, r3			@ physical address space
>> +1:	ldmia	r5, {r3, r4}			@ value, mask
>> +	and	r4, r4, r9			@ mask wanted bits
>> +	teq	r3, r4
>> +	beq	2f
>> +	add	r5, r5, #PROC_INFO_SZ		@ sizeof(proc_info_list)
>> +	cmp	r5, r6
>> +	blo	1b
>> +	mov	r5, #0				@ unknown processor
>> +2:	mov	pc, lr
>> +ENDPROC(__lookup_processor_type)
>
> The ldmia r5 above can cause a data abort depending on the compiler
> pixies. This can happen if r5 address is unaligned. Here's a fix
> for that.
>
> Regards,
>
> Tony
>
>
> From: Tony Lindgren<tony@atomide.com>
> Date: Thu, 21 Oct 2010 19:00:41 -0700
> Subject: [PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type
>
> Commit 5085f3ff458521045f7e43da62b8c30ea7df2e82 added better support for
> CONFIG_HOTPLUG_CPU by keeping proc_info around. However, depending on
> the Kconfig options selected, this can make the booting fail mysteriously
> early on.
>
> Turns out a data abort can happen in __lookup_processor in ldmia r5 {r3, r4}.
> When it happens the address loaded to r5 is not aligned. Fix the problem by
> aligning proc_info.
>
> Reported-by: Anand Gadiyar<gadiyar@ti.com>
> Signed-off-by: Tony Lindgren<tony@atomide.com>

Tested-by: Anand Gadiyar <gadiyar@ti.com>

This patch lets linux-next as of 20101021 boot up on my OMAP4 boards.


>
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index 1953e3d..a58b91d 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -114,6 +114,7 @@ SECTIONS
>   			*(.glue_7)
>   			*(.glue_7t)
>   		*(.got)			/* Global offset table		*/
> +			. = ALIGN(4);
>   			ARM_CPU_KEEP(PROC_INFO)
>   	}
>

Re: [PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type (Re: [PATCH 03/10] ARM: hotplug cpu: Keep processor information, startup code & __lookup_processor_type)

[Russell King - ARM Linux]

On Fri, Oct 22, 2010 at 11:51:08AM -0700, Tony Lindgren wrote:
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index 1953e3d..a58b91d 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -114,6 +114,7 @@ SECTIONS
>  			*(.glue_7)
>  			*(.glue_7t)
>  		*(.got)			/* Global offset table		*/
> +			. = ALIGN(4);
>  			ARM_CPU_KEEP(PROC_INFO)

The got table should also be word aligned - but the kernel doesn't use
one.  In any case, it would be a good idea to place it before the .got.

Re: [PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type (Re: [PATCH 03/10] ARM: hotplug cpu: Keep processor information, startup code & __lookup_processor_type)

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [101023 01:21]:
> On Fri, Oct 22, 2010 at 11:51:08AM -0700, Tony Lindgren wrote:
> > diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> > index 1953e3d..a58b91d 100644
> > --- a/arch/arm/kernel/vmlinux.lds.S
> > +++ b/arch/arm/kernel/vmlinux.lds.S
> > @@ -114,6 +114,7 @@ SECTIONS
> >  			*(.glue_7)
> >  			*(.glue_7t)
> >  		*(.got)			/* Global offset table		*/
> > +			. = ALIGN(4);
> >  			ARM_CPU_KEEP(PROC_INFO)
> 
> The got table should also be word aligned - but the kernel doesn't use
> one.  In any case, it would be a good idea to place it before the .got.

OK, updated patch below.

By moving ARM_CPU_KEEP(PROC_INFO) around I've confirmed that it's
happening at rodata.*. Do you want also another align before .glue_7
as well?

Regards,

Tony

From: Tony Lindgren <tony@atomide.com>
Date: Fri, 22 Oct 2010 12:41:55 -0700
Subject: [PATCH] ARM: Fix data abort accessing proc_info from __lookup_processor_type

Commit 5085f3ff458521045f7e43da62b8c30ea7df2e82 added better support for
CONFIG_HOTPLUG_CPU by keeping proc_info around. However, depending on
the Kconfig options selected, this can make the booting fail mysteriously
early on.

Turns out a data abort can happen in __lookup_processor in ldmia r5 {r3, r4}.
When it happens the address loaded to r5 is not aligned. Fix the problem by
aligning proc_info.

Reported-by: Anand Gadiyar <gadiyar@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Tested-by: Anand Gadiyar <gadiyar@ti.com>

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 1953e3d..cead889 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -113,6 +113,7 @@ SECTIONS
 			*(.rodata.*)
 			*(.glue_7)
 			*(.glue_7t)
+		. = ALIGN(4);
 		*(.got)			/* Global offset table		*/
 			ARM_CPU_KEEP(PROC_INFO)
 	}