[PATCH v21 0/8] coresight: ctcu: Enable byte-cntr function for TMC ETR

[PATCH v21 0/8] coresight: ctcu: Enable byte-cntr function for TMC ETR

[Jie Gan]

The byte-cntr function provided by the CTCU device is used to count the
trace data entering the ETR. An interrupt is triggered if the data size
exceeds the threshold set in the BYTECNTRVAL register. The interrupt
handler counts the number of triggered interruptions.

Based on this concept, the irq_cnt can be used to determine whether
the etr_buf is full. The ETR device will be disabled when the active
etr_buf is nearly full or a timeout occurs. The nearly full buffer will
be switched to background after synced. A new buffer will be picked from
the etr_buf_list, then restart the ETR device.

The byte-cntr reading functions can access data from the synced and
deactivated buffer, transferring trace data from the etr_buf to userspace
without stopping the ETR device.

The byte-cntr read operation has integrated with the file node tmc_etr,
for example:
/dev/tmc_etr0
/dev/tmc_etr1

There are two scenarios for the tmc_etr file node with byte-cntr function:
1. BYTECNTRVAL register is configured and byte-cntr is enabled -> byte-cntr read
2. BYTECNTRVAL register is reset or byte-cntr is disabled -> original behavior

Shell commands to enable byte-cntr reading for etr0:
echo 1 > /sys/bus/coresight/devices/ctcu0/irq_enabled0
echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
echo 1 > /sys/bus/coresight/devices/etm0/enable_source
cat /dev/tmc_etr0

Reset the BYTECNTR register for etr0:
echo 0 > /sys/bus/coresight/devices/ctcu0/irq_enabled0

---
Changes in v21:
1. coresight: tmc: add create/clean functions for etr_buf_list:
   - Make the byte-cntr read and the generic sysfs read of the TMC
     exclusive: tmc_create_etr_buf_list() returns -EBUSY if a generic
     read is in progress and tmc_read_prepare_etr() returns -EBUSY if the
     etr_buf_list is not empty.
   - Wrap the live sysfs_buf into the list only while the sink is enabled
     in sysfs mode, and keep it in tmc_clean_etr_buf_list() only in that
     mode, as the sink enable path may reuse or free the sysfs_buf
     otherwise.
   - Don't create the list in the reserved buffer mode, as all the
     etr_buf of this mode share the same reserved region.
   - Fixed the kernel-doc of struct etr_buf_node and
     tmc_create_etr_buf_list().
2. coresight: tmc: add a function to switch the ETR to a free etr_buf (new):
   - Move the buffer switch out of the byte-cntr patch to a TMC helper,
     which stops the ETR, swaps the etr_buf and restarts the ETR in one
     critical section. The switch was done in three separate critical
     sections before, so tmc_disable_etr_sink() could run in between and
     leave drvdata->etr_buf out of sync with the hardware.
3. coresight: ctcu: enable byte-cntr for TMC ETR devices:
   - Fixed the drain of the last etr_buf after the trace is stopped:
     tmc_etr_get_sysfs_trace() falls back to the generic read when
     byte-cntr has no more data (v20 returned EOF and lost the last
     etr_buf). Other results of the byte-cntr read, such as the one of a
     signal, are returned as they are.
   - Program and reset the byte-cntr register when the path is enabled
     and disabled, instead of in the read prepare/unprepare. The CTCU may
     be powered off once the trace path is released, and accessing its
     register from the read unprepare, which runs when the file is
     closed after the trace is stopped, hangs the bus.
   - Serialise the CTCU register programming with the ATID programming
     with drvdata->spin_lock, as both unlock and lock the same register.
   - Fixed the use-after-free of the byte-cntr data between
     ctcu_platform_remove() and the read unprepare: the remove waits for
     a completion which is the last access of the read unprepare, and a
     new path can't start byte-cntr once the device is being removed.
   - Wake up the blocked reader once the byte-cntr is stopped, instead of
     waiting for the timeout.
   - Start byte-cntr in ctcu_enable() only in sysfs mode and after the
     trace ID is set.
   - Check the interrupt returned by of_irq_get() against 0.
   - Report the interrupt overwrite of the etr_buf which is handed over
     for reading, instead of the one which is tracing.
   - Make byte_cntr_sysfs_ops static and fixed the kernel-doc of the
     structs in coresight-ctcu.h.
   - Count the byte-cntr interrupts since the trace is started, and not reset
     the count when the read session starts, so the first etr_buf is switched
     at the right fill level for a reader which opens after the trace started.
   - Ignore the ports which are not initialized by ctcu_byte_cntr_init().
   - Updated the commit message.
- Link to v20: https://lore.kernel.org/r/20261030-enable-byte-cntr-for-ctcu-v20-0-bd5c3ac9f916@oss.qualcomm.com

Changes in v20:
1. coresight: core: refactor ctcu_get_active_port and make it generic:
   - Check the port number returned by coresight_get_in_port() against
     ETR_MAX_NUM in ctcu_set_etr_traceid() to avoid an out-of-bounds
     access of the per-port arrays with a bad DT port number.
2. coresight: tmc: add create/clean functions for etr_buf_list:
   - Take drvdata->sysfs_buf and add the first node to etr_buf_list in
     one critical section, and do all etr_buf_list updates (add and
     delete) with drvdata->spinlock held.
   - Return -EBUSY from tmc_etr_get_sysfs_buffer() while etr_buf_list is
     not empty, so the ETR sink enable path cannot free or replace a
     buffer that is still owned by the list during a byte-cntr session.
3. coresight: ctcu: enable byte-cntr for TMC ETR devices:
   - Fixed a NULL pointer dereference in ctcu_get_byte_cntr() when the
     opened ETR is not connected to a CTCU device, since the byte-cntr
     sysfs ops are global to all the TMC ETRs.
   - Fixed the sysfs read fallback: tmc_byte_cntr_get_data() now returns
     -EINVAL unless a byte-cntr read session is active, and
     tmc_etr_get_sysfs_trace() falls back to the generic read only for
     -EINVAL. Errors and EOF of the byte-cntr read are returned as-is
     instead of reading the live buffer.
   - Fixed a race between the buffer switch and the sink disable path:
     tmc_etr_enable_disable_hw() now checks the sink mode with
     drvdata->spinlock held, so the ETR is not restarted after the sink
     has been disabled.
   - tmc_read_prepare_byte_cntr(): check irq_enabled with the
     byte_cntr_data->spin_lock held, and clear reading under the lock
     and wake up the waiter on the buffer list allocation failure path.
   - Pin the module with try_module_get() for the duration of a byte-cntr
     read session, so unloading the module fails with -EBUSY instead of
     blocking in ctcu_platform_remove(). Documented that a manual unbind
     still waits for the tmc_etr file to be closed.
   - Dropped IRQF_SHARED from the byte-cntr interrupt: the handler does
     not check the interrupt source.
   - Read curr_buf with drvdata->spinlock held and handle a missing
     current node in ctcu_byte_cntr_switch_buffer().
   - Initialize the lock and waitqueue of every port before requesting
     its interrupt, skip the ports without a registered IRQ in
     ctcu_platform_remove(), and only expose irq_enabled for the ports
     with a registered IRQ.
   - Refcount the byte-cntr sysfs ops registration across CTCU devices
     with a devm action, so removing one CTCU no longer disables
     byte-cntr for the others. Replaced the single-CTCU-instance comment
     on byte_cntr_sysfs_ops accordingly.
- Link to v19: https://lore.kernel.org/r/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com

Changes in v19:
1. coresight: tmc: add create/clean functions for etr_buf_list:
   - Simplified the kernel-doc "Locking:" note on tmc_create_etr_buf_list()
     and tmc_clean_etr_buf_list() to state only the required contract
     (caller must guarantee exclusive access to drvdata->etr_buf_list),
     dropping the over-specific call-path details.
   - Lock the required code block in create/clean functions.
2. coresight: ctcu: enable byte-cntr for TMC ETR devices:
   - Fixed a stale reading-state / double-free when irq_enabled was
     cleared between prepare and unprepare: the byte-cntr read_unprepare
     would return an error and tmc_read_unprepare_etr() would fall back
     to the normal unprepare path, freeing drvdata->sysfs_buf while it
     was still owned by the byte-cntr buffer list. irq_enabled_store()
     now returns -EBUSY while byte_cntr_data->reading is set, so the flag
     can no longer change mid-session, and tmc_read_unprepare_byte_cntr()
     additionally guards on byte_cntr_data->reading.
   - Moved enable_irq_wake()/disable_irq_wake() out of the
     byte_cntr_data->spin_lock critical section in
     tmc_read_prepare_byte_cntr()/tmc_read_unprepare_byte_cntr().
     irq_set_irq_wake() may sleep on slow-bus irqchips, so it must not
     be called under a raw spinlock. The threshold IRQ is already
     disabled by the in-lock register write before disable_irq_wake() on
     the unprepare path, so no wake event can race in the gap.
   - Added a comment on the byte_cntr_sysfs_ops pointer documenting the
     single-CTCU-instance-per-system assumption.
- Link to v18: https://lore.kernel.org/r/20260507-enable-byte-cntr-for-ctcu-v18-0-2b2d590463a3@oss.qualcomm.com

Changes in v18:
1. add a NULL check for the in_conns instance in patch 1.
2. fix a bug in patch 2: the tmc_alloc_etr_buf never return NULL and the
   previous check for the return value is incorrect.
3. add more kernel_doc description for tmc_clean_etr_buf_list function
   in patch 2
- Link to v17: https://lore.kernel.org/r/20260421-enable-byte-cntr-for-ctcu-v17-0-9cf36ff55fc0@oss.qualcomm.com

Changes in v17:
1. fix race issue during allocat buffer.
2. fix user after free issue observed when remove module.
- Link to v16: https://lore.kernel.org/r/20260323-enable-byte-cntr-for-ctcu-v16-0-7a413d211b8d@oss.qualcomm.com

Changes in v16:
1. Remove lock/unlock processes in patch "coresight: tmc: add create/clean
   functions for etr_buf_list" because we are allocating/freeing memory.
- Link to v15: https://lore.kernel.org/r/20260313-enable-byte-cntr-for-ctcu-v15-0-1777f14ed319@oss.qualcomm.com

Changes in v15:
1. add lockdep_assert_held in patch "coresight: tmc: add create/clean
   functions for etr_buf_list"
2. optimize tmc_clean_etr_buf_list function
3. optimize the patch "enable byte-cntr for TMC ETR devices" according
   to Suzuki's comments
   - call byte_cntr_sysfs_ops from etr_sysfs_ops
   - optimize the lock usage in all functions
   - remove the buf_node parameter in etr_drvdata, move it to
     byte_cntr_data
   - move the tmc_reset_sysfs_buf function to tmc-etr.c
   - add a read flag to struct etr_buf_node to allow updating pos while
     traversing etr_buf_list during data reads.
Link to v14: https://lore.kernel.org/r/20260309-enable-byte-cntr-for-ctcu-v14-0-c08823e5a8e6@oss.qualcomm.com

Changes in V14:
1. Drop the patch: integrate byte-cntr's sysfs_ops with tmc sysfs file_ops
2. Replace tmc_sysfs_ops with byte_cntr_sysfs_ops in byte_cntr_start
   function and restore etr_sysfs_ops in byte_cntr_unprepare function.
3. Remove redundant checks in byte‑cntr functions.
Link to V13: https://lore.kernel.org/all/20260223-enable-byte-cntr-for-ctcu-v13-0-9cb44178b250@oss.qualcomm.com/

Changes in v13:
1. initilize the byte_cntr_data->raw_spin_lock before using.
2. replace kzalloc with kzalloc_obj.
Link to V12: https://lore.kernel.org/all/20260203-enable-byte-cntr-for-ctcu-v12-0-7bf81b86b70e@oss.qualcomm.com/

Changes in v12:
1. Add a new function for retrieving the CTCU's coresight_dev instead of
   refactor the existing function.
Link to v11: https://lore.kernel.org/r/20260126-enable-byte-cntr-for-ctcu-v11-0-c0af66ba15cf@oss.qualcomm.com

Changes in v11:
1. Correct the description in patch1 for the function coresight_get_in_port.
2. Renaming the sysfs_ops to tmc_sysfs_ops per Suzuki's suggestion.
Link to v10: https://lore.kernel.org/r/20260122-enable-byte-cntr-for-ctcu-v10-0-22978e3c169f@oss.qualcomm.com

Changes in v10:
1. fix a free memory issue that is reported by robot for patch 2.
Link to v9: https://lore.kernel.org/r/20251224-enable-byte-cntr-for-ctcu-v9-0-886c4496fed4@oss.qualcomm.com

Changes in v9:
1. Drop the patch: add a new API to retrieve the helper device
2. Add a new patch to refactor the tmc_etr_get_catu_device function,
   making it generic to support all types of helper devices associated with ETR.
3. Optimizing the code for creating irq_threshold sysfs node.
4. Remove interrupt-name property and obtain the IRQ based on the
   in-port number.
Link to v8: https://lore.kernel.org/r/20251211-enable-byte-cntr-for-ctcu-v8-0-3e12ff313191@oss.qualcomm.com

Changes in V8:
1. Optimizing the patch 1 and patch 2 according to Suzuki's comments.
2. Combine the patch 3 and patch 4 together.
3. Rename the interrupt-name to prevent confusion, for example:etr0->etrirq0.
Link to V7 - https://lore.kernel.org/all/20251013-enable-byte-cntr-for-ctcu-v7-0-e1e8f41e15dd@oss.qualcomm.com/

Changes in V7:
1. rebased on tag next-20251010
2. updated info for sysfs node document
Link to V6 - https://lore.kernel.org/all/20250908-enable-byte-cntr-for-tmc-v6-0-1db9e621441a@oss.qualcomm.com/

Changes in V6:
1. rebased on next-20250905.
2. fixed the issue that the dtsi file has re-named from sa8775p.dtsi to
   lemans.dtsi.
3. fixed some minor issues about comments.
Link to V5 - https://lore.kernel.org/all/20250812083731.549-1-jie.gan@oss.qualcomm.com/

Changes in V5:
1. Add Mike's reviewed-by tag for patchset 1,2,5.
2. Remove the function pointer added to helper_ops according to Mike's
   comment, it also results the patchset has been removed.
3. Optimizing the paired create/clean functions for etr_buf_list.
4. Remove the unneeded parameter "reading" from the etr_buf_node.
Link to V4 - https://lore.kernel.org/all/20250725100806.1157-1-jie.gan@oss.qualcomm.com/

Changes in V4:
1. Rename the function to coresight_get_in_port_dest regarding to Mike's
comment (patch 1/10).
2. Add lock to protect the connections regarding to Mike's comment
(patch 2/10).
3. Move all byte-cntr functions to coresight-ctcu-byte-cntr file.
4. Add tmc_read_ops to wrap all read operations for TMC device.
5. Add a function in helper_ops to check whether the byte-cntr is
enabkled.
6. Call byte-cntr's read_ops if byte-cntr is enabled when reading data
from the sysfs node.
Link to V3 resend - https://lore.kernel.org/all/20250714063109.591-1-jie.gan@oss.qualcomm.com/

Changes in V3 resend:
1. rebased on next-20250711.
Link to V3 - https://lore.kernel.org/all/20250624060438.7469-1-jie.gan@oss.qualcomm.com/

Changes in V3:
1. The previous solution has been deprecated.
2. Add a etr_buf_list to manage allcated etr buffers.
3. Add a logic to switch buffer for ETR.
4. Add read functions to read trace data from synced etr buffer.
Link to V2 - https://lore.kernel.org/all/20250410013330.3609482-1-jie.gan@oss.qualcomm.com/

Changes in V2:
1. Removed the independent file node /dev/byte_cntr.
2. Integrated the byte-cntr's file operations with current ETR file
   node.
3. Optimized the driver code of the CTCU that associated with byte-cntr.
4. Add kernel document for the export API tmc_etr_get_rwp_offset.
5. Optimized the way to read the rwp_offset according to Mike's
   suggestion.
6. Removed the dependency of the dts patch.
Link to V1 - https://lore.kernel.org/all/20250310090407.2069489-1-quic_jiegan@quicinc.com/

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>

---
Jie Gan (8):
      coresight: core: refactor ctcu_get_active_port and make it generic
      coresight: tmc: add create/clean functions for etr_buf_list
      coresight: tmc: add a function to switch the ETR to a free etr_buf
      coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations
      coresight: etr: add a new function to retrieve the CTCU device
      dt-bindings: arm: add an interrupt property for Coresight CTCU
      coresight: ctcu: enable byte-cntr for TMC ETR devices
      arm64: dts: qcom: lemans: add interrupts to CTCU device

 .../ABI/testing/sysfs-bus-coresight-devices-ctcu   |   9 +
 .../bindings/arm/qcom,coresight-ctcu.yaml          |  10 +
 arch/arm64/boot/dts/qcom/lemans.dtsi               |   3 +
 drivers/hwtracing/coresight/Makefile               |   2 +-
 drivers/hwtracing/coresight/coresight-core.c       |  27 ++
 .../hwtracing/coresight/coresight-ctcu-byte-cntr.c | 400 +++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-ctcu-core.c  | 184 ++++++++--
 drivers/hwtracing/coresight/coresight-ctcu.h       |  88 ++++-
 drivers/hwtracing/coresight/coresight-priv.h       |   2 +
 drivers/hwtracing/coresight/coresight-tmc-core.c   |  55 ++-
 drivers/hwtracing/coresight/coresight-tmc-etr.c    | 344 +++++++++++++++++-
 drivers/hwtracing/coresight/coresight-tmc.h        |  43 +++
 12 files changed, 1089 insertions(+), 78 deletions(-)
---
base-commit: a39c950806213f6861d00f90729acaaa21f2d95a
change-id: 20260309-enable-byte-cntr-for-ctcu-ff86e6198b7f

Best regards,
-- 
Jie Gan <jie.gan@oss.qualcomm.com>

[PATCH v21 1/8] coresight: core: refactor ctcu_get_active_port and make it generic

[Jie Gan]

Remove ctcu_get_active_port from CTCU module and add it to the core
framework.

The port number is crucial for the CTCU device to identify which ETR
it serves. With the port number we can correctly get required parameters
of the CTCU device in TMC module.

Reviewed-by: Mike Leach <mike.leach@arm.com>
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-core.c      | 27 +++++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-ctcu-core.c | 21 ++----------------
 drivers/hwtracing/coresight/coresight-priv.h      |  2 ++
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index d2f9c1fb77d9..32e8b3e51519 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -773,6 +773,33 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
 }
 EXPORT_SYMBOL_GPL(coresight_get_sink);
 
+/**
+ * coresight_get_in_port: Find the input port number at @remote where the @csdev
+ * device is connected to.
+ *
+ * @csdev: csdev of the device.
+ * @remote: csdev of the remote device which is connected to @csdev.
+ *
+ * Return: port number upon success or -EINVAL for fail.
+ */
+int coresight_get_in_port(struct coresight_device *csdev,
+			  struct coresight_device *remote)
+{
+	struct coresight_platform_data *pdata = remote->pdata;
+	int i;
+
+	for (i = 0; i < pdata->nr_inconns; ++i) {
+		if (!pdata->in_conns[i])
+			continue;
+
+		if (pdata->in_conns[i]->src_dev == csdev)
+			return pdata->in_conns[i]->dest_port;
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(coresight_get_in_port);
+
 u32 coresight_get_sink_id(struct coresight_device *csdev)
 {
 	if (!csdev->ea)
diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
index 9043cad42f01..739105acb0a1 100644
--- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
+++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
@@ -116,23 +116,6 @@ static int __ctcu_set_etr_traceid(struct coresight_device *csdev, u8 traceid, in
 	return 0;
 }
 
-/*
- * Searching the sink device from helper's view in case there are multiple helper devices
- * connected to the sink device.
- */
-static int ctcu_get_active_port(struct coresight_device *sink, struct coresight_device *helper)
-{
-	struct coresight_platform_data *pdata = helper->pdata;
-	int i;
-
-	for (i = 0; i < pdata->nr_inconns; ++i) {
-		if (pdata->in_conns[i]->src_dev == sink)
-			return pdata->in_conns[i]->dest_port;
-	}
-
-	return -EINVAL;
-}
-
 static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight_path *path,
 				bool enable)
 {
@@ -145,8 +128,8 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight
 		return -EINVAL;
 	}
 
-	port_num = ctcu_get_active_port(sink, csdev);
-	if (port_num < 0)
+	port_num = coresight_get_in_port(sink, csdev);
+	if (port_num < 0 || port_num >= ETR_MAX_NUM)
 		return -EINVAL;
 
 	dev_dbg(&csdev->dev, "traceid is %d\n", traceid);
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index dddac946659f..854c0a3cb080 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -155,6 +155,8 @@ void coresight_remove_links(struct coresight_device *orig,
 u32 coresight_get_sink_id(struct coresight_device *csdev);
 int coresight_path_assign_trace_id(struct coresight_path *path,
 				   enum cs_mode mode);
+int coresight_get_in_port(struct coresight_device *csdev,
+			  struct coresight_device *remote);
 
 #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
 int etm_readl_cp14(u32 off, unsigned int *val);

-- 
2.34.1

[PATCH v21 2/8] coresight: tmc: add create/clean functions for etr_buf_list

[Jie Gan]

Introduce functions for creating and inserting or removing the
etr_buf_node to/from the etr_buf_list.

The byte-cntr functionality requires two etr_buf to receive trace data.
The active etr_buf collects the trace data from source device, while the
byte-cntr reading function accesses the deactivated etr_buf after is
has been filled and synced, transferring data to the userspace.

Reviewed-by: Mike Leach <mike.leach@arm.com>
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-core.c |   1 +
 drivers/hwtracing/coresight/coresight-tmc-etr.c  | 168 ++++++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-tmc.h      |  18 +++
 3 files changed, 185 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index bc5a133ada3e..bc7dd676da47 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -835,6 +835,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
 		idr_init(&drvdata->idr);
 		mutex_init(&drvdata->idr_mutex);
 		dev_list = "tmc_etr";
+		INIT_LIST_HEAD(&drvdata->etr_buf_list);
 		break;
 	case TMC_CONFIG_TYPE_ETF:
 		desc.groups = coresight_etf_groups;
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 361a433e6f0c..eee98a9269df 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1268,7 +1268,9 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
 		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 	}
 
-	if (drvdata->reading || coresight_get_mode(csdev) == CS_MODE_PERF) {
+	/* The sysfs_buf is owned by the etr_buf_list, don't replace it */
+	if (drvdata->reading || coresight_get_mode(csdev) == CS_MODE_PERF ||
+	    !list_empty(&drvdata->etr_buf_list)) {
 		ret = -EBUSY;
 		goto out;
 	}
@@ -1918,6 +1920,167 @@ const struct coresight_ops tmc_etr_cs_ops = {
 	.panic_ops	= &tmc_etr_sync_ops,
 };
 
+/**
+ * tmc_clean_etr_buf_list - clean the etr_buf_list.
+ * @drvdata:	driver data of the TMC device.
+ *
+ * Remove all nodes from @drvdata->etr_buf_list and free their buffers.
+ * If a node holds the live sysfs_buf and the sink is enabled in sysfs mode, the
+ * node is removed but the buffer is not freed; ownership stays with
+ * drvdata->sysfs_buf.
+ *
+ * Locking: must not be called with @drvdata->spinlock held. The spinlock is
+ * taken internally to serialise the @drvdata->sysfs_buf accesses and the
+ * removal of the nodes against the ETR sink enable/disable paths and the
+ * generic sysfs read. There must be only one owner of the list at a time.
+ * Must be called from process context: buffers are freed with the lock
+ * released.
+ */
+void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
+{
+	struct etr_buf_node *nd, *next;
+	unsigned long flags;
+
+	list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) {
+		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
+		if (nd->sysfs_buf == drvdata->sysfs_buf) {
+			if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
+				/*
+				 * The sink is still enabled in sysfs mode. Keep
+				 * the live buffer owned by drvdata->sysfs_buf
+				 * and only drop the list's reference to it.
+				 */
+				nd->sysfs_buf = NULL;
+			else
+				/* Free the buffer below through nd->sysfs_buf */
+				drvdata->sysfs_buf = NULL;
+		}
+		list_del(&nd->link);
+		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
+
+		/* Free the buffer (NULL is ignored) and the node out of the lock */
+		tmc_etr_free_sysfs_buf(nd->sysfs_buf);
+		kfree(nd);
+	}
+}
+EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
+
+/**
+ * tmc_create_etr_buf_list - create a list to manage the etr_buf_node.
+ * @drvdata:	driver data of the TMC device.
+ * @num_nodes:	number of nodes want to create with the list.
+ *
+ * Locking: must not be called with @drvdata->spinlock held. The spinlock is
+ * taken internally to serialise the @drvdata->sysfs_buf accesses and every
+ * update of @drvdata->etr_buf_list against the ETR sink enable/disable paths
+ * and the generic sysfs read. There must be only one owner of the list at a
+ * time. Must be called from process context: buffers and nodes are allocated
+ * with the lock released.
+ *
+ * Return: 0 upon success, else the error number: -EBUSY if a generic sysfs
+ * read is in progress, -EINVAL if the sink is not enabled in sysfs mode or
+ * the ETR is in the reserved buffer mode, or the error of the allocation.
+ */
+int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes)
+{
+	struct etr_buf_node *new_node;
+	struct etr_buf *sysfs_buf;
+	unsigned long flags;
+	int i = 0, ret = 0;
+
+	/* We don't need a list if there is only one node */
+	if (num_nodes < 2)
+		return -EINVAL;
+
+	/*
+	 * All the etr_buf of the reserved mode are mapped to the same reserved
+	 * region, so they can't be used as separate buffers of the list.
+	 */
+	if (drvdata->etr_mode == ETR_MODE_RESRV)
+		return -EINVAL;
+
+	/*
+	 * We expect that sysfs_buf in drvdata has already been allocated.
+	 * Wrap the live sysfs_buf into the first node so the captured trace
+	 * data is preserved. Take the sysfs_buf and add the node under the
+	 * lock, so that the ETR sink enable path sees a non-empty list and
+	 * doesn't replace the buffer.
+	 */
+	new_node = kzalloc_obj(*new_node, GFP_KERNEL);
+	if (!new_node)
+		return -ENOMEM;
+
+	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
+	/* The sysfs_buf is in use by a sysfs read session */
+	if (drvdata->reading) {
+		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
+		kfree(new_node);
+		return -EBUSY;
+	}
+
+	/*
+	 * The sink enable path reuses or frees the sysfs_buf of a sink which
+	 * isn't enabled, so only wrap it while the sink is enabled in sysfs mode.
+	 */
+	if (coresight_get_mode(drvdata->csdev) != CS_MODE_SYSFS) {
+		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
+		kfree(new_node);
+		return -EINVAL;
+	}
+
+	if (drvdata->sysfs_buf) {
+		new_node->sysfs_buf = drvdata->sysfs_buf;
+		new_node->is_free = false;
+		list_add(&new_node->link, &drvdata->etr_buf_list);
+		new_node = NULL;
+		i++;
+	}
+	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	/* Free the node if there is no live sysfs_buf to wrap */
+	kfree(new_node);
+
+	while (i < num_nodes) {
+		new_node = kzalloc_obj(*new_node, GFP_KERNEL);
+		if (!new_node) {
+			ret = -ENOMEM;
+			break;
+		}
+
+		/* Allocate the buffer with the lock released */
+		sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
+		if (IS_ERR(sysfs_buf)) {
+			kfree(new_node);
+			ret = PTR_ERR(sysfs_buf);
+			break;
+		}
+
+		new_node->sysfs_buf = sysfs_buf;
+		/*
+		 * The drvdata->sysfs_buf write and the list update need the
+		 * spinlock, to serialise against the ETR sink enable/disable
+		 * paths.
+		 */
+		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
+		/* We don't have an available sysfs_buf in drvdata, set one up */
+		if (!drvdata->sysfs_buf) {
+			drvdata->sysfs_buf = sysfs_buf;
+			new_node->is_free = false;
+		} else {
+			new_node->is_free = true;
+		}
+		list_add_tail(&new_node->link, &drvdata->etr_buf_list);
+		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
+		i++;
+	}
+
+	/* Clean the list if there is an error */
+	if (ret)
+		tmc_clean_etr_buf_list(drvdata);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tmc_create_etr_buf_list);
+
 int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 {
 	int ret = 0;
@@ -1928,7 +2091,8 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 		return -EINVAL;
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
-	if (drvdata->reading) {
+	/* The sysfs_buf is owned by the etr_buf_list */
+	if (drvdata->reading || !list_empty(&drvdata->etr_buf_list)) {
 		ret = -EBUSY;
 		goto out;
 	}
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 319a354ede9f..7c604b9f92e5 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -208,6 +208,20 @@ struct tmc_resrv_buf {
 	s64		len;
 };
 
+/**
+ * struct etr_buf_node - node of the etr_buf_list
+ * @sysfs_buf:	Allocated sysfs_buf.
+ * @is_free:	Indicates whether the buffer is free to choose.
+ * @pos:	Offset to the start of the buffer.
+ * @link:	list_head of the node.
+ */
+struct etr_buf_node {
+	struct etr_buf		*sysfs_buf;
+	bool			is_free;
+	loff_t			pos;
+	struct list_head	link;
+};
+
 /**
  * struct tmc_drvdata - specifics associated to an TMC component
  * @atclk:	optional clock for the core parts of the TMC.
@@ -245,6 +259,7 @@ struct tmc_resrv_buf {
  *		(after crash) by default.
  * @crash_mdata: Reserved memory for storing tmc crash metadata.
  *		 Used by ETR/ETF.
+ * @etr_buf_list: List that is used to manage allocated etr_buf.
  */
 struct tmc_drvdata {
 	struct clk		*atclk;
@@ -275,6 +290,7 @@ struct tmc_drvdata {
 	struct etr_buf		*perf_buf;
 	struct tmc_resrv_buf	resrv_buf;
 	struct tmc_resrv_buf	crash_mdata;
+	struct list_head        etr_buf_list;
 };
 
 struct etr_buf_operations {
@@ -447,5 +463,7 @@ struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
 				   enum cs_mode mode,
 				   struct coresight_path *path);
 extern const struct attribute_group coresight_etr_group;
+void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata);
+int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes);
 
 #endif

-- 
2.34.1

[PATCH v21 3/8] coresight: tmc: add a function to switch the ETR to a free etr_buf

[Jie Gan]

Add tmc_etr_switch_buf_node() to switch the ETR to a free etr_buf of the
etr_buf_list without disabling the sink. It returns the node of the
switched out etr_buf, so its synced data can be read while the ETR keeps
tracing into the new one.

Stopping the ETR, swapping the etr_buf and restarting the ETR have to be
done in one critical section. Otherwise tmc_disable_etr_sink() can run
in between and leave drvdata->etr_buf out of sync with the hardware.
Provide the sequence as a TMC helper, so callers don't need to know
about the locking of the ETR state.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 65 +++++++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h     |  4 ++
 2 files changed, 69 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index eee98a9269df..2de31d73a4b4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1224,6 +1224,71 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
 
 }
 
+static void tmc_etr_reset_sysfs_buf(struct tmc_drvdata *drvdata)
+{
+	u32 sts;
+
+	CS_UNLOCK(drvdata->base);
+	tmc_write_rrp(drvdata, drvdata->sysfs_buf->hwaddr);
+	tmc_write_rwp(drvdata, drvdata->sysfs_buf->hwaddr);
+	sts = readl_relaxed(drvdata->base + TMC_STS) & ~TMC_STS_FULL;
+	writel_relaxed(sts, drvdata->base + TMC_STS);
+	CS_LOCK(drvdata->base);
+}
+
+/**
+ * tmc_etr_switch_buf_node - switch the ETR to a free buffer of the etr_buf_list.
+ * @drvdata:	drvdata of the TMC device.
+ *
+ * Stop the ETR, hand over the current buffer for reading, switch to a free
+ * buffer and restart the ETR. All of this is done with @drvdata->spinlock held,
+ * so it can't race with the sink disable path.
+ *
+ * Return: the node of the switched out buffer, or NULL if the sink isn't in
+ * sysfs mode or there is no free buffer. The ETR is left untouched in both
+ * cases.
+ */
+struct etr_buf_node *tmc_etr_switch_buf_node(struct tmc_drvdata *drvdata)
+{
+	struct etr_buf_node *nd, *curr_node = NULL, *picked_node = NULL;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
+	if (coresight_get_mode(drvdata->csdev) != CS_MODE_SYSFS)
+		goto out;
+
+	list_for_each_entry(nd, &drvdata->etr_buf_list, link) {
+		if (nd->sysfs_buf == drvdata->sysfs_buf)
+			curr_node = nd;
+		else if (!picked_node && nd->is_free)
+			picked_node = nd;
+	}
+
+	if (!curr_node || !picked_node) {
+		curr_node = NULL;
+		goto out;
+	}
+
+	__tmc_etr_disable_hw(drvdata);
+
+	/* The switched out buffer is free again once it has been read */
+	curr_node->is_free = true;
+	curr_node->pos = 0;
+	curr_node->reading = true;
+	picked_node->is_free = false;
+	drvdata->sysfs_buf = picked_node->sysfs_buf;
+	drvdata->etr_buf = picked_node->sysfs_buf;
+
+	tmc_etr_reset_sysfs_buf(drvdata);
+	if (__tmc_etr_enable_hw(drvdata))
+		dev_err(&drvdata->csdev->dev, "Failed to restart the ETR\n");
+out:
+	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
+
+	return curr_node;
+}
+EXPORT_SYMBOL_GPL(tmc_etr_switch_buf_node);
+
 void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
 {
 	__tmc_etr_disable_hw(drvdata);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 7c604b9f92e5..30b15c38b080 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -212,12 +212,15 @@ struct tmc_resrv_buf {
  * struct etr_buf_node - node of the etr_buf_list
  * @sysfs_buf:	Allocated sysfs_buf.
  * @is_free:	Indicates whether the buffer is free to choose.
+ * @reading:	Indicates the buffer attached to the node is being read by
+ *		the user.
  * @pos:	Offset to the start of the buffer.
  * @link:	list_head of the node.
  */
 struct etr_buf_node {
 	struct etr_buf		*sysfs_buf;
 	bool			is_free;
+	bool			reading;
 	loff_t			pos;
 	struct list_head	link;
 };
@@ -465,5 +468,6 @@ struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
 extern const struct attribute_group coresight_etr_group;
 void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata);
 int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes);
+struct etr_buf_node *tmc_etr_switch_buf_node(struct tmc_drvdata *drvdata);
 
 #endif

-- 
2.34.1

[PATCH v21 4/8] coresight: tmc: introduce tmc_sysfs_ops to wrap sysfs read operations

[Jie Gan]

Introduce tmc_sysfs_ops as a wrapper, wrap sysfs read operations,
for reading trace data from the TMC buffer.

Reviewed-by: Mike Leach <mike.leach@arm.com>
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-core.c | 51 ++++++++++--------------
 drivers/hwtracing/coresight/coresight-tmc.h      | 15 +++++++
 2 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index bc7dd676da47..4b40b692be4d 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -228,17 +228,10 @@ static int tmc_read_prepare(struct tmc_drvdata *drvdata)
 {
 	int ret = 0;
 
-	switch (drvdata->config_type) {
-	case TMC_CONFIG_TYPE_ETB:
-	case TMC_CONFIG_TYPE_ETF:
-		ret = tmc_read_prepare_etb(drvdata);
-		break;
-	case TMC_CONFIG_TYPE_ETR:
-		ret = tmc_read_prepare_etr(drvdata);
-		break;
-	default:
+	if (drvdata->sysfs_ops)
+		ret = drvdata->sysfs_ops->read_prepare(drvdata);
+	else
 		ret = -EINVAL;
-	}
 
 	if (!ret)
 		dev_dbg(&drvdata->csdev->dev, "TMC read start\n");
@@ -250,17 +243,10 @@ static int tmc_read_unprepare(struct tmc_drvdata *drvdata)
 {
 	int ret = 0;
 
-	switch (drvdata->config_type) {
-	case TMC_CONFIG_TYPE_ETB:
-	case TMC_CONFIG_TYPE_ETF:
-		ret = tmc_read_unprepare_etb(drvdata);
-		break;
-	case TMC_CONFIG_TYPE_ETR:
-		ret = tmc_read_unprepare_etr(drvdata);
-		break;
-	default:
+	if (drvdata->sysfs_ops)
+		ret = drvdata->sysfs_ops->read_unprepare(drvdata);
+	else
 		ret = -EINVAL;
-	}
 
 	if (!ret)
 		dev_dbg(&drvdata->csdev->dev, "TMC read end\n");
@@ -287,15 +273,7 @@ static int tmc_open(struct inode *inode, struct file *file)
 static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len,
 				   char **bufpp)
 {
-	switch (drvdata->config_type) {
-	case TMC_CONFIG_TYPE_ETB:
-	case TMC_CONFIG_TYPE_ETF:
-		return tmc_etb_get_sysfs_trace(drvdata, pos, len, bufpp);
-	case TMC_CONFIG_TYPE_ETR:
-		return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
-	}
-
-	return -EINVAL;
+	return drvdata->sysfs_ops->get_trace_data(drvdata, pos, len, bufpp);
 }
 
 static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
@@ -764,6 +742,18 @@ static void register_crash_dev_interface(struct tmc_drvdata *drvdata,
 			"Valid crash tracedata found\n");
 }
 
+static const struct tmc_sysfs_ops etb_sysfs_ops = {
+	.read_prepare	= tmc_read_prepare_etb,
+	.read_unprepare	= tmc_read_unprepare_etb,
+	.get_trace_data	= tmc_etb_get_sysfs_trace,
+};
+
+static const struct tmc_sysfs_ops etr_sysfs_ops = {
+	.read_prepare	= tmc_read_prepare_etr,
+	.read_unprepare	= tmc_read_unprepare_etr,
+	.get_trace_data	= tmc_etr_get_sysfs_trace,
+};
+
 static int __tmc_probe(struct device *dev, struct resource *res)
 {
 	int ret = 0;
@@ -823,6 +813,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
 		desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
 		desc.ops = &tmc_etb_cs_ops;
 		dev_list = "tmc_etb";
+		drvdata->sysfs_ops = &etb_sysfs_ops;
 		break;
 	case TMC_CONFIG_TYPE_ETR:
 		desc.groups = coresight_etr_groups;
@@ -835,6 +826,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
 		idr_init(&drvdata->idr);
 		mutex_init(&drvdata->idr_mutex);
 		dev_list = "tmc_etr";
+		drvdata->sysfs_ops = &etr_sysfs_ops;
 		INIT_LIST_HEAD(&drvdata->etr_buf_list);
 		break;
 	case TMC_CONFIG_TYPE_ETF:
@@ -844,6 +836,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
 		desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
 		desc.ops = &tmc_etf_cs_ops;
 		dev_list = "tmc_etf";
+		drvdata->sysfs_ops = &etb_sysfs_ops;
 		break;
 	default:
 		pr_err("%s: Unsupported TMC config\n", desc.name);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 30b15c38b080..36c80918958f 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -263,6 +263,7 @@ struct etr_buf_node {
  * @crash_mdata: Reserved memory for storing tmc crash metadata.
  *		 Used by ETR/ETF.
  * @etr_buf_list: List that is used to manage allocated etr_buf.
+ * @sysfs_ops:	Read operations for the sysfs mode.
  */
 struct tmc_drvdata {
 	struct clk		*atclk;
@@ -294,6 +295,20 @@ struct tmc_drvdata {
 	struct tmc_resrv_buf	resrv_buf;
 	struct tmc_resrv_buf	crash_mdata;
 	struct list_head        etr_buf_list;
+	const struct tmc_sysfs_ops	*sysfs_ops;
+};
+
+/**
+ * struct tmc_sysfs_ops - read operations for TMC and its helper devices
+ * @read_prepare:	prepare operation.
+ * @read_unprepare:	unprepare operation.
+ * @get_trace_data:	read operation.
+ */
+struct tmc_sysfs_ops {
+	int (*read_prepare)(struct tmc_drvdata *drvdata);
+	int (*read_unprepare)(struct tmc_drvdata *drvdata);
+	ssize_t (*get_trace_data)(struct tmc_drvdata *drvdata, loff_t pos,
+				  size_t len, char **bufpp);
 };
 
 struct etr_buf_operations {

-- 
2.34.1

[PATCH v21 5/8] coresight: etr: add a new function to retrieve the CTCU device

[Jie Gan]

Add tmc_etr_get_ctcu_device function to find the ptr of the
coresight_device of the CTCU device if the CTCU device is connected to
the TMC ETR device.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 24 ++++++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h     |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 2de31d73a4b4..bf4c1d8f7b2e 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -865,6 +865,30 @@ tmc_etr_get_catu_device(struct tmc_drvdata *drvdata)
 }
 EXPORT_SYMBOL_GPL(tmc_etr_get_catu_device);
 
+/*
+ * TMC ETR could be connected to a CTCU device, which can provide ATID filter
+ * and byte-cntr service. This is represented by the output port of the TMC
+ * (ETR) connected to the input port of the CTCU.
+ *
+ * Returns	: coresight_device ptr for the CTCU device if a CTCU is found.
+ *		: NULL otherwise.
+ */
+struct coresight_device *
+tmc_etr_get_ctcu_device(struct tmc_drvdata *drvdata)
+{
+	struct coresight_device *etr = drvdata->csdev;
+	union coresight_dev_subtype ctcu_subtype = {
+		.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CTCU
+	};
+
+	if (!IS_ENABLED(CONFIG_CORESIGHT_CTCU))
+		return NULL;
+
+	return coresight_find_output_type(etr->pdata, CORESIGHT_DEV_TYPE_HELPER,
+					  ctcu_subtype);
+}
+EXPORT_SYMBOL_GPL(tmc_etr_get_ctcu_device);
+
 static const struct etr_buf_operations *etr_buf_ops[] = {
 	[ETR_MODE_FLAT] = &etr_flat_buf_ops,
 	[ETR_MODE_ETR_SG] = &etr_sg_buf_ops,
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 36c80918958f..2345bdbb0b9d 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -474,6 +474,7 @@ static inline uint32_t find_crash_tracedata_crc(struct tmc_drvdata *drvdata,
 }
 
 struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata);
+struct coresight_device *tmc_etr_get_ctcu_device(struct tmc_drvdata *drvdata);
 
 void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu);
 void tmc_etr_remove_catu_ops(void);

-- 
2.34.1

[PATCH v21 6/8] dt-bindings: arm: add an interrupt property for Coresight CTCU

[Jie Gan]

Add an interrupt property to CTCU device. The interrupt will be triggered
when the data size in the ETR buffer exceeds the threshold of the
BYTECNTRVAL register. Programming a threshold in the BYTECNTRVAL register
of CTCU device will enable the interrupt.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Mike Leach <mike.leach@arm.com>
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
index 335a8e1a5a9e..decb404e2ab1 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
@@ -49,6 +49,11 @@ properties:
     items:
       - const: apb
 
+  interrupts:
+    items:
+      - description: Interrupt for the ETR device connected to in-port0.
+      - description: Interrupt for the ETR device connected to in-port1.
+
   label:
     description:
       Description of a coresight device.
@@ -70,6 +75,8 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
     ctcu@1001000 {
         compatible = "qcom,sa8775p-ctcu";
         reg = <0x1001000 0x1000>;
@@ -77,6 +84,9 @@ examples:
         clocks = <&aoss_qmp>;
         clock-names = "apb";
 
+        interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>;
+
         in-ports {
             #address-cells = <1>;
             #size-cells = <0>;

-- 
2.34.1

[PATCH v21 7/8] coresight: ctcu: enable byte-cntr for TMC ETR devices

[Jie Gan]

The byte-cntr function provided by the CTCU device is used to transfer data
from the ETR buffer to the userspace. An interrupt is triggered if the data
size exceeds the threshold set in the BYTECNTRVAL register. The interrupt
handler counts the number of triggered interruptions and the read function
will read the data from the synced ETR buffer.

When the current etr_buf is nearly full or the timeout is triggered, the
ETR is switched to the other etr_buf of the etr_buf_list and the synced
etr_buf becomes available for reading, while the ETR keeps tracing. Once
the trace has been stopped, the last etr_buf is read by the generic sysfs
read of the TMC.

The byte-cntr read is exclusive with the generic sysfs read of the TMC. It
is used only when it is enabled for the ETR, otherwise the TMC falls back
to its original read.

Byte-cntr workflow:
start -> ctcu_enable(ctcu_byte_cntr_start) -> tmc_enable_etr_sink ->
tmc_read_prepare_etr(jump to tmc_read_prepare_byte_cntr) ->
tmc_etr_get_sysfs_trace(jump to tmc_byte_cntr_get_data) ->
tmc_disable_etr_sink -> ctcu_disable(ctcu_byte_cntr_stop) ->
tmc_read_unprepare_etr(jump to tmc_read_unprepare_byte_cntr) -> finish

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../ABI/testing/sysfs-bus-coresight-devices-ctcu   |   9 +
 drivers/hwtracing/coresight/Makefile               |   2 +-
 .../hwtracing/coresight/coresight-ctcu-byte-cntr.c | 400 +++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-ctcu-core.c  | 163 ++++++++-
 drivers/hwtracing/coresight/coresight-ctcu.h       |  88 ++++-
 drivers/hwtracing/coresight/coresight-tmc-core.c   |   3 +-
 drivers/hwtracing/coresight/coresight-tmc-etr.c    |  87 ++++-
 drivers/hwtracing/coresight/coresight-tmc.h        |   5 +
 8 files changed, 729 insertions(+), 28 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
new file mode 100644
index 000000000000..08dbc8e6b2ec
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-ctcu
@@ -0,0 +1,9 @@
+What:           /sys/bus/coresight/devices/<ctcu-name>/irq_enabled[0:1]
+Date:           October 2026
+KernelVersion:  7.5
+Contact:        Tingwei Zhang <tingwei.zhang@oss.qualcomm.com>; Jinlong Mao <jinlong.mao@oss.qualcomm.com>; Jie Gan <jie.gan@oss.qualcomm.com>
+Description:
+		(RW) Configure the flag to enable interrupt to count data during CTCU enablement.
+		An interrupt is generated when the data size exceeds the value set in the IRQ register.
+		0 : disable
+		1 : enable
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index ab16d06783a5..821a1b06b20c 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -55,5 +55,5 @@ coresight-cti-y := coresight-cti-core.o	coresight-cti-platform.o \
 obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
 obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
 obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o
-coresight-ctcu-y := coresight-ctcu-core.o
+coresight-ctcu-y := coresight-ctcu-core.o coresight-ctcu-byte-cntr.o
 obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o
diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
new file mode 100644
index 000000000000..bff87e776a75
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/of_irq.h>
+#include <linux/uaccess.h>
+
+#include "coresight-ctcu.h"
+#include "coresight-priv.h"
+#include "coresight-tmc.h"
+
+/*
+ * The byte-cntr sysfs ops are shared by all the CTCU devices, so they are
+ * registered by the first CTCU with byte-cntr and removed with the last one.
+ */
+static DEFINE_MUTEX(byte_cntr_ops_lock);
+static int byte_cntr_ops_users;
+
+static void ctcu_byte_cntr_put_ops(void *data)
+{
+	guard(mutex)(&byte_cntr_ops_lock);
+	if (!--byte_cntr_ops_users)
+		tmc_etr_reset_byte_cntr_sysfs_ops();
+}
+
+static irqreturn_t byte_cntr_handler(int irq, void *data)
+{
+	struct ctcu_byte_cntr *byte_cntr_data = data;
+
+	atomic_inc(&byte_cntr_data->irq_cnt);
+	wake_up(&byte_cntr_data->wq);
+
+	return IRQ_HANDLED;
+}
+
+static void ctcu_cfg_byte_cntr_reg(struct ctcu_drvdata *drvdata, u32 val,
+				   u32 offset)
+{
+	/* A one value for IRQCTRL register represents 8 bytes */
+	ctcu_program_register(drvdata, val / 8, offset);
+}
+
+static struct ctcu_byte_cntr *ctcu_get_byte_cntr(struct coresight_device *ctcu,
+						 struct coresight_device *etr)
+{
+	struct ctcu_byte_cntr *byte_cntr_data;
+	struct ctcu_drvdata *drvdata;
+	int port;
+
+	/* The ETR may not be connected to a CTCU device */
+	if (!ctcu)
+		return NULL;
+
+	drvdata = dev_get_drvdata(ctcu->dev.parent);
+	port = coresight_get_in_port(etr, ctcu);
+	if (port < 0 || port >= ETR_MAX_NUM)
+		return NULL;
+
+	byte_cntr_data = &drvdata->byte_cntr_data[port];
+	/* The port is not initialized by ctcu_byte_cntr_init() */
+	if (!byte_cntr_data->ctcu_drvdata)
+		return NULL;
+
+	return byte_cntr_data;
+}
+
+/*
+ * ctcu_byte_cntr_get_data() - reads data from the deactivated and filled buffer.
+ * The byte-cntr reading work reads data from the deactivated and filled buffer.
+ * The read operation waits for a buffer to become available, either filled or
+ * upon timeout, and then reads trace data from the synced buffer.
+ */
+static ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *etr_drvdata, loff_t pos,
+				      size_t len, char **bufpp)
+{
+	struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata);
+	struct device *dev = &etr_drvdata->csdev->dev;
+	struct ctcu_byte_cntr *byte_cntr_data;
+	struct etr_buf *sysfs_buf;
+	atomic_t *irq_cnt;
+	ssize_t actual;
+	int ret;
+
+	/* No byte-cntr read session, let the caller use the generic read */
+	byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev);
+	if (!byte_cntr_data || !byte_cntr_data->irq_enabled ||
+	    !byte_cntr_data->reading)
+		return -EINVAL;
+
+	irq_cnt = &byte_cntr_data->irq_cnt;
+
+wait_buffer:
+	if (!byte_cntr_data->buf_node) {
+		ret = wait_event_interruptible_timeout(byte_cntr_data->wq,
+				(atomic_read(irq_cnt) >= MAX_IRQ_CNT - 1) ||
+				!byte_cntr_data->enable,
+				BYTE_CNTR_TIMEOUT);
+		if (ret < 0)
+			return ret;
+		/*
+		 * The current etr_buf is almost full or timeout is triggered,
+		 * so switch the buffer and mark the switched buffer as reading.
+		 */
+		if (byte_cntr_data->enable) {
+			byte_cntr_data->buf_node =
+				tmc_etr_switch_buf_node(etr_drvdata);
+			if (!byte_cntr_data->buf_node) {
+				/*
+				 * The sink has stopped, let the generic read
+				 * handle the last etr_buf.
+				 */
+				if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_SYSFS)
+					return 0;
+
+				dev_err(dev, "Switch buffer failed for the byte-cntr\n");
+				return -ENOMEM;
+			}
+
+			/*
+			 * Get the interrupt count of the switched out etr_buf
+			 * and reset it for the next etr_buf.
+			 */
+			if (atomic_xchg(irq_cnt, 0) >= MAX_IRQ_CNT)
+				dev_warn(dev, "Data overwrite happened\n");
+		} else {
+			/* Exit byte-cntr reading */
+			return 0;
+		}
+	}
+
+	pos = byte_cntr_data->buf_node->pos;
+	sysfs_buf = byte_cntr_data->buf_node->sysfs_buf;
+	actual = tmc_etr_read_sysfs_buf(sysfs_buf, pos, len, bufpp);
+	if (actual <= 0) {
+		/* Reset buf_node upon reading is finished or failed */
+		byte_cntr_data->buf_node->reading = false;
+		byte_cntr_data->buf_node = NULL;
+
+		/*
+		 * Nothing in the buffer, waiting for the next buffer
+		 * to be filled.
+		 */
+		if (actual == 0)
+			goto wait_buffer;
+	}
+
+	return actual;
+}
+
+static int tmc_read_prepare_byte_cntr(struct tmc_drvdata *etr_drvdata)
+{
+	struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata);
+	struct ctcu_byte_cntr *byte_cntr_data;
+	unsigned long flags;
+	int ret = 0;
+
+	/* byte-cntr is operating with SYSFS mode being enabled only */
+	if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_SYSFS)
+		return -EINVAL;
+
+	byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev);
+	if (!byte_cntr_data)
+		return -EINVAL;
+
+	raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
+	/*
+	 * irq_enabled can only be changed with the lock held and reading
+	 * cleared, so check it here to keep it stable once reading is set.
+	 */
+	if (!byte_cntr_data->irq_enabled) {
+		raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
+		return -EINVAL;
+	}
+
+	if (byte_cntr_data->reading) {
+		raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
+		return -EBUSY;
+	}
+
+	/* byte_cntr_data->enable may race with ctcu_platform_remove() */
+	if (!byte_cntr_data->enable) {
+		raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
+		return -ENODEV;
+	}
+
+	/*
+	 * Pin the module for the whole read session, so that rmmod fails with
+	 * -EBUSY instead of blocking in ctcu_platform_remove() until the file
+	 * is closed.
+	 */
+	if (!try_module_get(THIS_MODULE)) {
+		raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
+		return -ENODEV;
+	}
+
+	reinit_completion(&byte_cntr_data->session_done);
+	byte_cntr_data->reading = true;
+	raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
+
+	/* Setup an available etr_buf_list for byte-cntr */
+	ret = tmc_create_etr_buf_list(etr_drvdata, 2);
+	if (ret)
+		goto err_reading;
+
+	/*
+	 * The byte-cntr register is configured and the interrupts are counted
+	 * since the path was enabled, so irq_cnt is not reset here.
+	 */
+	scoped_guard(raw_spinlock_irqsave, &byte_cntr_data->spin_lock)
+		byte_cntr_data->buf_node = NULL;
+	/* enable_irq_wake() may sleep on slow-bus irqchips, call it unlocked */
+	enable_irq_wake(byte_cntr_data->irq);
+
+	return 0;
+
+err_reading:
+	/*
+	 * reading was set before the sleeping allocations, so
+	 * ctcu_platform_remove() may be waiting for the session to end. Complete
+	 * the session with reading cleared in one critical section, so that a
+	 * new session can't start in between.
+	 */
+	scoped_guard(raw_spinlock_irqsave, &byte_cntr_data->spin_lock) {
+		byte_cntr_data->reading = false;
+		complete_all(&byte_cntr_data->session_done);
+	}
+
+	module_put(THIS_MODULE);
+	return ret;
+}
+
+static int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *etr_drvdata)
+{
+	struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata);
+	struct ctcu_byte_cntr *byte_cntr_data;
+
+	/*
+	 * Do the unprepare operation only when the byte_cntr_data->reading
+	 * is truly set
+	 */
+	byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev);
+	if (!byte_cntr_data || !byte_cntr_data->irq_enabled ||
+	    !byte_cntr_data->reading)
+		return -EINVAL;
+
+	tmc_clean_etr_buf_list(etr_drvdata);
+
+	/* disable_irq_wake() may sleep on slow-bus irqchips, call it unlocked */
+	disable_irq_wake(byte_cntr_data->irq);
+
+	/*
+	 * The byte-cntr register is not accessed here: the trace path, which
+	 * keeps the CTCU powered, may have been released already. It is reset
+	 * when the path is disabled.
+	 */
+	scoped_guard(raw_spinlock_irqsave, &byte_cntr_data->spin_lock) {
+		byte_cntr_data->buf_node = NULL;
+		byte_cntr_data->reading = false;
+		/*
+		 * Complete the session with reading cleared in one critical
+		 * section, so that a new session can't start in between.
+		 * ctcu_platform_remove() may free byte_cntr_data after this,
+		 * don't use it anymore.
+		 */
+		complete_all(&byte_cntr_data->session_done);
+	}
+
+	module_put(THIS_MODULE);
+
+	return 0;
+}
+
+static const struct tmc_sysfs_ops byte_cntr_sysfs_ops = {
+	.read_prepare	= tmc_read_prepare_byte_cntr,
+	.read_unprepare	= tmc_read_unprepare_byte_cntr,
+	.get_trace_data	= tmc_byte_cntr_get_data,
+};
+
+/* Start the byte-cntr function when the path is enabled. */
+void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path)
+{
+	struct coresight_device *sink = coresight_get_sink(path);
+	struct tmc_drvdata *etr_drvdata;
+	struct ctcu_byte_cntr *byte_cntr_data;
+
+	byte_cntr_data = ctcu_get_byte_cntr(csdev, sink);
+	if (!byte_cntr_data)
+		return;
+
+	etr_drvdata = dev_get_drvdata(sink->dev.parent);
+	guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock);
+	/*
+	 * Don't start byte-cntr function when irq_enabled is not set, or when
+	 * the CTCU device is being removed.
+	 */
+	if (!byte_cntr_data->irq_enabled || byte_cntr_data->removing)
+		return;
+
+	/*
+	 * A new trace starts with an empty etr_buf. Don't reset the count if
+	 * the byte-cntr is already started by another path of the same sink.
+	 */
+	if (!byte_cntr_data->enable)
+		atomic_set(&byte_cntr_data->irq_cnt, 0);
+
+	/*
+	 * Configure the byte-cntr register to enable IRQ while the path is
+	 * enabled, the CTCU is powered then. The configured size is 5% of the
+	 * buffer_size.
+	 */
+	ctcu_cfg_byte_cntr_reg(byte_cntr_data->ctcu_drvdata,
+			       etr_drvdata->size / MAX_IRQ_CNT,
+			       byte_cntr_data->irq_ctrl_offset);
+	byte_cntr_data->enable = true;
+}
+
+/* Stop the byte-cntr function when the path is disabled. */
+void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path)
+{
+	struct coresight_device *sink = coresight_get_sink(path);
+	struct ctcu_byte_cntr *byte_cntr_data;
+
+	if (coresight_get_mode(sink) == CS_MODE_SYSFS)
+		return;
+
+	byte_cntr_data = ctcu_get_byte_cntr(csdev, sink);
+	if (!byte_cntr_data)
+		return;
+
+	scoped_guard(raw_spinlock_irqsave, &byte_cntr_data->spin_lock) {
+		/*
+		 * Configure the byte-cntr register to disable IRQ before the
+		 * path is released, the CTCU is still powered.
+		 */
+		if (byte_cntr_data->enable)
+			ctcu_cfg_byte_cntr_reg(byte_cntr_data->ctcu_drvdata, 0,
+					       byte_cntr_data->irq_ctrl_offset);
+		byte_cntr_data->enable = false;
+	}
+
+	/* Wake up the blocked reader, which exits the byte-cntr reading */
+	wake_up(&byte_cntr_data->wq);
+}
+
+void ctcu_byte_cntr_init(struct device *dev, struct ctcu_drvdata *drvdata, int etr_num)
+{
+	struct ctcu_byte_cntr *byte_cntr_data;
+	struct device_node *nd = dev->of_node;
+	int irq_num, ret, i, irq_registered = 0;
+
+	for (i = 0; i < etr_num; i++) {
+		byte_cntr_data = &drvdata->byte_cntr_data[i];
+		/*
+		 * Initialize the lock and the waitqueue before requesting the
+		 * IRQ, and for every port as ctcu_platform_remove() uses them
+		 * even if the IRQ of the port is not registered.
+		 */
+		byte_cntr_data->ctcu_drvdata = drvdata;
+		init_waitqueue_head(&byte_cntr_data->wq);
+		raw_spin_lock_init(&byte_cntr_data->spin_lock);
+		init_completion(&byte_cntr_data->session_done);
+		/* No read session in progress */
+		complete_all(&byte_cntr_data->session_done);
+
+		irq_num = of_irq_get(nd, i);
+		if (irq_num <= 0) {
+			dev_err(dev, "Failed to get IRQ from DT for port%d\n", i);
+			continue;
+		}
+
+		ret = devm_request_irq(dev, irq_num, byte_cntr_handler,
+				       IRQF_TRIGGER_RISING,
+				       dev_name(dev), byte_cntr_data);
+		if (ret) {
+			dev_err(dev, "Failed to register IRQ for port%d\n", i);
+			continue;
+		}
+
+		byte_cntr_data->irq = irq_num;
+		irq_registered++;
+	}
+
+	if (!irq_registered)
+		return;
+
+	scoped_guard(mutex, &byte_cntr_ops_lock) {
+		tmc_etr_set_byte_cntr_sysfs_ops(&byte_cntr_sysfs_ops);
+		byte_cntr_ops_users++;
+	}
+
+	/* Drop the ops user when the CTCU device is removed or probe fails */
+	if (devm_add_action_or_reset(dev, ctcu_byte_cntr_put_ops, NULL))
+		dev_err(dev, "Failed to add the byte-cntr devm action\n");
+}
diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
index 739105acb0a1..cce4e4d76da0 100644
--- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
+++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2024-2026 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
  */
 
 #include <linux/clk.h>
@@ -18,6 +19,7 @@
 
 #include "coresight-ctcu.h"
 #include "coresight-priv.h"
+#include "coresight-tmc.h"
 
 #define ctcu_writel(drvdata, val, offset)	__raw_writel((val), drvdata->base + offset)
 #define ctcu_readl(drvdata, offset)		__raw_readl(drvdata->base + offset)
@@ -43,17 +45,21 @@
 
 #define CTCU_ATID_REG_BIT(traceid)	(traceid % 32)
 #define CTCU_ATID_REG_SIZE		0x10
+#define CTCU_ETR0_IRQCTRL               0x6c
+#define CTCU_ETR1_IRQCTRL               0x70
 #define CTCU_ETR0_ATID0			0xf8
 #define CTCU_ETR1_ATID0			0x108
 
 static const struct ctcu_etr_config sa8775p_etr_cfgs[] = {
 	{
-		.atid_offset	= CTCU_ETR0_ATID0,
-		.port_num	= 0,
+		.atid_offset		= CTCU_ETR0_ATID0,
+		.irq_ctrl_offset	= CTCU_ETR0_IRQCTRL,
+		.port_num		= 0,
 	},
 	{
-		.atid_offset	= CTCU_ETR1_ATID0,
-		.port_num	= 1,
+		.atid_offset		= CTCU_ETR1_ATID0,
+		.irq_ctrl_offset	= CTCU_ETR1_IRQCTRL,
+		.port_num		= 1,
 	},
 };
 
