Re: [PATCH] kgdb, ppc: Corrected the address using of current->thread.evr register

[Jason Wessel <jason.wessel@windriver.com>]

On 11/16/2010 06:58 AM, Dongdong Deng wrote:
> Passing the address of current->thread.evr register to memcpy function.
>
>   

It turns out that out of all of my test configs and targets I did not
have any that defined both CONFIG_FSL_BOOKE and CONFIG_SPE else this
would have been caught in testing because will definitely crash
dereferencing the contents of the evr registers.

> Signed-off-by: Dongdong Deng <dongdong.deng@windriver.com>
> CC: Hai Shan <shan.hai@windriver.com>
> CC: Milton Miller <miltonm@bga.com>
> CC: linuxppc-dev@lists.ozlabs.org
> ---
>  arch/powerpc/kernel/kgdb.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> index 7a9db64..781acff 100644
> --- a/arch/powerpc/kernel/kgdb.c
> +++ b/arch/powerpc/kernel/kgdb.c
> @@ -337,7 +337,7 @@ char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
>  		/* FP registers 32 -> 63 */
>  #if defined(CONFIG_FSL_BOOKE) && defined(CONFIG_SPE)
>  		if (current)
> -			memcpy(mem, current->thread.evr[regno-32],
> +			memcpy(mem, (void *)&current->thread.evr[regno-32],
>  					dbg_reg_def[regno].size);
>   

As Sergei already pointed out, the (void *) casts should not be needed here.

This would have been fixed correctly had I seen the compile warnings
from the CONFIG_SPE at the test phase.   Namely at the compile phase
doesn't even complete:

arch/powerpc/kernel/kgdb.c: In function 'dbg_get_reg':
arch/powerpc/kernel/kgdb.c:341: error: passing argument 2 of 'memcpy'
makes pointer from integer without a cast
arch/powerpc/include/asm/string.h:25: note: expected 'const void *' but
argument is of type 'long unsigned int'
arch/powerpc/kernel/kgdb.c: In function 'dbg_set_reg':
arch/powerpc/kernel/kgdb.c:366: error: passing argument 1 of 'memcpy'
makes pointer from integer without a cast
arch/powerpc/include/asm/string.h:25: note: expected 'void *' but
argument is of type 'long unsigned int'


Moral of the story... I now have a board and config with the SPE turned
on, and after boot testing confirmed the evr registers are working.

I plan to have a pull request out to Linus with this fix and several
other accumulated regression fixes sometime in the next 48 hours.

Thanks,
Jason.