* [PATCH] coresight: etm4x: fix issues on trcseqevr access
@ 2020-09-02 10:37 Jonathan Zhou
2020-09-03 17:42 ` Mathieu Poirier
2020-09-09 16:42 ` Mathieu Poirier
0 siblings, 2 replies; 8+ messages in thread
From: Jonathan Zhou @ 2020-09-02 10:37 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mathieu Poirier, Suzuki K Poulose, lizixian, Shaokun Zhang,
Jonathan Zhou, Mike Leach
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@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: lizixian@hisilicon.com
Signed-off-by: Jonathan Zhou <jonathan.zhouwen@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));
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access 2020-09-02 10:37 [PATCH] coresight: etm4x: fix issues on trcseqevr access Jonathan Zhou @ 2020-09-03 17:42 ` Mathieu Poirier 2020-09-07 16:51 ` Mike Leach 2020-09-09 16:42 ` Mathieu Poirier 1 sibling, 1 reply; 8+ messages in thread From: Mathieu Poirier @ 2020-09-03 17:42 UTC (permalink / raw) To: Jonathan Zhou Cc: Shaokun Zhang, lizixian, Mike Leach, linux-arm-kernel, Suzuki K Poulose 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@linaro.org> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: Shaokun Zhang <zhangshaokun@hisilicon.com> > Cc: lizixian@hisilicon.com > > Signed-off-by: Jonathan Zhou <jonathan.zhouwen@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 trace unit state. 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access 2020-09-03 17:42 ` Mathieu Poirier @ 2020-09-07 16:51 ` Mike Leach 2020-09-08 1:39 ` Jonathan Zhou 2020-09-08 17:43 ` Mathieu Poirier 0 siblings, 2 replies; 8+ messages in thread From: Mike Leach @ 2020-09-07 16:51 UTC (permalink / raw) To: Mathieu Poirier Cc: Shaokun Zhang, lizixian, linux-arm-kernel, Jonathan Zhou, Suzuki K Poulose Hi, On Thu, 3 Sep 2020 at 18:42, Mathieu Poirier <mathieu.poirier@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@linaro.org> > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > Cc: Mike Leach <mike.leach@linaro.org> > > Cc: Shaokun Zhang <zhangshaokun@hisilicon.com> > > Cc: lizixian@hisilicon.com > > > > Signed-off-by: Jonathan Zhou <jonathan.zhouwen@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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access 2020-09-07 16:51 ` Mike Leach @ 2020-09-08 1:39 ` Jonathan Zhou 2020-09-08 17:43 ` Mathieu Poirier 1 sibling, 0 replies; 8+ messages in thread From: Jonathan Zhou @ 2020-09-08 1:39 UTC (permalink / raw) To: Mike Leach, Mathieu Poirier Cc: Shaokun Zhang, lizixian, linux-arm-kernel, Suzuki K Poulose Hi Mike, On 08/09/2020 00:51, Mike Leach wrote: > Hi, > > On Thu, 3 Sep 2020 at 18:42, Mathieu Poirier <mathieu.poirier@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@linaro.org> >>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >>> Cc: Mike Leach <mike.leach@linaro.org> >>> Cc: Shaokun Zhang <zhangshaokun@hisilicon.com> >>> Cc: lizixian@hisilicon.com >>> >>> Signed-off-by: Jonathan Zhou <jonathan.zhouwen@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. > Thanks for point out this, I just never thought it may be a typo. Regards Jonathan > 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 >>> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access 2020-09-07 16:51 ` Mike Leach 2020-09-08 1:39 ` Jonathan Zhou @ 2020-09-08 17:43 ` Mathieu Poirier 2020-09-09 9:41 ` Mike Leach 1 sibling, 1 reply; 8+ messages in thread From: Mathieu Poirier @ 2020-09-08 17:43 UTC (permalink / raw) To: Mike Leach Cc: Suzuki K Poulose, lizixian, Coresight ML, Shaokun Zhang, Jonathan Zhou, linux-arm-kernel Hi Mike, On Mon, 7 Sep 2020 at 10:52, Mike Leach <mike.leach@linaro.org> wrote: > > Hi, > > On Thu, 3 Sep 2020 at 18:42, Mathieu Poirier <mathieu.poirier@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@linaro.org> > > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > > Cc: Mike Leach <mike.leach@linaro.org> > > > Cc: Shaokun Zhang <zhangshaokun@hisilicon.com> > > > Cc: lizixian@hisilicon.com > > > > > > Signed-off-by: Jonathan Zhou <jonathan.zhouwen@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. Did you get an answer from the document people? Is this really a typographical problem? Thanks, Mathieu > > > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access 2020-09-08 17:43 ` Mathieu Poirier @ 2020-09-09 9:41 ` Mike Leach 2020-09-09 16:32 ` Mathieu Poirier 0 siblings, 1 reply; 8+ messages in thread From: Mike Leach @ 2020-09-09 9:41 UTC (permalink / raw) To: Mathieu Poirier Cc: Suzuki K Poulose, lizixian, Coresight ML, Shaokun Zhang, Jonathan Zhou, linux-arm-kernel Hi Mathieu, On Tue, 8 Sep 2020 at 18:43, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > Hi Mike, > > On Mon, 7 Sep 2020 at 10:52, Mike Leach <mike.leach@linaro.org> wrote: > > > > Hi, > > > > On Thu, 3 Sep 2020 at 18:42, Mathieu Poirier <mathieu.poirier@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@linaro.org> > > > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > > > Cc: Mike Leach <mike.leach@linaro.org> > > > > Cc: Shaokun Zhang <zhangshaokun@hisilicon.com> > > > > Cc: lizixian@hisilicon.com > > > > > > > > Signed-off-by: Jonathan Zhou <jonathan.zhouwen@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. > > Did you get an answer from the document people? Is this really a > typographical problem? > Haven't had a reply, but this is a clear error. All other parts of the spec, including programming the device and register descriptions give a valid n=0-2 for TRCSEQEVRn. TRCSEQEVR3 does not exist, and the assumed location is indeed reserved. That said, reserved registers are marked as RES0H - so no actual harm can come from reading / writing the register - but we should be consistent. I would regard section 3.4.3 as a list of registers to save / restore as correct with the caveat - "should they exist in the implementation". Quite a few of the register ranges defined in this list show the maximum extent of registers where the actual number of registers is implementation dependent. Thanks Mike > Thanks, > Mathieu > > > > > > 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 -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access 2020-09-09 9:41 ` Mike Leach @ 2020-09-09 16:32 ` Mathieu Poirier 0 siblings, 0 replies; 8+ messages in thread From: Mathieu Poirier @ 2020-09-09 16:32 UTC (permalink / raw) To: Mike Leach Cc: Suzuki K Poulose, lizixian, Coresight ML, Shaokun Zhang, Jonathan Zhou, linux-arm-kernel On Wed, Sep 09, 2020 at 10:41:27AM +0100, Mike Leach wrote: > Hi Mathieu, > > On Tue, 8 Sep 2020 at 18:43, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > > Hi Mike, > > > > On Mon, 7 Sep 2020 at 10:52, Mike Leach <mike.leach@linaro.org> wrote: > > > > > > Hi, > > > > > > On Thu, 3 Sep 2020 at 18:42, Mathieu Poirier <mathieu.poirier@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@linaro.org> > > > > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > > > > Cc: Mike Leach <mike.leach@linaro.org> > > > > > Cc: Shaokun Zhang <zhangshaokun@hisilicon.com> > > > > > Cc: lizixian@hisilicon.com > > > > > > > > > > Signed-off-by: Jonathan Zhou <jonathan.zhouwen@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. > > > > Did you get an answer from the document people? Is this really a > > typographical problem? > > > > Haven't had a reply, but this is a clear error. All other parts of the > spec, including programming the device and register descriptions give > a valid n=0-2 for TRCSEQEVRn. > TRCSEQEVR3 does not exist, and the assumed location is indeed > reserved. That said, reserved registers are marked as RES0H - so no > actual harm can come from reading / writing the register - but we > should be consistent. > > I would regard section 3.4.3 as a list of registers to save / restore > as correct with the caveat - "should they exist in the > implementation". Quite a few of the register ranges defined in this > list show the maximum extent of registers where the actual number of > registers is implementation dependent. Very well - I will proceed with this patch. > > Thanks > > Mike > > > > Thanks, > > Mathieu > > > > > > > > > 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 > > > > -- > Mike Leach > Principal Engineer, ARM Ltd. > Manchester Design Centre. UK _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access 2020-09-02 10:37 [PATCH] coresight: etm4x: fix issues on trcseqevr access Jonathan Zhou 2020-09-03 17:42 ` Mathieu Poirier @ 2020-09-09 16:42 ` Mathieu Poirier 1 sibling, 0 replies; 8+ messages in thread From: Mathieu Poirier @ 2020-09-09 16:42 UTC (permalink / raw) To: Jonathan Zhou Cc: Shaokun Zhang, lizixian, Mike Leach, linux-arm-kernel, Suzuki K Poulose 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@linaro.org> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: Shaokun Zhang <zhangshaokun@hisilicon.com> > Cc: lizixian@hisilicon.com > > Signed-off-by: Jonathan Zhou <jonathan.zhouwen@huawei.com> > --- > drivers/hwtracing/coresight/coresight-etm4x.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) I have applied this patch. Thanks, Mathieu > > 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)); > > 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-09 16:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-02 10:37 [PATCH] coresight: etm4x: fix issues on trcseqevr access Jonathan Zhou 2020-09-03 17:42 ` Mathieu Poirier 2020-09-07 16:51 ` Mike Leach 2020-09-08 1:39 ` Jonathan Zhou 2020-09-08 17:43 ` Mathieu Poirier 2020-09-09 9:41 ` Mike Leach 2020-09-09 16:32 ` Mathieu Poirier 2020-09-09 16:42 ` Mathieu Poirier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).