Re: [PATCH v5 10/15] coresight: trbe: Workaround TRBE errata overwrite in FILL mode

[Suzuki K Poulose]

On 18/10/2021 16:51, Mathieu Poirier wrote:
> On Thu, Oct 14, 2021 at 11:31:20PM +0100, Suzuki K Poulose wrote:
> > ARM Neoverse-N2 (#2139208) and Cortex-A710(##2119858) suffers from
> > an erratum, which when triggered, might cause the TRBE to overwrite
> > the trace data already collected in FILL mode, in the event of a WRAP.
> > i.e, the TRBE doesn't stop writing the data, instead wraps to the base
> > and could write upto 3 cache line size worth trace. Thus, this could
> > corrupt the trace at the "BASE" pointer.
> >
> > The workaround is to program the write pointer 256bytes from the
> > base, such that if the erratum is triggered, it doesn't overwrite
> > the trace data that was captured. This skipped region could be
> > padded with ignore packets at the end of the session, so that
> > the decoder sees a continuous buffer with some padding at the
> > beginning. The trace data written at the base is considered
> > lost as the limit could have been in the middle of the perf
> > ring buffer, and jumping to the "base" is not acceptable.
> > We set the flags already to indicate that some amount of trace
> > was lost during the FILL event IRQ. So this is fine.
> >
> > One important change with the work around is, we program the
> > TRBBASER_EL1 to current page where we are allowed to write.
> > Otherwise, it could overwrite a region that may be consumed
> > by the perf. Towards this, we always make sure that the
> > "handle->head" and thus the trbe_write is PAGE_SIZE aligned,
> > so that we can set the BASE to the PAGE base and move the
> > TRBPTR to the 256bytes offset.
> >
> > Cc: Mike Leach <mike.leach@xxxxxxxxxx>
> > Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> > Cc: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> > Cc: Leo Yan <leo.yan@xxxxxxxxxx>
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> > ---
> > Changes since v2:
> >   - Updated the ASCII art to include better description of
> >     all the steps in the work around
> > Change since v1:
> >   - Updated comment with ASCII art
> >   - Add _BYTES suffix for the space to skip for the work around.
> > ---
> >   drivers/hwtracing/coresight/coresight-trbe.c | 169 +++++++++++++++++--
> >   1 file changed, 158 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> > index 314e5e7374c7..b56b166b2dec 100644
> > --- a/drivers/hwtracing/coresight/coresight-trbe.c
> > +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> > @@ -16,6 +16,7 @@
> >   #define pr_fmt(fmt) DRVNAME ": " fmt
> >   
> >   #include <asm/barrier.h>
> > +#include <asm/cpufeature.h>
>
> Here too I get a checkpatch warning...

That is a false alarm. I guess that warns for including
linux/cpufeature.h? It is a bit odd, we include this
for the arm64 cpucaps, not the generic linux feature
checks. (They are used for "loading modules" based
on "features" which are more like ELF HWCAPs).

As such I chose to ignore it.

Suzuki