Re: [PATCH 6/7] perf cs-etm: Don't hard code config attribute when configuring the event

On Mon, Dec 01, 2025 at 04:41:09PM +0000, Coresight ML wrote:

[...]

@@ -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.

• 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