Skip to contentSkip to conversation

coresight: tmc-etr: Sync the trace buffer for the device

PATCH In review NoNine 3 participants 5 messages Sep 15 – Sep 19

Subsystems (2)
lore
Share & tools
lore

NoNine

author
#0 lore raw

cover coresight: tmc-etr: Sync the trace buffer for the device

From: Min Chen <[email protected]>

The flat ETR trace buffer is allocated with dma_alloc_noncoherent(), which reaches dma_alloc_pages(). That API zeroes the buffer with CPU stores and requires the caller to sync the memory for the device before the device writes into it. The TMC driver only ever syncs for the CPU afterwards, so a non-coherent sink is handed a buffer whose zero fill is still dirty in cache.

On an AD1000 EVB this shows up as 64-byte-aligned runs of all-zero formatter frames in the first window of a freshly allocated buffer (1,100-2,400 runs in the leading ~4 MiB; later windows and reused buffers are clean). Writing a known pattern over the buffer before the arm and cleaning it removes every run, which pins the loss to the buffer hand-over. The ETR node is not marked dma-coherent.

The patch adds a sync_for_device() buffer operation, implemented by the flat buffer, and calls it from __tmc_etr_enable_hw() before the TMC is enabled. It is intentionally flat-only: the ETR_SG and CATU data pages are allocated and immediately dma_map_page()d, so the map performs the architecture's for-device maintenance for the first hand-over. Their one remaining corner case - a barrier packet written into a full buffer before a live drain re-arms the same buffer - has no reproducer here and is left out.

Tested on the AD1000 EVB: five first windows on freshly allocated buffers, including the first capture of a boot, carry no all-zero-frame runs and show the ordinary clean-stream profile (2.851% zero bytes), against 1,141-2,360 runs for the same measurement without the change. The patch was rebased onto the Coresight for-next/queue tip 5442d22da7db and builds with arm64 defconfig (Image and modules, no warnings or errors). The hardware run used the vendor 6.6.87 tree; the rebase carries the same change.

Min Chen (1):
coresight: tmc-etr: Sync the trace buffer for the device

 .../hwtracing/coresight/coresight-tmc-etr.c   | 29 +++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h   |  1 +
 2 files changed, 30 insertions(+)
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH 1/1 coresight: tmc-etr: Sync the trace buffer for the device

From: Min Chen <[email protected]>

The flat ETR buffer comes from dma_alloc_noncoherent(), which zeroes it with CPU stores. The DMA API requires the caller to sync the buffer for the device before the device writes into it, but the TMC driver only ever syncs for the CPU afterwards. On a non-coherent sink the zero fill is therefore still dirty in cache when the ETR starts writing, and its write-back lands on top of the trace data.

Add a sync_for_device() buffer operation and call it from __tmc_etr_enable_hw() just before the TMC is enabled. Only the flat buffer implements it. The ETR_SG and CATU data pages are synced by dma_map_page() when they are allocated; their remaining corner case, a barrier packet followed by a live-drain re-arm, is left for a separate change.

Tested on an AD1000 EVB: five first windows on freshly allocated buffers, including the first capture of a boot, all without the previous all-zero-formatter-frame runs.

---
 .../hwtracing/coresight/coresight-tmc-etr.c   | 29 +++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tmc.h   |  1 +
 2 files changed, 30 insertions(+)
drivers/hwtracing/coresight/coresight-tmc-etr.c +29 -0
index 76a8cb2..bf1d6c6 100644
@@ -689,10 +689,29 @@ static ssize_t tmc_etr_get_data_flat_buf(struct etr_buf *etr_buf,
 	return len;
 }
 
+/*
+ * tmc_etr_sync_flat_buf_for_device: Drop any CPU cache lines over the trace
+ * buffer before the ETR is allowed to write into it.  The buffer is allocated
+ * with dma_alloc_noncoherent(), which zeroes it with CPU stores, and the DMA
+ * API requires a sync for the device before the device writes into the
+ * memory.  Without it a non-coherent sink writes into memory while the zero
+ * fill is still dirty in cache, and the write-back lands on top of the trace
+ * data.
+ */
+static void tmc_etr_sync_flat_buf_for_device(struct etr_buf *etr_buf)
+{
+	struct etr_flat_buf *flat_buf = etr_buf->private;
+	struct device *real_dev = flat_buf->dev->parent;
+
+	dma_sync_single_for_device(real_dev, flat_buf->daddr, etr_buf->size,
+				   DMA_FROM_DEVICE);
+}
+
 static const struct etr_buf_operations etr_flat_buf_ops = {
 	.alloc = tmc_etr_alloc_flat_buf,
 	.free = tmc_etr_free_flat_buf,
 	.sync = tmc_etr_sync_flat_buf,
+	.sync_for_device = tmc_etr_sync_flat_buf_for_device,
 	.get_data = tmc_etr_get_data_flat_buf,
 };
 
