Re: [PATCH 6/7] perf cs-etm: Don't hard code config attribute when configuring the event
From: Leo Yan <leo.yan@arm.com>
Date: 2025-12-02 12:15:31
Also in:
linux-perf-users, lkml
On Mon, Dec 01, 2025 at 04:41:09PM +0000, Coresight ML wrote: [...]
quoted hunk ↗ jump to hunk
@@ -103,13 +103,20 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel struct perf_cpu cpu) { int err; - __u64 val; - u64 contextid = evsel->core.attr.config & - (perf_pmu__format_bits(cs_etm_pmu, "contextid") | - perf_pmu__format_bits(cs_etm_pmu, "contextid1") | - perf_pmu__format_bits(cs_etm_pmu, "contextid2")); + u64 ctxt, ctxt1, ctxt2; + __u64 trcidr2; - if (!contextid) + err = evsel__get_config_val(cs_etm_pmu, evsel, "contextid", &ctxt); + if (err) + return err; + err = evsel__get_config_val(cs_etm_pmu, evsel, "contextid1", &ctxt1); + if (err) + return err; + err = evsel__get_config_val(cs_etm_pmu, evsel, "contextid2", &ctxt2); + if (err) + return err;
Seems to me, this is not right. The current code checks any context ID setting but it can tolerate if missing "contexid[*]" format. After calling evsel__get_config_val(), if any "contextid[*]" format is missed, it returns error and will diretly bail out. As a result, cs_etm_validate_context_id() will always return error.
quoted hunk ↗ jump to hunk
+ + if (!ctxt && !ctxt1 && !ctxt2) return 0; /* Not supported in etmv3 */@@ -120,12 +127,11 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel } /* Get a handle on TRCIDR2 */ - err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2], &val); + err = cs_etm_get_ro(cs_etm_pmu, cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR2], &trcidr2); if (err) return err; - if (contextid & - perf_pmu__format_bits(cs_etm_pmu, "contextid1")) { + if (ctxt1) { /* * TRCIDR2.CIDSIZE, bit [9-5], indicates whether contextID * tracing is supported:@@ -133,15 +139,14 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel * 0b00100 Maximum of 32-bit Context ID size. * All other values are reserved. */ - if (BMVAL(val, 5, 9) != 0x4) { + if (BMVAL(trcidr2, 5, 9) != 0x4) { pr_err("%s: CONTEXTIDR_EL1 isn't supported, disable with %s/contextid1=0/\n", CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME); return -EINVAL; } } - if (contextid & - perf_pmu__format_bits(cs_etm_pmu, "contextid2")) { + if (ctxt2) { /* * TRCIDR2.VMIDOPT[30:29] != 0 and * TRCIDR2.VMIDSIZE[14:10] == 0b00100 (32bit virtual contextid)@@ -149,7 +154,7 @@ static int cs_etm_validate_context_id(struct perf_pmu *cs_etm_pmu, struct evsel * virtual context id is < 32bit. * Any value of VMIDSIZE >= 4 (i.e, > 32bit) is fine for us. */ - if (!BMVAL(val, 29, 30) || BMVAL(val, 10, 14) < 4) { + if (!BMVAL(trcidr2, 29, 30) || BMVAL(trcidr2, 10, 14) < 4) { pr_err("%s: CONTEXTIDR_EL2 isn't supported, disable with %s/contextid2=0/\n", CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME); return -EINVAL;@@ -163,10 +168,14 @@ static int cs_etm_validate_timestamp(struct perf_pmu *cs_etm_pmu, struct evsel * struct perf_cpu cpu) { int err; - __u64 val; + u64 val; + __u64 trcidr0; - if (!(evsel->core.attr.config & - perf_pmu__format_bits(cs_etm_pmu, "timestamp"))) + err = evsel__get_config_val(cs_etm_pmu, evsel, "timestamp", &val); + if (err) + return err; + + if (!val) return 0;
Similiar issue here. The current code returns 0 if not find "timestamp" format. With this change, it returns error instead. Thanks, Leo