CoreSight

coresight@lists.linaro.org

3 participants
2731 discussions

Re: [PATCH v5 01/12] coresight: etm4x: fix wrong check of etm4x_sspcicrn_present()

by Suzuki K Poulose

On 15/04/2026 17:55, Yeoreum Yun wrote: > According to Embedded Trace Macrocell Architecture Specification > ETMv4.0 to ETM4.6 [0], TRCSSPCICR<n> is present only if all of > the following are true: > > - TRCIDR4.NUMSSCC > n. > - TRCIDR4.NUMPC > 0b0000. > - TRCSSCSR<n>.PC == 0b1. > > Comment for etm4x_sspcicrn_present() is align with the specification. > However, the check should use drvdata->nr_pe_cmp to check TRCIDR4.NUMPC > not nr_pe. > > Link: https://developer.arm.com/documentation/ihi0064/latest/ [0] Fixes: f6a18f354c58 ("coresight: etm4x: Handle access to TRCSSPCICRn") With that looks good to me Suzuki > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index d565a73f0042..74b7063d130e 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -89,7 +89,7 @@ static int etm4_probe_cpu(unsigned int cpu); > static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n) > { > return (n < drvdata->nr_ss_cmp) && > - drvdata->nr_pe && > + drvdata->nr_pe_cmp && > (drvdata->config.ss_status[n] & TRCSSCSRn_PC); > } >

3 weeks, 4 days

Re: [PATCH v5 05/12] coresight: etm4x: remove redundant fields in etmv4_save_state

by Leo Yan

On Wed, Apr 15, 2026 at 05:55:21PM +0100, Yeoreum Yun wrote: > Some of fields are redundant in etmv4_save_state and never used: > > ss_status => trcsscsr > seq_state => trcseqstr > cntr_val => trccntvr > vinst_ctrl => trcvictlr > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> Reviewed-by: Leo Yan <leo.yan(a)arm.com>

3 weeks, 4 days

Re: [PATCH v5 06/12] coresight: etm4x: fix leaked trace id

by Leo Yan

Hi Jie, On Fri, Apr 17, 2026 at 09:01:17AM +0800, Jie Gan wrote: [...] > ... we still need support static trace ID allocation in parallel for > the dummy sources and we should not break this logic in future refactor. Just confirm what is the reason you need to use static trace ID for the dummy sources? I am wandering if we could use dev->devt as trace ID for dummy devices. Since the device's MAJOR number is non-zero and occupies the upper bits (see MINORBITS), it is naturally separated from the hardware trace ID range. If so, we even don't need to bother ID alloc/release. Thanks, Leo

4 weeks, 1 day

Re: [PATCH v5 06/12] coresight: etm4x: fix leaked trace id

by Leo Yan

Hi Levi, On Thu, Apr 16, 2026 at 06:06:41PM +0100, Yeoreum Yun wrote: [...] > > We should use paired way for allocation and release. For example: > > > > coresight_enable_sysfs() > > { > > ... > > coresight_path_assign_trace_id(path); > > > > failed: > > coresight_path_unassign_trace_id(path); > > } > > > > coresight_disable_sysfs() > > { > > coresight_path_unassign_trace_id(path); > > } > > > > But this requires broader refactoring. E.g., the STM driver currently > > allocates system trace IDs statically during probe, we might need to > > consolidate for all modules to use dynamic allocation. > > So IIUC, Do we want to "map" per "session" and save this map information > in the "sink" driver? or just use "global" map but locate it in sink > driver? I prefer to save map in the sink's driver data, this is more scalable as the trace ID is allocated within a session rather than system wide. > I totally agree for above suggestion -- unsigned trace id > in the coresight_XXX function -- (but we need to add another callback > for this) but I think we don't need to sustain map per session > and it seems enough to use current storage for trace_id not move to > sink driver. > > Anyway It would be better to refactorying wiht another patchset... Yeah, we can come back to these ideas when work on it. Thanks, Leo

4 weeks, 1 day

Re: [PATCH v5 06/12] coresight: etm4x: fix leaked trace id

by Leo Yan

On Wed, Apr 15, 2026 at 05:55:22PM +0100, Yeoreum Yun wrote: > If etm4_enable_sysfs() fails in cscfg_csdev_enable_active_config(), > the trace ID may be leaked because it is not released. > > To address this, call etm4_release_trace_id() when etm4_enable_sysfs() > fails in cscfg_csdev_enable_active_config(). > > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index f55338a4989d..b199aebbdb60 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -920,8 +920,10 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); > if (cfg_hash) { > ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); > - if (ret) > + if (ret) { > + etm4_release_trace_id(drvdata); > return ret; > + } LGTM: Reviewed-by: Leo Yan <leo.yan(a)arm.com> Just recording a bit thoughts. As Suzuki mentioned, it would be better to allocate trace IDs within a session. We might consider maintaining the trace ID map in the sink driver data, since the sink driver is unique within a session so it is a central place to allocate trace ID. We should use paired way for allocation and release. For example: coresight_enable_sysfs() { ... coresight_path_assign_trace_id(path); failed: coresight_path_unassign_trace_id(path); } coresight_disable_sysfs() { coresight_path_unassign_trace_id(path); } But this requires broader refactoring. E.g., the STM driver currently allocates system trace IDs statically during probe, we might need to consolidate for all modules to use dynamic allocation. Thanks, Leo

1 month

Re: [PATCH v5 01/12] coresight: etm4x: fix wrong check of etm4x_sspcicrn_present()

by Leo Yan

