linux-kernel - Re: [PATCH v5 08/13] coresight: Interpret perf config with ATTR_CFG_GET

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: <416cf32d-d647-46dd-bcef-acc019490561@linaro.org>
Date: Wed, 19 Nov 2025 11:26:08 +0000
From: James Clark <james.clark@...aro.org>
To: Mike Leach <mike.leach@...aro.org>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jonathan Corbet <corbet@....net>, Leo Yan <leo.yan@....com>,
 Randy Dunlap <rdunlap@...radead.org>, coresight@...ts.linaro.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH v5 08/13] coresight: Interpret perf config with
 ATTR_CFG_GET_FLD()



On 19/11/2025 9:32 am, Mike Leach wrote:
> Hi James
> 
> On Tue, 18 Nov 2025 at 16:28, James Clark <james.clark@...aro.org> wrote:
>>
>> The "config:" string construction in format_attr_contextid_show() can be
>> removed because it either showed the existing context1 or context2
>> formats which have already been generated, so can be called themselves.
>>
>> The other conversions are straightforward replacements.
>>
>> Signed-off-by: James Clark <james.clark@...aro.org>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm-perf.c | 26 +++++++++++++++---------
>>   1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index 28f1bfc4579f..1b9ae832a576 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -80,12 +80,19 @@ static ssize_t format_attr_contextid_show(struct device *dev,
>>                                            struct device_attribute *attr,
>>                                            char *page)
>>   {
>> -       int pid_fmt = ETM_OPT_CTXTID;
>> +       /*
>> +        * is_kernel_in_hyp_mode() isn't defined in arm32 so avoid calling it
>> +        * even though is_visible() prevents this function from being called.
>> +        */
>> +#if IS_ENABLED(CONFIG_ARM64)
>> +       if (is_kernel_in_hyp_mode())
>> +               return contextid2_show(dev, attr, page);
>>
>> -#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>> -       pid_fmt = is_kernel_in_hyp_mode() ? ETM_OPT_CTXTID2 : ETM_OPT_CTXTID;
>> +       return contextid1_show(dev, attr, page);
>> +#else
>> +       WARN_ONCE(1, "ETM contextid not supported on arm32\n");
>> +       return 0;
> 
> Context ID is supported in aarch32 - and traced in etmv3 / ptm and etmv4.
> 
> Mike
> 

Not in Perf mode unless I'm missing something:

#define ETM3X_SUPPORTED_OPTIONS (ETMCR_CYC_ACC | \
				 ETMCR_TIMESTAMP_EN | \
				 ETMCR_RETURN_STACK)

Only these options are currently accepted. I suppose the comment is 
describing what the current behavior is, whether that is what we want is 
another thing.

But if we do start supporting context ID in ETMv3 we can update that 
comment.

>>   #endif
>> -       return sprintf(page, "config:%d\n", pid_fmt);
>>   }
>>
>>   static struct device_attribute format_attr_contextid =
>> @@ -337,7 +344,7 @@ static bool sinks_compatible(struct coresight_device *a,
>>   static void *etm_setup_aux(struct perf_event *event, void **pages,
>>                             int nr_pages, bool overwrite)
>>   {
>> -       u32 id, cfg_hash;
>> +       u32 sink_hash, cfg_hash;
>>          int cpu = event->cpu;
>>          cpumask_t *mask;
>>          struct coresight_device *sink = NULL;
>> @@ -350,13 +357,12 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
>>          INIT_WORK(&event_data->work, free_event_data);
>>
>>          /* First get the selected sink from user space. */
>> -       if (event->attr.config2 & GENMASK_ULL(31, 0)) {
>> -               id = (u32)event->attr.config2;
>> -               sink = user_sink = coresight_get_sink_by_id(id);
>> -       }
>> +       sink_hash = ATTR_CFG_GET_FLD(&event->attr, sinkid);
>> +       if (sink_hash)
>> +               sink = user_sink = coresight_get_sink_by_id(sink_hash);
>>
>>          /* check if user wants a coresight configuration selected */
>> -       cfg_hash = (u32)((event->attr.config2 & GENMASK_ULL(63, 32)) >> 32);
>> +       cfg_hash = ATTR_CFG_GET_FLD(&event->attr, configid);
>>          if (cfg_hash) {
>>                  if (cscfg_activate_config(cfg_hash))
>>                          goto err;
>>
>> --
>> 2.34.1
>>
> 
>