On 11/9/20 6:32 PM, Mathieu Poirier wrote:
On Wed, Oct 28, 2020 at 10:09:41PM +0000, Suzuki K Poulose wrote:
As per the specification any update to the TRCPRGCTLR must be synchronized by a context synchronization event (in our case an explicist ISB) before the TRCSTATR is checked.
Cc: Mike Leach mike.leach@linaro.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e36bc1c722c7..4bc2f15b6332 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -178,6 +178,15 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) /* Disable the trace unit before programming trace registers */ etm4x_relaxed_write32(csa, 0, TRCPRGCTLR);
- /*
* If we use system instructions, we need to synchronize the* write to the TRCPRGCTLR, before accessing the TRCSTATR.* See ARM IHI0064F, section* "4.3.7 Synchronization of register updates"*/- if (!csa->io_mem)
isb();When I first read the documentation on system instruction section 4.3.7 really got me thinking...
At the very top, right after the title "Synchronization of register updates" one can read "Software running on the PE...". Later in the text, when specifying the synchronisation rules, the term "trace analyzer" is used. _Typically_ a trace analyzer is an external box.
Very good point. The trace analyzer could still use the system register to program the ETM and causing a context synchronization event is tricky from within the trace analyzer. And I agree that there is a bit of confusion around the synchronization from a self-hosted point of view. I believe this is true for the self-hosted case too and should be clarified in the TRM.
Arm documentation is precise and usually doesn't overlook that kind of detail. The question is to understand if a context synchronisation event is also needed when programming is done on the PE. If so I think the documentation should be amended.
In that case:
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
Thanks Suzuki