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>
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
>
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!
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
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
On Mon, Apr 13, 2026 at 03:19:56PM +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().
>
> 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 8ebfd3924143..1bc9f13e33f7 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -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);
I am not familiar with the trace ID, seems to me, it just allocate a ID
for each tracer from the ID map and then always use this cached ID for
the tracers.
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 ?
Thanks,
Leo
> return ret;
> + }
> }
>
> raw_spin_lock(&drvdata->spinlock);
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>
On 4/14/2026 12:15 PM, Jie Gan wrote:
>
> I dont think you have looped relevant maintainers and reviewers for the Coresight subsystem.
>
> Thanks,
> Jie
I will send it to all relevant maintainers and reviewers for the Coresight subsystem in next revision.
Thanks,
Zane