LinuxPPC-Dev Archive mirror
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Athira Rajeev <atrajeev@linux.ibm.com>, <acme@kernel.org>,
	<jolsa@kernel.org>, <maddy@linux.ibm.com>, <irogers@google.com>,
	<namhyung@kernel.org>, <linux-perf-users@vger.kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>, <aboorvad@linux.ibm.com>,
	<sshegde@linux.ibm.com>, <hbathini@linux.vnet.ibm.com>,
	<Aditya.Bodkhe1@ibm.com>, <venkat88@linux.ibm.com>
Subject: Re: [PATCH V2 1/6] tools/perf: Add basic CONFIG_AUXTRACE support for VPA pmu on powerpc
Date: Mon, 15 Sep 2025 18:07:56 +0300	[thread overview]
Message-ID: <bd19b37e-6dfb-495a-bc05-47a347962515@intel.com> (raw)
In-Reply-To: <20250915072754.99850-2-atrajeev@linux.ibm.com>

On 15/09/2025 10:27, Athira Rajeev wrote:
> The powerpc PMU collecting Dispatch Trace Log (DTL) entries makes use of
> AUX support in perf infrastructure. The PMU driver has the functionality
> to collect trace entries in the aux buffer. On the tools side, this data
> is made available as PERF_RECORD_AUXTRACE records. This record is
> generated by "perf record" command. To enable the creation of
> PERF_RECORD_AUXTRACE, add functions to initialize auxtrace records ie
> "auxtrace_record__init()". Fill in fields for other callbacks like
> info_priv_size, info_fill, free, recording options etc. Define
> auxtrace_type as PERF_AUXTRACE_VPA_DTL. Add header file to define vpa
> dtl pmu specific details.
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> ---
> Changelog:
> Addressed review comments from Adrian:
> - Return VPADTL_AUXTRACE_PRIV_SIZE in powerpc_vpadtl_info_priv_size
> - Remove unused powerpc_vpadtl_parse_snapshot_options
> - Some of the function parameters had "__maybe_unused", corrected it.
> - Used PERF_AUXTRACE_VPA_DTL instead of PERF_AUXTRACE_VPA_PMU
> - Moved powerpc_vpadtl_process_auxtrace_info to next patch
> 
>  tools/perf/arch/powerpc/util/Build      |   1 +
>  tools/perf/arch/powerpc/util/auxtrace.c | 114 ++++++++++++++++++++++++
>  tools/perf/util/auxtrace.c              |   1 +
>  tools/perf/util/auxtrace.h              |   1 +
>  tools/perf/util/powerpc-vpadtl.h        |  18 ++++
>  5 files changed, 135 insertions(+)
>  create mode 100644 tools/perf/arch/powerpc/util/auxtrace.c
>  create mode 100644 tools/perf/util/powerpc-vpadtl.h
> 
> diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
> index fdd6a77a3432..a5b0babd307e 100644
> --- a/tools/perf/arch/powerpc/util/Build
> +++ b/tools/perf/arch/powerpc/util/Build
> @@ -10,3 +10,4 @@ perf-util-$(CONFIG_LIBDW) += skip-callchain-idx.o
>  
>  perf-util-$(CONFIG_LIBUNWIND) += unwind-libunwind.o
>  perf-util-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
> +perf-util-$(CONFIG_AUXTRACE) += auxtrace.o
> diff --git a/tools/perf/arch/powerpc/util/auxtrace.c b/tools/perf/arch/powerpc/util/auxtrace.c
> new file mode 100644
> index 000000000000..803c582c0c6f
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/auxtrace.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * VPA support
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/bitops.h>
> +#include <linux/log2.h>

Are bitops.h and log2.h needed?

> +#include <linux/string.h>
> +#include <time.h>
> +
> +#include "../../util/cpumap.h"

Is cpumap.h needed?

> +#include "../../util/evsel.h"
> +#include "../../util/evlist.h"
> +#include "../../util/session.h"
> +#include "../../util/util.h"
> +#include "../../util/pmu.h"

Is pmu.h needed?

