[PATCH v4 00/15] CoreSight: Refactor power management for CoreSight path

[PATCH v4 00/15] CoreSight: Refactor power management for CoreSight path

[Leo Yan]

This series is extracted from [1], focusing on CoreSight path power
management.

Compared to the previous version, this series is updated heavily for:

1) Dropped the global per CPU variable for saving path pointers.
   Instead, the activate path is now stored in the source device's
   structure.  This allows fetching the path pointer naturally based on
   the source regardless of whether it is a per-CPU source or a system
   source (such as STM).

   This improvement addresses Mike's comment that, later we can polish
   coresight-sysfs.c to remove the tracer_path variables.

2) To simplify the series and make it easier to review, the CTI driver
   related fixes have been removed from this series and which will be
   sent out separately.

3) This series disables the path when a CPU is hot-plugged off but does
   not re-enable it when the CPU is subsequently hot-plugged in.  This
   simplifies the implementation and keep it consistent with the perf
   session's behavior.

   It also improves security, as there is no risk of unintended tracing
   caused by a CPU being hot-plugged after a long period of inactivity.

This series is dependent on ETM driver's PM improvement series [2] and
has been verified on Juno-r2 and FVP RevC.

[1] https://lore.kernel.org/linux-arm-kernel/20250915-arm_coresight_power_management_fix-v3-0-ea49e91124ec@arm.com/
[2] https://lore.kernel.org/linux-arm-kernel/20251103-arm_coresight_power_management_fix-v5-0-f803c19aa153@arm.com/#t

---
Changes in v4:
- Changed to store path pointer in coresight_device, this is easier for
  fetching path pointer based on source device (Mike).
- Dropped changes in CTI driver.
- Only disabled path for CPU hot-plugged off but not enable path for
  hot-plugged in.
- Removed James' test tags for modified patches.
- Link to v3: https://lore.kernel.org/r/20250915-arm_coresight_power_management_fix-v3-0-ea49e91124ec@arm.com

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

---
Leo Yan (14):
      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 PM is needed
      coresight: etm4x: Remove redundant condition checks in save and restore
      coresight: syscfg: Use spinlock to protect active variables
      coresight: Introduce coresight_enable_source() helper
      coresight: Save activated path into source device
      coresight: Add 'in_idle' argument to enable/disable path 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

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       | 273 ++++++++++++++++++++-
 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-etm-perf.c   |   2 +-
 drivers/hwtracing/coresight/coresight-etm3x-core.c |   1 +
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 137 ++---------
 drivers/hwtracing/coresight/coresight-funnel.c     |   1 +
 drivers/hwtracing/coresight/coresight-priv.h       |   3 +
 drivers/hwtracing/coresight/coresight-replicator.c |   1 +
 drivers/hwtracing/coresight/coresight-stm.c        |   1 +
 drivers/hwtracing/coresight/coresight-syscfg.c     |  22 +-
 drivers/hwtracing/coresight/coresight-syscfg.h     |   2 +
 drivers/hwtracing/coresight/coresight-sysfs.c      |  12 +-
 drivers/hwtracing/coresight/coresight-tmc-core.c   |   1 +
 drivers/hwtracing/coresight/coresight-tnoc.c       |   2 +
 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                          |  13 +
 24 files changed, 425 insertions(+), 140 deletions(-)
---
base-commit: f9ac95561513e18c2a2cf8905355dc5f0e030c46
change-id: 20251104-arm_coresight_path_power_management_improvement-dab4966f8280

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

[PATCH v4 01/15] 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.

Populate CPU ID into the coresight_device structure, for components that
are not CPU bound, set this field to -1.

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

Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
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-sysfs.c      | 3 +++
 drivers/hwtracing/coresight/coresight-tmc-core.c   | 1 +
 drivers/hwtracing/coresight/coresight-tnoc.c       | 2 ++
 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 ++++
 20 files changed, 26 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index a3ccb7034ae14d7339bc2549bccadf11e28c45e2..2685e3a883b1f8067de643519b5fc91153123c57 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -575,6 +575,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 3267192f0c1c667b0570b9100c3c449064e7fb5e..db325dff884d21cc8d93f32bfe3be5c0491ffc8d 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1336,6 +1336,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 c586495e9a088a63cec481a82fd9f4ec7c645160..05497a5114785397a98e408466df0b937c87ab61 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..653d37cd0bd12a57c78e2615f839e0d8af25d7f0 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.cpu = -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 35db1b6093d154d67dc567df42f838e2ba3d1d58..8e0bbffdf36215fe82c89a34767e425e03f4a744 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -770,6 +770,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 a5e809589d3e382acde24ee457e94e5dcb18ea35..42115c4697d77d5dd86dc01a4da6b7ea076d4558 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -886,6 +886,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 d7da5f3b97b3b18d5d6eee0c20a3bdca0c4bec1a..15bf825a346826cd534ef3cd4f41c885c66321da 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -2053,6 +2053,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 3b248e54471a38f501777fe162fea850d1c851b3..457291bb24212c502de9aadecb41da218c733c2f 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -263,6 +263,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))
 		return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index e6472658235dc479cec91ac18f3737f76f8c74f0..38518ba862928ea904c953c8303559e349e45200 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -273,6 +273,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 e68529bf89c9815a8118955bf3114ad1ed4fb346..e2197ae0a7c8412785ce8b1161311048cbaa3f58 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -901,6 +901,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-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index 5e52324aa9ac7b3de9379bc3f2b349a2cdea83c2..cb02f1bfaf068ec3f50f997bcb94f7d5215ccea8 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -162,6 +162,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;
 }
 
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 36599c431be6203e871fdcb8de569cc6701c52bb..bd783c9dcb56afa0adfa9724e45fcd0c4cfe367b 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-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
index ff9a0a9cfe96e5f5e3077c750ea2f890cdd50d94..b310f832506db5cd9f2134ff66909f64d4c785ce 100644
--- a/drivers/hwtracing/coresight/coresight-tnoc.c
+++ b/drivers/hwtracing/coresight/coresight-tnoc.c
@@ -199,6 +199,8 @@ static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.dev = &adev->dev;
 	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
 	desc.groups = coresight_tnoc_groups;
+	desc.cpu = -1;
+
 	drvdata->csdev = coresight_register(&desc);
 	if (IS_ERR(drvdata->csdev)) {
 		coresight_trace_id_put_system_id(drvdata->atid);
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index 333b3cb236859f0feb1498f4ab81037c772143fd..6739deda402ed5024e17add349314d6db793dd13 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -324,6 +324,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 9463afdbda8ad74eee78c72185fe7603f81b7888..3ade05e268e31dd2572b84e91f9448fcf317f2ab 100644
--- a/drivers/hwtracing/coresight/coresight-tpiu.c
+++ b/drivers/hwtracing/coresight/coresight-tpiu.c
@@ -171,6 +171,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 43643d2c5bdd0ac432f976dbe666ff0f494033ee..3c82ab4394fde1e3dd4371a9b1703da4d8f6db9d 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -1289,6 +1289,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 3e5e5acd0c7fcde7d312d440da4355faaf682c7b..222597ec7a089e10ad763df206917e90f34bb5c2 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

[PATCH v4 02/15] 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'.

Reviewed-by: Mike Leach <mike.leach@linaro.org>
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 db325dff884d21cc8d93f32bfe3be5c0491ffc8d..25a530cde8938b366bbb144fdcc107271ebae276 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -34,6 +34,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
@@ -79,6 +80,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;
@@ -1393,6 +1399,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;
 
@@ -1424,6 +1434,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 v4 03/15] 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 device 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 | 70 ++++++++++++++++++++++++++++
 include/linux/coresight.h                    |  2 +
 2 files changed, 72 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 25a530cde8938b366bbb144fdcc107271ebae276..f642190740b93555084584abbc1d9426cc87ec7c 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -6,6 +6,7 @@
 #include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/build_bug.h>
+#include <linux/cpu_pm.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -1575,6 +1576,68 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
 }
 EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
 
