[PATCH v7 02/13] coresight: etm4x: fix underflow for nrseqstate

[Suzuki K Poulose]

On 01/06/2026 11:08, Yeoreum Yun wrote:
>> On 19/05/2026 16:48, Yeoreum Yun wrote:
>>> TCRSEQEVR<n> is implemented only when TCRIDR5.NUMSEQSTATE is 0b100,
>>> in which case n ranges from 0 to 2; otherwise, TCRIDR5.NUMSEQSTATE is 0b000.
>>>
>>> Therefore, drvdata->nrseqstate should be checked before entering the loop.
>>
>> for (i = 0; i < drvdata->nrseqstate - 1; i++)
>>
>> Wouldn't that check cover the case ? (provided i is signed int ?)
>>
>> Suzuki
> 
> Unfortunately, the drvdata->nrseqstate is u8 but i is signed int.
> I think in this case it would iterate 255 times (0xff).

Then why not fix it to signed ? Anyways, please see my other comment. We 
shouldn't be relying on nrseqstate for the number of seq_ctrl registers.

Suzuki


> 
>>
>>>
>>> Link: https://developer.arm.com/documentation/ihi0064/latest/ [0]
>>> Fixes: 2e1cdfe184b5 ("coresight-etm4x: Adding CoreSight ETM4x driver")
>>> Reviewed-by: Leo Yan <leo.yan at arm.com>
>>> Signed-off-by: Yeoreum Yun <yeoreum.yun at arm.com>
>>> ---
>>>    .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++++++--------
>>>    .../coresight/coresight-etm4x-sysfs.c          |  2 ++
>>>    2 files changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> index 1e3b0344dc00..94b9385e964a 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> @@ -542,9 +542,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>>>    	etm4x_relaxed_write32(csa, config->vissctlr, TRCVISSCTLR);
>>>    	if (drvdata->nr_pe_cmp)
>>>    		etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR);
>>> -	for (i = 0; i < drvdata->nrseqstate - 1; i++)
>>> -		etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i));
>>> +
>>>    	if (drvdata->nrseqstate) {
>>> +		for (i = 0; i < drvdata->nrseqstate - 1; i++)
>>> +			etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i));
>>> +
>>>    		etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR);
>>>    		etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR);
>>>    	}
>>> @@ -1896,10 +1898,10 @@ static int etm4_cpu_save(struct coresight_device *csdev)
>>>    	if (drvdata->nr_pe_cmp)
>>>    		state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR);
>>> -	for (i = 0; i < drvdata->nrseqstate - 1; i++)
>>> -		state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i));
>>> -
>>>    	if (drvdata->nrseqstate) {
>>> +		for (i = 0; i < drvdata->nrseqstate - 1; i++)
>>> +			state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i));
>>> +
>>>    		state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR);
>>>    		state->trcseqstr = etm4x_read32(csa, TRCSEQSTR);
>>>    	}
>>> @@ -2009,10 +2011,10 @@ static void etm4_cpu_restore(struct coresight_device *csdev)
>>>    	if (drvdata->nr_pe_cmp)
>>>    		etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR);
>>> -	for (i = 0; i < drvdata->nrseqstate - 1; i++)
>>> -		etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i));
>>> -
>>>    	if (drvdata->nrseqstate) {
>>> +		for (i = 0; i < drvdata->nrseqstate - 1; i++)
>>> +			etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i));
>>> +
>>>    		etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR);
>>>    		etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR);
>>>    	}
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>>> index e9eeea6240d5..0e1ad175aa1e 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
>>> @@ -1395,6 +1395,8 @@ static ssize_t seq_idx_store(struct device *dev,
>>>    	struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
>>>    	struct etmv4_config *config = &drvdata->config;
>>> +	if (!drvdata->nrseqstate)
>>> +		return -ENOTSUPP;
>>>    	if (kstrtoul(buf, 16, &val))
>>>    		return -EINVAL;
>>>    	if (val >= drvdata->nrseqstate - 1)
>>
>