> +#include "../../util/debug.h"
> +#include "../../util/auxtrace.h"
> +#include "../../util/powerpc-vpadtl.h"
> +#include "../../util/record.h"
> +#include <internal/lib.h> // page_size
> +
> +#define KiB(x) ((x) * 1024)
> +
> +static int
> +powerpc_vpadtl_recording_options(struct auxtrace_record *ar __maybe_unused,
> +			struct evlist *evlist __maybe_unused,
> +			struct record_opts *opts)
> +{
> +	opts->full_auxtrace = true;
> +
> +	/*
> +	 * Set auxtrace_mmap_pages to minimum
> +	 * two pages
> +	 */
> +	if (!opts->auxtrace_mmap_pages) {
> +		opts->auxtrace_mmap_pages = KiB(128) / page_size;
> +		if (opts->mmap_pages == UINT_MAX)
> +			opts->mmap_pages = KiB(256) / page_size;
> +	}
> +
> +	return 0;
> +}
> +
> +static size_t powerpc_vpadtl_info_priv_size(struct auxtrace_record *itr __maybe_unused,
> +					struct evlist *evlist __maybe_unused)
> +{
> +	return VPADTL_AUXTRACE_PRIV_SIZE;
> +}
> +
> +static int
> +powerpc_vpadtl_info_fill(struct auxtrace_record *itr __maybe_unused,
> +		struct perf_session *session __maybe_unused,
> +		struct perf_record_auxtrace_info *auxtrace_info,
> +		size_t priv_size __maybe_unused)
> +{
> +	auxtrace_info->type = PERF_AUXTRACE_VPA_DTL;
> +
> +	return 0;
> +}
> +
> +static void powerpc_vpadtl_free(struct auxtrace_record *itr)
> +{
> +	free(itr);
> +}
> +
> +static u64 powerpc_vpadtl_reference(struct auxtrace_record *itr __maybe_unused)
> +{
> +	return 0;
> +}
> +
> +struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
> +						int *err)
> +{
> +	struct auxtrace_record *aux;
> +	struct evsel *pos;
> +	char *pmu_name;
> +	int found = 0;
> +
> +	evlist__for_each_entry(evlist, pos) {
> +		pmu_name = strdup(pos->name);
> +		pmu_name = strtok(pmu_name, "/");
> +		if (strstarts(pmu_name, "vpa_dtl")) {

pmu_name is leaked, but maybe it is not needed at all e.g.

		if (strstarts(pos->name, "vpa_dtl")) {

> +			found = 1;
> +			pos->needs_auxtrace_mmap = true;
> +			break;
> +		}
> +	}
> +
> +	if (!found)
> +		return NULL;
> +
> +	/*
> +	 * To obtain the auxtrace buffer file descriptor, the auxtrace event
> +	 * must come first.
> +	 */
> +	evlist__to_front(pos->evlist, pos);
> +
> +	aux = zalloc(sizeof(*aux));
> +	if (aux == NULL) {
> +		pr_debug("aux record is NULL\n");
> +		*err = -ENOMEM;
> +		return NULL;
> +	}
> +
> +	aux->recording_options = powerpc_vpadtl_recording_options;
> +	aux->info_priv_size = powerpc_vpadtl_info_priv_size;
> +	aux->info_fill = powerpc_vpadtl_info_fill;
> +	aux->free = powerpc_vpadtl_free;
> +	aux->reference = powerpc_vpadtl_reference;
> +	return aux;
> +}
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index ebd32f1b8f12..f294658bb948 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -1393,6 +1393,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
>  	case PERF_AUXTRACE_HISI_PTT:
>  		err = hisi_ptt_process_auxtrace_info(event, session);
>  		break;
> +	case PERF_AUXTRACE_VPA_DTL:
>  	case PERF_AUXTRACE_UNKNOWN:
>  	default:
>  		return -EINVAL;
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index f001cbb68f8e..e0a5b39fed12 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -50,6 +50,7 @@ enum auxtrace_type {
>  	PERF_AUXTRACE_ARM_SPE,
>  	PERF_AUXTRACE_S390_CPUMSF,
>  	PERF_AUXTRACE_HISI_PTT,
> +	PERF_AUXTRACE_VPA_DTL,
>  };
>  
>  enum itrace_period_type {
> diff --git a/tools/perf/util/powerpc-vpadtl.h b/tools/perf/util/powerpc-vpadtl.h
> new file mode 100644
> index 000000000000..50a7aa24acbe
> --- /dev/null
> +++ b/tools/perf/util/powerpc-vpadtl.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * VPA DTL PMU Support
> + */
> +
> +#ifndef INCLUDE__PERF_POWERPC_VPADTL_H__
> +#define INCLUDE__PERF_POWERPC_VPADTL_H__
> +
> +#define POWERPC_VPADTL_NAME "powerpc_vpadtl_"

Never used

> +
> +enum {
> +	POWERPC_VPADTL_TYPE,
> +	VPADTL_AUXTRACE_PRIV_MAX,
> +};
> +
> +#define VPADTL_AUXTRACE_PRIV_SIZE (VPADTL_AUXTRACE_PRIV_MAX * sizeof(u64))
> +
> +#endif



  reply	other threads:[~2025-09-15 15:08 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-15  7:27 [PATCH V2 0/6] perf/tools: Add interface to expose vpa dtl counters via Athira Rajeev
2025-09-15  7:27 ` [PATCH V2 1/6] tools/perf: Add basic CONFIG_AUXTRACE support for VPA pmu on powerpc Athira Rajeev
2025-09-15 15:07   ` Adrian Hunter [this message]
2025-09-16  4:37     ` Athira Rajeev
2025-09-15  7:27 ` [PATCH V2 2/6] tools/perf: process auxtrace events and display in perf report -D Athira Rajeev
2025-09-15 15:08   ` Adrian Hunter
2025-09-16  4:37     ` Athira Rajeev
2025-09-15  7:27 ` [PATCH V2 3/6] tools/perf: Add event name as vpa-dtl of PERF_TYPE_SYNTH type to present DTL samples Athira Rajeev
2025-09-15  7:27 ` [PATCH V2 4/6] tools/perf: Allocate and setup aux buffer queue to help co-relate with other events across CPU's Athira Rajeev
2025-09-15 15:08   ` Adrian Hunter
2025-09-16  4:37     ` Athira Rajeev
2025-09-15  7:27 ` [PATCH V2 5/6] tools/perf: Process the DTL entries in queue and deliver samples Athira Rajeev
2025-09-15 15:08   ` Adrian Hunter
2025-09-15  7:27 ` [PATCH V2 6/6] tools/perf: Enable perf script to present the DTL entries Athira Rajeev
2025-09-15 15:09   ` Adrian Hunter
2025-09-16  4:38     ` Athira Rajeev
2025-09-15 16:07 ` [PATCH V2 0/6] perf/tools: Add interface to expose vpa dtl counters via Venkat
2025-09-18  6:31   ` Athira Rajeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bd19b37e-6dfb-495a-bc05-47a347962515@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=Aditya.Bodkhe1@ibm.com \
    --cc=aboorvad@linux.ibm.com \
    --cc=acme@kernel.org \
    --cc=atrajeev@linux.ibm.com \
    --cc=hbathini@linux.vnet.ibm.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=namhyung@kernel.org \
    --cc=sshegde@linux.ibm.com \
    --cc=venkat88@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).