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.
Reviewed-by: Leo Yan <leo.yan@arm.com>
Tested-by: Leo Yan <leo.yan@arm.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 16 +++++++++++++++-
drivers/hwtracing/coresight/coresight-etm-perf.h | 7 +++++++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 21 ++++++++++++---------
3 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index f677c08233ba..0c1b990fc56e 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -13,6 +13,7 @@
#include <linux/mm.h>
#include <linux/init.h>
#include <linux/perf_event.h>
+#include <linux/perf/arm_pmu.h>
#include <linux/percpu-defs.h>
#include <linux/slab.h>
#include <linux/stringhash.h>
@@ -69,7 +70,8 @@ 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");
-
+/* Interval = (2 ^ ts_level) */
+GEN_PMU_FORMAT_ATTR(ts_level);
/*
* contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
@@ -103,11 +105,23 @@ static struct attribute *etm_config_formats_attr[] = {
&format_attr_configid.attr,
&format_attr_branch_broadcast.attr,
&format_attr_cc_threshold.attr,
+ &format_attr_ts_level.attr,
NULL,
};
+static umode_t etm_format_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int unused)
+{
+ if (attr == &format_attr_ts_level.attr &&
+ !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
+ return 0;
+
+ return attr->mode;
+}
+
static const struct attribute_group etm_pmu_format_group = {
.name = "format",
+ .is_visible = etm_format_attr_is_visible,
.attrs = etm_config_formats_attr,
};
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
index 5febbcdb8696..d2664ffb33e5 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.h
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
@@ -7,6 +7,7 @@
#ifndef _CORESIGHT_ETM_PERF_H
#define _CORESIGHT_ETM_PERF_H
+#include <linux/bits.h>
#include <linux/percpu-defs.h>
#include "coresight-priv.h"
@@ -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)
+
/**
* struct etm_filter - single instruction range or start/stop configuration.
* @start_addr: The address to start tracing on.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 920d092ef862..034844f52bb2 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>
@@ -616,7 +617,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--------------+
@@ -627,11 +628,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)
@@ -667,12 +674,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
@@ -762,7 +765,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
--
2.34.1Hi James
On Thu, 2 Oct 2025 at 11:10, James Clark <james.clark@linaro.org> wrote:
>
> 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.
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> Tested-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-etm-perf.c | 16 +++++++++++++++-
> drivers/hwtracing/coresight/coresight-etm-perf.h | 7 +++++++
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 21 ++++++++++++---------
> 3 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index f677c08233ba..0c1b990fc56e 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -13,6 +13,7 @@
> #include <linux/mm.h>
> #include <linux/init.h>
> #include <linux/perf_event.h>
> +#include <linux/perf/arm_pmu.h>
> #include <linux/percpu-defs.h>
> #include <linux/slab.h>
> #include <linux/stringhash.h>
> @@ -69,7 +70,8 @@ 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");
> -
> +/* Interval = (2 ^ ts_level) */
> +GEN_PMU_FORMAT_ATTR(ts_level);
>
> /*
> * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
> @@ -103,11 +105,23 @@ static struct attribute *etm_config_formats_attr[] = {
> &format_attr_configid.attr,
> &format_attr_branch_broadcast.attr,
> &format_attr_cc_threshold.attr,
> + &format_attr_ts_level.attr,
> NULL,
> };
>
> +static umode_t etm_format_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int unused)
> +{
> + if (attr == &format_attr_ts_level.attr &&
> + !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> static const struct attribute_group etm_pmu_format_group = {
> .name = "format",
> + .is_visible = etm_format_attr_is_visible,
> .attrs = etm_config_formats_attr,
> };
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
> index 5febbcdb8696..d2664ffb33e5 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.h
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
> @@ -7,6 +7,7 @@
> #ifndef _CORESIGHT_ETM_PERF_H
> #define _CORESIGHT_ETM_PERF_H
>
> +#include <linux/bits.h>
> #include <linux/percpu-defs.h>
> #include "coresight-priv.h"
>
> @@ -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)
> +
> /**
> * struct etm_filter - single instruction range or start/stop configuration.
> * @start_addr: The address to start tracing on.
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 920d092ef862..034844f52bb2 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>
> @@ -616,7 +617,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--------------+
> @@ -627,11 +628,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;
>
Returning 0 from this function _enables_ the timestamps
> /* No point in trying if we don't have at least one counter */
> if (!drvdata->nr_cntr)
> @@ -667,12 +674,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
> @@ -762,7 +765,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
>
> --
> 2.34.1
>
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
On 09/10/2025 4:50 pm, Mike Leach wrote:
> Hi James
>
> On Thu, 2 Oct 2025 at 11:10, James Clark <james.clark@linaro.org> wrote:
>>
>> 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.
>>
>> Reviewed-by: Leo Yan <leo.yan@arm.com>
>> Tested-by: Leo Yan <leo.yan@arm.com>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> drivers/hwtracing/coresight/coresight-etm-perf.c | 16 +++++++++++++++-
>> drivers/hwtracing/coresight/coresight-etm-perf.h | 7 +++++++
>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 21 ++++++++++++---------
>> 3 files changed, 34 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index f677c08233ba..0c1b990fc56e 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -13,6 +13,7 @@
>> #include <linux/mm.h>
>> #include <linux/init.h>
>> #include <linux/perf_event.h>
>> +#include <linux/perf/arm_pmu.h>
>> #include <linux/percpu-defs.h>
>> #include <linux/slab.h>
>> #include <linux/stringhash.h>
>> @@ -69,7 +70,8 @@ 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");
>> -
>> +/* Interval = (2 ^ ts_level) */
>> +GEN_PMU_FORMAT_ATTR(ts_level);
>>
>> /*
>> * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
>> @@ -103,11 +105,23 @@ static struct attribute *etm_config_formats_attr[] = {
>> &format_attr_configid.attr,
>> &format_attr_branch_broadcast.attr,
>> &format_attr_cc_threshold.attr,
>> + &format_attr_ts_level.attr,
>> NULL,
>> };
>>
>> +static umode_t etm_format_attr_is_visible(struct kobject *kobj,
>> + struct attribute *attr, int unused)
>> +{
>> + if (attr == &format_attr_ts_level.attr &&
>> + !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
>> + return 0;
>> +
>> + return attr->mode;
>> +}
>> +
>> static const struct attribute_group etm_pmu_format_group = {
>> .name = "format",
>> + .is_visible = etm_format_attr_is_visible,
>> .attrs = etm_config_formats_attr,
>> };
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
>> index 5febbcdb8696..d2664ffb33e5 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
>> @@ -7,6 +7,7 @@
>> #ifndef _CORESIGHT_ETM_PERF_H
>> #define _CORESIGHT_ETM_PERF_H
>>
>> +#include <linux/bits.h>
>> #include <linux/percpu-defs.h>
>> #include "coresight-priv.h"
>>
>> @@ -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)
>> +
>> /**
>> * struct etm_filter - single instruction range or start/stop configuration.
>> * @start_addr: The address to start tracing on.
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 920d092ef862..034844f52bb2 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>
>> @@ -616,7 +617,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--------------+
>> @@ -627,11 +628,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;
>>
>
> Returning 0 from this function _enables_ the timestamps
>
Returning 0 just means that etm4_parse_event_config() doesn't exit with
an error. For ts_level == MAX we want to disable timestamps generated by
the counter, but we still want the minimum periodic timestamps.
To disable all timestamps you'd need to have attr->config &
BIT(ETM_OPT_TS) == false. This is set by the "timestamp" format flag
which I tried to explain that in the docs change.
I could also change the comment to say "/* Disable counter generated
timestamps with ts_level == MAX */"
It's unfortunate that there are now two format options for timestamps.
Maybe instead of adding a second option we can change "timestamp" from a
1 bit field to 4 bits, with the following meanings:
0: No counter timestamps or SYNC timestamps
1-14: Counter timestamps = 2 ^ x. Plus SYNC timestamps
15: Only SYNC timestamps
Now we basically have the same meanings except you also have to set the
timestamp bit. Seems a bit pointless.
Previous versions of Perf were hard coding the timestamp format bit
rather than reading it out of
"/sys/bus/event_source/devices/cs_etm/format/timestamp" though:
- /* All good, let the kernel know */
- evsel->core.attr.config |= (1 << ETM_OPT_TS);
For that reason we'd have to leave that one where it is for backwards
compatibility. If it's set it would be equivalent to the new wider
timestamp field == 1.
I don't know if there's any precedent for changing the bitfield that
backs a format field, but presumably that's the point of publishing them
in files rather than a header.
>> /* No point in trying if we don't have at least one counter */
>> if (!drvdata->nr_cntr)
>> @@ -667,12 +674,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
>> @@ -762,7 +765,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
>>
>> --
>> 2.34.1
>>
>
> Regards
>
>
> Mike
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
On Wed, Oct 15, 2025 at 04:19:04PM +0100, James Clark wrote:
[...]
> > > -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;
> > >
> >
> > Returning 0 from this function _enables_ the timestamps
> >
>
> Returning 0 just means that etm4_parse_event_config() doesn't exit with an
> error. For ts_level == MAX we want to disable timestamps generated by the
> counter, but we still want the minimum periodic timestamps.
>
> To disable all timestamps you'd need to have attr->config & BIT(ETM_OPT_TS)
> == false. This is set by the "timestamp" format flag which I tried to
> explain that in the docs change.
>
> I could also change the comment to say "/* Disable counter generated
> timestamps with ts_level == MAX */"
>
> It's unfortunate that there are now two format options for timestamps. Maybe
> instead of adding a second option we can change "timestamp" from a 1 bit
> field to 4 bits, with the following meanings:
>
> 0: No counter timestamps or SYNC timestamps
> 1-14: Counter timestamps = 2 ^ x. Plus SYNC timestamps
> 15: Only SYNC timestamps
I am just wandering how can extend "timestamp" from 1 bit to 4 bits.
#define ETM_OPT_TS 28
#define ETM_OPT_RETSTK 29
PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS));
PMU_FORMAT_ATTR(retstack, "config:" __stringify(ETM_OPT_RETSTK));
"retstack" has occupied a higher bit, we cannot naturelly extend
"timestamp" field?
Even we can extend "timestamp" format to 4 bits, it will be mess when
run the updated perf on old kernels. Let's see an example:
perf record -e cs_etm/timestamp=0/ -- test
perf record -e cs_etm/timestamp=2/ -- test
Because the lowest bit is cleared for both timestamp=0 and timestamp=2,
the old kernel support only one bit always treats these two setting as
timestamp disabled, or the perf tool needs to do extra checking for
old kernel.
> Now we basically have the same meanings except you also have to set the
> timestamp bit. Seems a bit pointless.
> Previous versions of Perf were hard coding the timestamp format bit rather
> than reading it out of
> "/sys/bus/event_source/devices/cs_etm/format/timestamp" though:
>
> - /* All good, let the kernel know */
> - evsel->core.attr.config |= (1 << ETM_OPT_TS);
>
> For that reason we'd have to leave that one where it is for backwards
> compatibility. If it's set it would be equivalent to the new wider timestamp
> field == 1.
Are you suggesting the timestamp field to be extended to two
non-consecutive fields?
For me, this is even worse than current two discrete formats. The reason
is it is complex in implementation, and it is not directive for usage
(users need to digest the field for three different semantics: on/off,
counter, and SYNC mode only).
Thanks,
Leo
> I don't know if there's any precedent for changing the bitfield that backs a
> format field, but presumably that's the point of publishing them in files
> rather than a header.
>
> > > /* No point in trying if we don't have at least one counter */
> > > if (!drvdata->nr_cntr)
> > > @@ -667,12 +674,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
> > > @@ -762,7 +765,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
> > >
> > > --
> > > 2.34.1
> > >
> >
> > Regards
> >
> >
> > Mike
> >
> > --
> > Mike Leach
> > Principal Engineer, ARM Ltd.
> > Manchester Design Centre. UK
>
On 21/10/2025 6:22 pm, Leo Yan wrote:
> On Wed, Oct 15, 2025 at 04:19:04PM +0100, James Clark wrote:
>
> [...]
>
>>>> -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;
>>>>
>>>
>>> Returning 0 from this function _enables_ the timestamps
>>>
>>
>> Returning 0 just means that etm4_parse_event_config() doesn't exit with an
>> error. For ts_level == MAX we want to disable timestamps generated by the
>> counter, but we still want the minimum periodic timestamps.
>>
>> To disable all timestamps you'd need to have attr->config & BIT(ETM_OPT_TS)
>> == false. This is set by the "timestamp" format flag which I tried to
>> explain that in the docs change.
>>
>> I could also change the comment to say "/* Disable counter generated
>> timestamps with ts_level == MAX */"
>>
>> It's unfortunate that there are now two format options for timestamps. Maybe
>> instead of adding a second option we can change "timestamp" from a 1 bit
>> field to 4 bits, with the following meanings:
>>
>> 0: No counter timestamps or SYNC timestamps
>> 1-14: Counter timestamps = 2 ^ x. Plus SYNC timestamps
>> 15: Only SYNC timestamps
>
> I am just wandering how can extend "timestamp" from 1 bit to 4 bits.
>
> #define ETM_OPT_TS 28
> #define ETM_OPT_RETSTK 29
>
> PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS));
> PMU_FORMAT_ATTR(retstack, "config:" __stringify(ETM_OPT_RETSTK));
>
> "retstack" has occupied a higher bit, we cannot naturelly extend
> "timestamp" field?
Easy, just put it wherever there is a hole. I think there's one in
"config:4-7", but it could be put in config3 or config4:
/* Old enable timestamp bit for backwards compatibility */
#define ETM_OPT_TS_old 28
PMU_FORMAT_ATTR(timestamp, "config:4-7");
The position of the timestamp field is published and tools read it, so
as long as we don't change the name of it it works fine.
We only keep the old bit 28 because we know old Perf was buggy and
didn't read the bit position, but if it wasn't we wouldn't even need to
do that.
>
> Even we can extend "timestamp" format to 4 bits, it will be mess when
> run the updated perf on old kernels. Let's see an example:
>
> perf record -e cs_etm/timestamp=0/ -- test
> perf record -e cs_etm/timestamp=2/ -- test
>
> Because the lowest bit is cleared for both timestamp=0 and timestamp=2,
> the old kernel support only one bit always treats these two setting as
> timestamp disabled, or the perf tool needs to do extra checking for
> old kernel.
It won't be. Old kernels report a 1 bit field and Perf errors out if you
try to put a 2 into 1 bit. Very old Perfs just set bit 28 which new and
old kernels will respect.
>
>> Now we basically have the same meanings except you also have to set the
>> timestamp bit. Seems a bit pointless.
>
>> Previous versions of Perf were hard coding the timestamp format bit rather
>> than reading it out of
>> "/sys/bus/event_source/devices/cs_etm/format/timestamp" though:
>>
>> - /* All good, let the kernel know */
>> - evsel->core.attr.config |= (1 << ETM_OPT_TS);
>>
>> For that reason we'd have to leave that one where it is for backwards
>> compatibility. If it's set it would be equivalent to the new wider timestamp
>> field == 1.
>
> Are you suggesting the timestamp field to be extended to two
> non-consecutive fields?
No, just a new 4 bit field in a new position.
>
> For me, this is even worse than current two discrete formats. The reason
> is it is complex in implementation, and it is not directive for usage
I really don't think the implementation is complex. We just extend the
field to 4 bits and make 0 off, max is the lowest rate possible, and
every other value in between is in between.
> (users need to digest the field for three different semantics: on/off,
> counter, and SYNC mode only).
>
That's like any enum, it has multiple meanings for each value. I'd argue
that two fields for the same thing is more complicated because now this
won't work out of the box, and it would work if we did 1 field:
perf record -e cs_etm/ts_level=2/ -- test
There will be no warning, but no timestamps either. You have to specify
both manually. And the only reason for that awkwardness in the API is
that we added a new field instead of extending the existing one:
perf record -e cs_etm/ts_level=2,timestamp/ -- test
> Thanks,
> Leo
>
>> I don't know if there's any precedent for changing the bitfield that backs a
>> format field, but presumably that's the point of publishing them in files
>> rather than a header.
>
>
>>
>>>> /* No point in trying if we don't have at least one counter */
>>>> if (!drvdata->nr_cntr)
>>>> @@ -667,12 +674,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
>>>> @@ -762,7 +765,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
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> Regards
>>>
>>>
>>> Mike
>>>
>>> --
>>> Mike Leach
>>> Principal Engineer, ARM Ltd.
>>> Manchester Design Centre. UK
>>
Hi James
On Thu, 2 Oct 2025 at 11:10, James Clark <james.clark@linaro.org> wrote:
>
> 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.
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> Tested-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-etm-perf.c | 16 +++++++++++++++-
> drivers/hwtracing/coresight/coresight-etm-perf.h | 7 +++++++
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 21 ++++++++++++---------
> 3 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index f677c08233ba..0c1b990fc56e 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -13,6 +13,7 @@
> #include <linux/mm.h>
> #include <linux/init.h>
> #include <linux/perf_event.h>
> +#include <linux/perf/arm_pmu.h>
> #include <linux/percpu-defs.h>
> #include <linux/slab.h>
> #include <linux/stringhash.h>
> @@ -69,7 +70,8 @@ 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");
> -
> +/* Interval = (2 ^ ts_level) */
> +GEN_PMU_FORMAT_ATTR(ts_level);
>
> /*
> * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
> @@ -103,11 +105,23 @@ static struct attribute *etm_config_formats_attr[] = {
> &format_attr_configid.attr,
> &format_attr_branch_broadcast.attr,
> &format_attr_cc_threshold.attr,
> + &format_attr_ts_level.attr,
> NULL,
> };
>
> +static umode_t etm_format_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int unused)
> +{
> + if (attr == &format_attr_ts_level.attr &&
> + !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> static const struct attribute_group etm_pmu_format_group = {
> .name = "format",
> + .is_visible = etm_format_attr_is_visible,
> .attrs = etm_config_formats_attr,
> };
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
> index 5febbcdb8696..d2664ffb33e5 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.h
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
> @@ -7,6 +7,7 @@
> #ifndef _CORESIGHT_ETM_PERF_H
> #define _CORESIGHT_ETM_PERF_H
>
> +#include <linux/bits.h>
> #include <linux/percpu-defs.h>
> #include "coresight-priv.h"
>
> @@ -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)
> +
> /**
> * struct etm_filter - single instruction range or start/stop configuration.
> * @start_addr: The address to start tracing on.
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 920d092ef862..034844f52bb2 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>
> @@ -616,7 +617,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--------------+
> @@ -627,11 +628,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)
> @@ -667,12 +674,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
> @@ -762,7 +765,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
>
> --
> 2.34.1
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
© 2016 - 2026 Red Hat, Inc.