Re: [PATCH v10 2/8] coresight: etm3x: fix inconsistencies with sysfs configuration

[Yeoreum Yun]

On Fri, Sep 11, 2026 at 11:07:29AM +0000, sashiko-bot@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Sysfs interface for post-trace hardware state returns stale data due to active_config separation (ABI Breakage).
> --
> 
> commit 729c4f4f776d1cbadf2571ee6a6363ece19f7a91
> Author: Yeoreum Yun <yeoreum.yun@xxxxxxx>
> 
> coresight: etm3x: fix inconsistencies with sysfs configuration
> 
> This commit resolves sysfs configuration inconsistencies by separating the
> configuration state into an active_config (for the current session) and config
> (the configuration set via sysfs).
> 
> > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> > --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> 
> [ ... ]
> 
> > @@ -565,7 +569,7 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
> >  static void etm_disable_hw(struct etm_drvdata *drvdata)
> >  {
> >  	int i;
> > -	struct etm_config *config = &drvdata->config;
> > +	struct etm_config *config = &drvdata->active_config;
> >  	struct coresight_device *csdev = drvdata->csdev;
> >  
> >  	CS_UNLOCK(drvdata->csa.base);
> 
> [Severity: High]
> Does this introduce a regression in the sysfs interface for post-trace
> hardware state analysis?
> 
> By switching to active_config here, the post-trace sequencer and counter
> states are saved into drvdata->active_config during etm_disable_hw():
> 
> drivers/hwtracing/coresight/coresight-etm3x-core.c:etm_disable_hw() {
> 	...
> 	/* Read back sequencer and counters for post trace analysis */
> 	config->seq_curr_state = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
> 
> 	for (i = 0; i < caps->nr_cntr; i++)
> 		config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
> 	...
> }
> 
> However, when tracing is disabled, userspace reads from the sysfs interface
> which still returns data from the stale drvdata->config:
> 
> drivers/hwtracing/coresight/coresight-etm3x-sysfs.c:seq_curr_state_show() {
> 	...
> 	val = config->seq_curr_state;
> 	...
> }
> 
> Will this cause sysfs reads for seq_curr_state and cntr_val to return
> stale or zeroed data instead of the actual hardware state?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260911-separate_etm_cfg_v2-v10-0-1b715d95927a@xxxxxxx?part=2

This seems valid review and I think the cntr_val and seq_current_state
shouldn't be modified via sysfs while the mode is taken by sysfs.

I'll fix this in later version for etm4 and etm3.

-- 
Sincerely,
Yeoreum Yun