[PATCH v2 00/28] CoreSight: Address CPU Power Management Issues

[PATCH v2 00/28] CoreSight: Address CPU Power Management Issues

[Leo Yan]

This series addresses CPU power management issues in the CoreSight
drivers. For easier review, the patches are organized into two
categories:

o Patches 01 ~ 10 focus on CPU power management within the ETM drivers.
  These patches fix SMP-safe access to the mode, correct context
  synchronization, and refactor the CPU suspend/resume flows.

o Patches 11 ~ 28 extend CPU power management to cover activated paths,
  including helpers, links, and sinks. These changes move CPU PM and
  hotplug notifiers from the ETMv4 driver into the CoreSight core layer.

Summary of Changes:

- Patches 01 ~ 03: Fix device mode access in the SMP mode.
- Patch 04       : A minor fix for polling bit.
- Patches 05 ~ 07: Improve the context synchronization based on the ETM
		   specification (IHI0064H.b) and Arm ARM (ARM DDI 0487
		   L.a).
- Patches 08 ~ 10: Refactor the context save/restore flow in the ETMv4
		   driver, in the end, the CPU PM callbacks reuse the
		   normal enabling and disabling flows.
- Patches 11 ~ 17: Move CPU PM code from ETMv4 driver to the core layer.
- Patches 18 ~ 25: Enhance device mode handling in the CTI driver,
		   distinguishes debug mode from trace modes (Perf or
		   SysFS mode). Extend support activated paths during
		   CPU idle. Support save and restore context for
		   per-CPU sink (TRBE).
- Patches 26 ~ 28: Move CPU hotplug notifier from the ETMv4 driver to
		   the core layer. The full path is now managed in CPU
		   hotplug.

Verification:

This series has been verified on the following platforms:
- Hikey960, Juno-r0 and Juno-r2:
  Note that the firmware on Hikey960 and Juno-r0 is unstable. While no
  CoreSight failures were observed, but the CPU hotplug or CPU idle
  itself may fail intermittently.
  Juno-r2 is much stable and can pass 24+ hours stress test.
- FVP RevC: Verified TRBE changes.

Stress test script:

  #!/usr/bin/bash

  echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink

  while true; do
          echo 0 > /sys/devices/system/cpu/cpu2/online;
          echo 1 > /sys/devices/system/cpu/cpu2/online;
  done &

  while true; do
          echo 1 > /sys/bus/coresight/devices/etm2/enable_source;
          echo 0 > /sys/bus/coresight/devices/etm2/enable_source;
  done &

---
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 (27):
      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: Reuse normal enable and disable logic in CPU idle
      coresight: Populate CPU ID into the coresight_device structure
      coresight: sysfs: Validate CPU online status for per-CPU sources
      coresight: Set per CPU source pointer
      coresight: Register CPU PM notifier in core layer
      coresight: etm4x: Hook CPU PM callbacks
      coresight: Add callback to determine if context save/restore is needed
      coresight: etm4x: Remove redundant condition checks in save and restore
      coresight: cti: Fix race condition by using device mode
      coresight: cti: Introduce CS_MODE_DEBUG mode
      coresight: cti: Properly handle modes in CPU PM notifiers
      coresight: Add per-CPU path pointer
      coresight: Add 'in_idle' argument to path enable/disable functions
      coresight: Control path during CPU idle
      coresight: Add PM callbacks for percpu sink
      coresight: Take hotplug lock in enable_source_store() for Sysfs mode
      coresight: Move CPU hotplug callbacks to core layer
      coresight: Manage activated path during CPU hotplug

Yabin Cui (1):
      coresight: trbe: Save and restore state across CPU low power state

 drivers/hwtracing/coresight/coresight-catu.c       |   1 +
 drivers/hwtracing/coresight/coresight-core.c       | 337 ++++++++++++--
 drivers/hwtracing/coresight/coresight-ctcu-core.c  |   1 +
 drivers/hwtracing/coresight/coresight-cti-core.c   |  40 +-
 drivers/hwtracing/coresight/coresight-cti-sysfs.c  |   2 +-
 drivers/hwtracing/coresight/coresight-dummy.c      |   1 +
 drivers/hwtracing/coresight/coresight-etb10.c      |   1 +
 drivers/hwtracing/coresight/coresight-etm3x-core.c |  61 ++-
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 488 ++++++---------------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  62 ---
 drivers/hwtracing/coresight/coresight-funnel.c     |   1 +
 drivers/hwtracing/coresight/coresight-replicator.c |   1 +
 drivers/hwtracing/coresight/coresight-stm.c        |   1 +
 drivers/hwtracing/coresight/coresight-sysfs.c      |  10 +
 drivers/hwtracing/coresight/coresight-tmc-core.c   |   1 +
 drivers/hwtracing/coresight/coresight-tpda.c       |   1 +
 drivers/hwtracing/coresight/coresight-tpdm.c       |   1 +
 drivers/hwtracing/coresight/coresight-tpiu.c       |   1 +
 drivers/hwtracing/coresight/coresight-trbe.c       |  85 ++++
 drivers/hwtracing/coresight/ultrasoc-smb.c         |   1 +
 include/linux/coresight.h                          |  55 ++-
 21 files changed, 665 insertions(+), 487 deletions(-)
---
base-commit: 66701750d5565c574af42bef0b789ce0203e3071
change-id: 20250611-arm_cs_pm_fix_v3-f4ae29bb7d81

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

[PATCH v2 01/28] 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()")
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 4ac65c68bbf44b98db22c3dad2d83a224ce5278e..5fd3d08824e5a91a197aa01daf0fc392392f3e55 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 */
@@ -650,13 +646,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,
@@ -672,7 +669,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

Re: [PATCH v2 01/28] coresight: Change device mode to atomic type

[Yeoreum Yun]

Hi Leo,

>  {
> -	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);
>  }

Just question. why is acquire symentic enough in here?
before this change, local_cmpxchg seems to use full_fenced.

--
Sincerely,
Yeoreum Yun

Re: [PATCH v2 01/28] coresight: Change device mode to atomic type

[Leo Yan]

Hi Levi,

On Wed, Jul 02, 2025 at 10:49:01AM +0100, Yeoreum Yun wrote:
> Hi Leo,
> 
> >  {
> > -	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);
> >  }
> 
> Just question. why is acquire symentic enough in here?

My understanding is that acquire semantics ensure ordering between
cmpxchg_acquire() and all memory accesses that follow it. However, it
does not guarantee that memory accesses appearing before the acquire
are ordered as well.

This is exactly what we want in the driver. We must ensure to first grab
an active device mode, then it is safe to proceed later operations (e.g.
set configurations in driver data and access registers).

> before this change, local_cmpxchg seems to use full_fenced.

Not really. Arm64 has atomic instruction for cmpxchg, it does not use
full_fenced. It should run into the path of arch_cmpxchg().

Thanks,
Leo

Re: [PATCH v2 01/28] coresight: Change device mode to atomic type

[Yeoreum Yun]

Hi,
>
> On Wed, Jul 02, 2025 at 10:49:01AM +0100, Yeoreum Yun wrote:
> > Hi Leo,
> >
> > >  {
> > > -	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);
> > >  }
> >
> > Just question. why is acquire symentic enough in here?
>
> My understanding is that acquire semantics ensure ordering between
> cmpxchg_acquire() and all memory accesses that follow it. However, it
> does not guarantee that memory accesses appearing before the acquire
> are ordered as well.
>
> This is exactly what we want in the driver. We must ensure to first grab
> an active device mode, then it is safe to proceed later operations (e.g.
> set configurations in driver data and access registers).
>
> > before this change, local_cmpxchg seems to use full_fenced.
>
> Not really. Arm64 has atomic instruction for cmpxchg, it does not use
> full_fenced. It should run into the path of arch_cmpxchg().

No, It local_cmpxchg backend with atomic_cmpxchg() and it seems include
full fenced.

But, you've explained the what i concerned with below diagram clear

CPU0                                          CPU1
// store csmode in store buffer
atomic_cmpxchg_acquired(csmode)
                                             // couldn't see the contents in store buffer
                                             atomic_read_acquire(csmode);

                                             Send SMP call
              ,-------------------------------/
             v
atomic_cmpxchg_acquired(csmode)  ==> Fail to grab another mode

However, I think it would be good to change the atomic_try_cmpxchg()
instead of atomic_cmpxchg_acquired() to prevent redundnat IPI trigger for
observation failure.

Thanks.

--
Sincerely,
Yeoreum Yun

Re: [PATCH v2 01/28] coresight: Change device mode to atomic type

[Anshuman Khandual]

On 01/07/25 8:23 PM, Leo Yan wrote:
> 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.


But have we really faced such problems on real systems due to local_t
or this is just an improvement ?

> 
> Fixes: 22fd532eaa0c ("coresight: etm3x: adding operation mode for etm_enable()")
> 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 4ac65c68bbf44b98db22c3dad2d83a224ce5278e..5fd3d08824e5a91a197aa01daf0fc392392f3e55 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 */
> @@ -650,13 +646,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,
> @@ -672,7 +669,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);
> 

Re: [PATCH v2 01/28] coresight: Change device mode to atomic type

[Leo Yan]

On Tue, Jul 15, 2025 at 12:23:22PM +0530, Anshuman Khandual wrote:
> On 01/07/25 8:23 PM, Leo Yan wrote:
> > 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.
> 
> 
> But have we really faced such problems on real systems due to local_t
> or this is just an improvement ?

This is an improvement based on reading code. The reason is the
variable is accessed by mutliple CPUs but not per-CPU access.

Thanks,
Leo

[PATCH v2 02/28] 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_hw_smp_call() function
for SMP calls, which disables the tracer and explicitly set the mode to
DISABLED during SysFS operations.

The flow is updated with this change:

 CPU0                                    CPU1
 etm4_enable()
  ` etm4_enable_sysfs()
     ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
                                          ` coresight_take_mode(SYSFS)
	                                    Failed, set back to DISABLED
                                          ` coresight_set_mode(DISABLED)

                                          CPU idle:
                                          etm4_cpu_save()
                                           ` coresight_get_mode()
                                              ^^^
                                              Read out the DISABLED mode

Fixes: c38a9ec2b2c1 ("coresight: etm4x: moving etm_drvdata::enable to atomic field")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++-------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 42e5d37403addc6ec81f2e3184522d67d1677c04..ee405c88ea5faa130819f96b00b8307f8764d58a 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -590,10 +590,23 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 static void etm4_enable_hw_smp_call(void *info)
 {
 	struct etm4_enable_arg *arg = info;
+	struct coresight_device *csdev;
 
 	if (WARN_ON(!arg))
 		return;
+
+	csdev = arg->drvdata->csdev;
+	if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
+		/* Someone is already using the tracer */
+		arg->rc = -EBUSY;
+		return;
+	}
+
 	arg->rc = etm4_enable_hw(arg->drvdata);
+
+	/* The tracer didn't start */
+	if (arg->rc)
+		coresight_set_mode(csdev, CS_MODE_DISABLED);
 }
 
 /*
@@ -809,6 +822,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 	int ret = 0;
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
+	if (!coresight_take_mode(csdev, CS_MODE_PERF))
+		return -EBUSY;
+
 	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
 		ret = -EINVAL;
 		goto out;
@@ -828,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 	ret = etm4_enable_hw(drvdata);
 
 out:
+	/* The tracer didn't start */
+	if (ret)
+		coresight_set_mode(csdev, CS_MODE_DISABLED);
 	return ret;
 }
 
@@ -880,11 +899,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
 {
 	int ret;
 
-	if (!coresight_take_mode(csdev, mode)) {
-		/* Someone is already using the tracer */
-		return -EBUSY;
-	}
-
 	switch (mode) {
 	case CS_MODE_SYSFS:
 		ret = etm4_enable_sysfs(csdev, path);
@@ -896,10 +910,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
 		ret = -EINVAL;
 	}
 
-	/* The tracer didn't start */
-	if (ret)
-		coresight_set_mode(csdev, CS_MODE_DISABLED);
-
 	return ret;
 }
 
@@ -951,10 +961,9 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
 	isb();
 }
 
-static void etm4_disable_hw(void *info)
+static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
 {
 	u32 control;
-	struct etmv4_drvdata *drvdata = info;
 	struct etmv4_config *config = &drvdata->config;
 	struct coresight_device *csdev = drvdata->csdev;
 	struct csdev_access *csa = &csdev->access;
@@ -991,6 +1000,15 @@ static void etm4_disable_hw(void *info)
 		"cpu: %d disable smp call done\n", drvdata->cpu);
 }
 
