CoreSight

coresight@lists.linaro.org

1 participants
2839 discussions

Re: [PATCH 1/2] coresight: etm4x: fix inconsistencies with sysfs configration

by Leo Yan

On Tue, Mar 17, 2026 at 06:17:04PM +0000, Yeoreum Yun wrote: > The current ETM4x configuration via sysfs can lead to the following > inconsistencies: > > - If a configuration is modified via sysfs while a perf session is > active, the running configuration may differ between before > a sched-out and after a subsequent sched-in. > > - Once a perf session is enabled, some read-only register fields > (e.g., TRCSSCSR<n>) may not be reported correctly, > because drvdata->config is cleared while enabling with perf mode, > even though the information was previously read via etm4_init_arch_data(). > > To resolve these inconsistencies, the configuration should be separated into: > > - active_config, which represents the currently applied configuration > - config, which stores the settings configured via sysfs. > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > .../hwtracing/coresight/coresight-etm4x-cfg.c | 2 +- > .../coresight/coresight-etm4x-core.c | 45 +++++++++++-------- > drivers/hwtracing/coresight/coresight-etm4x.h | 2 + > 3 files changed, 30 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > index c302072b293a..84213d40d1ae 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > @@ -47,7 +47,7 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata, > struct cscfg_regval_csdev *reg_csdev, u32 offset) > { > int err = -EINVAL, idx; > - struct etmv4_config *drvcfg = &drvdata->config; > + struct etmv4_config *drvcfg = &drvdata->active_config; I'd suggest we leave out complex cfg things, we can refactor it later. In this series, let us first separate active_config and config, and keep using drvdata->config to save complex cfg ? > u32 off_mask; > > if (((offset >= TRCEVENTCTL0R) && (offset <= TRCVIPCSSCTLR)) || > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index d565a73f0042..c552129c4a0c 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -88,9 +88,11 @@ static int etm4_probe_cpu(unsigned int cpu); > */ > static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n) > { > + struct etmv4_config *config = &drvdata->active_config; > + > return (n < drvdata->nr_ss_cmp) && > drvdata->nr_pe && > - (drvdata->config.ss_status[n] & TRCSSCSRn_PC); > + (config->ss_status[n] & TRCSSCSRn_PC); As Suzuki suggested in another reply, we need to extract capabilities into a separate structure. I'd also extract status related registers into a new structure: struct etm4_cap { int nr_ss_cmp; bool pe_comparator; // TRCSSCSRn.PC bool dv_comparator; // TRCSSCSRn.DV bool da_comparator; // TRCSSCSRn.DA bool inst_comparator; // TRCSSCSRn.INST int ns_ex_level; int nr_pe; int nr_pe_cmp; int nr_resource; ... } struct etm4_status_reg { u32 ss_status[ETM_MAX_SS_CMP]; u32 cntr_val[ETMv4_MAX_CNTR]; } [...] > @@ -911,14 +915,17 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > > /* enable any config activated by configfs */ > cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); > + > + raw_spin_lock(&drvdata->spinlock); > + > + drvdata->active_config = drvdata->config; This is not an issue introduced by this patch, but we might need to consider to copy active config until it has acquired SYSFS mode. Otherwise, it might update config here but will disturb a perf session has been running. > @@ -2246,7 +2254,8 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) > if (!desc.name) > return -ENOMEM; > > - etm4_set_default(&drvdata->config); > + etm4_set_default(&drvdata->active_config); Should we set default values to drvdata->config ? My understanding is drvdata->active_config would be always set at the runtime, but "drvdata->config" should be initialized properly so it can be consumed by sysfs knobs. Thanks, Leo

5 months

Re: [PATCH v2] coresight: tpdm: fix invalid MMIO access issue

by Suzuki K Poulose

On Tue, 07 Apr 2026 19:09:05 +0800, Jie Gan wrote: > Create the csdev_access struct only when a valid MMIO resource is > available. In tpdm_probe(), base is uninitialized for static TPDM > instances that lack an MMIO resource, causing csdev_access to be > created with a garbage address. > > So far there has no register access for static instance, but this > change helps mitigate potential risks in the future. > > [...] Applied, thanks! [1/1] coresight: tpdm: fix invalid MMIO access issue https://git.kernel.org/coresight/c/971f3474f889 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

5 months

Re: [PATCH] coresight: tpdm: fix invalid MMIO access issue

by Leo Yan

Hi Jie, On Tue, Apr 07, 2026 at 04:33:22PM +0800, Jie Gan wrote: > On 4/7/2026 4:10 PM, Leo Yan wrote: > > On Tue, Apr 07, 2026 at 12:47:11PM +0800, Jie Gan wrote: > > > Create the csdev_access struct only when a valid MMIO resource is > > > available. In tpdm_probe(), base is uninitialized for static TPDM > > > instances that lack an MMIO resource, causing csdev_access to be > > > created with a garbage address and potentially leading to > > > unexpected issues. > > > > This patch itself is fine for me. However, I am wandering if this > > is sufficient. > > > > As mentioned "potentially leading to unexpected issues", can I > > understand some code pieces access register with uninitialized base? > > If so, you would also explictly add coresight_is_static_tpdm() to > > prevent register access. > > > > Actually, we havent MMIO access for the static TPDM device, So no issues are > observed. The commit message here may be misleading. do I need rephrase the > commit message? Yes, good to clarify a bit in commit log: "So far there has no register access for static instance, but this change helps mitigate potential risks in the future." With this: Reviewed-by: Leo Yan <leo.yan(a)arm.com>

5 months

Re: [PATCH] coresight: tpdm: fix invalid MMIO access issue

by Leo Yan

On Tue, Apr 07, 2026 at 12:47:11PM +0800, Jie Gan wrote: > Create the csdev_access struct only when a valid MMIO resource is > available. In tpdm_probe(), base is uninitialized for static TPDM > instances that lack an MMIO resource, causing csdev_access to be > created with a garbage address and potentially leading to > unexpected issues. This patch itself is fine for me. However, I am wandering if this is sufficient. As mentioned "potentially leading to unexpected issues", can I understand some code pieces access register with uninitialized base? If so, you would also explictly add coresight_is_static_tpdm() to prevent register access. Thanks, Leo > Fixes: 14ae052f7947 ("coresight: tpdm: add static tpdm support") > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-tpdm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > index 9b16f368a58b..eaf7210af648 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.c > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -1430,6 +1430,7 @@ static int tpdm_probe(struct device *dev, struct resource *res) > if (ret) > return ret; > > + desc.access = CSDEV_ACCESS_IOMEM(base); > if (tpdm_has_dsb_dataset(drvdata)) > of_property_read_u32(drvdata->dev->of_node, > "qcom,dsb-msrs-num", &drvdata->dsb_msr_num); > @@ -1452,7 +1453,6 @@ static int tpdm_probe(struct device *dev, struct resource *res) > desc.ops = &tpdm_cs_ops; > desc.pdata = dev->platform_data; > desc.dev = dev; > - desc.access = CSDEV_ACCESS_IOMEM(base); > if (res) > desc.groups = tpdm_attr_grps; > else > > --- > base-commit: 816f193dd0d95246f208590924dd962b192def78 > change-id: 20260407-fix-potential-issue-in-tpdm-b07b44416051 > > Best regards, > -- > Jie Gan <jie.gan(a)oss.qualcomm.com> >

5 months

[PATCH v9 00/20] CoreSight: Refactor power management for CoreSight path

by Leo Yan

This series focuses on CoreSight path power management. The changes can be divided into four parts for review: Patches 01 - 04: Refactor the CPU ID storing in csdev, later patches consume csdev->cpu. Move CPU lock to sysfs layer so it is safe for later changes. Patches 05 - 09: Refactor the CPU idle flow with moving common code into the CoreSight core layer. Patches 10 - 15: Refactor path enabling and disabling with range, add path control during CPU idle. Patches 16 - 17: Support the sink (TRBE) control during CPU idle. Patches 18 - 20: Move CPU hotplug into the core layer, and fix sysfs mode for hotplug. This series is rebased on the coresight-next branch and has been verified on Juno-r2 (ETM + ETR) and FVP RevC (ETE + TRBE). Built successfully for armv7 (ARCH=arm). --- Changes in v9: - Changed to use per-CPU path pointer with lockless access. - Removed the change for adding csdev->path, the related refactoring will be sent separately. - Re-orged patches to avoid intermediate breakage (sashiko). - Link to v8: https://lore.kernel.org/r/20260325-arm_coresight_path_power_management_impr… Changes in v8: - Moved the "cpu" field in coresight_device for better pack with new patch 01 (Suzuki). - Added check if not set CPU for per_cpu_source/per_cpu_sink (Suzuki). - Renamed spinlock name in syscfg (Suzuki). - Refactored paired enable and disable path with new patches 10 and 12 (Suzuki). - Link to v7: https://lore.kernel.org/r/20260320-arm_coresight_path_power_management_impr… Changes in v7: - Added a flag in coresight_desc to indicate CPU bound device (Suzuki). - Used coresight_mutex to protect per-CPU source pointer (Suzuki). - Added a spinlock for exclusive access per-CPU source pointer (Suzuki). - Dropped .pm_is_needed() callback (Suzuki). - Supported range in path enabling / disabling (Suzuki). - Gathered test and review tags (Levi / James). - Link to v6: https://lore.kernel.org/r/20260305-arm_coresight_path_power_management_impr… Signed-off-by: Leo Yan <leo.yan(a)arm.com> --- Leo Yan (19): coresight: Extract device init into coresight_init_device() coresight: Populate CPU ID into coresight_device coresight: Remove .cpu_id() callback from source ops coresight: Take hotplug lock in enable_source_store() for Sysfs mode coresight: etm4x: Set per-CPU path on local CPU coresight: etm3x: Set per-CPU path on local CPU coresight: Register CPU PM notifier in core layer coresight: etm4x: Hook CPU PM callbacks coresight: etm4x: Remove redundant condition checks in save and restore coresight: syscfg: Use spinlock to protect active variables coresight: Move source helper disabling to coresight_disable_path() coresight: Control path with range coresight: Use helpers to fetch first and last nodes coresight: Introduce coresight_enable_source() helper coresight: Control path during CPU idle coresight: Add PM callbacks for sink device coresight: sysfs: Increment refcount only for system tracers coresight: Move CPU hotplug callbacks to core layer coresight: sysfs: Validate CPU online status for per-CPU sources Yabin Cui (1): coresight: trbe: Save and restore state across CPU low power state drivers/hwtracing/coresight/coresight-catu.c | 2 +- drivers/hwtracing/coresight/coresight-core.c | 371 ++++++++++++++++++--- drivers/hwtracing/coresight/coresight-cti-core.c | 9 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 4 +- drivers/hwtracing/coresight/coresight-etm3x-core.c | 73 +--- drivers/hwtracing/coresight/coresight-etm4x-core.c | 156 ++------- drivers/hwtracing/coresight/coresight-priv.h | 4 + drivers/hwtracing/coresight/coresight-syscfg.c | 21 +- drivers/hwtracing/coresight/coresight-syscfg.h | 2 + drivers/hwtracing/coresight/coresight-sysfs.c | 50 ++- drivers/hwtracing/coresight/coresight-trbe.c | 61 +++- include/linux/coresight.h | 13 +- 12 files changed, 503 insertions(+), 263 deletions(-) --- base-commit: ec687ba84000d7d50cf243558041f6729d1d8119 change-id: 20251104-arm_coresight_path_power_management_improvement-dab4966f8280 Best regards, -- Leo Yan <leo.yan(a)arm.com>

5 months, 1 week

Re: [PATCH 1/2] coresight: etm4x: fix inconsistencies with sysfs configration

by Suzuki K Poulose

Hi Levi On 17/03/2026 18:17, Yeoreum Yun wrote: > The current ETM4x configuration via sysfs can lead to the following > inconsistencies: > > - If a configuration is modified via sysfs while a perf session is > active, the running configuration may differ between before > a sched-out and after a subsequent sched-in. > > - Once a perf session is enabled, some read-only register fields > (e.g., TRCSSCSR<n>) may not be reported correctly, > because drvdata->config is cleared while enabling with perf mode, > even though the information was previously read via etm4_init_arch_data(). Thank you for the patch. The patch looks good to me. Some minor comments below. > > To resolve these inconsistencies, the configuration should be separated into: > > - active_config, which represents the currently applied configuration To be more precise, it is the "Applied configuration for the current session" > - config, which stores the settings configured via sysfs. While we are at it, should we stop using the drvdata->config for the "capabilities" for the ETM (e.g., TRCSSCSRn in ss_status ?) Instead we could save it in "drvdata->ss_status". This keeps everything separated: 1. Boot time probed capabilities of ETM 2. Sysfs configuration for the next Run 3. Current active configuration for the ETM (sysfs or perf) Suzuki > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > .../hwtracing/coresight/coresight-etm4x-cfg.c | 2 +- > .../coresight/coresight-etm4x-core.c | 45 +++++++++++-------- > drivers/hwtracing/coresight/coresight-etm4x.h | 2 + > 3 files changed, 30 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > index c302072b293a..84213d40d1ae 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > @@ -47,7 +47,7 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata, > struct cscfg_regval_csdev *reg_csdev, u32 offset) > { > int err = -EINVAL, idx; > - struct etmv4_config *drvcfg = &drvdata->config; > + struct etmv4_config *drvcfg = &drvdata->active_config; > u32 off_mask; > > if (((offset >= TRCEVENTCTL0R) && (offset <= TRCVIPCSSCTLR)) || > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index d565a73f0042..c552129c4a0c 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -88,9 +88,11 @@ static int etm4_probe_cpu(unsigned int cpu); > */ > static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n) > { > + struct etmv4_config *config = &drvdata->active_config; > + > return (n < drvdata->nr_ss_cmp) && > drvdata->nr_pe && > - (drvdata->config.ss_status[n] & TRCSSCSRn_PC); > + (config->ss_status[n] & TRCSSCSRn_PC); > } > > u64 etm4x_sysreg_read(u32 offset, bool _relaxed, bool _64bit) > @@ -266,10 +268,11 @@ static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata) > static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata) > { > u64 trfcr = drvdata->trfcr; > + struct etmv4_config *config = &drvdata->active_config; > > - if (drvdata->config.mode & ETM_MODE_EXCL_KERN) > + if (config->mode & ETM_MODE_EXCL_KERN) > trfcr &= ~TRFCR_EL1_ExTRE; > - if (drvdata->config.mode & ETM_MODE_EXCL_USER) > + if (config->mode & ETM_MODE_EXCL_USER) > trfcr &= ~TRFCR_EL1_E0TRE; > > return trfcr; > @@ -277,7 +280,7 @@ static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata) > > /* > * etm4x_allow_trace - Allow CPU tracing in the respective ELs, > - * as configured by the drvdata->config.mode for the current > + * as configured by the drvdata->active_config.mode for the current > * session. Even though we have TRCVICTLR bits to filter the > * trace in the ELs, it doesn't prevent the ETM from generating > * a packet (e.g, TraceInfo) that might contain the addresses from > @@ -288,12 +291,13 @@ static u64 etm4x_get_kern_user_filter(struct etmv4_drvdata *drvdata) > static void etm4x_allow_trace(struct etmv4_drvdata *drvdata) > { > u64 trfcr, guest_trfcr; > + struct etmv4_config *config = &drvdata->active_config; > > /* If the CPU doesn't support FEAT_TRF, nothing to do */ > if (!drvdata->trfcr) > return; > > - if (drvdata->config.mode & ETM_MODE_EXCL_HOST) > + if (config->mode & ETM_MODE_EXCL_HOST) > trfcr = drvdata->trfcr & ~(TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE); > else > trfcr = etm4x_get_kern_user_filter(drvdata); > @@ -301,7 +305,7 @@ static void etm4x_allow_trace(struct etmv4_drvdata *drvdata) > write_trfcr(trfcr); > > /* Set filters for guests and pass to KVM */ > - if (drvdata->config.mode & ETM_MODE_EXCL_GUEST) > + if (config->mode & ETM_MODE_EXCL_GUEST) > guest_trfcr = drvdata->trfcr & ~(TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE); > else > guest_trfcr = etm4x_get_kern_user_filter(drvdata); > @@ -494,7 +498,7 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata) > static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > { > int i, rc; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > struct coresight_device *csdev = drvdata->csdev; > struct device *etm_dev = &csdev->dev; > struct csdev_access *csa = &csdev->access; > @@ -666,7 +670,7 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata, > { > int ctridx; > int rselector; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > > /* No point in trying if we don't have at least one counter */ > if (!drvdata->nr_cntr) > @@ -749,7 +753,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev, > { > int ret = 0; > struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > struct perf_event_attr max_timestamp = { > .ATTR_CFG_FLD_timestamp_CFG = U64_MAX, > }; > @@ -911,14 +915,17 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > > /* enable any config activated by configfs */ > cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); > + > + raw_spin_lock(&drvdata->spinlock); > + > + drvdata->active_config = drvdata->config; > + > if (cfg_hash) { > ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); > if (ret) > return ret; > } > > - raw_spin_lock(&drvdata->spinlock); > - > drvdata->trcid = path->trace_id; > > /* Tracer will never be paused in sysfs mode */ > @@ -1028,7 +1035,7 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata) > static void etm4_disable_hw(struct etmv4_drvdata *drvdata) > { > u32 control; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > struct coresight_device *csdev = drvdata->csdev; > struct csdev_access *csa = &csdev->access; > int i; > @@ -1374,12 +1381,14 @@ static void etm4_init_arch_data(void *info) > u32 etmidr5; > struct etm4_init_arg *init_arg = info; > struct etmv4_drvdata *drvdata; > + struct etmv4_config *config; > struct csdev_access *csa; > struct device *dev = init_arg->dev; > int i; > > drvdata = dev_get_drvdata(init_arg->dev); > csa = init_arg->csa; > + config = &drvdata->active_config; > > /* > * If we are unable to detect the access mechanism, > @@ -1440,7 +1449,7 @@ static void etm4_init_arch_data(void *info) > > /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */ > drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3); > - drvdata->config.s_ex_level = drvdata->s_ex_level; > + config->s_ex_level = drvdata->s_ex_level; > /* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */ > drvdata->ns_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_NS_MASK, etmidr3); > /* > @@ -1491,8 +1500,7 @@ static void etm4_init_arch_data(void *info) > */ > drvdata->nr_ss_cmp = FIELD_GET(TRCIDR4_NUMSSCC_MASK, etmidr4); > for (i = 0; i < drvdata->nr_ss_cmp; i++) { > - drvdata->config.ss_status[i] = > - etm4x_relaxed_read32(csa, TRCSSCSRn(i)); > + config->ss_status[i] = etm4x_relaxed_read32(csa, TRCSSCSRn(i)); > } > /* NUMCIDC, bits[27:24] number of Context ID comparators for tracing */ > drvdata->numcidc = FIELD_GET(TRCIDR4_NUMCIDC_MASK, etmidr4); > @@ -1685,7 +1693,7 @@ static void etm4_set_default(struct etmv4_config *config) > static int etm4_get_next_comparator(struct etmv4_drvdata *drvdata, u32 type) > { > int nr_comparator, index = 0; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > > /* > * nr_addr_cmp holds the number of comparator _pair_, so time 2 > @@ -1726,7 +1734,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata, > { > int i, comparator, ret = 0; > u64 address; > - struct etmv4_config *config = &drvdata->config; > + struct etmv4_config *config = &drvdata->active_config; > struct etm_filters *filters = event->hw.addr_filters; > > if (!filters) > @@ -2246,7 +2254,8 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) > if (!desc.name) > return -ENOMEM; > > - etm4_set_default(&drvdata->config); > + etm4_set_default(&drvdata->active_config); > + drvdata->config = drvdata->active_config; > > pdata = coresight_get_platform_data(dev); > if (IS_ERR(pdata)) > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index 89d81ce4e04e..76beb896f188 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -1022,6 +1022,7 @@ struct etmv4_save_state { > * allows tracing at all ELs. We don't want to compute this > * at runtime, due to the additional setting of TRFCR_CX when > * in EL2. Otherwise, 0. > + * @active_config: structure holding current applied configuration parameters. > * @config: structure holding configuration parameters. > * @save_state: State to be preserved across power loss > * @skip_power_up: Indicates if an implementation can skip powering up > @@ -1079,6 +1080,7 @@ struct etmv4_drvdata { > bool skip_power_up : 1; > bool paused : 1; > u64 trfcr; > + struct etmv4_config active_config; > struct etmv4_config config; > struct etmv4_save_state *save_state; > DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);

5 months, 1 week

Re: [PATCH v4] coresight: tpdm: add traceid_show for checking traceid

by Suzuki K Poulose

On Tue, 31 Mar 2026 18:05:22 +0800, Jie Gan wrote: > Save the trace ID in drvdata during TPDM enablement and expose it > to userspace to support trace data parsing. > > The TPDM device’s trace ID corresponds to the trace ID allocated > to the connected TPDA device. > > > [...] Applied, thanks! [1/1] coresight: tpdm: add traceid_show for checking traceid commit: ec687ba84000d7d50cf243558041f6729d1d8119 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

5 months, 1 week

Re: [PATCH v4] coresight: tpdm: add traceid_show for checking traceid

by James Clark

On 31/03/2026 11:05 am, Jie Gan wrote: > Save the trace ID in drvdata during TPDM enablement and expose it > to userspace to support trace data parsing. > > The TPDM device’s trace ID corresponds to the trace ID allocated > to the connected TPDA device. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > Changes in v4: > 1. Return -EINVAL if the traceid is 0. > - Link to v3: https://lore.kernel.org/r/20260325-add-traceid-show-for-tpdm-v3-1-0eb836d4e… > > Changes in v3: > 1. Only allow user to read the traceid while the TPDM device is enabled. > - Link to v2: https://lore.kernel.org/r/20260316-add-traceid-show-for-tpdm-v2-1-1dec2a67e… > > Changes in V2: > 1. Use sysfs_emit instead of sprintf. > Link to V1 - https://lore.kernel.org/all/20260306-add-traceid-show-for-tpdm-v1-1-0658a8e… > --- > .../ABI/testing/sysfs-bus-coresight-devices-tpdm | 10 +++++++ > drivers/hwtracing/coresight/coresight-tpdm.c | 34 +++++++++++++++++++++- > drivers/hwtracing/coresight/coresight-tpdm.h | 2 ++ > 3 files changed, 45 insertions(+), 1 deletion(-) > Reviewed-by: James Clark <james.clark(a)linaro.org> > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > index f8016df64532..bc36ba32c900 100644 > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > @@ -278,3 +278,13 @@ Date: Aug 2025 > KernelVersion 6.18 > Contact: Mao Jinlong <quic_jinlmao(a)quicinc.com> > Description: (Read) Show hardware context information of device. > + > +What: /sys/bus/coresight/devices/<tpdm-name>/traceid > +Date: March 2026 > +KernelVersion: 7.1 > +Contact: Jie Gan <jie.gan(a)oss.qualcomm.com> > +Description: > + (R) Show the trace ID that will appear in the trace stream > + coming from this TPDM. The trace ID is inherited from the > + connected TPDA device and is fixed for the lifetime of the > + device. Returns -EINVAL if the device has not been enabled yet. > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > index da77bdaad0a4..9b16f368a58b 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.c > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -481,7 +481,7 @@ static void __tpdm_enable(struct tpdm_drvdata *drvdata) > > static int tpdm_enable(struct coresight_device *csdev, struct perf_event *event, > enum cs_mode mode, > - __maybe_unused struct coresight_path *path) > + struct coresight_path *path) > { > struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > > @@ -497,6 +497,7 @@ static int tpdm_enable(struct coresight_device *csdev, struct perf_event *event, > } > > __tpdm_enable(drvdata); > + drvdata->traceid = path->trace_id; > drvdata->enable = true; > spin_unlock(&drvdata->spinlock); > > @@ -693,6 +694,29 @@ static struct attribute_group tpdm_attr_grp = { > .attrs = tpdm_attrs, > }; > > +static ssize_t traceid_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + unsigned long val; > + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + val = drvdata->traceid; > + if (!val) > + return -EINVAL; > + > + return sysfs_emit(buf, "%#lx\n", val); > +} > +static DEVICE_ATTR_RO(traceid); > + > +static struct attribute *traceid_attrs[] = { > + &dev_attr_traceid.attr, > + NULL, > +}; > + > +static struct attribute_group traceid_attr_grp = { > + .attrs = traceid_attrs, > +}; > + > static ssize_t dsb_mode_show(struct device *dev, > struct device_attribute *attr, > char *buf) > @@ -1367,6 +1391,12 @@ static const struct attribute_group *tpdm_attr_grps[] = { > &tpdm_cmb_patt_grp, > &tpdm_cmb_msr_grp, > &tpdm_mcmb_attr_grp, > + &traceid_attr_grp, > + NULL, > +}; > + > +static const struct attribute_group *static_tpdm_attr_grps[] = { > + &traceid_attr_grp, > NULL, > }; > > @@ -1425,6 +1455,8 @@ static int tpdm_probe(struct device *dev, struct resource *res) > desc.access = CSDEV_ACCESS_IOMEM(base); > if (res) > desc.groups = tpdm_attr_grps; > + else > + desc.groups = static_tpdm_attr_grps; > drvdata->csdev = coresight_register(&desc); > if (IS_ERR(drvdata->csdev)) > return PTR_ERR(drvdata->csdev); > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > index 2867f3ab8186..11da64e1ade8 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.h > +++ b/drivers/hwtracing/coresight/coresight-tpdm.h > @@ -300,6 +300,7 @@ struct cmb_dataset { > * @cmb Specifics associated to TPDM CMB. > * @dsb_msr_num Number of MSR supported by DSB TPDM > * @cmb_msr_num Number of MSR supported by CMB TPDM > + * @traceid Trace ID of the path. > */ > > struct tpdm_drvdata { > @@ -313,6 +314,7 @@ struct tpdm_drvdata { > struct cmb_dataset *cmb; > u32 dsb_msr_num; > u32 cmb_msr_num; > + u8 traceid; > }; > > /* Enumerate members of various datasets */ > > --- > base-commit: b84a0ebe421ca56995ff78b66307667b62b3a900 > change-id: 20260316-add-traceid-show-for-tpdm-88d040651f00 > > Best regards,

5 months, 1 week

Re: [PATCH v3] coresight: tpdm: add traceid_show for checking traceid

by James Clark

On 31/03/2026 10:33 am, Jie Gan wrote: > > > On 3/31/2026 5:26 PM, James Clark wrote: >> >> >> On 31/03/2026 4:18 am, Jie Gan wrote: >>> Hi James, >>> >>> On 3/31/2026 9:29 AM, Jie Gan wrote: >>>> >>>> Hi James, >>>> >>>> On 3/30/2026 10:55 PM, James Clark wrote: >>>>> >>>>> >>>>> On 25/03/2026 3:10 am, Jie Gan wrote: >>>>>> Save the trace ID in drvdata during TPDM enablement and expose it >>>>>> to userspace to support trace data parsing. >>>>>> >>>>>> The TPDM device’s trace ID corresponds to the trace ID allocated >>>>>> to the connected TPDA device. >>>>>> >>>>>> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>>>>> --- >>>>>> Changes in v3: >>>>>> 1. Only allow user to read the traceid while the TPDM device is >>>>>> enabled. >>>>>> - Link to v2: https://lore.kernel.org/r/20260316-add-traceid-show- >>>>>> for- tpdm-v2-1-1dec2a67e4ed(a)oss.qualcomm.com >>>>>> >>>>>> Changes in V2: >>>>>> 1. Use sysfs_emit instead of sprintf. >>>>>> Link to V1 - https://lore.kernel.org/all/20260306-add-traceid- >>>>>> show- for-tpdm-v1-1-0658a8edb972(a)oss.qualcomm.com/ >>>>>> --- >>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 34 ++++++++++++++ >>>>>> + + + + +++++++++- >>>>>> drivers/hwtracing/coresight/coresight-tpdm.h | 2 ++ >>>>>> 2 files changed, 35 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/ >>>>>> drivers/ hwtracing/coresight/coresight-tpdm.c >>>>>> index da77bdaad0a4..c8339b973bfc 100644 >>>>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >>>>>> @@ -481,7 +481,7 @@ static void __tpdm_enable(struct tpdm_drvdata >>>>>> *drvdata) >>>>>> static int tpdm_enable(struct coresight_device *csdev, struct >>>>>> perf_event *event, >>>>>> enum cs_mode mode, >>>>>> - __maybe_unused struct coresight_path *path) >>>>>> + struct coresight_path *path) >>>>>> { >>>>>> struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev- >>>>>> >dev.parent); >>>>>> @@ -497,6 +497,7 @@ static int tpdm_enable(struct coresight_device >>>>>> *csdev, struct perf_event *event, >>>>>> } >>>>>> __tpdm_enable(drvdata); >>>>>> + drvdata->traceid = path->trace_id; >>>>>> drvdata->enable = true; >>>>>> spin_unlock(&drvdata->spinlock); >>>>>> @@ -693,6 +694,29 @@ static struct attribute_group tpdm_attr_grp = { >>>>>> .attrs = tpdm_attrs, >>>>>> }; >>>>>> +static ssize_t traceid_show(struct device *dev, >>>>>> + struct device_attribute *attr, char *buf) >>>>>> +{ >>>>>> + unsigned long val; >>>>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); >>>>>> + >>>>>> + if (coresight_get_mode(drvdata->csdev) == CS_MODE_DISABLED) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + val = drvdata->traceid; >>>>> >>>>> You probably need to take the coresight_mutex here otherwise you >>>>> could still return an invalid or stale value despite checking the >>>>> mode. >>>>> >>>> >>>> Acked. I have missed this potential race condition. >>>> >>>>> There might also be some value in it returning the last used trace >>>>> ID even if the mode isn't enabled anymore. Because you can still >>>>> read out of the sink after disabling, so it makes more sense for a >>>>> script to read it at that point rather than when it's enabled. >>>>> Also, you probably don't want to be doing other things in your >>>>> script in the point between enabling and disabling. >>>> >>>> That's making sense. I shouldnt add such restriction for the read >>>> process. >>>> >>> >>> I missed one point in last message. >>> >>> Is that acceptable to export the coresight_mutex from the core module? >>> Currently, the coresight_mutex is used within the module only. >>> >>> Thanks, >>> Jie >>> >> >> If the plan is to only check for non-zero trace ID and not check the >> mode any more then you don't need to lock. Maybe lets see what Suzuki >> thinks about returning the last trace ID though as it was his idea to >> add -EINVAL. >> > > Sure. The trace ID is allocated during TPDA device probe and remains > unchanged for the entire lifetime of the device. > > Thanks, > Jie > > Oh I missed that because it's assigned every time in tpdm_enable(). In that case I don't see why it can't be just: val = drvdata->traceid; if (!val) return -EINVAL; return sysfs_emit(...) >>>> Scenarios for reading: >>>> 1. device is enabled -> trace ID is valid >>>> 2. device is enabled then disabled -> trace ID is valid for the last >>>> trace event >>>> 3. device is never enabled -> invalid trace ID (value 0) >>>> >>>> we only need to check the validation of the trace ID. >>>> >>>> mutex_lock(&coresight_mutex); >>>> val = drvdata->traceid; >>>> mutex_unlock(&coresight_mutex); >>>> >>>> if (!val) >>>> return -EINVAL; >>>> >>>> return sysfs_emit(buf, "%#lx\n", val); >>>> >>>> Thanks, >>>> Jie >>>> >>>>> >>>>>> + return sysfs_emit(buf, "%#lx\n", val); >>>>>> +} >>>>>> +static DEVICE_ATTR_RO(traceid); >>>>>> + >>>>>> +static struct attribute *traceid_attrs[] = { >>>>>> + &dev_attr_traceid.attr, >>>>>> + NULL, >>>>>> +}; >>>>>> + >>>>>> +static struct attribute_group traceid_attr_grp = { >>>>>> + .attrs = traceid_attrs, >>>>>> +}; >>>>>> + >>>>>> static ssize_t dsb_mode_show(struct device *dev, >>>>>> struct device_attribute *attr, >>>>>> char *buf) >>>>>> @@ -1367,6 +1391,12 @@ static const struct attribute_group >>>>>> *tpdm_attr_grps[] = { >>>>>> &tpdm_cmb_patt_grp, >>>>>> &tpdm_cmb_msr_grp, >>>>>> &tpdm_mcmb_attr_grp, >>>>>> + &traceid_attr_grp, >>>>>> + NULL, >>>>>> +}; >>>>>> + >>>>>> +static const struct attribute_group *static_tpdm_attr_grps[] = { >>>>>> + &traceid_attr_grp, >>>>>> NULL, >>>>>> }; >>>>>> @@ -1425,6 +1455,8 @@ static int tpdm_probe(struct device *dev, >>>>>> struct resource *res) >>>>>> desc.access = CSDEV_ACCESS_IOMEM(base); >>>>>> if (res) >>>>>> desc.groups = tpdm_attr_grps; >>>>>> + else >>>>>> + desc.groups = static_tpdm_attr_grps; >>>>>> drvdata->csdev = coresight_register(&desc); >>>>>> if (IS_ERR(drvdata->csdev)) >>>>>> return PTR_ERR(drvdata->csdev); >>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/ >>>>>> drivers/ hwtracing/coresight/coresight-tpdm.h >>>>>> index 2867f3ab8186..11da64e1ade8 100644 >>>>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h >>>>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h >>>>>> @@ -300,6 +300,7 @@ struct cmb_dataset { >>>>>> * @cmb Specifics associated to TPDM CMB. >>>>>> * @dsb_msr_num Number of MSR supported by DSB TPDM >>>>>> * @cmb_msr_num Number of MSR supported by CMB TPDM >>>>>> + * @traceid Trace ID of the path. >>>>>> */ >>>>>> struct tpdm_drvdata { >>>>>> @@ -313,6 +314,7 @@ struct tpdm_drvdata { >>>>>> struct cmb_dataset *cmb; >>>>>> u32 dsb_msr_num; >>>>>> u32 cmb_msr_num; >>>>>> + u8 traceid; >>>>>> }; >>>>>> /* Enumerate members of various datasets */ >>>>>> >>>>>> --- >>>>>> base-commit: b84a0ebe421ca56995ff78b66307667b62b3a900 >>>>>> change-id: 20260316-add-traceid-show-for-tpdm-88d040651f00 >>>>>> >>>>>> Best regards, >>>>> >>>> >>> >> >

5 months, 1 week

Re: [PATCH v3] coresight: tpdm: add traceid_show for checking traceid

by James Clark

On 31/03/2026 4:18 am, Jie Gan wrote: > Hi James, > > On 3/31/2026 9:29 AM, Jie Gan wrote: >> >> Hi James, >> >> On 3/30/2026 10:55 PM, James Clark wrote: >>> >>> >>> On 25/03/2026 3:10 am, Jie Gan wrote: >>>> Save the trace ID in drvdata during TPDM enablement and expose it >>>> to userspace to support trace data parsing. >>>> >>>> The TPDM device’s trace ID corresponds to the trace ID allocated >>>> to the connected TPDA device. >>>> >>>> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>>> --- >>>> Changes in v3: >>>> 1. Only allow user to read the traceid while the TPDM device is >>>> enabled. >>>> - Link to v2: https://lore.kernel.org/r/20260316-add-traceid-show- >>>> for- tpdm-v2-1-1dec2a67e4ed(a)oss.qualcomm.com >>>> >>>> Changes in V2: >>>> 1. Use sysfs_emit instead of sprintf. >>>> Link to V1 - https://lore.kernel.org/all/20260306-add-traceid-show- >>>> for-tpdm-v1-1-0658a8edb972(a)oss.qualcomm.com/ >>>> --- >>>> drivers/hwtracing/coresight/coresight-tpdm.c | 34 ++++++++++++++++ >>>> + + +++++++++- >>>> drivers/hwtracing/coresight/coresight-tpdm.h | 2 ++ >>>> 2 files changed, 35 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/ >>>> hwtracing/coresight/coresight-tpdm.c >>>> index da77bdaad0a4..c8339b973bfc 100644 >>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >>>> @@ -481,7 +481,7 @@ static void __tpdm_enable(struct tpdm_drvdata >>>> *drvdata) >>>> static int tpdm_enable(struct coresight_device *csdev, struct >>>> perf_event *event, >>>> enum cs_mode mode, >>>> - __maybe_unused struct coresight_path *path) >>>> + struct coresight_path *path) >>>> { >>>> struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev- >>>> >dev.parent); >>>> @@ -497,6 +497,7 @@ static int tpdm_enable(struct coresight_device >>>> *csdev, struct perf_event *event, >>>> } >>>> __tpdm_enable(drvdata); >>>> + drvdata->traceid = path->trace_id; >>>> drvdata->enable = true; >>>> spin_unlock(&drvdata->spinlock); >>>> @@ -693,6 +694,29 @@ static struct attribute_group tpdm_attr_grp = { >>>> .attrs = tpdm_attrs, >>>> }; >>>> +static ssize_t traceid_show(struct device *dev, >>>> + struct device_attribute *attr, char *buf) >>>> +{ >>>> + unsigned long val; >>>> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); >>>> + >>>> + if (coresight_get_mode(drvdata->csdev) == CS_MODE_DISABLED) >>>> + return -EINVAL; >>>> + >>>> + val = drvdata->traceid; >>> >>> You probably need to take the coresight_mutex here otherwise you >>> could still return an invalid or stale value despite checking the mode. >>> >> >> Acked. I have missed this potential race condition. >> >>> There might also be some value in it returning the last used trace ID >>> even if the mode isn't enabled anymore. Because you can still read >>> out of the sink after disabling, so it makes more sense for a script >>> to read it at that point rather than when it's enabled. Also, you >>> probably don't want to be doing other things in your script in the >>> point between enabling and disabling. >> >> That's making sense. I shouldnt add such restriction for the read >> process. >> > > I missed one point in last message. > > Is that acceptable to export the coresight_mutex from the core module? > Currently, the coresight_mutex is used within the module only. > > Thanks, > Jie > If the plan is to only check for non-zero trace ID and not check the mode any more then you don't need to lock. Maybe lets see what Suzuki thinks about returning the last trace ID though as it was his idea to add -EINVAL. >> Scenarios for reading: >> 1. device is enabled -> trace ID is valid >> 2. device is enabled then disabled -> trace ID is valid for the last >> trace event >> 3. device is never enabled -> invalid trace ID (value 0) >> >> we only need to check the validation of the trace ID. >> >> mutex_lock(&coresight_mutex); >> val = drvdata->traceid; >> mutex_unlock(&coresight_mutex); >> >> if (!val) >> return -EINVAL; >> >> return sysfs_emit(buf, "%#lx\n", val); >> >> Thanks, >> Jie >> >>> >>>> + return sysfs_emit(buf, "%#lx\n", val); >>>> +} >>>> +static DEVICE_ATTR_RO(traceid); >>>> + >>>> +static struct attribute *traceid_attrs[] = { >>>> + &dev_attr_traceid.attr, >>>> + NULL, >>>> +}; >>>> + >>>> +static struct attribute_group traceid_attr_grp = { >>>> + .attrs = traceid_attrs, >>>> +}; >>>> + >>>> static ssize_t dsb_mode_show(struct device *dev, >>>> struct device_attribute *attr, >>>> char *buf) >>>> @@ -1367,6 +1391,12 @@ static const struct attribute_group >>>> *tpdm_attr_grps[] = { >>>> &tpdm_cmb_patt_grp, >>>> &tpdm_cmb_msr_grp, >>>> &tpdm_mcmb_attr_grp, >>>> + &traceid_attr_grp, >>>> + NULL, >>>> +}; >>>> + >>>> +static const struct attribute_group *static_tpdm_attr_grps[] = { >>>> + &traceid_attr_grp, >>>> NULL, >>>> }; >>>> @@ -1425,6 +1455,8 @@ static int tpdm_probe(struct device *dev, >>>> struct resource *res) >>>> desc.access = CSDEV_ACCESS_IOMEM(base); >>>> if (res) >>>> desc.groups = tpdm_attr_grps; >>>> + else >>>> + desc.groups = static_tpdm_attr_grps; >>>> drvdata->csdev = coresight_register(&desc); >>>> if (IS_ERR(drvdata->csdev)) >>>> return PTR_ERR(drvdata->csdev); >>>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/ >>>> hwtracing/coresight/coresight-tpdm.h >>>> index 2867f3ab8186..11da64e1ade8 100644 >>>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h >>>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h >>>> @@ -300,6 +300,7 @@ struct cmb_dataset { >>>> * @cmb Specifics associated to TPDM CMB. >>>> * @dsb_msr_num Number of MSR supported by DSB TPDM >>>> * @cmb_msr_num Number of MSR supported by CMB TPDM >>>> + * @traceid Trace ID of the path. >>>> */ >>>> struct tpdm_drvdata { >>>> @@ -313,6 +314,7 @@ struct tpdm_drvdata { >>>> struct cmb_dataset *cmb; >>>> u32 dsb_msr_num; >>>> u32 cmb_msr_num; >>>> + u8 traceid; >>>> }; >>>> /* Enumerate members of various datasets */ >>>> >>>> --- >>>> base-commit: b84a0ebe421ca56995ff78b66307667b62b3a900 >>>> change-id: 20260316-add-traceid-show-for-tpdm-88d040651f00 >>>> >>>> Best regards, >>> >> >

5 months, 1 week

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight