diff mbox series

[v3,04/13] coresight: etm4x: Update ETM4 driver to use Trace ID API

Message ID 20220809223401.24599-5-mike.leach@linaro.org (mailing list archive)
State New, archived
Headers show
Series coresight: Add new API to allocate trace source ID values | expand

Commit Message

Mike Leach Aug. 9, 2022, 10:33 p.m. UTC
The trace ID API is now used to allocate trace IDs for ETM4.x / ETE
devices.

For perf sessions, these will be allocated on enable, and released on
disable.

For sysfs sessions, these will be allocated on enable, but only released
on reset. This allows the sysfs session to interrogate the Trace ID used
after the session is over - maintaining functional consistency with the
previous allocation scheme.

The trace ID will also be allocated on read of the mgmt/trctraceid file.
This ensures that if perf or sysfs read this before enabling trace, the
value will be the one used for the trace session.

Trace ID initialisation is removed from the _probe() function.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 .../coresight/coresight-etm4x-core.c          | 79 +++++++++++++++++--
 .../coresight/coresight-etm4x-sysfs.c         | 27 ++++++-
 drivers/hwtracing/coresight/coresight-etm4x.h |  3 +
 3 files changed, 100 insertions(+), 9 deletions(-)

Comments

Suzuki K Poulose Oct. 3, 2022, 9:31 a.m. UTC | #1
On 09/08/2022 23:33, Mike Leach wrote:
> The trace ID API is now used to allocate trace IDs for ETM4.x / ETE
> devices.
> 
> For perf sessions, these will be allocated on enable, and released on
> disable.
> 
> For sysfs sessions, these will be allocated on enable, but only released
> on reset. This allows the sysfs session to interrogate the Trace ID used
> after the session is over - maintaining functional consistency with the
> previous allocation scheme.
> 
> The trace ID will also be allocated on read of the mgmt/trctraceid file.
> This ensures that if perf or sysfs read this before enabling trace, the
> value will be the one used for the trace session.
> 
> Trace ID initialisation is removed from the _probe() function.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>   .../coresight/coresight-etm4x-core.c          | 79 +++++++++++++++++--
>   .../coresight/coresight-etm4x-sysfs.c         | 27 ++++++-
>   drivers/hwtracing/coresight/coresight-etm4x.h |  3 +
>   3 files changed, 100 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index cf249ecad5a5..b4fb28ce89fd 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -42,6 +42,7 @@
>   #include "coresight-etm4x-cfg.h"
>   #include "coresight-self-hosted-trace.h"
>   #include "coresight-syscfg.h"
> +#include "coresight-trace-id.h"
>   
>   static int boot_enable;
>   module_param(boot_enable, int, 0444);
> @@ -234,6 +235,50 @@ static int etm4_trace_id(struct coresight_device *csdev)
>   	return drvdata->trcid;
>   }
>   
> +int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata)
> +{
> +	int trace_id;
> +
> +	/*
> +	 * This will allocate a trace ID to the cpu,
> +	 * or return the one currently allocated.
> +	 */
> +	/* trace id function has its own lock */
> +	trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu);
> +	if (IS_VALID_ID(trace_id))
> +		drvdata->trcid = (u8)trace_id;
> +	else
> +		dev_err(&drvdata->csdev->dev,
> +			"Failed to allocate trace ID for %s on CPU%d\n",
> +			dev_name(&drvdata->csdev->dev), drvdata->cpu);
> +	return trace_id;
> +}
> +
> +static int etm4_set_current_trace_id(struct etmv4_drvdata *drvdata)
> +{
> +	int trace_id;
> +
> +	/*
> +	 * Set the currently allocated trace ID - perf allocates IDs
> +	 * as part of setup_aux for all CPUs it may use.
> +	 */
> +	trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu);
> +	if (IS_VALID_ID(trace_id)) {
> +		drvdata->trcid = (u8)trace_id;
> +		return 0;
> +	}
> +
> +	dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n",
> +		dev_name(&drvdata->csdev->dev), drvdata->cpu);
> +
> +	return -EINVAL;
> +}


