Subject: Re: linux-next: Tree for Dec 16

From: Stephen Rothwell <[email-masked]>
Date: Wed, 17 Dec 2014 10:57:22 +1100

Hi Andrew,

On Tue, 16 Dec 2014 10:45:49 +0000 James Hogan <[email-masked]> wrote:
>
> On 16/12/14 09:42, James Hogan wrote:
> > On 16/12/14 06:55, Guenter Roeck wrote:
> >> On 12/15/2014 10:44 PM, Andrew Morton wrote:
> >>> On Mon, 15 Dec 2014 22:35:49 -0800 Guenter Roeck <[email-masked]>
> >>> wrote:
> >>>
> >>>> My metag buildbot test fails with this kernel.
> >>>
> >>> Thanks.  What are the error messages?
> >>>
> >> Nothing. It just hangs until killed.
> >>
> >> qemu log:
> >> char device redirected to /dev/pts/5
> >> Unable to load ROM!
> >> qemu: terminating on signal 15 from pid 17843
> >>
> >> http://server.roeck-us.net:8010/builders/qemu-metag-next/builds/60/steps/qemubuildcommand/logs/stdio
> > 
> > Thanks a lot Guenter for report and bisection. I've confirmed on real
> > hardware too. I'll try debugging it.
> 
> The fmt in the first printk gets corrupted and causes a memory fault
> because the stack isn't 64-bit aligned. The args get saved with a 64-bit
> store (but unaligned access checking isn't turned on yet so this
> silently does the wrong thing), and then read as 1 with a 32-bit load.
> 
> Enabling unaligned access checking from boot makes it fail at
> metag_start_kernel, immediately after stack pointer is set to
> init_thread_union + THREAD_INFO_SIZE. Basically the restart_block was
> the only thing keeping the struct thread_info as a whole 64-bit aligned
> on Meta.
> 
> The patch below fixes it. Please can it be squashed into commit "all
> arches, signal: move restart_block to struct task_struct".
> 
> Thanks
> James
> 
> From 3ec117304ebde939fd181129227de28c068e7fa2 Mon Sep 17 00:00:00 2001
> From: James Hogan <[email-masked]>
> Date: Tue, 16 Dec 2014 10:15:33 +0000
> Subject: [PATCH] metag: Align thread_info::supervisor_stack
> 
> Commit a82be12232dc ("all arches, signal: move restart_block to struct
> task_struct") removed restart_block from struct thread_info which was
> the only thing keeping supervisor_stack and the struct as a whole 64-bit
> aligned. This resulted in the initial stack pointer not being 64-bit
> aligned, so when arguments are saved to the stack with a 64-bit SETL
> instruction the values are corrupted resulting in a pretty early
> unserviced memory fault in printk.
> 
> This is fixed by explicitly aligning supervisor_stack to 8 bytes.
> 
> Fixes: a82be12232dc ("all arches, signal: move restart_block to struct task_struct")
> Reported-by: Guenter Roeck <[email-masked]>
> Signed-off-by: James Hogan <[email-masked]>
> Cc: Andy Lutomirski <[email-masked]>
> Cc: Andrew Morton <[email-masked]>
> ---
>  arch/metag/include/asm/thread_info.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/metag/include/asm/thread_info.h b/arch/metag/include/asm/thread_info.h
> index ff4332435d15..afb3ca4776d1 100644
> --- a/arch/metag/include/asm/thread_info.h
> +++ b/arch/metag/include/asm/thread_info.h
> @@ -36,7 +36,7 @@ struct thread_info {
>  
>  	mm_segment_t addr_limit;	/* thread address space */
>  
> -	u8 supervisor_stack[0];
> +	u8 supervisor_stack[0] __aligned(8);
>  };
>  
>  #else /* !__ASSEMBLY__ */
> -- 
> 2.0.4

So should I just dump that patch into my copy of your tree?  (it seems
harmless and lets people get onto other stuff)

-- 
Cheers,
Stephen Rothwell                    [email-masked]