Re: [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER

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

Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> On 28/11/18 9:04 AM, Michael Ellerman wrote:
>> Arnaldo Carvalho de Melo <acme@kernel.org> writes:
>>
>>> Em Mon, Nov 26, 2018 at 11:34:08PM +0530, Madhavan Srinivasan escreveu:
>>>> On each sample, Sample Instruction Event Register (SIER) content
>>>> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
>>>> but instead, SIER content is saved in the "dar" register of pt_regs.
>>>>
>>>> Patch adds another entry to the perf_regs structure to include the "SIER"
>>>> printing which internally maps to the "dar" of pt_regs.
>>> I think the patch is ok, when we talked in Vancouver I thought I saw
>>> something like this before, i.e. adding more registers to a perf_regs.h
>>> file, this was the cset:
>>>
>>>    commit 0da0017f72554c005c1a04c3adc5da9eb64fa7e5
>>>    Author: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
>>>    Date:   Wed Nov 8 07:30:15 2017 +0100
>>>
>>>        s390/perf: extend perf_regs support to include floating-point registers
>>>
>>> That I came across because it broke the perf build, making me add this
>>> cset:
>>>
>>>    commit 10b9baa701d5023897f70a4acb3bf0235da3dc4f
>>>    Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>    Date:   Tue Nov 28 11:08:41 2017 -0300
>>>
>>>      tools arch s390: Do not include header files from the kernel sources
>>>
>>> :-)
>>>
>>> Michael? What about the ppc specific details?
>> The only possible objection is that not all CPUs have an SIER register,
>> so on CPUs without it you'll get the content of the DAR register rather
>> than the SIER (because we (ab)use the DAR slot of pt_regs for the SIER).
>>
>> Perhaps we should make sure that we return 0 on CPUs that don't have the
>> register?
>
>
> Yes this make sense. We should make it zero instead of having the DAR
> value. I will respin the patch with that change.

Thanks.

There's three cases we need to cover.

The first is when we're not using core-book3s.c at all, ie. when we're
using the Freescale PMU code. You can probably handle that at build time
using CONFIG_FSL_EMB_PERF_EVENT?

Then there's the 32-bit case in core-book3s.c, currently that version of
perf_read_regs() never sets a value in dar.

And then there's the 64-bit case but PPMU_HAS_SIER is not set.

cheers