Timestamps are currently emitted at the maximum rate possible, which is
much too frequent for most use cases. Add an attribute to be able to set
the interval. Granular control is not required, so save space in the
config by interpreting it as 2 ^ ts_interval. And then 4 bits (0 - 15) is
enough to set the interval to be larger than the existing SYNC timestamp
interval.
No sysfs file is needed for this attribute because counter generated
timestamps are only configured for Perf mode.
Only show this attribute for ETM4x because timestamps aren't configured
in the same way for ETM3x. The attribute is only ever read in
coresight-etm4x-core.c.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 13 ++++++++++++-
drivers/hwtracing/coresight/coresight-etm4x-core.c | 21 ++++++++++++---------
drivers/hwtracing/coresight/coresight-etm4x.h | 6 ++++++
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index f677c08233ba..af937bbb933c 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -25,6 +25,11 @@
#include "coresight-syscfg.h"
#include "coresight-trace-id.h"
+#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
+#include <linux/perf/arm_pmu.h>
+#include "coresight-etm4x.h"
+#endif
+
static struct pmu etm_pmu;
static bool etm_perf_up;
@@ -69,7 +74,10 @@ PMU_FORMAT_ATTR(sinkid, "config2:0-31");
/* config ID - set if a system configuration is selected */
PMU_FORMAT_ATTR(configid, "config2:32-63");
PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
-
+#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
+/* Interval = (2 ^ ts_level) */
+GEN_PMU_FORMAT_ATTR(ts_level);
+#endif
/*
* contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
@@ -103,6 +111,9 @@ static struct attribute *etm_config_formats_attr[] = {
&format_attr_configid.attr,
&format_attr_branch_broadcast.attr,
&format_attr_cc_threshold.attr,
+#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
+ &format_attr_ts_level.attr,
+#endif
NULL,
};
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 1a2d02bdcb88..42277c201d4f 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -28,6 +28,7 @@
#include <linux/amba/bus.h>
#include <linux/seq_file.h>
#include <linux/uaccess.h>
+#include <linux/perf/arm_pmu.h>
#include <linux/perf_event.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
@@ -615,7 +616,7 @@ static void etm4_enable_hw_smp_call(void *info)
* +--------------+
* |
* +------v-------+
- * | Counter x | (reload to 1 on underflow)
+ * | Counter x | (reload to 2 ^ ts_level on underflow)
* +--------------+
* |
* +------v--------------+
@@ -626,11 +627,17 @@ static void etm4_enable_hw_smp_call(void *info)
* | Timestamp Generator | (timestamp on resource y)
* +----------------------+
*/
-static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
+static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata,
+ struct perf_event_attr *attr)
{
int ctridx;
int rselector;
struct etmv4_config *config = &drvdata->config;
+ u8 ts_level = ATTR_CFG_GET_FLD(attr, ts_level);
+
+ /* Disable when ts_level == MAX */
+ if (ts_level == FIELD_GET(ATTR_CFG_FLD_ts_level_MASK, UINT_MAX))
+ return 0;
/* No point in trying if we don't have at least one counter */
if (!drvdata->nr_cntr)
@@ -666,12 +673,8 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
return -ENOSPC;
}
- /*
- * Initialise original and reload counter value to the smallest
- * possible value in order to get as much precision as we can.
- */
- config->cntr_val[ctridx] = 1;
- config->cntrldvr[ctridx] = 1;
+ /* Initialise original and reload counter value. */
+ config->cntr_val[ctridx] = config->cntrldvr[ctridx] = 1 << ts_level;
/*
* Trace Counter Control Register TRCCNTCTLRn
@@ -761,7 +764,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
* order to correlate instructions executed on different CPUs
* (CPU-wide trace scenarios).
*/
- ret = etm4_config_timestamp_event(drvdata);
+ ret = etm4_config_timestamp_event(drvdata, attr);
/*
* No need to go further if timestamp intervals can't
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index aaa6633b2d67..54558de158fa 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -598,6 +598,12 @@
#define ETM_CNTR_MAX_VAL 0xFFFF
#define ETM_TRACEID_MASK 0x3f
+#define ATTR_CFG_FLD_ts_level_CFG config3
+#define ATTR_CFG_FLD_ts_level_LO 12
+#define ATTR_CFG_FLD_ts_level_HI 15
+#define ATTR_CFG_FLD_ts_level_MASK GENMASK(ATTR_CFG_FLD_ts_level_HI, \
+ ATTR_CFG_FLD_ts_level_LO)
+
/* ETMv4 programming modes */
#define ETM_MODE_EXCLUDE BIT(0)
#define ETM_MODE_LOAD BIT(1)
--
2.34.1On Thu, Aug 14, 2025 at 11:49:56AM +0100, James Clark wrote:
[...]
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -25,6 +25,11 @@
> #include "coresight-syscfg.h"
> #include "coresight-trace-id.h"
>
> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> +#include <linux/perf/arm_pmu.h>
> +#include "coresight-etm4x.h"
> +#endif
> +
> static struct pmu etm_pmu;
> static bool etm_perf_up;
>
> @@ -69,7 +74,10 @@ PMU_FORMAT_ATTR(sinkid, "config2:0-31");
> /* config ID - set if a system configuration is selected */
> PMU_FORMAT_ATTR(configid, "config2:32-63");
> PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
> -
> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> +/* Interval = (2 ^ ts_level) */
> +GEN_PMU_FORMAT_ATTR(ts_level);
> +#endif
>
> /*
> * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
> @@ -103,6 +111,9 @@ static struct attribute *etm_config_formats_attr[] = {
> &format_attr_configid.attr,
> &format_attr_branch_broadcast.attr,
> &format_attr_cc_threshold.attr,
> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> + &format_attr_ts_level.attr,
> +#endif
By using .visible() callback for attrs, we can improve a bit code
without spreading "#ifdef IS_ENABLED()" in this file. E.g.,
static umode_t format_attr_is_visible(struct kobject *kobj,
struct attribute *attr, int n)
{
struct device *dev = kobj_to_dev(kobj);
if (attr == &format_attr_ts_level.attr &&
!IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
return 0;
return attr->mode;
}
Otherwise, LGTM:
Reviewed-by: Leo Yan <leo.yan@arm.com>
> NULL,
> };
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 1a2d02bdcb88..42277c201d4f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -28,6 +28,7 @@
> #include <linux/amba/bus.h>
> #include <linux/seq_file.h>
> #include <linux/uaccess.h>
> +#include <linux/perf/arm_pmu.h>
> #include <linux/perf_event.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> @@ -615,7 +616,7 @@ static void etm4_enable_hw_smp_call(void *info)
> * +--------------+
> * |
> * +------v-------+
> - * | Counter x | (reload to 1 on underflow)
> + * | Counter x | (reload to 2 ^ ts_level on underflow)
> * +--------------+
> * |
> * +------v--------------+
> @@ -626,11 +627,17 @@ static void etm4_enable_hw_smp_call(void *info)
> * | Timestamp Generator | (timestamp on resource y)
> * +----------------------+
> */
> -static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
> +static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata,
> + struct perf_event_attr *attr)
> {
> int ctridx;
> int rselector;
> struct etmv4_config *config = &drvdata->config;
> + u8 ts_level = ATTR_CFG_GET_FLD(attr, ts_level);
> +
> + /* Disable when ts_level == MAX */
> + if (ts_level == FIELD_GET(ATTR_CFG_FLD_ts_level_MASK, UINT_MAX))
> + return 0;
>
> /* No point in trying if we don't have at least one counter */
> if (!drvdata->nr_cntr)
> @@ -666,12 +673,8 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
> return -ENOSPC;
> }
>
> - /*
> - * Initialise original and reload counter value to the smallest
> - * possible value in order to get as much precision as we can.
> - */
> - config->cntr_val[ctridx] = 1;
> - config->cntrldvr[ctridx] = 1;
> + /* Initialise original and reload counter value. */
> + config->cntr_val[ctridx] = config->cntrldvr[ctridx] = 1 << ts_level;
>
> /*
> * Trace Counter Control Register TRCCNTCTLRn
> @@ -761,7 +764,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> * order to correlate instructions executed on different CPUs
> * (CPU-wide trace scenarios).
> */
> - ret = etm4_config_timestamp_event(drvdata);
> + ret = etm4_config_timestamp_event(drvdata, attr);
>
> /*
> * No need to go further if timestamp intervals can't
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index aaa6633b2d67..54558de158fa 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -598,6 +598,12 @@
> #define ETM_CNTR_MAX_VAL 0xFFFF
> #define ETM_TRACEID_MASK 0x3f
>
> +#define ATTR_CFG_FLD_ts_level_CFG config3
> +#define ATTR_CFG_FLD_ts_level_LO 12
> +#define ATTR_CFG_FLD_ts_level_HI 15
> +#define ATTR_CFG_FLD_ts_level_MASK GENMASK(ATTR_CFG_FLD_ts_level_HI, \
> + ATTR_CFG_FLD_ts_level_LO)
> +
> /* ETMv4 programming modes */
> #define ETM_MODE_EXCLUDE BIT(0)
> #define ETM_MODE_LOAD BIT(1)
>
> --
> 2.34.1
>
On 30/09/2025 4:14 pm, Leo Yan wrote:
> On Thu, Aug 14, 2025 at 11:49:56AM +0100, James Clark wrote:
>
> [...]
>
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -25,6 +25,11 @@
>> #include "coresight-syscfg.h"
>> #include "coresight-trace-id.h"
>>
>> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>> +#include <linux/perf/arm_pmu.h>
>> +#include "coresight-etm4x.h"
>> +#endif
>> +
>> static struct pmu etm_pmu;
>> static bool etm_perf_up;
>>
>> @@ -69,7 +74,10 @@ PMU_FORMAT_ATTR(sinkid, "config2:0-31");
>> /* config ID - set if a system configuration is selected */
>> PMU_FORMAT_ATTR(configid, "config2:32-63");
>> PMU_FORMAT_ATTR(cc_threshold, "config3:0-11");
>> -
>> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>> +/* Interval = (2 ^ ts_level) */
>> +GEN_PMU_FORMAT_ATTR(ts_level);
>> +#endif
>>
>> /*
>> * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
>> @@ -103,6 +111,9 @@ static struct attribute *etm_config_formats_attr[] = {
>> &format_attr_configid.attr,
>> &format_attr_branch_broadcast.attr,
>> &format_attr_cc_threshold.attr,
>> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>> + &format_attr_ts_level.attr,
>> +#endif
>
> By using .visible() callback for attrs, we can improve a bit code
> without spreading "#ifdef IS_ENABLED()" in this file. E.g.,
>
> static umode_t format_attr_is_visible(struct kobject *kobj,
> struct attribute *attr, int n)
> {
> struct device *dev = kobj_to_dev(kobj);
>
> if (attr == &format_attr_ts_level.attr &&
> !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
> return 0;
>
> return attr->mode;
> }
>
> Otherwise, LGTM:
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
>
Unfortunately that won't work because you'd have to always include
coresight-etm4x.h. This file is compiled for both arm32 and arm64 so it
would break the arm32 build.
I could define the TTR_CFG_FLD_ts_level_* stuff somewhere else but then
it becomes messier than just doing the #ifdefs here.
>> NULL,
>> };
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 1a2d02bdcb88..42277c201d4f 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -28,6 +28,7 @@
>> #include <linux/amba/bus.h>
>> #include <linux/seq_file.h>
>> #include <linux/uaccess.h>
>> +#include <linux/perf/arm_pmu.h>
>> #include <linux/perf_event.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> @@ -615,7 +616,7 @@ static void etm4_enable_hw_smp_call(void *info)
>> * +--------------+
>> * |
>> * +------v-------+
>> - * | Counter x | (reload to 1 on underflow)
>> + * | Counter x | (reload to 2 ^ ts_level on underflow)
>> * +--------------+
>> * |
>> * +------v--------------+
>> @@ -626,11 +627,17 @@ static void etm4_enable_hw_smp_call(void *info)
>> * | Timestamp Generator | (timestamp on resource y)
>> * +----------------------+
>> */
>> -static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
>> +static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata,
>> + struct perf_event_attr *attr)
>> {
>> int ctridx;
>> int rselector;
>> struct etmv4_config *config = &drvdata->config;
>> + u8 ts_level = ATTR_CFG_GET_FLD(attr, ts_level);
>> +
>> + /* Disable when ts_level == MAX */
>> + if (ts_level == FIELD_GET(ATTR_CFG_FLD_ts_level_MASK, UINT_MAX))
>> + return 0;
>>
>> /* No point in trying if we don't have at least one counter */
>> if (!drvdata->nr_cntr)
>> @@ -666,12 +673,8 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
>> return -ENOSPC;
>> }
>>
>> - /*
>> - * Initialise original and reload counter value to the smallest
>> - * possible value in order to get as much precision as we can.
>> - */
>> - config->cntr_val[ctridx] = 1;
>> - config->cntrldvr[ctridx] = 1;
>> + /* Initialise original and reload counter value. */
>> + config->cntr_val[ctridx] = config->cntrldvr[ctridx] = 1 << ts_level;
>>
>> /*
>> * Trace Counter Control Register TRCCNTCTLRn
>> @@ -761,7 +764,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>> * order to correlate instructions executed on different CPUs
>> * (CPU-wide trace scenarios).
>> */
>> - ret = etm4_config_timestamp_event(drvdata);
>> + ret = etm4_config_timestamp_event(drvdata, attr);
>>
>> /*
>> * No need to go further if timestamp intervals can't
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index aaa6633b2d67..54558de158fa 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -598,6 +598,12 @@
>> #define ETM_CNTR_MAX_VAL 0xFFFF
>> #define ETM_TRACEID_MASK 0x3f
>>
>> +#define ATTR_CFG_FLD_ts_level_CFG config3
>> +#define ATTR_CFG_FLD_ts_level_LO 12
>> +#define ATTR_CFG_FLD_ts_level_HI 15
>> +#define ATTR_CFG_FLD_ts_level_MASK GENMASK(ATTR_CFG_FLD_ts_level_HI, \
>> + ATTR_CFG_FLD_ts_level_LO)
>> +
>> /* ETMv4 programming modes */
>> #define ETM_MODE_EXCLUDE BIT(0)
>> #define ETM_MODE_LOAD BIT(1)
>>
>> --
>> 2.34.1
>>
On Wed, Oct 01, 2025 at 01:40:37PM +0100, James Clark wrote:
[...]
> > > @@ -103,6 +111,9 @@ static struct attribute *etm_config_formats_attr[] = {
> > > &format_attr_configid.attr,
> > > &format_attr_branch_broadcast.attr,
> > > &format_attr_cc_threshold.attr,
> > > +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > > + &format_attr_ts_level.attr,
> > > +#endif
> >
> > By using .visible() callback for attrs, we can improve a bit code
> > without spreading "#ifdef IS_ENABLED()" in this file. E.g.,
> >
> > static umode_t format_attr_is_visible(struct kobject *kobj,
> > struct attribute *attr, int n)
> > {
> > struct device *dev = kobj_to_dev(kobj);
> >
> > if (attr == &format_attr_ts_level.attr &&
> > !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
> > return 0;
> >
> > return attr->mode;
> > }
> >
> > Otherwise, LGTM:
> >
> > Reviewed-by: Leo Yan <leo.yan@arm.com>
> >
>
> Unfortunately that won't work because you'd have to always include
> coresight-etm4x.h. This file is compiled for both arm32 and arm64 so it
> would break the arm32 build.
>
> I could define the TTR_CFG_FLD_ts_level_* stuff somewhere else but then it
> becomes messier than just doing the #ifdefs here.
ATTR_CFG_FLD_ts_level_* is only used in coresight-etm4x-core.c, it is not
used in coresight-etm-perf.c. Thus, we don't need to include
coresight-etm4x.h in coresight-etm-perf.c. Do I miss anything?
A similiar case is the attr 'cc_threshold' is only used by ETMv4, it is
exported always. It is not bad for me to always expose these attrs but
in the are ignored in the ETMv3 driver - so we even don't need to
bother adding .visible() callback.
Thanks,
Leo
On 01/10/2025 2:28 pm, Leo Yan wrote:
> On Wed, Oct 01, 2025 at 01:40:37PM +0100, James Clark wrote:
>
> [...]
>
>>>> @@ -103,6 +111,9 @@ static struct attribute *etm_config_formats_attr[] = {
>>>> &format_attr_configid.attr,
>>>> &format_attr_branch_broadcast.attr,
>>>> &format_attr_cc_threshold.attr,
>>>> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>>>> + &format_attr_ts_level.attr,
>>>> +#endif
>>>
>>> By using .visible() callback for attrs, we can improve a bit code
>>> without spreading "#ifdef IS_ENABLED()" in this file. E.g.,
>>>
>>> static umode_t format_attr_is_visible(struct kobject *kobj,
>>> struct attribute *attr, int n)
>>> {
>>> struct device *dev = kobj_to_dev(kobj);
>>>
>>> if (attr == &format_attr_ts_level.attr &&
>>> !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
>>> return 0;
>>>
>>> return attr->mode;
>>> }
>>>
>>> Otherwise, LGTM:
>>>
>>> Reviewed-by: Leo Yan <leo.yan@arm.com>
>>>
>>
>> Unfortunately that won't work because you'd have to always include
>> coresight-etm4x.h. This file is compiled for both arm32 and arm64 so it
>> would break the arm32 build.
>>
>> I could define the TTR_CFG_FLD_ts_level_* stuff somewhere else but then it
>> becomes messier than just doing the #ifdefs here.
>
> ATTR_CFG_FLD_ts_level_* is only used in coresight-etm4x-core.c, it is not
> used in coresight-etm-perf.c. Thus, we don't need to include
> coresight-etm4x.h in coresight-etm-perf.c. Do I miss anything?
Yes, GEN_PMU_FORMAT_ATTR() uses them but it makes it hard to see.
>
> A similiar case is the attr 'cc_threshold' is only used by ETMv4, it is
> exported always. It is not bad for me to always expose these attrs but
> in the are ignored in the ETMv3 driver - so we even don't need to
> bother adding .visible() callback.
>
I disagree with always showing them. I think they should be hidden if
they're not used, or at least return an error to avoid confusing users.
It also wastes config bits if they're allocated but never used.
Either way, this was done because of the header mechanics which can only
be avoided by adding more changes than just the #ifdefs. There are also
already ETM4 #ifdefs in the file.
> Thanks,
> Leo
On Wed, Oct 01, 2025 at 02:44:06PM +0100, James Clark wrote:
[...]
> > ATTR_CFG_FLD_ts_level_* is only used in coresight-etm4x-core.c, it is not
> > used in coresight-etm-perf.c. Thus, we don't need to include
> > coresight-etm4x.h in coresight-etm-perf.c. Do I miss anything?
>
> Yes, GEN_PMU_FORMAT_ATTR() uses them but it makes it hard to see.
I did a quick test, it is feasible to move ATTR_CFG_* macros in
coresight-etm-perf.h. This is a more suitable ?
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
index 5febbcdb8696..2679d5b2dd9a 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.h
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
@@ -8,6 +8,7 @@
#define _CORESIGHT_ETM_PERF_H
#include <linux/percpu-defs.h>
+#include <linux/perf/arm_pmu.h>
#include "coresight-priv.h"
struct coresight_device;
@@ -20,6 +21,12 @@ struct cscfg_config_desc;
*/
#define ETM_ADDR_CMP_MAX 8
+#define ATTR_CFG_FLD_ts_level_CFG config3
+#define ATTR_CFG_FLD_ts_level_LO 12
+#define ATTR_CFG_FLD_ts_level_HI 15
+#define ATTR_CFG_FLD_ts_level_MASK GENMASK(ATTR_CFG_FLD_ts_level_HI, \
+ ATTR_CFG_FLD_ts_level_LO)
+
> > A similiar case is the attr 'cc_threshold' is only used by ETMv4, it is
> > exported always. It is not bad for me to always expose these attrs but
> > in the are ignored in the ETMv3 driver - so we even don't need to
> > bother adding .visible() callback.
> >
>
> I disagree with always showing them. I think they should be hidden if
> they're not used, or at least return an error to avoid confusing users. It
> also wastes config bits if they're allocated but never used.
It is fine for not exposing ETMv4 only attrs for ETMv3.
> Either way, this was done because of the header mechanics which can only be
> avoided by adding more changes than just the #ifdefs. There are also already
> ETM4 #ifdefs in the file.
Yeah, actually we can remove ETM4 #ifdefs, something like:
/*
* contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
@@ -90,9 +83,9 @@ static ssize_t format_attr_contextid_show(struct device *dev,
{
int pid_fmt = ETM_OPT_CTXTID;
-#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
- pid_fmt = is_kernel_in_hyp_mode() ? ETM_OPT_CTXTID2 : ETM_OPT_CTXTID;
-#endif
+ if (IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
+ pid_fmt = is_kernel_in_hyp_mode() ? ETM_OPT_CTXTID2 : ;
+
return sprintf(page, "config:%d\n", pid_fmt);
}
Thanks,
Leo
On Wed, Oct 01, 2025 at 02:28:15PM +0100, Coresight ML wrote: [...] > > Unfortunately that won't work because you'd have to always include > > coresight-etm4x.h. This file is compiled for both arm32 and arm64 so it > > would break the arm32 build. > > > > I could define the TTR_CFG_FLD_ts_level_* stuff somewhere else but then it > > becomes messier than just doing the #ifdefs here. > > ATTR_CFG_FLD_ts_level_* is only used in coresight-etm4x-core.c, it is not > used in coresight-etm-perf.c. Thus, we don't need to include > coresight-etm4x.h in coresight-etm-perf.c. Do I miss anything? Now I understand that you are using GEN_PMU_FORMAT_ATTR, so need to used TTR_CFG_FLD_ts_level_* macro defined in coresight-etm4x.h. Thanks, Leo
© 2016 - 2025 Red Hat, Inc.