Subject: Re: [PATCH 6/16] cell: abstract spu management routines

From: Arnd Bergmann <[email-masked]>
Date: Tue, 14 Nov 2006 10:55:21 +0100

On Tuesday 14 November 2006 04:44, Michael Ellerman wrote:
> OK, back to the task at hand :)  For the moment I'd rather see you leave
> out dump_data_fields(), as neither HV or baremetal implementations do
> anything. Just put the offending xmon code inside an #ifdef
> CONFIG_PPC_CELL_NATIVE. eg:
> 
> Index: cell/arch/powerpc/xmon/xmon.c
> ===================================================================
> --- cell.orig/arch/powerpc/xmon/xmon.c  2006-11-14 14:43:11.000000000 +1100
> +++ cell/arch/powerpc/xmon/xmon.c       2006-11-14 14:42:35.000000000 +1100
> @@ -2807,12 +2807,11 @@ static void dump_spu_fields(struct spu *
>                         in_be32(&spu->problem->spu_status_R));
>         DUMP_VALUE("0x%x", problem->spu_npc_RW,
>                         in_be32(&spu->problem->spu_npc_RW));
> +#ifdef CONFIG_PPC_CELL_NATIVE
>         DUMP_FIELD(spu, "0x%p", priv1);
> -
> -       if (spu->priv1) {
> -               DUMP_VALUE("0x%lx", priv1->mfc_sr1_RW,
> -                               in_be64(&spu->priv1->mfc_sr1_RW));
> -       }
> +       DUMP_VALUE("0x%lx", priv1->mfc_sr1_RW,
> +                       in_be64(&spu->priv1->mfc_sr1_RW));
> +#endif

Oops. Null pointer dereference.

You can't do this if you want to compile in both native and PS3PF
support in a single kernel. I think your original code that prints
priv1 and the sr1 only if priv1 exists is fine on both ways.

	Arnd <><