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

[Michael Ellerman <mpe@ellerman.id.au>]

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