> +
> +void etm4_release_trace_id(struct etmv4_drvdata *drvdata)
> +{
> +	coresight_trace_id_put_cpu_id(drvdata->cpu);
> +}
> +
>   struct etm4_enable_arg {
>   	struct etmv4_drvdata *drvdata;
>   	int rc;
> @@ -729,6 +774,15 @@ static int etm4_enable_perf(struct coresight_device *csdev,
>   	ret = etm4_parse_event_config(csdev, event);
>   	if (ret)
>   		goto out;
> +
> +	/*
> +	 * perf allocates cpu ids as part of setup - device needs to use
> +	 * the allocated ID.
> +	 */
> +	ret = etm4_set_current_trace_id(drvdata);

So, when do we allocate an id in perf mode ? As far as I can see, this
should be the same as etm4_read_alloc_trace_id() ? Why are they any
different ?

> +	if (ret < 0)
> +		goto out;
> +
>   	/* And enable it */
>   	ret = etm4_enable_hw(drvdata);
>   
> @@ -753,6 +807,11 @@ static int etm4_enable_sysfs(struct coresight_device *csdev)
>   
>   	spin_lock(&drvdata->spinlock);
>   
> +	/* sysfs needs to read and allocate a trace ID */
> +	ret = etm4_read_alloc_trace_id(drvdata);
> +	if (ret < 0)
> +		goto unlock_sysfs_enable;
> +
>   	/*
>   	 * Executing etm4_enable_hw on the cpu whose ETM is being enabled
>   	 * ensures that register writes occur when cpu is powered.
> @@ -764,6 +823,11 @@ static int etm4_enable_sysfs(struct coresight_device *csdev)
>   		ret = arg.rc;
>   	if (!ret)
>   		drvdata->sticky_enable = true;
> +
> +	if (ret)
> +		etm4_release_trace_id(drvdata);
> +
> +unlock_sysfs_enable:
>   	spin_unlock(&drvdata->spinlock);
>   
>   	if (!ret)
> @@ -895,6 +959,8 @@ static int etm4_disable_perf(struct coresight_device *csdev,
>   	/* TRCVICTLR::SSSTATUS, bit[9] */
>   	filters->ssstatus = (control & BIT(9));
>   
> +	/* The perf event will release trace ids when it is destroyed */
> +

At this patch level, there is no release of trace id ? Is that missed in
this patch ? Or am I missing something ?

Suzuki
Suzuki K Poulose Oct. 3, 2022, 9:37 a.m. UTC | #2
On 03/10/2022 10:31, Suzuki K Poulose wrote:
> On 09/08/2022 23:33, Mike Leach wrote:
>> The trace ID API is now used to allocate trace IDs for ETM4.x / ETE
>> devices.
>>
>> For perf sessions, these will be allocated on enable, and released on
>> disable.
>>
>> For sysfs sessions, these will be allocated on enable, but only released
>> on reset. This allows the sysfs session to interrogate the Trace ID used
>> after the session is over - maintaining functional consistency with the
>> previous allocation scheme.
>>
>> The trace ID will also be allocated on read of the mgmt/trctraceid file.
>> This ensures that if perf or sysfs read this before enabling trace, the
>> value will be the one used for the trace session.
>>
>> Trace ID initialisation is removed from the _probe() function.
>>
>> Signed-off-by: Mike Leach <mike.leach@linaro.org>
>> ---
>>   .../coresight/coresight-etm4x-core.c          | 79 +++++++++++++++++--
>>   .../coresight/coresight-etm4x-sysfs.c         | 27 ++++++-
>>   drivers/hwtracing/coresight/coresight-etm4x.h |  3 +
>>   3 files changed, 100 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c 
>> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index cf249ecad5a5..b4fb28ce89fd 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -42,6 +42,7 @@
>>   #include "coresight-etm4x-cfg.h"
>>   #include "coresight-self-hosted-trace.h"
>>   #include "coresight-syscfg.h"
>> +#include "coresight-trace-id.h"
>>   static int boot_enable;
>>   module_param(boot_enable, int, 0444);
>> @@ -234,6 +235,50 @@ static int etm4_trace_id(struct coresight_device 
>> *csdev)
>>       return drvdata->trcid;
>>   }
>> +int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata)
>> +{
>> +    int trace_id;
>> +
>> +    /*
>> +     * This will allocate a trace ID to the cpu,
>> +     * or return the one currently allocated.
>> +     */
>> +    /* trace id function has its own lock */
>> +    trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu);
>> +    if (IS_VALID_ID(trace_id))
>> +        drvdata->trcid = (u8)trace_id;
>> +    else
>> +        dev_err(&drvdata->csdev->dev,
>> +            "Failed to allocate trace ID for %s on CPU%d\n",
>> +            dev_name(&drvdata->csdev->dev), drvdata->cpu);
>> +    return trace_id;
>> +}
>> +
>> +static int etm4_set_current_trace_id(struct etmv4_drvdata *drvdata)
>> +{
>> +    int trace_id;
>> +
>> +    /*
>> +     * Set the currently allocated trace ID - perf allocates IDs
>> +     * as part of setup_aux for all CPUs it may use.
>> +     */
>> +    trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu);
>> +    if (IS_VALID_ID(trace_id)) {
>> +        drvdata->trcid = (u8)trace_id;
>> +        return 0;
>> +    }
>> +
>> +    dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on 
>> CPU%d\n",
>> +        dev_name(&drvdata->csdev->dev), drvdata->cpu);
>> +
>> +    return -EINVAL;
>> +}
> 
> 
>> +
>> +void etm4_release_trace_id(struct etmv4_drvdata *drvdata)
>> +{
>> +    coresight_trace_id_put_cpu_id(drvdata->cpu);
>> +}
>> +
>>   struct etm4_enable_arg {
>>       struct etmv4_drvdata *drvdata;
>>       int rc;
>> @@ -729,6 +774,15 @@ static int etm4_enable_perf(struct 
>> coresight_device *csdev,
>>       ret = etm4_parse_event_config(csdev, event);
>>       if (ret)
>>           goto out;
>> +
>> +    /*
>> +     * perf allocates cpu ids as part of setup - device needs to use
>> +     * the allocated ID.
>> +     */
>> +    ret = etm4_set_current_trace_id(drvdata);
> 
> So, when do we allocate an id in perf mode ? As far as I can see, this
> should be the same as etm4_read_alloc_trace_id() ? Why are they any
> different ?
> 
>> +    if (ret < 0)
>> +        goto out;
>> +
>>       /* And enable it */
>>       ret = etm4_enable_hw(drvdata);
>> @@ -753,6 +807,11 @@ static int etm4_enable_sysfs(struct 
>> coresight_device *csdev)
>>       spin_lock(&drvdata->spinlock);
>> +    /* sysfs needs to read and allocate a trace ID */
>> +    ret = etm4_read_alloc_trace_id(drvdata);
>> +    if (ret < 0)
>> +        goto unlock_sysfs_enable;
>> +
>>       /*
>>        * Executing etm4_enable_hw on the cpu whose ETM is being enabled
>>        * ensures that register writes occur when cpu is powered.
>> @@ -764,6 +823,11 @@ static int etm4_enable_sysfs(struct 
>> coresight_device *csdev)
>>           ret = arg.rc;
>>       if (!ret)
>>           drvdata->sticky_enable = true;
>> +
>> +    if (ret)
>> +        etm4_release_trace_id(drvdata);
>> +
>> +unlock_sysfs_enable:
>>       spin_unlock(&drvdata->spinlock);
>>       if (!ret)
>> @@ -895,6 +959,8 @@ static int etm4_disable_perf(struct 
>> coresight_device *csdev,
>>       /* TRCVICTLR::SSSTATUS, bit[9] */
>>       filters->ssstatus = (control & BIT(9));
>> +    /* The perf event will release trace ids when it is destroyed */
>> +
> 
> At this patch level, there is no release of trace id ? Is that missed in
> this patch ? Or am I missing something ?

I think the above change only comes in PATCH 7. May be that patch needs 
to be rearranged in order ? Otherwise git-bisect can break running a 
perf session on cs_etm, with missing traceid.

Suzuki
Mike Leach Oct. 6, 2022, 1:47 p.m. UTC | #3
Hi Suzuki,

On Mon, 3 Oct 2022 at 10:37, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 03/10/2022 10:31, Suzuki K Poulose wrote:
> > On 09/08/2022 23:33, Mike Leach wrote:
> >> The trace ID API is now used to allocate trace IDs for ETM4.x / ETE
> >> devices.
> >>
> >> For perf sessions, these will be allocated on enable, and released on
> >> disable.
> >>
> >> For sysfs sessions, these will be allocated on enable, but only released
> >> on reset. This allows the sysfs session to interrogate the Trace ID used
> >> after the session is over - maintaining functional consistency with the
> >> previous allocation scheme.
> >>
> >> The trace ID will also be allocated on read of the mgmt/trctraceid file.
> >> This ensures that if perf or sysfs read this before enabling trace, the
> >> value will be the one used for the trace session.
> >>
> >> Trace ID initialisation is removed from the _probe() function.
> >>
> >> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> >> ---
> >>   .../coresight/coresight-etm4x-core.c          | 79 +++++++++++++++++--
> >>   .../coresight/coresight-etm4x-sysfs.c         | 27 ++++++-
> >>   drivers/hwtracing/coresight/coresight-etm4x.h |  3 +
> >>   3 files changed, 100 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> index cf249ecad5a5..b4fb28ce89fd 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> @@ -42,6 +42,7 @@
> >>   #include "coresight-etm4x-cfg.h"
> >>   #include "coresight-self-hosted-trace.h"
> >>   #include "coresight-syscfg.h"
> >> +#include "coresight-trace-id.h"
> >>   static int boot_enable;
> >>   module_param(boot_enable, int, 0444);
> >> @@ -234,6 +235,50 @@ static int etm4_trace_id(struct coresight_device
> >> *csdev)
> >>       return drvdata->trcid;
> >>   }
> >> +int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata)
> >> +{
> >> +    int trace_id;
> >> +
> >> +    /*
> >> +     * This will allocate a trace ID to the cpu,
> >> +     * or return the one currently allocated.
> >> +     */
> >> +    /* trace id function has its own lock */
> >> +    trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu);
> >> +    if (IS_VALID_ID(trace_id))
> >> +        drvdata->trcid = (u8)trace_id;
> >> +    else
> >> +        dev_err(&drvdata->csdev->dev,
> >> +            "Failed to allocate trace ID for %s on CPU%d\n",
> >> +            dev_name(&drvdata->csdev->dev), drvdata->cpu);
> >> +    return trace_id;
> >> +}
> >> +
> >> +static int etm4_set_current_trace_id(struct etmv4_drvdata *drvdata)
> >> +{
> >> +    int trace_id;
> >> +
> >> +    /*
> >> +     * Set the currently allocated trace ID - perf allocates IDs
> >> +     * as part of setup_aux for all CPUs it may use.
> >> +     */
> >> +    trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu);
> >> +    if (IS_VALID_ID(trace_id)) {
> >> +        drvdata->trcid = (u8)trace_id;
> >> +        return 0;
> >> +    }
> >> +
> >> +    dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on
> >> CPU%d\n",
> >> +        dev_name(&drvdata->csdev->dev), drvdata->cpu);
> >> +
> >> +    return -EINVAL;
> >> +}
> >
> >
> >> +
> >> +void etm4_release_trace_id(struct etmv4_drvdata *drvdata)
> >> +{
> >> +    coresight_trace_id_put_cpu_id(drvdata->cpu);
> >> +}
> >> +
> >>   struct etm4_enable_arg {
> >>       struct etmv4_drvdata *drvdata;
> >>       int rc;
> >> @@ -729,6 +774,15 @@ static int etm4_enable_perf(struct
> >> coresight_device *csdev,
> >>       ret = etm4_parse_event_config(csdev, event);
> >>       if (ret)
> >>           goto out;
> >> +
> >> +    /*
> >> +     * perf allocates cpu ids as part of setup - device needs to use
> >> +     * the allocated ID.
> >> +     */
> >> +    ret = etm4_set_current_trace_id(drvdata);
> >
> > So, when do we allocate an id in perf mode ? As far as I can see, this
> > should be the same as etm4_read_alloc_trace_id() ? Why are they any
> > different ?
> >

Trace IDs are allocated in etm_setup_aux(), then released in free_event_data().
This way the value are guaranteed to remain constant for the entire
session,. We cannot release an ID when a given ETM stops tracing, as
it may restart later for the same perf session.
In between these two events perf takes its own locks - for whatever
reason. If we use read_alloc trace id - which takes that allocation /
release lock in the ID system then we see at lot of
LOCKDEP issues at this point.

Hence we do a safe unlocked read of the value - perf will never
release the value till after the session is completed and the event is
released.

> >> +    if (ret < 0)
> >> +        goto out;
> >> +
> >>       /* And enable it */
> >>       ret = etm4_enable_hw(drvdata);
> >> @@ -753,6 +807,11 @@ static int etm4_enable_sysfs(struct
> >> coresight_device *csdev)
> >>       spin_lock(&drvdata->spinlock);
> >> +    /* sysfs needs to read and allocate a trace ID */
> >> +    ret = etm4_read_alloc_trace_id(drvdata);
> >> +    if (ret < 0)
> >> +        goto unlock_sysfs_enable;
> >> +
> >>       /*
> >>        * Executing etm4_enable_hw on the cpu whose ETM is being enabled
> >>        * ensures that register writes occur when cpu is powered.
> >> @@ -764,6 +823,11 @@ static int etm4_enable_sysfs(struct
> >> coresight_device *csdev)
> >>           ret = arg.rc;
> >>       if (!ret)
> >>           drvdata->sticky_enable = true;
> >> +
> >> +    if (ret)
> >> +        etm4_release_trace_id(drvdata);
> >> +
> >> +unlock_sysfs_enable:
> >>       spin_unlock(&drvdata->spinlock);
> >>       if (!ret)
> >> @@ -895,6 +959,8 @@ static int etm4_disable_perf(struct
> >> coresight_device *csdev,
> >>       /* TRCVICTLR::SSSTATUS, bit[9] */
> >>       filters->ssstatus = (control & BIT(9));
> >> +    /* The perf event will release trace ids when it is destroyed */
> >> +
> >
> > At this patch level, there is no release of trace id ? Is that missed in
> > this patch ? Or am I missing something ?
>

See above  - perf allocation is handled in coresight-etm-perf.c.


> I think the above change only comes in PATCH 7. May be that patch needs
> to be rearranged in order ? Otherwise git-bisect can break running a
> perf session on cs_etm, with missing traceid.
>

This is probably true - we can move the perf allocation to before the
ETM changes as the extended backward compatibility introduced this set
should permit it.


> Suzuki

Thanks for the review

Mike
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index cf249ecad5a5..b4fb28ce89fd 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -42,6 +42,7 @@ 
 #include "coresight-etm4x-cfg.h"
 #include "coresight-self-hosted-trace.h"
 #include "coresight-syscfg.h"
+#include "coresight-trace-id.h"
 
 static int boot_enable;
 module_param(boot_enable, int, 0444);
@@ -234,6 +235,50 @@  static int etm4_trace_id(struct coresight_device *csdev)
 	return drvdata->trcid;
 }
 
+int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata)
+{
+	int trace_id;
+
+	/*
+	 * This will allocate a trace ID to the cpu,
+	 * or return the one currently allocated.
+	 */
+	/* trace id function has its own lock */
+	trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu);
+	if (IS_VALID_ID(trace_id))
+		drvdata->trcid = (u8)trace_id;
+	else
+		dev_err(&drvdata->csdev->dev,
+			"Failed to allocate trace ID for %s on CPU%d\n",
+			dev_name(&drvdata->csdev->dev), drvdata->cpu);
+	return trace_id;
+}
+
+static int etm4_set_current_trace_id(struct etmv4_drvdata *drvdata)
+{
+	int trace_id;
+
+	/*
+	 * Set the currently allocated trace ID - perf allocates IDs
+	 * as part of setup_aux for all CPUs it may use.
+	 */
+	trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu);
+	if (IS_VALID_ID(trace_id)) {
+		drvdata->trcid = (u8)trace_id;
+		return 0;
+	}
+
+	dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n",
+		dev_name(&drvdata->csdev->dev), drvdata->cpu);
+
+	return -EINVAL;
+}
+
+void etm4_release_trace_id(struct etmv4_drvdata *drvdata)
+{
+	coresight_trace_id_put_cpu_id(drvdata->cpu);
+}
+
 struct etm4_enable_arg {
 	struct etmv4_drvdata *drvdata;
 	int rc;
@@ -729,6 +774,15 @@  static int etm4_enable_perf(struct coresight_device *csdev,
 	ret = etm4_parse_event_config(csdev, event);
 	if (ret)
 		goto out;
+
+	/*
+	 * perf allocates cpu ids as part of setup - device needs to use
+	 * the allocated ID.
+	 */
+	ret = etm4_set_current_trace_id(drvdata);
+	if (ret < 0)
+		goto out;
+
 	/* And enable it */
 	ret = etm4_enable_hw(drvdata);
 
@@ -753,6 +807,11 @@  static int etm4_enable_sysfs(struct coresight_device *csdev)
 
 	spin_lock(&drvdata->spinlock);
 
+	/* sysfs needs to read and allocate a trace ID */
+	ret = etm4_read_alloc_trace_id(drvdata);
+	if (ret < 0)
+		goto unlock_sysfs_enable;
+
 	/*
 	 * Executing etm4_enable_hw on the cpu whose ETM is being enabled
 	 * ensures that register writes occur when cpu is powered.
@@ -764,6 +823,11 @@  static int etm4_enable_sysfs(struct coresight_device *csdev)
 		ret = arg.rc;
 	if (!ret)
 		drvdata->sticky_enable = true;
+
+	if (ret)
+		etm4_release_trace_id(drvdata);
+
+unlock_sysfs_enable:
 	spin_unlock(&drvdata->spinlock);
 
 	if (!ret)
@@ -895,6 +959,8 @@  static int etm4_disable_perf(struct coresight_device *csdev,
 	/* TRCVICTLR::SSSTATUS, bit[9] */
 	filters->ssstatus = (control & BIT(9));
 
+	/* The perf event will release trace ids when it is destroyed */
+
 	return 0;
 }
 
@@ -920,6 +986,13 @@  static void etm4_disable_sysfs(struct coresight_device *csdev)
 	spin_unlock(&drvdata->spinlock);
 	cpus_read_unlock();
 
+	/*
+	 * we only release trace IDs when resetting sysfs.
+	 * This permits sysfs users to read the trace ID after the trace
+	 * session has completed. This maintains operational behaviour with
+	 * prior trace id allocation method
+	 */
+
 	dev_dbg(&csdev->dev, "ETM tracing disabled\n");
 }
 
