[v2,2/2] powerpc: oprofile: enable support for ppc750 processors

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 arch/powerpc/kernel/cputable.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

On Tue, Feb 24, 2009 at 6:09 AM, Octavian Purdila <opurdila@ixiacom.com> wrote:
> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
> ---
>  arch/powerpc/kernel/cputable.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> index 923f87a..4e20cfb 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -726,6 +726,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
>                .cpu_setup              = __setup_cpu_750,
>                .machine_check          = machine_check_generic,
>                .platform               = "ppc750",
> +               .oprofile_cpu_type      = "ppc/7450",
> +               .oprofile_type          = PPC_OPROFILE_G4,
>        },


I know this saves you some code, but it seems hacky.  It would be
better to modify oprofile to detect the proper cpu type.  Also, this
will screw things up if you try to use the different event set that
the 750 has.

Also, one more concern is the long-standing errata which makes this
quite dangerous.  All of the versions of the 750 I'm aware of have a
bug where if a Performance Monitor exception occurs within one cycle
of the Decrementer exception, the cpu will lose the ability to return
from the interrupt (SRR0/SRR1 become corrupted).  It's possible the
750s you have modified to support oprofile don't have this errata.
Alternatively, we can decide we don't care, as you have to be root to
use oprofile.  But this is why I didn't add support for anything
before the 7450.

Andy

On Thu, 2009-02-26 at 16:30 -0600, Andy Fleming wrote:
> 
> I know this saves you some code, but it seems hacky.  It would be
> better to modify oprofile to detect the proper cpu type.  Also, this
> will screw things up if you try to use the different event set that
> the 750 has.

Agreed. Note that Jack Miller (CC) has some patches for the oprofile
side.

> Also, one more concern is the long-standing errata which makes this
> quite dangerous.  All of the versions of the 750 I'm aware of have a
> bug where if a Performance Monitor exception occurs within one cycle
> of the Decrementer exception, the cpu will lose the ability to return
> from the interrupt (SRR0/SRR1 become corrupted).  It's possible the
> 750s you have modified to support oprofile don't have this errata.
> Alternatively, we can decide we don't care, as you have to be root to
> use oprofile.  But this is why I didn't add support for anything
> before the 7450.

I think we need to advertise it as broken in some way... I -think- the
latest batch of IBM 750CL have that bug fixed but I'm not 100% certain.

Cheers,
Ben.

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> On Thu, 2009-02-26 at 16:30 -0600, Andy Fleming wrote:
> > I know this saves you some code, but it seems hacky.  It would be
> > better to modify oprofile to detect the proper cpu type.  Also, this
> > will screw things up if you try to use the different event set that
> > the 750 has.
>
> Agreed. Note that Jack Miller (CC) has some patches for the oprofile
> side.
>

Not sure I understand, but the oprofile userspace  tool sees it as new 
processor (ppc750). I had to patch it to add the events for the new processor.

> > Also, one more concern is the long-standing errata which makes this
> > quite dangerous.  All of the versions of the 750 I'm aware of have a
> > bug where if a Performance Monitor exception occurs within one cycle
> > of the Decrementer exception, the cpu will lose the ability to return
> > from the interrupt (SRR0/SRR1 become corrupted).  It's possible the
> > 750s you have modified to support oprofile don't have this errata.
> > Alternatively, we can decide we don't care, as you have to be root to
> > use oprofile.  But this is why I didn't add support for anything
> > before the 7450.
>

Yes, I understand. We knew about the errata and we used the timer interrupt 
for profiling for some time, but then we run into some issue were we had to use 
advanced counters as L1 cache misses / TLB misses to diagnose some issues and 
we decided to give it a try. And _seems_ to work fine. Although we didn't 
stress it too hard (around 10-20 minutes of continuous run).

So maybe it would be useful for other people in the same situation as ours. 
BTW, this is the out of cat /proc/cpuinfo on our hw:

processor       : 0
cpu             : 750GX
temperature     : 10-12 C (uncalibrated)
revision        : 1.2 (pvr 7002 0102)
bogomips        : 1597.44
vendor          : Ixia
machine         : TCPX [0x6b]

 
> I think we need to advertise it as broken in some way... 

Perhaps set it to use the timer by default, and allow the user to switch to 
the hardware performance counter + printing a message that this mode is 
dangerous?

Thanks,
tavi