@@ -1113,6 +1132,16 @@ static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
 	writel_relaxed(ffcr, drvdata->base + TMC_FFCR);
 
 	writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
+
+	/*
+	 * Hand the buffer over in a state the device can write into: drop
+	 * any dirty CPU cache lines first, or they get written back over
+	 * the trace data the ETR produces.  Only the flat buffer needs
+	 * this; the ETR_SG and CATU data pages are synced by
+	 * dma_map_page() when they are allocated.
+	 */
+	if (etr_buf->ops->sync_for_device)
+		etr_buf->ops->sync_for_device(etr_buf);
 	tmc_enable_hw(drvdata);
 
 	CS_LOCK(drvdata->base);
drivers/hwtracing/coresight/coresight-tmc.h +1 -0
index 6541a27..3dd17da 100644
@@ -277,6 +277,7 @@ struct etr_buf_operations {
 	int (*alloc)(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf,
 		     int node, void **pages);
 	void (*sync)(struct etr_buf *etr_buf, u64 rrp, u64 rwp);
+	void (*sync_for_device)(struct etr_buf *etr_buf);
 	ssize_t (*get_data)(struct etr_buf *etr_buf, u64 offset, size_t len,
 			    char **bufpp);
 	void (*free)(struct etr_buf *etr_buf);
Signature · 1 line
2.34.1
#2 lore raw

Hi,

Quoted 9 lines · NoNine

From: Min Chen <[email protected]>

The flat ETR buffer comes from dma_alloc_noncoherent(), which zeroes it with CPU stores. The DMA API requires the caller to sync the buffer for the device before the device writes into it, but the TMC driver only ever syncs for the CPU afterwards. On a non-coherent sink the zero fill is therefore still dirty in cache when the ETR starts writing, and its write-back lands on top of the trace data.

Agree, without the sync, the dirty data may overwrites the trace data.

Quoted 55 lines

Add a sync_for_device() buffer operation and call it from __tmc_etr_enable_hw() just before the TMC is enabled. Only the flat buffer implements it. The ETR_SG and CATU data pages are synced by dma_map_page() when they are allocated; their remaining corner case, a barrier packet followed by a live-drain re-arm, is left for a separate change.

Tested on an AD1000 EVB: five first windows on freshly allocated buffers, including the first capture of a boot, all without the previous all-zero-formatter-frame runs.

---
  .../hwtracing/coresight/coresight-tmc-etr.c   | 29 +++++++++++++++++++
  drivers/hwtracing/coresight/coresight-tmc.h   |  1 +
  2 files changed, 30 insertions(+)
drivers/hwtracing/coresight/coresight-tmc-etr.c +19 -0
index 76a8cb2..bf1d6c6 100644
@@ -689,10 +689,29 @@ static ssize_t tmc_etr_get_data_flat_buf(struct etr_buf *etr_buf,
  	return len;
  }
 
+/*
+ * tmc_etr_sync_flat_buf_for_device: Drop any CPU cache lines over the trace
+ * buffer before the ETR is allowed to write into it.  The buffer is allocated
+ * with dma_alloc_noncoherent(), which zeroes it with CPU stores, and the DMA
+ * API requires a sync for the device before the device writes into the
+ * memory.  Without it a non-coherent sink writes into memory while the zero
+ * fill is still dirty in cache, and the write-back lands on top of the trace
+ * data.
+ */
+static void tmc_etr_sync_flat_buf_for_device(struct etr_buf *etr_buf)
+{
+	struct etr_flat_buf *flat_buf = etr_buf->private;
+	struct device *real_dev = flat_buf->dev->parent;
+
+	dma_sync_single_for_device(real_dev, flat_buf->daddr, etr_buf->size,
+				   DMA_FROM_DEVICE);
+}
+
  static const struct etr_buf_operations etr_flat_buf_ops = {
  	.alloc = tmc_etr_alloc_flat_buf,
  	.free = tmc_etr_free_flat_buf,
  	.sync = tmc_etr_sync_flat_buf,
+	.sync_for_device = tmc_etr_sync_flat_buf_for_device,
  	.get_data = tmc_etr_get_data_flat_buf,
  };
 
@@ -1113,6 +1132,16 @@ static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
  	writel_relaxed(ffcr, drvdata->base + TMC_FFCR);
 
  	writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);

