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

From: Jonathan Zhou <jonathan.zhouwen@huawei.com>
To: Mike Leach <mike.leach@linaro.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Shaokun Zhang <zhangshaokun@hisilicon.com>,
	lizixian@hisilicon.com,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>
Subject: Re: [PATCH] coresight: etm4x: fix issues on trcseqevr access
Date: Tue, 8 Sep 2020 09:39:21 +0800	[thread overview]
Message-ID: <b12593fa-b607-a8a1-e817-cc71bd73d260@huawei.com> (raw)
In-Reply-To: <CAJ9a7Vg+=j5LkgYQtZ_kS8H97gFy92pcwn2Y+kBvAuxnagRW2Q@mail.gmail.com>

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