[PATCH v4 00/11] CoreSight: Refactor power management for ETMv3/4

[PATCH v4 00/11] CoreSight: Refactor power management for ETMv3/4

[Leo Yan]

This series is extracted from the CoreSight power management fixes and
refactoring [1], focusing on ETMv3/4 power management.  The remaining
parts will be sent out separately. Hopefully, this makes things easier
for us to review and merge.

Compared to the previous version, only one new patch has been added — to
retain the sequencer state for ETMv4. No other changes are included.

This series has been verified on Juno-r2 platform.

[1] https://lore.kernel.org/linux-arm-kernel/20250915-arm_coresight_power_management_fix-v3-0-ea49e91124ec@arm.com/

---
Changes in v4:
- Added patch 10 for retaining sequencer state in ETMv4 driver (Mike).
- Added Mike's review tags.
- Added James' test tags.
- Link to v3: https://lore.kernel.org/r/20250915-arm_coresight_power_management_fix-v3-0-ea49e91124ec@arm.com

Changes in v3:
- Fixed building failure in ETMv3 driver (kernel test robot).
- Refactoring ETMv3 change for checking CPU ID (Levi).
- Fixed NULL pointer issue during CPU idle (James).
- Fixed lockdep complaint for HARDIRQ-safe and HARDIRA-unsafe (James).
- Fixed acquiring mutex in atomic context (James).
- Rebased on the latest coresight-next branch.
- Link to v2: https://lore.kernel.org/r/20250701-arm_cs_pm_fix_v3-v2-0-23ebb864fcc1@arm.com

Changes in v2:
- Refactored ETMv4 suspend and resume for reusing the normal enabling
  and disabling flows (James).
- Used a per-CPU structure to maintain path pointers (James).
- Supported helpers in CPU PM flows (James).
- Fixed the SMP-safe access to device mode.
- Fixed the context synchronization in ETMv4x driver.
- Link to v1: https://lore.kernel.org/linux-arm-kernel/20250516160742.1200904-1-leo.yan@arm.com/

Signed-off-by: Leo Yan <leo.yan@arm.com>

---
Leo Yan (11):
      coresight: Change device mode to atomic type
      coresight: etm4x: Always set tracer's device mode on target CPU
      coresight: etm3x: Always set tracer's device mode on target CPU
      coresight: etm4x: Correct polling IDLE bit
      coresight: etm4x: Ensure context synchronization is not ignored
      coresight: etm4x: Add context synchronization before enabling trace
      coresight: etm4x: Properly control filter in CPU idle with FEAT_TRF
      coresight: etm4x: Remove the state_needs_restore flag
      coresight: etm4x: Add flag to control single-shot restart
      coresight: etm4x: Retain sequencer state
      coresight: etm4x: Reuse normal enable and disable logic in CPU idle

 drivers/hwtracing/coresight/coresight-etm3x-core.c |  59 ++--
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 372 +++++++--------------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  62 ----
 include/linux/coresight.h                          |  25 +-
 4 files changed, 175 insertions(+), 343 deletions(-)
---
base-commit: 6fab32bb6508abbb8b7b1c5498e44f0c32320ed5
change-id: 20250909-arm_coresight_power_management_fix-139873f942e8

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

[PATCH v4 01/11] coresight: Change device mode to atomic type

[Leo Yan]

The device mode is defined as local type. This type cannot promise
SMP-safe access.

Change to atomic type and impose relax ordering, which ensures the
SMP-safe synchronisation and the ordering between the mode setting and
relevant operations.

Fixes: 22fd532eaa0c ("coresight: etm3x: adding operation mode for etm_enable()")
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 include/linux/coresight.h | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 6de59ce8ef8ca45c29e2f09c1b979eb7686b685f..3e5e5acd0c7fcde7d312d440da4355faaf682c7b 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -251,15 +251,11 @@ struct coresight_trace_id_map {
  *		by @coresight_ops.
  * @access:	Device i/o access abstraction for this device.
  * @dev:	The device entity associated to this component.
- * @mode:	This tracer's mode, i.e sysFS, Perf or disabled. This is
- *		actually an 'enum cs_mode', but is stored in an atomic type.
- *		This is always accessed through local_read() and local_set(),
- *		but wherever it's done from within the Coresight device's lock,
- *		a non-atomic read would also work. This is the main point of
- *		synchronisation between code happening inside the sysfs mode's
- *		coresight_mutex and outside when running in Perf mode. A compare
- *		and exchange swap is done to atomically claim one mode or the
- *		other.
+ * @mode:	The device mode, i.e sysFS, Perf or disabled. This is actually
+ *		an 'enum cs_mode' but stored in an atomic type. Access is always
+ *		through atomic APIs, ensuring SMP-safe synchronisation between
+ *		racing from sysFS and Perf mode. A compare-and-exchange
+ *		operation is done to atomically claim one mode or the other.
  * @refcnt:	keep track of what is in use. Only access this outside of the
  *		device's spinlock when the coresight_mutex held and mode ==
  *		CS_MODE_SYSFS. Otherwise it must be accessed from inside the
@@ -288,7 +284,7 @@ struct coresight_device {
 	const struct coresight_ops *ops;
 	struct csdev_access access;
 	struct device dev;
-	local_t	mode;
+	atomic_t mode;
 	int refcnt;
 	bool orphan;
 	/* sink specific fields */
@@ -621,13 +617,14 @@ static inline bool coresight_is_percpu_sink(struct coresight_device *csdev)
 static inline bool coresight_take_mode(struct coresight_device *csdev,
 				       enum cs_mode new_mode)
 {
-	return local_cmpxchg(&csdev->mode, CS_MODE_DISABLED, new_mode) ==
-	       CS_MODE_DISABLED;
+	int curr = CS_MODE_DISABLED;
+
+	return atomic_try_cmpxchg_acquire(&csdev->mode, &curr, new_mode);
 }
 
 static inline enum cs_mode coresight_get_mode(struct coresight_device *csdev)
 {
-	return local_read(&csdev->mode);
+	return atomic_read_acquire(&csdev->mode);
 }
 
 static inline void coresight_set_mode(struct coresight_device *csdev,
@@ -643,7 +640,7 @@ static inline void coresight_set_mode(struct coresight_device *csdev,
 	WARN(new_mode != CS_MODE_DISABLED && current_mode != CS_MODE_DISABLED &&
 	     current_mode != new_mode, "Device already in use\n");
 
-	local_set(&csdev->mode, new_mode);
+	atomic_set_release(&csdev->mode, new_mode);
 }
 
 struct coresight_device *coresight_register(struct coresight_desc *desc);

-- 
2.34.1

[PATCH v4 02/11] coresight: etm4x: Always set tracer's device mode on target CPU

[Leo Yan]

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_sysfs_smp_call()
function for SMP calls, which disables the tracer and explicitly set the
mode to DISABLED during SysFS operations. Rename
etm4_disable_hw_smp_call() to etm4_disable_sysfs_smp_call() for naming
consistency.

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")
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 60 ++++++++++++++--------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 020f070bf17dc0557dfb3ae4f282b0a0c1778bd8..1324b40d54210078b70a760ac1b084a4b20e8770 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -589,13 +589,26 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 	return rc;
 }
 
-static void etm4_enable_hw_smp_call(void *info)
+static void etm4_enable_sysfs_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);
 }
 
 /*
@@ -808,13 +821,14 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 			    struct perf_event *event,
 			    struct coresight_path *path)
 {
-	int ret = 0;
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+	int ret;
 
-	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
+		return -EINVAL;
+
+	if (!coresight_take_mode(csdev, CS_MODE_PERF))
+		return -EBUSY;
 
 	/* Configure the tracer based on the session's specifics */
 	ret = etm4_parse_event_config(csdev, event);
@@ -830,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 	ret = etm4_enable_hw(drvdata);
 
 out:
+	/* Failed to start tracer; roll back to DISABLED mode */
+	if (ret)
+		coresight_set_mode(csdev, CS_MODE_DISABLED);
 	return ret;
 }
 
@@ -861,7 +878,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
 	 */
 	arg.drvdata = drvdata;
 	ret = smp_call_function_single(drvdata->cpu,
-				       etm4_enable_hw_smp_call, &arg, 1);
+				       etm4_enable_sysfs_smp_call, &arg, 1);
 	if (!ret)
 		ret = arg.rc;
 	if (!ret)
@@ -882,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);
@@ -898,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;
 }
 
@@ -953,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;
@@ -993,6 +1000,15 @@ static void etm4_disable_hw(void *info)
 		"cpu: %d disable smp call done\n", drvdata->cpu);
 }
 
+static void etm4_disable_sysfs_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)
 {
@@ -1022,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.
@@ -1047,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_sysfs_smp_call,
+				 drvdata, 1);
 
 	raw_spin_unlock(&drvdata->spinlock);
 
@@ -1087,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

[PATCH v4 03/11] coresight: etm3x: Always set tracer's device mode on target CPU

[Leo Yan]

The ETMv3 driver shares the same issue as ETMv4 regarding race
conditions when accessing the device mode.

This commit applies the same fix: ensuring that the device mode is
modified only by the target CPU to eliminate race conditions across
CPUs.

Fixes: 22fd532eaa0c ("coresight: etm3x: adding operation mode for etm_enable()")
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm3x-core.c | 59 +++++++++++++++-------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 45630a1cd32fbd05ec8b2a6979f0174cacce365e..a5e809589d3e382acde24ee457e94e5dcb18ea35 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -439,13 +439,26 @@ struct etm_enable_arg {
 	int rc;
 };
 
-static void etm_enable_hw_smp_call(void *info)
+static void etm_enable_sysfs_smp_call(void *info)
 {
 	struct etm_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 = etm_enable_hw(arg->drvdata);
+
+	/* The tracer didn't start */
+	if (arg->rc)
+		coresight_set_mode(csdev, CS_MODE_DISABLED);
 }
 
 static int etm_cpu_id(struct coresight_device *csdev)
@@ -465,16 +478,26 @@ static int etm_enable_perf(struct coresight_device *csdev,
 			   struct coresight_path *path)
 {
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+	int ret;
 
 	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
 		return -EINVAL;
 
+	if (!coresight_take_mode(csdev, CS_MODE_PERF))
+		return -EBUSY;
+
 	/* Configure the tracer based on the session's specifics */
 	etm_parse_event_config(drvdata, event);
 	drvdata->traceid = path->trace_id;
 
 	/* And enable it */
-	return etm_enable_hw(drvdata);
+	ret = etm_enable_hw(drvdata);
+
+	/* Failed to start tracer; roll back to DISABLED mode */
+	if (ret)
+		coresight_set_mode(csdev, CS_MODE_DISABLED);
+
+	return ret;
 }
 
 static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path)
