@@ -78,9 +78,9 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr,
char path[PATH_MAX];
int err;
u32 val;
- u64 contextid =
- evsel->core.attr.config &
- (perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") |
+ u64 contextid = evsel->core.attr.config &
+ (perf_pmu__format_bits(&cs_etm_pmu->format, "contextid") |
+ perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") |
perf_pmu__format_bits(&cs_etm_pmu->format, "contextid2"));
if (!contextid)
@@ -114,8 +114,7 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr,
* 0b00100 Maximum of 32-bit Context ID size.
* All other values are reserved.
*/
- val = BMVAL(val, 5, 9);
- if (!val || val != 0x4) {
+ if (BMVAL(val, 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;
Pre 5.11 kernels don't support 'contextid1' and 'contextid2' so validation would be skipped. By adding an additional check for 'contextid', old kernels will still have validation done even though contextid would either be contextid1 or contextid2. Additionally now that it's possible to override options, an existing bug in the validation is revealed. 'val' is overwritten by the contextid1 validation, and re-used for contextid2 validation causing it to always fail. '!val || val != 0x4' is the same as 'val != 0x4' because 0 is also != 4, so that expression can be simplified and the temp variable not overwritten. Fixes: 35c51f83dd1e ("perf cs-etm: Validate options after applying them") Reviewed-by: Leo Yan <leo.yan@linaro.org> Link: https://lore.kernel.org/all/20230501073452.GA4660@leoy-yangtze.lan Signed-off-by: James Clark <james.clark@arm.com> --- tools/perf/arch/arm/util/cs-etm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)