On 11/10/20 10:11 AM, Suzuki K Poulose wrote: > 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 <[email protected]> >>> Cc: Mathieu Poirier <[email protected]> >>> Signed-off-by: Suzuki K Poulose <[email protected]> >>> --- >>> 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. >
The ETM architecture uses "trace analyzer" to mean self-hosted software and an external debugger. It's a useful term that generically covers "the thing that's in charge of tracing" and "the thing that's capturing and/or decoding the trace", regardless of whether either of these are external or self-hosted (or even a mixture!). So in 4.3.7, yes this does mean that context synchronization events are needed to synchronize register updates when using system instructions to program the trace unit. I'll take a look at what we can improve here :-) Cheers, John. >> 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 <[email protected]> >> > >Thanks >Suzuki >_______________________________________________ >CoreSight mailing list >[email protected] >https://lists.linaro.org/mailman/listinfo/coresight
