[PATCH] coresight: etm4x: fix issues on trcseqevr access

[Mike Leach]

Hi,

On Thu, 3 Sep 2020 at 18:42, Mathieu Poirier <mathieu.poirier at linaro.org> wrote:
>
> On Wed, Sep 02, 2020 at 06:37:13PM +0800, Jonathan Zhou wrote:
> > The TRCSEQEVR(3) is reserved, using '@nrseqstate - 1' instead to avoid
> > accessing the reserved register.
> >
> > Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
> > Cc: Mathieu Poirier <mathieu.poirier at linaro.org>
> > Cc: Suzuki K Poulose <suzuki.poulose at arm.com>
> > Cc: Mike Leach <mike.leach at linaro.org>
> > Cc: Shaokun Zhang <zhangshaokun at hisilicon.com>
> > Cc: lizixian at hisilicon.com
> >
> > Signed-off-by: Jonathan Zhou <jonathan.zhouwen at huawei.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-etm4x.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> > index 96425e818fc2..44e44c817bf8 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> > @@ -1183,7 +1183,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
> >       state->trcvdsacctlr = readl(drvdata->base + TRCVDSACCTLR);
> >       state->trcvdarcctlr = readl(drvdata->base + TRCVDARCCTLR);
> >
> > -     for (i = 0; i < drvdata->nrseqstate; i++)
> > +     for (i = 0; i < drvdata->nrseqstate - 1; i++)
> >               state->trcseqevr[i] = readl(drvdata->base + TRCSEQEVRn(i));
>
> The section 3.4.3 "Guidelines for trace unit registers to be saved and restored"
> of the ETM4 Architecture Specification (ARM IHI0064F ID042818) is clear on the
> fact that registers TRCSEQEVR0-3 have to be taken into account when saving the

I think this is a typo in the TRM. I'll ping the docs people in ARM.

> trace unit state.
>

Looking @ the register descriptions for TRCSEQEVRn (7.3.63) in the
above document n=0-2.
The number of states is set by TRCIDR5.NUMSEQSTATE (7.3.35). This can
take the value 0 or 4.
If 4 then there are 3 TRCSEQEVR(n) registers - 0 to 2 - one for each
state transition.

Thus this patch is correct in using nrseqstate - 1.

Regards

Mike

> Thanks,
> Mathieu
>
> >
> >       state->trcseqrstevr = readl(drvdata->base + TRCSEQRSTEVR);
> > @@ -1288,7 +1288,7 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
> >       writel_relaxed(state->trcvdsacctlr, drvdata->base + TRCVDSACCTLR);
> >       writel_relaxed(state->trcvdarcctlr, drvdata->base + TRCVDARCCTLR);
> >
> > -     for (i = 0; i < drvdata->nrseqstate; i++)
> > +     for (i = 0; i < drvdata->nrseqstate - 1; i++)
> >               writel_relaxed(state->trcseqevr[i],
> >                              drvdata->base + TRCSEQEVRn(i));
> > --
> > 1.9.1
> >



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK