[PATCH v7 00/13] coresight: Update timestamp attribute to be an interval instead of bool
Do some cleanups then expand the timestamp format attribute from 1 bit to 4 bits for ETMv4 in Perf mode. The current interval is too high for most use cases, and particularly on the FVP the number of timestamps generated is excessive. This change not only still allows disabling or enabling timestamps, but also allows the interval to be configured.
The old bit is kept deprecated and undocumented for now. There are known broken versions of Perf that don't read the format attribute positions from sysfs and instead hard code the timestamp bit. We can leave the old bit in the driver until we need the bit for another feature or enough time has passed that these old Perfs are unlikely to be used.
The interval option is added as an event format attribute, rather than a Coresight config because it's something that the driver is already configuring automatically in Perf mode using any unused counter, so it's not possible to modify this with a config.
Applies to coresight/next
Signed-off-by: James Clark james.clark@linaro.org --- Changes in v7: - Change TRCCNTCTLRn and TRCTSCTLR register definitions to use a combined field for TYPE and SEL (EVENT) so that they can be used with the new utilities. - Add utility functions for creating resource selectors that do compile and runtime checking of the resource selector ID. - Link to v6: https://lore.kernel.org/r/20251119-james-cs-syncfreq-v6-0-740d24a29e51@linar...
Changes in v6: - #ifdef out format attributes for ETMv3 instead of using is_visible(). Then the same block can be used to define format_attr_contextid_show() which avoids an awkward WARN_ONCE() and comments in arm32 for a function that's never called. - Link to v5: https://lore.kernel.org/r/20251118-james-cs-syncfreq-v5-0-82efd7b1a751@linar...
Changes in v5: - Add parens to interval calculation in docs (Randy) - Swap "minimum interval" and "maximum interval" in docs. (Leo) - Add TRCSYNCPR.PERIOD to docs (Leo) - Use CONFIG_ARM64 to avoid is_kernel_in_hyp_mode() (Leo) - Add a comment for hidden ETMv3 format attributes (Leo) - Hide configid for ETMv3 (Leo) - Link to v4: https://lore.kernel.org/r/20251112-james-cs-syncfreq-v4-0-165ba21401dc@linar...
Changes in v4: - Add #defines for true and false resources ETM_RES_SEL_TRUE/FALSE - Reword comment about finding a counter to say if there are no resources there are no counters. - Extend existing timestamp format attribute instead of adding a new one - Refactor all the config definitions and parsing to use GEN_PMU_FORMAT_ATTR()/ATTR_CFG_GET_FLD() so we can see where the unused bits are. - Link to v3: https://lore.kernel.org/r/20251002-james-cs-syncfreq-v3-0-fe5df2bf91d1@linar...
Changes in v3: - Move the format attr definitions to coresight-etm-perf.h we can compile on arm32 without #ifdefs - (Leo) - Convert the new #ifdefs to a single one in an is_visible() function so that the code is cleaner - (Leo) - Drop the change to remove the holes in struct etmv4_config as they were grouped by function - (Mike) - Link to v2: https://lore.kernel.org/r/20250814-james-cs-syncfreq-v2-0-c76fcb87696d@linar...
Changes in v2: - Only show the attribute for ETMv4 to improve usability and fix the arm32 build error. Wrapping everything in IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X) isn't ideal, but the -perf.c file is shared between ETMv3 and ETMv4, and there is already precedent for doing it this way. - Link to v1: https://lore.kernel.org/r/20250811-james-cs-syncfreq-v1-0-b001cd6e3404@linar...
--- James Clark (13): coresight: Change syncfreq to be a u8 coresight: Repack struct etmv4_drvdata coresight: Refactor etm4_config_timestamp_event() coresight: Hide unused ETMv3 format attributes coresight: Define format attributes with GEN_PMU_FORMAT_ATTR() coresight: Interpret ETMv3 config with ATTR_CFG_GET_FLD() coresight: Don't reject unrecognized ETMv3 format attributes coresight: Interpret perf config with ATTR_CFG_GET_FLD() coresight: Interpret ETMv4 config with ATTR_CFG_GET_FLD() coresight: Remove misleading definitions coresight: Extend width of timestamp format attribute coresight: Allow setting the timestamp interval coresight: docs: Document etm4x timestamp interval option
Documentation/trace/coresight/coresight.rst | 16 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 68 ++++----- drivers/hwtracing/coresight/coresight-etm-perf.h | 39 +++++ drivers/hwtracing/coresight/coresight-etm3x-core.c | 36 ++--- drivers/hwtracing/coresight/coresight-etm4x-core.c | 164 +++++++++++++-------- drivers/hwtracing/coresight/coresight-etm4x.h | 92 +++++++++--- include/linux/coresight-pmu.h | 24 --- 7 files changed, 275 insertions(+), 164 deletions(-) --- base-commit: 9e9182cab5ebc3ee7544e60ef08ba19fdf216920 change-id: 20250724-james-cs-syncfreq-7c2257a38ed3
Best regards,
Fix holes and convert the long list of bools to single bits to save some space because there's one of these for each ETM.
Reviewed-by: Leo Yan leo.yan@arm.com Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm4x.h | 36 +++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 0287d19ce12e..d178d79d9827 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -1016,27 +1016,27 @@ struct etmv4_drvdata { u8 ns_ex_level; u8 q_support; u8 os_lock_model; - bool sticky_enable; - bool boot_enable; - bool os_unlock; - bool instrp0; - bool q_filt; - bool trcbb; - bool trccond; - bool retstack; - bool trccci; - bool trc_error; - bool syncpr; - bool stallctl; - bool sysstall; - bool nooverflow; - bool atbtrig; - bool lpoverride; + bool sticky_enable : 1; + bool boot_enable : 1; + bool os_unlock : 1; + bool instrp0 : 1; + bool q_filt : 1; + bool trcbb : 1; + bool trccond : 1; + bool retstack : 1; + bool trccci : 1; + bool trc_error : 1; + bool syncpr : 1; + bool stallctl : 1; + bool sysstall : 1; + bool nooverflow : 1; + bool atbtrig : 1; + bool lpoverride : 1; + bool skip_power_up : 1; + bool paused : 1; u64 trfcr; struct etmv4_config config; struct etmv4_save_state *save_state; - bool skip_power_up; - bool paused; DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX); };
Remove some of the magic numbers and try to clarify some of the documentation so it's clearer how this sets up the timestamp interval.
Return errors directly instead of jumping to out and returning ret, nothing needs to be cleaned up at the end and it only obscures the flow and return value.
Add utilities for programming resource selectors that do compile time checks for constants or WARN_ONs for non-constant values. FIELD_PREP includes compile time checks so we only need to add an additional BUILD_BUG_ON for resource == 0 in pair mode.
Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 96 ++++++++++++++-------- drivers/hwtracing/coresight/coresight-etm4x.h | 54 ++++++++++-- 2 files changed, 112 insertions(+), 38 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 560975b70474..2ec2ae1fef58 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -642,18 +642,33 @@ static void etm4_enable_sysfs_smp_call(void *info) * TRCRSCTLR1 (always true) used to get the counter to decrement. From * there a resource selector is configured with the counter and the * timestamp control register to use the resource selector to trigger the - * event that will insert a timestamp packet in the stream. + * event that will insert a timestamp packet in the stream: + * + * +--------------+ + * | Resource 1 | fixed "always-true" resource + * +--------------+ + * | + * +------v-------+ + * | Counter x | (reload to 1 on underflow) + * +--------------+ + * | + * +------v--------------+ + * | Resource Selector y | (trigger on counter x == 0) + * +---------------------+ + * | + * +------v---------------+ + * | Timestamp Generator | (timestamp on resource y) + * +----------------------+ */ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata) { - int ctridx, ret = -EINVAL; - int counter, rselector; - u32 val = 0; + int ctridx; + int rselector; struct etmv4_config *config = &drvdata->config;
/* No point in trying if we don't have at least one counter */ if (!drvdata->nr_cntr) - goto out; + return -EINVAL;
/* Find a counter that hasn't been initialised */ for (ctridx = 0; ctridx < drvdata->nr_cntr; ctridx++) @@ -663,15 +678,19 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata) /* All the counters have been configured already, bail out */ if (ctridx == drvdata->nr_cntr) { pr_debug("%s: no available counter found\n", __func__); - ret = -ENOSPC; - goto out; + return -ENOSPC; }
/* - * Searching for an available resource selector to use, starting at - * '2' since every implementation has at least 2 resource selector. - * ETMIDR4 gives the number of resource selector _pairs_, - * hence multiply by 2. + * Searching for an available resource selector to use, starting at '2' + * since resource 0 is the fixed 'always returns false' resource and 1 + * is the fixed 'always returns true' resource. See IHI0064H_b '7.3.64 + * TRCRSCTLRn, Resource Selection Control Registers, n=2-31'. If there + * are no resources, there would also be no counters so wouldn't get + * here. + * + * ETMIDR4 gives the number of resource selector _pairs_, hence multiply + * by 2. */ for (rselector = 2; rselector < drvdata->nr_resource * 2; rselector++) if (!config->res_ctrl[rselector]) @@ -680,13 +699,9 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata) if (rselector == drvdata->nr_resource * 2) { pr_debug("%s: no available resource selector found\n", __func__); - ret = -ENOSPC; - goto out; + return -ENOSPC; }
- /* Remember what counter we used */ - counter = 1 << ctridx; - /* * Initialise original and reload counter value to the smallest * possible value in order to get as much precision as we can. @@ -694,26 +709,41 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata) config->cntr_val[ctridx] = 1; config->cntrldvr[ctridx] = 1;
- /* Set the trace counter control register */ - val = 0x1 << 16 | /* Bit 16, reload counter automatically */ - 0x0 << 7 | /* Select single resource selector */ - 0x1; /* Resource selector 1, i.e always true */ - - config->cntr_ctrl[ctridx] = val; - - val = 0x2 << 16 | /* Group 0b0010 - Counter and sequencers */ - counter << 0; /* Counter to use */ - - config->res_ctrl[rselector] = val; + /* + * Trace Counter Control Register TRCCNTCTLRn + * + * CNTCHAIN = 0, don't reload on the previous counter + * RLDSELF = true, reload counter automatically on underflow + * RLDEVENT = RES_SEL_FALSE (0), reload on single false resource (never reload) + * CNTEVENT = RES_SEL_TRUE (1), count single fixed 'always true' resource (always decrement) + */ + config->cntr_ctrl[ctridx] = TRCCNTCTLRn_RLDSELF | + FIELD_PREP(TRCCNTCTLRn_RLDEVENT_MASK, + etm4_res_sel_single(ETM4_RES_SEL_FALSE)) | + FIELD_PREP(TRCCNTCTLRn_CNTEVENT_MASK, + etm4_res_sel_single(ETM4_RES_SEL_TRUE));
- val = 0x0 << 7 | /* Select single resource selector */ - rselector; /* Resource selector */ + /* + * Resource Selection Control Register TRCRSCTLRn + * + * PAIRINV = 0, INV = 0, don't invert + * GROUP = 2, SELECT = ctridx, trigger when counter 'ctridx' reaches 0 + * + * Multiple counters can be selected, and each bit signifies a counter, + * so set bit 'ctridx' to select our counter. + */ + config->res_ctrl[rselector] = FIELD_PREP(TRCRSCTLRn_GROUP_MASK, 2) | + FIELD_PREP(TRCRSCTLRn_SELECT_MASK, 1 << ctridx);
- config->ts_ctrl = val; + /* + * Global Timestamp Control Register TRCTSCTLR + * + * EVENT = generate timestamp on single resource 'rselector' + */ + config->ts_ctrl = FIELD_PREP(TRCTSCTLR_EVENT_MASK, + etm4_res_sel_single(rselector));
- ret = 0; -out: - return ret; + return 0; }
static int etm4_parse_event_config(struct coresight_device *csdev, diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index d178d79d9827..89d81ce4e04e 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -225,6 +225,50 @@ #define TRCRSCTLRn_GROUP_MASK GENMASK(19, 16) #define TRCRSCTLRn_SELECT_MASK GENMASK(15, 0)
+#define TRCCNTCTLRn_CNTCHAIN BIT(17) +#define TRCCNTCTLRn_RLDSELF BIT(16) +#define TRCCNTCTLRn_RLDEVENT_MASK GENMASK(15, 8) +#define TRCCNTCTLRn_CNTEVENT_MASK GENMASK(7, 0) + +#define TRCTSCTLR_EVENT_MASK GENMASK(7, 0) + +#define ETM4_RES_SEL_FALSE 0 /* Fixed function 'always false' resource selector */ +#define ETM4_RES_SEL_TRUE 1 /* Fixed function 'always true' resource selector */ + +#define ETM4_RES_SEL_SINGLE_MASK GENMASK(4, 0) +#define ETM4_RES_SEL_PAIR_MASK GENMASK(3, 0) +#define ETM4_RES_SEL_TYPE_PAIR BIT(7) + +/* + * Utilities for programming EVENT resource selectors, e.g. TRCCNTCTLRn_RLDEVENT. + * + * Resource selectors have a common format across registers: + * + * 7 6 5 4 0 + * +------+------+-------+ + * | TYPE | RES0 | SEL | + * +------+------+-------+ + * + * Where TYPE indicates whether the selector is for a single event or a pair. + * When TYPE is pair, SEL is 4 bits wide and using pair 0 is UNPREDICTABLE. + * Otherwise for single it's 5 bits wide. + */ +static inline u32 etm4_res_sel_single(u8 res_sel_idx) +{ + WARN_ON_ONCE(!FIELD_FIT(ETM4_RES_SEL_SINGLE_MASK, res_sel_idx)); + return FIELD_PREP(ETM4_RES_SEL_SINGLE_MASK, res_sel_idx); +} + +static inline u32 etm4_res_sel_pair(u8 res_sel_idx) +{ + if (__builtin_constant_p(res_sel_idx)) + BUILD_BUG_ON(res_sel_idx == 0); + WARN_ON_ONCE(!FIELD_FIT(ETM4_RES_SEL_PAIR_MASK, res_sel_idx) || + (res_sel_idx == 0)); + return FIELD_PREP(ETM4_RES_SEL_PAIR_MASK, res_sel_idx) | + ETM4_RES_SEL_TYPE_PAIR; +} + /* * System instructions to access ETM registers. * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions @@ -824,7 +868,7 @@ struct etmv4_config { u32 eventctrl0; u32 eventctrl1; u32 stall_ctrl; - u32 ts_ctrl; + u32 ts_ctrl; /* TRCTSCTLR */ u32 ccctlr; u32 bb_ctrl; u32 vinst_ctrl; @@ -837,11 +881,11 @@ struct etmv4_config { u32 seq_rst; u32 seq_state; u8 cntr_idx; - u32 cntrldvr[ETMv4_MAX_CNTR]; - u32 cntr_ctrl[ETMv4_MAX_CNTR]; - u32 cntr_val[ETMv4_MAX_CNTR]; + u32 cntrldvr[ETMv4_MAX_CNTR]; /* TRCCNTRLDVRn */ + u32 cntr_ctrl[ETMv4_MAX_CNTR]; /* TRCCNTCTLRn */ + u32 cntr_val[ETMv4_MAX_CNTR]; /* TRCCNTVRn */ u8 res_idx; - u32 res_ctrl[ETM_MAX_RES_SEL]; + u32 res_ctrl[ETM_MAX_RES_SEL]; /* TRCRSCTLRn */ u8 ss_idx; u32 ss_ctrl[ETM_MAX_SS_CMP]; u32 ss_status[ETM_MAX_SS_CMP];
This allows us to define and consume them in a unified way in later commits.
A lot of the existing code has open coded bit shifts or direct usage of whole config values which is error prone and hides which bits are in use and which are free.
Reviewed-by: Leo Yan leo.yan@arm.com Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 21 ++++++++-------- drivers/hwtracing/coresight/coresight-etm-perf.h | 31 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 3805282b97e8..bf4b105e0f41 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> @@ -50,22 +51,22 @@ struct etm_ctxt { static DEFINE_PER_CPU(struct etm_ctxt, etm_ctxt); static DEFINE_PER_CPU(struct coresight_device *, csdev_src);
-PMU_FORMAT_ATTR(cycacc, "config:" __stringify(ETM_OPT_CYCACC)); -PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS)); -PMU_FORMAT_ATTR(retstack, "config:" __stringify(ETM_OPT_RETSTK)); -PMU_FORMAT_ATTR(sinkid, "config2:0-31"); +GEN_PMU_FORMAT_ATTR(cycacc); +GEN_PMU_FORMAT_ATTR(timestamp); +GEN_PMU_FORMAT_ATTR(retstack); +GEN_PMU_FORMAT_ATTR(sinkid);
#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X) -PMU_FORMAT_ATTR(branch_broadcast, "config:"__stringify(ETM_OPT_BRANCH_BROADCAST)); +GEN_PMU_FORMAT_ATTR(branch_broadcast); /* contextid1 enables tracing CONTEXTIDR_EL1*/ -PMU_FORMAT_ATTR(contextid1, "config:" __stringify(ETM_OPT_CTXTID)); +GEN_PMU_FORMAT_ATTR(contextid1); /* contextid2 enables tracing CONTEXTIDR_EL2*/ -PMU_FORMAT_ATTR(contextid2, "config:" __stringify(ETM_OPT_CTXTID2)); +GEN_PMU_FORMAT_ATTR(contextid2); /* preset - if sink ID is used as a configuration selector */ -PMU_FORMAT_ATTR(preset, "config:0-3"); +GEN_PMU_FORMAT_ATTR(preset); /* config ID - set if a system configuration is selected */ -PMU_FORMAT_ATTR(configid, "config2:32-63"); -PMU_FORMAT_ATTR(cc_threshold, "config3:0-11"); +GEN_PMU_FORMAT_ATTR(configid); +GEN_PMU_FORMAT_ATTR(cc_threshold);
/* * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1 diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 5febbcdb8696..c794087a0e99 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -20,6 +20,37 @@ struct cscfg_config_desc; */ #define ETM_ADDR_CMP_MAX 8
+#define ATTR_CFG_FLD_preset_CFG config +#define ATTR_CFG_FLD_preset_LO 0 +#define ATTR_CFG_FLD_preset_HI 3 +#define ATTR_CFG_FLD_branch_broadcast_CFG config +#define ATTR_CFG_FLD_branch_broadcast_LO 8 +#define ATTR_CFG_FLD_branch_broadcast_HI 8 +#define ATTR_CFG_FLD_cycacc_CFG config +#define ATTR_CFG_FLD_cycacc_LO 12 +#define ATTR_CFG_FLD_cycacc_HI 12 +#define ATTR_CFG_FLD_contextid1_CFG config +#define ATTR_CFG_FLD_contextid1_LO 14 +#define ATTR_CFG_FLD_contextid1_HI 14 +#define ATTR_CFG_FLD_contextid2_CFG config +#define ATTR_CFG_FLD_contextid2_LO 15 +#define ATTR_CFG_FLD_contextid2_HI 15 +#define ATTR_CFG_FLD_timestamp_CFG config +#define ATTR_CFG_FLD_timestamp_LO 28 +#define ATTR_CFG_FLD_timestamp_HI 28 +#define ATTR_CFG_FLD_retstack_CFG config +#define ATTR_CFG_FLD_retstack_LO 29 +#define ATTR_CFG_FLD_retstack_HI 29 +#define ATTR_CFG_FLD_sinkid_CFG config2 +#define ATTR_CFG_FLD_sinkid_LO 0 +#define ATTR_CFG_FLD_sinkid_HI 31 +#define ATTR_CFG_FLD_configid_CFG config2 +#define ATTR_CFG_FLD_configid_LO 32 +#define ATTR_CFG_FLD_configid_HI 63 +#define ATTR_CFG_FLD_cc_threshold_CFG config3 +#define ATTR_CFG_FLD_cc_threshold_LO 0 +#define ATTR_CFG_FLD_cc_threshold_HI 11 + /** * struct etm_filter - single instruction range or start/stop configuration. * @start_addr: The address to start tracing on.
Currently we're programming attr->config directly into ETMCR after some validation. This obscures which fields are being used, and also makes it impossible to move fields around or use other configN fields in the future.
Improve it by only reading the fields that are valid and then setting the appropriate ETMCR bits based on each one.
The ETMCR_CTXID_SIZE part can be removed as it was never a valid option because it's not in ETM3X_SUPPORTED_OPTIONS.
Reviewed-by: Leo Yan leo.yan@arm.com Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm3x-core.c | 24 ++++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index a5e809589d3e..4511fc2f8d72 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -28,6 +28,7 @@ #include <linux/uaccess.h> #include <linux/clk.h> #include <linux/perf_event.h> +#include <linux/perf/arm_pmu.h> #include <asm/sections.h>
#include "coresight-etm.h" @@ -339,21 +340,22 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata, if (attr->config & ~ETM3X_SUPPORTED_OPTIONS) return -EINVAL;
- config->ctrl = attr->config; + config->ctrl = 0;
- /* Don't trace contextID when runs in non-root PID namespace */ - if (!task_is_in_init_pid_ns(current)) - config->ctrl &= ~ETMCR_CTXID_SIZE; + if (ATTR_CFG_GET_FLD(attr, cycacc)) + config->ctrl |= ETMCR_CYC_ACC; + + if (ATTR_CFG_GET_FLD(attr, timestamp)) + config->ctrl |= ETMCR_TIMESTAMP_EN;
/* - * Possible to have cores with PTM (supports ret stack) and ETM - * (never has ret stack) on the same SoC. So if we have a request - * for return stack that can't be honoured on this core then - * clear the bit - trace will still continue normally + * Possible to have cores with PTM (supports ret stack) and ETM (never + * has ret stack) on the same SoC. So only enable when it can be honored + * - trace will still continue normally otherwise. */ - if ((config->ctrl & ETMCR_RETURN_STACK) && - !(drvdata->etmccer & ETMCCER_RETSTACK)) - config->ctrl &= ~ETMCR_RETURN_STACK; + if (ATTR_CFG_GET_FLD(attr, retstack) && + (drvdata->etmccer & ETMCCER_RETSTACK)) + config->ctrl |= ETMCR_RETURN_STACK;
return 0; }
config isn't the only field, there are also config1, config2, etc. Rejecting unrecognized attributes is therefore inconsistent as it wasn't done for all fields. It was only necessary when we were directly programming attr->config into ETMCR and didn't hide the unsupported fields, but now it's not needed so remove it.
Reviewed-by: Leo Yan leo.yan@arm.com Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm3x-core.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 4511fc2f8d72..584d653eda81 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -333,13 +333,6 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata, if (config->mode) etm_config_trace_mode(config);
- /* - * At this time only cycle accurate, return stack and timestamp - * options are available. - */ - if (attr->config & ~ETM3X_SUPPORTED_OPTIONS) - return -EINVAL; - config->ctrl = 0;
if (ATTR_CFG_GET_FLD(attr, cycacc))
TRCSYNCPR.PERIOD is the only functional part of TRCSYNCPR and it only has 5 valid bits so it can be stored in a u8.
Reviewed-by: Mike Leach mike.leach@linaro.org Reviewed-by: Leo Yan leo.yan@arm.com Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 012c52fd1933..0287d19ce12e 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -825,7 +825,6 @@ struct etmv4_config { u32 eventctrl1; u32 stall_ctrl; u32 ts_ctrl; - u32 syncfreq; u32 ccctlr; u32 bb_ctrl; u32 vinst_ctrl; @@ -833,6 +832,7 @@ struct etmv4_config { u32 vissctlr; u32 vipcssctlr; u8 seq_idx; + u8 syncfreq; u32 seq_ctrl[ETM_MAX_SEQ_STATES]; u32 seq_rst; u32 seq_state;
ETMv3 only has a few attributes, and setting unused ones results in an error, so hide them to begin with.
Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 37 +++++++++++++----------- 1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 17afa0f4cdee..3805282b97e8 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -50,27 +50,23 @@ struct etm_ctxt { static DEFINE_PER_CPU(struct etm_ctxt, etm_ctxt); static DEFINE_PER_CPU(struct coresight_device *, csdev_src);
-/* - * The PMU formats were orignally for ETMv3.5/PTM's ETMCR 'config'; - * now take them as general formats and apply on all ETMs. - */ -PMU_FORMAT_ATTR(branch_broadcast, "config:"__stringify(ETM_OPT_BRANCH_BROADCAST)); PMU_FORMAT_ATTR(cycacc, "config:" __stringify(ETM_OPT_CYCACC)); -/* contextid1 enables tracing CONTEXTIDR_EL1 for ETMv4 */ -PMU_FORMAT_ATTR(contextid1, "config:" __stringify(ETM_OPT_CTXTID)); -/* contextid2 enables tracing CONTEXTIDR_EL2 for ETMv4 */ -PMU_FORMAT_ATTR(contextid2, "config:" __stringify(ETM_OPT_CTXTID2)); PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS)); PMU_FORMAT_ATTR(retstack, "config:" __stringify(ETM_OPT_RETSTK)); +PMU_FORMAT_ATTR(sinkid, "config2:0-31"); + +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X) +PMU_FORMAT_ATTR(branch_broadcast, "config:"__stringify(ETM_OPT_BRANCH_BROADCAST)); +/* contextid1 enables tracing CONTEXTIDR_EL1*/ +PMU_FORMAT_ATTR(contextid1, "config:" __stringify(ETM_OPT_CTXTID)); +/* contextid2 enables tracing CONTEXTIDR_EL2*/ +PMU_FORMAT_ATTR(contextid2, "config:" __stringify(ETM_OPT_CTXTID2)); /* preset - if sink ID is used as a configuration selector */ PMU_FORMAT_ATTR(preset, "config:0-3"); -/* Sink ID - same for all ETMs */ -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");
- /* * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1 * when the kernel is running at EL1; when the kernel is at EL2, @@ -82,27 +78,34 @@ 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 return sprintf(page, "config:%d\n", pid_fmt); }
static struct device_attribute format_attr_contextid = __ATTR(contextid, 0444, format_attr_contextid_show, NULL); +#endif
+/* + * ETMv3 only uses the first 3 attributes for programming itself (see + * ETM3X_SUPPORTED_OPTIONS). Sink ID is also supported for selecting a + * sink in both, but not used for configuring the ETM. The remaining + * attributes are ETMv4 specific. + */ static struct attribute *etm_config_formats_attr[] = { &format_attr_cycacc.attr, - &format_attr_contextid.attr, - &format_attr_contextid1.attr, - &format_attr_contextid2.attr, &format_attr_timestamp.attr, &format_attr_retstack.attr, &format_attr_sinkid.attr, +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X) + &format_attr_contextid.attr, + &format_attr_contextid1.attr, + &format_attr_contextid2.attr, &format_attr_preset.attr, &format_attr_configid.attr, &format_attr_branch_broadcast.attr, &format_attr_cc_threshold.attr, +#endif NULL, };
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@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index bf4b105e0f41..3c8a6f795094 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -77,10 +77,9 @@ static ssize_t format_attr_contextid_show(struct device *dev, struct device_attribute *attr, char *page) { - int pid_fmt = ETM_OPT_CTXTID; - - pid_fmt = is_kernel_in_hyp_mode() ? ETM_OPT_CTXTID2 : ETM_OPT_CTXTID; - return sprintf(page, "config:%d\n", pid_fmt); + if (is_kernel_in_hyp_mode()) + return contextid2_show(dev, attr, page); + return contextid1_show(dev, attr, page); }
static struct device_attribute format_attr_contextid = @@ -319,7 +318,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; @@ -332,13 +331,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;
Remove hard coded bitfield extractions and shifts and replace with ATTR_CFG_GET_FLD().
ETM4_CFG_BIT_BB was defined to give the register bit positions to userspace, TRCCONFIGR_BB should be used in the kernel so replace it.
Reviewed-by: Leo Yan leo.yan@arm.com Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 44 ++++++++++------------ 1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 2ec2ae1fef58..b457f182efbe 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -29,6 +29,7 @@ #include <linux/seq_file.h> #include <linux/uaccess.h> #include <linux/perf_event.h> +#include <linux/perf/arm_pmu.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/property.h> @@ -780,17 +781,17 @@ static int etm4_parse_event_config(struct coresight_device *csdev, goto out;
/* Go from generic option to ETMv4 specifics */ - if (attr->config & BIT(ETM_OPT_CYCACC)) { + if (ATTR_CFG_GET_FLD(attr, cycacc)) { config->cfg |= TRCCONFIGR_CCI; /* TRM: Must program this for cycacc to work */ - cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK; + cc_threshold = ATTR_CFG_GET_FLD(attr, cc_threshold); if (!cc_threshold) cc_threshold = ETM_CYC_THRESHOLD_DEFAULT; if (cc_threshold < drvdata->ccitmin) cc_threshold = drvdata->ccitmin; config->ccctlr = cc_threshold; } - if (attr->config & BIT(ETM_OPT_TS)) { + if (ATTR_CFG_GET_FLD(attr, timestamp)) { /* * Configure timestamps to be emitted at regular intervals in * order to correlate instructions executed on different CPUs @@ -810,17 +811,17 @@ static int etm4_parse_event_config(struct coresight_device *csdev, }
/* Only trace contextID when runs in root PID namespace */ - if ((attr->config & BIT(ETM_OPT_CTXTID)) && + if (ATTR_CFG_GET_FLD(attr, contextid1) && task_is_in_init_pid_ns(current)) /* bit[6], Context ID tracing bit */ config->cfg |= TRCCONFIGR_CID;
/* - * If set bit ETM_OPT_CTXTID2 in perf config, this asks to trace VMID - * for recording CONTEXTIDR_EL2. Do not enable VMID tracing if the - * kernel is not running in EL2. + * If set bit contextid2 in perf config, this asks to trace VMID for + * recording CONTEXTIDR_EL2. Do not enable VMID tracing if the kernel + * is not running in EL2. */ - if (attr->config & BIT(ETM_OPT_CTXTID2)) { + if (ATTR_CFG_GET_FLD(attr, contextid2)) { if (!is_kernel_in_hyp_mode()) { ret = -EINVAL; goto out; @@ -831,26 +832,22 @@ static int etm4_parse_event_config(struct coresight_device *csdev, }
/* return stack - enable if selected and supported */ - if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack) + if (ATTR_CFG_GET_FLD(attr, retstack) && drvdata->retstack) /* bit[12], Return stack enable bit */ config->cfg |= TRCCONFIGR_RS;
/* - * Set any selected configuration and preset. - * - * This extracts the values of PMU_FORMAT_ATTR(configid) and PMU_FORMAT_ATTR(preset) - * in the perf attributes defined in coresight-etm-perf.c. - * configid uses bits 63:32 of attr->config2, preset uses bits 3:0 of attr->config. - * A zero configid means no configuration active, preset = 0 means no preset selected. + * Set any selected configuration and preset. A zero configid means no + * configuration active, preset = 0 means no preset selected. */ - if (attr->config2 & GENMASK_ULL(63, 32)) { - cfg_hash = (u32)(attr->config2 >> 32); - preset = attr->config & 0xF; + cfg_hash = ATTR_CFG_GET_FLD(attr, configid); + if (cfg_hash) { + preset = ATTR_CFG_GET_FLD(attr, preset); ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); }
/* branch broadcast - enable if selected and supported */ - if (attr->config & BIT(ETM_OPT_BRANCH_BROADCAST)) { + if (ATTR_CFG_GET_FLD(attr, branch_broadcast)) { if (!drvdata->trcbb) { /* * Missing BB support could cause silent decode errors @@ -859,7 +856,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev, ret = -EINVAL; goto out; } else { - config->cfg |= BIT(ETM4_CFG_BIT_BB); + config->cfg |= TRCCONFIGR_BB; } }
@@ -1083,11 +1080,8 @@ static int etm4_disable_perf(struct coresight_device *csdev, return -EINVAL;
etm4_disable_hw(drvdata); - /* - * The config_id occupies bits 63:32 of the config2 perf event attr - * field. If this is non-zero then we will have enabled a config. - */ - if (attr->config2 & GENMASK_ULL(63, 32)) + /* If configid is non-zero then we will have enabled a config. */ + if (ATTR_CFG_GET_FLD(attr, configid)) cscfg_csdev_disable_active_config(csdev);
/*
'timestamp' is currently 1 bit wide for on/off. To enable setting different intervals in a later commit, extend it to 4 bits wide. Keep the old bit position for backward compatibility but don't publish in the format/ folder. It will be removed from the documentation and can be removed completely after enough time has passed.
ETM3x doesn't support different intervals, so validate that the value is either 0 or 1.
Tools that read the bit positions from the format/ folder will continue to work as before, setting either 0 or 1 for off/on. Tools that incorrectly didn't do this and set the ETM_OPT_TS bit directly will also continue to work because that old bit is still checked.
This avoids adding a second timestamp attribute for setting the interval. This would be awkward to use because tools would have to be updated to ensure that the timestamps are always enabled when an interval is set, and the driver would have to validate that both options are provided together. All this does is implement the semantics of a single enum but spread over multiple fields.
Reviewed-by: Leo Yan leo.yan@arm.com Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.h | 13 ++++++++++--- drivers/hwtracing/coresight/coresight-etm3x-core.c | 9 ++++++++- drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +++- 3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index c794087a0e99..24d929428633 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -23,6 +23,9 @@ struct cscfg_config_desc; #define ATTR_CFG_FLD_preset_CFG config #define ATTR_CFG_FLD_preset_LO 0 #define ATTR_CFG_FLD_preset_HI 3 +#define ATTR_CFG_FLD_timestamp_CFG config +#define ATTR_CFG_FLD_timestamp_LO 4 +#define ATTR_CFG_FLD_timestamp_HI 7 #define ATTR_CFG_FLD_branch_broadcast_CFG config #define ATTR_CFG_FLD_branch_broadcast_LO 8 #define ATTR_CFG_FLD_branch_broadcast_HI 8 @@ -35,9 +38,13 @@ struct cscfg_config_desc; #define ATTR_CFG_FLD_contextid2_CFG config #define ATTR_CFG_FLD_contextid2_LO 15 #define ATTR_CFG_FLD_contextid2_HI 15 -#define ATTR_CFG_FLD_timestamp_CFG config -#define ATTR_CFG_FLD_timestamp_LO 28 -#define ATTR_CFG_FLD_timestamp_HI 28 +/* + * Old position of 'timestamp' and not published in sysfs. Remove at a later + * date if necessary. + */ +#define ATTR_CFG_FLD_deprecated_timestamp_CFG config +#define ATTR_CFG_FLD_deprecated_timestamp_LO 28 +#define ATTR_CFG_FLD_deprecated_timestamp_HI 28 #define ATTR_CFG_FLD_retstack_CFG config #define ATTR_CFG_FLD_retstack_LO 29 #define ATTR_CFG_FLD_retstack_HI 29 diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 584d653eda81..d4c04e563bf6 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -338,9 +338,16 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata, if (ATTR_CFG_GET_FLD(attr, cycacc)) config->ctrl |= ETMCR_CYC_ACC;
- if (ATTR_CFG_GET_FLD(attr, timestamp)) + if (ATTR_CFG_GET_FLD(attr, deprecated_timestamp) || + ATTR_CFG_GET_FLD(attr, timestamp)) config->ctrl |= ETMCR_TIMESTAMP_EN;
+ if (ATTR_CFG_GET_FLD(attr, timestamp) > 1) { + dev_dbg(&drvdata->csdev->dev, + "timestamp format attribute should be 0 (off) or 1 (on)\n"); + return -EINVAL; + } + /* * Possible to have cores with PTM (supports ret stack) and ETM (never * has ret stack) on the same SoC. So only enable when it can be honored diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index b457f182efbe..c7bf73c8f2d7 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/perf/arm_pmu.h> #include <linux/platform_device.h> @@ -791,7 +792,8 @@ static int etm4_parse_event_config(struct coresight_device *csdev, cc_threshold = drvdata->ccitmin; config->ccctlr = cc_threshold; } - if (ATTR_CFG_GET_FLD(attr, timestamp)) { + if (ATTR_CFG_GET_FLD(attr, deprecated_timestamp) || + ATTR_CFG_GET_FLD(attr, timestamp)) { /* * Configure timestamps to be emitted at regular intervals in * order to correlate instructions executed on different CPUs
Timestamps are currently emitted at the maximum rate possible, which is much too frequent for most use cases. Set the interval using the value from the timestamp field. Granular control is not required, so save space in the config by interpreting it as 2 ^ timestamp. And then 4 bits (0 - 15) is enough to set the interval to be larger than the existing SYNC timestamp interval.
No sysfs mode support is needed for this attribute because counter generated timestamps are only configured for Perf mode.
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.h | 1 + drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 +++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 24d929428633..128f80bb1443 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"
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index c7bf73c8f2d7..0129b0502726 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -651,7 +651,7 @@ static void etm4_enable_sysfs_smp_call(void *info) * +--------------+ * | * +------v-------+ - * | Counter x | (reload to 1 on underflow) + * | Counter x | (reload to 2 ^ timestamp on underflow) * +--------------+ * | * +------v--------------+ @@ -662,11 +662,25 @@ static void etm4_enable_sysfs_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; + struct perf_event_attr max_timestamp = { + .ATTR_CFG_FLD_timestamp_CFG = U64_MAX, + }; + + /* timestamp may be 0 if deprecated_timestamp is used, so make min 1 */ + u8 ts_level = max(1, ATTR_CFG_GET_FLD(attr, timestamp)); + + /* + * Disable counter generated timestamps when timestamp == MAX. Leave + * only SYNC timestamps. + */ + if (ts_level == ATTR_CFG_GET_FLD(&max_timestamp, timestamp)) + return 0;
/* No point in trying if we don't have at least one counter */ if (!drvdata->nr_cntr) @@ -704,12 +718,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 - 1);
/* * Trace Counter Control Register TRCCNTCTLRn @@ -799,7 +809,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
Document how the new field is used, maximum value and the interaction with SYNC timestamps.
Signed-off-by: James Clark james.clark@linaro.org --- Documentation/trace/coresight/coresight.rst | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst index 806699871b80..d461de4e067e 100644 --- a/Documentation/trace/coresight/coresight.rst +++ b/Documentation/trace/coresight/coresight.rst @@ -613,8 +613,20 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/ - Session local version of the system wide setting: :ref:`ETM_MODE_RETURNSTACK <coresight-return-stack>` * - timestamp - - Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP - <coresight-timestamp>` + - Controls generation and interval of timestamps. + + 0 = off, 1 = minimum interval .. 15 = maximum interval. + + Values 1 - 14 use a counter that decrements every cycle to generate a + timestamp on underflow. The reload value for the counter is 2 ^ (interval + - 1). If the value is 1 then the reload value is 1, if the value is 11 + then the reload value is 1024 etc. + + Setting the maximum interval (15) will disable the counter generated + timestamps, freeing the counter resource, leaving only ones emitted when + a SYNC packet is generated. The sync interval is controlled with + TRCSYNCPR.PERIOD which is every 4096 bytes of trace by default. + * - cc_threshold - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold
ETM_OPT_* definitions duplicate the PMU format attributes that have always been published in sysfs. Hardcoding them here makes it misleading as to what the 'real' PMU API is and prevents attributes from being rearranged in the future.
ETM4_CFG_BIT_* definitions just define what the Arm Architecture is which is not the responsibility of the kernel to do and doesn't scale to other registers or versions of ETM. It's not an actual software ABI/API and these definitions here mislead that it is.
Any tools using the first ones would be broken anyway as they won't work when attributes are moved, so removing them is the right thing to do and will prompt a fix. Tools using the second ones can trivially redefine them locally.
Perf also has its own copy of the headers so both of these things can be fixed up at a later date.
Reviewed-by: Leo Yan leo.yan@arm.com Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: James Clark james.clark@linaro.org --- include/linux/coresight-pmu.h | 24 ------------------------ 1 file changed, 24 deletions(-)
diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h index 89b0ac0014b0..2e179abe472a 100644 --- a/include/linux/coresight-pmu.h +++ b/include/linux/coresight-pmu.h @@ -21,30 +21,6 @@ */ #define CORESIGHT_LEGACY_CPU_TRACE_ID(cpu) (0x10 + (cpu * 2))
-/* - * Below are the definition of bit offsets for perf option, and works as - * arbitrary values for all ETM versions. - * - * Most of them are orignally from ETMv3.5/PTM's ETMCR config, therefore, - * ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and - * directly use below macros as config bits. - */ -#define ETM_OPT_BRANCH_BROADCAST 8 -#define ETM_OPT_CYCACC 12 -#define ETM_OPT_CTXTID 14 -#define ETM_OPT_CTXTID2 15 -#define ETM_OPT_TS 28 -#define ETM_OPT_RETSTK 29 - -/* ETMv4 CONFIGR programming bits for the ETM OPTs */ -#define ETM4_CFG_BIT_BB 3 -#define ETM4_CFG_BIT_CYCACC 4 -#define ETM4_CFG_BIT_CTXTID 6 -#define ETM4_CFG_BIT_VMID 7 -#define ETM4_CFG_BIT_TS 11 -#define ETM4_CFG_BIT_RETSTK 12 -#define ETM4_CFG_BIT_VMID_OPT 15 - /* * Interpretation of the PERF_RECORD_AUX_OUTPUT_HW_ID payload. * Used to associate a CPU with the CoreSight Trace ID.
On Wed, Nov 26, 2025 at 10:54:29AM +0000, James Clark wrote:
Do some cleanups then expand the timestamp format attribute from 1 bit to 4 bits for ETMv4 in Perf mode. The current interval is too high for most use cases, and particularly on the FVP the number of timestamps generated is excessive. This change not only still allows disabling or enabling timestamps, but also allows the interval to be configured.
The old bit is kept deprecated and undocumented for now. There are known broken versions of Perf that don't read the format attribute positions from sysfs and instead hard code the timestamp bit. We can leave the old bit in the driver until we need the bit for another feature or enough time has passed that these old Perfs are unlikely to be used.
The interval option is added as an event format attribute, rather than a Coresight config because it's something that the driver is already configuring automatically in Perf mode using any unused counter, so it's not possible to modify this with a config.
Applies to coresight/next
Signed-off-by: James Clark james.clark@linaro.org
Tested on Juno-r2 board (though all timestamps are zeros but it still can record timestamp packets):
# perf record -e cs_etm/timestamp=15/ -- ls # perf script -D | grep I_TIMESTAMP | wc 1324 11916 84561
# perf record -e cs_etm/timestamp=14/ -- ls # perf script -D | grep I_TIMESTAMP | wc 2415 21735 154205
# perf record -e cs_etm/timestamp=1/ -- ls # perf script -D | grep I_TIMESTAMP | wc 130743 1176687 8359898
# perf record -e cs_etm/timestamp=0/ -- ls # perf script -D | grep I_TIMESTAMP | wc 0 0 0
Tested-by: Leo Yan leo.yan@arm.com
On Wed, Nov 26, 2025 at 10:54:42AM +0000, James Clark wrote:
Document how the new field is used, maximum value and the interaction with SYNC timestamps.
Signed-off-by: James Clark james.clark@linaro.org
Reviewed-by: Leo Yan leo.yan@arm.com
Documentation/trace/coresight/coresight.rst | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst index 806699871b80..d461de4e067e 100644 --- a/Documentation/trace/coresight/coresight.rst +++ b/Documentation/trace/coresight/coresight.rst @@ -613,8 +613,20 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/ - Session local version of the system wide setting: :ref:`ETM_MODE_RETURNSTACK <coresight-return-stack>` * - timestamp
- Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP<coresight-timestamp>`
- Controls generation and interval of timestamps.0 = off, 1 = minimum interval .. 15 = maximum interval.Values 1 - 14 use a counter that decrements every cycle to generate atimestamp on underflow. The reload value for the counter is 2 ^ (interval- 1). If the value is 1 then the reload value is 1, if the value is 11then the reload value is 1024 etc.Setting the maximum interval (15) will disable the counter generatedtimestamps, freeing the counter resource, leaving only ones emitted whena SYNC packet is generated. The sync interval is controlled withTRCSYNCPR.PERIOD which is every 4096 bytes of trace by default.
- cc_threshold
- Cycle count threshold value. If nothing is provided here or the provided value is 0, then the default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold
-- 2.34.1
Hi,
On Wed, 26 Nov 2025 at 14:01, Leo Yan leo.yan@arm.com wrote:
On Wed, Nov 26, 2025 at 10:54:42AM +0000, James Clark wrote:
Document how the new field is used, maximum value and the interaction with SYNC timestamps.
Signed-off-by: James Clark james.clark@linaro.org
Reviewed-by: Leo Yan leo.yan@arm.com
Documentation/trace/coresight/coresight.rst | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst index 806699871b80..d461de4e067e 100644 --- a/Documentation/trace/coresight/coresight.rst +++ b/Documentation/trace/coresight/coresight.rst @@ -613,8 +613,20 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/ - Session local version of the system wide setting: :ref:`ETM_MODE_RETURNSTACK <coresight-return-stack>` * - timestamp
- Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP<coresight-timestamp>`
- Controls generation and interval of timestamps.0 = off, 1 = minimum interval .. 15 = maximum interval.Values 1 - 14 use a counter that decrements every cycle to generate atimestamp on underflow. The reload value for the counter is 2 ^ (interval- 1). If the value is 1 then the reload value is 1, if the value is 11then the reload value is 1024 etc.Setting the maximum interval (15) will disable the counter generatedtimestamps, freeing the counter resource, leaving only ones emitted whena SYNC packet is generated. The sync interval is controlled withTRCSYNCPR.PERIOD which is every 4096 bytes of trace by default.
What is the default value?
As far as I recall when this command line parameter was a bool then: perf -e cs_etm/timestamp/ <program> is sufficient to turn on timestamping.
This is worth mentioning so users can correctly assess what happens for any existing scripts they might have.
Based on this then the same command must set the timestamp to 1 - which will have the same effect as before as we do not want to break existing behaviour.
Mike
* - cc_threshold - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold-- 2.34.1
On Wed, Nov 26, 2025 at 02:20:14PM +0000, Mike Leach wrote:
[...]
* - timestamp
- Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP<coresight-timestamp>`
- Controls generation and interval of timestamps.0 = off, 1 = minimum interval .. 15 = maximum interval.Values 1 - 14 use a counter that decrements every cycle to generate atimestamp on underflow. The reload value for the counter is 2 ^ (interval- 1). If the value is 1 then the reload value is 1, if the value is 11then the reload value is 1024 etc.Setting the maximum interval (15) will disable the counter generatedtimestamps, freeing the counter resource, leaving only ones emitted whena SYNC packet is generated. The sync interval is controlled withTRCSYNCPR.PERIOD which is every 4096 bytes of trace by default.What is the default value?
From driver's pespective, the default value is 0 (disabled). We do set default values in perf: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tool...
IIUC, the default value would be the same with or without this series.
As far as I recall when this command line parameter was a bool then: perf -e cs_etm/timestamp/ <program> is sufficient to turn on timestamping.
Hmm... with the latest perf, we must assign value to `timestamp`, otherwise perf will report error:
# /mnt/build/perf record -e cs_etm/timestamp/ -C 0 -- taskset -c 0 ls event syntax error: 'cs_etm/timestamp/' ___ Bad event or PMU
Unable to find PMU or event on a PMU of 'cs_etm'
event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term
event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term Run 'perf list' for a list of valid events
This is worth mentioning so users can correctly assess what happens for any existing scripts they might have.
Based on this then the same command must set the timestamp to 1 - which will have the same effect as before as we do not want to break existing behaviour.
Mike
* - cc_threshold - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold-- 2.34.1
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
On 26/11/2025 2:44 pm, Leo Yan wrote:
On Wed, Nov 26, 2025 at 02:20:14PM +0000, Mike Leach wrote:
[...]
* - timestamp
- Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP<coresight-timestamp>`
- Controls generation and interval of timestamps.0 = off, 1 = minimum interval .. 15 = maximum interval.Values 1 - 14 use a counter that decrements every cycle to generate atimestamp on underflow. The reload value for the counter is 2 ^ (interval- 1). If the value is 1 then the reload value is 1, if the value is 11then the reload value is 1024 etc.Setting the maximum interval (15) will disable the counter generatedtimestamps, freeing the counter resource, leaving only ones emitted whena SYNC packet is generated. The sync interval is controlled withTRCSYNCPR.PERIOD which is every 4096 bytes of trace by default.What is the default value?
From driver's pespective, the default value is 0 (disabled). We do set default values in perf: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tool...
IIUC, the default value would be the same with or without this series.
As far as I recall when this command line parameter was a bool then: perf -e cs_etm/timestamp/ <program> is sufficient to turn on timestamping.
Hmm... with the latest perf, we must assign value to `timestamp`, otherwise perf will report error:
# /mnt/build/perf record -e cs_etm/timestamp/ -C 0 -- taskset -c 0 ls event syntax error: 'cs_etm/timestamp/' ___ Bad event or PMU Unable to find PMU or event on a PMU of 'cs_etm' event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term Run 'perf list' for a list of valid events
That's unfortunate and not what I expected. And I don't think it makes sense to remove that validation from Perf. The test uses "timestamp=1" so I didn't notice.
Can we accept that people are most likely using the defaults so timestamps are already on and they wouldn't be using it? The only real use case of that at the moment is to do timestamp=0 and that doesn't fail.
Although it's not the default for per-thread mode and I did find the OpenCSD HOWTO.md uses it as an example. timestamps make less sense in per-thread mode as you don't need to correlate between CPUs or watch for context switches.
I suppose we need to choose what's worse, breaking some subset of Perf commands in a slightly annoying way or having two separate options to control timestamps that you have to use together. I think it's 50/50, maybe with the breakage being the slightly better option.
This is worth mentioning so users can correctly assess what happens for any existing scripts they might have.
Based on this then the same command must set the timestamp to 1 - which will have the same effect as before as we do not want to break existing behaviour.
Mike
* - cc_threshold - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold-- 2.34.1
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
On Wed, Nov 26, 2025 at 02:44:37PM +0000, Coresight ML wrote:
[...]
As far as I recall when this command line parameter was a bool then: perf -e cs_etm/timestamp/ <program> is sufficient to turn on timestamping.
Hmm... with the latest perf, we must assign value to `timestamp`, otherwise perf will report error:
# /mnt/build/perf record -e cs_etm/timestamp/ -C 0 -- taskset -c 0 ls event syntax error: 'cs_etm/timestamp/' ___ Bad event or PMU Unable to find PMU or event on a PMU of 'cs_etm' event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term Run 'perf list' for a list of valid events
Apologize for this misinformation. When `timestamp` is bool, it does support the `-e cs_etm/timestamp/` format.
This is worth mentioning so users can correctly assess what happens for any existing scripts they might have.
Based on this then the same command must set the timestamp to 1 - which will have the same effect as before as we do not want to break existing behaviour.
Not sure if we need to record such info. Seems to me, it is weird that record a common behaviour for perf formats in this doc.
The perf error log would be sufficient for users to setup a proper format?
Thanks, Leo
On 26/11/2025 2:44 pm, Leo Yan wrote:
On Wed, Nov 26, 2025 at 02:20:14PM +0000, Mike Leach wrote:
[...]
* - timestamp
- Session local version of the system wide setting::ref:`ETMv4_MODE_TIMESTAMP
<coresight-timestamp>`
- Controls generation and interval of timestamps.0 = off, 1 = minimum interval .. 15 = maximum interval.Values 1 - 14 use a counter that decrements every cycle to generate atimestamp on underflow. The reload value for the counter is 2 ^(interval
- 1). If the value is 1 then the reload value is 1, if the value is 11then the reload value is 1024 etc.Setting the maximum interval (15) will disable the counter generatedtimestamps, freeing the counter resource, leaving only ones emittedwhen
a SYNC packet is generated. The sync interval is controlled withTRCSYNCPR.PERIOD which is every 4096 bytes of trace by default.What is the default value?
From driver's pespective, the default value is 0 (disabled). We do set default values in perf: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre e/tools/perf/arch/arm/util/cs-etm.c#n444
IIUC, the default value would be the same with or without this series.
As far as I recall when this command line parameter was a bool then: perf -e cs_etm/timestamp/ <program> is sufficient to turn on timestamping.
Hmm... with the latest perf, we must assign value to `timestamp`, otherwise perf will report error:
# /mnt/build/perf record -e cs_etm/timestamp/ -C 0 -- taskset -c 0 ls event syntax error: 'cs_etm/timestamp/' ___ Bad event or PMU
Unable to find PMU or event on a PMU of 'cs_etm'
event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term
event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term Run 'perf list' for a list of valid events
That's unfortunate and not what I expected. And I don't think it makes sense to remove that validation from Perf. The test uses "timestamp=1" so I didn't notice.
Can we accept that people are most likely using the defaults so timestamps are already on and they wouldn't be using it? The only real use case of that at the moment is to do timestamp=0 and that doesn't fail.
Although it's not the default for per-thread mode and I did find the OpenCSD HOWTO.md uses it as an example. timestamps make less sense in per-thread mode as you don't need to correlate between CPUs or watch for context switches.
Timestamps have a more specialised use in per-thread mode, they are as you say less essential for switching in the right context to decode the trace, and less relevant to BOLT/AutoFDO style usage where the trace is collapsed into a heat-map profile.
But trace can also be used to get a detailed timeline of CPU activity - a non-invasive timeline that can trace even through interrupt-disabled kernel code. And for that, having a global constant-frequency timebase becomes more useful, both in its own right, and to line up traces from each CPU with other CPUs and system-level traces. It's also the only way we have to indirectly observe CPU frequency adjustments. (Intel's Processor Trace, which is generally similar to ETM/ETE, has specific packets that trace CPU frequency changes.)
Al
I suppose we need to choose what's worse, breaking some subset of Perf commands in a slightly annoying way or having two separate options to control timestamps that you have to use together. I think it's 50/50, maybe with the breakage being the slightly better option.
This is worth mentioning so users can correctly assess what happens for any existing scripts they might have.
Based on this then the same command must set the timestamp to 1 - which will have the same effect as before as we do not want to break existing behaviour.
Mike
* - cc_threshold - Cycle count threshold value. If nothing is provided here or the providedvalue is 0, then the
default value i.e 0x100 will be used. If provided value isless than minimum cycles threshold
-- 2.34.1
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
CoreSight mailing list -- coresight@lists.linaro.org To unsubscribe send an email to coresight-leave@lists.linaro.org
On Wed, Nov 26, 2025 at 03:36:58PM +0000, Al Grant wrote:
[...]
As far as I recall when this command line parameter was a bool then: perf -e cs_etm/timestamp/ <program> is sufficient to turn on timestamping.
Hmm... with the latest perf, we must assign value to `timestamp`, otherwise perf will report error:
# /mnt/build/perf record -e cs_etm/timestamp/ -C 0 -- taskset -c 0 ls event syntax error: 'cs_etm/timestamp/' ___ Bad event or PMU
Unable to find PMU or event on a PMU of 'cs_etm'
event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term
event syntax error: 'cs_etm/timestamp/' ___ no value assigned for term Run 'perf list' for a list of valid events
That's unfortunate and not what I expected. And I don't think it makes sense to remove that validation from Perf. The test uses "timestamp=1" so I didn't notice.
Can we accept that people are most likely using the defaults so timestamps are already on and they wouldn't be using it? The only real use case of that at the moment is to do timestamp=0 and that doesn't fail.
Although it's not the default for per-thread mode and I did find the OpenCSD HOWTO.md uses it as an example. timestamps make less sense in per-thread mode as you don't need to correlate between CPUs or watch for context switches.
Timestamps have a more specialised use in per-thread mode, they are as you say less essential for switching in the right context to decode the trace, and less relevant to BOLT/AutoFDO style usage where the trace is collapsed into a heat-map profile.
But trace can also be used to get a detailed timeline of CPU activity - a non-invasive timeline that can trace even through interrupt-disabled kernel code. And for that, having a global constant-frequency timebase becomes more useful, both in its own right, and to line up traces from each CPU with other CPUs and system-level traces. It's also the only way we have to indirectly observe CPU frequency adjustments. (Intel's Processor Trace, which is generally similar to ETM/ETE, has specific packets that trace CPU frequency changes.)
Just clarify a bit to make sure us on the same page.
This series does not break timestamp functionality, it just changes the PMU format 'timestamp' from 1-bit to 4-bits for counter support. The updated 'timestamp' format still supports per-thread mode and CPU wise trace modes.
The only difference is now users need to specify 'cs_etm/timestamp=1/' rather than 'cs_etm/timestamp/' when enabling timestamp. Given PMU format is not an ABI, it is fine for me for the updated format.
To avoid confusion for users, a well-written document is deserved — which is exactly this patch for. And perf log would be helpful. I think we are well prepared for the change.
Thanks, Leo
Hi James
On Wed, 26 Nov 2025 at 10:57, 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. Set the interval using the value from the timestamp field. Granular control is not required, so save space in the config by interpreting it as 2 ^ timestamp. And then 4 bits (0 - 15) is enough to set the interval to be larger than the existing SYNC timestamp interval.
No sysfs mode support is needed for this attribute because counter generated timestamps are only configured for Perf mode.
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.h | 1 + drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 +++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 24d929428633..128f80bb1443 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"
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index c7bf73c8f2d7..0129b0502726 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -651,7 +651,7 @@ static void etm4_enable_sysfs_smp_call(void *info)
- +--------------+
|- +------v-------+
- | Counter x | (reload to 1 on underflow)
- | Counter x | (reload to 2 ^ timestamp on underflow)
- +--------------+
|- +------v--------------+
@@ -662,11 +662,25 @@ static void etm4_enable_sysfs_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;
struct perf_event_attr max_timestamp = {.ATTR_CFG_FLD_timestamp_CFG = U64_MAX,};/* timestamp may be 0 if deprecated_timestamp is used, so make min 1 */u8 ts_level = max(1, ATTR_CFG_GET_FLD(attr, timestamp));
I could be missing something here - but if we have a perf command line:
perf -e cs_etm/timestamp=0/
is this bit not changing that to timestamp=1 regardless? The docs (patch 13) indicate timestamp=0 to be timestamps off.
This command is used in test_arm_coresight.sh when testing the combination of options on the CS system.
Mike
/** Disable counter generated timestamps when timestamp == MAX. Leave* only SYNC timestamps.*/if (ts_level == ATTR_CFG_GET_FLD(&max_timestamp, timestamp))return 0; /* No point in trying if we don't have at least one counter */ if (!drvdata->nr_cntr)@@ -704,12 +718,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 - 1); /* * Trace Counter Control Register TRCCNTCTLRn@@ -799,7 +809,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
On 27/11/2025 3:48 pm, Mike Leach wrote:
Hi James
On Wed, 26 Nov 2025 at 10:57, 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. Set the interval using the value from the timestamp field. Granular control is not required, so save space in the config by interpreting it as 2 ^ timestamp. And then 4 bits (0 - 15) is enough to set the interval to be larger than the existing SYNC timestamp interval.
No sysfs mode support is needed for this attribute because counter generated timestamps are only configured for Perf mode.
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.h | 1 + drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 +++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 24d929428633..128f80bb1443 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"
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index c7bf73c8f2d7..0129b0502726 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -651,7 +651,7 @@ static void etm4_enable_sysfs_smp_call(void *info)
- +--------------+
|- +------v-------+
- | Counter x | (reload to 1 on underflow)
- | Counter x | (reload to 2 ^ timestamp on underflow)
- +--------------+
|- +------v--------------+
@@ -662,11 +662,25 @@ static void etm4_enable_sysfs_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)struct perf_event_attr max_timestamp = {.ATTR_CFG_FLD_timestamp_CFG = U64_MAX,};/* timestamp may be 0 if deprecated_timestamp is used, so make min 1 */u8 ts_level = max(1, ATTR_CFG_GET_FLD(attr, timestamp));I could be missing something here - but if we have a perf command line:
perf -e cs_etm/timestamp=0/
is this bit not changing that to timestamp=1 regardless? The docs (patch 13) indicate timestamp=0 to be timestamps off.
This command is used in test_arm_coresight.sh when testing the combination of options on the CS system.
Mike
No, no change. This function never gets called if either timestamp or deprecated_timestamp = 0.
timestamp=0 and timestamp=1 still behave exactly the same as before. The only new change is timestamp values > 1 and that the bits used in the config are different.
/** Disable counter generated timestamps when timestamp == MAX. Leave* only SYNC timestamps.*/if (ts_level == ATTR_CFG_GET_FLD(&max_timestamp, timestamp))return 0; /* No point in trying if we don't have at least one counter */ if (!drvdata->nr_cntr)@@ -704,12 +718,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 - 1); /* * Trace Counter Control Register TRCCNTCTLRn@@ -799,7 +809,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
OK I see it now. The real problem is that the logic to determine if we are to set a counter based timestamp is confusingly spread across two functions and two patches.
On Thu, 27 Nov 2025 at 15:48, Mike Leach mike.leach@linaro.org wrote:
Hi James
On Wed, 26 Nov 2025 at 10:57, 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. Set the interval using the value from the timestamp field. Granular control is not required, so save space in the config by interpreting it as 2 ^ timestamp. And then 4 bits (0 - 15) is enough to set the interval to be larger than the existing SYNC timestamp interval.
No sysfs mode support is needed for this attribute because counter generated timestamps are only configured for Perf mode.
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.h | 1 + drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 +++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 24d929428633..128f80bb1443 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"
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index c7bf73c8f2d7..0129b0502726 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -651,7 +651,7 @@ static void etm4_enable_sysfs_smp_call(void *info)
- +--------------+
|- +------v-------+
- | Counter x | (reload to 1 on underflow)
- | Counter x | (reload to 2 ^ timestamp on underflow)
- +--------------+
|- +------v--------------+
@@ -662,11 +662,25 @@ static void etm4_enable_sysfs_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)
Should pass ts_level in here
{ int ctridx; int rselector; struct etmv4_config *config = &drvdata->config;
struct perf_event_attr max_timestamp = {.ATTR_CFG_FLD_timestamp_CFG = U64_MAX,};/* timestamp may be 0 if deprecated_timestamp is used, so make min 1 */u8 ts_level = max(1, ATTR_CFG_GET_FLD(attr, timestamp));I could be missing something here - but if we have a perf command line:
perf -e cs_etm/timestamp=0/
is this bit not changing that to timestamp=1 regardless? The docs (patch 13) indicate timestamp=0 to be timestamps off.
This command is used in test_arm_coresight.sh when testing the combination of options on the CS system.
Mike
/** Disable counter generated timestamps when timestamp == MAX. Leave* only SYNC timestamps.*/if (ts_level == ATTR_CFG_GET_FLD(&max_timestamp, timestamp))return 0;
All the attr manipulation logic should be where this function is called not in here.
Mike
/* No point in trying if we don't have at least one counter */ if (!drvdata->nr_cntr)@@ -704,12 +718,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 - 1); /* * Trace Counter Control Register TRCCNTCTLRn@@ -799,7 +809,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
-
Al Grant -
James Clark -
Leo Yan -
Mike Leach