@@ -62,6 +68,93 @@ static const struct ctcu_config sa8775p_cfgs = {
 	.num_etr_config	= ARRAY_SIZE(sa8775p_etr_cfgs),
 };
 
+/*
+ * The register access is unlocked and locked in the same way as the ATID
+ * register, so serialise them with drvdata->spin_lock. Otherwise one side may
+ * lock the registers between the unlock and the write of the other side.
+ */
+void ctcu_program_register(struct ctcu_drvdata *drvdata, u32 val, u32 offset)
+{
+	guard(raw_spinlock_irqsave)(&drvdata->spin_lock);
+	CS_UNLOCK(drvdata->base);
+	ctcu_writel(drvdata, val, offset);
+	CS_LOCK(drvdata->base);
+}
+
+static ssize_t irq_enabled_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct ctcu_byte_cntr_irq_attribute *irq_attr =
+		container_of(attr, struct ctcu_byte_cntr_irq_attribute, attr);
+	struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	u8 port = irq_attr->port;
+
+	if (!drvdata->byte_cntr_data[port].irq_ctrl_offset)
+		return -EINVAL;
+
+	return sysfs_emit(buf, "%u\n",
+			(unsigned int)drvdata->byte_cntr_data[port].irq_enabled);
+}
+
+static ssize_t irq_enabled_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf,
+				 size_t size)
+{
+	struct ctcu_byte_cntr_irq_attribute *irq_attr =
+		container_of(attr, struct ctcu_byte_cntr_irq_attribute, attr);
+	struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	u8 port = irq_attr->port;
+	unsigned long val;
+
+	if (kstrtoul(buf, 0, &val))
+		return -EINVAL;
+
+	guard(raw_spinlock_irqsave)(&drvdata->byte_cntr_data[port].spin_lock);
+	if (drvdata->byte_cntr_data[port].reading)
+		return -EBUSY;
+	else if (drvdata->byte_cntr_data[port].irq_ctrl_offset)
+		drvdata->byte_cntr_data[port].irq_enabled = !!val;
+
+	return size;
+}
+
+static umode_t irq_enabled_is_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
+{
+	struct device_attribute *dev_attr =
+		container_of(attr, struct device_attribute, attr);
+	struct ctcu_byte_cntr_irq_attribute *irq_attr =
+		container_of(dev_attr, struct ctcu_byte_cntr_irq_attribute, attr);
+	struct device *dev = kobj_to_dev(kobj);
+	struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	u8 port = irq_attr->port;
+
+	/* Expose irq_enabled only for the ports with a registered IRQ */
+	if (drvdata && drvdata->byte_cntr_data[port].irq_ctrl_offset &&
+	    drvdata->byte_cntr_data[port].irq)
+		return attr->mode;
+
+	return 0;
+}
+
+static struct attribute *ctcu_attrs[] = {
+	ctcu_byte_cntr_irq_rw(0),
+	ctcu_byte_cntr_irq_rw(1),
+	NULL,
+};
+
+static struct attribute_group ctcu_attr_grp = {
+	.attrs = ctcu_attrs,
+	.is_visible = irq_enabled_is_visible,
+};
+
+static const struct attribute_group *ctcu_attr_grps[] = {
+	&ctcu_attr_grp,
+	NULL,
+};
+
 static void ctcu_program_atid_register(struct ctcu_drvdata *drvdata, u32 reg_offset,
 				       u8 bit, bool enable)
 {
@@ -140,11 +233,23 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight
 static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode,
 		       struct coresight_path *path)
 {
-	return ctcu_set_etr_traceid(csdev, path, true);
+	int ret;
+
+	ret = ctcu_set_etr_traceid(csdev, path, true);
+	/*
+	 * Byte-cntr works in sysfs mode only. Start it once the trace ID is set,
+	 * as ctcu_disable() won't be called to stop it if the enabling failed.
+	 */
+	if (!ret && mode == CS_MODE_SYSFS)
+		ctcu_byte_cntr_start(csdev, path);
+
+	return ret;
 }
 
 static int ctcu_disable(struct coresight_device *csdev, struct coresight_path *path)
 {
+	ctcu_byte_cntr_stop(csdev, path);
+
 	return ctcu_set_etr_traceid(csdev, path, false);
 }
 
@@ -195,7 +300,10 @@ static int ctcu_probe(struct platform_device *pdev)
 			for (i = 0; i < cfgs->num_etr_config; i++) {
 				etr_cfg = &cfgs->etr_cfgs[i];
 				drvdata->atid_offset[i] = etr_cfg->atid_offset;
+				drvdata->byte_cntr_data[i].irq_ctrl_offset =
+					etr_cfg->irq_ctrl_offset;
 			}
+			ctcu_byte_cntr_init(dev, drvdata, cfgs->num_etr_config);
 		}
 	}
 
@@ -209,6 +317,7 @@ static int ctcu_probe(struct platform_device *pdev)
 	desc.dev = dev;
 	desc.ops = &ctcu_ops;
 	desc.access = CSDEV_ACCESS_IOMEM(base);
+	desc.groups = ctcu_attr_grps;
 	raw_spin_lock_init(&drvdata->spin_lock);
 
 	drvdata->csdev = coresight_register(&desc);
@@ -244,10 +353,52 @@ static int ctcu_platform_probe(struct platform_device *pdev)
 static void ctcu_platform_remove(struct platform_device *pdev)
 {
 	struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev);
+	struct ctcu_byte_cntr *byte_cntr_data;
+	unsigned long flags;
+	int i;
 
 	if (WARN_ON(!drvdata))
 		return;
 
+	/*
+	 * Signal all active byte-cntr readers to exit, then wait for them to
+	 * finish before resetting the ops pointer and freeing driver data.
+	 * Without this, a reader blocked in wait_event_interruptible_timeout()
+	 * would access the freed ctcu_drvdata wait-queue head (use-after-free).
+	 *
+	 * The wait ends only when the userspace closes the tmc_etr file, so an
+	 * unbind with the file left open blocks until then. Unloading the module
+	 * is rejected with -EBUSY while a read session is open, so only a manual
+	 * unbind can block here.
+	 */
+	for (i = 0; i < ETR_MAX_NUM; i++) {
+		byte_cntr_data = &drvdata->byte_cntr_data[i];
+		/* The port was not initialized by ctcu_byte_cntr_init() */
+		if (!byte_cntr_data->irq)
+			continue;
+
+		raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
+		/*
+		 * Set enable=false to signal teardown to racing readers, and
+		 * removing=true to keep a new path from enabling byte-cntr again.
+		 */
+		byte_cntr_data->enable = false;
+		byte_cntr_data->removing = true;
+		raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
+
+		/* Wake up the blocked reader, then wait for the session to end */
+		wake_up_all(&byte_cntr_data->wq);
+		wait_for_completion(&byte_cntr_data->session_done);
+
+		/*
+		 * The ending session completes it with the lock held. Take the
+		 * lock once more, to be sure that it has released the lock
+		 * before byte_cntr_data is freed.
+		 */
+		raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
+		raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
+	}
+
 	ctcu_remove(pdev);
 	pm_runtime_disable(&pdev->dev);
 }
diff --git a/drivers/hwtracing/coresight/coresight-ctcu.h b/drivers/hwtracing/coresight/coresight-ctcu.h
index e9594c38dd91..60dded125ece 100644
--- a/drivers/hwtracing/coresight/coresight-ctcu.h
+++ b/drivers/hwtracing/coresight/coresight-ctcu.h
@@ -1,23 +1,32 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2024-2026 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
  */
 
 #ifndef _CORESIGHT_CTCU_H
 #define _CORESIGHT_CTCU_H
+
+#include <linux/completion.h>
+#include <linux/time.h>
 #include "coresight-trace-id.h"
 
 /* Maximum number of supported ETR devices for a single CTCU. */
 #define ETR_MAX_NUM	2
 
+#define BYTE_CNTR_TIMEOUT	(3 * HZ)
+#define MAX_IRQ_CNT		20
+
 /**
- * struct ctcu_etr_config
+ * struct ctcu_etr_config - ETR specific configuration of the CTCU device
  * @atid_offset:	offset to the ATID0 Register.
- * @port_num:		in-port number of CTCU device that connected to ETR.
+ * @port_num:		in-port number of the CTCU device that connected to ETR.
+ * @irq_ctrl_offset:    offset to the BYTECNTRVAL register.
  */
 struct ctcu_etr_config {
 	const u32 atid_offset;
 	const u32 port_num;
+	const u32 irq_ctrl_offset;
 };
 
 struct ctcu_config {
@@ -25,15 +34,72 @@ struct ctcu_config {
 	int num_etr_config;
 };
 
-struct ctcu_drvdata {
-	void __iomem		*base;
-	struct clk		*apb_clk;
-	struct device		*dev;
-	struct coresight_device	*csdev;
+/**
+ * struct ctcu_byte_cntr - byte-cntr data of a port of the CTCU device
+ * @enable:		indicates that byte_cntr function is enabled or not.
+ * @irq_enabled:	indicates that the interruption is enabled.
+ * @reading:		indicates that byte_cntr is reading.
+ * @removing:		indicates that the CTCU device is being removed.
+ * @irq:		allocated number of the IRQ.
+ * @irq_cnt:		IRQ count number of the triggered interruptions.
+ * @wq:			waitqueue for reading data from ETR buffer.
+ * @spin_lock:		spinlock of the byte_cntr_data.
+ * @session_done:	completed when there is no read session in progress.
+ * @irq_ctrl_offset:	offset to the BYTECNTVAL Register.
+ * @ctcu_drvdata:	drvdata of the CTCU device.
+ * @buf_node:		etr_buf_node for reading.
+ */
+struct ctcu_byte_cntr {
+	bool			enable;
+	bool			irq_enabled;
+	bool			reading;
+	bool			removing;
+	int			irq;
+	atomic_t		irq_cnt;
+	wait_queue_head_t	wq;
 	raw_spinlock_t		spin_lock;
-	u32			atid_offset[ETR_MAX_NUM];
-	/* refcnt for each traceid of each sink */
-	u8			traceid_refcnt[ETR_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP];
+	struct completion	session_done;
+	u32			irq_ctrl_offset;
+	struct ctcu_drvdata	*ctcu_drvdata;
+	struct etr_buf_node	*buf_node;
 };
 
+struct ctcu_drvdata {
+	void __iomem			*base;
+	struct clk			*apb_clk;
+	struct device			*dev;
+	struct coresight_device		*csdev;
+	struct ctcu_byte_cntr		byte_cntr_data[ETR_MAX_NUM];
+	raw_spinlock_t			spin_lock;
+	u32				atid_offset[ETR_MAX_NUM];
+	/* refcnt for each traceid of each sink */
+	u8				traceid_refcnt[ETR_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP];
+};
+
+/**
+ * struct ctcu_byte_cntr_irq_attribute - sysfs attribute of the byte-cntr IRQ
+ * @attr:	The device attribute.
+ * @port:	port number.
+ */
+struct ctcu_byte_cntr_irq_attribute {
+	struct device_attribute	attr;
+	u8			port;
+};
+
+#define ctcu_byte_cntr_irq_rw(port)					\
+	(&((struct ctcu_byte_cntr_irq_attribute[]) {			\
+	   {								\
+		__ATTR(irq_enabled##port, 0644, irq_enabled_show,	\
+		irq_enabled_store),					\
+		port,							\
+	   }								\
+	})[0].attr.attr)
+
+void ctcu_program_register(struct ctcu_drvdata *drvdata, u32 val, u32 offset);
+
+/* Byte-cntr functions */
+void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path);
+void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path);
+void ctcu_byte_cntr_init(struct device *dev, struct ctcu_drvdata *drvdata, int port_num);
+
 #endif
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 4b40b692be4d..6ad09995ba87 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -293,7 +293,8 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
 		return -EFAULT;
 	}
 
-	*ppos += actual;
+	if (!tmc_etr_update_buf_node_pos(drvdata, actual))
+		*ppos += actual;
 	dev_dbg(&drvdata->csdev->dev, "%zu bytes copied\n", actual);
 
 	return actual;
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index bf4c1d8f7b2e..20d6bc60a446 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1168,6 +1168,9 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
 	return rc;
 }
 
+/* Shared by the CTCU devices, set by the first one and reset by the last one. */
+static const struct tmc_sysfs_ops *byte_cntr_sysfs_ops;
+
 /*
  * Return the available trace data in the buffer (starts at etr_buf->offset,
  * limited by etr_buf->len) from @pos, with a maximum limit of @len,
@@ -1178,23 +1181,44 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
  * We are protected here by drvdata->reading != 0, which ensures the
  * sysfs_buf stays alive.
  */
-ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
-				loff_t pos, size_t len, char **bufpp)
+ssize_t tmc_etr_read_sysfs_buf(struct etr_buf *sysfs_buf, loff_t pos,
+			       size_t len, char **bufpp)
 {
 	s64 offset;
 	ssize_t actual = len;
-	struct etr_buf *etr_buf = drvdata->sysfs_buf;
 
-	if (pos + actual > etr_buf->len)
-		actual = etr_buf->len - pos;
+	if (pos + actual > sysfs_buf->len)
+		actual = sysfs_buf->len - pos;
 	if (actual <= 0)
 		return actual;
 
 	/* Compute the offset from which we read the data */
-	offset = etr_buf->offset + pos;
-	if (offset >= etr_buf->size)
-		offset -= etr_buf->size;
-	return tmc_etr_buf_get_data(etr_buf, offset, actual, bufpp);
+	offset = sysfs_buf->offset + pos;
+	if (offset >= sysfs_buf->size)
+		offset -= sysfs_buf->size;
+	return tmc_etr_buf_get_data(sysfs_buf, offset, actual, bufpp);
+}
+EXPORT_SYMBOL_GPL(tmc_etr_read_sysfs_buf);
+
+ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
+				loff_t pos, size_t len, char **bufpp)
+{
+	ssize_t ret;
+	const struct tmc_sysfs_ops *byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
+
+	if (byte_cntr_ops) {
+		ret = byte_cntr_ops->get_trace_data(drvdata, pos, len, bufpp);
+		/*
+		 * -EINVAL means byte-cntr has no active read session. 0 means
+		 * byte-cntr has no more data because the sink has stopped, the
+		 * last etr_buf is read by the generic read. Anything else is
+		 * the result of the byte-cntr session.
+		 */
+		if (ret != -EINVAL && ret != 0)
+			return ret;
+	}
+
+	return tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp);
 }
 
 static struct etr_buf *
@@ -2170,15 +2194,54 @@ int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes)
 }
 EXPORT_SYMBOL_GPL(tmc_create_etr_buf_list);
 
+void tmc_etr_set_byte_cntr_sysfs_ops(const struct tmc_sysfs_ops *sysfs_ops)
+{
+	WRITE_ONCE(byte_cntr_sysfs_ops, sysfs_ops);
+}
+EXPORT_SYMBOL_GPL(tmc_etr_set_byte_cntr_sysfs_ops);
+
+void tmc_etr_reset_byte_cntr_sysfs_ops(void)
+{
+	WRITE_ONCE(byte_cntr_sysfs_ops, NULL);
+}
+EXPORT_SYMBOL_GPL(tmc_etr_reset_byte_cntr_sysfs_ops);
+
+bool tmc_etr_update_buf_node_pos(struct tmc_drvdata *drvdata, ssize_t size)
+{
+	struct etr_buf_node *nd, *next;
+
+	if (drvdata->config_type != TMC_CONFIG_TYPE_ETR)
+		return false;
+
+	list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) {
+		if (nd && nd->reading) {
+			nd->pos += size;
+			return true;
+		}
+	}
+
+	return false;
+}
+
 int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 {
 	int ret = 0;
 	unsigned long flags;
+	const struct tmc_sysfs_ops *byte_cntr_ops;
 
 	/* config types are set a boot time and never change */
 	if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
 		return -EINVAL;
 
+	byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
+	if (byte_cntr_ops) {
+		ret = byte_cntr_ops->read_prepare(drvdata);
+		if (!ret || ret == -EBUSY)
+			return ret;
+
+		ret = 0;
+	}
+
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 	/* The sysfs_buf is owned by the etr_buf_list */
 	if (drvdata->reading || !list_empty(&drvdata->etr_buf_list)) {
@@ -2211,11 +2274,17 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
 {
 	unsigned long flags;
 	struct etr_buf *sysfs_buf = NULL;
+	const struct tmc_sysfs_ops *byte_cntr_ops;
 
 	/* config types are set a boot time and never change */
 	if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
 		return -EINVAL;
 
+	byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
+	if (byte_cntr_ops)
+		if (!byte_cntr_ops->read_unprepare(drvdata))
+			return 0;
+
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 
 	/* RE-enable the TMC if need be */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 2345bdbb0b9d..af89825ec041 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -484,6 +484,11 @@ struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
 extern const struct attribute_group coresight_etr_group;
 void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata);
 int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes);
+void tmc_etr_set_byte_cntr_sysfs_ops(const struct tmc_sysfs_ops *sysfs_ops);
+void tmc_etr_reset_byte_cntr_sysfs_ops(void);
 struct etr_buf_node *tmc_etr_switch_buf_node(struct tmc_drvdata *drvdata);
+bool tmc_etr_update_buf_node_pos(struct tmc_drvdata *drvdata, ssize_t size);
+ssize_t tmc_etr_read_sysfs_buf(struct etr_buf *sysfs_buf, loff_t pos,
+			       size_t len, char **bufpp);
 
 #endif

-- 
2.34.1

[PATCH v21 8/8] arm64: dts: qcom: lemans: add interrupts to CTCU device

[Jie Gan]

Add interrupts to enable byte-cntr function for TMC ETR devices.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/lemans.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
index 1fdb72fa5518..852a1142100f 100644
--- a/arch/arm64/boot/dts/qcom/lemans.dtsi
+++ b/arch/arm64/boot/dts/qcom/lemans.dtsi
@@ -3132,6 +3132,9 @@ ctcu@4001000 {
 			clocks = <&aoss_qmp>;
 			clock-names = "apb";
 
+			interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>;
+
 			in-ports {
 				#address-cells = <1>;
 				#size-cells = <0>;

-- 
2.34.1

Re: [PATCH v21 2/8] coresight: tmc: add create/clean functions for etr_buf_list

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] coresight: tmc-etr: sink disable race in tmc_create_etr_buf_list()
- [Medium] coresight: tmc-etr: incomplete ETR_MODE_RESRV check in tmc_create_etr_buf_list()
--

commit d6d5c00634b7362a8e8cc1b6d8b0dc4cadb55d89
Author: Jie Gan <jie.gan@oss.qualcomm.com>

coresight: tmc: add create/clean functions for etr_buf_list

This commit introduces tmc_create_etr_buf_list() and
tmc_clean_etr_buf_list() to manage a list of etr_buf_node buffers on
TMC ETR devices for byte-cntr trace collection.

> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 361a433e6f0c5..eee98a9269df4 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
[ ... ]
> @@ -1918,6 +1920,167 @@ const struct coresight_ops tmc_etr_cs_ops = {
>  	.panic_ops	= &tmc_etr_sync_ops,
>  };
>  
> +/**
> + * tmc_clean_etr_buf_list - clean the etr_buf_list.
> + * @drvdata:	driver data of the TMC device.
> + *
> + * Remove all nodes from @drvdata->etr_buf_list and free their buffers.
> + * If a node holds the live sysfs_buf and the sink is enabled in sysfs mode, the
> + * node is removed but the buffer is not freed; ownership stays with
> + * drvdata->sysfs_buf.
> + *
> + * Locking: must not be called with @drvdata->spinlock held. The spinlock is
> + * taken internally to serialise the @drvdata->sysfs_buf accesses and the
> + * removal of the nodes against the ETR sink enable/disable paths and the
> + * generic sysfs read. There must be only one owner of the list at a time.
> + * Must be called from process context: buffers are freed with the lock
> + * released.
> + */
> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
> +{
> +	struct etr_buf_node *nd, *next;
> +	unsigned long flags;
> +
> +	list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) {
> +		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +		if (nd->sysfs_buf == drvdata->sysfs_buf) {
> +			if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
> +				/*
> +				 * The sink is still enabled in sysfs mode. Keep
> +				 * the live buffer owned by drvdata->sysfs_buf
> +				 * and only drop the list's reference to it.
> +				 */
> +				nd->sysfs_buf = NULL;
> +			else
> +				/* Free the buffer below through nd->sysfs_buf */
> +				drvdata->sysfs_buf = NULL;
> +		}
> +		list_del(&nd->link);
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +
> +		/* Free the buffer (NULL is ignored) and the node out of the lock */
> +		tmc_etr_free_sysfs_buf(nd->sysfs_buf);
> +		kfree(nd);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
> +
> +/**
> + * tmc_create_etr_buf_list - create a list to manage the etr_buf_node.
> + * @drvdata:	driver data of the TMC device.
> + * @num_nodes:	number of nodes want to create with the list.
> + *
> + * Locking: must not be called with @drvdata->spinlock held. The spinlock is
> + * taken internally to serialise the @drvdata->sysfs_buf accesses and every
> + * update of @drvdata->etr_buf_list against the ETR sink enable/disable paths
> + * and the generic sysfs read. There must be only one owner of the list at a
> + * time. Must be called from process context: buffers and nodes are allocated
> + * with the lock released.
> + *
> + * Return: 0 upon success, else the error number: -EBUSY if a generic sysfs
> + * read is in progress, -EINVAL if the sink is not enabled in sysfs mode or
> + * the ETR is in the reserved buffer mode, or the error of the allocation.
> + */
> +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes)
> +{
> +	struct etr_buf_node *new_node;
> +	struct etr_buf *sysfs_buf;
> +	unsigned long flags;
> +	int i = 0, ret = 0;
> +
> +	/* We don't need a list if there is only one node */
> +	if (num_nodes < 2)
> +		return -EINVAL;
> +
> +	/*
> +	 * All the etr_buf of the reserved mode are mapped to the same reserved
> +	 * region, so they can't be used as separate buffers of the list.
> +	 */
> +	if (drvdata->etr_mode == ETR_MODE_RESRV)
> +		return -EINVAL;

[Severity: Medium]
As reported by Sashiko on v20 [1], can ETR_MODE_RESRV buffers still end up
in drvdata->etr_buf_list despite this check?

In tmc_create_etr_buf_list(), drvdata->etr_mode == ETR_MODE_RESRV is
only checked once without holding drvdata->spinlock, and
drvdata->sysfs_buf->mode is not checked when wrapping the existing
drvdata->sysfs_buf into the list.

Because buf_mode_preferred_store() allows userspace to change
drvdata->etr_mode at any time via sysfs:
1. If the ETR sink was enabled while drvdata->etr_mode was
   ETR_MODE_RESRV (or reused a prior ETR_MODE_RESRV buffer in
   tmc_etr_get_sysfs_buffer()), drvdata->sysfs_buf->mode is
   ETR_MODE_RESRV. If buf_mode_preferred is then switched to another
   mode before tmc_create_etr_buf_list() runs, this check passes and
   the ETR_MODE_RESRV drvdata->sysfs_buf is wrapped into
   drvdata->etr_buf_list.
2. If buf_mode_preferred_store() -> buf_mode_set_resrv() changes
   drvdata->etr_mode to ETR_MODE_RESRV after this check while
   tmc_create_etr_buf_list() is in the unlocked allocation loop,
   tmc_alloc_etr_buf() re-reads drvdata->etr_mode and calls
   tmc_etr_alloc_resrv_buf(), mapping multiple nodes in
   drvdata->etr_buf_list to the same static drvdata->resrv_buf region.

Should tmc_create_etr_buf_list() also verify that
drvdata->sysfs_buf->mode != ETR_MODE_RESRV and that newly allocated
buffers do not have sysfs_buf->mode == ETR_MODE_RESRV?

> +
> +	/*
> +	 * We expect that sysfs_buf in drvdata has already been allocated.
> +	 * Wrap the live sysfs_buf into the first node so the captured trace
> +	 * data is preserved. Take the sysfs_buf and add the node under the
> +	 * lock, so that the ETR sink enable path sees a non-empty list and
> +	 * doesn't replace the buffer.
> +	 */
> +	new_node = kzalloc_obj(*new_node, GFP_KERNEL);
> +	if (!new_node)
> +		return -ENOMEM;
> +
> +	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +	/* The sysfs_buf is in use by a sysfs read session */
> +	if (drvdata->reading) {
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +		kfree(new_node);
> +		return -EBUSY;
> +	}
> +
> +	/*
> +	 * The sink enable path reuses or frees the sysfs_buf of a sink which
> +	 * isn't enabled, so only wrap it while the sink is enabled in sysfs mode.
> +	 */
> +	if (coresight_get_mode(drvdata->csdev) != CS_MODE_SYSFS) {
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +		kfree(new_node);
> +		return -EINVAL;
> +	}
> +
> +	if (drvdata->sysfs_buf) {
> +		new_node->sysfs_buf = drvdata->sysfs_buf;
> +		new_node->is_free = false;
> +		list_add(&new_node->link, &drvdata->etr_buf_list);
> +		new_node = NULL;
> +		i++;
> +	}
> +	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +	/* Free the node if there is no live sysfs_buf to wrap */
> +	kfree(new_node);
> +
> +	while (i < num_nodes) {
> +		new_node = kzalloc_obj(*new_node, GFP_KERNEL);
> +		if (!new_node) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +
> +		/* Allocate the buffer with the lock released */
> +		sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
> +		if (IS_ERR(sysfs_buf)) {
> +			kfree(new_node);
> +			ret = PTR_ERR(sysfs_buf);
> +			break;
> +		}
> +
> +		new_node->sysfs_buf = sysfs_buf;
> +		/*
> +		 * The drvdata->sysfs_buf write and the list update need the
> +		 * spinlock, to serialise against the ETR sink enable/disable
> +		 * paths.
> +		 */
> +		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +		/* We don't have an available sysfs_buf in drvdata, set one up */
> +		if (!drvdata->sysfs_buf) {
> +			drvdata->sysfs_buf = sysfs_buf;
> +			new_node->is_free = false;
> +		} else {
> +			new_node->is_free = true;
> +		}
> +		list_add_tail(&new_node->link, &drvdata->etr_buf_list);
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +		i++;
> +	}
> +
> +	/* Clean the list if there is an error */
> +	if (ret)
> +		tmc_clean_etr_buf_list(drvdata);
> +
> +	return ret;
> +}

[Severity: High]
As reported by Sashiko on v20 [2], what happens if the ETR sink is
disabled via sysfs while tmc_create_etr_buf_list() has dropped
drvdata->spinlock during the while (i < num_nodes) allocation loop?

Because drvdata->reading is not set and tmc_disable_etr_sink() only
checks drvdata->reading (not list_empty(&drvdata->etr_buf_list)), a
concurrent coresight_disable_sysfs() -> tmc_disable_etr_sink() can flush
and sync the hardware trace data into drvdata->sysfs_buf and set
drvdata->csdev mode to CS_MODE_DISABLED (after which
tmc_enable_etr_sink_perf() could also transition it to CS_MODE_PERF)
while drvdata->spinlock is released for kzalloc_obj() and
tmc_alloc_etr_buf():

1. If a subsequent allocation in the loop fails (ret != 0),
   tmc_create_etr_buf_list() calls tmc_clean_etr_buf_list(drvdata).
   Because coresight_get_mode(drvdata->csdev) is no longer
   CS_MODE_SYSFS, tmc_clean_etr_buf_list() takes the else branch:

	if (nd->sysfs_buf == drvdata->sysfs_buf) {
		if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
			nd->sysfs_buf = NULL;
		else
			/* Free the buffer below through nd->sysfs_buf */
			drvdata->sysfs_buf = NULL;
	}

   and frees the pre-existing drvdata->sysfs_buf via
   tmc_etr_free_sysfs_buf(nd->sysfs_buf), discarding the synced sysfs
   trace data on an error rollback.

2. If all allocations in the loop succeed after the sink was
   concurrently disabled, the loop re-acquires drvdata->spinlock and
   adds the nodes to drvdata->etr_buf_list without re-checking
   coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS, returning 0
   with a populated etr_buf_list on a sink that is no longer in
   CS_MODE_SYSFS.

Could tmc_create_etr_buf_list() re-check
coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS under
drvdata->spinlock and ensure error cleanup does not free the
pre-existing drvdata->sysfs_buf?

[1] https://lore.kernel.org/r/20261030-enable-byte-cntr-for-ctcu-v20-2-bd5c3ac9f916@oss.qualcomm.com
[2] https://lore.kernel.org/r/20261030-enable-byte-cntr-for-ctcu-v20-2-bd5c3ac9f916@oss.qualcomm.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-enable-byte-cntr-for-ctcu-v21-0-231e069b7853@oss.qualcomm.com?part=2

Re: [PATCH v21 3/8] coresight: tmc: add a function to switch the ETR to a free etr_buf

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] coresight: tmc: active read buffer reuse in tmc_etr_switch_buf_node()
--

commit edde9bcbd54db59cd64f7044e1302e2f89c8fc07
Author: Jie Gan <jie.gan@oss.qualcomm.com>

coresight: tmc: add a function to switch the ETR to a free etr_buf

This commit adds tmc_etr_switch_buf_node() to stop the ETR, switch to a
free etr_buf from etr_buf_list, and restart the ETR under drvdata->spinlock
while returning the switched-out buffer node for reading.

> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index eee98a9269df4..2de31d73a4b47 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1224,6 +1224,71 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)

[ ... ]

> +struct etr_buf_node *tmc_etr_switch_buf_node(struct tmc_drvdata *drvdata)
> +{
> +	struct etr_buf_node *nd, *curr_node = NULL, *picked_node = NULL;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +	if (coresight_get_mode(drvdata->csdev) != CS_MODE_SYSFS)
> +		goto out;
> +
> +	list_for_each_entry(nd, &drvdata->etr_buf_list, link) {
> +		if (nd->sysfs_buf == drvdata->sysfs_buf)
> +			curr_node = nd;
> +		else if (!picked_node && nd->is_free)
> +			picked_node = nd;
> +	}
> +
> +	if (!curr_node || !picked_node) {
> +		curr_node = NULL;
> +		goto out;
> +	}
> +
> +	__tmc_etr_disable_hw(drvdata);
> +
> +	/* The switched out buffer is free again once it has been read */
> +	curr_node->is_free = true;
> +	curr_node->pos = 0;
> +	curr_node->reading = true;
> +	picked_node->is_free = false;

[Severity: High]
Can tmc_etr_switch_buf_node() re-select a buffer node while it is still
being read by userspace?

When curr_node is switched out for reading, curr_node->is_free is
immediately set to true alongside curr_node->reading = true, while the
selection loop above only checks nd->is_free and does not check
!nd->reading:

	list_for_each_entry(nd, &drvdata->etr_buf_list, link) {
		if (nd->sysfs_buf == drvdata->sysfs_buf)
			curr_node = nd;
		else if (!picked_node && nd->is_free)
			picked_node = nd;
	}

In tmc_create_etr_buf_list(), all nodes other than the active
drvdata->sysfs_buf start with is_free = true, and in the caller added later
in the series (tmc_byte_cntr_get_data() in coresight-ctcu-byte-cntr.c),
finishing a buffer read only clears byte_cntr_data->buf_node->reading =
false without updating is_free.

Because every node other than the active drvdata->sysfs_buf always has
is_free == true even while reading == true, the documented NULL return when
there is no free buffer cannot be reached. If tmc_etr_switch_buf_node() is
called again before the previous buffer has been fully read (for instance,
via concurrent read() calls on the same open descriptor since tmc_open()
uses nonseekable_open(), or when num_nodes > 2 and an earlier node in
etr_buf_list is still being read while a later node is idle), won't it pick
a node with reading == true and restart ETR DMA into that buffer while it
is being read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-enable-byte-cntr-for-ctcu-v21-0-231e069b7853@oss.qualcomm.com?part=3

Re: [PATCH v21 8/8] arm64: dts: qcom: lemans: add interrupts to CTCU device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Medium] coresight: tmc: negative error codes converted to EOF in tmc_read()
- [Medium] coresight: tmc-etr: unhandled errors in tmc_read_prepare_etr()
- [Medium] coresight: tmc-etr: buffer mismatch and data loss in tmc_create_etr_buf_list()
- [Medium] coresight: ctcu: active state not checked in irq_enabled_store()
- [Medium] coresight: tmc-etr: unsynced buffer read in tmc_etr_get_sysfs_trace()
- [Medium] coresight: ctcu: active session not checked in ctcu_byte_cntr_start()
--

commit 2fcbfd8bf479b22e68a324c313723f1a742b7f24
Author: Jie Gan <jie.gan@oss.qualcomm.com>

arm64: dts: qcom: lemans: add interrupts to CTCU device

This series enables the CTCU byte-cntr function for TMC ETR devices,
allowing userspace to read trace data from switched ETR buffers while
tracing continues, and wires up the CTCU interrupts on Lemans.

> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
> new file mode 100644
> index 0000000000000..bff87e776a755
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
[ ... ]
> +/* Start the byte-cntr function when the path is enabled. */
> +void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path)
> +{
> +	struct coresight_device *sink = coresight_get_sink(path);
> +	struct tmc_drvdata *etr_drvdata;
> +	struct ctcu_byte_cntr *byte_cntr_data;
> +
> +	byte_cntr_data = ctcu_get_byte_cntr(csdev, sink);
> +	if (!byte_cntr_data)
> +		return;
> +
> +	etr_drvdata = dev_get_drvdata(sink->dev.parent);
> +	guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock);
> +	/*
> +	 * Don't start byte-cntr function when irq_enabled is not set, or when
> +	 * the CTCU device is being removed.
> +	 */
> +	if (!byte_cntr_data->irq_enabled || byte_cntr_data->removing)
> +		return;
> +
> +	/*
> +	 * A new trace starts with an empty etr_buf. Don't reset the count if
> +	 * the byte-cntr is already started by another path of the same sink.
> +	 */
> +	if (!byte_cntr_data->enable)
> +		atomic_set(&byte_cntr_data->irq_cnt, 0);
> +
> +	/*
> +	 * Configure the byte-cntr register to enable IRQ while the path is
> +	 * enabled, the CTCU is powered then. The configured size is 5% of the
> +	 * buffer_size.
> +	 */
> +	ctcu_cfg_byte_cntr_reg(byte_cntr_data->ctcu_drvdata,
> +			       etr_drvdata->size / MAX_IRQ_CNT,
> +			       byte_cntr_data->irq_ctrl_offset);
> +	byte_cntr_data->enable = true;
> +}

[Severity: Medium]
What happens in ctcu_byte_cntr_start() if a previous byte-cntr read session
still has the ETR device open to drain the final stopped buffer when a new
sysfs trace path is enabled?

In coresight_enable_path_from_to(), helpers are enabled before the sink:

  coresight_enable_path_from_to()
    coresight_enable_helpers()
      ctcu_enable()
        ctcu_byte_cntr_start()
    coresight_enable_sink()
      tmc_enable_etr_sink_sysfs()
        tmc_etr_get_sysfs_buffer() -> returns -EBUSY

Because ctcu_byte_cntr_start() does not check byte_cntr_data->reading, it
sets byte_cntr_data->enable = true, resets byte_cntr_data->irq_cnt to 0,
and reprograms the IRQCTRL register before tmc_etr_get_sysfs_buffer()
rejects the new path with -EBUSY (potentially after sleeping in
tmc_etr_setup_sysfs_buf()). During that window, a read by the existing
reader in tmc_byte_cntr_get_data() will see byte_cntr_data->enable == true
and block in wait_event_interruptible_timeout() instead of returning 0 to
drain the final buffer.

Also, when a second path is enabled on an already-active ETR sink where
byte_cntr_data->enable is already true, ctcu_byte_cntr_start()
unconditionally reprograms the IRQCTRL register using etr_drvdata->size /
MAX_IRQ_CNT even if buffer_size was modified in sysfs after the active
sysfs_buf was allocated. Should ctcu_byte_cntr_start() bail out when
byte_cntr_data->reading or byte_cntr_data->enable is already set?

> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> index 9043cad42f01e..cce4e4d76da0d 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
[ ... ]
> +static ssize_t irq_enabled_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf,
> +				 size_t size)
> +{
> +	struct ctcu_byte_cntr_irq_attribute *irq_attr =
> +		container_of(attr, struct ctcu_byte_cntr_irq_attribute, attr);
> +	struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	u8 port = irq_attr->port;
> +	unsigned long val;
> +
> +	if (kstrtoul(buf, 0, &val))
> +		return -EINVAL;
> +
> +	guard(raw_spinlock_irqsave)(&drvdata->byte_cntr_data[port].spin_lock);
> +	if (drvdata->byte_cntr_data[port].reading)
> +		return -EBUSY;
> +	else if (drvdata->byte_cntr_data[port].irq_ctrl_offset)
> +		drvdata->byte_cntr_data[port].irq_enabled = !!val;
> +
> +	return size;
> +}

[Severity: Medium]
Can toggling irq_enabled in irq_enabled_store() while a trace path is
already active (before /dev/tmc_etr* is opened) leave software state out of
sync with the CTCU hardware?

irq_enabled_store() only checks drvdata->byte_cntr_data[port].reading, not
drvdata->byte_cntr_data[port].enable or whether a trace path is active on
that port.

If irq_enabled is changed from 1 to 0 after ctcu_byte_cntr_start() has
already enabled hardware interrupts and set byte_cntr_data->enable = true,
opening the ETR device causes tmc_read_prepare_byte_cntr() to return
-EINVAL, and tmc_read_prepare_etr() falls back to generic sysfs read
preparation (stopping the ETR hardware and setting drvdata->reading = true)
while CTCU interrupts and byte_cntr_data->enable remain active. Conversely,
if irq_enabled is changed from 0 to 1 after the path was enabled without
starting byte-cntr, tmc_read_prepare_byte_cntr() sees irq_enabled == true
and enable == false, returns -ENODEV, and tmc_read_prepare_etr() similarly
falls through to stopping the ETR hardware.

Should irq_enabled_store() also reject updates when
drvdata->byte_cntr_data[port].enable is true or when the port has active
trace IDs?

> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index bc5a133ada3e3..6ad09995ba87c 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
[ ... ]
> @@ -287,15 +273,7 @@ static int tmc_open(struct inode *inode, struct file *file)
>  static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len,
>  				   char **bufpp)
>  {
> -	switch (drvdata->config_type) {
> -	case TMC_CONFIG_TYPE_ETB:
> -	case TMC_CONFIG_TYPE_ETF:
> -		return tmc_etb_get_sysfs_trace(drvdata, pos, len, bufpp);
> -	case TMC_CONFIG_TYPE_ETR:
> -		return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
> -	}
> -
> -	return -EINVAL;
> +	return drvdata->sysfs_ops->get_trace_data(drvdata, pos, len, bufpp);
>  }
>  
>  static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
> @@ -315,7 +293,8 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
>  		return -EFAULT;
>  	}
>  
> -	*ppos += actual;
> +	if (!tmc_etr_update_buf_node_pos(drvdata, actual))
> +		*ppos += actual;
>  	dev_dbg(&drvdata->csdev->dev, "%zu bytes copied\n", actual);
>  
>  	return actual;

[Severity: Medium]
Doesn't tmc_read() in drivers/hwtracing/coresight/coresight-tmc-core.c turn
negative error codes from byte-cntr reads into 0 (EOF)?

In tmc_byte_cntr_get_data(), wait_event_interruptible_timeout() returns
-ERESTARTSYS when interrupted by a signal, and a failed buffer switch while
the sink is still in CS_MODE_SYSFS returns -ENOMEM. Then
tmc_etr_get_sysfs_trace() propagates those negative error codes:

  if (ret != -EINVAL && ret != 0)
          return ret;

However, tmc_read() does:

  actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp);
  if (actual <= 0)
          return 0;

As a result, -ERESTARTSYS and -ENOMEM are returned to userspace as 0,
causing trace readers to treat a signal interruption or buffer switch error
as end-of-file. Should tmc_read() return actual when actual <= 0?

> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 361a433e6f0c5..20d6bc60a446e 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
[ ... ]
> +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
> +				loff_t pos, size_t len, char **bufpp)
> +{
> +	ssize_t ret;
> +	const struct tmc_sysfs_ops *byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
> +
> +	if (byte_cntr_ops) {
> +		ret = byte_cntr_ops->get_trace_data(drvdata, pos, len, bufpp);
> +		/*
> +		 * -EINVAL means byte-cntr has no active read session. 0 means
> +		 * byte-cntr has no more data because the sink has stopped, the
> +		 * last etr_buf is read by the generic read. Anything else is
> +		 * the result of the byte-cntr session.
> +		 */
> +		if (ret != -EINVAL && ret != 0)
> +			return ret;
> +	}
> +
> +	return tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp);
>  }
>  
>  static struct etr_buf *
> @@ -1224,6 +1272,71 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
>  
>  }
>  
> +static void tmc_etr_reset_sysfs_buf(struct tmc_drvdata *drvdata)
> +{
> +	u32 sts;
> +
> +	CS_UNLOCK(drvdata->base);
> +	tmc_write_rrp(drvdata, drvdata->sysfs_buf->hwaddr);
> +	tmc_write_rwp(drvdata, drvdata->sysfs_buf->hwaddr);
> +	sts = readl_relaxed(drvdata->base + TMC_STS) & ~TMC_STS_FULL;
> +	writel_relaxed(sts, drvdata->base + TMC_STS);
> +	CS_LOCK(drvdata->base);
> +}
> +
> +/**
> + * tmc_etr_switch_buf_node - switch the ETR to a free buffer of the etr_buf_list.
> + * @drvdata:	drvdata of the TMC device.
> + *
> + * Stop the ETR, hand over the current buffer for reading, switch to a free
> + * buffer and restart the ETR. All of this is done with @drvdata->spinlock held,
> + * so it can't race with the sink disable path.
> + *
> + * Return: the node of the switched out buffer, or NULL if the sink isn't in
> + * sysfs mode or there is no free buffer. The ETR is left untouched in both
> + * cases.
> + */
> +struct etr_buf_node *tmc_etr_switch_buf_node(struct tmc_drvdata *drvdata)
> +{
> +	struct etr_buf_node *nd, *curr_node = NULL, *picked_node = NULL;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +	if (coresight_get_mode(drvdata->csdev) != CS_MODE_SYSFS)
> +		goto out;
> +
> +	list_for_each_entry(nd, &drvdata->etr_buf_list, link) {
> +		if (nd->sysfs_buf == drvdata->sysfs_buf)
> +			curr_node = nd;
> +		else if (!picked_node && nd->is_free)
> +			picked_node = nd;
> +	}
> +
> +	if (!curr_node || !picked_node) {
> +		curr_node = NULL;
> +		goto out;
> +	}
> +
> +	__tmc_etr_disable_hw(drvdata);
> +
> +	/* The switched out buffer is free again once it has been read */
> +	curr_node->is_free = true;
> +	curr_node->pos = 0;
> +	curr_node->reading = true;
> +	picked_node->is_free = false;
> +	drvdata->sysfs_buf = picked_node->sysfs_buf;
> +	drvdata->etr_buf = picked_node->sysfs_buf;
> +
> +	tmc_etr_reset_sysfs_buf(drvdata);
> +	if (__tmc_etr_enable_hw(drvdata))
> +		dev_err(&drvdata->csdev->dev, "Failed to restart the ETR\n");
> +out:
> +	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +
> +	return curr_node;
> +}

[Severity: Medium]
Could tmc_etr_get_sysfs_trace() end up reading an unsynced, live DMA buffer
if ctcu_platform_remove() runs while the ETR sink is still active in
CS_MODE_SYSFS?

In ctcu_platform_remove(), byte_cntr_data->enable is set to false and
byte_cntr_data->wq is woken while the ETR sink can still be running in
CS_MODE_SYSFS. In tmc_byte_cntr_get_data(), seeing !byte_cntr_data->enable
causes it to return 0:

  ctcu_platform_remove()
    byte_cntr_data->enable = false;
    wake_up_all(&byte_cntr_data->wq);

  tmc_etr_get_sysfs_trace()
    tmc_byte_cntr_get_data() -> returns 0
    tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp)

tmc_etr_get_sysfs_trace() treats 0 as meaning the sink has stopped and
immediately calls tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, ...). However,
if at least one buffer switch has already happened in
tmc_etr_switch_buf_node(), picked_node->sysfs_buf->len was never reset to 0
when switching to it, so drvdata->sysfs_buf->len still holds the non-zero
length from its previous fill while the ETR hardware is actively writing to
that DMA buffer without having been stopped or synced.

Should tmc_etr_switch_buf_node() reset picked_node->sysfs_buf->len (and
offset) to 0 before restarting the ETR, and should tmc_byte_cntr_get_data()
return an error (such as -ENODEV) instead of 0 when byte_cntr_data->removing
is set or when the sink is still in CS_MODE_SYSFS?

[ ... ]
> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
> +{
> +	struct etr_buf_node *nd, *next;
> +	unsigned long flags;
> +
> +	list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) {
> +		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +		if (nd->sysfs_buf == drvdata->sysfs_buf) {
> +			if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
> +				/*
> +				 * The sink is still enabled in sysfs mode. Keep
> +				 * the live buffer owned by drvdata->sysfs_buf
> +				 * and only drop the list's reference to it.
> +				 */
> +				nd->sysfs_buf = NULL;
> +			else
> +				/* Free the buffer below through nd->sysfs_buf */
> +				drvdata->sysfs_buf = NULL;
> +		}
> +		list_del(&nd->link);
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +
> +		/* Free the buffer (NULL is ignored) and the node out of the lock */
> +		tmc_etr_free_sysfs_buf(nd->sysfs_buf);
> +		kfree(nd);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
> +
> +/**
> + * tmc_create_etr_buf_list - create a list to manage the etr_buf_node.
> + * @drvdata:	driver data of the TMC device.
> + * @num_nodes:	number of nodes want to create with the list.
> + *
> + * Locking: must not be called with @drvdata->spinlock held. The spinlock is
> + * taken internally to serialise the @drvdata->sysfs_buf accesses and every
> + * update of @drvdata->etr_buf_list against the ETR sink enable/disable paths
> + * and the generic sysfs read. There must be only one owner of the list at a
> + * time. Must be called from process context: buffers and nodes are allocated
> + * with the lock released.
> + *
> + * Return: 0 upon success, else the error number: -EBUSY if a generic sysfs
> + * read is in progress, -EINVAL if the sink is not enabled in sysfs mode or
> + * the ETR is in the reserved buffer mode, or the error of the allocation.
> + */
> +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes)
> +{
> +	struct etr_buf_node *new_node;
> +	struct etr_buf *sysfs_buf;
> +	unsigned long flags;
> +	int i = 0, ret = 0;
> +
> +	/* We don't need a list if there is only one node */
> +	if (num_nodes < 2)
> +		return -EINVAL;
> +
> +	/*
> +	 * All the etr_buf of the reserved mode are mapped to the same reserved
> +	 * region, so they can't be used as separate buffers of the list.
> +	 */
> +	if (drvdata->etr_mode == ETR_MODE_RESRV)
> +		return -EINVAL;
> +
> +	/*
> +	 * We expect that sysfs_buf in drvdata has already been allocated.
> +	 * Wrap the live sysfs_buf into the first node so the captured trace
> +	 * data is preserved. Take the sysfs_buf and add the node under the
> +	 * lock, so that the ETR sink enable path sees a non-empty list and
> +	 * doesn't replace the buffer.
> +	 */
> +	new_node = kzalloc_obj(*new_node, GFP_KERNEL);
> +	if (!new_node)
> +		return -ENOMEM;
> +
> +	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +	/* The sysfs_buf is in use by a sysfs read session */
> +	if (drvdata->reading) {
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +		kfree(new_node);
> +		return -EBUSY;
> +	}
> +
> +	/*
> +	 * The sink enable path reuses or frees the sysfs_buf of a sink which
> +	 * isn't enabled, so only wrap it while the sink is enabled in sysfs mode.
> +	 */
> +	if (coresight_get_mode(drvdata->csdev) != CS_MODE_SYSFS) {
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +		kfree(new_node);
> +		return -EINVAL;
> +	}
> +
> +	if (drvdata->sysfs_buf) {
> +		new_node->sysfs_buf = drvdata->sysfs_buf;
> +		new_node->is_free = false;
> +		list_add(&new_node->link, &drvdata->etr_buf_list);
> +		new_node = NULL;
> +		i++;
> +	}
> +	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +	/* Free the node if there is no live sysfs_buf to wrap */
> +	kfree(new_node);
> +
> +	while (i < num_nodes) {
> +		new_node = kzalloc_obj(*new_node, GFP_KERNEL);
> +		if (!new_node) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +
> +		/* Allocate the buffer with the lock released */
> +		sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
> +		if (IS_ERR(sysfs_buf)) {
> +			kfree(new_node);
> +			ret = PTR_ERR(sysfs_buf);
> +			break;
> +		}
> +
> +		new_node->sysfs_buf = sysfs_buf;
> +		/*
> +		 * The drvdata->sysfs_buf write and the list update need the
> +		 * spinlock, to serialise against the ETR sink enable/disable
> +		 * paths.
> +		 */
> +		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +		/* We don't have an available sysfs_buf in drvdata, set one up */
> +		if (!drvdata->sysfs_buf) {
> +			drvdata->sysfs_buf = sysfs_buf;
> +			new_node->is_free = false;
> +		} else {
> +			new_node->is_free = true;
> +		}
> +		list_add_tail(&new_node->link, &drvdata->etr_buf_list);
> +		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +		i++;
> +	}
> +
> +	/* Clean the list if there is an error */
> +	if (ret)
> +		tmc_clean_etr_buf_list(drvdata);
> +
> +	return ret;
> +}

[Severity: Medium]
Can tmc_create_etr_buf_list() free the pre-existing drvdata->sysfs_buf on
error, or allocate mismatched buffers if sysfs settings changed after the
sink was enabled?

First, tmc_create_etr_buf_list() wraps the live drvdata->sysfs_buf into the
first node and then drops drvdata->spinlock while allocating the second
buffer. If a concurrent tmc_disable_etr_sink() stops the sink during that
window (changing the mode to CS_MODE_DISABLED) and the second allocation
then fails, the error path calls tmc_clean_etr_buf_list(). Because
coresight_get_mode(drvdata->csdev) is no longer CS_MODE_SYSFS,
tmc_clean_etr_buf_list() sets drvdata->sysfs_buf = NULL and frees the
pre-existing drvdata->sysfs_buf, losing the captured trace data on a failed
open().

Second, tmc_create_etr_buf_list() checks drvdata->etr_mode == ETR_MODE_RESRV
without holding drvdata->spinlock and allocates subsequent buffers using
drvdata->size rather than the wrapped drvdata->sysfs_buf->mode and
drvdata->sysfs_buf->size. If buf_mode_preferred or buffer_size was modified
in sysfs after the sink was enabled, tmc_create_etr_buf_list() can wrap an
existing ETR_MODE_RESRV buffer or allocate a second buffer whose size does
not match the first buffer and the programmed CTCU IRQ threshold.

[ ... ]
>  int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>  {
>  	int ret = 0;
>  	unsigned long flags;
> +	const struct tmc_sysfs_ops *byte_cntr_ops;
>  
>  	/* config types are set a boot time and never change */
>  	if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
>  		return -EINVAL;
>  
> +	byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
> +	if (byte_cntr_ops) {
> +		ret = byte_cntr_ops->read_prepare(drvdata);
> +		if (!ret || ret == -EBUSY)
> +			return ret;
> +
> +		ret = 0;
> +	}

[Severity: Medium]
In tmc_read_prepare_etr(), what happens when byte_cntr_ops->read_prepare()
fails with an error other than -EBUSY?

When byte-cntr is active on the trace path, tmc_read_prepare_byte_cntr() can
return -ENOMEM (if tmc_create_etr_buf_list() fails to allocate a node or
DMA buffer), -EINVAL (if tmc_create_etr_buf_list() rejects ETR_MODE_RESRV),
or -ENODEV (if try_module_get() fails or ctcu_platform_remove() cleared
byte_cntr_data->enable).

Because tmc_read_prepare_etr() only returns when !ret || ret == -EBUSY and
otherwise resets ret = 0, any of those failures falls through to the generic
ETR read preparation path. When the sink is active in CS_MODE_SYSFS, that
fallback calls __tmc_etr_disable_hw(drvdata), stopping the ETR hardware in
the middle of an active byte-cntr trace session, and sets drvdata->reading =
true. If the trace path is later disabled while the file remains open,
tmc_disable_etr_sink() returns -EBUSY (leaving the sink mode as
CS_MODE_SYSFS), which causes ctcu_byte_cntr_stop() to return early without
clearing byte_cntr_data->enable or disabling the CTCU IRQCTRL register
before coresight_release_path() drops the CTCU runtime PM reference.

Should tmc_read_prepare_byte_cntr() use a distinct return code (or a boolean
out-parameter) when byte-cntr is not applicable, so that
tmc_read_prepare_etr() propagates real errors instead of falling back to
generic sysfs read preparation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-enable-byte-cntr-for-ctcu-v21-0-231e069b7853@oss.qualcomm.com?part=8

Re: [PATCH v21 7/8] coresight: ctcu: enable byte-cntr for TMC ETR devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] coresight: ctcu: race in tmc_read_unprepare_byte_cntr() during close
- [High] coresight: ctcu: zero-length read in tmc_byte_cntr_get_data() drops buffers and loops
- [High] coresight: ctcu: unbind in ctcu_platform_remove() reads live DMA buffer
- [High] coresight: ctcu: unsynchronized concurrent read() in tmc_byte_cntr_get_data()
- [High] coresight: ctcu: UAF race between byte_cntr_sysfs_ops and CTCU teardown
- [High] coresight: ctcu: unsynchronized state transitions in byte-cntr lifecycle
- [High] coresight: tmc: broken fallback and race in tmc_read_prepare_etr()
- [Medium] coresight: ctcu: ctcu_byte_cntr_start() corrupts active read or trace state
- [Medium] coresight: ctcu: unhandled devm_add_action_or_reset() failure in ctcu_byte_cntr_init()
- [Medium] coresight: tmc: tmc_read() converts negative errors into EOF
--

commit 21dfce27a3a23b8dd103a86f6d7455d0e9197bff
Author: Jie Gan <jie.gan@oss.qualcomm.com>

coresight: ctcu: enable byte-cntr for TMC ETR devices

This commit enables the CTCU byte-cntr functionality to transfer trace data
from TMC ETR buffers to userspace via interrupt-driven buffer switching in
sysfs mode.

> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
> new file mode 100644
> index 0000000000000..bff87e776a755
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
[ ... ]
> +static struct ctcu_byte_cntr *ctcu_get_byte_cntr(struct coresight_device *ctcu,
> +						 struct coresight_device *etr)
> +{
> +	struct ctcu_byte_cntr *byte_cntr_data;
> +	struct ctcu_drvdata *drvdata;
> +	int port;
> +
> +	/* The ETR may not be connected to a CTCU device */
> +	if (!ctcu)
> +		return NULL;
> +
> +	drvdata = dev_get_drvdata(ctcu->dev.parent);
> +	port = coresight_get_in_port(etr, ctcu);
> +	if (port < 0 || port >= ETR_MAX_NUM)
> +		return NULL;
> +
> +	byte_cntr_data = &drvdata->byte_cntr_data[port];
> +	/* The port is not initialized by ctcu_byte_cntr_init() */
> +	if (!byte_cntr_data->ctcu_drvdata)
> +		return NULL;
> +
> +	return byte_cntr_data;
> +}

[Severity: High]
As reported by Sashiko on v19 [1], can concurrent /dev/tmc_etr* file
operations race with CTCU device removal or module unload and trigger a
use-after-free?

Because CONFIG_CORESIGHT_LINK_AND_SINK_TMC and CONFIG_CORESIGHT_CTCU are
separate modules, opening /dev/tmc_etr* pins coresight-tmc rather than
coresight-ctcu. In coresight-tmc-etr.c, tmc_read_prepare_etr(),
tmc_etr_get_sysfs_trace(), and tmc_read_unprepare_etr() load
READ_ONCE(byte_cntr_sysfs_ops) and invoke its callbacks without holding a
lock, coresight_mutex, or a module reference prior to the call:

1. In tmc_read_prepare_byte_cntr(), try_module_get(THIS_MODULE) and
reinit_completion(&byte_cntr_data->session_done) are only executed after
tmc_read_prepare_etr() has dereferenced byte_cntr_ops->read_prepare and
after tmc_read_prepare_byte_cntr() has called tmc_etr_get_ctcu_device(),
ctcu_get_byte_cntr(), and raw_spin_lock_irqsave(&byte_cntr_data->spin_lock,
flags). Similarly, in tmc_read_unprepare_byte_cntr(),
module_put(THIS_MODULE) and the function return execute after
complete_all(&byte_cntr_data->session_done).

2. For normal non-byte-cntr ETR sysfs read sessions (for instance when
reading /dev/tmc_etr* after stopping the trace path or when irq_enabled is
0), tmc_read_prepare_byte_cntr() returns -EINVAL without pinning
THIS_MODULE or reinitializing session_done, yet every subsequent read() via
tmc_etr_get_sysfs_trace() -> tmc_byte_cntr_get_data() and close() via
tmc_read_unprepare_etr() -> tmc_read_unprepare_byte_cntr() still invokes
byte_cntr_ops and calls tmc_etr_get_ctcu_device() and ctcu_get_byte_cntr()
while session_done is already completed and coresight-ctcu can have a
module refcount of 0.

3. When ctcu_platform_remove() runs concurrently,
wait_for_completion(&byte_cntr_data->session_done) returns immediately if
no byte-cntr session is active. ctcu_platform_remove() then calls
ctcu_remove() -> coresight_unregister(drvdata->csdev) before
devres_release_all() runs ctcu_byte_cntr_put_ops() to clear
byte_cntr_sysfs_ops (and on multi-CTCU systems, byte_cntr_sysfs_ops remains
set when one CTCU device is removed). Inside coresight_unregister(),
coresight_release_platform_data() frees ctcu->pdata->in_conns and
ctcu->pdata, and device_unregister() frees ctcu; devres_release_all() then
frees drvdata and clears dev->driver_data. Concurrent calls to
tmc_etr_get_ctcu_device() and ctcu_get_byte_cntr() can therefore
dereference freed ctcu, freed ctcu->pdata->in_conns in
coresight_get_in_port(), NULL or freed drvdata, or unmapped byte_cntr_ops.

> +
> +/*
> + * ctcu_byte_cntr_get_data() - reads data from the deactivated and filled buffer.
> + * The byte-cntr reading work reads data from the deactivated and filled buffer.
> + * The read operation waits for a buffer to become available, either filled or
> + * upon timeout, and then reads trace data from the synced buffer.
> + */
> +static ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *etr_drvdata, loff_t pos,
> +				      size_t len, char **bufpp)
> +{
> +	struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata);
> +	struct device *dev = &etr_drvdata->csdev->dev;
> +	struct ctcu_byte_cntr *byte_cntr_data;
> +	struct etr_buf *sysfs_buf;
> +	atomic_t *irq_cnt;
> +	ssize_t actual;
> +	int ret;
> +
> +	/* No byte-cntr read session, let the caller use the generic read */
> +	byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev);
> +	if (!byte_cntr_data || !byte_cntr_data->irq_enabled ||
> +	    !byte_cntr_data->reading)
> +		return -EINVAL;
> +
> +	irq_cnt = &byte_cntr_data->irq_cnt;
> +
> +wait_buffer:
> +	if (!byte_cntr_data->buf_node) {
> +		ret = wait_event_interruptible_timeout(byte_cntr_data->wq,
> +				(atomic_read(irq_cnt) >= MAX_IRQ_CNT - 1) ||
> +				!byte_cntr_data->enable,
> +				BYTE_CNTR_TIMEOUT);
> +		if (ret < 0)
> +			return ret;
> +		/*
> +		 * The current etr_buf is almost full or timeout is triggered,
> +		 * so switch the buffer and mark the switched buffer as reading.
> +		 */
> +		if (byte_cntr_data->enable) {
> +			byte_cntr_data->buf_node =
> +				tmc_etr_switch_buf_node(etr_drvdata);

[Severity: High]
As reported by Sashiko on v19 [2], what prevents concurrent read() calls on
the same open /dev/tmc_etr* file descriptor from racing on
byte_cntr_data->buf_node and nd->pos?

Because /dev/tmc_etr* is a character device opened with nonseekable_open(),
FMODE_ATOMIC_POS is not set and vfs_read() does not serialize concurrent
readers sharing an open file description via file->f_pos_lock. While
tmc_read_prepare_byte_cntr() prevents multiple concurrent open() calls via
byte_cntr_data->reading, multiple threads or processes sharing a single open
file descriptor can enter tmc_read() -> tmc_etr_get_sysfs_trace() ->
tmc_byte_cntr_get_data() and tmc_etr_update_buf_node_pos() simultaneously
without holding any lock:

1. When one thread reaches the end of the current buffer (actual == 0 from
tmc_etr_read_sysfs_buf()), it sets byte_cntr_data->buf_node->reading =
false and byte_cntr_data->buf_node = NULL while a second thread that
already passed the !byte_cntr_data->buf_node check is about to dereference
byte_cntr_data->buf_node->pos and byte_cntr_data->buf_node->sysfs_buf,
causing a NULL pointer dereference.

2. If two threads both see !byte_cntr_data->buf_node and wake from
wait_event_interruptible_timeout(), both can call
tmc_etr_switch_buf_node(). Because tmc_etr_switch_buf_node() sets
curr_node->is_free = true at the same time as curr_node->reading = true,
the second call selects the first thread's curr_node as picked_node and
restarts ETR hardware DMA into that buffer while the first thread is
copying it to userspace.

3. In tmc_read(), tmc_etr_update_buf_node_pos() iterates
drvdata->etr_buf_list and updates nd->pos after copy_to_user() without
synchronization.

[ ... ]
> +	pos = byte_cntr_data->buf_node->pos;
> +	sysfs_buf = byte_cntr_data->buf_node->sysfs_buf;
> +	actual = tmc_etr_read_sysfs_buf(sysfs_buf, pos, len, bufpp);
> +	if (actual <= 0) {
> +		/* Reset buf_node upon reading is finished or failed */
> +		byte_cntr_data->buf_node->reading = false;
> +		byte_cntr_data->buf_node = NULL;
> +
> +		/*
> +		 * Nothing in the buffer, waiting for the next buffer
> +		 * to be filled.
> +		 */
> +		if (actual == 0)
> +			goto wait_buffer;
> +	}

[Severity: High]
What happens here if userspace issues a zero-length read(fd, buf, 0) on
/dev/tmc_etr* during a byte-cntr read session?

vfs_read() invokes file->f_op->read() even when count is 0, passing len ==
0 down to tmc_read() -> tmc_get_sysfs_trace() -> tmc_etr_get_sysfs_trace()
-> tmc_byte_cntr_get_data(). In tmc_etr_read_sysfs_buf(), actual is
initialized to len (0), so the check:

	if (actual <= 0)
		return actual;

immediately returns 0 regardless of how much unread trace data remains in
sysfs_buf. tmc_byte_cntr_get_data() treats actual == 0 as buffer
exhaustion, clears byte_cntr_data->buf_node->reading = false and
byte_cntr_data->buf_node = NULL (discarding all unread data in that
switched-out buffer), and jumps back to wait_buffer, where it loops
waiting for and discarding subsequent buffers until tracing stops or a
signal arrives.

[ ... ]
> +static int tmc_read_unprepare_byte_cntr(struct tmc_drvdata *etr_drvdata)
> +{
> +	struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata);
> +	struct ctcu_byte_cntr *byte_cntr_data;
> +
> +	/*
> +	 * Do the unprepare operation only when the byte_cntr_data->reading
> +	 * is truly set
> +	 */
> +	byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev);
> +	if (!byte_cntr_data || !byte_cntr_data->irq_enabled ||
> +	    !byte_cntr_data->reading)
> +		return -EINVAL;
> +
> +	tmc_clean_etr_buf_list(etr_drvdata);
> +
> +	/* disable_irq_wake() may sleep on slow-bus irqchips, call it unlocked */
> +	disable_irq_wake(byte_cntr_data->irq);
> +
> +	/*
> +	 * The byte-cntr register is not accessed here: the trace path, which
> +	 * keeps the CTCU powered, may have been released already. It is reset
> +	 * when the path is disabled.
> +	 */
> +	scoped_guard(raw_spinlock_irqsave, &byte_cntr_data->spin_lock) {
> +		byte_cntr_data->buf_node = NULL;
> +		byte_cntr_data->reading = false;

[Severity: High]
Can tmc_read_unprepare_byte_cntr() race with a concurrent open() and close()
when the trace path is disabled during teardown?

In tmc_read_unprepare_byte_cntr(), byte_cntr_data->reading is checked
without holding byte_cntr_data->spin_lock and remains true while
tmc_clean_etr_buf_list(etr_drvdata) and the sleeping
disable_irq_wake(byte_cntr_data->irq) run.

If the ETR sink is in CS_MODE_SYSFS when tmc_clean_etr_buf_list() runs,
drvdata->etr_buf_list is emptied while drvdata->sysfs_buf is kept. If the
trace path is then disabled before byte_cntr_data->reading is cleared under
byte_cntr_data->spin_lock, coresight_get_mode(etr_drvdata->csdev) becomes
CS_MODE_DISABLED with an empty drvdata->etr_buf_list, drvdata->reading ==
false, drvdata->sysfs_buf != NULL, and byte_cntr_data->reading == true.

A second process opening /dev/tmc_etr* will call
tmc_read_prepare_byte_cntr(), which sees coresight_get_mode() !=
CS_MODE_SYSFS and returns -EINVAL before checking byte_cntr_data->reading.
tmc_read_prepare_etr() then falls through to generic ETR read preparation
and sets drvdata->reading = true. If that second process closes its file
descriptor before the first process clears byte_cntr_data->reading, the
second close() will also see byte_cntr_data->reading == true in
tmc_read_unprepare_byte_cntr(), calling disable_irq_wake() and
module_put(THIS_MODULE) a second time and returning 0 without clearing
drvdata->reading or freeing drvdata->sysfs_buf.

[ ... ]
> +/* Start the byte-cntr function when the path is enabled. */
> +void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path)
> +{
> +	struct coresight_device *sink = coresight_get_sink(path);
> +	struct tmc_drvdata *etr_drvdata;
> +	struct ctcu_byte_cntr *byte_cntr_data;
> +
> +	byte_cntr_data = ctcu_get_byte_cntr(csdev, sink);
> +	if (!byte_cntr_data)
> +		return;
> +
> +	etr_drvdata = dev_get_drvdata(sink->dev.parent);
> +	guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock);
> +	/*
> +	 * Don't start byte-cntr function when irq_enabled is not set, or when
> +	 * the CTCU device is being removed.
> +	 */
> +	if (!byte_cntr_data->irq_enabled || byte_cntr_data->removing)
> +		return;
> +
> +	/*
> +	 * A new trace starts with an empty etr_buf. Don't reset the count if
> +	 * the byte-cntr is already started by another path of the same sink.
> +	 */
> +	if (!byte_cntr_data->enable)
> +		atomic_set(&byte_cntr_data->irq_cnt, 0);
> +
> +	/*
> +	 * Configure the byte-cntr register to enable IRQ while the path is
> +	 * enabled, the CTCU is powered then. The configured size is 5% of the
> +	 * buffer_size.
> +	 */
> +	ctcu_cfg_byte_cntr_reg(byte_cntr_data->ctcu_drvdata,
> +			       etr_drvdata->size / MAX_IRQ_CNT,
> +			       byte_cntr_data->irq_ctrl_offset);
> +	byte_cntr_data->enable = true;
> +}

[Severity: Medium]
Can ctcu_byte_cntr_start() corrupt an active read session or an already
running trace session here?

1. In coresight_enable_path_from_to(), helpers on the sink (ctcu_enable()
-> ctcu_byte_cntr_start()) are enabled before coresight_enable_sink() calls
tmc_enable_etr_sink_sysfs(). When a trace stops while /dev/tmc_etr* is still
open in byte-cntr mode, ctcu_byte_cntr_stop() sets byte_cntr_data->enable =
false while byte_cntr_data->reading remains true until the reader drains
the remaining data and closes the file. If another sysfs trace enable is
attempted on the same sink before the file is closed,
ctcu_byte_cntr_start() does not check byte_cntr_data->reading, so it resets
byte_cntr_data->irq_cnt to 0, reprograms IRQCTRL, and sets
byte_cntr_data->enable = true before tmc_enable_etr_sink_sysfs() rejects
the enable with -EBUSY. During that window, a concurrent read() in
tmc_byte_cntr_get_data() sees byte_cntr_data->enable == true and blocks in
wait_event_interruptible_timeout() instead of returning 0 to drain the
final buffer.

2. When a second source path is enabled on an already-running ETR sink
where byte_cntr_data->enable is already true, ctcu_byte_cntr_start() avoids
resetting irq_cnt, but still unconditionally reprograms the IRQCTRL
register using etr_drvdata->size / MAX_IRQ_CNT. Because buffer_size_store()
allows changing etr_drvdata->size while the ETR is running while
tmc_enable_etr_sink_sysfs() continues using the existing sysfs_buf of the
original size, reprogramming IRQCTRL when byte_cntr_data->enable is already
true can overwrite the threshold with a mismatched value (or 0 if
etr_drvdata->size < MAX_IRQ_CNT * 8, disabling interrupts while
byte_cntr_data->enable is true).

[ ... ]
> +void ctcu_byte_cntr_init(struct device *dev, struct ctcu_drvdata *drvdata, int etr_num)
> +{
[ ... ]
> +	scoped_guard(mutex, &byte_cntr_ops_lock) {
> +		tmc_etr_set_byte_cntr_sysfs_ops(&byte_cntr_sysfs_ops);
> +		byte_cntr_ops_users++;
> +	}
> +
> +	/* Drop the ops user when the CTCU device is removed or probe fails */
> +	if (devm_add_action_or_reset(dev, ctcu_byte_cntr_put_ops, NULL))
> +		dev_err(dev, "Failed to add the byte-cntr devm action\n");
> +}

[Severity: Medium]
As reported by Sashiko on v20 [3], if devm_add_action_or_reset() fails in
ctcu_byte_cntr_init(), ctcu_byte_cntr_put_ops() is immediately called to
decrement byte_cntr_ops_users and reset byte_cntr_sysfs_ops to NULL, but
ctcu_byte_cntr_init() returns void without propagating the error to
ctcu_probe() or clearing byte_cntr_data[i].irq.

Doesn't this allow ctcu_probe() to succeed with the IRQs still registered
and the irq_enabled sysfs attributes exposed while byte_cntr_ops_users no
longer accounts for this device?

> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> index 739105acb0a13..cce4e4d76da0d 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
[ ... ]
> +static ssize_t irq_enabled_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf,
> +				 size_t size)
> +{
> +	struct ctcu_byte_cntr_irq_attribute *irq_attr =
> +		container_of(attr, struct ctcu_byte_cntr_irq_attribute, attr);
> +	struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	u8 port = irq_attr->port;
> +	unsigned long val;
> +
> +	if (kstrtoul(buf, 0, &val))
> +		return -EINVAL;
> +
> +	guard(raw_spinlock_irqsave)(&drvdata->byte_cntr_data[port].spin_lock);
> +	if (drvdata->byte_cntr_data[port].reading)
> +		return -EBUSY;
> +	else if (drvdata->byte_cntr_data[port].irq_ctrl_offset)
> +		drvdata->byte_cntr_data[port].irq_enabled = !!val;
> +
> +	return size;
> +}

[Severity: High]
Can modifying irq_enabled while a trace path or generic ETR read session is
active desynchronize byte_cntr_data and ETR state?

1. irq_enabled_store() only checks drvdata->byte_cntr_data[port].reading,
not drvdata->byte_cntr_data[port].enable. If a sysfs trace path is enabled
with irq_enabled == 1, ctcu_byte_cntr_start() programs the BYTECNTRVAL
register and sets byte_cntr_data->enable = true while
byte_cntr_data->reading remains false until /dev/tmc_etr* is opened. If
userspace writes 0 to irq_enabled before opening /dev/tmc_etr*,
tmc_read_prepare_byte_cntr() returns -EINVAL and tmc_read_prepare_etr()
falls back to generic ETR read (setting drvdata->reading = true while
byte_cntr_data->enable remains true and byte_cntr_data->reading is false).
If userspace then writes 1 to irq_enabled and a second process opens
/dev/tmc_etr*, tmc_read_prepare_byte_cntr() sets byte_cntr_data->reading =
true before tmc_create_etr_buf_list() fails with -EBUSY on
drvdata->reading. A concurrent close() of the first session during this
window will enter tmc_read_unprepare_byte_cntr() instead of generic
unprepare, calling disable_irq_wake() and module_put(THIS_MODULE) and
leaving drvdata->reading permanently stuck at true.

2. Similarly, if a generic sysfs read session is open (drvdata->reading ==
true) while byte_cntr_data->enable is true and the trace path is disabled,
tmc_disable_etr_sink() returns -EBUSY and leaves the sink mode as
CS_MODE_SYSFS. When ctcu_disable() -> ctcu_byte_cntr_stop() then runs, it
sees coresight_get_mode(sink) == CS_MODE_SYSFS and returns early without
clearing the hardware IRQCTRL register or setting byte_cntr_data->enable =
false, even though the CTCU helper is being disabled.

[ ... ]
> @@ -244,10 +353,52 @@ static int ctcu_platform_probe(struct platform_device *pdev)
>  static void ctcu_platform_remove(struct platform_device *pdev)
>  {
>  	struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev);
> +	struct ctcu_byte_cntr *byte_cntr_data;
> +	unsigned long flags;
> +	int i;
>  
>  	if (WARN_ON(!drvdata))
>  		return;
>  
> +	/*
> +	 * Signal all active byte-cntr readers to exit, then wait for them to
> +	 * finish before resetting the ops pointer and freeing driver data.
> +	 * Without this, a reader blocked in wait_event_interruptible_timeout()
> +	 * would access the freed ctcu_drvdata wait-queue head (use-after-free).
> +	 *
> +	 * The wait ends only when the userspace closes the tmc_etr file, so an
> +	 * unbind with the file left open blocks until then. Unloading the module
> +	 * is rejected with -EBUSY while a read session is open, so only a manual
> +	 * unbind can block here.
> +	 */
> +	for (i = 0; i < ETR_MAX_NUM; i++) {
> +		byte_cntr_data = &drvdata->byte_cntr_data[i];
> +		/* The port was not initialized by ctcu_byte_cntr_init() */
> +		if (!byte_cntr_data->irq)
> +			continue;
> +
> +		raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
> +		/*
> +		 * Set enable=false to signal teardown to racing readers, and
> +		 * removing=true to keep a new path from enabling byte-cntr again.
> +		 */
> +		byte_cntr_data->enable = false;
> +		byte_cntr_data->removing = true;
> +		raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
> +
> +		/* Wake up the blocked reader, then wait for the session to end */
> +		wake_up_all(&byte_cntr_data->wq);
> +		wait_for_completion(&byte_cntr_data->session_done);

[Severity: High]
When ctcu_platform_remove() clears byte_cntr_data->enable and wakes a
blocked reader while the ETR sink is still actively tracing in
CS_MODE_SYSFS, won't this cause an unsynced read of the live DMA buffer and
leave the hardware IRQCTRL register enabled?

1. When woken in tmc_byte_cntr_get_data(), the reader sees
!byte_cntr_data->enable and returns 0. In tmc_etr_get_sysfs_trace(), a
return value of 0 is treated as meaning the sink has stopped and synced its
final buffer, so it falls through to
tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp). If at least
one buffer switch has already occurred via tmc_etr_switch_buf_node(), the
currently active drvdata->sysfs_buf still holds a non-zero stale
sysfs_buf->len from its previous fill cycle, so tmc_etr_read_sysfs_buf()
reads directly from the live drvdata->sysfs_buf while the ETR hardware is
actively DMA-writing to it (and without drvdata->reading synchronization
against a concurrent tmc_disable_etr_sink() -> tmc_etr_sync_sysfs_buf()).

2. Setting byte_cntr_data->enable = false here without clearing the
hardware BYTECNTRVAL register via ctcu_cfg_byte_cntr_reg() also causes
ctcu_byte_cntr_stop() to skip clearing the register when the path is later
disabled because byte_cntr_data->enable is already false.

> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 4b40b692be4d8..6ad09995ba87c 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -293,7 +293,8 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len,
>  		return -EFAULT;
>  	}
>  
> -	*ppos += actual;
> +	if (!tmc_etr_update_buf_node_pos(drvdata, actual))
> +		*ppos += actual;
>  	dev_dbg(&drvdata->csdev->dev, "%zu bytes copied\n", actual);
>  
>  	return actual;

[Severity: Medium]
As reported by Sashiko on v19 [4], in tmc_read(), negative error codes
returned by tmc_get_sysfs_trace() -> tmc_etr_get_sysfs_trace() ->
tmc_byte_cntr_get_data() (such as -ERESTARTSYS when
wait_event_interruptible_timeout() is interrupted by a signal, or -ENOMEM
when tmc_etr_switch_buf_node() fails) are caught by:

	actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp);
	if (actual <= 0)
		return 0;

Doesn't this convert -ERESTARTSYS and -ENOMEM into 0 (EOF) and terminate
the trace read session prematurely instead of propagating the error to
userspace?

> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index bf4c1d8f7b2e0..20d6bc60a446e 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
[ ... ]
> @@ -2170,15 +2194,54 @@ int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes)
[ ... ]
>  int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>  {
>  	int ret = 0;
>  	unsigned long flags;
> +	const struct tmc_sysfs_ops *byte_cntr_ops;
>  
>  	/* config types are set a boot time and never change */
>  	if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
>  		return -EINVAL;
>  
> +	byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
> +	if (byte_cntr_ops) {
> +		ret = byte_cntr_ops->read_prepare(drvdata);
> +		if (!ret || ret == -EBUSY)
> +			return ret;
> +
> +		ret = 0;
> +	}

[Severity: High]
As reported by Sashiko on v19 [5], why does tmc_read_prepare_etr() ignore
errors other than -EBUSY (such as -ENOMEM or -ENODEV) from
byte_cntr_ops->read_prepare(drvdata) and fall through to the generic ETR
read path?

When byte-cntr is active in CS_MODE_SYSFS and tmc_read_prepare_byte_cntr()
fails (for example with -ENOMEM in tmc_create_etr_buf_list(), or -EINVAL
when drvdata->etr_mode == ETR_MODE_RESRV), falling back to the generic ETR
read path halts the running ETR hardware via __tmc_etr_disable_hw() and
sets drvdata->reading = true while byte_cntr_data->reading remains false:

1. If a second process then calls open() on /dev/tmc_etr* while the first
process holds that fallback generic session open, tmc_read_prepare_etr()
calls tmc_read_prepare_byte_cntr() before checking drvdata->reading. The
second process sees byte_cntr_data->reading == false, calls
try_module_get(THIS_MODULE), and sets byte_cntr_data->reading = true before
tmc_create_etr_buf_list() checks drvdata->reading under drvdata->spinlock
and fails with -EBUSY. If the first process calls close() during this
window, tmc_read_unprepare_etr() calls tmc_read_unprepare_byte_cntr(),
which sees byte_cntr_data->reading == true and tears down the second
process's half-initialized state: it calls disable_irq_wake() before
enable_irq_wake() was called, clears byte_cntr_data->reading, calls
module_put(THIS_MODULE) (which the second process's err_reading path then
calls a second time, underflowing the module refcount), and returns 0 so
tmc_read_unprepare_etr() skips clearing drvdata->reading, permanently
wedging the ETR device.

2. Furthermore, in tmc_create_etr_buf_list(), if the sink is disabled while
drvdata->spinlock is dropped to allocate the second buffer and that
allocation fails, the error path calls tmc_clean_etr_buf_list(drvdata),
which frees the pre-existing drvdata->sysfs_buf and loses the captured
trace data.

[1] https://lore.kernel.org/r/20260625110924.BA8EA1F000E9@smtp.kernel.org
[2] https://lore.kernel.org/r/20260625110924.BA8EA1F000E9@smtp.kernel.org
[3] https://lore.kernel.org/r/20261030-enable-byte-cntr-for-ctcu-v20-6-bd5c3ac9f916@oss.qualcomm.com
[4] https://lore.kernel.org/r/20260625110924.BA8EA1F000E9@smtp.kernel.org
[5] https://lore.kernel.org/r/20260625110924.BA8EA1F000E9@smtp.kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-enable-byte-cntr-for-ctcu-v21-0-231e069b7853@oss.qualcomm.com?part=7