@@ -494,7 +517,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
 	if (cpu_online(drvdata->cpu)) {
 		arg.drvdata = drvdata;
 		ret = smp_call_function_single(drvdata->cpu,
-					       etm_enable_hw_smp_call, &arg, 1);
+					       etm_enable_sysfs_smp_call, &arg, 1);
 		if (!ret)
 			ret = arg.rc;
 		if (!ret)
@@ -517,12 +540,6 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
 		      enum cs_mode mode, struct coresight_path *path)
 {
 	int ret;
-	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-
-	if (!coresight_take_mode(csdev, mode)) {
-		/* Someone is already using the tracer */
-		return -EBUSY;
-	}
 
 	switch (mode) {
 	case CS_MODE_SYSFS:
@@ -535,17 +552,12 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
 		ret = -EINVAL;
 	}
 
-	/* The tracer didn't start */
-	if (ret)
-		coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
-
 	return ret;
 }
 
-static void etm_disable_hw(void *info)
+static void etm_disable_hw(struct etm_drvdata *drvdata)
 {
 	int i;
-	struct etm_drvdata *drvdata = info;
 	struct etm_config *config = &drvdata->config;
 	struct coresight_device *csdev = drvdata->csdev;
 
@@ -567,6 +579,15 @@ static void etm_disable_hw(void *info)
 		"cpu: %d disable smp call done\n", drvdata->cpu);
 }
 
+static void etm_disable_sysfs_smp_call(void *info)
+{
+	struct etm_drvdata *drvdata = info;
+
+	etm_disable_hw(drvdata);
+
+	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
+}
+
 static void etm_disable_perf(struct coresight_device *csdev)
 {
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -588,6 +609,8 @@ static void etm_disable_perf(struct coresight_device *csdev)
 
 	CS_LOCK(drvdata->csa.base);
 
+	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
+
 	/*
 	 * perf will release trace ids when _free_aux()
 	 * is called at the end of the session
@@ -612,7 +635,8 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
 	 * Executing etm_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, etm_disable_hw, drvdata, 1);
+	smp_call_function_single(drvdata->cpu, etm_disable_sysfs_smp_call,
+				 drvdata, 1);
 
 	spin_unlock(&drvdata->spinlock);
 	cpus_read_unlock();
@@ -652,9 +676,6 @@ static void etm_disable(struct coresight_device *csdev,
 		WARN_ON_ONCE(mode);
 		return;
 	}
-
-	if (mode)
-		coresight_set_mode(csdev, CS_MODE_DISABLED);
 }
 
 static const struct coresight_ops_source etm_source_ops = {

-- 
2.34.1

[PATCH v4 04/11] coresight: etm4x: Correct polling IDLE bit

[Leo Yan]

Since commit 4ff6039ffb79 ("coresight-etm4x: add isb() before reading
the TRCSTATR"), the code has incorrectly been polling the PMSTABLE bit
instead of the IDLE bit.

This commit corrects the typo.

Fixes: 4ff6039ffb79 ("coresight-etm4x: add isb() before reading the TRCSTATR")
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@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 1324b40d54210078b70a760ac1b084a4b20e8770..c562f82985192ad71e181be1b570c9a2f334f29f 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1924,7 +1924,7 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 		state->trcpdcr = etm4x_read32(csa, TRCPDCR);
 
 	/* wait for TRCSTATR.IDLE to go up */
-	if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) {
+	if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1)) {
 		dev_err(etm_dev,
 			"timeout while waiting for Idle Trace Status\n");
 		etm4_os_unlock(drvdata);

-- 
2.34.1

[PATCH v4 05/11] coresight: etm4x: Ensure context synchronization is not ignored

[Leo Yan]

As recommended in section 4.3.7 "Synchronization of register updates" of
ARM IHI0064H.b, a self-hosted trace analyzer should always executes an
ISB instruction after programming the trace unit registers.

An ISB works as a context synchronization event; a DSB is not required.
Removes the redundant barrier in the enabling flow.

The ISB was placed at the end of the enable and disable functions.
However, this does not guarantee a context synchronization event in the
calling code, which may speculatively execute across function
boundaries.

ISB instructions are moved into callers to ensure that a context
synchronization is imposed immediately after enabling or disabling trace
unit.

Fixes: 40f682ae5086 ("coresight: etm4x: Extract the trace unit controlling")
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 38 +++++++++++++++-------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index c562f82985192ad71e181be1b570c9a2f334f29f..45bf76901650641d45a0d85965651667a8762b8c 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -460,13 +460,6 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
 		return -ETIME;
 	}
 
-	/*
-	 * As recommended by section 4.3.7 ("Synchronization when using the
-	 * memory-mapped interface") of ARM IHI 0064D
-	 */
-	dsb(sy);
-	isb();
-
 	return 0;
 }
 
@@ -581,6 +574,13 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 
 	if (!drvdata->paused)
 		rc = etm4_enable_trace_unit(drvdata);
+
+	/*
+	 * As recommended by section 4.3.7 (Synchronization of register updates)
+	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
+	 * ISB instruction after programming the trace unit registers.
+	 */
+	isb();
 done:
 	etm4_cs_lock(drvdata, csa);
 
@@ -954,11 +954,6 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
 	if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1))
 		dev_err(etm_dev,
 			"timeout while waiting for PM stable Trace Status\n");
-	/*
-	 * As recommended by section 4.3.7 (Synchronization of register updates)
-	 * of ARM IHI 0064H.b.
-	 */
-	isb();
 }
 
 static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
@@ -981,6 +976,13 @@ static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
 
 	etm4_disable_trace_unit(drvdata);
 
+	/*
+	 * As recommended by section 4.3.7 (Synchronization of register updates)
+	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
+	 * ISB instruction after programming the trace unit registers.
+	 */
+	isb();
+
 	/* read the status of the single shot comparators */
 	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
 		config->ss_status[i] =
@@ -1118,6 +1120,12 @@ static int etm4_resume_perf(struct coresight_device *csdev)
 
 	etm4_cs_unlock(drvdata, csa);
 	etm4_enable_trace_unit(drvdata);
+	/*
+	 * As recommended by section 4.3.7 (Synchronization of register updates)
+	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
+	 * ISB instruction after programming the trace unit registers.
+	 */
+	isb();
 	etm4_cs_lock(drvdata, csa);
 
 	drvdata->paused = false;
@@ -1134,6 +1142,12 @@ static void etm4_pause_perf(struct coresight_device *csdev)
 
 	etm4_cs_unlock(drvdata, csa);
 	etm4_disable_trace_unit(drvdata);
+	/*
+	 * As recommended by section 4.3.7 (Synchronization of register updates)
+	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
+	 * ISB instruction after programming the trace unit registers.
+	 */
+	isb();
 	etm4_cs_lock(drvdata, csa);
 
 	drvdata->paused = true;

-- 
2.34.1

Re: [PATCH v4 05/11] coresight: etm4x: Ensure context synchronization is not ignored

[Suzuki K Poulose]

On 24/10/2025 17:45, Leo Yan wrote:
> As recommended in section 4.3.7 "Synchronization of register updates" of
> ARM IHI0064H.b, a self-hosted trace analyzer should always executes an
> ISB instruction after programming the trace unit registers.
> 
> An ISB works as a context synchronization event; a DSB is not required.
> Removes the redundant barrier in the enabling flow.

It is required for MMIO based instances and must be retained.

> 
> The ISB was placed at the end of the enable and disable functions.
> However, this does not guarantee a context synchronization event in the
> calling code, which may speculatively execute across function
> boundaries.

It is not allowed to commit though, right ? I don't think this change
is required TBH.

Suzuki

> 
> ISB instructions are moved into callers to ensure that a context
> synchronization is imposed immediately after enabling or disabling trace
> unit.
> 
> Fixes: 40f682ae5086 ("coresight: etm4x: Extract the trace unit controlling")
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 38 +++++++++++++++-------
>   1 file changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index c562f82985192ad71e181be1b570c9a2f334f29f..45bf76901650641d45a0d85965651667a8762b8c 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -460,13 +460,6 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
>   		return -ETIME;
>   	}
>   
> -	/*
> -	 * As recommended by section 4.3.7 ("Synchronization when using the
> -	 * memory-mapped interface") of ARM IHI 0064D
> -	 */
> -	dsb(sy);
> -	isb();
> -
>   	return 0;
>   }
>   
> @@ -581,6 +574,13 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>   
>   	if (!drvdata->paused)
>   		rc = etm4_enable_trace_unit(drvdata);
> +
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();
>   done:
>   	etm4_cs_lock(drvdata, csa);
>   
> @@ -954,11 +954,6 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
>   	if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1))
>   		dev_err(etm_dev,
>   			"timeout while waiting for PM stable Trace Status\n");
> -	/*
> -	 * As recommended by section 4.3.7 (Synchronization of register updates)
> -	 * of ARM IHI 0064H.b.
> -	 */
> -	isb();
>   }
>   
>   static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
> @@ -981,6 +976,13 @@ static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
>   
>   	etm4_disable_trace_unit(drvdata);
>   
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();
> +
>   	/* read the status of the single shot comparators */
>   	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
>   		config->ss_status[i] =
> @@ -1118,6 +1120,12 @@ static int etm4_resume_perf(struct coresight_device *csdev)
>   
>   	etm4_cs_unlock(drvdata, csa);
>   	etm4_enable_trace_unit(drvdata);
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();
>   	etm4_cs_lock(drvdata, csa);
>   
>   	drvdata->paused = false;
> @@ -1134,6 +1142,12 @@ static void etm4_pause_perf(struct coresight_device *csdev)
>   
>   	etm4_cs_unlock(drvdata, csa);
>   	etm4_disable_trace_unit(drvdata);
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();
>   	etm4_cs_lock(drvdata, csa);
>   
>   	drvdata->paused = true;
> 

Re: [PATCH v4 05/11] coresight: etm4x: Ensure context synchronization is not ignored

[Yeoreum Yun]

Hi Suzuki,
> On 24/10/2025 17:45, Leo Yan wrote:
> > As recommended in section 4.3.7 "Synchronization of register updates" of
> > ARM IHI0064H.b, a self-hosted trace analyzer should always executes an
> > ISB instruction after programming the trace unit registers.
> >
> > An ISB works as a context synchronization event; a DSB is not required.
> > Removes the redundant barrier in the enabling flow.
>
> It is required for MMIO based instances and must be retained.

I think it seems fine. since the the etm4x device mmio is mapped as
Device-nGnRE and according to the section Leo mention:

  Synchronization when using the memory-mapped interface
  ..
  When disabling or enabling the trace unit, the trace analyzer must poll TRCSTATR to check the trace unit is either
  idle or not idle, as described in Use of the trace unit main enable bit on page 4-169:
    • When the memory is marked as Device-nGnRE or stronger.
      — Write to enable or disable the trace unit.
      — Poll TRCSTATR to ensure the previous write has completed.
      — Execute an ISB operation.