+static bool coresight_pm_is_needed(struct coresight_device *csdev)
+{
+	if (!csdev)
+		return false;
+
+	/* pm_save_disable() and pm_restore_enable() must be paired */
+	if (!coresight_ops(csdev)->pm_save_disable ||
+	    !coresight_ops(csdev)->pm_restore_enable)
+		return false;
+
+	return true;
+}
+
+static int coresight_pm_save(struct coresight_device *csdev)
+{
+	return coresight_ops(csdev)->pm_save_disable(csdev);
+}
+
+static void coresight_pm_restore(struct coresight_device *csdev)
+{
+	coresight_ops(csdev)->pm_restore_enable(csdev);
+}
+
+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 (!coresight_pm_is_needed(source))
+		return NOTIFY_OK;
+
+	switch (cmd) {
+	case CPU_PM_ENTER:
+		if (coresight_pm_save(source))
+			return NOTIFY_BAD;
+		break;
+	case CPU_PM_EXIT:
+	case CPU_PM_ENTER_FAILED:
+		coresight_pm_restore(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",
 };
@@ -1629,9 +1692,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:
@@ -1643,6 +1712,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 222597ec7a089e10ad763df206917e90f34bb5c2..b10ef4fa17a76b4d11223cc8fd43e5544b6ea8b9 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -439,6 +439,8 @@ struct coresight_ops_panic {
 struct coresight_ops {
 	int (*trace_id)(struct coresight_device *csdev, enum cs_mode mode,
 			struct coresight_device *sink);
+	int (*pm_save_disable)(struct coresight_device *csdev);
+	void (*pm_restore_enable)(struct coresight_device *csdev);
 	const struct coresight_ops_sink *sink_ops;
 	const struct coresight_ops_link *link_ops;
 	const struct coresight_ops_source *source_ops;

-- 
2.34.1

Re: [PATCH v4 03/15] coresight: Register CPU PM notifier in core layer

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> 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 device 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>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
>   drivers/hwtracing/coresight/coresight-core.c | 70 ++++++++++++++++++++++++++++
>   include/linux/coresight.h                    |  2 +
>   2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 25a530cde8938b366bbb144fdcc107271ebae276..f642190740b93555084584abbc1d9426cc87ec7c 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -6,6 +6,7 @@
>   #include <linux/acpi.h>
>   #include <linux/bitfield.h>
>   #include <linux/build_bug.h>
> +#include <linux/cpu_pm.h>
>   #include <linux/kernel.h>
>   #include <linux/init.h>
>   #include <linux/types.h>
> @@ -1575,6 +1576,68 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
>   }
>   EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
>   
> +static bool coresight_pm_is_needed(struct coresight_device *csdev)
> +{
> +	if (!csdev)
> +		return false;
> +
> +	/* pm_save_disable() and pm_restore_enable() must be paired */
> +	if (!coresight_ops(csdev)->pm_save_disable ||
> +	    !coresight_ops(csdev)->pm_restore_enable)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int coresight_pm_save(struct coresight_device *csdev)
> +{
> +	return coresight_ops(csdev)->pm_save_disable(csdev);
> +}
> +
> +static void coresight_pm_restore(struct coresight_device *csdev)
> +{
> +	coresight_ops(csdev)->pm_restore_enable(csdev);
> +}
> +
> +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 (!coresight_pm_is_needed(source))
> +		return NOTIFY_OK;
> +
> +	switch (cmd) {
> +	case CPU_PM_ENTER:
> +		if (coresight_pm_save(source))
> +			return NOTIFY_BAD;
> +		break;
> +	case CPU_PM_EXIT:
> +	case CPU_PM_ENTER_FAILED:
> +		coresight_pm_restore(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",
>   };
> @@ -1629,9 +1692,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:
> @@ -1643,6 +1712,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 222597ec7a089e10ad763df206917e90f34bb5c2..b10ef4fa17a76b4d11223cc8fd43e5544b6ea8b9 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -439,6 +439,8 @@ struct coresight_ops_panic {
>   struct coresight_ops {
>   	int (*trace_id)(struct coresight_device *csdev, enum cs_mode mode,
>   			struct coresight_device *sink);
> +	int (*pm_save_disable)(struct coresight_device *csdev);
> +	void (*pm_restore_enable)(struct coresight_device *csdev);
>   	const struct coresight_ops_sink *sink_ops;
>   	const struct coresight_ops_link *link_ops;
>   	const struct coresight_ops_source *source_ops;
> 

Re: [PATCH v4 03/15] coresight: Register CPU PM notifier in core layer

[Suzuki K Poulose]

On 04/11/2025 15:21, Leo Yan wrote:
> 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 device 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 | 70 ++++++++++++++++++++++++++++
>   include/linux/coresight.h                    |  2 +
>   2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 25a530cde8938b366bbb144fdcc107271ebae276..f642190740b93555084584abbc1d9426cc87ec7c 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -6,6 +6,7 @@
>   #include <linux/acpi.h>
>   #include <linux/bitfield.h>
>   #include <linux/build_bug.h>
> +#include <linux/cpu_pm.h>
>   #include <linux/kernel.h>
>   #include <linux/init.h>
>   #include <linux/types.h>
> @@ -1575,6 +1576,68 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
>   }
>   EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
>   
> +static bool coresight_pm_is_needed(struct coresight_device *csdev)
> +{
> +	if (!csdev)
> +		return false;
> +
> +	/* pm_save_disable() and pm_restore_enable() must be paired */
> +	if (!coresight_ops(csdev)->pm_save_disable ||
> +	    !coresight_ops(csdev)->pm_restore_enable)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int coresight_pm_save(struct coresight_device *csdev)
> +{
> +	return coresight_ops(csdev)->pm_save_disable(csdev);
> +}
> +
> +static void coresight_pm_restore(struct coresight_device *csdev)
> +{
> +	coresight_ops(csdev)->pm_restore_enable(csdev);
> +}
> +
> +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 (!coresight_pm_is_needed(source))
> +		return NOTIFY_OK;
> +
> +	switch (cmd) {
> +	case CPU_PM_ENTER:
> +		if (coresight_pm_save(source))

Unless we do the "set_per_cpu_source" *ON the CPU*, what guarantees that
this will vanish while we do the PM save ? e.g., UNLOAD coresight_etm4x
driver

Rest looks fine to me.

Suzuki


> +			return NOTIFY_BAD;
> +		break;
> +	case CPU_PM_EXIT:
> +	case CPU_PM_ENTER_FAILED:
> +		coresight_pm_restore(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",
>   };
> @@ -1629,9 +1692,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:
> @@ -1643,6 +1712,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 222597ec7a089e10ad763df206917e90f34bb5c2..b10ef4fa17a76b4d11223cc8fd43e5544b6ea8b9 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -439,6 +439,8 @@ struct coresight_ops_panic {
>   struct coresight_ops {
>   	int (*trace_id)(struct coresight_device *csdev, enum cs_mode mode,
>   			struct coresight_device *sink);
> +	int (*pm_save_disable)(struct coresight_device *csdev);
> +	void (*pm_restore_enable)(struct coresight_device *csdev);
>   	const struct coresight_ops_sink *sink_ops;
>   	const struct coresight_ops_link *link_ops;
>   	const struct coresight_ops_source *source_ops;
> 

Re: [PATCH v4 03/15] coresight: Register CPU PM notifier in core layer

[Leo Yan]

On Tue, Nov 11, 2025 at 12:13:38PM +0000, Suzuki Kuruppassery Poulose wrote:

[...]

> > --- a/drivers/hwtracing/coresight/coresight-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-core.c

> > +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 (!coresight_pm_is_needed(source))
> > +		return NOTIFY_OK;
> > +
> > +	switch (cmd) {
> > +	case CPU_PM_ENTER:
> > +		if (coresight_pm_save(source))
> 
> Unless we do the "set_per_cpu_source" *ON the CPU*, what guarantees that
> this will vanish while we do the PM save ? e.g., UNLOAD coresight_etm4x
> driver

Good catch!  I will use SMP call to set per CPU source in next version.

Thanks,
Leo

[PATCH v4 04/15] 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 | 59 ++++------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 15bf825a346826cd534ef3cd4f41c885c66321da..adb5c6b12c6f3113747f1f455087b1ab497a53f6 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1185,11 +1185,6 @@ static const struct coresight_ops_source etm4_source_ops = {
 	.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);
@@ -1894,8 +1889,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)
@@ -1918,8 +1914,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;
 
@@ -1927,38 +1925,11 @@ 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 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,
+	.pm_save_disable	= etm4_cpu_save,
+	.pm_restore_enable	= etm4_cpu_restore,
+	.source_ops		= &etm4_source_ops,
 };
 
 /* Setup PM. Deals with error conditions and counts */
@@ -1966,16 +1937,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",
@@ -1989,15 +1956,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

Re: [PATCH v4 04/15] coresight: etm4x: Hook CPU PM callbacks

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> 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>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 59 ++++------------------
>   1 file changed, 11 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 15bf825a346826cd534ef3cd4f41c885c66321da..adb5c6b12c6f3113747f1f455087b1ab497a53f6 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1185,11 +1185,6 @@ static const struct coresight_ops_source etm4_source_ops = {
>   	.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);
> @@ -1894,8 +1889,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)
> @@ -1918,8 +1914,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;
>   
> @@ -1927,38 +1925,11 @@ 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 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,
> +	.pm_save_disable	= etm4_cpu_save,
> +	.pm_restore_enable	= etm4_cpu_restore,
> +	.source_ops		= &etm4_source_ops,
>   };
>   
>   /* Setup PM. Deals with error conditions and counts */
> @@ -1966,16 +1937,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",
> @@ -1989,15 +1956,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);
> 

[PATCH v4 05/15] coresight: Add callback to determine if PM is needed

[Leo Yan]

Add a callback in the source device that returns a boolean indicating
whether power management 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       |  9 ++++++++-
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 16 ++++++++++++++++
 include/linux/coresight.h                          |  1 +
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index f642190740b93555084584abbc1d9426cc87ec7c..0d4e935094297023b8a7786ff4b1b3783164016b 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1586,7 +1586,14 @@ static bool coresight_pm_is_needed(struct coresight_device *csdev)
 	    !coresight_ops(csdev)->pm_restore_enable)
 		return false;
 
-	return true;
+	/*
+	 * PM callbacks are provided but pm_is_neended() is absent, it means
+	 * no extra check is needed.
+	 */
+	if (!coresight_ops(csdev)->pm_is_needed)
+		return true;
+
+	return coresight_ops(csdev)->pm_is_needed(csdev);
 }
 
 static int coresight_pm_save(struct coresight_device *csdev)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index adb5c6b12c6f3113747f1f455087b1ab497a53f6..da135849c70833366aa1089a0752bc7844c0d96d 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1838,6 +1838,21 @@ static int etm4_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
+static bool etm4_need_save_restore_context(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;
@@ -1929,6 +1944,7 @@ static const struct coresight_ops etm4_cs_ops = {
 	.trace_id		= coresight_etm_get_trace_id,
 	.pm_save_disable	= etm4_cpu_save,
 	.pm_restore_enable	= etm4_cpu_restore,
+	.pm_is_needed		= etm4_need_save_restore_context,
 	.source_ops		= &etm4_source_ops,
 };
 
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index b10ef4fa17a76b4d11223cc8fd43e5544b6ea8b9..3d59be214dd25dfa7ad9148a6688628e0d1a98dd 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -441,6 +441,7 @@ struct coresight_ops {
 			struct coresight_device *sink);
 	int (*pm_save_disable)(struct coresight_device *csdev);
 	void (*pm_restore_enable)(struct coresight_device *csdev);
+	bool (*pm_is_needed)(struct coresight_device *csdev);
 	const struct coresight_ops_sink *sink_ops;
 	const struct coresight_ops_link *link_ops;
 	const struct coresight_ops_source *source_ops;

-- 
2.34.1

Re: [PATCH v4 05/15] coresight: Add callback to determine if PM is needed

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> Add a callback in the source device that returns a boolean indicating
> whether power management 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>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
>   drivers/hwtracing/coresight/coresight-core.c       |  9 ++++++++-
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 16 ++++++++++++++++
>   include/linux/coresight.h                          |  1 +
>   3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index f642190740b93555084584abbc1d9426cc87ec7c..0d4e935094297023b8a7786ff4b1b3783164016b 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1586,7 +1586,14 @@ static bool coresight_pm_is_needed(struct coresight_device *csdev)
>   	    !coresight_ops(csdev)->pm_restore_enable)
>   		return false;
>   
> -	return true;
> +	/*
> +	 * PM callbacks are provided but pm_is_neended() is absent, it means
> +	 * no extra check is needed.
> +	 */
> +	if (!coresight_ops(csdev)->pm_is_needed)
> +		return true;
> +
> +	return coresight_ops(csdev)->pm_is_needed(csdev);
>   }
>   
>   static int coresight_pm_save(struct coresight_device *csdev)
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index adb5c6b12c6f3113747f1f455087b1ab497a53f6..da135849c70833366aa1089a0752bc7844c0d96d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1838,6 +1838,21 @@ static int etm4_dying_cpu(unsigned int cpu)
>   	return 0;
>   }
>   
> +static bool etm4_need_save_restore_context(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;
> @@ -1929,6 +1944,7 @@ static const struct coresight_ops etm4_cs_ops = {
>   	.trace_id		= coresight_etm_get_trace_id,
>   	.pm_save_disable	= etm4_cpu_save,
>   	.pm_restore_enable	= etm4_cpu_restore,
> +	.pm_is_needed		= etm4_need_save_restore_context,
>   	.source_ops		= &etm4_source_ops,
>   };
>   
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index b10ef4fa17a76b4d11223cc8fd43e5544b6ea8b9..3d59be214dd25dfa7ad9148a6688628e0d1a98dd 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -441,6 +441,7 @@ struct coresight_ops {
>   			struct coresight_device *sink);
>   	int (*pm_save_disable)(struct coresight_device *csdev);
>   	void (*pm_restore_enable)(struct coresight_device *csdev);
> +	bool (*pm_is_needed)(struct coresight_device *csdev);
>   	const struct coresight_ops_sink *sink_ops;
>   	const struct coresight_ops_link *link_ops;
>   	const struct coresight_ops_source *source_ops;
> 

[PATCH v4 06/15] 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.

Reviewed-by: Mike Leach <mike.leach@linaro.org>
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 da135849c70833366aa1089a0752bc7844c0d96d..af2793cdb983c85f883efe10d7633f9cf718a44f 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1853,10 +1853,10 @@ static bool etm4_need_save_restore_context(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;
 
@@ -1904,40 +1904,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 etm4_cs_ops = {

-- 
2.34.1

[PATCH v4 07/15] coresight: syscfg: Use spinlock to protect active variables

[Leo Yan]

To support dynamically enabling and disabling the CoreSight path during
CPU idle, cscfg_config_sysfs_get_active_cfg() may be called by the ETM
driver in an atomic context. However, the function currently acquires a
mutex, which is a sleepable lock, and this is not allowed in atomic
context.

To make cscfg_config_sysfs_get_active_cfg() called in the idle flow,
replace the mutex with a raw spinlock to protect the active
variables.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-syscfg.c | 22 +++++++++++++---------
 drivers/hwtracing/coresight/coresight-syscfg.h |  2 ++
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
index 6836b05986e8091b4f8ad7d082d6a016f8cf7d74..53aa1a44f9ac8f65b4e3cce06a33255abfc17351 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg.c
@@ -957,15 +957,19 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti
 
 	cfg_hash = (unsigned long)config_desc->event_ea->var;
 
+	raw_spin_lock(&cscfg_mgr->spinlock);
+
 	if (activate) {
+
 		/* cannot be a current active value to activate this */
 		if (cscfg_mgr->sysfs_active_config) {
 			err = -EBUSY;
-			goto exit_unlock;
+		} else {
+			err = _cscfg_activate_config(cfg_hash);
+			if (!err)
+				cscfg_mgr->sysfs_active_config = cfg_hash;
 		}
-		err = _cscfg_activate_config(cfg_hash);
-		if (!err)
-			cscfg_mgr->sysfs_active_config = cfg_hash;
+
 	} else {
 		/* disable if matching current value */
 		if (cscfg_mgr->sysfs_active_config == cfg_hash) {
@@ -975,7 +979,8 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti
 			err = -EINVAL;
 	}
 
-exit_unlock:
+	raw_spin_unlock(&cscfg_mgr->spinlock);
+
 	mutex_unlock(&cscfg_mutex);
 	return err;
 }
@@ -983,9 +988,8 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti
 /* set the sysfs preset value */
 void cscfg_config_sysfs_set_preset(int preset)
 {
-	mutex_lock(&cscfg_mutex);
+	guard(raw_spinlock)(&cscfg_mgr->spinlock);
 	cscfg_mgr->sysfs_active_preset = preset;
-	mutex_unlock(&cscfg_mutex);
 }
 
 /*
@@ -994,10 +998,9 @@ void cscfg_config_sysfs_set_preset(int preset)
  */
 void cscfg_config_sysfs_get_active_cfg(unsigned long *cfg_hash, int *preset)
 {
-	mutex_lock(&cscfg_mutex);
+	guard(raw_spinlock)(&cscfg_mgr->spinlock);
 	*preset = cscfg_mgr->sysfs_active_preset;
 	*cfg_hash = cscfg_mgr->sysfs_active_config;
-	mutex_unlock(&cscfg_mutex);
 }
 EXPORT_SYMBOL_GPL(cscfg_config_sysfs_get_active_cfg);
 
@@ -1201,6 +1204,7 @@ static int cscfg_create_device(void)
 	INIT_LIST_HEAD(&cscfg_mgr->load_order_list);
 	atomic_set(&cscfg_mgr->sys_active_cnt, 0);
 	cscfg_mgr->load_state = CSCFG_NONE;
+	raw_spin_lock_init(&cscfg_mgr->spinlock);
 
 	/* setup the device */
 	dev = cscfg_device();
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
index 66e2db890d8203853a0c3c907b48aa66dd8014e6..f52aba4608dadc1c7666e0e28c9b43dc1c33d1eb 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.h
+++ b/drivers/hwtracing/coresight/coresight-syscfg.h
@@ -42,6 +42,7 @@ enum cscfg_load_ops {
  * @sysfs_active_config:Active config hash used if CoreSight controlled from sysfs.
  * @sysfs_active_preset:Active preset index used if CoreSight controlled from sysfs.
  * @load_state:		A multi-stage load/unload operation is in progress.
+ * @spinlock:		Exclusive access sysfs_active_* variables.
  */
 struct cscfg_manager {
 	struct device dev;
@@ -54,6 +55,7 @@ struct cscfg_manager {
 	u32 sysfs_active_config;
 	int sysfs_active_preset;
 	enum cscfg_load_ops load_state;
+	raw_spinlock_t spinlock;
 };
 
 /* get reference to dev in cscfg_manager */

-- 
2.34.1

Re: [PATCH v4 07/15] coresight: syscfg: Use spinlock to protect active variables

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> To support dynamically enabling and disabling the CoreSight path during
> CPU idle, cscfg_config_sysfs_get_active_cfg() may be called by the ETM
> driver in an atomic context. However, the function currently acquires a
> mutex, which is a sleepable lock, and this is not allowed in atomic
> context.
> 
> To make cscfg_config_sysfs_get_active_cfg() called in the idle flow,
> replace the mutex with a raw spinlock to protect the active
> variables.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
>   drivers/hwtracing/coresight/coresight-syscfg.c | 22 +++++++++++++---------
>   drivers/hwtracing/coresight/coresight-syscfg.h |  2 ++
>   2 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 6836b05986e8091b4f8ad7d082d6a016f8cf7d74..53aa1a44f9ac8f65b4e3cce06a33255abfc17351 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -957,15 +957,19 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti
>   
>   	cfg_hash = (unsigned long)config_desc->event_ea->var;
>   
> +	raw_spin_lock(&cscfg_mgr->spinlock);
> +
>   	if (activate) {
> +
>   		/* cannot be a current active value to activate this */
>   		if (cscfg_mgr->sysfs_active_config) {
>   			err = -EBUSY;
> -			goto exit_unlock;
> +		} else {
> +			err = _cscfg_activate_config(cfg_hash);
> +			if (!err)
> +				cscfg_mgr->sysfs_active_config = cfg_hash;
>   		}
> -		err = _cscfg_activate_config(cfg_hash);
> -		if (!err)
> -			cscfg_mgr->sysfs_active_config = cfg_hash;
> +
>   	} else {
>   		/* disable if matching current value */
>   		if (cscfg_mgr->sysfs_active_config == cfg_hash) {
> @@ -975,7 +979,8 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti
>   			err = -EINVAL;
>   	}
>   
> -exit_unlock:
> +	raw_spin_unlock(&cscfg_mgr->spinlock);
> +
>   	mutex_unlock(&cscfg_mutex);
>   	return err;
>   }
> @@ -983,9 +988,8 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti
>   /* set the sysfs preset value */
>   void cscfg_config_sysfs_set_preset(int preset)
>   {
> -	mutex_lock(&cscfg_mutex);
> +	guard(raw_spinlock)(&cscfg_mgr->spinlock);
>   	cscfg_mgr->sysfs_active_preset = preset;
> -	mutex_unlock(&cscfg_mutex);
>   }
>   
>   /*
> @@ -994,10 +998,9 @@ void cscfg_config_sysfs_set_preset(int preset)
>    */
>   void cscfg_config_sysfs_get_active_cfg(unsigned long *cfg_hash, int *preset)
>   {
> -	mutex_lock(&cscfg_mutex);
> +	guard(raw_spinlock)(&cscfg_mgr->spinlock);
>   	*preset = cscfg_mgr->sysfs_active_preset;
>   	*cfg_hash = cscfg_mgr->sysfs_active_config;
> -	mutex_unlock(&cscfg_mutex);
>   }
>   EXPORT_SYMBOL_GPL(cscfg_config_sysfs_get_active_cfg);
>   
> @@ -1201,6 +1204,7 @@ static int cscfg_create_device(void)
>   	INIT_LIST_HEAD(&cscfg_mgr->load_order_list);
>   	atomic_set(&cscfg_mgr->sys_active_cnt, 0);
>   	cscfg_mgr->load_state = CSCFG_NONE;
> +	raw_spin_lock_init(&cscfg_mgr->spinlock);
>   
>   	/* setup the device */
>   	dev = cscfg_device();
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> index 66e2db890d8203853a0c3c907b48aa66dd8014e6..f52aba4608dadc1c7666e0e28c9b43dc1c33d1eb 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> @@ -42,6 +42,7 @@ enum cscfg_load_ops {
>    * @sysfs_active_config:Active config hash used if CoreSight controlled from sysfs.
>    * @sysfs_active_preset:Active preset index used if CoreSight controlled from sysfs.
>    * @load_state:		A multi-stage load/unload operation is in progress.
> + * @spinlock:		Exclusive access sysfs_active_* variables.
>    */
>   struct cscfg_manager {
>   	struct device dev;
> @@ -54,6 +55,7 @@ struct cscfg_manager {
>   	u32 sysfs_active_config;
>   	int sysfs_active_preset;
>   	enum cscfg_load_ops load_state;
> +	raw_spinlock_t spinlock;
>   };
>   
>   /* get reference to dev in cscfg_manager */
> 

[PATCH v4 08/15] coresight: Introduce coresight_enable_source() helper

[Leo Yan]

Introduce the coresight_enable_source() helper for enabling source
device, refine the comment for the imbalance between enable and disable
pair functions.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c     | 15 ++++++++++++---
 drivers/hwtracing/coresight/coresight-etm-perf.c |  2 +-
 drivers/hwtracing/coresight/coresight-priv.h     |  3 +++
 drivers/hwtracing/coresight/coresight-sysfs.c    |  2 +-
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 0d4e935094297023b8a7786ff4b1b3783164016b..c28f3d255b0e19b3d982de95b8e34d0fc2954b95 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -385,15 +385,24 @@ static void coresight_disable_helpers(struct coresight_device *csdev, void *data
 }
 
 /*
- * Helper function to call source_ops(csdev)->disable and also disable the
- * helpers.
+ * coresight_enable_source() only enables the source but does nothing for the
+ * associated helpers. In contrast, coresight_disable_source() calls
+ * source_ops(csdev)->disable() and also disables the helpers.
  *
  * There is an imbalance between coresight_enable_path() and
  * coresight_disable_path(). Enabling also enables the source's helpers as part
  * of the path, but disabling always skips the first item in the path (which is
  * the source), so sources and their helpers don't get disabled as part of that
- * function and we need the extra step here.
+ * function and we need the extra step in coresight_disable_source().
  */
+int coresight_enable_source(struct coresight_device *csdev,
+			    struct perf_event *event, enum cs_mode mode,
+			    struct coresight_path *path)
+{
+	return source_ops(csdev)->enable(csdev, event, mode, path);
+}
+EXPORT_SYMBOL_GPL(coresight_enable_source);
+
 void coresight_disable_source(struct coresight_device *csdev, void *data)
 {
 	source_ops(csdev)->disable(csdev, data);
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index f677c08233ba1a28b277674662c6e6db904873dd..dd1968dd173d608fb72835343cd647818c0bad1a 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -530,7 +530,7 @@ static void etm_event_start(struct perf_event *event, int flags)
 		goto fail_end_stop;
 
 	/* Finally enable the tracer */
-	if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF, path))
+	if (coresight_enable_source(csdev, event, CS_MODE_PERF, path))
 		goto fail_disable_path;
 
 	/*
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 33e22b1ba043264ad50acac69ece1ea6de25893b..8b978528cb49af43ef56d034f7ecc520f80f60df 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -250,6 +250,9 @@ void coresight_add_helper(struct coresight_device *csdev,
 
 void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev);
 struct coresight_device *coresight_get_percpu_sink(int cpu);
+int coresight_enable_source(struct coresight_device *csdev,
+			    struct perf_event *event, enum cs_mode mode,
+			    struct coresight_path *path);
 void coresight_disable_source(struct coresight_device *csdev, void *data);
 void coresight_pause_source(struct coresight_device *csdev);
 int coresight_resume_source(struct coresight_device *csdev);
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index cb02f1bfaf068ec3f50f997bcb94f7d5215ccea8..72e9607fc6aea4663abf7d1f7f03f247ea060b83 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -66,7 +66,7 @@ static int coresight_enable_source_sysfs(struct coresight_device *csdev,
 	 */
 	lockdep_assert_held(&coresight_mutex);
 	if (coresight_get_mode(csdev) != CS_MODE_SYSFS) {
-		ret = source_ops(csdev)->enable(csdev, NULL, mode, path);
+		ret = coresight_enable_source(csdev, NULL, mode, path);
 		if (ret)
 			return ret;
 	}

-- 
2.34.1

Re: [PATCH v4 08/15] coresight: Introduce coresight_enable_source() helper

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> Introduce the coresight_enable_source() helper for enabling source
> device, refine the comment for the imbalance between enable and disable
> pair functions.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
>   drivers/hwtracing/coresight/coresight-core.c     | 15 ++++++++++++---
>   drivers/hwtracing/coresight/coresight-etm-perf.c |  2 +-
>   drivers/hwtracing/coresight/coresight-priv.h     |  3 +++
>   drivers/hwtracing/coresight/coresight-sysfs.c    |  2 +-
>   4 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 0d4e935094297023b8a7786ff4b1b3783164016b..c28f3d255b0e19b3d982de95b8e34d0fc2954b95 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -385,15 +385,24 @@ static void coresight_disable_helpers(struct coresight_device *csdev, void *data
>   }
>   
>   /*
> - * Helper function to call source_ops(csdev)->disable and also disable the
> - * helpers.
> + * coresight_enable_source() only enables the source but does nothing for the
> + * associated helpers. In contrast, coresight_disable_source() calls
> + * source_ops(csdev)->disable() and also disables the helpers.
>    *
>    * There is an imbalance between coresight_enable_path() and
>    * coresight_disable_path(). Enabling also enables the source's helpers as part
>    * of the path, but disabling always skips the first item in the path (which is
>    * the source), so sources and their helpers don't get disabled as part of that
> - * function and we need the extra step here.
> + * function and we need the extra step in coresight_disable_source().
>    */
> +int coresight_enable_source(struct coresight_device *csdev,
> +			    struct perf_event *event, enum cs_mode mode,
> +			    struct coresight_path *path)
> +{
> +	return source_ops(csdev)->enable(csdev, event, mode, path);
> +}
> +EXPORT_SYMBOL_GPL(coresight_enable_source);
> +
>   void coresight_disable_source(struct coresight_device *csdev, void *data)
>   {
>   	source_ops(csdev)->disable(csdev, data);
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index f677c08233ba1a28b277674662c6e6db904873dd..dd1968dd173d608fb72835343cd647818c0bad1a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -530,7 +530,7 @@ static void etm_event_start(struct perf_event *event, int flags)
>   		goto fail_end_stop;
>   
>   	/* Finally enable the tracer */
> -	if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF, path))
> +	if (coresight_enable_source(csdev, event, CS_MODE_PERF, path))
>   		goto fail_disable_path;
>   
>   	/*
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 33e22b1ba043264ad50acac69ece1ea6de25893b..8b978528cb49af43ef56d034f7ecc520f80f60df 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -250,6 +250,9 @@ void coresight_add_helper(struct coresight_device *csdev,
>   
>   void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev);
>   struct coresight_device *coresight_get_percpu_sink(int cpu);
> +int coresight_enable_source(struct coresight_device *csdev,
> +			    struct perf_event *event, enum cs_mode mode,
> +			    struct coresight_path *path);
>   void coresight_disable_source(struct coresight_device *csdev, void *data);
>   void coresight_pause_source(struct coresight_device *csdev);
>   int coresight_resume_source(struct coresight_device *csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index cb02f1bfaf068ec3f50f997bcb94f7d5215ccea8..72e9607fc6aea4663abf7d1f7f03f247ea060b83 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -66,7 +66,7 @@ static int coresight_enable_source_sysfs(struct coresight_device *csdev,
>   	 */
>   	lockdep_assert_held(&coresight_mutex);
>   	if (coresight_get_mode(csdev) != CS_MODE_SYSFS) {
> -		ret = source_ops(csdev)->enable(csdev, NULL, mode, path);
> +		ret = coresight_enable_source(csdev, NULL, mode, path);
>   		if (ret)
>   			return ret;
>   	}
> 

[PATCH v4 09/15] coresight: Save activated path into source device

[Leo Yan]

Save activated path into the source device's coresight_device structure.
The path pointer 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.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index c28f3d255b0e19b3d982de95b8e34d0fc2954b95..3ea31ed121f7b59d7822fba4df4c43efb1c76fe7 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -399,13 +399,50 @@ int coresight_enable_source(struct coresight_device *csdev,
 			    struct perf_event *event, enum cs_mode mode,
 			    struct coresight_path *path)
 {
-	return source_ops(csdev)->enable(csdev, event, mode, path);
+	int ret;
+
+	/*
+	 * Record the path in the source device. The path pointer is first
+	 * assigned, followed by transitioning from DISABLED mode to an enabled
+	 * state on the target CPU. Conversely, during the disable flow, the
+	 * device mode is set to DISABLED before the path pointer is cleared.
+	 *
+	 * This ordering ensures the path pointer to be safely access under the
+	 * following race condition:
+	 *
+	 *  CPU(a)                          CPU(b)
+	 *
+	 *  coresight_enable_source()
+	 *    STORE source->path;
+	 *    smp_mb();
+	 *    source_ops(csdev)->enable();
+	 *                              `-> etm4_enable_sysfs_smp_call()
+	 *                                    STORE source->mode;
+	 *
+	 * This sequence ensures that accessing the path pointer is safe when
+	 * the device is in enabled mode.
+	 */
+	csdev->path = path;
+
+	/* Synchronization between csdev->path and csdev->mode */
+	smp_mb();
+
+	ret = source_ops(csdev)->enable(csdev, event, mode, path);
+	if (ret)
+		csdev->path = NULL;
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(coresight_enable_source);
 
 void coresight_disable_source(struct coresight_device *csdev, void *data)
 {
 	source_ops(csdev)->disable(csdev, data);
+
+	/* Synchronization between csdev->path and csdev->mode */
+	smp_mb();
+	csdev->path = NULL;
+
 	coresight_disable_helpers(csdev, NULL);
 }
 EXPORT_SYMBOL_GPL(coresight_disable_source);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 3d59be214dd25dfa7ad9148a6688628e0d1a98dd..58484c225e58a68dd74739a48c08a409ce9ddd73 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -264,6 +264,7 @@ struct coresight_trace_id_map {
  *		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).
+ * @path:	Activated path pointer (only used for source device).
  * @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
@@ -291,6 +292,7 @@ struct coresight_device {
 	int refcnt;
 	bool orphan;
 	int cpu;
+	struct coresight_path *path;
 	/* sink specific fields */
 	bool sysfs_sink_activated;
 	struct dev_ext_attribute *ea;

-- 
2.34.1

Re: [PATCH v4 09/15] coresight: Save activated path into source device

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> Save activated path into the source device's coresight_device structure.
> The path pointer 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.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 39 +++++++++++++++++++++++++++-
>   include/linux/coresight.h                    |  2 ++
>   2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c28f3d255b0e19b3d982de95b8e34d0fc2954b95..3ea31ed121f7b59d7822fba4df4c43efb1c76fe7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -399,13 +399,50 @@ int coresight_enable_source(struct coresight_device *csdev,
>   			    struct perf_event *event, enum cs_mode mode,
>   			    struct coresight_path *path)
>   {
> -	return source_ops(csdev)->enable(csdev, event, mode, path);
> +	int ret;
> +
> +	/*
> +	 * Record the path in the source device. The path pointer is first
> +	 * assigned, followed by transitioning from DISABLED mode to an enabled
> +	 * state on the target CPU. Conversely, during the disable flow, the
> +	 * device mode is set to DISABLED before the path pointer is cleared.
> +	 *
> +	 * This ordering ensures the path pointer to be safely access under the
> +	 * following race condition:
> +	 *
> +	 *  CPU(a)                          CPU(b)
> +	 *
> +	 *  coresight_enable_source()
> +	 *    STORE source->path;
> +	 *    smp_mb();
> +	 *    source_ops(csdev)->enable();
> +	 *                              `-> etm4_enable_sysfs_smp_call()
> +	 *                                    STORE source->mode;
> +	 *
> +	 * This sequence ensures that accessing the path pointer is safe when
> +	 * the device is in enabled mode.

Doesn't that only work if you meticulously use READ_ONCE() for accessing 
path on the read side? Which doesn't look like it has been done.

I'm not sure why path is special though, there are plenty of variables 
in csdev that are accessed while the device is active. Shouldn't path be 
covered by the existing locks in the same way? It would be much safer 
and easier to understand if it was.

> +	 */
> +	csdev->path = path;
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +
> +	ret = source_ops(csdev)->enable(csdev, event, mode, path);
> +	if (ret)
> +		csdev->path = NULL;
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(coresight_enable_source);
>   
>   void coresight_disable_source(struct coresight_device *csdev, void *data)
>   {
>   	source_ops(csdev)->disable(csdev, data);
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +	csdev->path = NULL;
> +
>   	coresight_disable_helpers(csdev, NULL);
>   }
>   EXPORT_SYMBOL_GPL(coresight_disable_source);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 3d59be214dd25dfa7ad9148a6688628e0d1a98dd..58484c225e58a68dd74739a48c08a409ce9ddd73 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -264,6 +264,7 @@ struct coresight_trace_id_map {
>    *		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).
> + * @path:	Activated path pointer (only used for source device).
>    * @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
> @@ -291,6 +292,7 @@ struct coresight_device {
>   	int refcnt;
>   	bool orphan;
>   	int cpu;
> +	struct coresight_path *path;
>   	/* sink specific fields */
>   	bool sysfs_sink_activated;
>   	struct dev_ext_attribute *ea;
> 

Re: [PATCH v4 09/15] coresight: Save activated path into source device

[Leo Yan]

On Mon, Nov 10, 2025 at 11:18:02AM +0000, James Clark wrote:

[...]

> > @@ -399,13 +399,50 @@ int coresight_enable_source(struct coresight_device *csdev,
> >   			    struct perf_event *event, enum cs_mode mode,
> >   			    struct coresight_path *path)
> >   {
> > -	return source_ops(csdev)->enable(csdev, event, mode, path);
> > +	int ret;
> > +
> > +	/*
> > +	 * Record the path in the source device. The path pointer is first
> > +	 * assigned, followed by transitioning from DISABLED mode to an enabled
> > +	 * state on the target CPU. Conversely, during the disable flow, the
> > +	 * device mode is set to DISABLED before the path pointer is cleared.
> > +	 *
> > +	 * This ordering ensures the path pointer to be safely access under the
> > +	 * following race condition:
> > +	 *
> > +	 *  CPU(a)                          CPU(b)
> > +	 *
> > +	 *  coresight_enable_source()
> > +	 *    STORE source->path;
> > +	 *    smp_mb();
> > +	 *    source_ops(csdev)->enable();
> > +	 *                              `-> etm4_enable_sysfs_smp_call()
> > +	 *                                    STORE source->mode;
> > +	 *
> > +	 * This sequence ensures that accessing the path pointer is safe when
> > +	 * the device is in enabled mode.
> 
> Doesn't that only work if you meticulously use READ_ONCE() for accessing
> path on the read side? Which doesn't look like it has been done.

As Suzuki suggested, I will move the path setting onto the target CPU
(along with the mode setting).  Since the coresight mode functions
already have applied memory barriers, we might can ensure the data
sequence between mode and path if all run on the target CPU, no need to
introduce new barriers.

> I'm not sure why path is special though, there are plenty of variables in
> csdev that are accessed while the device is active. Shouldn't path be
> covered by the existing locks in the same way? It would be much safer and
> easier to understand if it was.

Unlike other fields in csdev, path can be retrieved on SMP cores, and
we don't want to introduce race condition between low level's CPU idle
and high level's sysfs knobs, we don't use spinlock to protect it.

We relies on state machine (device's mode) for safely access the path
pointer.

Thanks,
Leo

Re: [PATCH v4 09/15] coresight: Save activated path into source device

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> Save activated path into the source device's coresight_device structure.
> The path pointer 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.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 39 +++++++++++++++++++++++++++-
>   include/linux/coresight.h                    |  2 ++
>   2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c28f3d255b0e19b3d982de95b8e34d0fc2954b95..3ea31ed121f7b59d7822fba4df4c43efb1c76fe7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -399,13 +399,50 @@ int coresight_enable_source(struct coresight_device *csdev,
>   			    struct perf_event *event, enum cs_mode mode,
>   			    struct coresight_path *path)
>   {
> -	return source_ops(csdev)->enable(csdev, event, mode, path);
> +	int ret;
> +
> +	/*
> +	 * Record the path in the source device. The path pointer is first
> +	 * assigned, followed by transitioning from DISABLED mode to an enabled
> +	 * state on the target CPU. Conversely, during the disable flow, the
> +	 * device mode is set to DISABLED before the path pointer is cleared.
> +	 *
> +	 * This ordering ensures the path pointer to be safely access under the
> +	 * following race condition:
> +	 *
> +	 *  CPU(a)                          CPU(b)
> +	 *
> +	 *  coresight_enable_source()
> +	 *    STORE source->path;
> +	 *    smp_mb();
> +	 *    source_ops(csdev)->enable();
> +	 *                              `-> etm4_enable_sysfs_smp_call()
> +	 *                                    STORE source->mode;
> +	 *
> +	 * This sequence ensures that accessing the path pointer is safe when
> +	 * the device is in enabled mode.
> +	 */
> +	csdev->path = path;
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +
> +	ret = source_ops(csdev)->enable(csdev, event, mode, path);
> +	if (ret)
> +		csdev->path = NULL;
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(coresight_enable_source);
>   
>   void coresight_disable_source(struct coresight_device *csdev, void *data)
>   {
>   	source_ops(csdev)->disable(csdev, data);
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +	csdev->path = NULL;
> +
>   	coresight_disable_helpers(csdev, NULL);
>   }
>   EXPORT_SYMBOL_GPL(coresight_disable_source);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 3d59be214dd25dfa7ad9148a6688628e0d1a98dd..58484c225e58a68dd74739a48c08a409ce9ddd73 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -264,6 +264,7 @@ struct coresight_trace_id_map {
>    *		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).
> + * @path:	Activated path pointer (only used for source device).

Isn't it only needed for per-cpu sources specifically? Rather than all 
sources?

That's why I still think it should be a new global per-cpu variable in 
coresight-core.c, rather than a generic thing that needs to be used very 
carefully. Sysfs mode also already has a global map and per-cpu store of 
its paths to sources. This makes it a bit confusing because it 
duplicates some of that functionality.

Do we even need that storage in sysfs mode anymore if the path is here? 
We should pick one way and stick with it.

>    * @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
> @@ -291,6 +292,7 @@ struct coresight_device {
>   	int refcnt;
>   	bool orphan;
>   	int cpu;
> +	struct coresight_path *path;
>   	/* sink specific fields */
>   	bool sysfs_sink_activated;
>   	struct dev_ext_attribute *ea;
> 

Re: [PATCH v4 09/15] coresight: Save activated path into source device

[Suzuki K Poulose]

On 04/11/2025 15:21, Leo Yan wrote:
> Save activated path into the source device's coresight_device structure.
> The path pointer 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.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 39 +++++++++++++++++++++++++++-
>   include/linux/coresight.h                    |  2 ++
>   2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c28f3d255b0e19b3d982de95b8e34d0fc2954b95..3ea31ed121f7b59d7822fba4df4c43efb1c76fe7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -399,13 +399,50 @@ int coresight_enable_source(struct coresight_device *csdev,
>   			    struct perf_event *event, enum cs_mode mode,
>   			    struct coresight_path *path)
>   {
> -	return source_ops(csdev)->enable(csdev, event, mode, path);
> +	int ret;
> +
> +	/*
> +	 * Record the path in the source device. The path pointer is first
> +	 * assigned, followed by transitioning from DISABLED mode to an enabled
> +	 * state on the target CPU. Conversely, during the disable flow, the
> +	 * device mode is set to DISABLED before the path pointer is cleared.
> +	 *
> +	 * This ordering ensures the path pointer to be safely access under the
> +	 * following race condition:
> +	 *
> +	 *  CPU(a)                          CPU(b)
> +	 *
> +	 *  coresight_enable_source()
> +	 *    STORE source->path;
> +	 *    smp_mb();
> +	 *    source_ops(csdev)->enable();
> +	 *                              `-> etm4_enable_sysfs_smp_call()
> +	 *                                    STORE source->mode;
> +	 *
> +	 * This sequence ensures that accessing the path pointer is safe when
> +	 * the device is in enabled mode.
> +	 */
> +	csdev->path = path;
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +
> +	ret = source_ops(csdev)->enable(csdev, event, mode, path);

What happens if the csdev is already enabled ? We corrupt the 
csdev->path ? Can we not move this into the source devices and let them
handle it gracefully, as they do for the "mode" ?

To address James' comment on reusing the sysfs path, I think we could
make this a global thing for all types of source/mode.

Suzuki


> +	if (ret)
> +		csdev->path = NULL;
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(coresight_enable_source);
>   
>   void coresight_disable_source(struct coresight_device *csdev, void *data)
>   {
>   	source_ops(csdev)->disable(csdev, data);
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +	csdev->path = NULL;
> +
>   	coresight_disable_helpers(csdev, NULL);
>   }
>   EXPORT_SYMBOL_GPL(coresight_disable_source);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 3d59be214dd25dfa7ad9148a6688628e0d1a98dd..58484c225e58a68dd74739a48c08a409ce9ddd73 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -264,6 +264,7 @@ struct coresight_trace_id_map {
>    *		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).
> + * @path:	Activated path pointer (only used for source device).
>    * @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
> @@ -291,6 +292,7 @@ struct coresight_device {
>   	int refcnt;
>   	bool orphan;
>   	int cpu;
> +	struct coresight_path *path;
>   	/* sink specific fields */
>   	bool sysfs_sink_activated;
>   	struct dev_ext_attribute *ea;
> 

Re: [PATCH v4 09/15] coresight: Save activated path into source device

[Leo Yan]

On Tue, Nov 11, 2025 at 02:10:10PM +0000, Suzuki Kuruppassery Poulose wrote:

[...]

> > @@ -399,13 +399,50 @@ int coresight_enable_source(struct coresight_device *csdev,
> >   			    struct perf_event *event, enum cs_mode mode,
> >   			    struct coresight_path *path)
> >   {
> > -	return source_ops(csdev)->enable(csdev, event, mode, path);
> > +	int ret;
> > +
> > +	/*
> > +	 * Record the path in the source device. The path pointer is first
> > +	 * assigned, followed by transitioning from DISABLED mode to an enabled
> > +	 * state on the target CPU. Conversely, during the disable flow, the
> > +	 * device mode is set to DISABLED before the path pointer is cleared.
> > +	 *
> > +	 * This ordering ensures the path pointer to be safely access under the
> > +	 * following race condition:
> > +	 *
> > +	 *  CPU(a)                          CPU(b)
> > +	 *
> > +	 *  coresight_enable_source()
> > +	 *    STORE source->path;
> > +	 *    smp_mb();
> > +	 *    source_ops(csdev)->enable();
> > +	 *                              `-> etm4_enable_sysfs_smp_call()
> > +	 *                                    STORE source->mode;
> > +	 *
> > +	 * This sequence ensures that accessing the path pointer is safe when
> > +	 * the device is in enabled mode.
> > +	 */
> > +	csdev->path = path;
> > +
> > +	/* Synchronization between csdev->path and csdev->mode */
> > +	smp_mb();
> > +
> > +	ret = source_ops(csdev)->enable(csdev, event, mode, path);
> 
> What happens if the csdev is already enabled ? We corrupt the csdev->path ?
> Can we not move this into the source devices and let them
> handle it gracefully, as they do for the "mode" ?

Indeed.  I will move the path setting on the target CPU, along with
setting mode.

> To address James' comment on reusing the sysfs path, I think we could
> make this a global thing for all types of source/mode.

In the previous version, I used a global variable to store path pointer,
but in this version I moved back the path pointer into csdev for two
reasons:

- Except we need to store (per-CPU) ETM's path pointer, we also need
  extra data structure for storing system source.

- For both perf and sysfs sessions, we need to retrieve path pointer
  based on the given source's csdev.  And we need to use the source
  device's mode to decide if the path poiter is valid.

For above reasons, I prefer to store the path pointer into the source
device's csdev - based on the csdev, it is easy to retrieve path pointer,
and it is easy to access the other associated info (mode).

Thanks,
Leo

[PATCH v4 10/15] coresight: Add 'in_idle' argument to enable/disable path functions

[Leo Yan]

Introduce an 'in_idle' argument to the path enable and disable
functions. When set to true for idle flow, it skips to touch the sink
device to avoid long latency caused by the sink operations.

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 | 35 +++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 3ea31ed121f7b59d7822fba4df4c43efb1c76fe7..6c12692efb51c63f3aa9da6f65a349eae1926f4d 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -475,7 +475,8 @@ EXPORT_SYMBOL_GPL(coresight_resume_source);
  * 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;
@@ -498,6 +499,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);
@@ -527,7 +532,7 @@ static void coresight_disable_path_from(struct coresight_path *path,
 
 void coresight_disable_path(struct coresight_path *path)
 {
-	coresight_disable_path_from(path, NULL);
+	coresight_disable_path_from(path, NULL, false);
 }
 EXPORT_SYMBOL_GPL(coresight_disable_path);
 
@@ -550,8 +555,9 @@ 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_internal(struct coresight_path *path,
+					  enum cs_mode mode, void *sink_data,
+					  bool in_idle)
 {
 	int ret = 0;
 	u32 type;
@@ -564,10 +570,6 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
 		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
@@ -579,6 +581,15 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
 						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);
@@ -614,10 +625,16 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
 err_disable_helpers:
 	coresight_disable_helpers(csdev, path);
 err_disable_path:
-	coresight_disable_path_from(path, nd);
+	coresight_disable_path_from(path, nd, in_idle);
 	goto out;
 }
 
+int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
+			  void *sink_data)
+{
+	return coresight_enable_path_internal(path, mode, sink_data, false);
+}
+
 struct coresight_device *coresight_get_sink(struct coresight_path *path)
 {
 	struct coresight_device *csdev;

-- 
2.34.1

Re: [PATCH v4 10/15] coresight: Add 'in_idle' argument to enable/disable path functions

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> Introduce an 'in_idle' argument to the path enable and disable
> functions. When set to true for idle flow, it skips to touch the sink
> device to avoid long latency caused by the sink operations.
> 
> 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 | 35 +++++++++++++++++++++-------
>   1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 3ea31ed121f7b59d7822fba4df4c43efb1c76fe7..6c12692efb51c63f3aa9da6f65a349eae1926f4d 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -475,7 +475,8 @@ EXPORT_SYMBOL_GPL(coresight_resume_source);
>    * 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;
> @@ -498,6 +499,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);
> @@ -527,7 +532,7 @@ static void coresight_disable_path_from(struct coresight_path *path,
>   
>   void coresight_disable_path(struct coresight_path *path)
>   {
> -	coresight_disable_path_from(path, NULL);
> +	coresight_disable_path_from(path, NULL, false);

Something like a 'reason' enum would be more readable than a boolean. We 
might have more than two options in the future anyway.

>   }
>   EXPORT_SYMBOL_GPL(coresight_disable_path);
>   
> @@ -550,8 +555,9 @@ 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_internal(struct coresight_path *path,
> +					  enum cs_mode mode, void *sink_data,
> +					  bool in_idle)
>   {
>   	int ret = 0;
>   	u32 type;
> @@ -564,10 +570,6 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
>   		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
> @@ -579,6 +581,15 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
>   						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;

I can imagine that not being wanted for all sink types in the future, so 
we could let the sink decide this. But it's probably fine to leave in 
the core for now and it would be easy to change later.

> +
> +		/* 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);
> @@ -614,10 +625,16 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
>   err_disable_helpers:
>   	coresight_disable_helpers(csdev, path);
>   err_disable_path:
> -	coresight_disable_path_from(path, nd);
> +	coresight_disable_path_from(path, nd, in_idle);
>   	goto out;
>   }
>   
> +int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
> +			  void *sink_data)
> +{
> +	return coresight_enable_path_internal(path, mode, sink_data, false);
> +}
> +
>   struct coresight_device *coresight_get_sink(struct coresight_path *path)
>   {
>   	struct coresight_device *csdev;
> 

Re: [PATCH v4 10/15] coresight: Add 'in_idle' argument to enable/disable path functions

[James Clark]

On 10/11/2025 11:25 am, James Clark wrote:
> 
> 
> On 04/11/2025 3:21 pm, Leo Yan wrote:
>> Introduce an 'in_idle' argument to the path enable and disable
>> functions. When set to true for idle flow, it skips to touch the sink
>> device to avoid long latency caused by the sink operations.
>>
>> 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 | 35 ++++++++++++++++++ 
>> +++-------
>>   1 file changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ 
>> hwtracing/coresight/coresight-core.c
>> index 
>> 3ea31ed121f7b59d7822fba4df4c43efb1c76fe7..6c12692efb51c63f3aa9da6f65a349eae1926f4d 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -475,7 +475,8 @@ EXPORT_SYMBOL_GPL(coresight_resume_source);
>>    * 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;
>> @@ -498,6 +499,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);
>> @@ -527,7 +532,7 @@ static void coresight_disable_path_from(struct 
>> coresight_path *path,
>>   void coresight_disable_path(struct coresight_path *path)
>>   {
>> -    coresight_disable_path_from(path, NULL);
>> +    coresight_disable_path_from(path, NULL, false);
> 
> Something like a 'reason' enum would be more readable than a boolean. We 
> might have more than two options in the future anyway.
> 
>>   }
>>   EXPORT_SYMBOL_GPL(coresight_disable_path);
>> @@ -550,8 +555,9 @@ 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_internal(struct coresight_path *path,
>> +                      enum cs_mode mode, void *sink_data,
>> +                      bool in_idle)
>>   {
>>       int ret = 0;
>>       u32 type;
>> @@ -564,10 +570,6 @@ int coresight_enable_path(struct coresight_path 
>> *path, enum cs_mode mode,
>>           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
>> @@ -579,6 +581,15 @@ int coresight_enable_path(struct coresight_path 
>> *path, enum cs_mode mode,
>>                           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;
> 
> I can imagine that not being wanted for all sink types in the future, so 
> we could let the sink decide this. But it's probably fine to leave in 
> the core for now and it would be easy to change later.

I got to patch 12 and it adds a special condition in the core to do 
something different for TRBE. We already have pm_is_needed() so 
shouldn't the sinks be deciding this for themselves?

> 
>> +
>> +        /* 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);
>> @@ -614,10 +625,16 @@ int coresight_enable_path(struct coresight_path 
>> *path, enum cs_mode mode,
>>   err_disable_helpers:
>>       coresight_disable_helpers(csdev, path);
>>   err_disable_path:
>> -    coresight_disable_path_from(path, nd);
>> +    coresight_disable_path_from(path, nd, in_idle);
>>       goto out;
>>   }
>> +int coresight_enable_path(struct coresight_path *path, enum cs_mode 
>> mode,
>> +              void *sink_data)
>> +{
>> +    return coresight_enable_path_internal(path, mode, sink_data, false);
>> +}
>> +
>>   struct coresight_device *coresight_get_sink(struct coresight_path 
>> *path)
>>   {
>>       struct coresight_device *csdev;
>>
> 

Re: [PATCH v4 10/15] coresight: Add 'in_idle' argument to enable/disable path functions

[Leo Yan]

On Mon, Nov 10, 2025 at 11:48:45AM +0000, James Clark wrote:

[...]

> > >   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;
> > > @@ -498,6 +499,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);
> > > @@ -527,7 +532,7 @@ static void coresight_disable_path_from(struct
> > > coresight_path *path,
> > >   void coresight_disable_path(struct coresight_path *path)
> > >   {
> > > -    coresight_disable_path_from(path, NULL);
> > > +    coresight_disable_path_from(path, NULL, false);
> > 
> > Something like a 'reason' enum would be more readable than a boolean. We
> > might have more than two options in the future anyway.

I will add a "path->in_idle" flag instead.

> > >   }
> > >   EXPORT_SYMBOL_GPL(coresight_disable_path);
> > > @@ -550,8 +555,9 @@ 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_internal(struct coresight_path *path,
> > > +                      enum cs_mode mode, void *sink_data,
> > > +                      bool in_idle)
> > >   {
> > >       int ret = 0;
> > >       u32 type;
> > > @@ -564,10 +570,6 @@ int coresight_enable_path(struct coresight_path
> > > *path, enum cs_mode mode,
> > >           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
> > > @@ -579,6 +581,15 @@ int coresight_enable_path(struct coresight_path
> > > *path, enum cs_mode mode,
> > >                           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;
> > 
> > I can imagine that not being wanted for all sink types in the future, so
> > we could let the sink decide this. But it's probably fine to leave in
> > the core for now and it would be easy to change later.
> 
> I got to patch 12 and it adds a special condition in the core to do
> something different for TRBE. We already have pm_is_needed() so shouldn't
> the sinks be deciding this for themselves?

pm_is_needed() is related to pm_save_disable()/pm_restore_enable()
pairs. OTOH, coresight_enable_path() invokes enable()/disable()
callbacks.  I don't want to mix up two different things.

But I will check if can use pm_is_needed() for sink device in
cpu_pm_save()/cpu_pm_restore().

Thanks,
Leo

[PATCH v4 11/15] 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 | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 6c12692efb51c63f3aa9da6f65a349eae1926f4d..73be22c8f315525111f2dafd692e61bc6db85c25 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1661,11 +1661,31 @@ static bool coresight_pm_is_needed(struct coresight_device *csdev)
 
 static int coresight_pm_save(struct coresight_device *csdev)
 {
-	return coresight_ops(csdev)->pm_save_disable(csdev);
+	int ret;
+
+	if (WARN_ON(!csdev->path))
+		return -EINVAL;
+
+	ret = coresight_ops(csdev)->pm_save_disable(csdev);
+	if (ret)
+		return ret;
+
+	coresight_disable_helpers(csdev, NULL);
+	coresight_disable_path_from(csdev->path, NULL, true);
+	return 0;
 }
 
 static void coresight_pm_restore(struct coresight_device *csdev)
 {
+	if (WARN_ON(!csdev->path))
+		return;
+
+	/*
+	 * During CPU idle, the sink device is not accessed, so it is okay to
+	 * pass a NULL pointer for the 'sink_data' parameter.
+	 */
+	coresight_enable_path_internal(csdev->path, coresight_get_mode(csdev),
+				       NULL, true);
 	coresight_ops(csdev)->pm_restore_enable(csdev);
 }
 

-- 
2.34.1

Re: [PATCH v4 11/15] coresight: Control path during CPU idle

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> 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>

Reviewed-by: James Clark <james.clark@linaro.org>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 22 +++++++++++++++++++++-
>   1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6c12692efb51c63f3aa9da6f65a349eae1926f4d..73be22c8f315525111f2dafd692e61bc6db85c25 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1661,11 +1661,31 @@ static bool coresight_pm_is_needed(struct coresight_device *csdev)
>   
>   static int coresight_pm_save(struct coresight_device *csdev)
>   {
> -	return coresight_ops(csdev)->pm_save_disable(csdev);
> +	int ret;
> +
> +	if (WARN_ON(!csdev->path))
> +		return -EINVAL;
> +
> +	ret = coresight_ops(csdev)->pm_save_disable(csdev);
> +	if (ret)
> +		return ret;
> +
> +	coresight_disable_helpers(csdev, NULL);
> +	coresight_disable_path_from(csdev->path, NULL, true);
> +	return 0;
>   }
>   
>   static void coresight_pm_restore(struct coresight_device *csdev)
>   {
> +	if (WARN_ON(!csdev->path))
> +		return;
> +
> +	/*
> +	 * During CPU idle, the sink device is not accessed, so it is okay to
> +	 * pass a NULL pointer for the 'sink_data' parameter.
> +	 */
> +	coresight_enable_path_internal(csdev->path, coresight_get_mode(csdev),
> +				       NULL, true);
>   	coresight_ops(csdev)->pm_restore_enable(csdev);
>   }
>   
> 

[PATCH v4 12/15] 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.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 73be22c8f315525111f2dafd692e61bc6db85c25..7095f9befa98c7d638f857466351ef7cd960fde7 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1659,8 +1659,25 @@ static bool coresight_pm_is_needed(struct coresight_device *csdev)
 	return coresight_ops(csdev)->pm_is_needed(csdev);
 }
 
+static int coresight_pm_percpu_sink_save(struct coresight_device *sink)
+{
+	if (!sink || !coresight_ops(sink)->pm_save_disable)
+		return 0;
+
+	return coresight_ops(sink)->pm_save_disable(sink);
+}
+
+static void coresight_pm_percpu_sink_restore(struct coresight_device *sink)
+{
+	if (!sink || !coresight_ops(sink)->pm_restore_enable)
+		return;
+
+	coresight_ops(sink)->pm_restore_enable(sink);
+}
+
 static int coresight_pm_save(struct coresight_device *csdev)
 {
+	struct coresight_device *sink;
 	int ret;
 
 	if (WARN_ON(!csdev->path))
@@ -1670,22 +1687,42 @@ static int coresight_pm_save(struct coresight_device *csdev)
 	if (ret)
 		return ret;
 
-	coresight_disable_helpers(csdev, NULL);
-	coresight_disable_path_from(csdev->path, NULL, true);
+	sink = coresight_get_sink(csdev->path);
+
+	if (coresight_is_percpu_sink(sink)) {
+		ret = coresight_pm_percpu_sink_save(sink);
+		if (ret) {
+			coresight_ops(csdev)->pm_restore_enable(csdev);
+			return ret;
+		}
+	} else {
+		coresight_disable_helpers(csdev, NULL);
+		coresight_disable_path_from(csdev->path, NULL, true);
+	}
+
 	return 0;
 }
 
 static void coresight_pm_restore(struct coresight_device *csdev)
 {
+	struct coresight_device *sink;
+
 	if (WARN_ON(!csdev->path))
 		return;
 
-	/*
-	 * During CPU idle, the sink device is not accessed, so it is okay to
-	 * pass a NULL pointer for the 'sink_data' parameter.
-	 */
-	coresight_enable_path_internal(csdev->path, coresight_get_mode(csdev),
-				       NULL, true);
+	sink = coresight_get_sink(csdev->path);
+
+	if (coresight_is_percpu_sink(sink))
+		coresight_pm_percpu_sink_restore(sink);
+	else
+		/*
+		 * During CPU idle, the sink device is not accessed, so it is
+		 * okay to pass a NULL pointer for the 'sink_data' parameter.
+		 */
+		coresight_enable_path_internal(csdev->path,
+					       coresight_get_mode(csdev),
+					       NULL, true);
+
 	coresight_ops(csdev)->pm_restore_enable(csdev);
 }
 
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 58484c225e58a68dd74739a48c08a409ce9ddd73..d2f02f6322e69bfbda82f5df5d90cb8afeb7d21d 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -364,6 +364,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,
@@ -376,6 +378,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 v4 13/15] 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, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 3c82ab4394fde1e3dd4371a9b1703da4d8f6db9d..9888a153bc2f99b4826f6103bdbc12304c9c94cb 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -116,6 +116,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
@@ -134,6 +148,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);
 };
 
@@ -1189,6 +1204,71 @@ 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 void 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;
+
+	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();
+}
+
 static const struct coresight_ops_sink arm_trbe_sink_ops = {
 	.enable		= arm_trbe_enable,
 	.disable	= arm_trbe_disable,
@@ -1198,7 +1278,9 @@ static const struct coresight_ops_sink arm_trbe_sink_ops = {
 };
 
 static const struct coresight_ops arm_trbe_cs_ops = {
-	.sink_ops	= &arm_trbe_sink_ops,
+	.pm_save_disable	= arm_trbe_save,
+	.pm_restore_enable	= arm_trbe_restore,
+	.sink_ops		= &arm_trbe_sink_ops,
 };
 
 static ssize_t align_show(struct device *dev, struct device_attribute *attr, char *buf)

-- 
2.34.1

Re: [PATCH v4 13/15] coresight: trbe: Save and restore state across CPU low power state

[James Clark]

On 04/11/2025 3:21 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>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 84 +++++++++++++++++++++++++++-
>   1 file changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 3c82ab4394fde1e3dd4371a9b1703da4d8f6db9d..9888a153bc2f99b4826f6103bdbc12304c9c94cb 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -116,6 +116,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
> @@ -134,6 +148,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);
>   };
>   
> @@ -1189,6 +1204,71 @@ 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();

Can we use set_trbe_disabled() here, and set_trbe_enabled() for restore 
below. There is a bit of duplication and they do the same thing except 
this version here always has trbe_drain_buffer() rather than it being 
conditional. I'm not sure if that is correct as they should both be the 
same?

set_trbe_disabled() already reads TRBLIMITR_EL1 so you can return it.

> +
> +	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 void 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;
> +
> +	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();

Especially this double isb() part shouldn't be duplicated as that could 
get broken in a refactor in the future.

> +}
> +
>   static const struct coresight_ops_sink arm_trbe_sink_ops = {
>   	.enable		= arm_trbe_enable,
>   	.disable	= arm_trbe_disable,
> @@ -1198,7 +1278,9 @@ static const struct coresight_ops_sink arm_trbe_sink_ops = {
>   };
>   
>   static const struct coresight_ops arm_trbe_cs_ops = {
> -	.sink_ops	= &arm_trbe_sink_ops,
> +	.pm_save_disable	= arm_trbe_save,
> +	.pm_restore_enable	= arm_trbe_restore,
> +	.sink_ops		= &arm_trbe_sink_ops,
>   };
>   
>   static ssize_t align_show(struct device *dev, struct device_attribute *attr, char *buf)
> 

Re: [PATCH v4 13/15] coresight: trbe: Save and restore state across CPU low power state

[Suzuki K Poulose]

On 10/11/2025 12:01, James Clark wrote:
> 
> 
> On 04/11/2025 3:21 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>
>> ---
>>   drivers/hwtracing/coresight/coresight-trbe.c | 84 ++++++++++++++++++ 
>> +++++++++-
>>   1 file changed, 83 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/ 
>> hwtracing/coresight/coresight-trbe.c
>> index 
>> 3c82ab4394fde1e3dd4371a9b1703da4d8f6db9d..9888a153bc2f99b4826f6103bdbc12304c9c94cb 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -116,6 +116,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
>> @@ -134,6 +148,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);
>>   };
>> @@ -1189,6 +1204,71 @@ 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();
> 
> Can we use set_trbe_disabled() here, and set_trbe_enabled() for restore 
> below. There is a bit of duplication and they do the same thing except 
> this version here always has trbe_drain_buffer() rather than it being 
> conditional. I'm not sure if that is correct as they should both be the 
> same?
> 
> set_trbe_disabled() already reads TRBLIMITR_EL1 so you can return it.

+1

>> +    if (trbe_needs_ctxt_sync_after_enable(cpudata))
>> +        isb();
> 
> Especially this double isb() part shouldn't be duplicated as that could 
> get broken in a refactor in the future.

+1

Suzuki

[PATCH v4 14/15] 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.

Reviewed-by: Mike Leach <mike.leach@linaro.org>
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 af2793cdb983c85f883efe10d7633f9cf718a44f..7bea3d2c6f49a2169b8e3de1cb07cc22147aae3a 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1090,13 +1090,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);
 
 	/*
@@ -1110,8 +1103,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 72e9607fc6aea4663abf7d1f7f03f247ea060b83..781290d8ecd63ef7d568d8045d422d87c6221e6b 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -363,6 +363,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 v4 15/15] 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 disables a source and the path when hotplug-off a CPU.

When hotplug-in a CPU, it does not re-enable even the trace was enabled
before hotplug-off, in this case, users are asked to re-enable the trace
via Sysfs interface.

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

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 7095f9befa98c7d638f857466351ef7cd960fde7..9149f028031808dc2b7285f621b97f3631a02b01 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1726,6 +1726,32 @@ static void coresight_pm_restore(struct coresight_device *csdev)
 	coresight_ops(csdev)->pm_restore_enable(csdev);
 }
 
+static int coresight_dying_cpu(unsigned int cpu)
+{
+	struct coresight_device *source = per_cpu(csdev_source, cpu);
+	struct coresight_path *path;
+
+	if (!source || !source->path)
+		return 0;
+
+	/*
+	 * The perf event layer will disable PMU events in the CPU hotplug.
+	 * CoreSight driver should never handle the CS_MODE_PERF case.
+	 */
+	if (coresight_get_mode(source) != CS_MODE_SYSFS)
+		return 0;
+
+	/*
+	 * Save 'source->path' here, as it will be cleared in
+	 * coresight_disable_source().
+	 */
+	path = source->path;
+
+	coresight_disable_source(source, NULL);
+	coresight_disable_path(path);
+	return 0;
+}
+
 static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
 				   void *v)
 {
@@ -1757,11 +1783,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",
+					NULL, 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 7bea3d2c6f49a2169b8e3de1cb07cc22147aae3a..1730cee3c3e1845efb2be815553085aba1246cca 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_save_restore_context(struct coresight_device *csdev)
 {
 	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
@@ -1915,13 +1888,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);
@@ -1932,14 +1898,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;

-- 
2.34.1

Re: [PATCH v4 15/15] coresight: Move CPU hotplug callbacks to core layer

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> 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 disables a source and the path when hotplug-off a CPU.
> 
> When hotplug-in a CPU, it does not re-enable even the trace was enabled
> before hotplug-off, in this case, users are asked to re-enable the trace
> via Sysfs interface.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c       | 41 +++++++++++++++++++++-
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 -------------------
>   2 files changed, 40 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 7095f9befa98c7d638f857466351ef7cd960fde7..9149f028031808dc2b7285f621b97f3631a02b01 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1726,6 +1726,32 @@ static void coresight_pm_restore(struct coresight_device *csdev)
>   	coresight_ops(csdev)->pm_restore_enable(csdev);
>   }
>   
> +static int coresight_dying_cpu(unsigned int cpu)
> +{
> +	struct coresight_device *source = per_cpu(csdev_source, cpu);
> +	struct coresight_path *path;
> +
> +	if (!source || !source->path)
> +		return 0;
> +
> +	/*
> +	 * The perf event layer will disable PMU events in the CPU hotplug.
> +	 * CoreSight driver should never handle the CS_MODE_PERF case.
> +	 */
> +	if (coresight_get_mode(source) != CS_MODE_SYSFS)
> +		return 0;
> +
> +	/*
> +	 * Save 'source->path' here, as it will be cleared in
> +	 * coresight_disable_source().
> +	 */
> +	path = source->path;
> +
> +	coresight_disable_source(source, NULL);
> +	coresight_disable_path(path);
> +	return 0;

If the user is expected to re-enable and this new state is visible, 
don't you need to use the regular coresight_disable_sysfs() function? It 
calls coresight_disable_source_sysfs() which updates a refcount.

Maybe you didn't do it because it has a mutex in it? It would be easier 
to change that to a spinlock or take the mutex in a wrapper function and 
share the core disabling code with the hotplug path.

> +}
> +
>   static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
>   				   void *v)
>   {
> @@ -1757,11 +1783,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",
> +					NULL, 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 7bea3d2c6f49a2169b8e3de1cb07cc22147aae3a..1730cee3c3e1845efb2be815553085aba1246cca 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_save_restore_context(struct coresight_device *csdev)
>   {
>   	if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED)
> @@ -1915,13 +1888,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);
> @@ -1932,14 +1898,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;
> 

Re: [PATCH v4 15/15] coresight: Move CPU hotplug callbacks to core layer

[Leo Yan]

On Mon, Nov 10, 2025 at 12:20:06PM +0000, James Clark wrote:

[...]

> > +static int coresight_dying_cpu(unsigned int cpu)
> > +{
> > +	struct coresight_device *source = per_cpu(csdev_source, cpu);
> > +	struct coresight_path *path;
> > +
> > +	if (!source || !source->path)
> > +		return 0;
> > +
> > +	/*
> > +	 * The perf event layer will disable PMU events in the CPU hotplug.
> > +	 * CoreSight driver should never handle the CS_MODE_PERF case.
> > +	 */
> > +	if (coresight_get_mode(source) != CS_MODE_SYSFS)
> > +		return 0;
> > +
> > +	/*
> > +	 * Save 'source->path' here, as it will be cleared in
> > +	 * coresight_disable_source().
> > +	 */
> > +	path = source->path;
> > +
> > +	coresight_disable_source(source, NULL);
> > +	coresight_disable_path(path);
> > +	return 0;
> 
> If the user is expected to re-enable and this new state is visible, don't
> you need to use the regular coresight_disable_sysfs() function? It calls
> coresight_disable_source_sysfs() which updates a refcount.

Good point!  We only need to maintain refcnt for system tracers (e.g.,
STM).  The per-CPU tracer is only binary states (on or off), we can
simply use "mode" to track state and no need refcnt.

I will use a separate patch to refactor refcnt.

> Maybe you didn't do it because it has a mutex in it? It would be easier to
> change that to a spinlock or take the mutex in a wrapper function and share
> the core disabling code with the hotplug path.

We have used cpus_read_lock() to avoid race between sysfs knobs and CPU
hotplug callbacks, we should not acquire mutex or spinlock in this
case.  For this reason, I would keep to call low level functions.

Thanks,
Leo

Re: [PATCH v4 15/15] coresight: Move CPU hotplug callbacks to core layer

[James Clark]

On 19/11/2025 2:48 pm, Leo Yan wrote:
> On Mon, Nov 10, 2025 at 12:20:06PM +0000, James Clark wrote:
> 
> [...]
> 
>>> +static int coresight_dying_cpu(unsigned int cpu)
>>> +{
>>> +	struct coresight_device *source = per_cpu(csdev_source, cpu);
>>> +	struct coresight_path *path;
>>> +
>>> +	if (!source || !source->path)
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * The perf event layer will disable PMU events in the CPU hotplug.
>>> +	 * CoreSight driver should never handle the CS_MODE_PERF case.
>>> +	 */
>>> +	if (coresight_get_mode(source) != CS_MODE_SYSFS)
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * Save 'source->path' here, as it will be cleared in
>>> +	 * coresight_disable_source().
>>> +	 */
>>> +	path = source->path;
>>> +
>>> +	coresight_disable_source(source, NULL);
>>> +	coresight_disable_path(path);
>>> +	return 0;
>>
>> If the user is expected to re-enable and this new state is visible, don't
>> you need to use the regular coresight_disable_sysfs() function? It calls
>> coresight_disable_source_sysfs() which updates a refcount.
> 
> Good point!  We only need to maintain refcnt for system tracers (e.g.,
> STM).  The per-CPU tracer is only binary states (on or off), we can
> simply use "mode" to track state and no need refcnt.
> 
> I will use a separate patch to refactor refcnt.
> 

If you remove the refcount then wouldn't it break scripts that make 
multiple enable calls? I thought the same logic for the system sources 
applies to CPU sources.

TBH I don't really know what problem the refcount and multiple balanced 
enable/disable calls solves. But even if we decide it's not needed, it's 
too late to change now.

>> Maybe you didn't do it because it has a mutex in it? It would be easier to
>> change that to a spinlock or take the mutex in a wrapper function and share
>> the core disabling code with the hotplug path.
> 
> We have used cpus_read_lock() to avoid race between sysfs knobs and CPU
> hotplug callbacks, we should not acquire mutex or spinlock in this
> case.  For this reason, I would keep to call low level functions.
> 
> Thanks,
> Leo

Re: [PATCH v4 15/15] coresight: Move CPU hotplug callbacks to core layer

[Leo Yan]

On Wed, Nov 19, 2025 at 03:19:55PM +0000, James Clark wrote:

[...]

> > > > +static int coresight_dying_cpu(unsigned int cpu)
> > > > +{
> > > > +	struct coresight_device *source = per_cpu(csdev_source, cpu);
> > > > +	struct coresight_path *path;
> > > > +
> > > > +	if (!source || !source->path)
> > > > +		return 0;
> > > > +
> > > > +	/*
> > > > +	 * The perf event layer will disable PMU events in the CPU hotplug.
> > > > +	 * CoreSight driver should never handle the CS_MODE_PERF case.
> > > > +	 */
> > > > +	if (coresight_get_mode(source) != CS_MODE_SYSFS)
> > > > +		return 0;
> > > > +
> > > > +	/*
> > > > +	 * Save 'source->path' here, as it will be cleared in
> > > > +	 * coresight_disable_source().
> > > > +	 */
> > > > +	path = source->path;
> > > > +
> > > > +	coresight_disable_source(source, NULL);
> > > > +	coresight_disable_path(path);
> > > > +	return 0;
> > > 
> > > If the user is expected to re-enable and this new state is visible, don't
> > > you need to use the regular coresight_disable_sysfs() function? It calls
> > > coresight_disable_source_sysfs() which updates a refcount.
> > 
> > Good point!  We only need to maintain refcnt for system tracers (e.g.,
> > STM).  The per-CPU tracer is only binary states (on or off), we can
> > simply use "mode" to track state and no need refcnt.
> > 
> > I will use a separate patch to refactor refcnt.
> 
> If you remove the refcount then wouldn't it break scripts that make multiple
> enable calls? I thought the same logic for the system sources applies to CPU
> sources.

I am not removing the refcount. Since we have csdev->mode to track the
device state, I just keep refcount == 0 for the CPU source instead of
refcount == 1.  This is transparnt for user space (no any change).

Before my change:

  echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
  echo 1 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 1
  echo 1 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 1
  echo 1 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 1

  echo 0 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 0

After my change:

  echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
  echo 1 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 0
  echo 1 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 0
  echo 1 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 0

  echo 0 > /sys/bus/coresight/devices/etm2/enable_source
  refcount = 0

As a result, the CPU PM flow doesn't need to bother refcount when to
disable the path.

Thanks,
Leo

> TBH I don't really know what problem the refcount and multiple balanced
> enable/disable calls solves. But even if we decide it's not needed, it's too
> late to change now.

Re: [PATCH v4 15/15] coresight: Move CPU hotplug callbacks to core layer

[James Clark]

On 19/11/2025 3:36 pm, Leo Yan wrote:
> On Wed, Nov 19, 2025 at 03:19:55PM +0000, James Clark wrote:
> 
> [...]
> 
>>>>> +static int coresight_dying_cpu(unsigned int cpu)
>>>>> +{
>>>>> +	struct coresight_device *source = per_cpu(csdev_source, cpu);
>>>>> +	struct coresight_path *path;
>>>>> +
>>>>> +	if (!source || !source->path)
>>>>> +		return 0;
>>>>> +
>>>>> +	/*
>>>>> +	 * The perf event layer will disable PMU events in the CPU hotplug.
>>>>> +	 * CoreSight driver should never handle the CS_MODE_PERF case.
>>>>> +	 */
>>>>> +	if (coresight_get_mode(source) != CS_MODE_SYSFS)
>>>>> +		return 0;
>>>>> +
>>>>> +	/*
>>>>> +	 * Save 'source->path' here, as it will be cleared in
>>>>> +	 * coresight_disable_source().
>>>>> +	 */
>>>>> +	path = source->path;
>>>>> +
>>>>> +	coresight_disable_source(source, NULL);
>>>>> +	coresight_disable_path(path);
>>>>> +	return 0;
>>>>
>>>> If the user is expected to re-enable and this new state is visible, don't
>>>> you need to use the regular coresight_disable_sysfs() function? It calls
>>>> coresight_disable_source_sysfs() which updates a refcount.
>>>
>>> Good point!  We only need to maintain refcnt for system tracers (e.g.,
>>> STM).  The per-CPU tracer is only binary states (on or off), we can
>>> simply use "mode" to track state and no need refcnt.
>>>
>>> I will use a separate patch to refactor refcnt.
>>
>> If you remove the refcount then wouldn't it break scripts that make multiple
>> enable calls? I thought the same logic for the system sources applies to CPU
>> sources.
> 
> I am not removing the refcount. Since we have csdev->mode to track the
> device state, I just keep refcount == 0 for the CPU source instead of
> refcount == 1.  This is transparnt for user space (no any change).
> 
> Before my change:
> 
>    echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 1
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 1
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 1
> 
>    echo 0 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
> 
> After my change:
> 
>    echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
> 
>    echo 0 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
> 
> As a result, the CPU PM flow doesn't need to bother refcount when to
> disable the path.
> 
> Thanks,
> Leo
> 

Oh ok I missed that it's not incremented for ETMs, that's fine then.

It's confusing that it behaves differently depending on some hidden 
property of the device. Not something that users should be expected to 
work out.

>> TBH I don't really know what problem the refcount and multiple balanced
>> enable/disable calls solves. But even if we decide it's not needed, it's too
>> late to change now.

Re: [PATCH v4 00/15] CoreSight: Refactor power management for CoreSight path

[James Clark]

On 04/11/2025 3:21 pm, Leo Yan wrote:
> This series is extracted from [1], focusing on CoreSight path power
> management.
> 
> Compared to the previous version, this series is updated heavily for:
> 
> 1) Dropped the global per CPU variable for saving path pointers.
>     Instead, the activate path is now stored in the source device's
>     structure.  This allows fetching the path pointer naturally based on
>     the source regardless of whether it is a per-CPU source or a system
>     source (such as STM).
> 
>     This improvement addresses Mike's comment that, later we can polish
>     coresight-sysfs.c to remove the tracer_path variables.
> 
> 2) To simplify the series and make it easier to review, the CTI driver
>     related fixes have been removed from this series and which will be
>     sent out separately.
> 
> 3) This series disables the path when a CPU is hot-plugged off but does
>     not re-enable it when the CPU is subsequently hot-plugged in.  This
>     simplifies the implementation and keep it consistent with the perf
>     session's behavior.
> 
>     It also improves security, as there is no risk of unintended tracing
>     caused by a CPU being hot-plugged after a long period of inactivity.
> 
> This series is dependent on ETM driver's PM improvement series [2] and
> has been verified on Juno-r2 and FVP RevC.
> 
> [1] https://lore.kernel.org/linux-arm-kernel/20250915-arm_coresight_power_management_fix-v3-0-ea49e91124ec@arm.com/
> [2] https://lore.kernel.org/linux-arm-kernel/20251103-arm_coresight_power_management_fix-v5-0-f803c19aa153@arm.com/#t
> 
> ---
> Changes in v4:
> - Changed to store path pointer in coresight_device, this is easier for
>    fetching path pointer based on source device (Mike).
> - Dropped changes in CTI driver.
> - Only disabled path for CPU hot-plugged off but not enable path for
>    hot-plugged in.
> - Removed James' test tags for modified patches.
> - Link to v3: https://lore.kernel.org/r/20250915-arm_coresight_power_management_fix-v3-0-ea49e91124ec@arm.com
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> 

Tested-by: James Clark <james.clark@linaro.org>

> ---
> Leo Yan (14):
>        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 PM is needed
>        coresight: etm4x: Remove redundant condition checks in save and restore
>        coresight: syscfg: Use spinlock to protect active variables
>        coresight: Introduce coresight_enable_source() helper
>        coresight: Save activated path into source device
>        coresight: Add 'in_idle' argument to enable/disable path 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
> 
> 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       | 273 ++++++++++++++++++++-
>   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-etm-perf.c   |   2 +-
>   drivers/hwtracing/coresight/coresight-etm3x-core.c |   1 +
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 137 ++---------
>   drivers/hwtracing/coresight/coresight-funnel.c     |   1 +
>   drivers/hwtracing/coresight/coresight-priv.h       |   3 +
>   drivers/hwtracing/coresight/coresight-replicator.c |   1 +
>   drivers/hwtracing/coresight/coresight-stm.c        |   1 +
>   drivers/hwtracing/coresight/coresight-syscfg.c     |  22 +-
>   drivers/hwtracing/coresight/coresight-syscfg.h     |   2 +
>   drivers/hwtracing/coresight/coresight-sysfs.c      |  12 +-
>   drivers/hwtracing/coresight/coresight-tmc-core.c   |   1 +
>   drivers/hwtracing/coresight/coresight-tnoc.c       |   2 +
>   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                          |  13 +
>   24 files changed, 425 insertions(+), 140 deletions(-)
> ---
> base-commit: f9ac95561513e18c2a2cf8905355dc5f0e030c46
> change-id: 20251104-arm_coresight_path_power_management_improvement-dab4966f8280
> 
> Best regards,