Re: [PATCH v13 08/14] coresight: etm4x: exclude ss_status from drvdata->config

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] coresight: etm4x: state leakage of single-shot comparator status in perf sessions
--

commit 3d98ebedda262d5221467f55a32e0267bdd2e584
Author: Yeoreum Yun <yeoreum.yun@arm.com>
coresight: etm4x: exclude ss_status from drvdata->config

This patch moves the single-shot comparator status (ss_status) from the
configuration structure to the driver data structure. It updates the usage
of ss_status to clear both TRCSSCSRn_STATUS and TRCSSCSRn_PENDING bits
before writing to the register.

> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 3900b96e9c7cc..4a8c1070bbcbb 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c

[ ... ]

> @@ -572,11 +572,11 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>  		etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i));
>  
>  	for (i = 0; i < caps->nr_ss_cmp; i++) {
> -		/* always clear status bit on restart if using single-shot */
> +		/* always clear status and pending bits on restart if using single-shot */
>  		if (config->ss_ctrl[i] || config->ss_pe_cmp[i])
> -			config->ss_status[i] &= ~TRCSSCSRn_STATUS;

[Severity: Medium]
Will this bypass the manual clearing of the TRCSSCSRn_STATUS and
TRCSSCSRn_PENDING bits during perf sessions?

Since enabling a perf session uses a zeroed configuration, config->ss_ctrl[i]
and config->ss_pe_cmp[i] will evaluate to false. Previously, this wasn't an
issue because the entire config structure was zeroed via memset. Now that
ss_status is moved to drvdata, bypassing this check means the bits are not
cleared.

> +			drvdata->ss_status[i] &= ~(TRCSSCSRn_STATUS | TRCSSCSRn_PENDING);
>  		etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
> -		etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
> +		etm4x_relaxed_write32(csa, drvdata->ss_status[i], TRCSSCSRn(i));

Does this write a stale, unmasked drvdata->ss_status[i] directly to the
TRCSSCSRn hardware register during perf sessions?

If the manual clearing above is skipped, this could cause state leakage and
trigger unintended hardware trace behavior.

>  		if (etm4x_sspcicrn_present(drvdata, i))
>  			etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002-separate_etm_cfg_v2-v13-0-fe429d8d96fb@arm.com?part=8