Therefore, we can omit the dsb in here.

[...]

Thanks.

--
Sincerely,
Yeoreum Yun

Re: [PATCH v4 05/11] coresight: etm4x: Ensure context synchronization is not ignored

[Suzuki K Poulose]

On 27/10/2025 10:43, Yeoreum Yun wrote:
> Hi Suzuki,
>> On 24/10/2025 17:45, Leo Yan wrote:
>>> As recommended in section 4.3.7 "Synchronization of register updates" of
>>> ARM IHI0064H.b, a self-hosted trace analyzer should always executes an
>>> ISB instruction after programming the trace unit registers.
>>>
>>> An ISB works as a context synchronization event; a DSB is not required.
>>> Removes the redundant barrier in the enabling flow.
>>
>> It is required for MMIO based instances and must be retained.
> 
> I think it seems fine. since the the etm4x device mmio is mapped as
> Device-nGnRE and according to the section Leo mention:
> 
>    Synchronization when using the memory-mapped interface
>    ..
>    When disabling or enabling the trace unit, the trace analyzer must poll TRCSTATR to check the trace unit is either
>    idle or not idle, as described in Use of the trace unit main enable bit on page 4-169:
>      • When the memory is marked as Device-nGnRE or stronger.
>        — Write to enable or disable the trace unit.
>        — Poll TRCSTATR to ensure the previous write has completed.
>        — Execute an ISB operation.
> 
> Therefore, we can omit the dsb in here.

You are right, to be precise, as long as we quote the following, I am fine.


Section 4.3.7, Synchronization when using the memory-mapped interface -

When using the memory-mapped interface to program the trace unit, the 
trace analyzer must ensure that writes have completed, to ensure that 
the trace unit is fully programmed and either enabled or disabled.

• If the memory marked is as Device-nGnRE or stronger, read back the 
value of any register in the trace unit. This relies on peripheral 
coherence order defined in the Arm architecture.


Suzuki


> 
> [...]
> 
> Thanks.
> 
> --
> Sincerely,
> Yeoreum Yun

Re: [PATCH v4 05/11] coresight: etm4x: Ensure context synchronization is not ignored

[Leo Yan]

On Mon, Oct 27, 2025 at 09:49:39AM +0000, Suzuki Kuruppassery Poulose wrote:
> On 24/10/2025 17:45, Leo Yan wrote:
> > As recommended in section 4.3.7 "Synchronization of register updates" of
> > ARM IHI0064H.b, a self-hosted trace analyzer should always executes an
> > ISB instruction after programming the trace unit registers.
> > 
> > An ISB works as a context synchronization event; a DSB is not required.
> > Removes the redundant barrier in the enabling flow.
> 
> It is required for MMIO based instances and must be retained.