drivers/hwtracing/coresight/coresight-tmc-etr.c static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) · line 1144

Quoted 10 lines
drivers/hwtracing/coresight/coresight-tmc-etr.c +10 -0
+
+	/*
+	 * Hand the buffer over in a state the device can write into: drop
+	 * any dirty CPU cache lines first, or they get written back over
+	 * the trace data the ETR produces.  Only the flat buffer needs
+	 * this; the ETR_SG and CATU data pages are synced by
+	 * dma_map_page() when they are allocated.
+	 */
+	if (etr_buf->ops->sync_for_device)
+		etr_buf->ops->sync_for_device(etr_buf);

Can we add a condition to perform the synchronization only when flat_buf mode is enabled, instead of introducing a new operation that is required only for flat_buf mode?

if (etr_buf->mode == ETR_MODE_FLAT)

	tmc_etr_sync_flat_buf_for_device(etr_buf);

Thanks,
Jie

Quoted 15 lines
drivers/hwtracing/coresight/coresight-tmc-etr.c +0 -0
  	tmc_enable_hw(drvdata);
 
  	CS_LOCK(drvdata->base);
drivers/hwtracing/coresight/coresight-tmc.h +1 -0
index 6541a27..3dd17da 100644
@@ -277,6 +277,7 @@ struct etr_buf_operations {
  	int (*alloc)(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf,
  		     int node, void **pages);
  	void (*sync)(struct etr_buf *etr_buf, u64 rrp, u64 rwp);
+	void (*sync_for_device)(struct etr_buf *etr_buf);
  	ssize_t (*get_data)(struct etr_buf *etr_buf, u64 offset, size_t len,
  			    char **bufpp);
  	void (*free)(struct etr_buf *etr_buf);
reviewer
#3 lore raw

On Wed, Sep 16, 2026 at 11:04:20AM +0800, Jie Gan wrote:

[...]

Quoted 8 lines
Quoted 8 lines

From: Min Chen <[email protected]>

The flat ETR buffer comes from dma_alloc_noncoherent(), which zeroes it with CPU stores. The DMA API requires the caller to sync the buffer for the device before the device writes into it, but the TMC driver only ever syncs for the CPU afterwards. On a non-coherent sink the zero fill is therefore still dirty in cache when the ETR starts writing, and its write-back lands on top of the trace data.

Good catch! I'm curious how you observed the dirty cache lines overwriting trace data in DDR and causing corruption.

Quoted 4 lines

Agree, without the sync, the dirty data may overwrites the trace data.

Quoted 2 lines

Add a sync_for_device() buffer operation and call it from __tmc_etr_enable_hw() just before the TMC is enabled.

I don't think __tmc_etr_enable_hw() is the best place for the sync, as it can be called frequently when an event is enabled, e.g. when a task is scheduled in or migrated between CPUs. We should be able to sync once after dma_alloc_noncoherent() instead.

The issue is not limited to buffer init. The driver also injects barrier packets into the bounce buffer, which can race with the sink. Even worse, the barrier packet write may collide with trace data when they share a cache line.

I think we should consider writing barrier packets directly into the AUX buffer. This would avoid stale cache data from barrier packet writes and simplify the flow without additional sync operations.

Would you mind if I pick up this patch (keeping you as the author) and add a second patch to address the barrier packet issue? That part may need some several rounds refactoring so can have better shape, I think it would be easier to consolidate the fixes on my side.

Thanks,
Leo

P.s. Please CC me on future CoreSight patches. If you're using the mainline ./scripts/get_maintainer.pl, it should add me automatically. I didn't receive this patch directly, which is why I'm replying to Jie's email (also thanks Jie's review).

author
#4 lore raw

From: Min Chen <[email protected]>

Hi Leo,

Quoted 2 lines

Good catch! I'm curious how you observed the dirty cache lines overwriting trace data in DDR and causing corruption.

The story is straightforward, I was testing the newly implemented CoreSight source driver of AD1000 NPU subsystem and in the captured trace data the starting part of a couple of MBs cannot be extracted by following the ARC Trace spec.

Then parse the malformed data:
Leading region - all-zero formatter frames, nothing else (first non-zero byte is at 0x180):

00000000: 0000 0000 0000 0000 0000 0000 0000 0000 00000010: 0000 0000 0000 0000 0000 0000 0000 0000 00000020: 0000 0000 0000 0000 0000 0000 0000 0000 00000030: 0000 0000 0000 0000 0000 0000 0000 0000

zero frames interleaved into the fill; 5 of the 8 frames here are zero, then the fill resumes at 0x40080:

00040030: 0000 0000 0000 0000 0000 0000 0000 0000 00040040: 0000 0000 0000 0000 0000 0000 0000 0000 00040050: 0000 0000 0000 0000 0000 0000 0000 0000 00040060: 0000 0000 0000 0000 0000 0000 0000 0000 00040070: 0000 0000 0000 0000 0000 0000 0000 0000 00040080: feff fe36 f0ce c036 f0ce c036 f0ce c003 00040090: 54ff feff 36f0 cec0 36f0 cec0 36f0 ce00 000400a0: c054 fefe fe36 f0ce c036 f0ce c036 f006

The same area in the unwrapped ATDATA stream (offset 0x40000), where the zeros appear as 60-byte runs bounded by truncated fill words
- 36 f0 ce 00 on entry, 00 c0 54 ff fe ff on exit:

0003fff0: 36f0 cec0 54ff feff 36f0 cec0 36f0 cec0 00040000: 36f0 cec0 54ff feff ffff ffff 36f0 cec0 00040010: 36f0 cec0 36f0 cec0 54ff feff 36f0 ce00 00040020: 0000 0000 0000 0000 0000 0000 0000 0000
...
00040050: 0000 0000 0000 00c0 54ff feff 36f0 cec0 00040060: 36f0 cec0 36f0 cec0 54ff feff 36f0 cec0 00040080: 36f0 cec0 54ff feff ffff ffff 36f0 cec0

Note the quantization: every zero run in the plateau is a multiple of 60 ATDATA bytes (60, 120, 180, 240, ... - 6,251 runs at 60 B, 1,467 at 120 B, decaying), i.e. 4, 8, 12 all-zero formatter frames, minimum 4. There is no run shorter than 4 frames and no other non-fill content between them.

The non-zero data is not random corruption, most of them are decodable as ARC Trace message. And a further experiment confirms that the error mode is overwritting not inserting, where the zeros are removed and trying to decode the remaining stream still reports errors.

The next step is trying to figure out how the overwriting happens by writing a fixed pattern 0x5a5aa5a5 to the ETR flat buffer before enabling it, and a call to dma_sync_single_for_device() is added to make sure the patterns are flushed to memory.

Then the issue cannot be reproduced any more, it is detected a cache sync problem (dma_sync_single_for_device fixed the issue) without too much effect since the debug change is small.

Quoted 6 lines
Quoted 1 line

Agree, without the sync, the dirty data may overwrites the trace data.

I don't think __tmc_etr_enable_hw() is the best place for the sync, as it can be called frequently when an event is enabled, e.g. when a task is scheduled in or migrated between CPUs. We should be able to sync once after dma_alloc_noncoherent() instead.

Agree, perform the sync just after the buffer/page allocation is reasonable and this is what ETR_SG and CATU paths already do.

Quoted 4 lines

The issue is not limited to buffer init. The driver also injects barrier packets into the bounce buffer, which can race with the sink. Even worse, the barrier packet write may collide with trace data when they share a cache line.

I think the timing of the barrier packet writing need further confirmation. Is it only written to the buffer when the ETR is disabled?

Quoted 3 lines

I think we should consider writing barrier packets directly into the AUX buffer. This would avoid stale cache data from barrier packet writes and simplify the flow without additional sync operations.

The AUX buffer only covers Perf path, and what I used is the sysfs path:
echo 0x10000000 > /sys/bus/coresight/devices/tmc_etr0/buffer_size
echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
echo 1 > /sys/bus/coresight/devices/arct0/enable_source
# Run the workload here
echo 0 > /sys/bus/coresight/devices/arct0/enable_source
echo 0 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
cat /dev/tmc_etr0 > arc-trace.bin

Need a solution for both paths.

Quoted 4 lines

Would you mind if I pick up this patch (keeping you as the author) and add a second patch to address the barrier packet issue? That part may need some several rounds refactoring so can have better shape, I think it would be easier to consolidate the fixes on my side.

Please go ahead.

Quoted 7 lines

Thanks,
Leo

P.s. Please CC me on future CoreSight patches. If you're using the mainline ./scripts/get_maintainer.pl, it should add me automatically. I didn't receive this patch directly, which is why I'm replying to Jie's email (also thanks Jie's review).

It's my fault, I did not get the list from the latest trunk.