diff mbox series

coresight-etm4: Fix for() loop drvdata->nr_addr_cmp range bug

Message ID 4a4ee61ce8ef402615a4528b21a051de3444fb7b.1677540079.git.scclevenger@os.amperecomputing.com (mailing list archive)
State New, archived
Headers show
Series coresight-etm4: Fix for() loop drvdata->nr_addr_cmp range bug | expand

Commit Message

Steve Clevenger Feb. 27, 2023, 11:54 p.m. UTC
In etm4_enable_hw, fix for() loop range to represent address comparator pairs.

Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

James Clark Feb. 28, 2023, 11:27 a.m. UTC | #1
On 27/02/2023 23:54, Steve Clevenger wrote:
> In etm4_enable_hw, fix for() loop range to represent address comparator pairs.
> 
> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 1ea8f173cca0..104333c2c8a3 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>  		if (etm4x_sspcicrn_present(drvdata, i))
>  			etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
>  	}
> -	for (i = 0; i < drvdata->nr_addr_cmp; i++) {
> +	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
>  		etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i));
>  		etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i));
>  	}

Reviewed-by: James Clark <james.clark@arm.com>

Are there other ones in coresight-etm3x-sysfs.c:reset_store() and
coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different?
Mike Leach Feb. 28, 2023, 12:32 p.m. UTC | #2
Hi

On Tue, 28 Feb 2023 at 11:27, James Clark <james.clark@arm.com> wrote:
>
>
>
> On 27/02/2023 23:54, Steve Clevenger wrote:
> > In etm4_enable_hw, fix for() loop range to represent address comparator pairs.
> >
> > Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > index 1ea8f173cca0..104333c2c8a3 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> >               if (etm4x_sspcicrn_present(drvdata, i))
> >                       etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
> >       }
> > -     for (i = 0; i < drvdata->nr_addr_cmp; i++) {
> > +     for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
> >               etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i));
> >               etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i));
> >       }
>
> Reviewed-by: James Clark <james.clark@arm.com>
>
> Are there other ones in coresight-etm3x-sysfs.c:reset_store() and
> coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different?

For reasons passing understanding - the etm3x code does the 2 *
multiplier at the point that config->nr_addr_cmp is initialised, so it
reflects the number of comparators rather than the number of pairs.
That said the comments and docs in
./Documents/ABI/testing/sysfs-bus-coresight-devices-etm3x claim that
the value is still the number of pairs.

Mike



--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Suzuki K Poulose Feb. 28, 2023, 4:16 p.m. UTC | #3
On 28/02/2023 11:27, James Clark wrote:
> 
> 
> On 27/02/2023 23:54, Steve Clevenger wrote:
>> In etm4_enable_hw, fix for() loop range to represent address comparator pairs.
>>
>> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 1ea8f173cca0..104333c2c8a3 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>>   		if (etm4x_sspcicrn_present(drvdata, i))
>>   			etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
>>   	}
>> -	for (i = 0; i < drvdata->nr_addr_cmp; i++) {
>> +	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
>>   		etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i));
>>   		etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i));
>>   	}
> 
> Reviewed-by: James Clark <james.clark@arm.com>
> 
> Are there other ones in coresight-etm3x-sysfs.c:reset_store() and > coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different?

Fixes: 2e1cdfe184b5 ("coresight-etm4x: Adding CoreSight ETM4x driver")

I have queued this as fixes with the above tag and will send it out for v6.3


Thanks
Suzuki
Steve Clevenger Feb. 28, 2023, 5:39 p.m. UTC | #4
Hi Mike and James,

I did see the etm3x initialization code. etm4x seems to not do it that way.

Steve

On 2/28/2023 4:32 AM, Mike Leach wrote:
> Hi
> 
> On Tue, 28 Feb 2023 at 11:27, James Clark <james.clark@arm.com> wrote:
>>
>>
>>
>> On 27/02/2023 23:54, Steve Clevenger wrote:
>>> In etm4_enable_hw, fix for() loop range to represent address comparator pairs.
>>>
>>> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com>
>>> ---
>>>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> index 1ea8f173cca0..104333c2c8a3 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> @@ -472,7 +472,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>>>               if (etm4x_sspcicrn_present(drvdata, i))
>>>                       etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
>>>       }
>>> -     for (i = 0; i < drvdata->nr_addr_cmp; i++) {
>>> +     for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
>>>               etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i));
>>>               etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i));
>>>       }
>>
>> Reviewed-by: James Clark <james.clark@arm.com>
>>
>> Are there other ones in coresight-etm3x-sysfs.c:reset_store() and
>> coresight-etm3x-core.c:etm_enable_hw()? Or is etm3 different?
> 
> For reasons passing understanding - the etm3x code does the 2 *
> multiplier at the point that config->nr_addr_cmp is initialised, so it
> reflects the number of comparators rather than the number of pairs.
> That said the comments and docs in
> ./Documents/ABI/testing/sysfs-bus-coresight-devices-etm3x claim that
> the value is still the number of pairs.
> 
> Mike
> 
> 
> 
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 1ea8f173cca0..104333c2c8a3 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -472,7 +472,7 @@  static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 		if (etm4x_sspcicrn_present(drvdata, i))
 			etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
 	}
-	for (i = 0; i < drvdata->nr_addr_cmp; i++) {
+	for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {
 		etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i));
 		etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i));
 	}