linux-kernel - Re: [RFC PATCH v1 3/4] perf tool: Add HiSilicon PMCU data recording support

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: <20230317151322.000034eb@Huawei.com>
Date:   Fri, 17 Mar 2023 15:13:22 +0000
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Jie Zhan <zhanjie9@...ilicon.com>
CC:     <will@...nel.org>, <mark.rutland@....com>,
        <mathieu.poirier@...aro.org>, <suzuki.poulose@....com>,
        <mike.leach@...aro.org>, <leo.yan@...aro.org>,
        <john.g.garry@...cle.com>, <james.clark@....com>,
        <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
        <corbet@....net>, <zhangshaokun@...ilicon.com>,
        <shenyang39@...wei.com>, <hejunhao3@...wei.com>,
        <yangyicong@...ilicon.com>, <prime.zeng@...wei.com>,
        <suntao25@...wei.com>, <jiazhao4@...ilicon.com>,
        <linuxarm@...wei.com>, <linux-doc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-perf-users@...r.kernel.org>
Subject: Re: [RFC PATCH v1 3/4] perf tool: Add HiSilicon PMCU data recording
 support

On Mon, 6 Feb 2023 14:51:45 +0800
Jie Zhan <zhanjie9@...ilicon.com> wrote:

> Support for HiSilicon PMCU data recording using 'perf-record'.
> 
> Users can start PMCU profiling through 'perf-record'. Event numbers are
> passed by a sysfs interface. The following optional parameters can be
> passed through 'perf-record':
> - nr_sample: number of samples to take
> - sample_period_ms: time in ms for PMU counters to stay on for an event
> - pmccfiltr: bits[31-24] of system register PMCCFILTR_EL0
> 
> Example usage:
> 
> 1. Enter event numbers in the 'user_events' file:
> 
> 	echo "0x10 0x11" > /sys/devices/hisi_pmcu_sccl3/user_events
> 
> 2. Start the sampling with 'perf-record':
> 
> 	perf record -e hisi_pmcu_sccl3/nr_sample=1000,sample_period_ms=1/
> 
> In this example, the PMCU takes 1000 samples of event 0x0010 and 0x0011
> with a sampling period of 1ms. Data will be written to a 'perf.data' file.
> 
> Co-developed-by: Yang Shen <shenyang39@...wei.com>
> Signed-off-by: Yang Shen <shenyang39@...wei.com>
> Signed-off-by: Jie Zhan <zhanjie9@...ilicon.com>

I'm not particularly knowledgeable about perf tool so just some superficial comments
from me.

> ---

> diff --git a/tools/perf/arch/arm64/util/hisi-pmcu.c b/tools/perf/arch/arm64/util/hisi-pmcu.c
> new file mode 100644
> index 000000000000..7c33abf1182d
> --- /dev/null
> +++ b/tools/perf/arch/arm64/util/hisi-pmcu.c

> +struct hisi_pmcu_record {
> +	struct auxtrace_record itr;
> +	struct perf_pmu *hisi_pmcu_pmu;
> +	struct evlist *evlist;
> +};

...

> +struct auxtrace_record *hisi_pmcu_recording_init(int *err,
> +						 struct perf_pmu *hisi_pmcu_pmu)
> +{

...

> +	pmcu_record->hisi_pmcu_pmu = hisi_pmcu_pmu;
> +	pmcu_record->itr.recording_options = hisi_pmcu_recording_options;
> +	pmcu_record->itr.info_priv_size = hisi_pmcu_info_priv_size;
> +	pmcu_record->itr.info_fill = hisi_pmcu_info_fill;
> +	pmcu_record->itr.free = hisi_pmcu_record_free;
> +	pmcu_record->itr.reference = hisi_pmcu_reference;
> +	pmcu_record->itr.read_finish = auxtrace_record__read_finish;
> +	pmcu_record->itr.alignment = HISI_PMCU_DATA_ALIGNMENT;
> +	pmcu_record->itr.pmu = hisi_pmcu_pmu;

Maybe a local variable for itr - or if you can rely on c99 in perf tool
a compound literal to use structure field names etc.

	pmcu_record->itr = (struct xxx){
		.recording_options = ,
etc


> +
> +	*err = 0;
> +	return &pmcu_record->itr;
> +}

> diff --git a/tools/perf/util/hisi-pmcu.h b/tools/perf/util/hisi-pmcu.h
> new file mode 100644
> index 000000000000..d46d523a3aee
> --- /dev/null
> +++ b/tools/perf/util/hisi-pmcu.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * HiSilicon Performance Monitor Control Unit (PMCU) support
> + *
> + * Copyright (C) 2022 HiSilicon Limited

Probably want to update the dates if any substantial changes for v2.