linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Clark <james.clark@linaro.org>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	 Arnaldo Carvalho de Melo <acme@kernel.org>,
	 Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	 Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	 Adrian Hunter <adrian.hunter@intel.com>,
	 Suzuki K Poulose <suzuki.poulose@arm.com>,
	 Mike Leach <mike.leach@linaro.org>,
	John Garry <john.g.garry@oracle.com>,
	 Will Deacon <will@kernel.org>, Leo Yan <leo.yan@linux.dev>
Cc: linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	 coresight@lists.linaro.org,
	linux-arm-kernel@lists.infradead.org,
	 James Clark <james.clark@linaro.org>
Subject: [PATCH 2/7] perf tools: apply evsel__set_config_if_unset() to all config fields
Date: Mon, 01 Dec 2025 16:41:05 +0000	[thread overview]
Message-ID: <20251201-james-perf-config-bits-v1-2-22ecbbf8007c@linaro.org> (raw)
In-Reply-To: <20251201-james-perf-config-bits-v1-0-22ecbbf8007c@linaro.org>

Misleadingly, evsel__set_config_if_unset() only works with the config
field and not config1, config2, etc. This is fine at the moment because
all users of it happen to operate on bits that are in that config field.
Fix it before there are any new users of the function which operate on
bits in different config fields.

In theory it's also possible for a driver to move an existing bit to
another config field and this fixes that scenario too, although this
hasn't happened yet either.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/util/pmu.c | 60 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 7a7db31be70d..c8968cddc0a9 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/list.h>
+#include <linux/bitfield.h>
 #include <linux/compiler.h>
 #include <linux/string.h>
 #include <linux/zalloc.h>
@@ -1371,6 +1372,18 @@ bool evsel__is_aux_event(const struct evsel *evsel)
 	return pmu && pmu->auxtrace;
 }
 
+static struct perf_pmu_format *
+pmu_find_format(const struct list_head *formats, const char *name)
+{
+	struct perf_pmu_format *format;
+
+	list_for_each_entry(format, formats, list)
+		if (!strcmp(format->name, name))
+			return format;
+
+	return NULL;
+}
+
 /*
  * Set @config_name to @val as long as the user hasn't already set or cleared it
  * by passing a config term on the command line.
@@ -1379,12 +1392,39 @@ bool evsel__is_aux_event(const struct evsel *evsel)
  * the bit pattern. It is shifted into position by this function, so to set
  * something to true, pass 1 for val rather than a pre shifted value.
  */
-#define field_prep(_mask, _val) (((_val) << (ffsll(_mask) - 1)) & (_mask))
 void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
 				const char *config_name, u64 val)
 {
 	u64 user_bits = 0, bits;
-	struct evsel_config_term *term = evsel__get_config_term(evsel, USR_CHG_CONFIG);
+	struct evsel_config_term *term;
+	struct perf_pmu_format *format = pmu_find_format(&pmu->format, config_name);
+	__u64 *vp;
+
+	switch (format->value) {
+	case PERF_PMU_FORMAT_VALUE_CONFIG:
+		term = evsel__get_config_term(evsel, USR_CHG_CONFIG);
+		vp = &evsel->core.attr.config;
+		break;
+	case PERF_PMU_FORMAT_VALUE_CONFIG1:
+		term = evsel__get_config_term(evsel, USR_CHG_CONFIG1);
+		vp = &evsel->core.attr.config1;
+		break;
+	case PERF_PMU_FORMAT_VALUE_CONFIG2:
+		term = evsel__get_config_term(evsel, USR_CHG_CONFIG2);
+		vp = &evsel->core.attr.config2;
+		break;
+	case PERF_PMU_FORMAT_VALUE_CONFIG3:
+		term = evsel__get_config_term(evsel, USR_CHG_CONFIG3);
+		vp = &evsel->core.attr.config3;
+		break;
+	case PERF_PMU_FORMAT_VALUE_CONFIG4:
+		term = evsel__get_config_term(evsel, USR_CHG_CONFIG4);
+		vp = &evsel->core.attr.config4;
+		break;
+	default:
+		pr_err("Unknown format value: %d\n", format->value);
+		return;
+	}
 
 	if (term)
 		user_bits = term->val.cfg_chg;
@@ -1396,20 +1436,8 @@ void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
 		return;
 
 	/* Otherwise replace it */
-	evsel->core.attr.config &= ~bits;
-	evsel->core.attr.config |= field_prep(bits, val);
-}
-
-static struct perf_pmu_format *
-pmu_find_format(const struct list_head *formats, const char *name)
-{
-	struct perf_pmu_format *format;
-
-	list_for_each_entry(format, formats, list)
-		if (!strcmp(format->name, name))
-			return format;
-
-	return NULL;
+	*vp &= ~bits;
+	*vp |= FIELD_PREP(bits, val);
 }
 
 __u64 perf_pmu__format_bits(struct perf_pmu *pmu, const char *name)

-- 
2.34.1


  parent reply	other threads:[~2025-12-01 16:41 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-01 16:41 [PATCH 0/7] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
2025-12-01 16:41 ` [PATCH 1/7] perf tools: Track all user changed config bits James Clark
2025-12-02 10:15   ` Leo Yan
2025-12-02 10:40     ` James Clark
2025-12-02 11:21       ` Leo Yan
2025-12-02 12:36         ` Leo Yan
2025-12-01 16:41 ` James Clark [this message]
2025-12-02 11:14   ` [PATCH 2/7] perf tools: apply evsel__set_config_if_unset() to all config fields Leo Yan
2025-12-04 10:55     ` James Clark
2025-12-01 16:41 ` [PATCH 3/7] perf cs-etm: Make a helper to find the Coresight evsel James Clark
2025-12-02 11:24   ` Leo Yan
2025-12-01 16:41 ` [PATCH 4/7] perf cs-etm: Don't use hard coded config bits when setting up ETMCR James Clark
2025-12-02 11:43   ` Leo Yan
2025-12-02 11:53     ` James Clark
2025-12-04 10:55       ` James Clark
2025-12-04 13:45         ` Mike Leach
2025-12-04 13:48           ` James Clark
2025-12-01 16:41 ` [PATCH 5/7] perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR James Clark
2025-12-02 12:01   ` Leo Yan
2025-12-01 16:41 ` [PATCH 6/7] perf cs-etm: Don't hard code config attribute when configuring the event James Clark
2025-12-02 12:15   ` Leo Yan
2025-12-04 14:08     ` James Clark
2025-12-04 14:43       ` Leo Yan
2025-12-01 16:41 ` [PATCH 7/7] perf arm-spe: Don't hard code config attribute James Clark
2025-12-02 12:28   ` Leo Yan
2025-12-02 12:42     ` Leo Yan
2025-12-02 12:59       ` James Clark

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=20251201-james-perf-config-bits-v1-2-22ecbbf8007c@linaro.org \
    --to=james.clark@linaro.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=coresight@lists.linaro.org \
    --cc=irogers@google.com \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=leo.yan@linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mike.leach@linaro.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    /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 NNTP newsgroup(s).