linux-kernel - Re: [PATCH v2 1/4] perf tools: Support Arm arch timer counter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <27ed6ebf-d9e6-e0ea-1577-2bba3d6f35c1@huawei.com>
Date:   Thu, 20 Aug 2020 10:56:46 +0800
From:   "liwei (GF)" <liwei391@...wei.com>
To:     Leo Yan <leo.yan@...aro.org>
CC:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ian Rogers <irogers@...gle.com>, Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Kemeng Shi <shikemeng@...wei.com>,
        "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Igor Lubashev <ilubashe@...mai.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        Stephane Eranian <eranian@...gle.com>,
        James Clark <james.clark@....com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/4] perf tools: Support Arm arch timer counter

Hi Leo

On 2020/8/7 15:16, Leo Yan wrote:
> The Arm arch timer can be used to calculate timestamp, the basic idea is
> the arch timer's counter value can be recorded in the hardware tracing
> data, e.g. the arch timer's counter value can be used for Arm CoreSight
> (not now but might be implemented later) and Arm SPE.  So we need a way
> to convert the arch timer's counter to the system time, the conversion
> is dependent on some related parameters, e.g. 'time_shift', 'time_mult',
> 'time_offset', etc; furthermore, to handle the counter wrapping issue,
> perf tool needs to know 'time_cycles' and 'time_mask' for correction.
> 
> This patch is to support Arm arch timer by reading out the relevant
> parameters from the head of first mmaped page.  And these parameters
> will be stored into the structure 'perf_arch_timer_conversion' for
> later calculation timestamp.
> 
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
> ---
>  tools/perf/arch/arm64/util/Build        |  1 +
>  tools/perf/arch/arm64/util/arch_timer.c | 50 +++++++++++++++++++++++++
>  tools/perf/util/arm_arch_timer.h        | 20 ++++++++++
>  3 files changed, 71 insertions(+)
>  create mode 100644 tools/perf/arch/arm64/util/arch_timer.c
>  create mode 100644 tools/perf/util/arm_arch_timer.h
> 
> diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
> index 5c13438c7bd4..77f4d7b30932 100644
> --- a/tools/perf/arch/arm64/util/Build
> +++ b/tools/perf/arch/arm64/util/Build
> @@ -1,3 +1,4 @@
> +perf-y += arch_timer.o
>  perf-y += header.o
>  perf-y += machine.o
>  perf-y += perf_regs.o
> diff --git a/tools/perf/arch/arm64/util/arch_timer.c b/tools/perf/arch/arm64/util/arch_timer.c
> new file mode 100644
> index 000000000000..dcc217c294fc
> --- /dev/null
> +++ b/tools/perf/arch/arm64/util/arch_timer.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdbool.h>
> +#include <errno.h>
> +
> +#include <linux/stddef.h>
> +#include <linux/perf_event.h>
> +
> +#include <linux/types.h>
> +#include <asm/barrier.h>
> +#include "../../../util/debug.h"
> +#include "../../../util/event.h"
> +#include "../../../util/synthetic-events.h"
> +#include "../../../util/arm_arch_timer.h"
> +
> +int perf_read_arch_timer_conversion(const struct perf_event_mmap_page *pc,
> +				    struct perf_arch_timer_conversion *tc)
> +{
> +	bool cap_user_time_zero, cap_user_time_short;
> +	u32 seq;
> +	int i = 0;
> +
> +	while (1) {
> +		seq = pc->lock;
> +		/* Add barrier between the sequence lock and data accessing */
> +		rmb();
> +
> +		tc->time_mult = pc->time_mult;
> +		tc->time_shift = pc->time_shift;
> +		tc->time_zero = pc->time_zero;
> +		tc->time_cycles = pc->time_cycles;
> +		tc->time_mask = pc->time_mask;
> +		cap_user_time_zero = pc->cap_user_time_zero;
> +		cap_user_time_short = pc->cap_user_time_short;
> +
> +		/* Add barrier between the data accessing and sequence lock */
> +		rmb();
> +		if (pc->lock == seq && !(seq & 1))
> +			break;
> +		if (++i > 10000) {
> +			pr_debug("%s: failed to get perf_event_mmap_page lock\n",
> +				 __func__);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (!cap_user_time_zero || !cap_user_time_short)
> +		return -EOPNOTSUPP;
> +
> +	return 0;
> +}

Should we implement the perf_event__synth_time_conv() method? As it can be supported
on arm64 arch now.

These is also a tsc get method called rdtsc(), weak implementation in 'tools/perf/util/tsc.c',
maybe we should rename it to an arch independent name, and implement the arm64 version
(read_cntvct_el0 in patch 3) here. Thus we can also make the testcase generic,
instead of adding a new one for arm64 :).

Thanks,
Wei

> diff --git a/tools/perf/util/arm_arch_timer.h b/tools/perf/util/arm_arch_timer.h
> new file mode 100644
> index 000000000000..a3263cc4e5cf
> --- /dev/null
> +++ b/tools/perf/util/arm_arch_timer.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PERF_ARM_ARCH_TIMER_H
> +#define __PERF_ARM_ARCH_TIMER_H
> +
> +#include <linux/types.h>
> +
> +struct perf_arch_timer_conversion {
> +	u16 time_shift;
> +	u32 time_mult;
> +	u64 time_zero;
> +	u64 time_cycles;
> +	u64 time_mask;
> +};
> +
> +struct perf_event_mmap_page;
> +
> +int perf_read_arch_timer_conversion(const struct perf_event_mmap_page *pc,
> +				    struct perf_arch_timer_conversion *tc);
> +
> +#endif // __PERF_ARM_ARCH_TIMER_H
>