On Wed, Apr 15, 2026 at 05:55:17PM +0100, Yeoreum Yun wrote: > According to Embedded Trace Macrocell Architecture Specification > ETMv4.0 to ETM4.6 [0], TRCSSPCICR<n> is present only if all of > the following are true: > > - TRCIDR4.NUMSSCC > n. > - TRCIDR4.NUMPC > 0b0000. > - TRCSSCSR<n>.PC == 0b1. > > Comment for etm4x_sspcicrn_present() is align with the specification. > However, the check should use drvdata->nr_pe_cmp to check TRCIDR4.NUMPC > not nr_pe. > > Link: https://developer.arm.com/documentation/ihi0064/latest/ [0] > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> Reviewed-by: Leo Yan <leo.yan(a)arm.com>

1 month

Re: [PATCH v4 3/9] coresight: etm4x: fix leaked trace id

by Suzuki K Poulose

On 15/04/2026 09:45, Jie Gan wrote: > > > On 4/15/2026 4:32 PM, Leo Yan wrote: >> On Wed, Apr 15, 2026 at 09:01:09AM +0100, Yeoreum Yun wrote: >> >> [...] >> >>>>> What I am thinking is as SoCs continue to grow more complex with an >>>>> increasing number of subsystems, trace IDs may be exhausted in the >>>>> near >>>>> future. (that's why we have dynamic trace ID allocation/release). >>>> >>>> Thanks for the input. >>>> >>>> I am wandering if we can use "dev->devt" as the trace ID. A device's >>>> major/minor number is unique in kernel and dev_t is defined as u32: >>>> >>>> typedef u32 __kernel_dev_t; >>>> >>>> And we can consolidate this for both SYSFS and PERF modes. >>>> >>> >>> When I see the CORESIGHT_TRACE_ID_MAX: >>> >>> /* architecturally we have 128 IDs some of which are reserved */ >>> #define CORESIGHT_TRACE_IDS_MAX 128 >>> >>> I think this came from the hardware restriction for number of TRACE_IDs. >>> In this case, clamping the device_id to trace_id seems more complex and >>> reduce some performance perspective. >> >> Sigh, my stupid. Please ignore my previous comment, let us first fix >> ID leak issue. >> >> Given Jie's comment on the use-out issue, it is valid for me especially >> if a system have many dummy tracers. We can defer to refactor it >> later (e.g., use separate ranges for hardware and dummy tracers). >> >> thanks for correction! > > Just share some info: > > With my memory, The ARM AMBA ATB Protocol Specification defined a 7-bit > width field for the trace ID, that's where the 128 comes from. (in each > frame, we also have 7-bit field for containing the trace ID) That is true and some IDs in the range (0-128) are reserved. So we actually have less than 128. We need the dynamic allocation, preferrably isolated to a "pool" for the relevant session to make the full use of the space. Suzuki > > Thanks, > Jie >

1 month

Re: [PATCH v4 3/9] coresight: etm4x: fix leaked trace id

by Leo Yan

On Wed, Apr 15, 2026 at 09:01:09AM +0100, Yeoreum Yun wrote: [...] > > > What I am thinking is as SoCs continue to grow more complex with an > > > increasing number of subsystems, trace IDs may be exhausted in the near > > > future. (that's why we have dynamic trace ID allocation/release). > > > > Thanks for the input. > > > > I am wandering if we can use "dev->devt" as the trace ID. A device's > > major/minor number is unique in kernel and dev_t is defined as u32: > > > > typedef u32 __kernel_dev_t; > > > > And we can consolidate this for both SYSFS and PERF modes. > > > > When I see the CORESIGHT_TRACE_ID_MAX: > > /* architecturally we have 128 IDs some of which are reserved */ > #define CORESIGHT_TRACE_IDS_MAX 128 > > I think this came from the hardware restriction for number of TRACE_IDs. > In this case, clamping the device_id to trace_id seems more complex and > reduce some performance perspective. Sigh, my stupid. Please ignore my previous comment, let us first fix ID leak issue. Given Jie's comment on the use-out issue, it is valid for me especially if a system have many dummy tracers. We can defer to refactor it later (e.g., use separate ranges for hardware and dummy tracers). thanks for correction!

1 month

Re: [PATCH v7 0/4] Add Qualcomm extended CTI support

by Leo Yan

Hi Yingchao, On Wed, Apr 15, 2026 at 11:22:49AM +0800, Yingchao Deng (Consultant) wrote: [...] > Gentle reminder. This series would be on Mike's radar. I will also look into details once I finish Levi's series review. Thanks, Leo

1 month

Re: [PATCH v4 3/9] coresight: etm4x: fix leaked trace id

by Leo Yan

On Wed, Apr 15, 2026 at 09:21:21AM +0800, Jie Gan wrote: [...] > > > > @@ -918,8 +918,10 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa > > > > cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); > > > > if (cfg_hash) { > > > > ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset); > > > > - if (ret) > > > > + if (ret) { > > > > + etm4_release_trace_id(drvdata); > > > > > > If so, even an ID is reserved for failures, and the ID map is big enough > > > for each CPU, we don't need to worry memory leak or ID used out issue ? > > > > However, in theory, this could lead to an ID leak, > > so it would be better to release it in error cases. > > What I am thinking is as SoCs continue to grow more complex with an > increasing number of subsystems, trace IDs may be exhausted in the near > future. (that's why we have dynamic trace ID allocation/release). Thanks for the input. I am wandering if we can use "dev->devt" as the trace ID. A device's major/minor number is unique in kernel and dev_t is defined as u32: typedef u32 __kernel_dev_t; And we can consolidate this for both SYSFS and PERF modes. Thanks, Leo

1 month

← Newer
1
2
3
4
5
6
7
8
...
274
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight