[PATCH v3 1/8] coresight: tmc-etr: Don't stop Perf cleanup for active sysfs reads

[James Clark]

The linked fixes commit deliberately allows reads of an old sysfs buffer
while in Perf mode because they are separate software buffers. However
it didn't modify tmc_disable_etr_sink() to match this relaxation. The
result is that when a Perf event ends while the sysfs buffer is being
read, clean up will be skipped.

Fix it by ignoring the sysfs_reading flag unless the active session is
a sysfs one.

When sysfs and Perf share the same memory in ETR_MODE_RESRV mode, a new
Perf session needs to overwrite an old inactive sysfs session by zeroing
len. This avoids sysfs from reading stale data because it has a separate
set of offsets in its etr_buf struct, even if that's backed by the same
memory as the Perf one.

Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
Fixes: cad5f8d399bb ("coresight: tmc-etr: Relax collection of trace from sysfs mode")
Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 16 ++++----
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 50 +++++++++++++++++++++----
 drivers/hwtracing/coresight/coresight-tmc.h     |  7 +++-
 3 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 8882b1c4cdc0..3836063031d7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -198,7 +198,7 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev)
 		raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 	}
 
-	if (drvdata->reading) {
+	if (drvdata->sysfs_reading) {
 		ret = -EBUSY;
 		goto out;
 	}
@@ -259,7 +259,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev,
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 	do {
 		ret = -EINVAL;
-		if (drvdata->reading)
+		if (drvdata->sysfs_reading)
 			break;
 		/*
 		 * No need to continue if the ETB/ETF is already operated
@@ -337,7 +337,7 @@ static int tmc_disable_etf_sink(struct coresight_device *csdev)
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 
-	if (drvdata->reading) {
+	if (drvdata->sysfs_reading) {
 		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 		return -EBUSY;
 	}
@@ -371,7 +371,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
 	bool first_enable = false;
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
-	if (drvdata->reading) {
+	if (drvdata->sysfs_reading) {
 		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 		return -EBUSY;
 	}
@@ -401,7 +401,7 @@ static void tmc_disable_etf_link(struct coresight_device *csdev,
 	bool last_disable = false;
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
-	if (drvdata->reading) {
+	if (drvdata->sysfs_reading) {
 		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 		return;
 	}
@@ -718,7 +718,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 
-	if (drvdata->reading) {
+	if (drvdata->sysfs_reading) {
 		ret = -EBUSY;
 		goto out;
 	}
@@ -746,7 +746,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
 		__tmc_etb_disable_hw(drvdata);
 	}
 
-	drvdata->reading = true;
+	drvdata->sysfs_reading = true;
 out:
 	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
@@ -797,7 +797,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
 		drvdata->buf = NULL;
 	}
 
-	drvdata->reading = false;
+	drvdata->sysfs_reading = false;
 	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	/*
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 361a433e6f0c..be0bbe036d02 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1151,7 +1151,7 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
  * starts at anywhere in the buffer, depending on the RRP, we adjust the
  * @len returned to handle buffer wrapping around.
  *
- * We are protected here by drvdata->reading != 0, which ensures the
+ * We are protected here by drvdata->sysfs_reading != 0, which ensures the
  * sysfs_buf stays alive.
  */
 ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
@@ -1268,7 +1268,7 @@ 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) {
+	if (drvdata->sysfs_reading || coresight_get_mode(csdev) == CS_MODE_PERF) {
 		ret = -EBUSY;
 		goto out;
 	}
@@ -1732,6 +1732,16 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
 	return size;
 }
 
+static bool tmc_perf_sysfs_shared(struct tmc_drvdata *drvdata,
+				  struct etr_buf *perf_buf)
+{
+	/* In ETR_MODE_RESRV mode, sysfs and Perf share the same memory. */
+	return perf_buf &&
+	       drvdata->sysfs_buf &&
+	       drvdata->sysfs_buf->mode == ETR_MODE_RESRV &&
+	       perf_buf->mode == ETR_MODE_RESRV;
+}
+
 static int tmc_enable_etr_sink_perf(struct coresight_device *csdev,
 				    struct coresight_path *path)
 {
@@ -1772,6 +1782,18 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev,
 		goto unlock_out;
 	}
 
+	/*
+	 * Don't use if it's shared and being read by sysfs. Sysfs may only
+	 * start reading (the cleared zero length buffer) after the first
+	 * tmc_enable_etr_sink_perf(), which changes the result of this check,
+	 * so it should only be done once.
+	 */
+	if ((drvdata->sysfs_reading &&
+	     tmc_perf_sysfs_shared(drvdata, etr_perf->etr_buf))) {
+		rc = -EBUSY;
+		goto unlock_out;
+	}
+
 	rc = tmc_etr_enable_hw(drvdata, etr_perf->etr_buf);
 	if (!rc) {
 		/* Associate with monitored process. */
@@ -1779,6 +1801,10 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev,
 		coresight_set_mode(csdev, CS_MODE_PERF);
 		drvdata->perf_buf = etr_perf->etr_buf;
 		csdev->refcnt++;
+
+		/* A new Perf session clears an old sysfs one if the buffer is shared */
+		if (tmc_perf_sysfs_shared(drvdata, etr_perf->etr_buf))
+			drvdata->sysfs_buf->len = 0;
 	}
 
 unlock_out:
@@ -1807,7 +1833,13 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
 
-	if (drvdata->reading) {
+	/*
+	 * In SYSFS mode an active read is responsible for disabling and
+	 * enabling HW. Otherwise in Perf mode, an old inactive sysfs session
+	 * may be read which Perf should ignore.
+	 */
+	if (drvdata->sysfs_reading &&
+	    coresight_get_mode(csdev) == CS_MODE_SYSFS) {
 		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 		return -EBUSY;
 	}
@@ -1928,14 +1960,16 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 		return -EINVAL;
 
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
-	if (drvdata->reading) {
+	if (drvdata->sysfs_reading) {
 		ret = -EBUSY;
 		goto out;
 	}
 
 	/*
-	 * We can safely allow reads even if the ETR is operating in PERF mode,
-	 * since the sysfs session is captured in mode specific data.
+	 * We can safely allow reads even if the ETR is operating in PERF mode
+	 * since sysfs has it's own buffer. For ETR_MODE_RESRV the buffers are
+	 * shared but Perf discards sysfs data before starting a session to
+	 * avoid corruption.
 	 * If drvdata::sysfs_data is NULL the trace data has been read already.
 	 */
 	if (!drvdata->sysfs_buf) {
@@ -1947,7 +1981,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 	if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
 		__tmc_etr_disable_hw(drvdata);
 
-	drvdata->reading = true;
+	drvdata->sysfs_reading = true;
 out:
 	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
@@ -1982,7 +2016,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
 		drvdata->sysfs_buf = NULL;
 	}
 
-	drvdata->reading = false;
+	drvdata->sysfs_reading = false;
 	raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	/* Free allocated memory out side of the spinlock */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 319a354ede9f..dc1a57ab8011 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -221,7 +221,10 @@ struct tmc_resrv_buf {
  * @pid:	Process ID of the process that owns the session that is using
  *		this component. For example this would be the pid of the Perf
  *		process.
- * @reading:	buffer's in the reading through "/dev/xyz.tmc" entry
+ * @sysfs_reading: Sysfs mode buffer is being read through "/dev/xyz.tmc" entry.
+ *                Note: ETR has a separate software buffer for the two modes so
+ *		  the device can still be read while in Perf mode if there is a
+ *		  previous inactive sysfs session.
  * @stop_on_flush: Stop on flush trigger user configuration.
  * @buf:	Snapshot of the trace data for ETF/ETB.
  * @etr_buf:	details of buffer used in TMC-ETR
@@ -255,7 +258,7 @@ struct tmc_drvdata {
 	struct miscdevice	crashdev;
 	raw_spinlock_t		spinlock;
 	pid_t			pid;
-	bool			reading;
+	bool			sysfs_reading;
 	bool			stop_on_flush;
 	union {
 		char		*buf;		/* TMC ETB */

-- 
2.34.1