Hi Leo,
> 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.
>
> 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.
>
> 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")
> 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 0f2a8b8459c93ca29d270b6fa05928244e0761c5..af9d3b2319c5f49ccd40dfa0ccf0f694ce9e2f4f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -459,13 +459,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;
> }
>
> @@ -579,6 +572,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();
But according to 4.3.7 ("Synchronization when using memory-mapped
interface"), doesn't it need to dsb like:
if (csa->iomem)
dsb(sy);
isb();
Or am I missing something?
> 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;
>
> --
> 2.34.1
>
--
Sincerely,
Yeoreum Yun