3.5+: yaboot, Invalid memory access

On Tue, 2012-09-04 at 02:32 -0700, Christian Kujau wrote:
> On Tue, 4 Sep 2012 at 16:51, Michael Ellerman wrote:
> > My guess would be we're calling that quite early and the __put_user()
> > check is getting confused and failing. That means we'll have left some
> > code unpatched, which then fails.
> > 
> > Can you try with the patch applied, but instead of returning if the
> > __put_user() fails, just continue on anyway.
> 
> You mean, like this?

Try this:

powerpc: Don't use __put_user() in patch_instruction

patch_instruction() can be called very early on ppc32, when the kernel
isn't yet running at it's linked address. That can cause the !
is_kernel_addr() test in __put_user() to trip and call might_sleep()
which is very bad at that point during boot.

Use a lower level function instead for now, at least until we get to
rework ppc32 boot process to do the code patching later, like ppc64
does.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

On Wed, 5 Sep 2012 at 11:08, Benjamin Herrenschmidt wrote:
> Try this:
> 
> powerpc: Don't use __put_user() in patch_instruction

Perfect! With this patch applied, the machine boots again.

  Tested-by: Christian Kujau <lists@nerdbynature.de>

I sure hope that other people will benefit from this as well. I can't 
stand the thought that you guys are always putting out fixes for this ol' 
PowerBook of mine :-\

Thanks so much,
Christian.

> 
> patch_instruction() can be called very early on ppc32, when the kernel
> isn't yet running at it's linked address. That can cause the !
> is_kernel_addr() test in __put_user() to trip and call might_sleep()
> which is very bad at that point during boot.
> 
> Use a lower level function instead for now, at least until we get to
> rework ppc32 boot process to do the code patching later, like ppc64
> does.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index dd223b3..17e5b23 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -20,7 +20,7 @@ int patch_instruction(unsigned int *addr, unsigned int instr)
>  {
>  	int err;
>  
> -	err = __put_user(instr, addr);
> +	__put_user_size(instr, addr, 4, err);
>  	if (err)
>  		return err;
>  	asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" : : "r" (addr));
> 
> 
>

On Tue, 2012-09-04 at 22:25 -0700, Christian Kujau wrote:
> I sure hope that other people will benefit from this as well. I can't 
> stand the thought that you guys are always putting out fixes for this
> ol' 
> PowerBook of mine :-\

Well, for some strange reason I didn't observe the problem on a couple
of old macs here, might have got lucky or something (could depend on
whether might_sleep() is compiled to do something or not... depends
on .config debug options).

Anyways, I'll send that fix to Linus asap. Thanks for reporting !

Cheers,
Ben.