Hi Andrew,
On 27/06/2019 09:35, Andrew Murray wrote:
Synchronization is recommended before disabling the trace registers
to prevent any start or stop points being speculative at the point
of disabling the unit (section 7.3.77 of ARM IHI 0064D).
Synchronization is also recommended after programming the trace
registers to ensure all updates are committed prior to normal code
resuming (section 4.3.7 of ARM IHI 0064D).
Let's ensure these syncronization points are present in the code
and clearly commented.
Please could you also mention why we switched from mb() ?
Note that we could rely on the barriers in CS_LOCK and
coresight_disclaim_device_unlocked or the context switch to user
space - however coresight may be of use in the kernel.
Signed-off-by: Andrew Murray <andrew.murray@xxxxxxx>
CC: stable@xxxxxxxxxxxxxxx
---
drivers/hwtracing/coresight/coresight-etm4x.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index c89190d464ab..68e8e3954cef 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -188,6 +188,10 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
dev_err(etm_dev,
"timeout while waiting for Idle Trace Status\n");
+ /* As recommended by 4.3.7 of ARM IHI 0064D */
nit: It would be good to mention the "section name" to help the reader
find the same on a different version of the document. Also within the same
version, this is listed in the subsection:
"Synchronization when using the memory-mapped interface"
Please could you update the comment to reflect the same ?
+ dsb(sy);
+ isb();
+
done:
CS_LOCK(drvdata->base);
@@ -454,7 +458,8 @@ static void etm4_disable_hw(void *info)
control &= ~0x1;
/* make sure everything completes before disabling */
- mb();
+ /* As recommended by 7.3.77 of ARM IHI 0064D */
Nit: This refers to completely unrelated section. Shouldn't this be the same
as above ?
With the above fixed:
Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
