[PATCH] drivers/perf: arm_pmu: save/restore cpu cycle counter in cpu_pm_pmu_notify

[Jia He]

Thanks, got it

Cheers,

Jia


On 11/20/2017 10:04 PM, Mark Rutland Wrote:
> On Mon, Nov 20, 2017 at 09:50:15PM +0800, Jia He wrote:
>> On 11/20/2017 8:32 PM, Mark Rutland Wrote:
>>> On Thu, Nov 16, 2017 at 06:27:28AM +0000, Jia He wrote:
>>>> Sometimes userspace need a high resolution cycle counter by reading
>>>> pmccntr_el0.
>>>>
>>>> In commit da4e4f18afe0 ("drivers/perf: arm_pmu: implement CPU_PM
>>>> notifier"), it resets all the counters even when the pmcr_el0.E and
>>>> pmcntenset_el0.C are both 1 . That is incorrect.
>>> I appreciate that you may wish to make use of the cycle counter from
>>> userspace, but this is the intended behaviour kernel-side. Direct
>>> userspace counter acceess is not supported.
>> Sorry for my previous description. Not only user space, but also any
>> pmccntr_el0 reading
>> in kernel space is 0 except perf infrastructure.
> The only user of PMCCNTR in the kernel is the perf infrastructure.
>
> If you want to perform in-kernel counts, please use
> perf_event_create_kernel_counter() and related functions.
>
>>> In power states where context is lost, any perf events are
>>> saved/restored by cpu_pm_pmu_setup(). So we certainly shouldn't be
>>> modifying the counter registers in any other PM code.
>>>
>>> We *could* expose counters to userspace on homogeneous systems, so long
>>> as users stuck to the usual perf data page interface. However, this
>>> comes with a number of subtle problems, and no-one's done the work to
>>> enable this.
>>>
>>> Even then, perf may modify counters at any point in time, and
>>> monotonicity (and/or presence) of counters is not guaranteed.
>> Yes, the cycle counter register pmccntr_el0 will be impacted by perf
>> usage.But do you think pmccntr_el0 is intented for perf only?
> We only intend for the in-kernel perf infrastructure to access
> pmccntr_el0; nothing else should touch it.
>
>> Currently, many user space/kernel space programs will use pmccntr_el0
>> as a timestamp( just likethe rdtsc() on X86).
> This is not supported, and will return completely unusable numbers if
> it were. This will be randomly reset/modified, CPUs aren't in lock-step,
> so this isn't monotonic, etc.
>
> If you want a timestamp, the architecture provides an counter that is
> monotonic and uniform across all CPUs. We expose that to userspace via
> the VDSO (e.g. clock_gettime with CLOCK_MONOTONIC_RAW) and it can be
> read in-kernel through the usual clocksource APIs.
>
> PMCCNTR is *not* usable in this manner.
>
>> After commit da4e4f18afe0, all the cycle counter readingis 0 because
>> of CPU PM enter/exit state changes in armv8pmu_reset.
> Previously, had a CPU been reset, the counter would hold an UNKNOWN
> value (i.e. it would be reset to junk), and may or may not have been
> accessible depending on what CNTKCTL reset to.
>
> So if you tried to read PMCCNTR in userspace, and happened to have been
> granted access, the values would have been bogus and unusable.
>
> Commit da4e4f18afe0 fixed things and made the behaviour consistent, as
> intended.
>
> Thanks,
> Mark.
>