+static void etm4_disable_hw_smp_call(void *info)
+{
+	struct etmv4_drvdata *drvdata = info;
+
+	etm4_disable_hw(drvdata);
+
+	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
+}
+
 static int etm4_disable_perf(struct coresight_device *csdev,
 			     struct perf_event *event)
 {
@@ -1020,6 +1038,8 @@ static int etm4_disable_perf(struct coresight_device *csdev,
 	/* TRCVICTLR::SSSTATUS, bit[9] */
 	filters->ssstatus = (control & BIT(9));
 
+	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
+
 	/*
 	 * perf will release trace ids when _free_aux() is
 	 * called at the end of the session.
@@ -1045,7 +1065,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
 	 * Executing etm4_disable_hw on the cpu whose ETM is being disabled
 	 * ensures that register writes occur when cpu is powered.
 	 */
-	smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
+	smp_call_function_single(drvdata->cpu, etm4_disable_hw_smp_call,
+				 drvdata, 1);
 
 	raw_spin_unlock(&drvdata->spinlock);
 
@@ -1085,9 +1106,6 @@ static void etm4_disable(struct coresight_device *csdev,
 		etm4_disable_perf(csdev, event);
 		break;
 	}
-
-	if (mode)
-		coresight_set_mode(csdev, CS_MODE_DISABLED);
 }
 
 static int etm4_resume_perf(struct coresight_device *csdev)

-- 
2.34.1

Re: [PATCH v2 02/28] coresight: etm4x: Always set tracer's device mode on target CPU

[Yeoreum Yun]

LGTM.

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

> When enabling a tracer via SysFS interface, the device mode may be set
> by any CPU - not necessarily the target CPU. This can lead to race
> condition in SMP, and may result in incorrect mode values being read.
>
> Consider the following example, where CPU0 attempts to enable the tracer
> on CPU1 (the target CPU):
>
>  CPU0                                    CPU1
>  etm4_enable()
>   ` coresight_take_mode(SYSFS)
>   ` etm4_enable_sysfs()
>      ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
>      			                /
>                                        /  CPU idle:
>                                       /   etm4_cpu_save()
>                                      /     ` coresight_get_mode()
> 	       Failed to enable h/w /         ^^^
>   ` coresight_set_mode(DISABLED) <-'          Read the intermediate SYSFS mode
>
> In this case, CPU0 initiates the operation by taking the SYSFS mode to
> avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
> configure the tracer registers. If any error occurs during this process,
> CPU0 rolls back by setting the mode to DISABLED.
>
> However, if CPU1 enters an idle state during this time, it might read
> the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
> save and restore tracer context that is actually disabled.
>
> To resolve the issue, this commit moves the device mode setting logic on
> the target CPU. This ensures that the device mode is only modified by
> the target CPU, eliminating race condition between mode writes and reads
> across CPUs.
>
> An additional change introduces the etm4_disable_hw_smp_call() function
> for SMP calls, which disables the tracer and explicitly set the mode to
> DISABLED during SysFS operations.
>
> The flow is updated with this change:
>
>  CPU0                                    CPU1
>  etm4_enable()
>   ` etm4_enable_sysfs()
>      ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
>                                           ` coresight_take_mode(SYSFS)
> 	                                    Failed, set back to DISABLED
>                                           ` coresight_set_mode(DISABLED)
>
>                                           CPU idle:
>                                           etm4_cpu_save()
>                                            ` coresight_get_mode()
>                                               ^^^
>                                               Read out the DISABLED mode
>
> Fixes: c38a9ec2b2c1 ("coresight: etm4x: moving etm_drvdata::enable to atomic field")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++-------
>  1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 42e5d37403addc6ec81f2e3184522d67d1677c04..ee405c88ea5faa130819f96b00b8307f8764d58a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -590,10 +590,23 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>  static void etm4_enable_hw_smp_call(void *info)
>  {
>  	struct etm4_enable_arg *arg = info;
> +	struct coresight_device *csdev;
>
>  	if (WARN_ON(!arg))
>  		return;
> +
> +	csdev = arg->drvdata->csdev;
> +	if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
> +		/* Someone is already using the tracer */
> +		arg->rc = -EBUSY;
> +		return;
> +	}
> +
>  	arg->rc = etm4_enable_hw(arg->drvdata);
> +
> +	/* The tracer didn't start */
> +	if (arg->rc)
> +		coresight_set_mode(csdev, CS_MODE_DISABLED);
>  }
>
>  /*
> @@ -809,6 +822,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
>  	int ret = 0;
>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> +	if (!coresight_take_mode(csdev, CS_MODE_PERF))
> +		return -EBUSY;
> +
>  	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
>  		ret = -EINVAL;
>  		goto out;
> @@ -828,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
>  	ret = etm4_enable_hw(drvdata);
>
>  out:
> +	/* The tracer didn't start */
> +	if (ret)
> +		coresight_set_mode(csdev, CS_MODE_DISABLED);
>  	return ret;
>  }
>
> @@ -880,11 +899,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
>  {
>  	int ret;
>
> -	if (!coresight_take_mode(csdev, mode)) {
> -		/* Someone is already using the tracer */
> -		return -EBUSY;
> -	}
> -
>  	switch (mode) {
>  	case CS_MODE_SYSFS:
>  		ret = etm4_enable_sysfs(csdev, path);
> @@ -896,10 +910,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
>  		ret = -EINVAL;
>  	}
>
> -	/* The tracer didn't start */
> -	if (ret)
> -		coresight_set_mode(csdev, CS_MODE_DISABLED);
> -
>  	return ret;
>  }
>
> @@ -951,10 +961,9 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
>  	isb();
>  }
>
> -static void etm4_disable_hw(void *info)
> +static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
>  {
>  	u32 control;
> -	struct etmv4_drvdata *drvdata = info;
>  	struct etmv4_config *config = &drvdata->config;
>  	struct coresight_device *csdev = drvdata->csdev;
>  	struct csdev_access *csa = &csdev->access;
> @@ -991,6 +1000,15 @@ static void etm4_disable_hw(void *info)
>  		"cpu: %d disable smp call done\n", drvdata->cpu);
>  }
>
> +static void etm4_disable_hw_smp_call(void *info)
> +{
> +	struct etmv4_drvdata *drvdata = info;
> +
> +	etm4_disable_hw(drvdata);
> +
> +	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +}
> +
>  static int etm4_disable_perf(struct coresight_device *csdev,
>  			     struct perf_event *event)
>  {
> @@ -1020,6 +1038,8 @@ static int etm4_disable_perf(struct coresight_device *csdev,
>  	/* TRCVICTLR::SSSTATUS, bit[9] */
>  	filters->ssstatus = (control & BIT(9));
>
> +	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +
>  	/*
>  	 * perf will release trace ids when _free_aux() is
>  	 * called at the end of the session.
> @@ -1045,7 +1065,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
>  	 * Executing etm4_disable_hw on the cpu whose ETM is being disabled
>  	 * ensures that register writes occur when cpu is powered.
>  	 */
> -	smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
> +	smp_call_function_single(drvdata->cpu, etm4_disable_hw_smp_call,
> +				 drvdata, 1);
>
>  	raw_spin_unlock(&drvdata->spinlock);
>
> @@ -1085,9 +1106,6 @@ static void etm4_disable(struct coresight_device *csdev,
>  		etm4_disable_perf(csdev, event);
>  		break;
>  	}
> -
> -	if (mode)
> -		coresight_set_mode(csdev, CS_MODE_DISABLED);
>  }
>
>  static int etm4_resume_perf(struct coresight_device *csdev)
>
> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun

Re: [PATCH v2 02/28] coresight: etm4x: Always set tracer's device mode on target CPU

[Anshuman Khandual]

On 01/07/25 8:23 PM, Leo Yan wrote:
> When enabling a tracer via SysFS interface, the device mode may be set
> by any CPU - not necessarily the target CPU. This can lead to race
> condition in SMP, and may result in incorrect mode values being read.
> 
> Consider the following example, where CPU0 attempts to enable the tracer
> on CPU1 (the target CPU):
> 
>  CPU0                                    CPU1
>  etm4_enable()
>   ` coresight_take_mode(SYSFS)
>   ` etm4_enable_sysfs()
>      ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
>      			                /
>                                        /  CPU idle:
>                                       /   etm4_cpu_save()
>                                      /     ` coresight_get_mode()
> 	       Failed to enable h/w /         ^^^
>   ` coresight_set_mode(DISABLED) <-'          Read the intermediate SYSFS mode

The problem is - CPU1's HW state and CPU1's sysfs mode state might not
remain in sync if CPU1 goes into idle state just after an unsuccessful
etm4_enable_sysfs() attempt from CPU0. In which case a subsequent read
coresight_get_mode() on CPU1 might erroneously give us DISABLED state,
which actually does not seem to be too bad as the earlier enablement
attempt had failed anyway. Just trying to understand what is the real
problem here.

> 
> In this case, CPU0 initiates the operation by taking the SYSFS mode to
> avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
> configure the tracer registers. If any error occurs during this process,

What kind of error can happen during this process ?

> CPU0 rolls back by setting the mode to DISABLED.

Which seems OK.

> 
> However, if CPU1 enters an idle state during this time, it might read
> the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
> save and restore tracer context that is actually disabled.

Right but CPU0 had marked the CPU1' state as DISABLED after the enable
attempt had failed. So what is the problem ?

> 
> To resolve the issue, this commit moves the device mode setting logic on
> the target CPU. This ensures that the device mode is only modified by
> the target CPU, eliminating race condition between mode writes and reads
> across CPUs.
> 
> An additional change introduces the etm4_disable_hw_smp_call() function
> for SMP calls, which disables the tracer and explicitly set the mode to
> DISABLED during SysFS operations.
> 
> The flow is updated with this change:
> 
>  CPU0                                    CPU1
>  etm4_enable()
>   ` etm4_enable_sysfs()
>      ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
>                                           ` coresight_take_mode(SYSFS)
> 	                                    Failed, set back to DISABLED
>                                           ` coresight_set_mode(DISABLED)
> 
>                                           CPU idle:
>                                           etm4_cpu_save()
>                                            ` coresight_get_mode()
>                                               ^^^
>                                               Read out the DISABLED mode
> 
> Fixes: c38a9ec2b2c1 ("coresight: etm4x: moving etm_drvdata::enable to atomic field")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++-------
>  1 file changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 42e5d37403addc6ec81f2e3184522d67d1677c04..ee405c88ea5faa130819f96b00b8307f8764d58a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -590,10 +590,23 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>  static void etm4_enable_hw_smp_call(void *info)
>  {
>  	struct etm4_enable_arg *arg = info;
> +	struct coresight_device *csdev;
>  
>  	if (WARN_ON(!arg))
>  		return;
> +
> +	csdev = arg->drvdata->csdev;
> +	if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
> +		/* Someone is already using the tracer */
> +		arg->rc = -EBUSY;
> +		return;
> +	}
> +
>  	arg->rc = etm4_enable_hw(arg->drvdata);
> +
> +	/* The tracer didn't start */
> +	if (arg->rc)
> +		coresight_set_mode(csdev, CS_MODE_DISABLED);
>  }
>  
>  /*
> @@ -809,6 +822,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
>  	int ret = 0;
>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>  
> +	if (!coresight_take_mode(csdev, CS_MODE_PERF))
> +		return -EBUSY;
> +
>  	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
>  		ret = -EINVAL;
>  		goto out;
> @@ -828,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev,
>  	ret = etm4_enable_hw(drvdata);
>  
>  out:
> +	/* The tracer didn't start */
> +	if (ret)
> +		coresight_set_mode(csdev, CS_MODE_DISABLED);
>  	return ret;
>  }
>  
> @@ -880,11 +899,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
>  {
>  	int ret;
>  
> -	if (!coresight_take_mode(csdev, mode)) {
> -		/* Someone is already using the tracer */
> -		return -EBUSY;
> -	}
> -
>  	switch (mode) {
>  	case CS_MODE_SYSFS:
>  		ret = etm4_enable_sysfs(csdev, path);
> @@ -896,10 +910,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
>  		ret = -EINVAL;
>  	}
>  
> -	/* The tracer didn't start */
> -	if (ret)
> -		coresight_set_mode(csdev, CS_MODE_DISABLED);
> -
>  	return ret;
>  }
>  
> @@ -951,10 +961,9 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata)
>  	isb();
>  }
>  
> -static void etm4_disable_hw(void *info)
> +static void etm4_disable_hw(struct etmv4_drvdata *drvdata)
>  {
>  	u32 control;
> -	struct etmv4_drvdata *drvdata = info;
>  	struct etmv4_config *config = &drvdata->config;
>  	struct coresight_device *csdev = drvdata->csdev;
>  	struct csdev_access *csa = &csdev->access;
> @@ -991,6 +1000,15 @@ static void etm4_disable_hw(void *info)
>  		"cpu: %d disable smp call done\n", drvdata->cpu);
>  }
>  
> +static void etm4_disable_hw_smp_call(void *info)
> +{
> +	struct etmv4_drvdata *drvdata = info;
> +
> +	etm4_disable_hw(drvdata);
> +
> +	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +}
> +
>  static int etm4_disable_perf(struct coresight_device *csdev,
>  			     struct perf_event *event)
>  {
> @@ -1020,6 +1038,8 @@ static int etm4_disable_perf(struct coresight_device *csdev,
>  	/* TRCVICTLR::SSSTATUS, bit[9] */
>  	filters->ssstatus = (control & BIT(9));
>  
> +	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
> +
>  	/*
>  	 * perf will release trace ids when _free_aux() is
>  	 * called at the end of the session.
> @@ -1045,7 +1065,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
>  	 * Executing etm4_disable_hw on the cpu whose ETM is being disabled
>  	 * ensures that register writes occur when cpu is powered.
>  	 */
> -	smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
> +	smp_call_function_single(drvdata->cpu, etm4_disable_hw_smp_call,
> +				 drvdata, 1);
>  
>  	raw_spin_unlock(&drvdata->spinlock);
>  
> @@ -1085,9 +1106,6 @@ static void etm4_disable(struct coresight_device *csdev,
>  		etm4_disable_perf(csdev, event);
>  		break;
>  	}
> -
> -	if (mode)
> -		coresight_set_mode(csdev, CS_MODE_DISABLED);
>  }
>  
>  static int etm4_resume_perf(struct coresight_device *csdev)
> 

Re: [PATCH v2 02/28] coresight: etm4x: Always set tracer's device mode on target CPU

[Leo Yan]

On Tue, Jul 15, 2025 at 12:56:54PM +0530, Anshuman Khandual wrote:
> 
> On 01/07/25 8:23 PM, Leo Yan wrote:
> > When enabling a tracer via SysFS interface, the device mode may be set
> > by any CPU - not necessarily the target CPU. This can lead to race
> > condition in SMP, and may result in incorrect mode values being read.
> > 
> > Consider the following example, where CPU0 attempts to enable the tracer
> > on CPU1 (the target CPU):
> > 
> >  CPU0                                    CPU1
> >  etm4_enable()
> >   ` coresight_take_mode(SYSFS)
> >   ` etm4_enable_sysfs()
> >      ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
> >      			                /
> >                                        /  CPU idle:
> >                                       /   etm4_cpu_save()
> >                                      /     ` coresight_get_mode()
> > 	       Failed to enable h/w /         ^^^
> >   ` coresight_set_mode(DISABLED) <-'          Read the intermediate SYSFS mode
> 
> The problem is - CPU1's HW state and CPU1's sysfs mode state might not
> remain in sync if CPU1 goes into idle state just after an unsuccessful
> etm4_enable_sysfs() attempt from CPU0. In which case a subsequent read
> coresight_get_mode() on CPU1 might erroneously give us DISABLED state,

In this case, CPU1 reads an intermediate "SYSFS" state, even though it
failed in etm4_enable_hw_smp_call(). The current code defers setting
the state to DISABLED on CPU0. As a result, CPU1 will incorrectly save
and restore the ETM context based on the intermediate "SYSFS" state.

> which actually does not seem to be too bad as the earlier enablement
> attempt had failed anyway. Just trying to understand what is the real
> problem here.

The problem is CPU1 might get an intermediate state, it turns out a
stale value and might guide CPU idle flow to wrongly save and restore
ETM context.

> > In this case, CPU0 initiates the operation by taking the SYSFS mode to
> > avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
> > configure the tracer registers. If any error occurs during this process,
> 
> What kind of error can happen during this process ?

So far, it might fail to claim a device and return an error.

A similar issue might occur when CPU1 exits an idle state. For example,
if CPU0 initiates the ETM enabling flow and sets the SYSFS mode in
advance, once CPU1 is woken up from idle by an IPI, it reads the ETM
state (SYSFS mode) and then restores and enables the ETM. This can
happen even before CPU1 invokes etm4_enable_hw_smp_call() to complete
the ETM enable flow.

> > CPU0 rolls back by setting the mode to DISABLED.
> 
> Which seems OK.
> 
> > 
> > However, if CPU1 enters an idle state during this time, it might read
> > the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
> > save and restore tracer context that is actually disabled.
> 
> Right but CPU0 had marked the CPU1' state as DISABLED after the enable
> attempt had failed. So what is the problem ?

There is a race condition between CPU0 writing the state and CPU1
reading the state (during its CPU idle flow). CPU1 might read a state
that is inconsistent with the actual ETM hardware state, which causes
CPU1 to save and restore the ETM context incorrectly.

A wider view is this series heavily relies on the ETM state to decide
the linked path has been enabled and take action for saving and
restoring all components on the path (not for ETM device only). We need
a reliable state machine to reflect hardware state. To avoid any
intermediate state, we always set the state on the target CPU.

Thanks,
Leo

Re: [PATCH v2 02/28] coresight: etm4x: Always set tracer's device mode on target CPU

[James Clark]

On 01/07/2025 3:53 pm, Leo Yan wrote:
> When enabling a tracer via SysFS interface, the device mode may be set
> by any CPU - not necessarily the target CPU. This can lead to race
> condition in SMP, and may result in incorrect mode values being read.
> 
> Consider the following example, where CPU0 attempts to enable the tracer
> on CPU1 (the target CPU):
> 
>   CPU0                                    CPU1
>   etm4_enable()
>    ` coresight_take_mode(SYSFS)
>    ` etm4_enable_sysfs()
>       ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
>       			                /
>                                         /  CPU idle:
>                                        /   etm4_cpu_save()
>                                       /     ` coresight_get_mode()
> 	       Failed to enable h/w /         ^^^
>    ` coresight_set_mode(DISABLED) <-'          Read the intermediate SYSFS mode
> 


> In this case, CPU0 initiates the operation by taking the SYSFS mode to
> avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to
> configure the tracer registers. If any error occurs during this process,
> CPU0 rolls back by setting the mode to DISABLED.
> 
> However, if CPU1 enters an idle state during this time, it might read
> the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly
> save and restore tracer context that is actually disabled.
> 
> To resolve the issue, this commit moves the device mode setting logic on
> the target CPU. This ensures that the device mode is only modified by
> the target CPU, eliminating race condition between mode writes and reads
> across CPUs.
> 
> An additional change introduces the etm4_disable_hw_smp_call() function
> for SMP calls, which disables the tracer and explicitly set the mode to
> DISABLED during SysFS operations.
> 
> The flow is updated with this change:
> 
>   CPU0                                    CPU1
>   etm4_enable()
>    ` etm4_enable_sysfs()
>       ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
>                                            ` coresight_take_mode(SYSFS)
> 	                                    Failed, set back to DISABLED
>                                            ` coresight_set_mode(DISABLED)
> 
>                                            CPU idle:
>                                            etm4_cpu_save()
>                                             ` coresight_get_mode()
>                                                ^^^
>                                                Read out the DISABLED mode

There's no lock though, so can't it do this:

  CPU0                                    CPU1
  etm4_enable()
    ` etm4_enable_sysfs()
       ` smp_call_function_single() --->  etm4_enable_hw_smp_call()
                                              `coresight_take_mode(SYSFS)

                                          CPU idle:
                                          etm4_cpu_save()
                                            ` coresight_get_mode()
                                           ^^ Intermediate SYSFS mode

This is why I was voting for changing the compare and swap mode stuff to 
spinlocks so we can be sure it's correct. The lock shouldn't be released 
until the mode is at its final value.

Re: [PATCH v2 02/28] coresight: etm4x: Always set tracer's device mode on target CPU

[Leo Yan]

On Thu, Aug 21, 2025 at 10:45:10AM +0100, James Clark wrote:

[...]

> > The flow is updated with this change:
> > 
> >   CPU0                                    CPU1
> >   etm4_enable()
> >    ` etm4_enable_sysfs()
> >       ` smp_call_function_single() ---->  etm4_enable_hw_smp_call()
> >                                            ` coresight_take_mode(SYSFS)
> > 	                                    Failed, set back to DISABLED
> >                                            ` coresight_set_mode(DISABLED)
> > 
> >                                            CPU idle:
> >                                            etm4_cpu_save()
> >                                             ` coresight_get_mode()
> >                                                ^^^
> >                                                Read out the DISABLED mode
> 
> There's no lock though, so can't it do this:
> 
>  CPU0                                    CPU1
>  etm4_enable()
>    ` etm4_enable_sysfs()
>       ` smp_call_function_single() --->  etm4_enable_hw_smp_call()
>                                              `coresight_take_mode(SYSFS)
> 
>                                          CPU idle:
>                                          etm4_cpu_save()
>                                            ` coresight_get_mode()
>                                           ^^ Intermediate SYSFS mode

No.

The point is a target CPU will execute etm4_enable_hw_smp_call() and CPU
idle flow sequentially. It is no chance for the idle flow to preempt the
ETM flow. This is why using the target CPU to set the ETM state machine
will dismiss the race condition.

> This is why I was voting for changing the compare and swap mode stuff to
> spinlocks so we can be sure it's correct. The lock shouldn't be released
> until the mode is at its final value.

The question is a spinlock usage. If both initiate CPUs and target CPU
try to acquire the spinlock, the low level's CPU idle flow
(etm4_cpu_save() / etm4_cpu_restore()) needs to wait for high level's
SysFS operation to complete - which is what we try to avoid.

If we move the state machine on the target CPU - the operation and its
state transition on the same CPU so that dismiss a race condition. And
the idle flow is sequential to the ETM enabling/disabling, then, we can
achieve lockless approach.

A global picture is ETM's state is used in multiple places (Sysfs knobs,
ETM enabling / disabling, CPU idle and hotplug flow), if we start to use
spinlock for exclusively access the statch machine, then everywhere will
use it. This is why this series wants to do reliable state transition
based on appropriate atomic APIs (it also can promise atomicity and
ordering but lockless).

Thanks,
Leo

[PATCH v2 03/28] 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()")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm3x-core.c | 60 +++++++++++++++-------
 1 file changed, 42 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 1c6204e1442211be6f3d7ca34bd2251ba796601b..0f160f2f97344e6a96343cd8658f4f19806193e0 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -442,10 +442,23 @@ struct etm_enable_arg {
 static void etm_enable_hw_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)
@@ -464,17 +477,29 @@ static int etm_enable_perf(struct coresight_device *csdev,
 			   struct perf_event *event,
 			   struct coresight_path *path)
 {
+	int ret = 0;
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
-		return -EINVAL;
+	if (!coresight_take_mode(csdev, CS_MODE_PERF))
+		return -EBUSY;
+
+	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	/* 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);
+
+out:
+	/* The tracer didn't start */
+	if (ret)
+		coresight_set_mode(csdev, CS_MODE_DISABLED);
+	return ret;
 }
 
 static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path)
@@ -519,11 +544,6 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
 	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:
 		ret = etm_enable_sysfs(csdev, path);
@@ -535,17 +555,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 etmv4_drvdata *drvdata)
 {
 	int i;
-	struct etm_drvdata *drvdata = info;
 	struct etm_config *config = &drvdata->config;
 	struct coresight_device *csdev = drvdata->csdev;
 
@@ -567,6 +582,15 @@ static void etm_disable_hw(void *info)
 		"cpu: %d disable smp call done\n", drvdata->cpu);
 }
 
+static void etm_disable_hw_smp_call(void *info)
+{
+	struct etmv_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 +612,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 +638,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_hw_smp_call,
+				 drvdata, 1);
 
 	spin_unlock(&drvdata->spinlock);
 	cpus_read_unlock();
@@ -652,9 +679,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

Re: [PATCH v2 03/28] coresight: etm3x: Always set tracer's device mode on target CPU

[kernel test robot]

Hi Leo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 66701750d5565c574af42bef0b789ce0203e3071]

url:    https://github.com/intel-lab-lkp/linux/commits/Leo-Yan/coresight-Change-device-mode-to-atomic-type/20250701-230354
base:   66701750d5565c574af42bef0b789ce0203e3071
patch link:    https://lore.kernel.org/r/20250701-arm_cs_pm_fix_v3-v2-3-23ebb864fcc1%40arm.com
patch subject: [PATCH v2 03/28] coresight: etm3x: Always set tracer's device mode on target CPU
config: arm-u8500_defconfig (https://download.01.org/0day-ci/archive/20250702/202507021259.wYmjhqn4-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250702/202507021259.wYmjhqn4-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507021259.wYmjhqn4-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/hwtracing/coresight/coresight-etm3x-core.c: In function 'etm_enable':
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:545:29: warning: unused variable 'drvdata' [-Wunused-variable]
     545 |         struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
         |                             ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c: At top level:
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: warning: 'struct etmv4_drvdata' declared inside parameter list will not be visible outside of this definition or declaration
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^~~~~~~~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c: In function 'etm_disable_hw':
   drivers/hwtracing/coresight/coresight-etm3x-core.c:564:45: error: invalid use of undefined type 'struct etmv4_drvdata'
     564 |         struct etm_config *config = &drvdata->config;
         |                                             ^~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:565:49: error: invalid use of undefined type 'struct etmv4_drvdata'
     565 |         struct coresight_device *csdev = drvdata->csdev;
         |                                                 ^~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:567:26: error: invalid use of undefined type 'struct etmv4_drvdata'
     567 |         CS_UNLOCK(drvdata->csa.base);
         |                          ^~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:568:22: error: passing argument 1 of 'etm_set_prog' from incompatible pointer type [-Wincompatible-pointer-types]
     568 |         etm_set_prog(drvdata);
         |                      ^~~~~~~
         |                      |
         |                      struct etmv4_drvdata *
   drivers/hwtracing/coresight/coresight-etm3x-core.c:154:46: note: expected 'struct etm_drvdata *' but argument is of type 'struct etmv4_drvdata *'
     154 | static void etm_set_prog(struct etm_drvdata *drvdata)
         |                          ~~~~~~~~~~~~~~~~~~~~^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:571:45: error: passing argument 1 of 'etm_readl' from incompatible pointer type [-Wincompatible-pointer-types]
     571 |         config->seq_curr_state = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
         |                                             ^~~~~~~
         |                                             |
         |                                             struct etmv4_drvdata *
   In file included from drivers/hwtracing/coresight/coresight-etm3x-core.c:33:
   drivers/hwtracing/coresight/coresight-etm.h:267:58: note: expected 'struct etm_drvdata *' but argument is of type 'struct etmv4_drvdata *'
     267 | static inline unsigned int etm_readl(struct etm_drvdata *drvdata, u32 off)
         |                                      ~~~~~~~~~~~~~~~~~~~~^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:573:32: error: invalid use of undefined type 'struct etmv4_drvdata'
     573 |         for (i = 0; i < drvdata->nr_cntr; i++)
         |                                ^~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:574:49: error: passing argument 1 of 'etm_readl' from incompatible pointer type [-Wincompatible-pointer-types]
     574 |                 config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
         |                                                 ^~~~~~~
         |                                                 |
         |                                                 struct etmv4_drvdata *
   drivers/hwtracing/coresight/coresight-etm.h:267:58: note: expected 'struct etm_drvdata *' but argument is of type 'struct etmv4_drvdata *'
     267 | static inline unsigned int etm_readl(struct etm_drvdata *drvdata, u32 off)
         |                                      ~~~~~~~~~~~~~~~~~~~~^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:576:24: error: passing argument 1 of 'etm_set_pwrdwn' from incompatible pointer type [-Wincompatible-pointer-types]
     576 |         etm_set_pwrdwn(drvdata);
         |                        ^~~~~~~
         |                        |
         |                        struct etmv4_drvdata *
   drivers/hwtracing/coresight/coresight-etm3x-core.c:61:48: note: expected 'struct etm_drvdata *' but argument is of type 'struct etmv4_drvdata *'
      61 | static void etm_set_pwrdwn(struct etm_drvdata *drvdata)
         |                            ~~~~~~~~~~~~~~~~~~~~^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:579:24: error: invalid use of undefined type 'struct etmv4_drvdata'
     579 |         CS_LOCK(drvdata->csa.base);
         |                        ^~
   In file included from include/linux/device.h:15,
                    from drivers/hwtracing/coresight/coresight-etm3x-core.c:12:
   drivers/hwtracing/coresight/coresight-etm3x-core.c:581:25: error: invalid use of undefined type 'struct etmv4_drvdata'
     581 |         dev_dbg(&drvdata->csdev->dev,
         |                         ^~
   include/linux/dev_printk.h:139:44: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                            ^~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:581:9: note: in expansion of macro 'dev_dbg'
     581 |         dev_dbg(&drvdata->csdev->dev,
         |         ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:582:59: error: invalid use of undefined type 'struct etmv4_drvdata'
     582 |                 "cpu: %d disable smp call done\n", drvdata->cpu);
         |                                                           ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:581:9: note: in expansion of macro 'dev_dbg'
     581 |         dev_dbg(&drvdata->csdev->dev,
         |         ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c: In function 'etm_disable_hw_smp_call':
   drivers/hwtracing/coresight/coresight-etm3x-core.c:589:24: error: passing argument 1 of 'etm_disable_hw' from incompatible pointer type [-Wincompatible-pointer-types]
     589 |         etm_disable_hw(drvdata);
         |                        ^~~~~~~
         |                        |
         |                        struct etmv_drvdata *
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:50: note: expected 'struct etmv4_drvdata *' but argument is of type 'struct etmv_drvdata *'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                            ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:591:35: error: invalid use of undefined type 'struct etmv_drvdata'
     591 |         coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
         |                                   ^~
   drivers/hwtracing/coresight/coresight-etm3x-core.c: In function 'etm_dying_cpu':
   drivers/hwtracing/coresight/coresight-etm3x-core.c:729:42: error: passing argument 1 of 'etm_disable_hw' from incompatible pointer type [-Wincompatible-pointer-types]
     729 |                 etm_disable_hw(etmdrvdata[cpu]);
         |                                ~~~~~~~~~~^~~~~
         |                                          |
         |                                          struct etm_drvdata *
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:50: note: expected 'struct etmv4_drvdata *' but argument is of type 'struct etm_drvdata *'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                            ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~


vim +/drvdata +545 drivers/hwtracing/coresight/coresight-etm3x-core.c

a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  540  
9fa3682869d4e1 drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark      2023-04-25  541  static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
7b365f056d8e02 drivers/hwtracing/coresight/coresight-etm3x-core.c Jie Gan          2025-03-03  542  		      enum cs_mode mode, struct coresight_path *path)
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  543  {
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  544  	int ret;
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17 @545  	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  546  
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  547  	switch (mode) {
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  548  	case CS_MODE_SYSFS:
7b365f056d8e02 drivers/hwtracing/coresight/coresight-etm3x-core.c Jie Gan          2025-03-03  549  		ret = etm_enable_sysfs(csdev, path);
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  550  		break;
882d5e112491c8 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  551  	case CS_MODE_PERF:
7b365f056d8e02 drivers/hwtracing/coresight/coresight-etm3x-core.c Jie Gan          2025-03-03  552  		ret = etm_enable_perf(csdev, event, path);
882d5e112491c8 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  553  		break;
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  554  	default:
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  555  		ret = -EINVAL;
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  556  	}
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  557  
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  558  	return ret;
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  559  }
22fd532eaa0c24 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  560  
a136c0aff8e112 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan          2025-07-01 @561  static void etm_disable_hw(struct etmv4_drvdata *drvdata)
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  562  {
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  563  	int i;
1925a470ce69cd drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  564  	struct etm_config *config = &drvdata->config;
8ce0029658ba16 drivers/hwtracing/coresight/coresight-etm3x-core.c Suzuki K Poulose 2021-02-01  565  	struct coresight_device *csdev = drvdata->csdev;
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  566  
a1b0e77ce517ec drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark      2025-03-25  567  	CS_UNLOCK(drvdata->csa.base);
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  568  	etm_set_prog(drvdata);
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  569  
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  570  	/* Read back sequencer and counters for post trace analysis */
1925a470ce69cd drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  571  	config->seq_curr_state = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  572  
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  573  	for (i = 0; i < drvdata->nr_cntr; i++)
1925a470ce69cd drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2016-02-17  574  		config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  575  
6dd4402f24a39a drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier  2018-11-30  576  	etm_set_pwrdwn(drvdata);
8ce0029658ba16 drivers/hwtracing/coresight/coresight-etm3x-core.c Suzuki K Poulose 2021-02-01  577  	coresight_disclaim_device_unlocked(csdev);
68a147752d04da drivers/hwtracing/coresight/coresight-etm3x.c      Suzuki K Poulose 2018-09-20  578  
a1b0e77ce517ec drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark      2025-03-25  579  	CS_LOCK(drvdata->csa.base);
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  580  
aaff7623284159 drivers/hwtracing/coresight/coresight-etm3x.c      Suzuki K Poulose 2019-06-19  581  	dev_dbg(&drvdata->csdev->dev,
aaff7623284159 drivers/hwtracing/coresight/coresight-etm3x.c      Suzuki K Poulose 2019-06-19  582  		"cpu: %d disable smp call done\n", drvdata->cpu);
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  583  }
a939fc5a71ad53 drivers/coresight/coresight-etm3x.c                Pratik Patel     2014-11-03  584  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 03/28] coresight: etm3x: Always set tracer's device mode on target CPU

[kernel test robot]

Hi Leo,

kernel test robot noticed the following build errors:

[auto build test ERROR on 66701750d5565c574af42bef0b789ce0203e3071]

url:    https://github.com/intel-lab-lkp/linux/commits/Leo-Yan/coresight-Change-device-mode-to-atomic-type/20250701-230354
base:   66701750d5565c574af42bef0b789ce0203e3071
patch link:    https://lore.kernel.org/r/20250701-arm_cs_pm_fix_v3-v2-3-23ebb864fcc1%40arm.com
patch subject: [PATCH v2 03/28] coresight: etm3x: Always set tracer's device mode on target CPU
config: arm-randconfig-004-20250702 (https://download.01.org/0day-ci/archive/20250702/202507021124.opIH3gFn-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250702/202507021124.opIH3gFn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507021124.opIH3gFn-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/hwtracing/coresight/coresight-etm3x-core.c:545:22: warning: unused variable 'drvdata' [-Wunused-variable]
     545 |         struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
         |                             ^~~~~~~
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: warning: declaration of 'struct etmv4_drvdata' will not be visible outside of this function [-Wvisibility]
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:564:38: error: incomplete definition of type 'struct etmv4_drvdata'
     564 |         struct etm_config *config = &drvdata->config;
         |                                      ~~~~~~~^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: note: forward declaration of 'struct etmv4_drvdata'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:565:42: error: incomplete definition of type 'struct etmv4_drvdata'
     565 |         struct coresight_device *csdev = drvdata->csdev;
         |                                          ~~~~~~~^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: note: forward declaration of 'struct etmv4_drvdata'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:567:19: error: incomplete definition of type 'struct etmv4_drvdata'
     567 |         CS_UNLOCK(drvdata->csa.base);
         |                   ~~~~~~~^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: note: forward declaration of 'struct etmv4_drvdata'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:568:15: error: incompatible pointer types passing 'struct etmv4_drvdata *' to parameter of type 'struct etm_drvdata *' [-Werror,-Wincompatible-pointer-types]
     568 |         etm_set_prog(drvdata);
         |                      ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:154:46: note: passing argument to parameter 'drvdata' here
     154 | static void etm_set_prog(struct etm_drvdata *drvdata)
         |                                              ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:571:38: error: incompatible pointer types passing 'struct etmv4_drvdata *' to parameter of type 'struct etm_drvdata *' [-Werror,-Wincompatible-pointer-types]
     571 |         config->seq_curr_state = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
         |                                             ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm.h:267:58: note: passing argument to parameter 'drvdata' here
     267 | static inline unsigned int etm_readl(struct etm_drvdata *drvdata, u32 off)
         |                                                          ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:573:25: error: incomplete definition of type 'struct etmv4_drvdata'
     573 |         for (i = 0; i < drvdata->nr_cntr; i++)
         |                         ~~~~~~~^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: note: forward declaration of 'struct etmv4_drvdata'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:574:35: error: incompatible pointer types passing 'struct etmv4_drvdata *' to parameter of type 'struct etm_drvdata *' [-Werror,-Wincompatible-pointer-types]
     574 |                 config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
         |                                                 ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm.h:267:58: note: passing argument to parameter 'drvdata' here
     267 | static inline unsigned int etm_readl(struct etm_drvdata *drvdata, u32 off)
         |                                                          ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:576:17: error: incompatible pointer types passing 'struct etmv4_drvdata *' to parameter of type 'struct etm_drvdata *' [-Werror,-Wincompatible-pointer-types]
     576 |         etm_set_pwrdwn(drvdata);
         |                        ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:61:48: note: passing argument to parameter 'drvdata' here
      61 | static void etm_set_pwrdwn(struct etm_drvdata *drvdata)
         |                                                ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:579:17: error: incomplete definition of type 'struct etmv4_drvdata'
     579 |         CS_LOCK(drvdata->csa.base);
         |                 ~~~~~~~^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: note: forward declaration of 'struct etmv4_drvdata'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:581:18: error: incomplete definition of type 'struct etmv4_drvdata'
     581 |         dev_dbg(&drvdata->csdev->dev,
         |                  ~~~~~~~^
   include/linux/dev_printk.h:171:28: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                   ^~~
   include/linux/dev_printk.h:139:23: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                            ^~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: note: forward declaration of 'struct etmv4_drvdata'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:582:45: error: incomplete definition of type 'struct etmv4_drvdata'
     582 |                 "cpu: %d disable smp call done\n", drvdata->cpu);
         |                                                    ~~~~~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:35: note: forward declaration of 'struct etmv4_drvdata'
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                   ^
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:589:17: error: incompatible pointer types passing 'struct etmv_drvdata *' to parameter of type 'struct etmv4_drvdata *' [-Werror,-Wincompatible-pointer-types]
     589 |         etm_disable_hw(drvdata);
         |                        ^~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:50: note: passing argument to parameter 'drvdata' here
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                                  ^
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:591:28: error: incomplete definition of type 'struct etmv_drvdata'
     591 |         coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
         |                            ~~~~~~~^
   drivers/hwtracing/coresight/coresight-etm3x-core.c:587:9: note: forward declaration of 'struct etmv_drvdata'
     587 |         struct etmv_drvdata *drvdata = info;
         |                ^
>> drivers/hwtracing/coresight/coresight-etm3x-core.c:729:18: error: incompatible pointer types passing 'struct etm_drvdata *' to parameter of type 'struct etmv4_drvdata *' [-Werror,-Wincompatible-pointer-types]
     729 |                 etm_disable_hw(etmdrvdata[cpu]);
         |                                ^~~~~~~~~~~~~~~
   drivers/hwtracing/coresight/coresight-etm3x-core.c:561:50: note: passing argument to parameter 'drvdata' here
     561 | static void etm_disable_hw(struct etmv4_drvdata *drvdata)
         |                                                  ^
   2 warnings and 14 errors generated.


vim +564 drivers/hwtracing/coresight/coresight-etm3x-core.c

22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  560  
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01 @561  static void etm_disable_hw(struct etmv4_drvdata *drvdata)
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  562  {
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  563  	int i;
1925a470ce69cdf drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17 @564  	struct etm_config *config = &drvdata->config;
8ce0029658ba16c drivers/hwtracing/coresight/coresight-etm3x-core.c Suzuki K Poulose          2021-02-01  565  	struct coresight_device *csdev = drvdata->csdev;
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  566  
a1b0e77ce517ec0 drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2025-03-25  567  	CS_UNLOCK(drvdata->csa.base);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03 @568  	etm_set_prog(drvdata);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  569  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  570  	/* Read back sequencer and counters for post trace analysis */
1925a470ce69cdf drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  571  	config->seq_curr_state = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  572  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  573  	for (i = 0; i < drvdata->nr_cntr; i++)
1925a470ce69cdf drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  574  		config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  575  
6dd4402f24a39a9 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2018-11-30  576  	etm_set_pwrdwn(drvdata);
8ce0029658ba16c drivers/hwtracing/coresight/coresight-etm3x-core.c Suzuki K Poulose          2021-02-01  577  	coresight_disclaim_device_unlocked(csdev);
68a147752d04da7 drivers/hwtracing/coresight/coresight-etm3x.c      Suzuki K Poulose          2018-09-20  578  
a1b0e77ce517ec0 drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2025-03-25  579  	CS_LOCK(drvdata->csa.base);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  580  
aaff7623284159a drivers/hwtracing/coresight/coresight-etm3x.c      Suzuki K Poulose          2019-06-19  581  	dev_dbg(&drvdata->csdev->dev,
aaff7623284159a drivers/hwtracing/coresight/coresight-etm3x.c      Suzuki K Poulose          2019-06-19  582  		"cpu: %d disable smp call done\n", drvdata->cpu);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  583  }
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  584  
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  585  static void etm_disable_hw_smp_call(void *info)
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  586  {
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  587  	struct etmv_drvdata *drvdata = info;
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  588  
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01 @589  	etm_disable_hw(drvdata);
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  590  
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01 @591  	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  592  }
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  593  
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  594  static void etm_disable_perf(struct coresight_device *csdev)
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  595  {
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  596  	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  597  
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  598  	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  599  		return;
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  600  
a1b0e77ce517ec0 drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2025-03-25  601  	CS_UNLOCK(drvdata->csa.base);
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  602  
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  603  	/* Setting the prog bit disables tracing immediately */
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  604  	etm_set_prog(drvdata);
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  605  
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  606  	/*
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  607  	 * There is no way to know when the tracer will be used again so
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  608  	 * power down the tracer.
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  609  	 */
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  610  	etm_set_pwrdwn(drvdata);
8ce0029658ba16c drivers/hwtracing/coresight/coresight-etm3x-core.c Suzuki K Poulose          2021-02-01  611  	coresight_disclaim_device_unlocked(csdev);
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  612  
a1b0e77ce517ec0 drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2025-03-25  613  	CS_LOCK(drvdata->csa.base);
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  614  
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  615  	coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  616  
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  617  	/*
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  618  	 * perf will release trace ids when _free_aux()
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  619  	 * is called at the end of the session
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  620  	 */
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  621  
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  622  }
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  623  
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  624  static void etm_disable_sysfs(struct coresight_device *csdev)
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  625  {
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  626  	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  627  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  628  	/*
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  629  	 * Taking hotplug lock here protects from clocks getting disabled
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  630  	 * with tracing being left on (crash scenario) if user disable occurs
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  631  	 * after cpu online mask indicates the cpu is offline but before the
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  632  	 * DYING hotplug callback is serviced by the ETM driver.
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  633  	 */
e560c89c8ac0baa drivers/hwtracing/coresight/coresight-etm3x.c      Sebastian Andrzej Siewior 2017-05-24  634  	cpus_read_lock();
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  635  	spin_lock(&drvdata->spinlock);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  636  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  637  	/*
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  638  	 * Executing etm_disable_hw on the cpu whose ETM is being disabled
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  639  	 * ensures that register writes occur when cpu is powered.
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  640  	 */
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  641  	smp_call_function_single(drvdata->cpu, etm_disable_hw_smp_call,
a136c0aff8e1120 drivers/hwtracing/coresight/coresight-etm3x-core.c Leo Yan                   2025-07-01  642  				 drvdata, 1);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  643  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  644  	spin_unlock(&drvdata->spinlock);
e560c89c8ac0baa drivers/hwtracing/coresight/coresight-etm3x.c      Sebastian Andrzej Siewior 2017-05-24  645  	cpus_read_unlock();
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  646  
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  647  	/*
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  648  	 * we only release trace IDs when resetting sysfs.
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  649  	 * This permits sysfs users to read the trace ID after the trace
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  650  	 * session has completed. This maintains operational behaviour with
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  651  	 * prior trace id allocation method
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  652  	 */
9edf291091f68f4 drivers/hwtracing/coresight/coresight-etm3x-core.c Mike Leach                2023-01-16  653  
aaff7623284159a drivers/hwtracing/coresight/coresight-etm3x.c      Suzuki K Poulose          2019-06-19  654  	dev_dbg(&csdev->dev, "ETM tracing disabled\n");
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  655  }
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  656  
68905d73df5d51b drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-08-25  657  static void etm_disable(struct coresight_device *csdev,
68905d73df5d51b drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-08-25  658  			struct perf_event *event)
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  659  {
9fa3682869d4e16 drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2023-04-25  660  	enum cs_mode mode;
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  661  
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  662  	/*
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  663  	 * For as long as the tracer isn't disabled another entity can't
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  664  	 * change its status.  As such we can read the status here without
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  665  	 * fearing it will change under us.
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  666  	 */
c95c2733e5feb1f drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2024-01-29  667  	mode = coresight_get_mode(csdev);
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  668  
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  669  	switch (mode) {
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  670  	case CS_MODE_DISABLED:
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  671  		break;
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  672  	case CS_MODE_SYSFS:
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  673  		etm_disable_sysfs(csdev);
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  674  		break;
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  675  	case CS_MODE_PERF:
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  676  		etm_disable_perf(csdev);
882d5e112491c87 drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  677  		break;
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  678  	default:
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  679  		WARN_ON_ONCE(mode);
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  680  		return;
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  681  	}
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  682  }
22fd532eaa0c24d drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-17  683  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  684  static const struct coresight_ops_source etm_source_ops = {
52210c8745e418f drivers/hwtracing/coresight/coresight-etm3x.c      Mathieu Poirier           2016-02-02  685  	.cpu_id		= etm_cpu_id,
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  686  	.enable		= etm_enable,
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  687  	.disable	= etm_disable,
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  688  };
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  689  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  690  static const struct coresight_ops etm_cs_ops = {
c367a89dec267c6 drivers/hwtracing/coresight/coresight-etm3x-core.c Jie Gan                   2025-03-03  691  	.trace_id	= coresight_etm_get_trace_id,
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  692  	.source_ops	= &etm_source_ops,
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  693  };
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  694  
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  695  static int etm_online_cpu(unsigned int cpu)
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  696  {
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  697  	if (!etmdrvdata[cpu])
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  698  		return 0;
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  699  
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  700  	if (etmdrvdata[cpu]->boot_enable && !etmdrvdata[cpu]->sticky_enable)
1f5149c7751c50a drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2024-01-29  701  		coresight_enable_sysfs(etmdrvdata[cpu]->csdev);
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  702  	return 0;
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  703  }
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  704  
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  705  static int etm_starting_cpu(unsigned int cpu)
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  706  {
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  707  	if (!etmdrvdata[cpu])
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  708  		return 0;
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  709  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  710  	spin_lock(&etmdrvdata[cpu]->spinlock);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  711  	if (!etmdrvdata[cpu]->os_unlock) {
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  712  		etm_os_unlock(etmdrvdata[cpu]);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  713  		etmdrvdata[cpu]->os_unlock = true;
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  714  	}
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  715  
c95c2733e5feb1f drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2024-01-29  716  	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  717  		etm_enable_hw(etmdrvdata[cpu]);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  718  	spin_unlock(&etmdrvdata[cpu]->spinlock);
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  719  	return 0;
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  720  }
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  721  
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  722  static int etm_dying_cpu(unsigned int cpu)
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  723  {
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  724  	if (!etmdrvdata[cpu])
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  725  		return 0;
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  726  
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  727  	spin_lock(&etmdrvdata[cpu]->spinlock);
c95c2733e5feb1f drivers/hwtracing/coresight/coresight-etm3x-core.c James Clark               2024-01-29  728  	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03 @729  		etm_disable_hw(etmdrvdata[cpu]);
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  730  	spin_unlock(&etmdrvdata[cpu]->spinlock);
2b5283d12f67839 drivers/hwtracing/coresight/coresight-etm3x.c      Richard Cochran           2016-07-13  731  	return 0;
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  732  }
a939fc5a71ad531 drivers/coresight/coresight-etm3x.c                Pratik Patel              2014-11-03  733  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 03/28] coresight: etm3x: Always set tracer's device mode on target CPU

[Yeoreum Yun]

Hi Leo,

[...]
> @@ -464,17 +477,29 @@ static int etm_enable_perf(struct coresight_device *csdev,
>  			   struct perf_event *event,
>  			   struct coresight_path *path)
>  {
> +	int ret = 0;
>  	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> -	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
> -		return -EINVAL;
> +	if (!coresight_take_mode(csdev, CS_MODE_PERF))
> +		return -EBUSY;
> +
> +	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
> +		ret = -EINVAL;
> +		goto out;
> +	}

Small question: why drvdata->cpu != smp_processor_id() check after
changing mode? Would it better to check before change of it?

>
>  	/* 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);
> +
> +out:
> +	/* The tracer didn't start */
> +	if (ret)
> +		coresight_set_mode(csdev, CS_MODE_DISABLED);
> +	return ret;
>  }
>
>  static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path)
> @@ -519,11 +544,6 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
>  	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:
>  		ret = etm_enable_sysfs(csdev, path);
> @@ -535,17 +555,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 etmv4_drvdata *drvdata)
>  {
>  	int i;
> -	struct etm_drvdata *drvdata = info;
>  	struct etm_config *config = &drvdata->config;
>  	struct coresight_device *csdev = drvdata->csdev;
>
> @@ -567,6 +582,15 @@ static void etm_disable_hw(void *info)
>  		"cpu: %d disable smp call done\n", drvdata->cpu);
>  }
>
> +static void etm_disable_hw_smp_call(void *info)
> +{
> +	struct etmv_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 +612,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 +638,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_hw_smp_call,
> +				 drvdata, 1);
>
>  	spin_unlock(&drvdata->spinlock);
>  	cpus_read_unlock();
> @@ -652,9 +679,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
>

--
Sincerely,
Yeoreum Yun

Re: [PATCH v2 03/28] coresight: etm3x: Always set tracer's device mode on target CPU

[Leo Yan]

On Wed, Jul 02, 2025 at 11:18:21AM +0100, Yeoreum Yun wrote:
> Hi Leo,
> 
> [...]
> > @@ -464,17 +477,29 @@ static int etm_enable_perf(struct coresight_device *csdev,
> >  			   struct perf_event *event,
> >  			   struct coresight_path *path)
> >  {
> > +	int ret = 0;
> >  	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> >
> > -	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
> > -		return -EINVAL;
> > +	if (!coresight_take_mode(csdev, CS_MODE_PERF))
> > +		return -EBUSY;
> > +
> > +	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> 
> Small question: why drvdata->cpu != smp_processor_id() check after
> changing mode? Would it better to check before change of it?

You are right. I will update in next version.

Thanks for pointing out this!

Leo

[PATCH v2 04/28] 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")
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 ee405c88ea5faa130819f96b00b8307f8764d58a..0f2a8b8459c93ca29d270b6fa05928244e0761c5 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1921,7 +1921,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

Re: [PATCH v2 04/28] coresight: etm4x: Correct polling IDLE bit

[Yeoreum Yun]

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

> 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")
> 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 ee405c88ea5faa130819f96b00b8307f8764d58a..0f2a8b8459c93ca29d270b6fa05928244e0761c5 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1921,7 +1921,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
>

--
Sincerely,
Yeoreum Yun

[PATCH v2 05/28] 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")
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 0f2a8b8459c93ca29d270b6fa05928244e0761c5..af9d3b2319c5f49ccd40dfa0ccf0f694ce9e2f4f 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -459,13 +459,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;
 }
 
@@ -579,6 +572,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 v2 05/28] coresight: etm4x: Ensure context synchronization is not ignored

[Yeoreum Yun]

Hi Leo,

> 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")
> 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 0f2a8b8459c93ca29d270b6fa05928244e0761c5..af9d3b2319c5f49ccd40dfa0ccf0f694ce9e2f4f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -459,13 +459,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;
>  }
>
> @@ -579,6 +572,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();

But according to 4.3.7 ("Synchronization when using memory-mapped
interface"), doesn't it need to dsb like:

  if (csa->iomem)
    dsb(sy);
  isb();

Or am I missing something?

>  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
>

--
Sincerely,
Yeoreum Yun

Re: [PATCH v2 05/28] coresight: etm4x: Ensure context synchronization is not ignored

[Leo Yan]

Hi Levi,

On Wed, Jul 02, 2025 at 12:10:17PM +0100, Yeoreum Yun wrote:

[...]

> > @@ -579,6 +572,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();
> 
> But according to 4.3.7 ("Synchronization when using memory-mapped
> interface"), doesn't it need to dsb like:
> 
>   if (csa->iomem)
>     dsb(sy);
>   isb();
> 
> Or am I missing something?

Section 4.3.7 suggests using a DSB barrier to ensure that writes have
completed in MMIO mode. It also mentions an alternative:

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

In the etm4_{enable|disable}_trace_unit() functions, each time the
TRCPRGCTLR register is written, the driver polls bits in TRCSTATR.
This acts as synchronization using read-after-write (RAW), which is
exactly the approach suggested above.

This is why we don't need DSB() anymore.

Thanks,
Leo

[PATCH v2 06/28] 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")
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 af9d3b2319c5f49ccd40dfa0ccf0f694ce9e2f4f..e3a73718d0c903ee9c72b97028b56565f5ee1053 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -445,13 +445,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 v2 06/28] coresight: etm4x: Add context synchronization before enabling trace

[Yeoreum Yun]

> 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")
> 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 af9d3b2319c5f49ccd40dfa0ccf0f694ce9e2f4f..e3a73718d0c903ee9c72b97028b56565f5ee1053 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -445,13 +445,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();
> +

But, when write to SYS_TRFCR_EL1 in etm4x_allow_trace(), it already does
isb(). Is it redundant?

> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun

Re: [PATCH v2 06/28] coresight: etm4x: Add context synchronization before enabling trace

[Leo Yan]

On Wed, Jul 02, 2025 at 12:05:10PM +0100, Yeoreum Yun wrote:

[...]

> > @@ -445,13 +445,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();
> > +
> 
> But, when write to SYS_TRFCR_EL1 in etm4x_allow_trace(), it already does
> isb(). Is it redundant?

Good point. It is not sufficient. As a system register writing in
kvm_tracing_set_el1_configuration(), this is why adds a ISB here.

Thanks,
Leo

Re: [PATCH v2 06/28] coresight: etm4x: Add context synchronization before enabling trace

[Yeoreum Yun]

> On Wed, Jul 02, 2025 at 12:05:10PM +0100, Yeoreum Yun wrote:
>
> [...]
>
> > > @@ -445,13 +445,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();
> > > +
> >
> > But, when write to SYS_TRFCR_EL1 in etm4x_allow_trace(), it already does
> > isb(). Is it redundant?
>
> Good point. It is not sufficient. As a system register writing in
> kvm_tracing_set_el1_configuration(), this is why adds a ISB here.

Ah, I missed the write_sysreg_s() doesn't include isb().
Sorry to make noise..


--
Sincerely,
Yeoreum Yun

[PATCH v2 07/28] 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")
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 e3a73718d0c903ee9c72b97028b56565f5ee1053..6cd76a2527cf11752963a7cb1b3b0e9a8be241f0 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1881,9 +1881,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);
@@ -1991,9 +2000,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.
@@ -2015,7 +2021,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);
@@ -2099,13 +2104,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 ac649515054d905fa365203bd35f1d839b03292f..4d32605a84ce91eae2101cd83cad5f5317e2638a 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -865,7 +865,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;
@@ -978,7 +977,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
@@ -1033,7 +1031,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

[PATCH v2 08/28] 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).

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 6cd76a2527cf11752963a7cb1b3b0e9a8be241f0..1faf8614366f38eea4909152558fe8e69730bde4 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1981,8 +1981,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
@@ -2000,11 +1998,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;
 }
@@ -2093,8 +2094,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
@@ -2120,7 +2119,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 4d32605a84ce91eae2101cd83cad5f5317e2638a..06438ed1071478cd5c1a143c6ef58a926d6992fb 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -978,7 +978,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.
@@ -1032,7 +1031,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

Re: [PATCH v2 08/28] coresight: etm4x: Remove the state_needs_restore flag

[Yeoreum Yun]

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

> 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).
>
> 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 6cd76a2527cf11752963a7cb1b3b0e9a8be241f0..1faf8614366f38eea4909152558fe8e69730bde4 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1981,8 +1981,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
> @@ -2000,11 +1998,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;
>  }
> @@ -2093,8 +2094,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
> @@ -2120,7 +2119,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 4d32605a84ce91eae2101cd83cad5f5317e2638a..06438ed1071478cd5c1a143c6ef58a926d6992fb 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -978,7 +978,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.
> @@ -1032,7 +1031,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
>

--
Sincerely,
Yeoreum Yun

[PATCH v2 09/28] 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.

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 1faf8614366f38eea4909152558fe8e69730bde4..7bbdee92859cd227fd4077df6ff25470b9d76950 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -486,7 +486,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;
@@ -560,9 +560,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))
@@ -626,7 +628,7 @@ static void etm4_enable_hw_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:
 	/* The tracer didn't start */
@@ -1830,7 +1832,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

[PATCH v2 10/28] 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.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 212 +--------------------
 drivers/hwtracing/coresight/coresight-etm4x.h      |  57 ------
 2 files changed, 5 insertions(+), 264 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 7bbdee92859cd227fd4077df6ff25470b9d76950..6ef1f2ea1a805b07bcac70b6034bab85cec604be 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1851,8 +1851,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;
@@ -1882,98 +1881,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);
-	}
-	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,
@@ -1983,14 +1895,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;
@@ -2014,109 +1918,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);
-	}
-	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)
@@ -2298,13 +2103,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 06438ed1071478cd5c1a143c6ef58a926d6992fb..f361ae62b122d84b5126fca6f8ba93d99dc2f992 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -861,61 +861,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
@@ -977,7 +922,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.
@@ -1030,7 +974,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

[PATCH v2 11/28] coresight: Populate CPU ID into the coresight_device structure

[Leo Yan]

The coresight_desc structure is a temporary structure for passing the
information to the coresight_device structure during registration.

Introduces a new "cpu" field in both structures to store the CPU ID. For
components that are not CPU bound, set this field to -1.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-catu.c       | 1 +
 drivers/hwtracing/coresight/coresight-core.c       | 1 +
 drivers/hwtracing/coresight/coresight-ctcu-core.c  | 1 +
 drivers/hwtracing/coresight/coresight-cti-core.c   | 1 +
 drivers/hwtracing/coresight/coresight-dummy.c      | 1 +
 drivers/hwtracing/coresight/coresight-etb10.c      | 1 +
 drivers/hwtracing/coresight/coresight-etm3x-core.c | 1 +
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 1 +
 drivers/hwtracing/coresight/coresight-funnel.c     | 1 +
 drivers/hwtracing/coresight/coresight-replicator.c | 1 +
 drivers/hwtracing/coresight/coresight-stm.c        | 1 +
 drivers/hwtracing/coresight/coresight-tmc-core.c   | 1 +
 drivers/hwtracing/coresight/coresight-tpda.c       | 1 +
 drivers/hwtracing/coresight/coresight-tpdm.c       | 1 +
 drivers/hwtracing/coresight/coresight-tpiu.c       | 1 +
 drivers/hwtracing/coresight/coresight-trbe.c       | 1 +
 drivers/hwtracing/coresight/ultrasoc-smb.c         | 1 +
 include/linux/coresight.h                          | 4 ++++
 18 files changed, 21 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index 5058432233da1932f1965008fc1b98b560490414..266d1fc2f5a8c572d4a373dc42d752090aa60505 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -565,6 +565,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
 	catu_desc.type = CORESIGHT_DEV_TYPE_HELPER;
 	catu_desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
 	catu_desc.ops = &catu_ops;
+	catu_desc.cpu = -1;
 
 	coresight_clear_self_claim_tag(&catu_desc.access);
 	drvdata->csdev = coresight_register(&catu_desc);
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index fa758cc21827552a5c97b6bdd05d22dec4994b22..1a4bde5d6cd8fcfdd43bcafb45aa4dc5f1dd2d34 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1334,6 +1334,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 	csdev->ops = desc->ops;
 	csdev->access = desc->access;
 	csdev->orphan = true;
+	csdev->cpu = desc->cpu;
 
 	csdev->dev.type = &coresight_dev_type[desc->type];
 	csdev->dev.groups = desc->groups;
diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
index c6bafc96db963310b5e77a095953c83a172cfc7c..f9aaf6f87445a17f7348e7c03bc524834f8e2f20 100644
--- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
+++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
@@ -231,6 +231,7 @@ static int ctcu_probe(struct platform_device *pdev)
 	desc.dev = dev;
 	desc.ops = &ctcu_ops;
 	desc.access = CSDEV_ACCESS_IOMEM(base);
+	desc.cpu = -1;
 
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev)) {
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 8fb30dd73fd20ae613a45b1a467f457a046a9412..2c8bf5dbe8b8206c92ae5ea64a26c947ef5b9582 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -931,6 +931,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
 	cti_desc.ops = &cti_ops;
 	cti_desc.groups = drvdata->ctidev.con_groups;
 	cti_desc.dev = dev;
+	cti_desc.cpu = drvdata->ctidev.cpu;
 
 	coresight_clear_self_claim_tag(&cti_desc.access);
 	drvdata->csdev = coresight_register(&cti_desc);
diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c
index aaa92b5081e3d2bb85d57f90ab68a1dc6a1f0dd8..63c14319ad923ba1444c5f5f9d76ca48e308e8e4 100644
--- a/drivers/hwtracing/coresight/coresight-dummy.c
+++ b/drivers/hwtracing/coresight/coresight-dummy.c
@@ -179,6 +179,7 @@ static int dummy_probe(struct platform_device *pdev)
 
 	desc.pdata = pdev->dev.platform_data;
 	desc.dev = &pdev->dev;
+	desc.dev = -1;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev)) {
 		ret = PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index d5efb085b30d36b51ca591c1b595ef82481f5569..4d59e668d7f6189a4de23064a0a8146a09931b79 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -772,6 +772,7 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.pdata = pdata;
 	desc.dev = dev;
 	desc.groups = coresight_etb_groups;
+	desc.cpu = -1;
 
 	coresight_clear_self_claim_tag(&desc.access);
 	drvdata->csdev = coresight_register(&desc);
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 0f160f2f97344e6a96343cd8658f4f19806193e0..a316e380df263f95c818e8eac44398482847b841 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -892,6 +892,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.pdata = pdata;
 	desc.dev = dev;
 	desc.groups = coresight_etm_groups;
+	desc.cpu = drvdata->cpu;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 6ef1f2ea1a805b07bcac70b6034bab85cec604be..0a585d7e0ab873ea4568e3318b0526b77a2e65e7 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -2059,6 +2059,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg)
 	desc.pdata = pdata;
 	desc.dev = dev;
 	desc.groups = coresight_etmv4_groups;
+	desc.cpu = drvdata->cpu;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index b1922dbe9292b02c91ca5730998e59ecdb08fdc7..3cab9b305ea2cdb1fea3d7b63ddf491897039c18 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -273,6 +273,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
 	desc.ops = &funnel_cs_ops;
 	desc.pdata = pdata;
 	desc.dev = dev;
+	desc.cpu = -1;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev)) {
 		ret = PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 06efd2b01a0f71620eac71166567d14655b58403..1154b7d222381a435f14e217d891377a9bb80a71 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -284,6 +284,7 @@ static int replicator_probe(struct device *dev, struct resource *res)
 	desc.ops = &replicator_cs_ops;
 	desc.pdata = dev->platform_data;
 	desc.dev = dev;
+	desc.cpu = -1;
 
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev)) {
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index e45c6c7204b4491e0f879bc7d5d445aa1d3118be..93f4b4e70eaf1590eecefa5135eeefda0394259d 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -907,6 +907,7 @@ static int __stm_probe(struct device *dev, struct resource *res)
 	desc.pdata = pdata;
 	desc.dev = dev;
 	desc.groups = coresight_stm_groups;
+	desc.cpu = -1;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev)) {
 		ret = PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 88afb16bb6bec395ba535155228d176250f38625..ebfa93b9a86f3d9b90062bce41a052a63836b6c3 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -868,6 +868,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
 	}
 	dev->platform_data = pdata;
 	desc.pdata = pdata;
+	desc.cpu = -1;
 
 	coresight_clear_self_claim_tag(&desc.access);
 	drvdata->csdev = coresight_register(&desc);
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 0633f04beb240b4415cf469b3296f52e0a5b1d05..0ed6b7712feef5e606c8d0aadf594ea8be36059c 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -321,6 +321,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.pdata = adev->dev.platform_data;
 	desc.dev = &adev->dev;
 	desc.access = CSDEV_ACCESS_IOMEM(base);
+	desc.cpu = -1;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index 7214e65097ec9ac69f6c7c9278bcd28d25945c9e..ec1e9a585f90a02f06db27d6dfb22091eb66cbd9 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -1392,6 +1392,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.dev = &adev->dev;
 	desc.access = CSDEV_ACCESS_IOMEM(base);
 	desc.groups = tpdm_attr_grps;
+	desc.cpu = -1;
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
index 3e01592884280819c16398cbb5e09cbaee5d3efb..b83be60a156034e05df52084aebc43897ebee34c 100644
--- a/drivers/hwtracing/coresight/coresight-tpiu.c
+++ b/drivers/hwtracing/coresight/coresight-tpiu.c
@@ -177,6 +177,7 @@ static int __tpiu_probe(struct device *dev, struct resource *res)
 	desc.ops = &tpiu_cs_ops;
 	desc.pdata = pdata;
 	desc.dev = dev;
+	desc.cpu = -1;
 	drvdata->csdev = coresight_register(&desc);
 
 	if (!IS_ERR(drvdata->csdev))
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 8267dd1a2130d37d9507791620ea7bc8cbcd675c..28450447693147b9afd207679832ac83e2a5848a 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -1287,6 +1287,7 @@ static void arm_trbe_register_coresight_cpu(struct trbe_drvdata *drvdata, int cp
 	desc.ops = &arm_trbe_cs_ops;
 	desc.groups = arm_trbe_groups;
 	desc.dev = dev;
+	desc.cpu = cpu;
 	trbe_csdev = coresight_register(&desc);
 	if (IS_ERR(trbe_csdev))
 		goto cpu_clear;
diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
index 26cfc939e5bd810295a336f392ac282ccf316f9f..e5897767d9ae2c21ef39e9280dea211c110bd168 100644
--- a/drivers/hwtracing/coresight/ultrasoc-smb.c
+++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
@@ -483,6 +483,7 @@ static int smb_register_sink(struct platform_device *pdev,
 		return -ENOMEM;
 	}
 	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
+	desc.cpu = -1;
 
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev))
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 5fd3d08824e5a91a197aa01daf0fc392392f3e55..4b15c67c200624fd46a258628dbff401bd1396d7 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -153,6 +153,7 @@ struct csdev_access {
  *		in the component's sysfs sub-directory.
  * @name:	name for the coresight device, also shown under sysfs.
  * @access:	Describe access to the device
+ * @cpu:	The CPU this component is affined to (-1 for not CPU bound).
  */
 struct coresight_desc {
 	enum coresight_dev_type type;
@@ -163,6 +164,7 @@ struct coresight_desc {
 	const struct attribute_group **groups;
 	const char *name;
 	struct csdev_access access;
+	int cpu;
 };
 
 /**
@@ -261,6 +263,7 @@ struct coresight_trace_id_map {
  *		CS_MODE_SYSFS. Otherwise it must be accessed from inside the
  *		spinlock.
  * @orphan:	true if the component has connections that haven't been linked.
+ * @cpu:	The CPU this component is affined to (-1 for not CPU bound).
  * @sysfs_sink_activated: 'true' when a sink has been selected for use via sysfs
  *		by writing a 1 to the 'enable_sink' file.  A sink can be
  *		activated but not yet enabled.  Enabling for a _sink_ happens
@@ -287,6 +290,7 @@ struct coresight_device {
 	atomic_t mode;
 	int refcnt;
 	bool orphan;
+	int cpu;
 	/* sink specific fields */
 	bool sysfs_sink_activated;
 	struct dev_ext_attribute *ea;

-- 
2.34.1

Re: [PATCH v2 11/28] coresight: Populate CPU ID into the coresight_device structure

[kernel test robot]

Hi Leo,

kernel test robot noticed the following build errors:

[auto build test ERROR on 66701750d5565c574af42bef0b789ce0203e3071]

url:    https://github.com/intel-lab-lkp/linux/commits/Leo-Yan/coresight-Change-device-mode-to-atomic-type/20250701-230354
base:   66701750d5565c574af42bef0b789ce0203e3071
patch link:    https://lore.kernel.org/r/20250701-arm_cs_pm_fix_v3-v2-11-23ebb864fcc1%40arm.com
patch subject: [PATCH v2 11/28] coresight: Populate CPU ID into the coresight_device structure
config: arm-randconfig-004-20250702 (https://download.01.org/0day-ci/archive/20250702/202507021422.Zj4Hv0CG-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250702/202507021422.Zj4Hv0CG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507021422.Zj4Hv0CG-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/hwtracing/coresight/coresight-dummy.c:182:11: error: incompatible integer to pointer conversion assigning to 'struct device *' from 'int' [-Wint-conversion]
     182 |         desc.dev = -1;
         |                  ^ ~~
   1 error generated.


vim +182 drivers/hwtracing/coresight/coresight-dummy.c

   113	
   114	static int dummy_probe(struct platform_device *pdev)
   115	{
   116		struct device *dev = &pdev->dev;
   117		struct device_node *node = dev->of_node;
   118		struct coresight_platform_data *pdata;
   119		struct dummy_drvdata *drvdata;
   120		struct coresight_desc desc = { 0 };
   121		int ret = 0, trace_id = 0;
   122	
   123		drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
   124		if (!drvdata)
   125			return -ENOMEM;
   126	
   127		if (of_device_is_compatible(node, "arm,coresight-dummy-source")) {
   128	
   129			desc.name = coresight_alloc_device_name(&source_devs, dev);
   130			if (!desc.name)
   131				return -ENOMEM;
   132	
   133			desc.type = CORESIGHT_DEV_TYPE_SOURCE;
   134			desc.subtype.source_subtype =
   135						CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS;
   136			desc.ops = &dummy_source_cs_ops;
   137			desc.groups = coresight_dummy_groups;
   138	
   139			ret = coresight_get_static_trace_id(dev, &trace_id);
   140			if (!ret) {
   141				/* Get the static id if id is set in device tree. */
   142				ret = coresight_trace_id_get_static_system_id(trace_id);
   143				if (ret < 0) {
   144					dev_err(dev, "Fail to get static id.\n");
   145					return ret;
   146				}
   147			} else {
   148				/* Get next available id if id is not set in device tree. */
   149				trace_id = coresight_trace_id_get_system_id();
   150				if (trace_id < 0) {
   151					ret = trace_id;
   152					return ret;
   153				}
   154			}
   155			drvdata->traceid = (u8)trace_id;
   156	
   157		} else if (of_device_is_compatible(node, "arm,coresight-dummy-sink")) {
   158			desc.name = coresight_alloc_device_name(&sink_devs, dev);
   159			if (!desc.name)
   160				return -ENOMEM;
   161	
   162			desc.type = CORESIGHT_DEV_TYPE_SINK;
   163			desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_DUMMY;
   164			desc.ops = &dummy_sink_cs_ops;
   165		} else {
   166			dev_err(dev, "Device type not set\n");
   167			return -EINVAL;
   168		}
   169	
   170		pdata = coresight_get_platform_data(dev);
   171		if (IS_ERR(pdata)) {
   172			ret = PTR_ERR(pdata);
   173			goto free_id;
   174		}
   175		pdev->dev.platform_data = pdata;
   176	
   177		drvdata->dev = &pdev->dev;
   178		platform_set_drvdata(pdev, drvdata);
   179	
   180		desc.pdata = pdev->dev.platform_data;
   181		desc.dev = &pdev->dev;
 > 182		desc.dev = -1;
   183		drvdata->csdev = coresight_register(&desc);
   184		if (IS_ERR(drvdata->csdev)) {
   185			ret = PTR_ERR(drvdata->csdev);
   186			goto free_id;
   187		}
   188	
   189		pm_runtime_enable(dev);
   190		dev_dbg(dev, "Dummy device initialized\n");
   191	
   192		ret = 0;
   193		goto out;
   194	
   195	free_id:
   196		if (IS_VALID_CS_TRACE_ID(drvdata->traceid))
   197			coresight_trace_id_put_system_id(drvdata->traceid);
   198	
   199	out:
   200		return ret;
   201	}
   202	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 12/28] coresight: sysfs: Validate CPU online status for per-CPU sources

[Leo Yan]

The current SysFS flow first enables the links and sink, then rolls back
to disable them if the source fails to enable. This failure can occur if
the associated CPU is offline, which causes the SMP call to fail.

Validate whether the associated CPU is online for a per-CPU tracer. If
the CPU is offline, return -ENODEV and bail out.

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

diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index feadaf065b5318c8426aa53420b8e5f67a258683..14ee15297b98115122068cbe932f0b2ce004b77e 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -161,6 +161,9 @@ static int coresight_validate_source_sysfs(struct coresight_device *csdev,
 		return -EINVAL;
 	}
 
+	if (coresight_is_percpu_source(csdev) && !cpu_online(csdev->cpu))
+		return -ENODEV;
+
 	return 0;
 }
 

-- 
2.34.1

Re: [PATCH v2 12/28] coresight: sysfs: Validate CPU online status for per-CPU sources

[Yeoreum Yun]

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

> The current SysFS flow first enables the links and sink, then rolls back
> to disable them if the source fails to enable. This failure can occur if
> the associated CPU is offline, which causes the SMP call to fail.
>
> Validate whether the associated CPU is online for a per-CPU tracer. If
> the CPU is offline, return -ENODEV and bail out.
>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-sysfs.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index feadaf065b5318c8426aa53420b8e5f67a258683..14ee15297b98115122068cbe932f0b2ce004b77e 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -161,6 +161,9 @@ static int coresight_validate_source_sysfs(struct coresight_device *csdev,
>  		return -EINVAL;
>  	}
>
> +	if (coresight_is_percpu_source(csdev) && !cpu_online(csdev->cpu))
> +		return -ENODEV;
> +
>  	return 0;
>  }
>
>
> --
> 2.34.1
>

--
Sincerely,
Yeoreum Yun

[PATCH v2 13/28] coresight: Set per CPU source pointer

[Leo Yan]

Introduce coresight_set_percpu_source() for setting CPU source device. The
sources are maintained in a per CPU structure 'csdev_source'.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 1a4bde5d6cd8fcfdd43bcafb45aa4dc5f1dd2d34..042e8e646ff521a15dc18d7264faa427f1ac0dc7 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -32,6 +32,7 @@
  */
 DEFINE_MUTEX(coresight_mutex);
 static DEFINE_PER_CPU(struct coresight_device *, csdev_sink);
+static DEFINE_PER_CPU(struct coresight_device *, csdev_source);
 
 /**
  * struct coresight_node - elements of a path, from source to sink
@@ -77,6 +78,11 @@ struct coresight_device *coresight_get_percpu_sink(int cpu)
 }
 EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
 
+static void coresight_set_percpu_source(int cpu, struct coresight_device *csdev)
+{
+	per_cpu(csdev_source, cpu) = csdev;
+}
+
 static struct coresight_device *coresight_get_source(struct coresight_path *path)
 {
 	struct coresight_device *csdev;
@@ -1390,6 +1396,10 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 			goto out_unlock;
 		}
 	}
+
+	if (coresight_is_percpu_source(csdev))
+		coresight_set_percpu_source(csdev->cpu, csdev);
+
 	/* Device is now registered */
 	registered = true;
 
@@ -1421,6 +1431,8 @@ EXPORT_SYMBOL_GPL(coresight_register);
 
 void coresight_unregister(struct coresight_device *csdev)
 {
+	if (coresight_is_percpu_source(csdev))
+		coresight_set_percpu_source(csdev->cpu, NULL);
 	etm_perf_del_symlink_sink(csdev);
 	/* Remove references of that device in the topology */
 	if (cti_assoc_ops && cti_assoc_ops->remove)

-- 
2.34.1

[PATCH v2 14/28] coresight: Register CPU PM notifier in core layer

[Leo Yan]

The current implementation only saves and restores the context for ETM
sources while ignoring the context of links.  However, if funnels or
replicators on a linked path resides in a CPU or cluster power domain,
the hardware context for the link will be lost after resuming from low
power states.

To support context management for links during CPU low power modes, a
better way is to implement CPU PM callbacks in the Arm CoreSight core
layer.  As the core layer has sufficient information for linked paths,
from tracers to links, which can be used for power management.

As a first step, this patch registers CPU PM notifier in the core layer.
If a source driver provides callbacks for saving and restoring context,
these callbacks will be invoked in CPU suspend and resume.

Further changes will extend for controlling path.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 62 ++++++++++++++++++++++++++++
 include/linux/coresight.h                    |  4 ++
 2 files changed, 66 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 042e8e646ff521a15dc18d7264faa427f1ac0dc7..233e128bc61c9e6ef69182e5387fe0dadf532324 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/build_bug.h>
+#include <linux/cpu_pm.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -420,6 +421,21 @@ int coresight_resume_source(struct coresight_device *csdev)
 }
 EXPORT_SYMBOL_GPL(coresight_resume_source);
 
+static int coresight_save_source(struct coresight_device *csdev)
+{
+	if (csdev && source_ops(csdev)->save)
+		return source_ops(csdev)->save(csdev);
+
+	/* Return success if callback is not supported */
+	return 0;
+}
+
+static void coresight_restore_source(struct coresight_device *csdev)
+{
+	if (csdev && source_ops(csdev)->restore)
+		source_ops(csdev)->restore(csdev);
+}
+
 /*
  * coresight_disable_path_from : Disable components in the given path beyond
  * @nd in the list. If @nd is NULL, all the components, except the SOURCE are
@@ -1572,6 +1588,45 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
 }
 EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
 
+static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
+				   void *v)
+{
+	unsigned int cpu = smp_processor_id();
+	struct coresight_device *source = per_cpu(csdev_source, cpu);
+
+	if (!source)
+		return NOTIFY_OK;
+
+	switch (cmd) {
+	case CPU_PM_ENTER:
+		if (coresight_save_source(source))
+			return NOTIFY_BAD;
+		break;
+	case CPU_PM_EXIT:
+	case CPU_PM_ENTER_FAILED:
+		coresight_restore_source(source);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block coresight_cpu_pm_nb = {
+	.notifier_call = coresight_cpu_pm_notify,
+};
+
+static int __init coresight_pm_setup(void)
+{
+	return cpu_pm_register_notifier(&coresight_cpu_pm_nb);
+}
+
+static void coresight_pm_cleanup(void)
+{
+	cpu_pm_unregister_notifier(&coresight_cpu_pm_nb);
+}
+
 const struct bus_type coresight_bustype = {
 	.name	= "coresight",
 };
@@ -1626,9 +1681,15 @@ static int __init coresight_init(void)
 
 	/* initialise the coresight syscfg API */
 	ret = cscfg_init();
+	if (ret)
+		goto exit_notifier;
+
+	ret = coresight_pm_setup();
 	if (!ret)
 		return 0;
 
+	cscfg_exit();
+exit_notifier:
 	atomic_notifier_chain_unregister(&panic_notifier_list,
 					     &coresight_notifier);
 exit_perf:
@@ -1640,6 +1701,7 @@ static int __init coresight_init(void)
 
 static void __exit coresight_exit(void)
 {
+	coresight_pm_cleanup();
 	cscfg_exit();
 	atomic_notifier_chain_unregister(&panic_notifier_list,
 					     &coresight_notifier);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 4b15c67c200624fd46a258628dbff401bd1396d7..8d6f7cb354d6e487c757d9fe86cda895ccb1a588 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -400,6 +400,8 @@ struct coresight_ops_link {
  * @disable:	disables tracing for a source.
  * @resume_perf: resumes tracing for a source in perf session.
  * @pause_perf:	pauses tracing for a source in perf session.
+ * @save:	save context for a source.
+ * @restore:	restore context for a source.
  */
 struct coresight_ops_source {
 	int (*cpu_id)(struct coresight_device *csdev);
@@ -409,6 +411,8 @@ struct coresight_ops_source {
 			struct perf_event *event);
 	int (*resume_perf)(struct coresight_device *csdev);
 	void (*pause_perf)(struct coresight_device *csdev);
+	int (*save)(struct coresight_device *csdev);
+	void (*restore)(struct coresight_device *csdev);
 };
 
 /**

-- 
2.34.1

[PATCH v2 15/28] coresight: etm4x: Hook CPU PM callbacks

[Leo Yan]

Since the CoreSight core layer has registered CPU PM notifiers, this
patch hooks CPU save and restore callbacks to be invoked from the core
layer.

The CPU PM notifier in the ETMv4 driver is no longer needed, remove it
along with its registration and unregistration code.

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

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 0a585d7e0ab873ea4568e3318b0526b77a2e65e7..b76107aef9d3e42214d4bc97598795ddd96ff2d8 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1184,19 +1184,6 @@ static void etm4_pause_perf(struct coresight_device *csdev)
 	drvdata->paused = true;
 }
 
-static const struct coresight_ops_source etm4_source_ops = {
-	.cpu_id		= etm4_cpu_id,
-	.enable		= etm4_enable,
-	.disable	= etm4_disable,
-	.resume_perf	= etm4_resume_perf,
-	.pause_perf	= etm4_pause_perf,
-};
-
-static const struct coresight_ops etm4_cs_ops = {
-	.trace_id	= coresight_etm_get_trace_id,
-	.source_ops	= &etm4_source_ops,
-};
-
 static bool cpu_supports_sysreg_trace(void)
 {
 	u64 dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
@@ -1900,8 +1887,9 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 	return ret;
 }
 
-static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
+static int etm4_cpu_save(struct coresight_device *csdev)
 {
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 	int ret = 0;
 
 	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
@@ -1924,8 +1912,10 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	etm4_enable_hw(drvdata, false);
 }
 
-static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
+static void etm4_cpu_restore(struct coresight_device *csdev)
 {
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
 	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
 		return;
 
@@ -1933,38 +1923,19 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 		__etm4_cpu_restore(drvdata);
 }
 
-static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
-			      void *v)
-{
-	struct etmv4_drvdata *drvdata;
-	unsigned int cpu = smp_processor_id();
-
-	if (!etmdrvdata[cpu])
-		return NOTIFY_OK;
-
-	drvdata = etmdrvdata[cpu];
-
-	if (WARN_ON_ONCE(drvdata->cpu != cpu))
-		return NOTIFY_BAD;
-
-	switch (cmd) {
-	case CPU_PM_ENTER:
-		if (etm4_cpu_save(drvdata))
-			return NOTIFY_BAD;
-		break;
-	case CPU_PM_EXIT:
-	case CPU_PM_ENTER_FAILED:
-		etm4_cpu_restore(drvdata);
-		break;
-	default:
-		return NOTIFY_DONE;
-	}
-
-	return NOTIFY_OK;
-}
+static const struct coresight_ops_source etm4_source_ops = {
+	.cpu_id		= etm4_cpu_id,
+	.enable		= etm4_enable,
+	.disable	= etm4_disable,
+	.resume_perf	= etm4_resume_perf,
+	.pause_perf	= etm4_pause_perf,
+	.save		= etm4_cpu_save,
+	.restore	= etm4_cpu_restore,
+};
 
-static struct notifier_block etm4_cpu_pm_nb = {
-	.notifier_call = etm4_cpu_pm_notify,
+static const struct coresight_ops etm4_cs_ops = {
+	.trace_id	= coresight_etm_get_trace_id,
+	.source_ops	= &etm4_source_ops,
 };
 
 /* Setup PM. Deals with error conditions and counts */
@@ -1972,16 +1943,12 @@ static int __init etm4_pm_setup(void)
 {
 	int ret;
 
-	ret = cpu_pm_register_notifier(&etm4_cpu_pm_nb);
-	if (ret)
-		return ret;
-
 	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
 					"arm/coresight4:starting",
 					etm4_starting_cpu, etm4_dying_cpu);
 
 	if (ret)
-		goto unregister_notifier;
+		return ret;
 
 	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
 					"arm/coresight4:online",
@@ -1995,15 +1962,11 @@ static int __init etm4_pm_setup(void)
 
 	/* failed dyn state - remove others */
 	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
-
-unregister_notifier:
-	cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
 	return ret;
 }
 
 static void etm4_pm_clear(void)
 {
-	cpu_pm_unregister_notifier(&etm4_cpu_pm_nb);
 	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	if (hp_online) {
 		cpuhp_remove_state_nocalls(hp_online);

-- 
2.34.1

[PATCH v2 16/28] coresight: Add callback to determine if context save/restore is needed

[Leo Yan]

Add a callback in the source device that returns a boolean indicating
whether context save and restore operations are required. The save and
restore flow is skipped if the callback returns false.

The ETMv4 driver implements its own version's callback.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c       | 16 +++++++++++-
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 30 +++++++++++++++++-----
 include/linux/coresight.h                          |  2 ++
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 233e128bc61c9e6ef69182e5387fe0dadf532324..45f15a1f51295b298073551cb328d20cc2c36cdb 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -421,6 +421,20 @@ int coresight_resume_source(struct coresight_device *csdev)
 }
 EXPORT_SYMBOL_GPL(coresight_resume_source);
 
+static bool coresight_need_save_restore_source(struct coresight_device *csdev)
+{
+	if (!csdev)
+		return false;
+
+	if (source_ops(csdev)->need_context_save_restore)
+		return source_ops(csdev)->need_context_save_restore(csdev);
+
+	if (coresight_get_mode(csdev))
+		return true;
+
+	return false;
+}
+
 static int coresight_save_source(struct coresight_device *csdev)
 {
 	if (csdev && source_ops(csdev)->save)
@@ -1594,7 +1608,7 @@ static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 	unsigned int cpu = smp_processor_id();
 	struct coresight_device *source = per_cpu(csdev_source, cpu);
 
-	if (!source)
+	if (!coresight_need_save_restore_source(source))
 		return NOTIFY_OK;
 
 	switch (cmd) {
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index b76107aef9d3e42214d4bc97598795ddd96ff2d8..92ded5c87614b2a74736005c4d127e266cb9b0e2 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1836,6 +1836,21 @@ static int etm4_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
+static bool etm4_need_context_save_restore(struct coresight_device *csdev)
+{
+	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
+		return false;
+
+	/*
+	 * Save and restore the ETM Trace registers only if
+	 * the ETM is active.
+	 */
+	if (coresight_get_mode(csdev))
+		return true;
+
+	return false;
+}
+
 static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 {
 	int ret = 0;
@@ -1924,13 +1939,14 @@ static void etm4_cpu_restore(struct coresight_device *csdev)
 }
 
 static const struct coresight_ops_source etm4_source_ops = {
-	.cpu_id		= etm4_cpu_id,
-	.enable		= etm4_enable,
-	.disable	= etm4_disable,
-	.resume_perf	= etm4_resume_perf,
-	.pause_perf	= etm4_pause_perf,
-	.save		= etm4_cpu_save,
-	.restore	= etm4_cpu_restore,
+	.cpu_id			   = etm4_cpu_id,
+	.enable			   = etm4_enable,
+	.disable		   = etm4_disable,
+	.resume_perf		   = etm4_resume_perf,
+	.pause_perf		   = etm4_pause_perf,
+	.save			   = etm4_cpu_save,
+	.restore		   = etm4_cpu_restore,
+	.need_context_save_restore = etm4_need_context_save_restore,
 };
 
 static const struct coresight_ops etm4_cs_ops = {
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 8d6f7cb354d6e487c757d9fe86cda895ccb1a588..4d5c07b7ddc67991a3871851fe45463f92bd32c8 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -400,6 +400,7 @@ struct coresight_ops_link {
  * @disable:	disables tracing for a source.
  * @resume_perf: resumes tracing for a source in perf session.
  * @pause_perf:	pauses tracing for a source in perf session.
+ * @need_context_save_restore: if need to save and restore context.
  * @save:	save context for a source.
  * @restore:	restore context for a source.
  */
@@ -411,6 +412,7 @@ struct coresight_ops_source {
 			struct perf_event *event);
 	int (*resume_perf)(struct coresight_device *csdev);
 	void (*pause_perf)(struct coresight_device *csdev);
+	bool (*need_context_save_restore)(struct coresight_device *csdev);
 	int (*save)(struct coresight_device *csdev);
 	void (*restore)(struct coresight_device *csdev);
 };

-- 
2.34.1

[PATCH v2 17/28] coresight: etm4x: Remove redundant condition checks in save and restore

[Leo Yan]

Since a dedicated callback is now used to determine whether context save
and restore operations are needed, performing the same check within the
save and restore callbacks is redundant.

The save and restore flows currently use two-level functions: the first
level handles the condition check, while the second level performs the
low level operations. As the checks are no longer necessary, simplify
the logic into single-level functions.

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

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 92ded5c87614b2a74736005c4d127e266cb9b0e2..d71ac055c3d0b74053279a86ede7e5592f2b2909 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1851,10 +1851,10 @@ static bool etm4_need_context_save_restore(struct coresight_device *csdev)
 	return false;
 }
 
-static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
+static int etm4_cpu_save(struct coresight_device *csdev)
 {
 	int ret = 0;
-	struct coresight_device *csdev = drvdata->csdev;
+	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 	struct csdev_access *csa;
 	struct device *etm_dev;
 
@@ -1902,40 +1902,11 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 	return ret;
 }
 
-static int etm4_cpu_save(struct coresight_device *csdev)
-{
-	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-	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))
-		ret = __etm4_cpu_save(drvdata);
-	return ret;
-}
-
-static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
-{
-	if (WARN_ON(!drvdata->csdev))
-		return;
-
-	etm4_enable_hw(drvdata, false);
-}
-
 static void etm4_cpu_restore(struct coresight_device *csdev)
 {
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
-		return;
-
-	if (coresight_get_mode(drvdata->csdev))
-		__etm4_cpu_restore(drvdata);
+	etm4_enable_hw(drvdata, false);
 }
 
 static const struct coresight_ops_source etm4_source_ops = {

-- 
2.34.1

[PATCH v2 18/28] coresight: cti: Fix race condition by using device mode

[Leo Yan]

A CTI device can be enabled as a helper in a CoreSight path or directly
via the SysFS knob. Currently, the driver lacks protection when multiple
flows contend for access to the same CTI device.

Use the device mode to mark the CTI as in use, and reset it to the
DISABLED state when the device is no longer needed. Any conflict will
cause the attempt to acquire the mode to fail, ensuring the device can
only be enabled exclusively.

Fixes: 1b5b1646e63d ("coresight: Fix CTI module refcount leak by making it a helper device")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-core.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 2c8bf5dbe8b8206c92ae5ea64a26c947ef5b9582..ba4635dfc2e30b4e9ae4972f91bdc6647975b719 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -87,20 +87,31 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
 }
 
 /* write regs to hardware and enable */
-static int cti_enable_hw(struct cti_drvdata *drvdata)
+static int cti_enable_hw(struct cti_drvdata *drvdata, enum cs_mode mode)
 {
 	struct cti_config *config = &drvdata->config;
+	struct coresight_device	*csdev = drvdata->csdev;
 	unsigned long flags;
 	int rc = 0;
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 
+	if (!drvdata->config.enable_req_count) {
+		coresight_set_mode(csdev, mode);
+	} else {
+		/* The device has been configured with an incompatible mode */
+		if (coresight_get_mode(csdev) != mode) {
+			rc = -EBUSY;
+			goto cti_err_not_enabled;
+		}
+	}
+
 	/* no need to do anything if enabled or unpowered*/
 	if (config->hw_enabled || !config->hw_powered)
 		goto cti_state_unchanged;
 
 	/* claim the device */
-	rc = coresight_claim_device(drvdata->csdev);
+	rc = coresight_claim_device(csdev);
 	if (rc)
 		goto cti_err_not_enabled;
 
@@ -116,6 +127,8 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
 
 	/* cannot enable due to error */
 cti_err_not_enabled:
+	if (!drvdata->config.enable_req_count)
+		coresight_set_mode(csdev, CS_MODE_DISABLED);
 	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 	return rc;
 }
@@ -165,6 +178,8 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
 	if (--drvdata->config.enable_req_count > 0)
 		goto cti_not_disabled;
 
+	coresight_set_mode(csdev, CS_MODE_DISABLED);
+
 	/* no need to do anything if disabled or cpu unpowered */
 	if (!config->hw_enabled || !config->hw_powered)
 		goto cti_not_disabled;
@@ -803,7 +818,7 @@ int cti_enable(struct coresight_device *csdev, enum cs_mode mode, void *data)
 {
 	struct cti_drvdata *drvdata = csdev_to_cti_drvdata(csdev);
 
-	return cti_enable_hw(drvdata);
+	return cti_enable_hw(drvdata, mode);
 }
 
 int cti_disable(struct coresight_device *csdev, void *data)

-- 
2.34.1

[PATCH v2 19/28] coresight: cti: Introduce CS_MODE_DEBUG mode

[Leo Yan]

Introduce a new CS_MODE_DEBUG mode to indicate that a device is being
used for debugging purposes, e.g, if a system enables CTI for debugging
but not for hardware trace

Update ETM4x driver to mute compiler warnings for the newly added mode.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-cti-sysfs.c  | 2 +-
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 ++
 include/linux/coresight.h                          | 9 +++++----
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 572b80ee96fbf18ec8cf9abc30d109a676dfbc5d..3e0039f75f5aad104cb86561bf37971c7dc4e408 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -112,7 +112,7 @@ static ssize_t enable_store(struct device *dev,
 		ret = pm_runtime_resume_and_get(dev->parent);
 		if (ret)
 			return ret;
-		ret = cti_enable(drvdata->csdev, CS_MODE_SYSFS, NULL);
+		ret = cti_enable(drvdata->csdev, CS_MODE_DEBUG, NULL);
 		if (ret)
 			pm_runtime_put(dev->parent);
 	} else {
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index d71ac055c3d0b74053279a86ede7e5592f2b2909..93ae8590ae2459db317f6367b6cffbf658b0e2f2 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -932,6 +932,7 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
 	case CS_MODE_PERF:
 		ret = etm4_enable_perf(csdev, event, path);
 		break;
+	case CS_MODE_DEBUG:
 	default:
 		ret = -EINVAL;
 	}
@@ -1130,6 +1131,7 @@ static void etm4_disable(struct coresight_device *csdev,
 	mode = coresight_get_mode(csdev);
 
 	switch (mode) {
+	case CS_MODE_DEBUG:
 	case CS_MODE_DISABLED:
 		break;
 	case CS_MODE_SYSFS:
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 4d5c07b7ddc67991a3871851fe45463f92bd32c8..f52e834640b72534ea83ab223aae7544b195bbaa 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -253,10 +253,10 @@ 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:	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
+ * @mode:	The device mode, i.e sysFS, Perf, debug 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 ==
@@ -344,6 +344,7 @@ enum cs_mode {
 	CS_MODE_DISABLED,
 	CS_MODE_SYSFS,
 	CS_MODE_PERF,
+	CS_MODE_DEBUG,
 };
 
 #define coresight_ops(csdev)	csdev->ops

-- 
2.34.1

[PATCH v2 20/28] coresight: cti: Properly handle modes in CPU PM notifiers

[Leo Yan]

This commit distinguishes CPU PM flows based on the mode.

The CTI driver retains its existing behavior for the CS_MODE_DEBUG mode.

For modes other than DEBUG, a future change will be made to manage CTI
devices by iterating through the CoreSight path in the core layer. To
avoid conflicts, the CTI driver no longer controls CTI hardware in CPU
PM notifiers for non DEBUG modes.

However, the CTI driver continues to update the hw_powered flag for all
modes to reflect the device's power state.

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

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index ba4635dfc2e30b4e9ae4972f91bdc6647975b719..c91d339d7ccbf71db71cb7156f9a6f2961ae2198 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -141,6 +141,9 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata)
 	raw_spin_lock(&drvdata->spinlock);
 	config->hw_powered = true;
 
+	if (coresight_get_mode(drvdata->csdev) != CS_MODE_DEBUG)
+		goto cti_hp_not_enabled;
+
 	/* no need to do anything if no enable request */
 	if (!drvdata->config.enable_req_count)
 		goto cti_hp_not_enabled;
@@ -697,21 +700,27 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 	case CPU_PM_ENTER:
 		/* CTI regs all static - we have a copy & nothing to save */
 		drvdata->config.hw_powered = false;
-		if (drvdata->config.hw_enabled)
+		if ((coresight_get_mode(drvdata->csdev) == CS_MODE_DEBUG) &&
+		    drvdata->config.hw_enabled)
 			coresight_disclaim_device(csdev);
 		break;
 
 	case CPU_PM_ENTER_FAILED:
 		drvdata->config.hw_powered = true;
-		if (drvdata->config.hw_enabled) {
+		if ((coresight_get_mode(drvdata->csdev) == CS_MODE_DEBUG) &&
+		    drvdata->config.hw_enabled) {
 			if (coresight_claim_device(csdev))
 				drvdata->config.hw_enabled = false;
 		}
 		break;
 
 	case CPU_PM_EXIT:
-		/* write hardware registers to re-enable. */
 		drvdata->config.hw_powered = true;
+
+		if (coresight_get_mode(drvdata->csdev) != CS_MODE_DEBUG)
+			break;
+
+		/* write hardware registers to re-enable. */
 		drvdata->config.hw_enabled = false;
 
 		/* check enable reference count to enable HW */
@@ -760,7 +769,8 @@ static int cti_dying_cpu(unsigned int cpu)
 
 	raw_spin_lock(&drvdata->spinlock);
 	drvdata->config.hw_powered = false;
-	if (drvdata->config.hw_enabled)
+	if ((coresight_get_mode(drvdata->csdev) == CS_MODE_DEBUG) &&
+	    drvdata->config.hw_enabled)
 		coresight_disclaim_device(drvdata->csdev);
 	raw_spin_unlock(&drvdata->spinlock);
 	return 0;

-- 
2.34.1

[PATCH v2 21/28] coresight: Add per-CPU path pointer

[Leo Yan]

Introduce per-CPU path pointers, save activated paths into the
structure. This will be used by later changes for controlling the path
during CPU idle.

The path pointer is assigned before setting the source device mode to
active, and it is cleared after the device is changed to an inactive
mode. So safe access to path pointers is guaranteed when the device is
in an active mode.

The path enabling has been refactored: a new _coresight_enable_path()
function is introduced to enable components in the path. The existing
coresight_enable_path() function now calls this new helper and updates
the per-CPU pointers accordingly.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 45f15a1f51295b298073551cb328d20cc2c36cdb..ef2a0de366d9b12b4f609027c8b67f690cf84558 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -35,6 +35,27 @@ DEFINE_MUTEX(coresight_mutex);
 static DEFINE_PER_CPU(struct coresight_device *, csdev_sink);
 static DEFINE_PER_CPU(struct coresight_device *, csdev_source);
 
+/*
+ * Activated path pointer for a per-CPU source device. When enabling a path,
+ * the path pointer is first assigned, followed by a synchronous SMP call on
+ * the target CPU to transition the device mode from DISABLED to an enabled
+ * state. Conversely, during the disable flow, an SMP call on the target CPU
+ * transitions the device mode to DISABLED, after which the path pointer is
+ * cleared.
+ *
+ *  per_cpu(csdev_cpu_path, csdev->cpu) = path
+ *  coresight_take_mode(csdev, CS_MODE_SYSFS or CS_MODE_PERF)
+ *
+ *  // Safe to access per_cpu(csdev_cpu_path, cpu);
+ *
+ *  coresight_set_mode(csdev, CS_MODE_DISABLED)
+ *  per_cpu(csdev_cpu_path, csdev->cpu) = NULL
+ *
+ * As a result, the device mode is used to determine whether it is safe
+ * to access the path pointer.
+ */
+static DEFINE_PER_CPU(struct coresight_path *, csdev_cpu_path);
+
 /**
  * struct coresight_node - elements of a path, from source to sink
  * @csdev:	Address of an element.
@@ -508,6 +529,12 @@ static void coresight_disable_path_from(struct coresight_path *path,
 
 void coresight_disable_path(struct coresight_path *path)
 {
+	struct coresight_device *source;
+
+	source = coresight_get_source(path);
+	if (coresight_is_percpu_source(source))
+		per_cpu(csdev_cpu_path, source->cpu) = NULL;
+
 	coresight_disable_path_from(path, NULL);
 }
 EXPORT_SYMBOL_GPL(coresight_disable_path);
@@ -531,8 +558,8 @@ static int coresight_enable_helpers(struct coresight_device *csdev,
 	return 0;
 }
 
-int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
-			  void *sink_data)
+static int _coresight_enable_path(struct coresight_path *path,
+				  enum cs_mode mode, void *sink_data)
 {
 	int ret = 0;
 	u32 type;
@@ -599,6 +626,23 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
 	goto out;
 }
 
+int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
+			  void *sink_data)
+{
+	int ret;
+	struct coresight_device *source;
+
+	ret = _coresight_enable_path(path, mode, sink_data);
+	if (ret)
+		return ret;
+
+	source = coresight_get_source(path);
+	if (coresight_is_percpu_source(source))
+		per_cpu(csdev_cpu_path, source->cpu) = path;
+
+	return 0;
+}
+
 struct coresight_device *coresight_get_sink(struct coresight_path *path)
 {
 	struct coresight_device *csdev;

-- 
2.34.1

[PATCH v2 22/28] coresight: Add 'in_idle' argument to path enable/disable functions

[Leo Yan]

Introduce an in_idle argument to the path enable and disable functions.
When set to true, it skips to touch the sink device. To avoid invoking
sink related helpers, the condition check is moved before the enable
helpers are called.

This is a preparation for managing the path during CPU idle.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index ef2a0de366d9b12b4f609027c8b67f690cf84558..b1c122d1c4164e3ca6f1aaad0bd24917032626be 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -477,7 +477,8 @@ static void coresight_restore_source(struct coresight_device *csdev)
  * disabled.
  */
 static void coresight_disable_path_from(struct coresight_path *path,
-					struct coresight_node *nd)
+					struct coresight_node *nd,
+					bool in_idle)
 {
 	u32 type;
 	struct coresight_device *csdev, *parent, *child;
@@ -500,6 +501,10 @@ static void coresight_disable_path_from(struct coresight_path *path,
 						CORESIGHT_DEV_TYPE_SINK :
 						CORESIGHT_DEV_TYPE_LINK;
 
+		/* To reduce latency, CPU idle does not touch the sink */
+		if (in_idle && type == CORESIGHT_DEV_TYPE_SINK)
+			continue;
+
 		switch (type) {
 		case CORESIGHT_DEV_TYPE_SINK:
 			coresight_disable_sink(csdev);
@@ -535,7 +540,7 @@ void coresight_disable_path(struct coresight_path *path)
 	if (coresight_is_percpu_source(source))
 		per_cpu(csdev_cpu_path, source->cpu) = NULL;
 
-	coresight_disable_path_from(path, NULL);
+	coresight_disable_path_from(path, NULL, false);
 }
 EXPORT_SYMBOL_GPL(coresight_disable_path);
 
@@ -559,7 +564,8 @@ static int coresight_enable_helpers(struct coresight_device *csdev,
 }
 
 static int _coresight_enable_path(struct coresight_path *path,
-				  enum cs_mode mode, void *sink_data)
+				  enum cs_mode mode, void *sink_data,
+				  bool in_idle)
 {
 	int ret = 0;
 	u32 type;
@@ -572,10 +578,6 @@ static int _coresight_enable_path(struct coresight_path *path,
 		csdev = nd->csdev;
 		type = csdev->type;
 
-		/* Enable all helpers adjacent to the path first */
-		ret = coresight_enable_helpers(csdev, mode, path);
-		if (ret)
-			goto err_disable_path;
 		/*
 		 * ETF devices are tricky... They can be a link or a sink,
 		 * depending on how they are configured.  If an ETF has been
@@ -587,6 +589,15 @@ static int _coresight_enable_path(struct coresight_path *path,
 						CORESIGHT_DEV_TYPE_SINK :
 						CORESIGHT_DEV_TYPE_LINK;
 
+		/* To reduce latency, CPU idle does not touch the sink */
+		if (in_idle && type == CORESIGHT_DEV_TYPE_SINK)
+			continue;
+
+		/* Enable all helpers adjacent to the path first */
+		ret = coresight_enable_helpers(csdev, mode, path);
+		if (ret)
+			goto err_disable_path;
+
 		switch (type) {
 		case CORESIGHT_DEV_TYPE_SINK:
 			ret = coresight_enable_sink(csdev, mode, sink_data);
@@ -622,7 +633,7 @@ static int _coresight_enable_path(struct coresight_path *path,
 err_disable_helpers:
 	coresight_disable_helpers(csdev, path);
 err_disable_path:
-	coresight_disable_path_from(path, nd);
+	coresight_disable_path_from(path, nd, false);
 	goto out;
 }
 
@@ -632,7 +643,7 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
 	int ret;
 	struct coresight_device *source;
 
-	ret = _coresight_enable_path(path, mode, sink_data);
+	ret = _coresight_enable_path(path, mode, sink_data, false);
 	if (ret)
 		return ret;
 

-- 
2.34.1

[PATCH v2 23/28] coresight: Control path during CPU idle

[Leo Yan]

Control links and helpers on an activated path during CPU idle. Since
coresight_disable_path_from() does not handle a source device's
helpers, explicitly disable them alongside the source device.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index b1c122d1c4164e3ca6f1aaad0bd24917032626be..7693a0eade1a8de6d0960d66f6de682b5d5aff17 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -458,8 +458,15 @@ static bool coresight_need_save_restore_source(struct coresight_device *csdev)
 
 static int coresight_save_source(struct coresight_device *csdev)
 {
-	if (csdev && source_ops(csdev)->save)
-		return source_ops(csdev)->save(csdev);
+	int ret;
+
+	if (csdev && source_ops(csdev)->save) {
+		ret = source_ops(csdev)->save(csdev);
+		if (ret)
+			return ret;
+
+		coresight_disable_helpers(csdev, NULL);
+	}
 
 	/* Return success if callback is not supported */
 	return 0;
@@ -1662,17 +1669,33 @@ static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 {
 	unsigned int cpu = smp_processor_id();
 	struct coresight_device *source = per_cpu(csdev_source, cpu);
+	struct coresight_path *path;
 
 	if (!coresight_need_save_restore_source(source))
 		return NOTIFY_OK;
 
+	/*
+	 * When run at here, the source device mode is enabled.
+	 * The activated path pointer must not be NULL.
+	 */
+	path = per_cpu(csdev_cpu_path, source->cpu);
+	if (WARN_ON(!path))
+		return NOTIFY_BAD;
+
 	switch (cmd) {
 	case CPU_PM_ENTER:
 		if (coresight_save_source(source))
 			return NOTIFY_BAD;
+
+		coresight_disable_path_from(path, NULL, true);
 		break;
 	case CPU_PM_EXIT:
 	case CPU_PM_ENTER_FAILED:
+		if (_coresight_enable_path(path,
+					   coresight_get_mode(source),
+					   NULL, true))
+			return NOTIFY_BAD;
+
 		coresight_restore_source(source);
 		break;
 	default:

-- 
2.34.1

[PATCH v2 24/28] coresight: Add PM callbacks for percpu sink

[Leo Yan]

Unlike a system level's sink, the per-CPU sink may lose power during CPU
idle states. Currently, this refers specifically to TRBE as the sink.
This commit registers save and restore callbacks for the per-CPU sink
via the PM notifier.

There are no changes to the coresight_enable_helpers() function; the
code movement is solely for compilation purposes.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 101 +++++++++++++++++++++------
 include/linux/coresight.h                    |   4 ++
 2 files changed, 82 insertions(+), 23 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 7693a0eade1a8de6d0960d66f6de682b5d5aff17..9978737d21177ab7cfcd449cf67a0b0736fcca5a 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -391,6 +391,25 @@ static void coresight_disable_helper(struct coresight_device *csdev, void *data)
 	helper_ops(csdev)->disable(csdev, data);
 }
 
+static int coresight_enable_helpers(struct coresight_device *csdev,
+				    enum cs_mode mode, void *data)
+{
+	int i, ret = 0;
+	struct coresight_device *helper;
+
+	for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
+		helper = csdev->pdata->out_conns[i]->dest_dev;
+		if (!helper || !coresight_is_helper(helper))
+			continue;
+
+		ret = coresight_enable_helper(helper, mode, data);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static void coresight_disable_helpers(struct coresight_device *csdev, void *data)
 {
 	int i;
@@ -478,6 +497,43 @@ static void coresight_restore_source(struct coresight_device *csdev)
 		source_ops(csdev)->restore(csdev);
 }
 
+static int coresight_save_percpu_sink(struct coresight_device *csdev)
+{
+	int ret;
+
+	if (csdev && sink_ops(csdev)->save) {
+		ret = sink_ops(csdev)->save(csdev);
+		if (ret)
+			return ret;
+
+		coresight_disable_helpers(csdev, NULL);
+	}
+
+	/* Return success if callback is not supported */
+	return 0;
+}
+
+static int coresight_restore_percpu_sink(struct coresight_device *csdev,
+					 struct coresight_path *path,
+					 enum cs_mode mode)
+{
+	int ret = 0;
+
+	if (csdev && sink_ops(csdev)->restore) {
+		ret = coresight_enable_helpers(csdev, mode, path);
+		if (ret)
+			return ret;
+
+		ret = sink_ops(csdev)->restore(csdev);
+		if (ret) {
+			coresight_disable_helpers(csdev, path);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
 /*
  * coresight_disable_path_from : Disable components in the given path beyond
  * @nd in the list. If @nd is NULL, all the components, except the SOURCE are
@@ -551,25 +607,6 @@ void coresight_disable_path(struct coresight_path *path)
 }
 EXPORT_SYMBOL_GPL(coresight_disable_path);
 
-static int coresight_enable_helpers(struct coresight_device *csdev,
-				    enum cs_mode mode, void *data)
-{
-	int i, ret = 0;
-	struct coresight_device *helper;
-
-	for (i = 0; i < csdev->pdata->nr_outconns; ++i) {
-		helper = csdev->pdata->out_conns[i]->dest_dev;
-		if (!helper || !coresight_is_helper(helper))
-			continue;
-
-		ret = coresight_enable_helper(helper, mode, data);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 static int _coresight_enable_path(struct coresight_path *path,
 				  enum cs_mode mode, void *sink_data,
 				  bool in_idle)
@@ -1667,9 +1704,12 @@ EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
 static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 				   void *v)
 {
+	int ret;
 	unsigned int cpu = smp_processor_id();
 	struct coresight_device *source = per_cpu(csdev_source, cpu);
 	struct coresight_path *path;
+	struct coresight_device *sink;
+	enum cs_mode mode;
 
 	if (!coresight_need_save_restore_source(source))
 		return NOTIFY_OK;
@@ -1682,18 +1722,33 @@ static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 	if (WARN_ON(!path))
 		return NOTIFY_BAD;
 
+	sink = coresight_get_sink(path);
+	mode = coresight_get_mode(source);
+
 	switch (cmd) {
 	case CPU_PM_ENTER:
 		if (coresight_save_source(source))
 			return NOTIFY_BAD;
 
-		coresight_disable_path_from(path, NULL, true);
+		ret = 0;
+		if (coresight_is_percpu_sink(sink))
+			ret = coresight_save_percpu_sink(sink);
+		else
+			coresight_disable_path_from(path, NULL, true);
+
+		if (ret) {
+			coresight_restore_source(source);
+			return NOTIFY_BAD;
+		}
 		break;
 	case CPU_PM_EXIT:
 	case CPU_PM_ENTER_FAILED:
-		if (_coresight_enable_path(path,
-					   coresight_get_mode(source),
-					   NULL, true))
+		if (coresight_is_percpu_sink(sink))
+			ret = coresight_restore_percpu_sink(sink, path, mode);
+		else
+			ret = _coresight_enable_path(path, mode, NULL, true);
+
+		if (ret)
 			return NOTIFY_BAD;
 
 		coresight_restore_source(source);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index f52e834640b72534ea83ab223aae7544b195bbaa..e551a36c40cc2311cd72948b799db5425b93fe68 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -363,6 +363,8 @@ enum cs_mode {
  * @alloc_buffer:	initialises perf's ring buffer for trace collection.
  * @free_buffer:	release memory allocated in @get_config.
  * @update_buffer:	update buffer pointers after a trace session.
+ * @save:		save context for a sink.
+ * @restore:		restore context for a sink.
  */
 struct coresight_ops_sink {
 	int (*enable)(struct coresight_device *csdev, enum cs_mode mode,
@@ -375,6 +377,8 @@ struct coresight_ops_sink {
 	unsigned long (*update_buffer)(struct coresight_device *csdev,
 			      struct perf_output_handle *handle,
 			      void *sink_config);
+	int (*save)(struct coresight_device *csdev);
+	int (*restore)(struct coresight_device *csdev);
 };
 
 /**

-- 
2.34.1

[PATCH v2 25/28] coresight: trbe: Save and restore state across CPU low power state

[Leo Yan]

From: Yabin Cui <yabinc@google.com>

Similar to ETE, TRBE may lose its context when a CPU enters low power
state. To make things worse, if ETE is restored without TRBE being
restored, an enabled source device with no enabled sink devices can
cause CPU hang on some devices (e.g., Pixel 9).

The save and restore flows are described in the section K5.5 "Context
switching" of Arm ARM (ARM DDI 0487 L.a). This commit adds save and
restore callbacks with following the software usages defined in the
architecture manual.

Signed-off-by: Yabin Cui <yabinc@google.com>
Co-developed-by: Leo Yan <leo.yan@arm.com>
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-trbe.c | 84 ++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 28450447693147b9afd207679832ac83e2a5848a..7f3d233f70b33058bd235ea0b798ce3884788856 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -115,6 +115,20 @@ static int trbe_errata_cpucaps[] = {
  */
 #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES	256
 
+/*
+ * struct trbe_save_state: Register values representing TRBE state
+ * @trblimitr		- Trace Buffer Limit Address Register value
+ * @trbbaser		- Trace Buffer Base Register value
+ * @trbptr		- Trace Buffer Write Pointer Register value
+ * @trbsr		- Trace Buffer Status Register value
+ */
+struct trbe_save_state {
+	u64 trblimitr;
+	u64 trbbaser;
+	u64 trbptr;
+	u64 trbsr;
+};
+
 /*
  * struct trbe_cpudata: TRBE instance specific data
  * @trbe_flag		- TRBE dirty/access flag support
@@ -133,6 +147,7 @@ struct trbe_cpudata {
 	enum cs_mode mode;
 	struct trbe_buf *buf;
 	struct trbe_drvdata *drvdata;
+	struct trbe_save_state save_state;
 	DECLARE_BITMAP(errata, TRBE_ERRATA_MAX);
 };
 
@@ -1187,12 +1202,81 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int arm_trbe_save(struct coresight_device *csdev)
+{
+	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
+	struct trbe_save_state *state = &cpudata->save_state;
+
+	if (cpudata->mode == CS_MODE_DISABLED)
+		return 0;
+
+	/*
+	 * According to the section K5.5 Context switching, Arm ARM (ARM DDI
+	 * 0487 L.a), the software usage VKHHY requires a TSB CSYNC instruction
+	 * to ensure the program-flow trace is flushed, which has been executed
+	 * in ETM driver.
+	 */
+
+	/* Disable trace buffer unit */
+	state->trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	write_sysreg_s(state->trblimitr & ~TRBLIMITR_EL1_E, SYS_TRBLIMITR_EL1);
+
+	/*
+	 * Execute a further Context synchronization event. Ensure the writes to
+	 * memory are complete.
+	 */
+	trbe_drain_buffer();
+
+	/* Synchronize the TRBE disabling */
+	isb();
+
+	state->trbbaser = read_sysreg_s(SYS_TRBBASER_EL1);
+	state->trbptr = read_sysreg_s(SYS_TRBPTR_EL1);
+	state->trbsr = read_sysreg_s(SYS_TRBSR_EL1);
+	return 0;
+}
+
+static int arm_trbe_restore(struct coresight_device *csdev)
+{
+	struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
+	struct trbe_save_state *state = &cpudata->save_state;
+
+	if (cpudata->mode == CS_MODE_DISABLED)
+		return 0;
+
+	write_sysreg_s(state->trbbaser, SYS_TRBBASER_EL1);
+	write_sysreg_s(state->trbptr, SYS_TRBPTR_EL1);
+	write_sysreg_s(state->trbsr, SYS_TRBSR_EL1);
+	write_sysreg_s(state->trblimitr & ~TRBLIMITR_EL1_E, SYS_TRBLIMITR_EL1);
+
+	/*
+	 * According to the section K5.5 Context switching, Arm ARM (ARM DDI
+	 * 0487 L.a), the software usage PKLXF requires a Context
+	 * synchronization event to guarantee the Trace Buffer Unit will observe
+	 * the new values of the System registers.
+	 */
+	isb();
+
+	/* Enable the Trace Buffer Unit */
+	write_sysreg_s(state->trblimitr, SYS_TRBLIMITR_EL1);
+
+	/* Synchronize the TRBE enable event */
+	isb();
+
+	if (trbe_needs_ctxt_sync_after_enable(cpudata))
+		isb();
+
+	return 0;
+}
+
 static const struct coresight_ops_sink arm_trbe_sink_ops = {
 	.enable		= arm_trbe_enable,
 	.disable	= arm_trbe_disable,
 	.alloc_buffer	= arm_trbe_alloc_buffer,
 	.free_buffer	= arm_trbe_free_buffer,
 	.update_buffer	= arm_trbe_update_buffer,
+	.save		= arm_trbe_save,
+	.restore	= arm_trbe_restore,
 };
 
 static const struct coresight_ops arm_trbe_cs_ops = {

-- 
2.34.1

Re: [PATCH v2 25/28] coresight: trbe: Save and restore state across CPU low power state

[James Clark]

On 01/07/2025 3:53 pm, Leo Yan wrote:
> From: Yabin Cui <yabinc@google.com>
> 
> Similar to ETE, TRBE may lose its context when a CPU enters low power
> state. To make things worse, if ETE is restored without TRBE being
> restored, an enabled source device with no enabled sink devices can
> cause CPU hang on some devices (e.g., Pixel 9).
> 
> The save and restore flows are described in the section K5.5 "Context
> switching" of Arm ARM (ARM DDI 0487 L.a). This commit adds save and
> restore callbacks with following the software usages defined in the
> architecture manual.
> 
> Signed-off-by: Yabin Cui <yabinc@google.com>
> Co-developed-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---

Hi Leo,

I tested this commit to try to avoid hitting any issues with the last 3 
hotplug changes but ran into two issues. They seemed to be hit when 
running the CPU online/offline/enable_source stress test and then after 
that running the Perf "Check Arm CoreSight trace data recording and 
synthesized samples" test.

It hit when doing them in either order, but not when doing only one 
after a reboot.

First one is just when running one of the tests:

  =====================================================
  WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
  6.16.0-rc3+ #475 Not tainted
  -----------------------------------------------------
  perf-exec/709 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
  ffff000804002cd0 (&drvdata->spinlock){+.+.}-{2:2}, at: 
cti_enable+0x40/0x130 [coresight_cti]

                and this task is already holding:
  ffff00080ab67e18 (&ctx->lock){....}-{2:2}, at: perf_event_exec+0xc4/0x6b8
  which would create a new lock dependency:
   (&ctx->lock){....}-{2:2} -> (&drvdata->spinlock){+.+.}-{2:2}

                but this new dependency connects a HARDIRQ-irq-safe lock:
   (&cpuctx_lock){-...}-{2:2}

                ... which became HARDIRQ-irq-safe at:
    lock_acquire+0x130/0x2c0
    _raw_spin_lock+0x60/0xa8
    __perf_install_in_context+0x5c/0x2f0
    remote_function+0x58/0x78
    __flush_smp_call_function_queue+0x1d8/0x9c0
    generic_smp_call_function_single_interrupt+0x20/0x38
    ipi_handler+0x118/0x338
    handle_percpu_devid_irq+0xb0/0x180
    generic_handle_domain_irq+0x4c/0x78
    gic_handle_irq+0x68/0xf0
    call_on_irq_stack+0x24/0x30
    do_interrupt_handler+0x88/0xd0
    el1_interrupt+0x34/0x68
    el1h_64_irq_handler+0x18/0x28
    el1h_64_irq+0x6c/0x70
    arch_local_irq_enable+0x8/0x10
    cpuidle_enter+0x44/0x68
    do_idle+0x1b0/0x2b8
    cpu_startup_entry+0x40/0x50
    rest_init+0x1c4/0x1d0
    start_kernel+0x394/0x458
    __primary_switched+0x88/0x98

                to a HARDIRQ-irq-unsafe lock:
   (&drvdata->spinlock){+.+.}-{2:2}

                ... which became HARDIRQ-irq-unsafe at:
  ...
    lock_acquire+0x130/0x2c0
    _raw_spin_lock+0x60/0xa8
    cti_disable+0x38/0xe8 [coresight_cti]
    coresight_disable_source+0x88/0xa8 [coresight]
    coresight_disable_sysfs+0xd0/0x1f0 [coresight]
    enable_source_store+0x78/0xb0 [coresight]
    dev_attr_store+0x24/0x40
    sysfs_kf_write+0xa8/0xd0
    kernfs_fop_write_iter+0x114/0x1c0
    vfs_write+0x2d8/0x310
    ksys_write+0x80/0xf8
    __arm64_sys_write+0x28/0x40
    invoke_syscall+0x4c/0x110
    el0_svc_common+0xb8/0xf0
    do_el0_svc+0x28/0x40
    el0_svc+0x4c/0xe8
    el0t_64_sync_handler+0x84/0x108
    el0t_64_sync+0x198/0x1a0

                other info that might help us debug this:

  Chain exists of:
                  &cpuctx_lock --> &ctx->lock --> &drvdata->spinlock

   Possible interrupt unsafe locking scenario:

         CPU0                    CPU1
         ----                    ----
    lock(&drvdata->spinlock);
                                 local_irq_disable();
                                 lock(&cpuctx_lock);
                                 lock(&ctx->lock);
    <Interrupt>
      lock(&cpuctx_lock);

                 *** DEADLOCK ***

  4 locks held by perf-exec/709:
   #0: ffff0008066b66f8 (&sig->cred_guard_mutex){+.+.}-{4:4}, at: 
bprm_execve+0x54/0x690
   #1: ffff0008066b67a0 (&sig->exec_update_lock){++++}-{4:4}, at: 
exec_mmap+0x48/0x2b0
   #2: ffff000976a467f0 (&cpuctx_lock){-...}-{2:2}, at: 
perf_event_exec+0xb4/0x6b8
   #3: ffff00080ab67e18 (&ctx->lock){....}-{2:2}, at: 
perf_event_exec+0xc4/0x6b8

                the dependencies between HARDIRQ-irq-safe lock and the 
holding lock:
   -> (&cpuctx_lock){-...}-{2:2} {
      IN-HARDIRQ-W at:
                        lock_acquire+0x130/0x2c0
                        _raw_spin_lock+0x60/0xa8
                        __perf_install_in_context+0x5c/0x2f0
                        remote_function+0x58/0x78
                        __flush_smp_call_function_queue+0x1d8/0x9c0
                        generic_smp_call_function_single_interrupt+0x20/0x38
                        ipi_handler+0x118/0x338
                        handle_percpu_devid_irq+0xb0/0x180
                        generic_handle_domain_irq+0x4c/0x78
                        gic_handle_irq+0x68/0xf0
                        call_on_irq_stack+0x24/0x30
                        do_interrupt_handler+0x88/0xd0
                        el1_interrupt+0x34/0x68
                        el1h_64_irq_handler+0x18/0x28
                        el1h_64_irq+0x6c/0x70
                        arch_local_irq_enable+0x8/0x10
                        cpuidle_enter+0x44/0x68
                        do_idle+0x1b0/0x2b8
                        cpu_startup_entry+0x40/0x50
                        rest_init+0x1c4/0x1d0
                        start_kernel+0x394/0x458
                        __primary_switched+0x88/0x98
      INITIAL USE at:
                       lock_acquire+0x130/0x2c0
                       _raw_spin_lock+0x60/0xa8
                       __perf_event_exit_context+0x3c/0xb0
                       generic_exec_single+0xb0/0x3a8
                       smp_call_function_single+0x180/0xa98
                       perf_event_exit_cpu+0x344/0x3d8
                       cpuhp_invoke_callback+0x120/0x2a0
                       cpuhp_thread_fun+0x170/0x1d8
                       smpboot_thread_fn+0x1c0/0x328
                       kthread+0x148/0x250
                       ret_from_fork+0x10/0x20
    }
    ... key      at: [<ffff800082bbe238>] cpuctx_lock+0x0/0x10
  -> (&ctx->lock){....}-{2:2} {
     INITIAL USE at:
                     lock_acquire+0x130/0x2c0
                     _raw_spin_lock_irq+0x70/0xb8
                     find_get_pmu_context+0x88/0x238
                     __arm64_sys_perf_event_open+0x794/0x1150
                     invoke_syscall+0x4c/0x110
                     el0_svc_common+0xb8/0xf0
                     do_el0_svc+0x28/0x40
                     el0_svc+0x4c/0xe8
                     el0t_64_sync_handler+0x84/0x108
                     el0t_64_sync+0x198/0x1a0
   }
   ... key      at: [<ffff800082bbe1d0>] 
__perf_event_init_context.__key+0x0/0x10
   ... acquired at:
     _raw_spin_lock+0x60/0xa8
     __perf_install_in_context+0x6c/0x2f0
     remote_function+0x58/0x78
     generic_exec_single+0xb0/0x3a8
     smp_call_function_single+0x180/0xa98
     perf_install_in_context+0x1a0/0x290
     __arm64_sys_perf_event_open+0x103c/0x1150
     invoke_syscall+0x4c/0x110
     el0_svc_common+0xb8/0xf0
     do_el0_svc+0x28/0x40
     el0_svc+0x4c/0xe8
     el0t_64_sync_handler+0x84/0x108
     el0t_64_sync+0x198/0x1a0


                the dependencies between the lock to be acquired
   and HARDIRQ-irq-unsafe lock:
  -> (&drvdata->spinlock){+.+.}-{2:2} {
     HARDIRQ-ON-W at:
                      lock_acquire+0x130/0x2c0
                      _raw_spin_lock+0x60/0xa8
                      cti_disable+0x38/0xe8 [coresight_cti]
                      coresight_disable_source+0x88/0xa8 [coresight]
                      coresight_disable_sysfs+0xd0/0x1f0 [coresight]
                      enable_source_store+0x78/0xb0 [coresight]
                      dev_attr_store+0x24/0x40
                      sysfs_kf_write+0xa8/0xd0
                      kernfs_fop_write_iter+0x114/0x1c0
                      vfs_write+0x2d8/0x310
                      ksys_write+0x80/0xf8
                      __arm64_sys_write+0x28/0x40
                      invoke_syscall+0x4c/0x110
                      el0_svc_common+0xb8/0xf0
                      do_el0_svc+0x28/0x40
                      el0_svc+0x4c/0xe8
                      el0t_64_sync_handler+0x84/0x108
                      el0t_64_sync+0x198/0x1a0
     SOFTIRQ-ON-W at:
                      lock_acquire+0x130/0x2c0
                      _raw_spin_lock+0x60/0xa8
                      cti_disable+0x38/0xe8 [coresight_cti]
                      coresight_disable_source+0x88/0xa8 [coresight]
                      coresight_disable_sysfs+0xd0/0x1f0 [coresight]
                      enable_source_store+0x78/0xb0 [coresight]
                      dev_attr_store+0x24/0x40
                      sysfs_kf_write+0xa8/0xd0
                      kernfs_fop_write_iter+0x114/0x1c0
                      vfs_write+0x2d8/0x310
                      ksys_write+0x80/0xf8
                      __arm64_sys_write+0x28/0x40
                      invoke_syscall+0x4c/0x110
                      el0_svc_common+0xb8/0xf0
                      do_el0_svc+0x28/0x40
                      el0_svc+0x4c/0xe8
                      el0t_64_sync_handler+0x84/0x108
                      el0t_64_sync+0x198/0x1a0
     INITIAL USE at:
                     lock_acquire+0x130/0x2c0
                     _raw_spin_lock+0x60/0xa8
                     cti_cpu_pm_notify+0x54/0x160 [coresight_cti]
                     notifier_call_chain+0xb8/0x1b8
                     raw_notifier_call_chain_robust+0x50/0xb0
                     cpu_pm_enter+0x50/0x90
                     psci_enter_idle_state+0x3c/0x80
                     cpuidle_enter_state+0x158/0x340
                     cpuidle_enter+0x44/0x68
                     do_idle+0x1b0/0x2b8
                     cpu_startup_entry+0x40/0x50
                     secondary_start_kernel+0x120/0x150
                     __secondary_switched+0xc0/0xc8
   }
   ... key      at: [<ffff80007b10d2a8>] 
cti_probe.__key+0x0/0xffffffffffffdd58 [coresight_cti]
   ... acquired at:
     _raw_spin_lock_irqsave+0x70/0xc0
     cti_enable+0x40/0x130 [coresight_cti]
     _coresight_enable_path+0x134/0x3c0 [coresight]
     coresight_enable_path+0x28/0x88 [coresight]
     etm_event_start+0xe0/0x228 [coresight]
     etm_event_add+0x40/0x68 [coresight]
     event_sched_in+0x270/0x418
     visit_groups_merge+0x428/0xcd0
     __pmu_ctx_sched_in+0xa0/0xe0
     ctx_sched_in+0x110/0x188
     ctx_resched+0x1c0/0x2b8
     perf_event_exec+0x29c/0x6b8
     begin_new_exec+0x378/0x558
     load_elf_binary+0x2b0/0xb00
     bprm_execve+0x394/0x690
     do_execveat_common+0x2a0/0x300
     __arm64_sys_execve+0x50/0x70
     invoke_syscall+0x4c/0x110
     el0_svc_common+0xb8/0xf0
     do_el0_svc+0x28/0x40
     el0_svc+0x4c/0xe8
     el0t_64_sync_handler+0x84/0x108
     el0t_64_sync+0x198/0x1a0

===============================================

And the second one is when reloading the modules:

  $ sudo rmmod coresight_stm coresight_funnel stm_core 
coresight_replicator coresight_tpiu coresight_etm4x coresight_tmc 
coresight_cti coresight_cpu_debug coresight_trbe coresight
  $ sudo modprobe coresight;  sudo modprobe coresight_stm ; sudo 
modprobe coresight_funnel; sudo modprobe stm_core; sudo modprobe 
coresight_replicator; sudo modprobe coresight_cpu_debug; sudo modprobe 
coresight_tpiu; sudo modprobe coresight_etm4x; sudo modprobe 
coresight_tmc; sudo modprobe coresight_trbe ; sudo modprobe coresight_cti ;

  Unable to handle kernel NULL pointer dereference at virtual address 
00000000000004f0
  pc : cti_cpu_pm_notify+0x74/0x160 [coresight_cti]
  lr : cti_cpu_pm_notify+0x54/0x160 [coresight_cti]
  Call trace:
   cti_cpu_pm_notify+0x74/0x160 [coresight_cti] (P)
   notifier_call_chain+0xb8/0x1b8
   raw_notifier_call_chain_robust+0x50/0xb0
   cpu_pm_enter+0x50/0x90
   psci_enter_idle_state+0x3c/0x80
   cpuidle_enter_state+0x158/0x340
   cpuidle_enter+0x44/0x68
   do_idle+0x1b0/0x2b8
   cpu_startup_entry+0x40/0x50
   secondary_start_kernel+0x120/0x150
   __secondary_switched+0xc0/0xc8

[PATCH v2 26/28] coresight: Take hotplug lock in enable_source_store() for Sysfs mode

[Leo Yan]

The hotplug lock is acquired and released in the etm4_disable_sysfs()
function, which is a low-level function located in the ETM4 driver.
This prevents us from a new solution for hotplug.

Firstly, hotplug callbacks cannot invoke etm4_disable_sysfs() to disable
the source; otherwise, a deadlock issue occurs.  The reason is that, in
the hotplug flow, the kernel acquires the hotplug lock before calling
callbacks.  Subsequently, if coresight_disable_source() is invoked and
it calls etm4_disable_sysfs(), the hotplug lock will be acquired twice,
leading to a double lock issue.

Secondly, when hotplugging a CPU on or off, if we want to manipulate all
components on a path attached to the CPU, we need to maintain atomicity
for the entire path.  Otherwise, a race condition may occur with users
setting the same path via the Sysfs knobs, ultimately causing mess
states in CoreSight components.

This patch moves the hotplug locking from etm4_disable_sysfs() into
enable_source_store().  As a result, when users control the Sysfs knobs,
the whole flow is protected by hotplug locking, ensuring it is mutual
exclusive with hotplug callbacks.

Note, the paired function etm4_enable_sysfs() does not use hotplug
locking, which is why this patch does not modify it.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 ---------
 drivers/hwtracing/coresight/coresight-sysfs.c      | 7 +++++++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 93ae8590ae2459db317f6367b6cffbf658b0e2f2..f99a48511850cd6e9682533880b22a3b8fc43135 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1086,13 +1086,6 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
 {
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	/*
-	 * Taking hotplug lock here protects from clocks getting disabled
-	 * with tracing being left on (crash scenario) if user disable occurs
-	 * after cpu online mask indicates the cpu is offline but before the
-	 * DYING hotplug callback is serviced by the ETM driver.
-	 */
-	cpus_read_lock();
 	raw_spin_lock(&drvdata->spinlock);
 
 	/*
@@ -1106,8 +1099,6 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
 
 	cscfg_csdev_disable_active_config(csdev);
 
-	cpus_read_unlock();
-
 	/*
 	 * we only release trace IDs when resetting sysfs.
 	 * This permits sysfs users to read the trace ID after the trace
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index 14ee15297b98115122068cbe932f0b2ce004b77e..e89ea98d680be8d363674c1d46675e46565f210d 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -362,6 +362,13 @@ static ssize_t enable_source_store(struct device *dev,
 	if (ret)
 		return ret;
 
+	/*
+	 * CoreSight hotplug callbacks in core layer control a activated path
+	 * from its source to sink. Taking hotplug lock here protects a race
+	 * condition with hotplug callbacks.
+	 */
+	guard(cpus_read_lock)();
+
 	if (val) {
 		ret = coresight_enable_sysfs(csdev);
 		if (ret)

-- 
2.34.1

[PATCH v2 27/28] coresight: Move CPU hotplug callbacks to core layer

[Leo Yan]

This commit moves CPU hotplug callbacks from ETMv4 driver to core layer.
The motivation is the core layer can control all components on an
activated path rather but not only managing tracer in ETMv4 driver.

The perf event layer will disable CoreSight PMU event 'cs_etm' when
hotplug off a CPU.  That means a perf mode will be always converted to
disabled mode in CPU hotplug.  Arm CoreSight CPU hotplug callbacks only
need to handle the Sysfs mode and ignore the perf mode.

The core layer invokes a high level API coresight_disable_source() to
disable a source when hotplug-off a CPU.  It disables a tracer and
changes the tracer's mode to CS_MODE_DISABLED.

When hotplug-in a CPU, if a activated path is detected - when the
activated path pointer is not NULL - in this case, the tracer will be
re-enabled.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c       | 61 +++++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 -------------
 include/linux/coresight.h                          | 17 +++---
 3 files changed, 70 insertions(+), 45 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 9978737d21177ab7cfcd449cf67a0b0736fcca5a..e1659b05738e73fcb6c66c74c8f1ce3a24ab2da4 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1701,6 +1701,52 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
 }
 EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
 
+static int coresight_starting_cpu(unsigned int cpu)
+{
+	struct coresight_device *source = per_cpu(csdev_source, cpu);
+	struct coresight_path *path;
+
+	if (!source)
+		return 0;
+
+	/* Re-enable components on an activated path */
+	path = per_cpu(csdev_cpu_path, cpu);
+	if (!path)
+		return 0;
+
+	if (path->saved_mode != CS_MODE_SYSFS)
+		return 0;
+
+	source_ops(source)->enable(source, NULL, path->saved_mode, path);
+	return 0;
+}
+
+static int coresight_dying_cpu(unsigned int cpu)
+{
+	struct coresight_device *source = per_cpu(csdev_source, cpu);
+	struct coresight_path *path;
+
+	if (!source)
+		return 0;
+
+	/* Don't proceed if no path is activated */
+	path = per_cpu(csdev_cpu_path, cpu);
+	if (!path)
+		return 0;
+
+	path->saved_mode = coresight_get_mode(source);
+
+	/*
+	 * The perf event layer will disable PMU events in the CPU hotplug.
+	 * CoreSight driver should never handle the CS_MODE_PERF case.
+	 */
+	if (WARN_ON(path->saved_mode != CS_MODE_SYSFS))
+		return 0;
+
+	source_ops(source)->disable(source, NULL);
+	return 0;
+}
+
 static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 				   void *v)
 {
@@ -1766,11 +1812,24 @@ static struct notifier_block coresight_cpu_pm_nb = {
 
 static int __init coresight_pm_setup(void)
 {
-	return cpu_pm_register_notifier(&coresight_cpu_pm_nb);
+	int ret;
+
+	ret = cpu_pm_register_notifier(&coresight_cpu_pm_nb);
+	if (ret)
+		return ret;
+
+	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
+					"arm/coresight-core:starting",
+					coresight_starting_cpu, coresight_dying_cpu);
+	if (ret)
+		cpu_pm_unregister_notifier(&coresight_cpu_pm_nb);
+
+	return ret;
 }
 
 static void coresight_pm_cleanup(void)
 {
+	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	cpu_pm_unregister_notifier(&coresight_cpu_pm_nb);
 }
 
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index f99a48511850cd6e9682533880b22a3b8fc43135..94872226dd63c5b9ed9ef95f17d717656c4e5589 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1802,33 +1802,6 @@ static int etm4_online_cpu(unsigned int cpu)
 	return 0;
 }
 
-static int etm4_starting_cpu(unsigned int cpu)
-{
-	if (!etmdrvdata[cpu])
-		return 0;
-
-	raw_spin_lock(&etmdrvdata[cpu]->spinlock);
-	if (!etmdrvdata[cpu]->os_unlock)
-		etm4_os_unlock(etmdrvdata[cpu]);
-
-	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
-		etm4_enable_hw(etmdrvdata[cpu], true);
-	raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
-	return 0;
-}
-
-static int etm4_dying_cpu(unsigned int cpu)
-{
-	if (!etmdrvdata[cpu])
-		return 0;
-
-	raw_spin_lock(&etmdrvdata[cpu]->spinlock);
-	if (coresight_get_mode(etmdrvdata[cpu]->csdev))
-		etm4_disable_hw(etmdrvdata[cpu]);
-	raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
-	return 0;
-}
-
 static bool etm4_need_context_save_restore(struct coresight_device *csdev)
 {
 	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
@@ -1923,13 +1896,6 @@ static int __init etm4_pm_setup(void)
 {
 	int ret;
 
-	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
-					"arm/coresight4:starting",
-					etm4_starting_cpu, etm4_dying_cpu);
-
-	if (ret)
-		return ret;
-
 	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
 					"arm/coresight4:online",
 					etm4_online_cpu, NULL);
@@ -1940,14 +1906,11 @@ static int __init etm4_pm_setup(void)
 		return 0;
 	}
 
-	/* failed dyn state - remove others */
-	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	return ret;
 }
 
 static void etm4_pm_clear(void)
 {
-	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
 	if (hp_online) {
 		cpuhp_remove_state_nocalls(hp_online);
 		hp_online = 0;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index e551a36c40cc2311cd72948b799db5425b93fe68..6daed3a75190617464515c4e1e0628d2169b26e7 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -330,21 +330,24 @@ static struct coresight_dev_list (var) = {				\
 
 #define to_coresight_device(d) container_of(d, struct coresight_device, dev)
 
+enum cs_mode {
+	CS_MODE_DISABLED,
+	CS_MODE_SYSFS,
+	CS_MODE_PERF,
+	CS_MODE_DEBUG,
+};
+
 /**
  * struct coresight_path - data needed by enable/disable path
  * @path_list:              path from source to sink.
  * @trace_id:          trace_id of the whole path.
+ * @saved_mode:        The saved device mode. It stores the source device's mode
+ *                     to represent the path mode during CPU hotplug off.
  */
 struct coresight_path {
 	struct list_head	path_list;
 	u8			trace_id;
-};
-
-enum cs_mode {
-	CS_MODE_DISABLED,
-	CS_MODE_SYSFS,
-	CS_MODE_PERF,
-	CS_MODE_DEBUG,
+	enum cs_mode		saved_mode;
 };
 
 #define coresight_ops(csdev)	csdev->ops

-- 
2.34.1

[PATCH v2 28/28] coresight: Manage activated path during CPU hotplug

[Leo Yan]

This commit handles activated path during the CPU hotplug process.

When a CPU is hotplug off or hotplug in, and if an activated path is
associated with it, the CPU PM notifier disables and enables the path,
including the sink and helpers.

When disabling a path, the sink's disable() callback updates its buffer
in SysFS mode.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index e1659b05738e73fcb6c66c74c8f1ce3a24ab2da4..0f77f4c5464550a28e2dce391e92160a7003c140 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1717,6 +1717,7 @@ static int coresight_starting_cpu(unsigned int cpu)
 	if (path->saved_mode != CS_MODE_SYSFS)
 		return 0;
 
+	_coresight_enable_path(path, path->saved_mode, NULL, false);
 	source_ops(source)->enable(source, NULL, path->saved_mode, path);
 	return 0;
 }
@@ -1743,7 +1744,8 @@ static int coresight_dying_cpu(unsigned int cpu)
 	if (WARN_ON(path->saved_mode != CS_MODE_SYSFS))
 		return 0;
 
-	source_ops(source)->disable(source, NULL);
+	coresight_disable_source(source, NULL);
+	coresight_disable_path_from(path, NULL, false);
 	return 0;
 }
 

-- 
2.34.1