When enabling a tracer via SysFS interface, the device mode may be set
by any CPU - not necessarily the target CPU. This can lead to race
condition in SMP, and may result in incorrect mode values being read.
Consider the following example, where CPU0 attempts to enable the tracer
on CPU1 (the target CPU):
CPU0 CPU1
etm4_enable()
` coresight_take_mode(SYSFS)
` etm4_enable_sysfs()
` smp_call_function_single() ----> etm4_enable_hw_smp_call()
/
/ CPU idle:
/ etm4_cpu_save()
/ ` coresight_get_mode()
Failed to enable h/w / ^^^
` coresight_set_mode(DISABLED) <-' Read the intermediate SYSFS mode
In this case, CPU0 initiates the operation by taking the SYSFS mode to
avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
configure the tracer registers. If any error occurs during this process,
CPU0 rolls back by setting the mode to DISABLED.
However, if CPU1 enters an idle state during this time, it might read
the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
save and restore tracer context that is actually disabled.
To resolve the issue, this commit moves the device mode setting logic on
the target CPU. This ensures that the device mode is only modified by
the target CPU, eliminating race condition between mode writes and reads
across CPUs.
An additional change introduces the etm4_disable_hw_smp_call() function
for SMP calls, which disables the tracer and explicitly set the mode to
DISABLED during SysFS operations.
The flow is updated with this change:
CPU0 CPU1
etm4_enable()
` etm4_enable_sysfs()
` smp_call_function_single() ----> etm4_enable_hw_smp_call()
` coresight_take_mode(SYSFS)
Failed, set back to DISABLED
` coresight_set_mode(DISABLED)
CPU idle:
etm4_cpu_save()
` coresight_get_mode()
^^^
Read out the DISABLED mode
Fixes: c38a9ec2b2c1 ("coresight: etm4x: moving etm_drvdata::enable to atomic field")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++-------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 42e5d37403addc6ec81f2e3184522d67d1677c04..ee405c88ea5faa130819f96b00b8307f8764d58a 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -590,10 +590,23 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
static void etm4_enable_hw_smp_call(void *info)
{
struct etm4_enable_arg *arg = info;
+ struct coresight_device *csdev;
if (WARN_ON(!arg))
return;
+
+ csdev = arg->drvdata->csdev;
+ if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
+ /* Someone is already using the tracer */
+ arg->rc = -EBUSY;
+ return;
+ }
+
arg->rc = etm4_enable_hw(arg->drvdata);
+
+ /* The tracer didn't start */
+ if (arg->rc)
+ coresight_set_mode(csdev, CS_MODE_DISABLED);
}
/*
@@ -809,6 +822,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
int ret = 0;
struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ if (!coresight_take_mode(csdev, CS_MODE_PERF))
+ return -EBUSY;
+
if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
ret = -EINVAL;
goto out;
@@ -828,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
ret = etm4_enable_hw(drvdata);
out:
+ /* The tracer didn't start */
+ if (ret)
+ coresight_set_mode(csdev, CS_MODE_DISABLED);
return ret;
}
@@ -880,11 +899,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
{
int ret;
- if (!coresight_take_mode(csdev, mode)) {
- /* Someone is already using the tracer */
- return -EBUSY;
- }
-
switch (mode) {
case CS_MODE_SYSFS:
ret = etm4_enable_sysfs(csdev, path);
@@ -896,10 +910,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
ret = -EINVAL;
}
- /* The tracer didn't start */
- if (ret)
- coresight_set_mode(csdev, CS_MODE_DISABLED);
-
return ret;
}
@@ -951,10 +961,9 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
isb();
}
-static void etm4_disable_hw(void *info)
+static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
{
u32 control;
- struct etmv4_drvdata *drvdata = info;
struct etmv4_config *config = &drvdata->config;
struct coresight_device *csdev = drvdata->csdev;
struct csdev_access *csa = &csdev->access;
@@ -991,6 +1000,15 @@ static void etm4_disable_hw(void *info)
"cpu: %d disable smp call done\n", drvdata->cpu);
}
+static void etm4_disable_hw_smp_call(void *info)
+{
+ struct etmv4_drvdata *drvdata = info;
+
+ etm4_disable_hw(drvdata);
+
+ coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
+}
+
static int etm4_disable_perf(struct coresight_device *csdev,
struct perf_event *event)
{
@@ -1020,6 +1038,8 @@ static int etm4_disable_perf(struct coresight_device *csdev,
/* TRCVICTLR::SSSTATUS, bit[9] */
filters->ssstatus = (control & BIT(9));
+ coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
+
/*
* perf will release trace ids when _free_aux() is
* called at the end of the session.
@@ -1045,7 +1065,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
* Executing etm4_disable_hw on the cpu whose ETM is being disabled
* ensures that register writes occur when cpu is powered.
*/
- smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
+ smp_call_function_single(drvdata->cpu, etm4_disable_hw_smp_call,
+ drvdata, 1);
raw_spin_unlock(&drvdata->spinlock);
@@ -1085,9 +1106,6 @@ static void etm4_disable(struct coresight_device *csdev,
etm4_disable_perf(csdev, event);
break;
}
-
- if (mode)
- coresight_set_mode(csdev, CS_MODE_DISABLED);
}
static int etm4_resume_perf(struct coresight_device *csdev)
--
2.34.1
On 01/07/2025 3:53 pm, Leo Yan wrote:
> When enabling a tracer via SysFS interface, the device mode may be set
> by any CPU - not necessarily the target CPU. This can lead to race
> condition in SMP, and may result in incorrect mode values being read.
>
> Consider the following example, where CPU0 attempts to enable the tracer
> on CPU1 (the target CPU):
>
> CPU0 CPU1
> etm4_enable()
> ` coresight_take_mode(SYSFS)
> ` etm4_enable_sysfs()
> ` smp_call_function_single() ----> etm4_enable_hw_smp_call()
> /
> / CPU idle:
> / etm4_cpu_save()
> / ` coresight_get_mode()
> Failed to enable h/w / ^^^
> ` coresight_set_mode(DISABLED) <-' Read the intermediate SYSFS mode
>
> In this case, CPU0 initiates the operation by taking the SYSFS mode to
> avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
> configure the tracer registers. If any error occurs during this process,
> CPU0 rolls back by setting the mode to DISABLED.
>
> However, if CPU1 enters an idle state during this time, it might read
> the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
> save and restore tracer context that is actually disabled.
>
> To resolve the issue, this commit moves the device mode setting logic on
> the target CPU. This ensures that the device mode is only modified by
> the target CPU, eliminating race condition between mode writes and reads
> across CPUs.
>
> An additional change introduces the etm4_disable_hw_smp_call() function
> for SMP calls, which disables the tracer and explicitly set the mode to
> DISABLED during SysFS operations.
>
> The flow is updated with this change:
>
> CPU0 CPU1
> etm4_enable()
> ` etm4_enable_sysfs()
> ` smp_call_function_single() ----> etm4_enable_hw_smp_call()
> ` coresight_take_mode(SYSFS)
> Failed, set back to DISABLED
> ` coresight_set_mode(DISABLED)
>
> CPU idle:
> etm4_cpu_save()
> ` coresight_get_mode()
> ^^^
> Read out the DISABLED mode
There's no lock though, so can't it do this:
CPU0 CPU1
etm4_enable()
` etm4_enable_sysfs()
` smp_call_function_single() ---> etm4_enable_hw_smp_call()
`coresight_take_mode(SYSFS)
CPU idle:
etm4_cpu_save()
` coresight_get_mode()
^^ Intermediate SYSFS mode
This is why I was voting for changing the compare and swap mode stuff to
spinlocks so we can be sure it's correct. The lock shouldn't be released
until the mode is at its final value.
On Thu, Aug 21, 2025 at 10:45:10AM +0100, James Clark wrote: [...] > > The flow is updated with this change: > > > > CPU0 CPU1 > > etm4_enable() > > ` etm4_enable_sysfs() > > ` smp_call_function_single() ----> etm4_enable_hw_smp_call() > > ` coresight_take_mode(SYSFS) > > Failed, set back to DISABLED > > ` coresight_set_mode(DISABLED) > > > > CPU idle: > > etm4_cpu_save() > > ` coresight_get_mode() > > ^^^ > > Read out the DISABLED mode > > There's no lock though, so can't it do this: > > CPU0 CPU1 > etm4_enable() > ` etm4_enable_sysfs() > ` smp_call_function_single() ---> etm4_enable_hw_smp_call() > `coresight_take_mode(SYSFS) > > CPU idle: > etm4_cpu_save() > ` coresight_get_mode() > ^^ Intermediate SYSFS mode No. The point is a target CPU will execute etm4_enable_hw_smp_call() and CPU idle flow sequentially. It is no chance for the idle flow to preempt the ETM flow. This is why using the target CPU to set the ETM state machine will dismiss the race condition. > This is why I was voting for changing the compare and swap mode stuff to > spinlocks so we can be sure it's correct. The lock shouldn't be released > until the mode is at its final value. The question is a spinlock usage. If both initiate CPUs and target CPU try to acquire the spinlock, the low level's CPU idle flow (etm4_cpu_save() / etm4_cpu_restore()) needs to wait for high level's SysFS operation to complete - which is what we try to avoid. If we move the state machine on the target CPU - the operation and its state transition on the same CPU so that dismiss a race condition. And the idle flow is sequential to the ETM enabling/disabling, then, we can achieve lockless approach. A global picture is ETM's state is used in multiple places (Sysfs knobs, ETM enabling / disabling, CPU idle and hotplug flow), if we start to use spinlock for exclusively access the statch machine, then everywhere will use it. This is why this series wants to do reliable state transition based on appropriate atomic APIs (it also can promise atomicity and ordering but lockless). Thanks, Leo
On 01/07/25 8:23 PM, Leo Yan wrote:
> When enabling a tracer via SysFS interface, the device mode may be set
> by any CPU - not necessarily the target CPU. This can lead to race
> condition in SMP, and may result in incorrect mode values being read.
>
> Consider the following example, where CPU0 attempts to enable the tracer
> on CPU1 (the target CPU):
>
> CPU0 CPU1
> etm4_enable()
> ` coresight_take_mode(SYSFS)
> ` etm4_enable_sysfs()
> ` smp_call_function_single() ----> etm4_enable_hw_smp_call()
> /
> / CPU idle:
> / etm4_cpu_save()
> / ` coresight_get_mode()
> Failed to enable h/w / ^^^
> ` coresight_set_mode(DISABLED) <-' Read the intermediate SYSFS mode
The problem is - CPU1's HW state and CPU1's sysfs mode state might not
remain in sync if CPU1 goes into idle state just after an unsuccessful
etm4_enable_sysfs() attempt from CPU0. In which case a subsequent read
coresight_get_mode() on CPU1 might erroneously give us DISABLED state,
which actually does not seem to be too bad as the earlier enablement
attempt had failed anyway. Just trying to understand what is the real
problem here.
>
> In this case, CPU0 initiates the operation by taking the SYSFS mode to
> avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
> configure the tracer registers. If any error occurs during this process,
What kind of error can happen during this process ?
> CPU0 rolls back by setting the mode to DISABLED.
Which seems OK.
>
> However, if CPU1 enters an idle state during this time, it might read
> the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
> save and restore tracer context that is actually disabled.
Right but CPU0 had marked the CPU1' state as DISABLED after the enable
attempt had failed. So what is the problem ?
>
> To resolve the issue, this commit moves the device mode setting logic on
> the target CPU. This ensures that the device mode is only modified by
> the target CPU, eliminating race condition between mode writes and reads
> across CPUs.
>
> An additional change introduces the etm4_disable_hw_smp_call() function
> for SMP calls, which disables the tracer and explicitly set the mode to
> DISABLED during SysFS operations.
>
> The flow is updated with this change:
>
> CPU0 CPU1
> etm4_enable()
> ` etm4_enable_sysfs()
> ` smp_call_function_single() ----> etm4_enable_hw_smp_call()
> ` coresight_take_mode(SYSFS)
> Failed, set back to DISABLED
> ` coresight_set_mode(DISABLED)
>
> CPU idle:
> etm4_cpu_save()
> ` coresight_get_mode()
> ^^^
> Read out the DISABLED mode
>
> Fixes: c38a9ec2b2c1 ("coresight: etm4x: moving etm_drvdata::enable to atomic field")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++-------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 42e5d37403addc6ec81f2e3184522d67d1677c04..ee405c88ea5faa130819f96b00b8307f8764d58a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -590,10 +590,23 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> static void etm4_enable_hw_smp_call(void *info)
> {
> struct etm4_enable_arg *arg = info;
> + struct coresight_device *csdev;
>
> if (WARN_ON(!arg))
> return;
> +
> + csdev = arg->drvdata->csdev;
> + if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
> + /* Someone is already using the tracer */
> + arg->rc = -EBUSY;
> + return;
> + }
> +
> arg->rc = etm4_enable_hw(arg->drvdata);
> +
> + /* The tracer didn't start */
> + if (arg->rc)
> + coresight_set_mode(csdev, CS_MODE_DISABLED);
> }
>
> /*
> @@ -809,6 +822,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
> int ret = 0;
> struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> + if (!coresight_take_mode(csdev, CS_MODE_PERF))
> + return -EBUSY;
> +
> if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
> ret = -EINVAL;
> goto out;
> @@ -828,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
> ret = etm4_enable_hw(drvdata);
>
> out:
> + /* The tracer didn't start */
> + if (ret)
> + coresight_set_mode(csdev, CS_MODE_DISABLED);
> return ret;
> }
>
> @@ -880,11 +899,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
> {
> int ret;
>
> - if (!coresight_take_mode(csdev, mode)) {
> - /* Someone is already using the tracer */
> - return -EBUSY;
> - }
> -
> switch (mode) {
> case CS_MODE_SYSFS:
> ret = etm4_enable_sysfs(csdev, path);
> @@ -896,10 +910,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
> ret = -EINVAL;
> }
>
> - /* The tracer didn't start */
> - if (ret)
> - coresight_set_mode(csdev, CS_MODE_DISABLED);
> -
> return ret;
> }
>
> @@ -951,10 +961,9 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
> isb();
> }
>
> -static void etm4_disable_hw(void *info)
> +static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
> {
> u32 control;
> - struct etmv4_drvdata *drvdata = info;
> struct etmv4_config *config = &drvdata->config;
> struct coresight_device *csdev = drvdata->csdev;
> struct csdev_access *csa = &csdev->access;
> @@ -991,6 +1000,15 @@ static void etm4_disable_hw(void *info)
> "cpu: %d disable smp call done\n", drvdata->cpu);
> }
>
> +static void etm4_disable_hw_smp_call(void *info)
> +{
> + struct etmv4_drvdata *drvdata = info;
> +
> + etm4_disable_hw(drvdata);
> +
> + coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +}
> +
> static int etm4_disable_perf(struct coresight_device *csdev,
> struct perf_event *event)
> {
> @@ -1020,6 +1038,8 @@ static int etm4_disable_perf(struct coresight_device *csdev,
> /* TRCVICTLR::SSSTATUS, bit[9] */
> filters->ssstatus = (control & BIT(9));
>
> + coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +
> /*
> * perf will release trace ids when _free_aux() is
> * called at the end of the session.
> @@ -1045,7 +1065,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
> * Executing etm4_disable_hw on the cpu whose ETM is being disabled
> * ensures that register writes occur when cpu is powered.
> */
> - smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
> + smp_call_function_single(drvdata->cpu, etm4_disable_hw_smp_call,
> + drvdata, 1);
>
> raw_spin_unlock(&drvdata->spinlock);
>
> @@ -1085,9 +1106,6 @@ static void etm4_disable(struct coresight_device *csdev,
> etm4_disable_perf(csdev, event);
> break;
> }
> -
> - if (mode)
> - coresight_set_mode(csdev, CS_MODE_DISABLED);
> }
>
> static int etm4_resume_perf(struct coresight_device *csdev)
>
On Tue, Jul 15, 2025 at 12:56:54PM +0530, Anshuman Khandual wrote: > > On 01/07/25 8:23 PM, Leo Yan wrote: > > When enabling a tracer via SysFS interface, the device mode may be set > > by any CPU - not necessarily the target CPU. This can lead to race > > condition in SMP, and may result in incorrect mode values being read. > > > > Consider the following example, where CPU0 attempts to enable the tracer > > on CPU1 (the target CPU): > > > > CPU0 CPU1 > > etm4_enable() > > ` coresight_take_mode(SYSFS) > > ` etm4_enable_sysfs() > > ` smp_call_function_single() ----> etm4_enable_hw_smp_call() > > / > > / CPU idle: > > / etm4_cpu_save() > > / ` coresight_get_mode() > > Failed to enable h/w / ^^^ > > ` coresight_set_mode(DISABLED) <-' Read the intermediate SYSFS mode > > The problem is - CPU1's HW state and CPU1's sysfs mode state might not > remain in sync if CPU1 goes into idle state just after an unsuccessful > etm4_enable_sysfs() attempt from CPU0. In which case a subsequent read > coresight_get_mode() on CPU1 might erroneously give us DISABLED state, In this case, CPU1 reads an intermediate "SYSFS" state, even though it failed in etm4_enable_hw_smp_call(). The current code defers setting the state to DISABLED on CPU0. As a result, CPU1 will incorrectly save and restore the ETM context based on the intermediate "SYSFS" state. > which actually does not seem to be too bad as the earlier enablement > attempt had failed anyway. Just trying to understand what is the real > problem here. The problem is CPU1 might get an intermediate state, it turns out a stale value and might guide CPU idle flow to wrongly save and restore ETM context. > > In this case, CPU0 initiates the operation by taking the SYSFS mode to > > avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to > > configure the tracer registers. If any error occurs during this process, > > What kind of error can happen during this process ? So far, it might fail to claim a device and return an error. A similar issue might occur when CPU1 exits an idle state. For example, if CPU0 initiates the ETM enabling flow and sets the SYSFS mode in advance, once CPU1 is woken up from idle by an IPI, it reads the ETM state (SYSFS mode) and then restores and enables the ETM. This can happen even before CPU1 invokes etm4_enable_hw_smp_call() to complete the ETM enable flow. > > CPU0 rolls back by setting the mode to DISABLED. > > Which seems OK. > > > > > However, if CPU1 enters an idle state during this time, it might read > > the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly > > save and restore tracer context that is actually disabled. > > Right but CPU0 had marked the CPU1' state as DISABLED after the enable > attempt had failed. So what is the problem ? There is a race condition between CPU0 writing the state and CPU1 reading the state (during its CPU idle flow). CPU1 might read a state that is inconsistent with the actual ETM hardware state, which causes CPU1 to save and restore the ETM context incorrectly. A wider view is this series heavily relies on the ETM state to decide the linked path has been enabled and take action for saving and restoring all components on the path (not for ETM device only). We need a reliable state machine to reflect hardware state. To avoid any intermediate state, we always set the state on the target CPU. Thanks, Leo
LGTM.
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
> When enabling a tracer via SysFS interface, the device mode may be set
> by any CPU - not necessarily the target CPU. This can lead to race
> condition in SMP, and may result in incorrect mode values being read.
>
> Consider the following example, where CPU0 attempts to enable the tracer
> on CPU1 (the target CPU):
>
> CPU0 CPU1
> etm4_enable()
> ` coresight_take_mode(SYSFS)
> ` etm4_enable_sysfs()
> ` smp_call_function_single() ----> etm4_enable_hw_smp_call()
> /
> / CPU idle:
> / etm4_cpu_save()
> / ` coresight_get_mode()
> Failed to enable h/w / ^^^
> ` coresight_set_mode(DISABLED) <-' Read the intermediate SYSFS mode
>
> In this case, CPU0 initiates the operation by taking the SYSFS mode to
> avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
> configure the tracer registers. If any error occurs during this process,
> CPU0 rolls back by setting the mode to DISABLED.
>
> However, if CPU1 enters an idle state during this time, it might read
> the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
> save and restore tracer context that is actually disabled.
>
> To resolve the issue, this commit moves the device mode setting logic on
> the target CPU. This ensures that the device mode is only modified by
> the target CPU, eliminating race condition between mode writes and reads
> across CPUs.
>
> An additional change introduces the etm4_disable_hw_smp_call() function
> for SMP calls, which disables the tracer and explicitly set the mode to
> DISABLED during SysFS operations.
>
> The flow is updated with this change:
>
> CPU0 CPU1
> etm4_enable()
> ` etm4_enable_sysfs()
> ` smp_call_function_single() ----> etm4_enable_hw_smp_call()
> ` coresight_take_mode(SYSFS)
> Failed, set back to DISABLED
> ` coresight_set_mode(DISABLED)
>
> CPU idle:
> etm4_cpu_save()
> ` coresight_get_mode()
> ^^^
> Read out the DISABLED mode
>
> Fixes: c38a9ec2b2c1 ("coresight: etm4x: moving etm_drvdata::enable to atomic field")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++-------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 42e5d37403addc6ec81f2e3184522d67d1677c04..ee405c88ea5faa130819f96b00b8307f8764d58a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -590,10 +590,23 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> static void etm4_enable_hw_smp_call(void *info)
> {
> struct etm4_enable_arg *arg = info;
> + struct coresight_device *csdev;
>
> if (WARN_ON(!arg))
> return;
> +
> + csdev = arg->drvdata->csdev;
> + if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
> + /* Someone is already using the tracer */
> + arg->rc = -EBUSY;
> + return;
> + }
> +
> arg->rc = etm4_enable_hw(arg->drvdata);
> +
> + /* The tracer didn't start */
> + if (arg->rc)
> + coresight_set_mode(csdev, CS_MODE_DISABLED);
> }
>
> /*
> @@ -809,6 +822,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
> int ret = 0;
> struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> + if (!coresight_take_mode(csdev, CS_MODE_PERF))
> + return -EBUSY;
> +
> if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
> ret = -EINVAL;
> goto out;
> @@ -828,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
> ret = etm4_enable_hw(drvdata);
>
> out:
> + /* The tracer didn't start */
> + if (ret)
> + coresight_set_mode(csdev, CS_MODE_DISABLED);
> return ret;
> }
>
> @@ -880,11 +899,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
> {
> int ret;
>
> - if (!coresight_take_mode(csdev, mode)) {
> - /* Someone is already using the tracer */
> - return -EBUSY;
> - }
> -
> switch (mode) {
> case CS_MODE_SYSFS:
> ret = etm4_enable_sysfs(csdev, path);
> @@ -896,10 +910,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
> ret = -EINVAL;
> }
>
> - /* The tracer didn't start */
> - if (ret)
> - coresight_set_mode(csdev, CS_MODE_DISABLED);
> -
> return ret;
> }
>
> @@ -951,10 +961,9 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
> isb();
> }
>
> -static void etm4_disable_hw(void *info)
> +static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
> {
> u32 control;
> - struct etmv4_drvdata *drvdata = info;
> struct etmv4_config *config = &drvdata->config;
> struct coresight_device *csdev = drvdata->csdev;
> struct csdev_access *csa = &csdev->access;
> @@ -991,6 +1000,15 @@ static void etm4_disable_hw(void *info)
> "cpu: %d disable smp call done\n", drvdata->cpu);
> }
>
> +static void etm4_disable_hw_smp_call(void *info)
> +{
> + struct etmv4_drvdata *drvdata = info;
> +
> + etm4_disable_hw(drvdata);
> +
> + coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +}
> +
> static int etm4_disable_perf(struct coresight_device *csdev,
> struct perf_event *event)
> {
> @@ -1020,6 +1038,8 @@ static int etm4_disable_perf(struct coresight_device *csdev,
> /* TRCVICTLR::SSSTATUS, bit[9] */
> filters->ssstatus = (control & BIT(9));
>
> + coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +
> /*
> * perf will release trace ids when _free_aux() is
> * called at the end of the session.
> @@ -1045,7 +1065,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
> * Executing etm4_disable_hw on the cpu whose ETM is being disabled
> * ensures that register writes occur when cpu is powered.
> */
> - smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
> + smp_call_function_single(drvdata->cpu, etm4_disable_hw_smp_call,
> + drvdata, 1);
>
> raw_spin_unlock(&drvdata->spinlock);
>
> @@ -1085,9 +1106,6 @@ static void etm4_disable(struct coresight_device *csdev,
> etm4_disable_perf(csdev, event);
> break;
> }
> -
> - if (mode)
> - coresight_set_mode(csdev, CS_MODE_DISABLED);
> }
>
> static int etm4_resume_perf(struct coresight_device *csdev)
>
> --
> 2.34.1
>
--
Sincerely,
Yeoreum Yun
© 2016 - 2025 Red Hat, Inc.