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

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: <20251202121528.GX724103@e132581.arm.com>
Date: Tue, 2 Dec 2025 12:15:28 +0000
From: Leo Yan <leo.yan@....com>
To: James Clark <james.clark@...aro.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Mike Leach <mike.leach@...aro.org>,
	John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
	Leo Yan <leo.yan@...ux.dev>, linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org, coresight@...ts.linaro.org,
	linux-arm-kernel@...ts.infradead.org
Subject: 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