Sorry I did not give details in commit log so caused confusion (and
thanks a lot for Levi's supplement info!)

I will amend in next spin.

> > The ISB was placed at the end of the enable and disable functions.
> > However, this does not guarantee a context synchronization event in the
> > calling code, which may speculatively execute across function
> > boundaries.
> 
> It is not allowed to commit though, right ? I don't think this change
> is required TBH.

Correct, I will drop this part.

Thanks,
Leo

[PATCH v4 06/11] coresight: etm4x: Add context synchronization before enabling trace

[Leo Yan]

According to the software usage PKLXF in Arm ARM (ARM DDI 0487 L.a), a
Context synchronization event is required before enabling the trace
unit. An ISB is added to meet this requirement.

Improved the barrier comments to provide more accurate information by
specifying which section of the document the requirement comes from and
clarifying its intended purpose.

Fixes: 1ab3bb9df5e3 ("coresight: etm4x: Add necessary synchronization for sysreg access")
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 +++++++++++++++++++---
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 45bf76901650641d45a0d85965651667a8762b8c..7eb30e85bf03d1eda8212faa3c831338b70f86c3 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -446,13 +446,37 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
 		etm4x_relaxed_write32(csa, TRCRSR_TA, TRCRSR);
 
 	etm4x_allow_trace(drvdata);
+
+	/*
+	 * According to software usage PKLXF in Arm ARM (ARM DDI 0487 L.a),
+	 * execute a Context synchronization event to guarantee the trace unit
+	 * will observe the new values of the System registers.
+	 */
+	if (!csa->io_mem)
+		isb();
+
 	/* Enable the trace unit */
 	etm4x_relaxed_write32(csa, 1, TRCPRGCTLR);
 
-	/* Synchronize the register updates for sysreg access */
+	/*
+	 * As recommended by section 4.3.7 ("Synchronization when using system
+	 * instructions to progrom the trace unit") of ARM IHI 0064H.b, the
+	 * self-hosted trace analyzer must perform a Context synchronization
+	 * event between writing to the TRCPRGCTLR and reading the TRCSTATR.
+	 */
 	if (!csa->io_mem)
 		isb();
 
+	/*
+	 * For the memory-mapped interface, the registers are mapped as Device
+	 * type (Device-nGnRE). As described in section 4.3.7 ("Synchronization
+	 * of register updates") of ARM IHI0064H.b, read back the value of any
+	 * register in the trace unit to ensure writes have completed.
+	 *
+	 * Therefore, polling on TRCSTATR ensures that the writing TRCPRGCTLR
+	 * is complete.
+	 */
+
 	/* wait for TRCSTATR.IDLE to go back down to '0' */
 	if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 0)) {
 		dev_err(etm_dev,
@@ -931,11 +955,16 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
 	 */
 	etm4x_prohibit_trace(drvdata);
 	/*
-	 * Make sure everything completes before disabling, as recommended
-	 * by section 7.3.77 ("TRCVICTLR, ViewInst Main Control Register,
-	 * SSTATUS") of ARM IHI 0064D
+	 * Prevent being speculative at the point of disabling the trace unit,
+	 * as recommended by section 7.3.77 ("TRCVICTLR, ViewInst Main Control
+	 * Register, SSTATUS") of ARM IHI 0064D
 	 */
 	dsb(sy);
+	/*
+	 * According to software usage VKHHY in Arm ARM (ARM DDI 0487 L.a),
+	 * execute a Context synchronization event to guarantee no new
+	 * program-flow trace is generated.
+	 */
 	isb();
 	/* Trace synchronization barrier, is a nop if not supported */
 	tsb_csync();

-- 
2.34.1

Re: [PATCH v4 06/11] coresight: etm4x: Add context synchronization before enabling trace

[Yeoreum Yun]

Reviewed-by: Yeoreun Yun <yeoreum.yun@arm.com>
> According to the software usage PKLXF in Arm ARM (ARM DDI 0487 L.a), a
> Context synchronization event is required before enabling the trace
> unit. An ISB is added to meet this requirement.
>
> Improved the barrier comments to provide more accurate information by
> specifying which section of the document the requirement comes from and
> clarifying its intended purpose.
>
> Fixes: 1ab3bb9df5e3 ("coresight: etm4x: Add necessary synchronization for sysreg access")
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 +++++++++++++++++++---
>  1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 45bf76901650641d45a0d85965651667a8762b8c..7eb30e85bf03d1eda8212faa3c831338b70f86c3 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -446,13 +446,37 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
>  		etm4x_relaxed_write32(csa, TRCRSR_TA, TRCRSR);
>
>  	etm4x_allow_trace(drvdata);
> +
> +	/*
> +	 * According to software usage PKLXF in Arm ARM (ARM DDI 0487 L.a),
> +	 * execute a Context synchronization event to guarantee the trace unit
> +	 * will observe the new values of the System registers.
> +	 */
> +	if (!csa->io_mem)
> +		isb();
> +
>  	/* Enable the trace unit */
>  	etm4x_relaxed_write32(csa, 1, TRCPRGCTLR);
>
> -	/* Synchronize the register updates for sysreg access */
> +	/*
> +	 * As recommended by section 4.3.7 ("Synchronization when using system
> +	 * instructions to progrom the trace unit") of ARM IHI 0064H.b, the
> +	 * self-hosted trace analyzer must perform a Context synchronization
> +	 * event between writing to the TRCPRGCTLR and reading the TRCSTATR.
> +	 */
>  	if (!csa->io_mem)
>  		isb();
>
> +	/*
> +	 * For the memory-mapped interface, the registers are mapped as Device
> +	 * type (Device-nGnRE). As described in section 4.3.7 ("Synchronization
> +	 * of register updates") of ARM IHI0064H.b, read back the value of any
> +	 * register in the trace unit to ensure writes have completed.
> +	 *
> +	 * Therefore, polling on TRCSTATR ensures that the writing TRCPRGCTLR
> +	 * is complete.
> +	 */
> +
>  	/* wait for TRCSTATR.IDLE to go back down to '0' */
>  	if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 0)) {
>  		dev_err(etm_dev,
> @@ -931,11 +955,16 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
>  	 */
>  	etm4x_prohibit_trace(drvdata);
>  	/*
> -	 * Make sure everything completes before disabling, as recommended
> -	 * by section 7.3.77 ("TRCVICTLR, ViewInst Main Control Register,
> -	 * SSTATUS") of ARM IHI 0064D
> +	 * Prevent being speculative at the point of disabling the trace unit,
> +	 * as recommended by section 7.3.77 ("TRCVICTLR, ViewInst Main Control
> +	 * Register, SSTATUS") of ARM IHI 0064D
>  	 */
>  	dsb(sy);
> +	/*
> +	 * According to software usage VKHHY in Arm ARM (ARM DDI 0487 L.a),
> +	 * execute a Context synchronization event to guarantee no new
> +	 * program-flow trace is generated.
> +	 */
>  	isb();
>  	/* Trace synchronization barrier, is a nop if not supported */
>  	tsb_csync();
>
> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun

[PATCH v4 07/11] coresight: etm4x: Properly control filter in CPU idle with FEAT_TRF

[Leo Yan]

If a CPU supports FEAT_TRF, as described in the section K5.5 "Context
switching", Arm ARM (ARM DDI 0487 L.a), it defines a flow to prohibit
program-flow trace, execute a TSB CSYNC instruction for flushing,
followed by clearing TRCPRGCTLR.EN bit.

To restore the state, the reverse sequence is required.

This differs from the procedure described in the section 3.4.1 "The
procedure when powering down the PE" of ARM IHI0064H.b, which involves
the OS Lock to prevent external debugger accesses and implicitly
disables trace.

To be compatible with different ETM versions, explicitly control trace
unit using etm4_disable_trace_unit() and etm4_enable_trace_unit()
during CPU idle to comply with FEAT_TRF.

As a result, the save states for TRFCR_ELx and trcprgctlr are redundant,
remove them.

Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 ++++++++++++++++------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  3 ---
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 7eb30e85bf03d1eda8212faa3c831338b70f86c3..df3260f2dfa72456c60fba4a8fa655fb6b48b95b 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1882,9 +1882,18 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 		goto out;
 	}
 
+	if (!drvdata->paused)
+		etm4_disable_trace_unit(drvdata);
+
+	/*
+	 * As recommended by section 4.3.7 (Synchronization of register updates)
+	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
+	 * ISB instruction after programming the trace unit registers.
+	 */
+	isb();
+
 	state = drvdata->save_state;
 
-	state->trcprgctlr = etm4x_read32(csa, TRCPRGCTLR);
 	if (drvdata->nr_pe)
 		state->trcprocselr = etm4x_read32(csa, TRCPROCSELR);
 	state->trcconfigr = etm4x_read32(csa, TRCCONFIGR);
@@ -1994,9 +2003,6 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
 {
 	int ret = 0;
 
-	/* Save the TRFCR irrespective of whether the ETM is ON */
-	if (drvdata->trfcr)
-		drvdata->save_trfcr = read_trfcr();
 	/*
 	 * Save and restore the ETM Trace registers only if
 	 * the ETM is active.
@@ -2018,7 +2024,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	etm4_cs_unlock(drvdata, csa);
 	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
 
-	etm4x_relaxed_write32(csa, state->trcprgctlr, TRCPRGCTLR);
 	if (drvdata->nr_pe)
 		etm4x_relaxed_write32(csa, state->trcprocselr, TRCPROCSELR);
 	etm4x_relaxed_write32(csa, state->trcconfigr, TRCCONFIGR);
@@ -2103,13 +2108,22 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 
 	/* Unlock the OS lock to re-enable trace and external debug access */
 	etm4_os_unlock(drvdata);
+
+	if (!drvdata->paused)
+		etm4_enable_trace_unit(drvdata);
+
+	/*
+	 * As recommended by section 4.3.7 (Synchronization of register updates)
+	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
+	 * ISB instruction after programming the trace unit registers.
+	 */
+	isb();
+
 	etm4_cs_lock(drvdata, csa);
 }
 
 static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 {
-	if (drvdata->trfcr)
-		write_trfcr(drvdata->save_trfcr);
 	if (drvdata->state_needs_restore)
 		__etm4_cpu_restore(drvdata);
 }
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 13ec9ecef46f5b60a38c464e8ebde3ef72e7d50b..b8796b4271025f6aba79fe3cd6e4d17d0b56952d 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -866,7 +866,6 @@ struct etmv4_config {
  * struct etm4_save_state - state to be preserved when ETM is without power
  */
 struct etmv4_save_state {
-	u32	trcprgctlr;
 	u32	trcprocselr;
 	u32	trcconfigr;
 	u32	trcauxctlr;
@@ -980,7 +979,6 @@ struct etmv4_save_state {
  *		at runtime, due to the additional setting of TRFCR_CX when
  *		in EL2. Otherwise, 0.
  * @config:	structure holding configuration parameters.
- * @save_trfcr:	Saved TRFCR_EL1 register during a CPU PM event.
  * @save_state:	State to be preserved across power loss
  * @state_needs_restore: True when there is context to restore after PM exit
  * @skip_power_up: Indicates if an implementation can skip powering up
@@ -1037,7 +1035,6 @@ struct etmv4_drvdata {
 	bool				lpoverride;
 	u64				trfcr;
 	struct etmv4_config		config;
-	u64				save_trfcr;
 	struct etmv4_save_state		*save_state;
 	bool				state_needs_restore;
 	bool				skip_power_up;

-- 
2.34.1

Re: [PATCH v4 07/11] coresight: etm4x: Properly control filter in CPU idle with FEAT_TRF

[Yeoreum Yun]

Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
On Fri, Oct 24, 2025 at 05:45:55PM +0100, Leo Yan wrote:
> If a CPU supports FEAT_TRF, as described in the section K5.5 "Context
> switching", Arm ARM (ARM DDI 0487 L.a), it defines a flow to prohibit
> program-flow trace, execute a TSB CSYNC instruction for flushing,
> followed by clearing TRCPRGCTLR.EN bit.
>
> To restore the state, the reverse sequence is required.
>
> This differs from the procedure described in the section 3.4.1 "The
> procedure when powering down the PE" of ARM IHI0064H.b, which involves
> the OS Lock to prevent external debugger accesses and implicitly
> disables trace.
>
> To be compatible with different ETM versions, explicitly control trace
> unit using etm4_disable_trace_unit() and etm4_enable_trace_unit()
> during CPU idle to comply with FEAT_TRF.
>
> As a result, the save states for TRFCR_ELx and trcprgctlr are redundant,
> remove them.
>
> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 ++++++++++++++++------
>  drivers/hwtracing/coresight/coresight-etm4x.h      |  3 ---
>  2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 7eb30e85bf03d1eda8212faa3c831338b70f86c3..df3260f2dfa72456c60fba4a8fa655fb6b48b95b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1882,9 +1882,18 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>  		goto out;
>  	}
>
> +	if (!drvdata->paused)
> +		etm4_disable_trace_unit(drvdata);
> +
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();
> +
>  	state = drvdata->save_state;
>
> -	state->trcprgctlr = etm4x_read32(csa, TRCPRGCTLR);
>  	if (drvdata->nr_pe)
>  		state->trcprocselr = etm4x_read32(csa, TRCPROCSELR);
>  	state->trcconfigr = etm4x_read32(csa, TRCCONFIGR);
> @@ -1994,9 +2003,6 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
>  {
>  	int ret = 0;
>
> -	/* Save the TRFCR irrespective of whether the ETM is ON */
> -	if (drvdata->trfcr)
> -		drvdata->save_trfcr = read_trfcr();
>  	/*
>  	 * Save and restore the ETM Trace registers only if
>  	 * the ETM is active.
> @@ -2018,7 +2024,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>  	etm4_cs_unlock(drvdata, csa);
>  	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
>
> -	etm4x_relaxed_write32(csa, state->trcprgctlr, TRCPRGCTLR);
>  	if (drvdata->nr_pe)
>  		etm4x_relaxed_write32(csa, state->trcprocselr, TRCPROCSELR);
>  	etm4x_relaxed_write32(csa, state->trcconfigr, TRCCONFIGR);
> @@ -2103,13 +2108,22 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>
>  	/* Unlock the OS lock to re-enable trace and external debug access */
>  	etm4_os_unlock(drvdata);
> +
> +	if (!drvdata->paused)
> +		etm4_enable_trace_unit(drvdata);
> +
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();
> +
>  	etm4_cs_lock(drvdata, csa);
>  }
>
>  static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>  {
> -	if (drvdata->trfcr)
> -		write_trfcr(drvdata->save_trfcr);
>  	if (drvdata->state_needs_restore)
>  		__etm4_cpu_restore(drvdata);
>  }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 13ec9ecef46f5b60a38c464e8ebde3ef72e7d50b..b8796b4271025f6aba79fe3cd6e4d17d0b56952d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -866,7 +866,6 @@ struct etmv4_config {
>   * struct etm4_save_state - state to be preserved when ETM is without power
>   */
>  struct etmv4_save_state {
> -	u32	trcprgctlr;
>  	u32	trcprocselr;
>  	u32	trcconfigr;
>  	u32	trcauxctlr;
> @@ -980,7 +979,6 @@ struct etmv4_save_state {
>   *		at runtime, due to the additional setting of TRFCR_CX when
>   *		in EL2. Otherwise, 0.
>   * @config:	structure holding configuration parameters.
> - * @save_trfcr:	Saved TRFCR_EL1 register during a CPU PM event.
>   * @save_state:	State to be preserved across power loss
>   * @state_needs_restore: True when there is context to restore after PM exit
>   * @skip_power_up: Indicates if an implementation can skip powering up
> @@ -1037,7 +1035,6 @@ struct etmv4_drvdata {
>  	bool				lpoverride;
>  	u64				trfcr;
>  	struct etmv4_config		config;
> -	u64				save_trfcr;
>  	struct etmv4_save_state		*save_state;
>  	bool				state_needs_restore;
>  	bool				skip_power_up;
>
> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun

Re: [PATCH v4 07/11] coresight: etm4x: Properly control filter in CPU idle with FEAT_TRF

[Suzuki K Poulose]

On 24/10/2025 17:45, Leo Yan wrote:
> If a CPU supports FEAT_TRF, as described in the section K5.5 "Context
> switching", Arm ARM (ARM DDI 0487 L.a), it defines a flow to prohibit
> program-flow trace, execute a TSB CSYNC instruction for flushing,
> followed by clearing TRCPRGCTLR.EN bit.
> 
> To restore the state, the reverse sequence is required.
> 
> This differs from the procedure described in the section 3.4.1 "The
> procedure when powering down the PE" of ARM IHI0064H.b, which involves
> the OS Lock to prevent external debugger accesses and implicitly
> disables trace.
> 
> To be compatible with different ETM versions, explicitly control trace
> unit using etm4_disable_trace_unit() and etm4_enable_trace_unit()
> during CPU idle to comply with FEAT_TRF.
> 
> As a result, the save states for TRFCR_ELx and trcprgctlr are redundant,
> remove them.
> 
> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 ++++++++++++++++------
>   drivers/hwtracing/coresight/coresight-etm4x.h      |  3 ---
>   2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 7eb30e85bf03d1eda8212faa3c831338b70f86c3..df3260f2dfa72456c60fba4a8fa655fb6b48b95b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1882,9 +1882,18 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>   		goto out;
>   	}
>  



> +	if (!drvdata->paused)
> +		etm4_disable_trace_unit(drvdata);
> +
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();
> +



>   	/* Unlock the OS lock to re-enable trace and external debug access */
>   	etm4_os_unlock(drvdata);
> +
> +	if (!drvdata->paused)
> +		etm4_enable_trace_unit(drvdata);
> +
> +	/*
> +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> +	 * ISB instruction after programming the trace unit registers.
> +	 */
> +	isb();

I guess these ISBs are not required according to the comment on the 
previous patch ? Otherwise looks good to me.

Suzuki


> +
>   	etm4_cs_lock(drvdata, csa);
>   }
>   
>   static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>   {
> -	if (drvdata->trfcr)
> -		write_trfcr(drvdata->save_trfcr);
>   	if (drvdata->state_needs_restore)
>   		__etm4_cpu_restore(drvdata);
>   }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 13ec9ecef46f5b60a38c464e8ebde3ef72e7d50b..b8796b4271025f6aba79fe3cd6e4d17d0b56952d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -866,7 +866,6 @@ struct etmv4_config {
>    * struct etm4_save_state - state to be preserved when ETM is without power
>    */
>   struct etmv4_save_state {
> -	u32	trcprgctlr;
>   	u32	trcprocselr;
>   	u32	trcconfigr;
>   	u32	trcauxctlr;
> @@ -980,7 +979,6 @@ struct etmv4_save_state {
>    *		at runtime, due to the additional setting of TRFCR_CX when
>    *		in EL2. Otherwise, 0.
>    * @config:	structure holding configuration parameters.
> - * @save_trfcr:	Saved TRFCR_EL1 register during a CPU PM event.
>    * @save_state:	State to be preserved across power loss
>    * @state_needs_restore: True when there is context to restore after PM exit
>    * @skip_power_up: Indicates if an implementation can skip powering up
> @@ -1037,7 +1035,6 @@ struct etmv4_drvdata {
>   	bool				lpoverride;
>   	u64				trfcr;
>   	struct etmv4_config		config;
> -	u64				save_trfcr;
>   	struct etmv4_save_state		*save_state;
>   	bool				state_needs_restore;
>   	bool				skip_power_up;
> 

Re: [PATCH v4 07/11] coresight: etm4x: Properly control filter in CPU idle with FEAT_TRF

[Leo Yan]

On Mon, Nov 03, 2025 at 02:30:28PM +0000, Suzuki Kuruppassery Poulose wrote:

[...]

> > @@ -1882,9 +1882,18 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
> >   		goto out;
> >   	}
> 
> > +	if (!drvdata->paused)
> > +		etm4_disable_trace_unit(drvdata);
> > +
> > +	/*
> > +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> > +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> > +	 * ISB instruction after programming the trace unit registers.
> > +	 */
> > +	isb();
> > +
> 
> 
> 
> >   	/* Unlock the OS lock to re-enable trace and external debug access */
> >   	etm4_os_unlock(drvdata);
> > +
> > +	if (!drvdata->paused)
> > +		etm4_enable_trace_unit(drvdata);
> > +
> > +	/*
> > +	 * As recommended by section 4.3.7 (Synchronization of register updates)
> > +	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> > +	 * ISB instruction after programming the trace unit registers.
> > +	 */
> > +	isb();
> 
> I guess these ISBs are not required according to the comment on the previous
> patch ? Otherwise looks good to me.

Yes, I will drop this in new version.  Thanks for reminding.

Leo

[PATCH v4 08/11] coresight: etm4x: Remove the state_needs_restore flag

[Leo Yan]

When the restore flow is invoked, it means no error occurred during the
save phase. Otherwise, if any errors happened while saving the context,
the function would return an error and abort the suspend sequence.

Therefore, the state_needs_restore flag is unnecessary. The save and
restore functions are changed to check two conditions:

1) The global flag pm_save_enable is SELF_HOSTED mode;
2) The device is in active mode (non DISABLED).

Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 14 ++++++++------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  2 --
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index df3260f2dfa72456c60fba4a8fa655fb6b48b95b..15f70268e5b605a6881504c0c60b001251fbbddf 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1984,8 +1984,6 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 		goto out;
 	}
 
-	drvdata->state_needs_restore = true;
-
 	/*
 	 * Power can be removed from the trace unit now. We do this to
 	 * potentially save power on systems that respect the TRCPDCR_PU
@@ -2003,11 +2001,14 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
 {
 	int ret = 0;
 
+	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
+		return 0;
+
 	/*
 	 * Save and restore the ETM Trace registers only if
 	 * the ETM is active.
 	 */
-	if (coresight_get_mode(drvdata->csdev) && drvdata->save_state)
+	if (coresight_get_mode(drvdata->csdev))
 		ret = __etm4_cpu_save(drvdata);
 	return ret;
 }
@@ -2097,8 +2098,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	if (!drvdata->skip_power_up)
 		etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);
 
-	drvdata->state_needs_restore = false;
-
 	/*
 	 * As recommended by section 4.3.7 ("Synchronization when using the
 	 * memory-mapped interface") of ARM IHI 0064D
@@ -2124,7 +2123,10 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 
 static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 {
-	if (drvdata->state_needs_restore)
+	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
+		return;
+
+	if (coresight_get_mode(drvdata->csdev))
 		__etm4_cpu_restore(drvdata);
 }
 
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index b8796b4271025f6aba79fe3cd6e4d17d0b56952d..012c52fd19338133129752d35523dc102df24604 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -980,7 +980,6 @@ struct etmv4_save_state {
  *		in EL2. Otherwise, 0.
  * @config:	structure holding configuration parameters.
  * @save_state:	State to be preserved across power loss
- * @state_needs_restore: True when there is context to restore after PM exit
  * @skip_power_up: Indicates if an implementation can skip powering up
  *		   the trace unit.
  * @paused:	Indicates if the trace unit is paused.
@@ -1036,7 +1035,6 @@ struct etmv4_drvdata {
 	u64				trfcr;
 	struct etmv4_config		config;
 	struct etmv4_save_state		*save_state;
-	bool				state_needs_restore;
 	bool				skip_power_up;
 	bool				paused;
 	DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);

-- 
2.34.1

[PATCH v4 09/11] coresight: etm4x: Add flag to control single-shot restart

[Leo Yan]

Add a new argument restart_ss to etm4_enable_hw(). When passed as true,
it resets the TRCSSCSRn.STATUS bit to 0 to re-enable single-shot
control.

No functional change. This is a preparation for a subsequent change.

Reviewed-by: Mike Leach <mike.leach@linaro.org>
Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 15f70268e5b605a6881504c0c60b001251fbbddf..4c6236ad241d1a13515160ca478fae839a891e38 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -487,7 +487,7 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
 	return 0;
 }
 
-static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
+static int etm4_enable_hw(struct etmv4_drvdata *drvdata, bool restart_ss)
 {
 	int i, rc;
 	struct etmv4_config *config = &drvdata->config;
@@ -562,9 +562,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 		etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i));
 
 	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
-		/* always clear status bit on restart if using single-shot */
-		if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
-			config->ss_status[i] &= ~TRCSSCSRn_STATUS;
+		if (restart_ss) {
+			/* always clear status bit on restart if using single-shot */
+			if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
+				config->ss_status[i] &= ~TRCSSCSRn_STATUS;
+		}
 		etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
 		etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
 		if (etm4x_sspcicrn_present(drvdata, i))
@@ -628,7 +630,7 @@ static void etm4_enable_sysfs_smp_call(void *info)
 		return;
 	}
 
-	arg->rc = etm4_enable_hw(arg->drvdata);
+	arg->rc = etm4_enable_hw(arg->drvdata, true);
 
 	/* The tracer didn't start */
 	if (arg->rc)
@@ -865,7 +867,7 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 	drvdata->paused = !!READ_ONCE(event->hw.aux_paused);
 
 	/* And enable it */
-	ret = etm4_enable_hw(drvdata);
+	ret = etm4_enable_hw(drvdata, true);
 
 out:
 	/* Failed to start tracer; roll back to DISABLED mode */
@@ -1831,7 +1833,7 @@ static int etm4_starting_cpu(unsigned int cpu)
 		etm4_os_unlock(etmdrvdata[cpu]);
 
 	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
-		etm4_enable_hw(etmdrvdata[cpu]);
+		etm4_enable_hw(etmdrvdata[cpu], true);
 	raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
 	return 0;
 }

-- 
2.34.1

Re: [PATCH v4 09/11] coresight: etm4x: Add flag to control single-shot restart

[Yeoreum Yun]

Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>

> Add a new argument restart_ss to etm4_enable_hw(). When passed as true,
> it resets the TRCSSCSRn.STATUS bit to 0 to re-enable single-shot
> control.
>
> No functional change. This is a preparation for a subsequent change.
>
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 15f70268e5b605a6881504c0c60b001251fbbddf..4c6236ad241d1a13515160ca478fae839a891e38 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -487,7 +487,7 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata)
>  	return 0;
>  }
>
> -static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> +static int etm4_enable_hw(struct etmv4_drvdata *drvdata, bool restart_ss)
>  {
>  	int i, rc;
>  	struct etmv4_config *config = &drvdata->config;
> @@ -562,9 +562,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>  		etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i));
>
>  	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> -		/* always clear status bit on restart if using single-shot */
> -		if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
> -			config->ss_status[i] &= ~TRCSSCSRn_STATUS;
> +		if (restart_ss) {
> +			/* always clear status bit on restart if using single-shot */
> +			if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
> +				config->ss_status[i] &= ~TRCSSCSRn_STATUS;
> +		}
>  		etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
>  		etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
>  		if (etm4x_sspcicrn_present(drvdata, i))
> @@ -628,7 +630,7 @@ static void etm4_enable_sysfs_smp_call(void *info)
>  		return;
>  	}
>
> -	arg->rc = etm4_enable_hw(arg->drvdata);
> +	arg->rc = etm4_enable_hw(arg->drvdata, true);
>
>  	/* The tracer didn't start */
>  	if (arg->rc)
> @@ -865,7 +867,7 @@ static int etm4_enable_perf(struct coresight_device *csdev,
>  	drvdata->paused = !!READ_ONCE(event->hw.aux_paused);
>
>  	/* And enable it */
> -	ret = etm4_enable_hw(drvdata);
> +	ret = etm4_enable_hw(drvdata, true);
>
>  out:
>  	/* Failed to start tracer; roll back to DISABLED mode */
> @@ -1831,7 +1833,7 @@ static int etm4_starting_cpu(unsigned int cpu)
>  		etm4_os_unlock(etmdrvdata[cpu]);
>
>  	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
> -		etm4_enable_hw(etmdrvdata[cpu]);
> +		etm4_enable_hw(etmdrvdata[cpu], true);
>  	raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
>  	return 0;
>  }
>
> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun

[PATCH v4 10/11] coresight: etm4x: Retain sequencer state

[Leo Yan]

Retain sequencer state in the disable flow and will be restored in the
enable flow.

As a result, the flow will be reused by CPU suspend and resume in
sequential change.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 4c6236ad241d1a13515160ca478fae839a891e38..71a4fcbca44091d5f441d4bc502b3c5f4fd3c984 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1026,6 +1026,10 @@ static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
 			etm4x_relaxed_read32(csa, TRCCNTVRn(i));
 	}
 
+	/* read back the sequencer state */
+	if (drvdata->nrseqstate)
+		config->seq_state = etm4x_relaxed_read32(csa, TRCSEQSTR);
+
 	coresight_disclaim_device_unlocked(csdev);
 	etm4_cs_lock(drvdata, csa);
 

-- 
2.34.1

[PATCH v4 11/11] coresight: etm4x: Reuse normal enable and disable logic in CPU idle

[Leo Yan]

The etm4_enable_trace_unit() function is almost identical to the restore
flow, with a few differences listed below:

1) TRCQCTLR register

   The TRCQCTLR register is saved and restored during CPU idle, but it
   is never touched in the normal flow. Given the Q element is not
   enabled (TRCCONFIGR.QE bits), it is acceptable to omit saving and
   restoring this register during idle.

2) TRCSSCSRn.STATUS bit

   The restore flow does not explicitly clear the TRCSSCSRn.STATUS bit
   but instead directly loads the saved value. In contrast, the normal
   flow clears this bit to 0 when re-enabling single-shot control.

   Therefore, the restore function passes the restart_ss argument as
   false to etm4_enable_hw() to avoid re-enabling single-shot mode.

