On 24/10/2025 17:45, Leo Yan wrote:
As recommended in section 4.3.7 "Synchronization of register updates" of ARM IHI0064H.b, a self-hosted trace analyzer should always executes an ISB instruction after programming the trace unit registers.
An ISB works as a context synchronization event; a DSB is not required. Removes the redundant barrier in the enabling flow.
It is required for MMIO based instances and must be retained.
The ISB was placed at the end of the enable and disable functions. However, this does not guarantee a context synchronization event in the calling code, which may speculatively execute across function boundaries.
It is not allowed to commit though, right ? I don't think this change is required TBH.
Suzuki
ISB instructions are moved into callers to ensure that a context synchronization is imposed immediately after enabling or disabling trace unit.
Fixes: 40f682ae5086 ("coresight: etm4x: Extract the trace unit controlling") Reviewed-by: Mike Leach mike.leach@linaro.org Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com
drivers/hwtracing/coresight/coresight-etm4x-core.c | 38 +++++++++++++++------- 1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index c562f82985192ad71e181be1b570c9a2f334f29f..45bf76901650641d45a0d85965651667a8762b8c 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -460,13 +460,6 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata) return -ETIME; }
- /*
* As recommended by section 4.3.7 ("Synchronization when using the* memory-mapped interface") of ARM IHI 0064D*/- dsb(sy);
- isb();
- return 0; }
@@ -581,6 +574,13 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) if (!drvdata->paused) rc = etm4_enable_trace_unit(drvdata);
- /*
* As recommended by section 4.3.7 (Synchronization of register updates)* of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an* ISB instruction after programming the trace unit registers.*/- isb(); done: etm4_cs_lock(drvdata, csa);
@@ -954,11 +954,6 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata) if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for PM stable Trace Status\n");
- /*
* As recommended by section 4.3.7 (Synchronization of register updates)* of ARM IHI 0064H.b.*/- isb(); }
static void etm4_disable_hw(struct etmv4_drvdata *drvdata) @@ -981,6 +976,13 @@ static void etm4_disable_hw(struct etmv4_drvdata *drvdata) etm4_disable_trace_unit(drvdata);
- /*
* As recommended by section 4.3.7 (Synchronization of register updates)* of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an* ISB instruction after programming the trace unit registers.*/- isb();
- /* read the status of the single shot comparators */ for (i = 0; i < drvdata->nr_ss_cmp; i++) { config->ss_status[i] =
@@ -1118,6 +1120,12 @@ static int etm4_resume_perf(struct coresight_device *csdev) etm4_cs_unlock(drvdata, csa); etm4_enable_trace_unit(drvdata);
- /*
* As recommended by section 4.3.7 (Synchronization of register updates)* of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an* ISB instruction after programming the trace unit registers.*/- isb(); etm4_cs_lock(drvdata, csa);
drvdata->paused = false; @@ -1134,6 +1142,12 @@ static void etm4_pause_perf(struct coresight_device *csdev) etm4_cs_unlock(drvdata, csa); etm4_disable_trace_unit(drvdata);
- /*
* As recommended by section 4.3.7 (Synchronization of register updates)* of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an* ISB instruction after programming the trace unit registers.*/- isb(); etm4_cs_lock(drvdata, csa);
drvdata->paused = true;