[PATCH v4 09/15] coresight: Save activated path into source device

[Suzuki K Poulose]

On 04/11/2025 15:21, Leo Yan wrote:
> Save activated path into the source device's coresight_device structure.
> The path pointer will be used by later changes for controlling the path
> during CPU idle.
> 
> The path pointer is assigned before setting the source device mode to
> active, and it is cleared after the device is changed to an inactive
> mode. So safe access to path pointers is guaranteed when the device is
> in an active mode.
> 
> Signed-off-by: Leo Yan <leo.yan at arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 39 +++++++++++++++++++++++++++-
>   include/linux/coresight.h                    |  2 ++
>   2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c28f3d255b0e19b3d982de95b8e34d0fc2954b95..3ea31ed121f7b59d7822fba4df4c43efb1c76fe7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -399,13 +399,50 @@ int coresight_enable_source(struct coresight_device *csdev,
>   			    struct perf_event *event, enum cs_mode mode,
>   			    struct coresight_path *path)
>   {
> -	return source_ops(csdev)->enable(csdev, event, mode, path);
> +	int ret;
> +
> +	/*
> +	 * Record the path in the source device. The path pointer is first
> +	 * assigned, followed by transitioning from DISABLED mode to an enabled
> +	 * state on the target CPU. Conversely, during the disable flow, the
> +	 * device mode is set to DISABLED before the path pointer is cleared.
> +	 *
> +	 * This ordering ensures the path pointer to be safely access under the
> +	 * following race condition:
> +	 *
> +	 *  CPU(a)                          CPU(b)
> +	 *
> +	 *  coresight_enable_source()
> +	 *    STORE source->path;
> +	 *    smp_mb();
> +	 *    source_ops(csdev)->enable();
> +	 *                              `-> etm4_enable_sysfs_smp_call()
> +	 *                                    STORE source->mode;
> +	 *
> +	 * This sequence ensures that accessing the path pointer is safe when
> +	 * the device is in enabled mode.
> +	 */
> +	csdev->path = path;
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +
> +	ret = source_ops(csdev)->enable(csdev, event, mode, path);

What happens if the csdev is already enabled ? We corrupt the 
csdev->path ? Can we not move this into the source devices and let them
handle it gracefully, as they do for the "mode" ?

To address James' comment on reusing the sysfs path, I think we could
make this a global thing for all types of source/mode.

Suzuki


> +	if (ret)
> +		csdev->path = NULL;
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(coresight_enable_source);
>   
>   void coresight_disable_source(struct coresight_device *csdev, void *data)
>   {
>   	source_ops(csdev)->disable(csdev, data);
> +
> +	/* Synchronization between csdev->path and csdev->mode */
> +	smp_mb();
> +	csdev->path = NULL;
> +
>   	coresight_disable_helpers(csdev, NULL);
>   }
>   EXPORT_SYMBOL_GPL(coresight_disable_source);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 3d59be214dd25dfa7ad9148a6688628e0d1a98dd..58484c225e58a68dd74739a48c08a409ce9ddd73 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -264,6 +264,7 @@ struct coresight_trace_id_map {
>    *		spinlock.
>    * @orphan:	true if the component has connections that haven't been linked.
>    * @cpu:	The CPU this component is affined to (-1 for not CPU bound).
> + * @path:	Activated path pointer (only used for source device).
>    * @sysfs_sink_activated: 'true' when a sink has been selected for use via sysfs
>    *		by writing a 1 to the 'enable_sink' file.  A sink can be
>    *		activated but not yet enabled.  Enabling for a _sink_ happens
> @@ -291,6 +292,7 @@ struct coresight_device {
>   	int refcnt;
>   	bool orphan;
>   	int cpu;
> +	struct coresight_path *path;
>   	/* sink specific fields */
>   	bool sysfs_sink_activated;
>   	struct dev_ext_attribute *ea;
>