Re: [PATCH v8 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration

[Leo Yan <leo.yan@arm.com>]

On Mon, Jun 29, 2026 at 10:00:01AM +0100, Yeoreum Yun wrote:

[...]

> @@ -922,25 +950,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
>  {
>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>  	struct etm4_enable_arg arg = { };
> -	unsigned long cfg_hash;
> -	int ret, preset;
> -
> -	/* enable any config activated by configfs */
> -	cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset);
> -	if (cfg_hash) {
> -		ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset);
> -		if (ret) {
> -			etm4_release_trace_id(drvdata);
> -			return ret;
> -		}
> -	}
> -
> -	raw_spin_lock(&drvdata->spinlock);
> -
> -	drvdata->trcid = path->trace_id;
> -
> -	/* Tracer will never be paused in sysfs mode */
> -	drvdata->paused = false;
> +	int ret;
>  
>  	/*
>  	 * Executing etm4_enable_hw on the cpu whose ETM is being enabled
> @@ -948,20 +958,20 @@ static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_pa
>  	 */
>  	arg.drvdata = drvdata;
>  	arg.path = path;
> +
> +	raw_spin_lock(&drvdata->spinlock);
> +	arg.config = drvdata->config;
> +	raw_spin_unlock(&drvdata->spinlock);
> +
>  	ret = smp_call_function_single(drvdata->cpu,
>  				       etm4_enable_sysfs_smp_call, &arg, 1);
>  	if (!ret)
>  		ret = arg.rc;
>  	if (!ret)
> -		drvdata->sticky_enable = true;
> -
> -	if (ret)
> +		dev_dbg(&csdev->dev, "ETM tracing enabled\n");
> +	else
>  		etm4_release_trace_id(drvdata);
>  
> -	raw_spin_unlock(&drvdata->spinlock);
> -
> -	if (!ret)
> -		dev_dbg(&csdev->dev, "ETM tracing enabled\n");
>  	return ret;
>  }

This is most valuable change for me, as now we will have much clear
scope for what is protected ("drvdata->config").

However, a corner case was mentioned by Sashiko:

 | It appears etm4_enable_hw() modifies drvdata->ss_status while executing
 | via IPI, but sshot_ctrl_store() can modify the same array concurrently
 | from process context since the lock is no longer held across the
 | smp_call_function_single() call.

"drvdata->ss_status" is a race condition between SMP call and sysfs
knobs. Should we change to spinlock_irqsave/irqrestore when access
drvdata->ss_status?

Thanks,
Leo