[PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[Christophe Leroy]

On PPC832x, perf record/report reports martian addresses

     2.62%  perf_reseau4  libpthread-2.18.so  [.] __libc_send
     2.56%  perf_reseau4  [kernel.kallsyms]   [k] __ip_make_skb
     1.62%  perf_reseau4  [kernel.kallsyms]   [k] __ip_append_data.isra.39
     1.55%  perf_reseau4  [kernel.kallsyms]   [k] ip_finish_output
     1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd94
     1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd95
     1.28%  perf_reseau4  [unknown]           [k] 0x7ffffd97
     1.26%  perf_reseau4  [unknown]           [k] 0x7ffffda3
     1.24%  perf_reseau4  [unknown]           [k] 0x7ffffd98
     1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd92
     1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd9b
     [.....]

This is due to function perf_instruction_pointer() reading SPR SIAR
which doesn't exist on e300 core. The perf_instruction_pointer() is
redefined in arch/powerpc/perf/core-book3s.c when CONFIG_PPC_PERF_CTRS
is selected.

This patch moves the selection of CONFIG_PPC_HAVE_PMU in 86xx section
so that CONFIG_PPC_PERF_CTRS won't be selected for other 6xx powerpc

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/platforms/86xx/Kconfig    | 1 +
 arch/powerpc/platforms/Kconfig.cputype | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/86xx/Kconfig b/arch/powerpc/platforms/86xx/Kconfig
index 1afd1e4..bb24d2a 100644
--- a/arch/powerpc/platforms/86xx/Kconfig
+++ b/arch/powerpc/platforms/86xx/Kconfig
@@ -5,6 +5,7 @@ menuconfig PPC_86xx
 	select FSL_SOC
 	select ALTIVEC
 	select ARCH_WANT_OPTIONAL_GPIOLIB
+	select PPC_HAVE_PMU_SUPPORT
 	help
 	  The Freescale E600 SoCs have 74xx cores.
 
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index d642cf9..aa63b12 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -143,7 +143,6 @@ config PPC_BOOK3E
 config 6xx
 	def_bool y
 	depends on PPC32 && PPC_BOOK3S
-	select PPC_HAVE_PMU_SUPPORT
 
 config TUNE_CELL
 	bool "Optimize for Cell Broadband Engine"
-- 
2.1.0

Re: [PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[Michael Ellerman]

On Thu, 2015-09-03 at 11:27 +0200, Christophe Leroy wrote:
> On PPC832x, perf record/report reports martian addresses
> 
>      2.62%  perf_reseau4  libpthread-2.18.so  [.] __libc_send
>      2.56%  perf_reseau4  [kernel.kallsyms]   [k] __ip_make_skb
>      1.62%  perf_reseau4  [kernel.kallsyms]   [k] __ip_append_data.isra.39
>      1.55%  perf_reseau4  [kernel.kallsyms]   [k] ip_finish_output
>      1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd94
>      1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd95
>      1.28%  perf_reseau4  [unknown]           [k] 0x7ffffd97
>      1.26%  perf_reseau4  [unknown]           [k] 0x7ffffda3
>      1.24%  perf_reseau4  [unknown]           [k] 0x7ffffd98
>      1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd92
>      1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd9b
>      [.....]
> 
> This is due to function perf_instruction_pointer() reading SPR SIAR
> which doesn't exist on e300 core. The perf_instruction_pointer() is
> redefined in arch/powerpc/perf/core-book3s.c when CONFIG_PPC_PERF_CTRS
> is selected.
> 
> This patch moves the selection of CONFIG_PPC_HAVE_PMU in 86xx section
> so that CONFIG_PPC_PERF_CTRS won't be selected for other 6xx powerpc

I don't know much about these old 6xx cpus, deferring to Paul.

cheers

Re: [PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[Scott Wood]

On Thu, Sep 03, 2015 at 11:27:03AM +0200, Christophe Leroy wrote:
> On PPC832x, perf record/report reports martian addresses
> 
>      2.62%  perf_reseau4  libpthread-2.18.so  [.] __libc_send
>      2.56%  perf_reseau4  [kernel.kallsyms]   [k] __ip_make_skb
>      1.62%  perf_reseau4  [kernel.kallsyms]   [k] __ip_append_data.isra.39
>      1.55%  perf_reseau4  [kernel.kallsyms]   [k] ip_finish_output
>      1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd94
>      1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd95
>      1.28%  perf_reseau4  [unknown]           [k] 0x7ffffd97
>      1.26%  perf_reseau4  [unknown]           [k] 0x7ffffda3
>      1.24%  perf_reseau4  [unknown]           [k] 0x7ffffd98
>      1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd92
>      1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd9b
>      [.....]
> 
> This is due to function perf_instruction_pointer() reading SPR SIAR
> which doesn't exist on e300 core. The perf_instruction_pointer() is
> redefined in arch/powerpc/perf/core-book3s.c when CONFIG_PPC_PERF_CTRS
> is selected.
> 
> This patch moves the selection of CONFIG_PPC_HAVE_PMU in 86xx section
> so that CONFIG_PPC_PERF_CTRS won't be selected for other 6xx powerpc
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

So, what happens when a kernel is built that supports both 83xx and 86xx? 
Plus, it's e300, not e600, that is the exception among 6xx-style cores.

-Scott

Re: [PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[christophe leroy]

Le 04/09/2015 18:43, Scott Wood a écrit :
> On Thu, Sep 03, 2015 at 11:27:03AM +0200, Christophe Leroy wrote:
>> On PPC832x, perf record/report reports martian addresses
>>
>>       2.62%  perf_reseau4  libpthread-2.18.so  [.] __libc_send
>>       2.56%  perf_reseau4  [kernel.kallsyms]   [k] __ip_make_skb
>>       1.62%  perf_reseau4  [kernel.kallsyms]   [k] __ip_append_data.isra.39
>>       1.55%  perf_reseau4  [kernel.kallsyms]   [k] ip_finish_output
>>       1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd94
>>       1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd95
>>       1.28%  perf_reseau4  [unknown]           [k] 0x7ffffd97
>>       1.26%  perf_reseau4  [unknown]           [k] 0x7ffffda3
>>       1.24%  perf_reseau4  [unknown]           [k] 0x7ffffd98
>>       1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd92
>>       1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd9b
>>       [.....]
>>
>> This is due to function perf_instruction_pointer() reading SPR SIAR
>> which doesn't exist on e300 core. The perf_instruction_pointer() is
>> redefined in arch/powerpc/perf/core-book3s.c when CONFIG_PPC_PERF_CTRS
>> is selected.
>>
>> This patch moves the selection of CONFIG_PPC_HAVE_PMU in 86xx section
>> so that CONFIG_PPC_PERF_CTRS won't be selected for other 6xx powerpc
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> So, what happens when a kernel is built that supports both 83xx and 86xx?
Right, so should we define a processor feature for it ?
> Plus, it's e300, not e600, that is the exception among 6xx-style cores.
Is it ? I've been looking for special register SIAR (spr 955) in several 
6xx reference manuals.
82xx doesn't have it, 52xx and 512x don't have it.
I found it only in the 86xx
Which other family has it ?

Christophe

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[Segher Boessenkool]

On Fri, Sep 04, 2015 at 08:00:59PM +0200, christophe leroy wrote:
> >Plus, it's e300, not e600, that is the exception among 6xx-style cores.
> Is it ? I've been looking for special register SIAR (spr 955) in several 
> 6xx reference manuals.
> 82xx doesn't have it, 52xx and 512x don't have it.
> I found it only in the 86xx
> Which other family has it ?

7xx, 7xxx have it.  I don't think any 603 or 604 have it.


Segher

Re: [PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[Segher Boessenkool]

[ it seems my previous mail disappeared down the void ]

On Fri, Sep 04, 2015 at 08:00:59PM +0200, christophe leroy wrote:
> >Plus, it's e300, not e600, that is the exception among 6xx-style cores.
> Is it ? I've been looking for special register SIAR (spr 955) in several 
> 6xx reference manuals.
> 82xx doesn't have it, 52xx and 512x don't have it.
> I found it only in the 86xx
> Which other family has it ?

All 7xx and 7xxx have it.  No 603 or 604 does afaik.


Segher

Re: [PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[Scott Wood]

On Fri, 2015-09-04 at 20:00 +0200, christophe leroy wrote:
> Le 04/09/2015 18:43, Scott Wood a écrit :
> > On Thu, Sep 03, 2015 at 11:27:03AM +0200, Christophe Leroy wrote:
> > > On PPC832x, perf record/report reports martian addresses
> > > 
> > >       2.62%  perf_reseau4  libpthread-2.18.so  [.] __libc_send
> > >       2.56%  perf_reseau4  [kernel.kallsyms]   [k] __ip_make_skb
> > >       1.62%  perf_reseau4  [kernel.kallsyms]   [k] 
> > > __ip_append_data.isra.39
> > >       1.55%  perf_reseau4  [kernel.kallsyms]   [k] ip_finish_output
> > >       1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd94
> > >       1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd95
> > >       1.28%  perf_reseau4  [unknown]           [k] 0x7ffffd97
> > >       1.26%  perf_reseau4  [unknown]           [k] 0x7ffffda3
> > >       1.24%  perf_reseau4  [unknown]           [k] 0x7ffffd98
> > >       1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd92
> > >       1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd9b
> > >       [.....]
> > > 
> > > This is due to function perf_instruction_pointer() reading SPR SIAR
> > > which doesn't exist on e300 core. The perf_instruction_pointer() is
> > > redefined in arch/powerpc/perf/core-book3s.c when CONFIG_PPC_PERF_CTRS
> > > is selected.
> > > 
> > > This patch moves the selection of CONFIG_PPC_HAVE_PMU in 86xx section
> > > so that CONFIG_PPC_PERF_CTRS won't be selected for other 6xx powerpc
> > > 
> > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > So, what happens when a kernel is built that supports both 83xx and 86xx?
> Right, so should we define a processor feature for it ?

There aren't many CPU feature flags left (without changing the mechanism to 
make more available), and runtime patching shouldn't be necessary.  Instead, 
use the existing oprofile_type field.  BTW, I don't know why we have pmc_type 
in addition to that -- the hardware only needs to be described once, 
regardless of how many subsystems care about it.

Note that this will only address the case of failing gracefully when the 
running hardware doesn't support the compiled-in perf mechanism.  Actually 
supporting both mechanisms in the same kernel (some e300 cores support fsl-
emb) would require a new indirection layer.

-Scott

Re: [PATCH] powerpc/book3s32: Only select PPC_HAVE_PMU on e600

[Michael Ellerman]

On Wed, 2015-09-09 at 17:24 -0500, Scott Wood wrote:
> On Fri, 2015-09-04 at 20:00 +0200, christophe leroy wrote:
> > Le 04/09/2015 18:43, Scott Wood a écrit :
> > > On Thu, Sep 03, 2015 at 11:27:03AM +0200, Christophe Leroy wrote:
> > > > On PPC832x, perf record/report reports martian addresses
> > > > 
> > > >       2.62%  perf_reseau4  libpthread-2.18.so  [.] __libc_send
> > > >       2.56%  perf_reseau4  [kernel.kallsyms]   [k] __ip_make_skb
> > > >       1.62%  perf_reseau4  [kernel.kallsyms]   [k] 
> > > > __ip_append_data.isra.39
> > > >       1.55%  perf_reseau4  [kernel.kallsyms]   [k] ip_finish_output
> > > >       1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd94
> > > >       1.33%  perf_reseau4  [unknown]           [k] 0x7ffffd95
> > > >       1.28%  perf_reseau4  [unknown]           [k] 0x7ffffd97
> > > >       1.26%  perf_reseau4  [unknown]           [k] 0x7ffffda3
> > > >       1.24%  perf_reseau4  [unknown]           [k] 0x7ffffd98
> > > >       1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd92
> > > >       1.22%  perf_reseau4  [unknown]           [k] 0x7ffffd9b
> > > >       [.....]
> > > > 
> > > > This is due to function perf_instruction_pointer() reading SPR SIAR
> > > > which doesn't exist on e300 core. The perf_instruction_pointer() is
> > > > redefined in arch/powerpc/perf/core-book3s.c when CONFIG_PPC_PERF_CTRS
> > > > is selected.
> > > > 
> > > > This patch moves the selection of CONFIG_PPC_HAVE_PMU in 86xx section
> > > > so that CONFIG_PPC_PERF_CTRS won't be selected for other 6xx powerpc
> > > > 
> > > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > So, what happens when a kernel is built that supports both 83xx and 86xx?
> > Right, so should we define a processor feature for it ?
> 
> There aren't many CPU feature flags left (without changing the mechanism to 
> make more available), and runtime patching shouldn't be necessary.  Instead, 
> use the existing oprofile_type field.  

Yep, +1. Only add new cpu feature flags if you really need them please.

> BTW, I don't know why we have pmc_type in addition to that -- the hardware
> only needs to be described once, regardless of how many subsystems care about
> it.

Right, I think that's all just accreted over the years, and predates perf. We
could probably consolidate some of it.

cheers