@@ -1562,11 +1635,6 @@  static int etm4_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
-static void etm4_init_trace_id(struct etmv4_drvdata *drvdata)
-{
-	drvdata->trcid = coresight_get_trace_id(drvdata->cpu);
-}
-
 static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 {
 	int i, ret = 0;
@@ -1971,7 +2039,6 @@  static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid)
 	if (!desc.name)
 		return -ENOMEM;
 
-	etm4_init_trace_id(drvdata);
 	etm4_set_default(&drvdata->config);
 
 	pdata = coresight_get_platform_data(dev);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 6ea8181816fc..d3c27c521d43 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -266,10 +266,11 @@  static ssize_t reset_store(struct device *dev,
 	config->vmid_mask0 = 0x0;
 	config->vmid_mask1 = 0x0;
 
-	drvdata->trcid = drvdata->cpu + 1;
-
 	spin_unlock(&drvdata->spinlock);
 
+	/* for sysfs - only release trace id when resetting */
+	etm4_release_trace_id(drvdata);
+
 	cscfg_csdev_reset_feats(to_coresight_device(dev));
 
 	return size;
@@ -2363,6 +2364,26 @@  static struct attribute *coresight_etmv4_attrs[] = {
 	NULL,
 };
 
+/*
+ * Trace ID allocated dynamically on enable - but also allocate on read
+ * in case sysfs or perf read before enable to ensure consistent metadata
+ * information for trace decode
+ */
+static ssize_t trctraceid_show(struct device *dev,
+			       struct device_attribute *attr,
+			       char *buf)
+{
+	int trace_id;
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+	trace_id = etm4_read_alloc_trace_id(drvdata);
+	if (trace_id < 0)
+		return trace_id;
+
+	return sysfs_emit(buf, "0x%x\n", trace_id);
+}
+static DEVICE_ATTR_RO(trctraceid);
+
 struct etmv4_reg {
 	struct coresight_device *csdev;
 	u32 offset;
@@ -2499,7 +2520,7 @@  static struct attribute *coresight_etmv4_mgmt_attrs[] = {
 	coresight_etm4x_reg(trcpidr3, TRCPIDR3),
 	coresight_etm4x_reg(trcoslsr, TRCOSLSR),
 	coresight_etm4x_reg(trcconfig, TRCCONFIGR),
-	coresight_etm4x_reg(trctraceid, TRCTRACEIDR),
+	&dev_attr_trctraceid.attr,
 	coresight_etm4x_reg(trcdevarch, TRCDEVARCH),
 	NULL,
 };
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index a7bfea31f7d8..793c361841d4 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -1095,4 +1095,7 @@  static inline bool etm4x_is_ete(struct etmv4_drvdata *drvdata)
 {
 	return drvdata->arch >= ETM_ARCH_ETE;
 }
+
+int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata);
+void etm4_release_trace_id(struct etmv4_drvdata *drvdata);
 #endif