3) Claim Tag Handling

   The claim tag is acquired and released in normal flow, on the other
   hand, the claim protocol is not applied in CPU idle flow - it simply
   restores the saved value.

   The claim bits serve two purpose:

   * Exclusive access between the kernel driver and an external
     debugger. During CPU idle, the kernel driver locks the OS Lock,
     ensuring that the external debugger cannot access the trace unit.
     Therefore, it is safe to release the claim tag during idle.

   * Notification to supervisory software to save/restore context for
     external debuggers. The kernel driver does not touch the external
     debugger's claim bit (ETMCLAIM[0]).

   Based on this, it is safe to acquire and release claim tag in the
   idle sequence.

4) OS Lock Behavior

   The OS Lock should be locked during CPU idle states. This differs
   from the normal flow, which unlock it. This special handling remains
   in the CPU save path.

This commit reuses the normal enable and disable logic in the CPU idle
path. The only addition is locking the OS Lock upon entering idle to
ensure exclusive access.

The save context in the driver data is no longer needed and has been
removed.

Tested-by: James Clark <james.clark@linaro.org>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 215 +--------------------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  57 ------
 2 files changed, 5 insertions(+), 267 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 71a4fcbca44091d5f441d4bc502b3c5f4fd3c984..6cfcede7e87846727bc040069399f7f9bb2e1abc 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1856,8 +1856,7 @@ static int etm4_dying_cpu(unsigned int cpu)
 
 static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 {
-	int i, ret = 0;
-	struct etmv4_save_state *state;
+	int ret = 0;
 	struct coresight_device *csdev = drvdata->csdev;
 	struct csdev_access *csa;
 	struct device *etm_dev;
@@ -1887,100 +1886,11 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 		ret = -EBUSY;
 		goto out;
 	}
+	etm4_cs_lock(drvdata, csa);
 
-	if (!drvdata->paused)
-		etm4_disable_trace_unit(drvdata);
-
-	/*
-	 * As recommended by section 4.3.7 (Synchronization of register updates)
-	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
-	 * ISB instruction after programming the trace unit registers.
-	 */
-	isb();
-
-	state = drvdata->save_state;
-
-	if (drvdata->nr_pe)
-		state->trcprocselr = etm4x_read32(csa, TRCPROCSELR);
-	state->trcconfigr = etm4x_read32(csa, TRCCONFIGR);
-	state->trcauxctlr = etm4x_read32(csa, TRCAUXCTLR);
-	state->trceventctl0r = etm4x_read32(csa, TRCEVENTCTL0R);
-	state->trceventctl1r = etm4x_read32(csa, TRCEVENTCTL1R);
-	if (drvdata->stallctl)
-		state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR);
-	state->trctsctlr = etm4x_read32(csa, TRCTSCTLR);
-	state->trcsyncpr = etm4x_read32(csa, TRCSYNCPR);
-	state->trcccctlr = etm4x_read32(csa, TRCCCCTLR);
-	state->trcbbctlr = etm4x_read32(csa, TRCBBCTLR);
-	state->trctraceidr = etm4x_read32(csa, TRCTRACEIDR);
-	if (drvdata->q_filt)
-		state->trcqctlr = etm4x_read32(csa, TRCQCTLR);
-
-	state->trcvictlr = etm4x_read32(csa, TRCVICTLR);
-	state->trcviiectlr = etm4x_read32(csa, TRCVIIECTLR);
-	state->trcvissctlr = etm4x_read32(csa, TRCVISSCTLR);
-	if (drvdata->nr_pe_cmp)
-		state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR);
-
-	for (i = 0; i < drvdata->nrseqstate - 1; i++)
-		state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i));
-
-	if (drvdata->nrseqstate) {
-		state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR);
-		state->trcseqstr = etm4x_read32(csa, TRCSEQSTR);
-	}
-
-	if (drvdata->numextinsel)
-		state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR);
-
-	for (i = 0; i < drvdata->nr_cntr; i++) {
-		state->trccntrldvr[i] = etm4x_read32(csa, TRCCNTRLDVRn(i));
-		state->trccntctlr[i] = etm4x_read32(csa, TRCCNTCTLRn(i));
-		state->trccntvr[i] = etm4x_read32(csa, TRCCNTVRn(i));
-	}
-
-	/* Resource selector pair 0 is reserved */
-	for (i = 2; i < drvdata->nr_resource * 2; i++)
-		state->trcrsctlr[i] = etm4x_read32(csa, TRCRSCTLRn(i));
-
-	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
-		state->trcssccr[i] = etm4x_read32(csa, TRCSSCCRn(i));
-		state->trcsscsr[i] = etm4x_read32(csa, TRCSSCSRn(i));
-		if (etm4x_sspcicrn_present(drvdata, i))
-			state->trcsspcicr[i] = etm4x_read32(csa, TRCSSPCICRn(i));
-	}
-
-	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
-		state->trcacvr[i] = etm4x_read64(csa, TRCACVRn(i));
-		state->trcacatr[i] = etm4x_read64(csa, TRCACATRn(i));
-	}
-
-	/*
-	 * Data trace stream is architecturally prohibited for A profile cores
-	 * so we don't save (or later restore) trcdvcvr and trcdvcmr - As per
-	 * section 1.3.4 ("Possible functional configurations of an ETMv4 trace
-	 * unit") of ARM IHI 0064D.
-	 */
-
-	for (i = 0; i < drvdata->numcidc; i++)
-		state->trccidcvr[i] = etm4x_read64(csa, TRCCIDCVRn(i));
-
-	for (i = 0; i < drvdata->numvmidc; i++)
-		state->trcvmidcvr[i] = etm4x_read64(csa, TRCVMIDCVRn(i));
-
-	state->trccidcctlr0 = etm4x_read32(csa, TRCCIDCCTLR0);
-	if (drvdata->numcidc > 4)
-		state->trccidcctlr1 = etm4x_read32(csa, TRCCIDCCTLR1);
-
-	state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR0);
-	if (drvdata->numvmidc > 4)
-		state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR1);
-
-	state->trcclaimset = etm4x_read32(csa, TRCCLAIMCLR);
-
-	if (!drvdata->skip_power_up)
-		state->trcpdcr = etm4x_read32(csa, TRCPDCR);
+	etm4_disable_hw(drvdata);
 
+	etm4_cs_unlock(drvdata, csa);
 	/* wait for TRCSTATR.IDLE to go up */
 	if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1)) {
 		dev_err(etm_dev,
@@ -1990,14 +1900,6 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 		goto out;
 	}
 
-	/*
-	 * Power can be removed from the trace unit now. We do this to
-	 * potentially save power on systems that respect the TRCPDCR_PU
-	 * despite requesting software to save/restore state.
-	 */
-	if (!drvdata->skip_power_up)
-		etm4x_relaxed_write32(csa, (state->trcpdcr & ~TRCPDCR_PU),
-				      TRCPDCR);
 out:
 	etm4_cs_lock(drvdata, csa);
 	return ret;
@@ -2021,110 +1923,10 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
 
 static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 {
-	int i;
-	struct etmv4_save_state *state = drvdata->save_state;
-	struct csdev_access *csa = &drvdata->csdev->access;
-
 	if (WARN_ON(!drvdata->csdev))
 		return;
 
-	etm4_cs_unlock(drvdata, csa);
-	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
-
-	if (drvdata->nr_pe)
-		etm4x_relaxed_write32(csa, state->trcprocselr, TRCPROCSELR);
-	etm4x_relaxed_write32(csa, state->trcconfigr, TRCCONFIGR);
-	etm4x_relaxed_write32(csa, state->trcauxctlr, TRCAUXCTLR);
-	etm4x_relaxed_write32(csa, state->trceventctl0r, TRCEVENTCTL0R);
-	etm4x_relaxed_write32(csa, state->trceventctl1r, TRCEVENTCTL1R);
-	if (drvdata->stallctl)
-		etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR);
-	etm4x_relaxed_write32(csa, state->trctsctlr, TRCTSCTLR);
-	etm4x_relaxed_write32(csa, state->trcsyncpr, TRCSYNCPR);
-	etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR);
-	etm4x_relaxed_write32(csa, state->trcbbctlr, TRCBBCTLR);
-	etm4x_relaxed_write32(csa, state->trctraceidr, TRCTRACEIDR);
-	if (drvdata->q_filt)
-		etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR);
-
-	etm4x_relaxed_write32(csa, state->trcvictlr, TRCVICTLR);
-	etm4x_relaxed_write32(csa, state->trcviiectlr, TRCVIIECTLR);
-	etm4x_relaxed_write32(csa, state->trcvissctlr, TRCVISSCTLR);
-	if (drvdata->nr_pe_cmp)
-		etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR);
-
-	for (i = 0; i < drvdata->nrseqstate - 1; i++)
-		etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i));
-
-	if (drvdata->nrseqstate) {
-		etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR);
-		etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR);
-	}
-	if (drvdata->numextinsel)
-		etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR);
-
-	for (i = 0; i < drvdata->nr_cntr; i++) {
-		etm4x_relaxed_write32(csa, state->trccntrldvr[i], TRCCNTRLDVRn(i));
-		etm4x_relaxed_write32(csa, state->trccntctlr[i], TRCCNTCTLRn(i));
-		etm4x_relaxed_write32(csa, state->trccntvr[i], TRCCNTVRn(i));
-	}
-
-	/* Resource selector pair 0 is reserved */
-	for (i = 2; i < drvdata->nr_resource * 2; i++)
-		etm4x_relaxed_write32(csa, state->trcrsctlr[i], TRCRSCTLRn(i));
-
-	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
-		etm4x_relaxed_write32(csa, state->trcssccr[i], TRCSSCCRn(i));
-		etm4x_relaxed_write32(csa, state->trcsscsr[i], TRCSSCSRn(i));
-		if (etm4x_sspcicrn_present(drvdata, i))
-			etm4x_relaxed_write32(csa, state->trcsspcicr[i], TRCSSPCICRn(i));
-	}
-
-	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
-		etm4x_relaxed_write64(csa, state->trcacvr[i], TRCACVRn(i));
-		etm4x_relaxed_write64(csa, state->trcacatr[i], TRCACATRn(i));
-	}
-
-	for (i = 0; i < drvdata->numcidc; i++)
-		etm4x_relaxed_write64(csa, state->trccidcvr[i], TRCCIDCVRn(i));
-
-	for (i = 0; i < drvdata->numvmidc; i++)
-		etm4x_relaxed_write64(csa, state->trcvmidcvr[i], TRCVMIDCVRn(i));
-
-	etm4x_relaxed_write32(csa, state->trccidcctlr0, TRCCIDCCTLR0);
-	if (drvdata->numcidc > 4)
-		etm4x_relaxed_write32(csa, state->trccidcctlr1, TRCCIDCCTLR1);
-
-	etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR0);
-	if (drvdata->numvmidc > 4)
-		etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1);
-
-	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
-
-	if (!drvdata->skip_power_up)
-		etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);
-
-	/*
-	 * As recommended by section 4.3.7 ("Synchronization when using the
-	 * memory-mapped interface") of ARM IHI 0064D
-	 */
-	dsb(sy);
-	isb();
-
-	/* Unlock the OS lock to re-enable trace and external debug access */
-	etm4_os_unlock(drvdata);
-
-	if (!drvdata->paused)
-		etm4_enable_trace_unit(drvdata);
-
-	/*
-	 * As recommended by section 4.3.7 (Synchronization of register updates)
-	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
-	 * ISB instruction after programming the trace unit registers.
-	 */
-	isb();
-
-	etm4_cs_lock(drvdata, csa);
+	etm4_enable_hw(drvdata, false);
 }
 
 static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
