Re: [PATCH 00/13] Add support for perf_arch_regs

[Nilay Vaish]

On 28 August 2016 at 16:00, Madhavan Srinivasan
<maddy@xxxxxxxxxxxxxxxxxx> wrote:
> Patchset to extend PERF_SAMPLE_REGS_INTR to include
> platform specific PMU registers.
>
> Patchset applies cleanly on tip:perf/core branch
>
> It's a perennial request from hardware folks to be able to
> see the raw values of the pmu registers. Partly it's so that
> they can verify perf is doing what they want, and some
> of it is that they're interested in some of the more obscure
> info that isn't plumbed out through other perf interfaces.
>
> Over the years internally we have used various hack to get
> the requested data out but this is an attempt to use a
> somewhat standard mechanism (using PERF_SAMPLE_REGS_INTR).
>
> This would also be helpful for those of us working on the perf
> hardware backends, to be able to verify that we're programming
> things correctly, without resorting to debug printks etc.
>
> Mechanism proposed:
>
> 1)perf_regs structure is extended with a perf_arch_regs structure
> which each arch/ can populate with their specific platform
> registers to sample on each perf interrupt and an arch_regs_mask
> variable, which is for perf tool to know about the perf_arch_regs
> that are supported.
>
> 2)perf/core func perf_sample_regs_intr() extended to update
> the perf_arch_regs structure and the perf_arch_reg_mask. Set of new
> support functions added perf_get_arch_regs_mask() and
> perf_get_arch_reg() to aid the updates from arch/ side.
>
> 3) perf/core funcs perf_prepare_sample() and perf_output_sample()
> are extended to support the update for the perf_arch_regs_mask and
> perf_arch_regs in the sample
>
> 4)perf/core func perf_output_sample_regs() extended to dump
> the arch_regs to the output sample.
>
> 5)Finally, perf tool side is updated to include a new element
> "arch_regs_mask" in the "struct regs_dump", event sample funcs
> and print functions are updated to support perf_arch_regs.
>

I read the patch series and I have one suggestion to make.  I think we
should not use 'arch regs' to refer to these pmu registers.  I think
architectural registers typically refer to the ones that hold the
state of the process.  Can we replace arch_regs by pmu_regs, or some
other choice?

Thanks
Nilay