@@ -2311,13 +2113,6 @@ static int etm4_probe(struct device *dev)
 		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
 			       PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
 
-	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
-		drvdata->save_state = devm_kmalloc(dev,
-				sizeof(struct etmv4_save_state), GFP_KERNEL);
-		if (!drvdata->save_state)
-			return -ENOMEM;
-	}
-
 	raw_spin_lock_init(&drvdata->spinlock);
 
 	drvdata->cpu = coresight_get_cpu(dev);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 012c52fd19338133129752d35523dc102df24604..9dfdcfc2b16748bedc278ba2ceaa7e39b29cc351 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -862,61 +862,6 @@ struct etmv4_config {
 	u8				s_ex_level;
 };
 
-/**
- * struct etm4_save_state - state to be preserved when ETM is without power
- */
-struct etmv4_save_state {
-	u32	trcprocselr;
-	u32	trcconfigr;
-	u32	trcauxctlr;
-	u32	trceventctl0r;
-	u32	trceventctl1r;
-	u32	trcstallctlr;
-	u32	trctsctlr;
-	u32	trcsyncpr;
-	u32	trcccctlr;
-	u32	trcbbctlr;
-	u32	trctraceidr;
-	u32	trcqctlr;
-
-	u32	trcvictlr;
-	u32	trcviiectlr;
-	u32	trcvissctlr;
-	u32	trcvipcssctlr;
-
-	u32	trcseqevr[ETM_MAX_SEQ_STATES];
-	u32	trcseqrstevr;
-	u32	trcseqstr;
-	u32	trcextinselr;
-	u32	trccntrldvr[ETMv4_MAX_CNTR];
-	u32	trccntctlr[ETMv4_MAX_CNTR];
-	u32	trccntvr[ETMv4_MAX_CNTR];
-
-	u32	trcrsctlr[ETM_MAX_RES_SEL];
-
-	u32	trcssccr[ETM_MAX_SS_CMP];
-	u32	trcsscsr[ETM_MAX_SS_CMP];
-	u32	trcsspcicr[ETM_MAX_SS_CMP];
-
-	u64	trcacvr[ETM_MAX_SINGLE_ADDR_CMP];
-	u64	trcacatr[ETM_MAX_SINGLE_ADDR_CMP];
-	u64	trccidcvr[ETMv4_MAX_CTXID_CMP];
-	u64	trcvmidcvr[ETM_MAX_VMID_CMP];
-	u32	trccidcctlr0;
-	u32	trccidcctlr1;
-	u32	trcvmidcctlr0;
-	u32	trcvmidcctlr1;
-
-	u32	trcclaimset;
-
-	u32	cntr_val[ETMv4_MAX_CNTR];
-	u32	seq_state;
-	u32	vinst_ctrl;
-	u32	ss_status[ETM_MAX_SS_CMP];
-
-	u32	trcpdcr;
-};
-
 /**
  * struct etm4_drvdata - specifics associated to an ETM component
  * @pclk:       APB clock if present, otherwise NULL
@@ -979,7 +924,6 @@ struct etmv4_save_state {
  *		at runtime, due to the additional setting of TRFCR_CX when
  *		in EL2. Otherwise, 0.
  * @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
  *		   the trace unit.
  * @paused:	Indicates if the trace unit is paused.
@@ -1034,7 +978,6 @@ struct etmv4_drvdata {
 	bool				lpoverride;
 	u64				trfcr;
 	struct etmv4_config		config;
-	struct etmv4_save_state		*save_state;
 	bool				skip_power_up;
 	bool				paused;
 	DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);

-- 
2.34.1

Re: [PATCH v4 11/11] coresight: etm4x: Reuse normal enable and disable logic in CPU idle

[Yeoreum Yun]

Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>

> The etm4_enable_trace_unit() function is almost identical to the restore
> flow, with a few differences listed below:
>
> 1) TRCQCTLR register
>
>    The TRCQCTLR register is saved and restored during CPU idle, but it
>    is never touched in the normal flow. Given the Q element is not
>    enabled (TRCCONFIGR.QE bits), it is acceptable to omit saving and
>    restoring this register during idle.
>
> 2) TRCSSCSRn.STATUS bit
>
>    The restore flow does not explicitly clear the TRCSSCSRn.STATUS bit
>    but instead directly loads the saved value. In contrast, the normal
>    flow clears this bit to 0 when re-enabling single-shot control.
>
>    Therefore, the restore function passes the restart_ss argument as
>    false to etm4_enable_hw() to avoid re-enabling single-shot mode.
>
> 3) Claim Tag Handling
>
>    The claim tag is acquired and released in normal flow, on the other
>    hand, the claim protocol is not applied in CPU idle flow - it simply
>    restores the saved value.
>
>    The claim bits serve two purpose:
>
>    * Exclusive access between the kernel driver and an external
>      debugger. During CPU idle, the kernel driver locks the OS Lock,
>      ensuring that the external debugger cannot access the trace unit.
>      Therefore, it is safe to release the claim tag during idle.
>
>    * Notification to supervisory software to save/restore context for
>      external debuggers. The kernel driver does not touch the external
>      debugger's claim bit (ETMCLAIM[0]).
>
>    Based on this, it is safe to acquire and release claim tag in the
>    idle sequence.
>
> 4) OS Lock Behavior
>
>    The OS Lock should be locked during CPU idle states. This differs
>    from the normal flow, which unlock it. This special handling remains
>    in the CPU save path.
>
> This commit reuses the normal enable and disable logic in the CPU idle
> path. The only addition is locking the OS Lock upon entering idle to
> ensure exclusive access.
>
> The save context in the driver data is no longer needed and has been
> removed.
>
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 215 +--------------------
>  drivers/hwtracing/coresight/coresight-etm4x.h      |  57 ------
>  2 files changed, 5 insertions(+), 267 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 71a4fcbca44091d5f441d4bc502b3c5f4fd3c984..6cfcede7e87846727bc040069399f7f9bb2e1abc 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1856,8 +1856,7 @@ static int etm4_dying_cpu(unsigned int cpu)
>
>  static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>  {
> -	int i, ret = 0;
> -	struct etmv4_save_state *state;
> +	int ret = 0;
>  	struct coresight_device *csdev = drvdata->csdev;
>  	struct csdev_access *csa;
>  	struct device *etm_dev;
> @@ -1887,100 +1886,11 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>  		ret = -EBUSY;
>  		goto out;
>  	}
> +	etm4_cs_lock(drvdata, csa);
>
> -	if (!drvdata->paused)
> -		etm4_disable_trace_unit(drvdata);
> -
> -	/*
> -	 * As recommended by section 4.3.7 (Synchronization of register updates)
> -	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> -	 * ISB instruction after programming the trace unit registers.
> -	 */
> -	isb();
> -
> -	state = drvdata->save_state;
> -
> -	if (drvdata->nr_pe)
> -		state->trcprocselr = etm4x_read32(csa, TRCPROCSELR);
> -	state->trcconfigr = etm4x_read32(csa, TRCCONFIGR);
> -	state->trcauxctlr = etm4x_read32(csa, TRCAUXCTLR);
> -	state->trceventctl0r = etm4x_read32(csa, TRCEVENTCTL0R);
> -	state->trceventctl1r = etm4x_read32(csa, TRCEVENTCTL1R);
> -	if (drvdata->stallctl)
> -		state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR);
> -	state->trctsctlr = etm4x_read32(csa, TRCTSCTLR);
> -	state->trcsyncpr = etm4x_read32(csa, TRCSYNCPR);
> -	state->trcccctlr = etm4x_read32(csa, TRCCCCTLR);
> -	state->trcbbctlr = etm4x_read32(csa, TRCBBCTLR);
> -	state->trctraceidr = etm4x_read32(csa, TRCTRACEIDR);
> -	if (drvdata->q_filt)
> -		state->trcqctlr = etm4x_read32(csa, TRCQCTLR);
> -
> -	state->trcvictlr = etm4x_read32(csa, TRCVICTLR);
> -	state->trcviiectlr = etm4x_read32(csa, TRCVIIECTLR);
> -	state->trcvissctlr = etm4x_read32(csa, TRCVISSCTLR);
> -	if (drvdata->nr_pe_cmp)
> -		state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR);
> -
> -	for (i = 0; i < drvdata->nrseqstate - 1; i++)
> -		state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i));
> -
> -	if (drvdata->nrseqstate) {
> -		state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR);
> -		state->trcseqstr = etm4x_read32(csa, TRCSEQSTR);
> -	}
> -
> -	if (drvdata->numextinsel)
> -		state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR);
> -
> -	for (i = 0; i < drvdata->nr_cntr; i++) {
> -		state->trccntrldvr[i] = etm4x_read32(csa, TRCCNTRLDVRn(i));
> -		state->trccntctlr[i] = etm4x_read32(csa, TRCCNTCTLRn(i));
> -		state->trccntvr[i] = etm4x_read32(csa, TRCCNTVRn(i));
> -	}
> -
> -	/* Resource selector pair 0 is reserved */
> -	for (i = 2; i < drvdata->nr_resource * 2; i++)
> -		state->trcrsctlr[i] = etm4x_read32(csa, TRCRSCTLRn(i));
> -
> -	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> -		state->trcssccr[i] = etm4x_read32(csa, TRCSSCCRn(i));
> -		state->trcsscsr[i] = etm4x_read32(csa, TRCSSCSRn(i));
> -		if (etm4x_sspcicrn_present(drvdata, i))
> -			state->trcsspcicr[i] = etm4x_read32(csa, TRCSSPCICRn(i));
> -	}
> -
> -	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
> -		state->trcacvr[i] = etm4x_read64(csa, TRCACVRn(i));
> -		state->trcacatr[i] = etm4x_read64(csa, TRCACATRn(i));
> -	}
> -
> -	/*
> -	 * Data trace stream is architecturally prohibited for A profile cores
> -	 * so we don't save (or later restore) trcdvcvr and trcdvcmr - As per
> -	 * section 1.3.4 ("Possible functional configurations of an ETMv4 trace
> -	 * unit") of ARM IHI 0064D.
> -	 */
> -
> -	for (i = 0; i < drvdata->numcidc; i++)
> -		state->trccidcvr[i] = etm4x_read64(csa, TRCCIDCVRn(i));
> -
> -	for (i = 0; i < drvdata->numvmidc; i++)
> -		state->trcvmidcvr[i] = etm4x_read64(csa, TRCVMIDCVRn(i));
> -
> -	state->trccidcctlr0 = etm4x_read32(csa, TRCCIDCCTLR0);
> -	if (drvdata->numcidc > 4)
> -		state->trccidcctlr1 = etm4x_read32(csa, TRCCIDCCTLR1);
> -
> -	state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR0);
> -	if (drvdata->numvmidc > 4)
> -		state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR1);
> -
> -	state->trcclaimset = etm4x_read32(csa, TRCCLAIMCLR);
> -
> -	if (!drvdata->skip_power_up)
> -		state->trcpdcr = etm4x_read32(csa, TRCPDCR);
> +	etm4_disable_hw(drvdata);
>
> +	etm4_cs_unlock(drvdata, csa);
>  	/* wait for TRCSTATR.IDLE to go up */
>  	if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1)) {
>  		dev_err(etm_dev,
> @@ -1990,14 +1900,6 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>  		goto out;
>  	}
>
> -	/*
> -	 * Power can be removed from the trace unit now. We do this to
> -	 * potentially save power on systems that respect the TRCPDCR_PU
> -	 * despite requesting software to save/restore state.
> -	 */
> -	if (!drvdata->skip_power_up)
> -		etm4x_relaxed_write32(csa, (state->trcpdcr & ~TRCPDCR_PU),
> -				      TRCPDCR);
>  out:
>  	etm4_cs_lock(drvdata, csa);
>  	return ret;
> @@ -2021,110 +1923,10 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
>
>  static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>  {
> -	int i;
> -	struct etmv4_save_state *state = drvdata->save_state;
> -	struct csdev_access *csa = &drvdata->csdev->access;
> -
>  	if (WARN_ON(!drvdata->csdev))
>  		return;
>
> -	etm4_cs_unlock(drvdata, csa);
> -	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
> -
> -	if (drvdata->nr_pe)
> -		etm4x_relaxed_write32(csa, state->trcprocselr, TRCPROCSELR);
> -	etm4x_relaxed_write32(csa, state->trcconfigr, TRCCONFIGR);
> -	etm4x_relaxed_write32(csa, state->trcauxctlr, TRCAUXCTLR);
> -	etm4x_relaxed_write32(csa, state->trceventctl0r, TRCEVENTCTL0R);
> -	etm4x_relaxed_write32(csa, state->trceventctl1r, TRCEVENTCTL1R);
> -	if (drvdata->stallctl)
> -		etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR);
> -	etm4x_relaxed_write32(csa, state->trctsctlr, TRCTSCTLR);
> -	etm4x_relaxed_write32(csa, state->trcsyncpr, TRCSYNCPR);
> -	etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR);
> -	etm4x_relaxed_write32(csa, state->trcbbctlr, TRCBBCTLR);
> -	etm4x_relaxed_write32(csa, state->trctraceidr, TRCTRACEIDR);
> -	if (drvdata->q_filt)
> -		etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR);
> -
> -	etm4x_relaxed_write32(csa, state->trcvictlr, TRCVICTLR);
> -	etm4x_relaxed_write32(csa, state->trcviiectlr, TRCVIIECTLR);
> -	etm4x_relaxed_write32(csa, state->trcvissctlr, TRCVISSCTLR);
> -	if (drvdata->nr_pe_cmp)
> -		etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR);
> -
> -	for (i = 0; i < drvdata->nrseqstate - 1; i++)
> -		etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i));
> -
> -	if (drvdata->nrseqstate) {
> -		etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR);
> -		etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR);
> -	}
> -	if (drvdata->numextinsel)
> -		etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR);
> -
> -	for (i = 0; i < drvdata->nr_cntr; i++) {
> -		etm4x_relaxed_write32(csa, state->trccntrldvr[i], TRCCNTRLDVRn(i));
> -		etm4x_relaxed_write32(csa, state->trccntctlr[i], TRCCNTCTLRn(i));
> -		etm4x_relaxed_write32(csa, state->trccntvr[i], TRCCNTVRn(i));
> -	}
> -
> -	/* Resource selector pair 0 is reserved */
> -	for (i = 2; i < drvdata->nr_resource * 2; i++)
> -		etm4x_relaxed_write32(csa, state->trcrsctlr[i], TRCRSCTLRn(i));
> -
> -	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> -		etm4x_relaxed_write32(csa, state->trcssccr[i], TRCSSCCRn(i));
> -		etm4x_relaxed_write32(csa, state->trcsscsr[i], TRCSSCSRn(i));
> -		if (etm4x_sspcicrn_present(drvdata, i))
> -			etm4x_relaxed_write32(csa, state->trcsspcicr[i], TRCSSPCICRn(i));
> -	}
> -
> -	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
> -		etm4x_relaxed_write64(csa, state->trcacvr[i], TRCACVRn(i));
> -		etm4x_relaxed_write64(csa, state->trcacatr[i], TRCACATRn(i));
> -	}
> -
> -	for (i = 0; i < drvdata->numcidc; i++)
> -		etm4x_relaxed_write64(csa, state->trccidcvr[i], TRCCIDCVRn(i));
> -
> -	for (i = 0; i < drvdata->numvmidc; i++)
> -		etm4x_relaxed_write64(csa, state->trcvmidcvr[i], TRCVMIDCVRn(i));
> -
> -	etm4x_relaxed_write32(csa, state->trccidcctlr0, TRCCIDCCTLR0);
> -	if (drvdata->numcidc > 4)
> -		etm4x_relaxed_write32(csa, state->trccidcctlr1, TRCCIDCCTLR1);
> -
> -	etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR0);
> -	if (drvdata->numvmidc > 4)
> -		etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1);
> -
> -	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
> -
> -	if (!drvdata->skip_power_up)
> -		etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);
> -
> -	/*
> -	 * As recommended by section 4.3.7 ("Synchronization when using the
> -	 * memory-mapped interface") of ARM IHI 0064D
> -	 */
> -	dsb(sy);
> -	isb();
> -
> -	/* Unlock the OS lock to re-enable trace and external debug access */
> -	etm4_os_unlock(drvdata);
> -
> -	if (!drvdata->paused)
> -		etm4_enable_trace_unit(drvdata);
> -
> -	/*
> -	 * As recommended by section 4.3.7 (Synchronization of register updates)
> -	 * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an
> -	 * ISB instruction after programming the trace unit registers.
> -	 */
> -	isb();
> -
> -	etm4_cs_lock(drvdata, csa);
> +	etm4_enable_hw(drvdata, false);
>  }
>
>  static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
> @@ -2311,13 +2113,6 @@ static int etm4_probe(struct device *dev)
>  		pm_save_enable = coresight_loses_context_with_cpu(dev) ?
>  			       PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;
>
> -	if (pm_save_enable != PARAM_PM_SAVE_NEVER) {
> -		drvdata->save_state = devm_kmalloc(dev,
> -				sizeof(struct etmv4_save_state), GFP_KERNEL);
> -		if (!drvdata->save_state)
> -			return -ENOMEM;
> -	}
> -
>  	raw_spin_lock_init(&drvdata->spinlock);
>
>  	drvdata->cpu = coresight_get_cpu(dev);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 012c52fd19338133129752d35523dc102df24604..9dfdcfc2b16748bedc278ba2ceaa7e39b29cc351 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -862,61 +862,6 @@ struct etmv4_config {
>  	u8				s_ex_level;
>  };
>
> -/**
> - * struct etm4_save_state - state to be preserved when ETM is without power
> - */
> -struct etmv4_save_state {
> -	u32	trcprocselr;
> -	u32	trcconfigr;
> -	u32	trcauxctlr;
> -	u32	trceventctl0r;
> -	u32	trceventctl1r;
> -	u32	trcstallctlr;
> -	u32	trctsctlr;
> -	u32	trcsyncpr;
> -	u32	trcccctlr;
> -	u32	trcbbctlr;
> -	u32	trctraceidr;
> -	u32	trcqctlr;
> -
> -	u32	trcvictlr;
> -	u32	trcviiectlr;
> -	u32	trcvissctlr;
> -	u32	trcvipcssctlr;
> -
> -	u32	trcseqevr[ETM_MAX_SEQ_STATES];
> -	u32	trcseqrstevr;
> -	u32	trcseqstr;
> -	u32	trcextinselr;
> -	u32	trccntrldvr[ETMv4_MAX_CNTR];
> -	u32	trccntctlr[ETMv4_MAX_CNTR];
> -	u32	trccntvr[ETMv4_MAX_CNTR];
> -
> -	u32	trcrsctlr[ETM_MAX_RES_SEL];
> -
> -	u32	trcssccr[ETM_MAX_SS_CMP];
> -	u32	trcsscsr[ETM_MAX_SS_CMP];
> -	u32	trcsspcicr[ETM_MAX_SS_CMP];
> -
> -	u64	trcacvr[ETM_MAX_SINGLE_ADDR_CMP];
> -	u64	trcacatr[ETM_MAX_SINGLE_ADDR_CMP];
> -	u64	trccidcvr[ETMv4_MAX_CTXID_CMP];
> -	u64	trcvmidcvr[ETM_MAX_VMID_CMP];
> -	u32	trccidcctlr0;
> -	u32	trccidcctlr1;
> -	u32	trcvmidcctlr0;
> -	u32	trcvmidcctlr1;
> -
> -	u32	trcclaimset;
> -
> -	u32	cntr_val[ETMv4_MAX_CNTR];
> -	u32	seq_state;
> -	u32	vinst_ctrl;
> -	u32	ss_status[ETM_MAX_SS_CMP];
> -
> -	u32	trcpdcr;
> -};
> -
>  /**
>   * struct etm4_drvdata - specifics associated to an ETM component
>   * @pclk:       APB clock if present, otherwise NULL
> @@ -979,7 +924,6 @@ struct etmv4_save_state {
>   *		at runtime, due to the additional setting of TRFCR_CX when
>   *		in EL2. Otherwise, 0.
>   * @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
>   *		   the trace unit.
>   * @paused:	Indicates if the trace unit is paused.
> @@ -1034,7 +978,6 @@ struct etmv4_drvdata {
>  	bool				lpoverride;
>  	u64				trfcr;
>  	struct etmv4_config		config;
> -	struct etmv4_save_state		*save_state;
>  	bool				skip_power_up;
>  	bool				paused;
>  	DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);
>
> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun