linux-kernel - Re: [PATCH v2 2/8] coresight: Introduce pause and resume APIs for source

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <65979e92-0047-427c-848d-53f908671905@arm.com>
Date: Mon, 10 Mar 2025 12:01:25 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Leo Yan <leo.yan@....com>, Mike Leach <mike.leach@...aro.org>,
 James Clark <james.clark@...aro.org>, Jonathan Corbet <corbet@....net>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Arnaldo Carvalho de Melo <acme@...hat.com>,
 Namhyung Kim <namhyung@...nel.org>, coresight@...ts.linaro.org,
 linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/8] coresight: Introduce pause and resume APIs for
 source

On 10/03/2025 10:49, Leo Yan wrote:
> Introduce APIs for pausing and resuming trace source and export as GPL
> symbols.
> 
> Signed-off-by: Leo Yan <leo.yan@....com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 20 ++++++++++++++++++++
>   drivers/hwtracing/coresight/coresight-priv.h |  2 ++
>   include/linux/coresight.h                    |  4 ++++
>   3 files changed, 26 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 0a9380350fb5..eb7b83a7bfa2 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -365,6 +365,26 @@ void coresight_disable_source(struct coresight_device *csdev, void *data)
>   }
>   EXPORT_SYMBOL_GPL(coresight_disable_source);
>   
> +void coresight_pause_source(struct coresight_device *csdev)
> +{
> +	if (!csdev || !coresight_is_percpu_source(csdev))

You may remove the !csdev check, coresight_is_percpu_source() already 
checks that.


> +		return;
> +
> +	if (source_ops(csdev)->pause)
> +		source_ops(csdev)->pause(csdev);
> +}
> +EXPORT_SYMBOL_GPL(coresight_pause_source);
> +
> +void coresight_resume_source(struct coresight_device *csdev)
> +{
> +	if (!csdev || !coresight_is_percpu_source(csdev))
> +		return;
> +

Same as above.

> +	if (source_ops(csdev)->resume)
> +		source_ops(csdev)->resume(csdev);
> +}
> +EXPORT_SYMBOL_GPL(coresight_resume_source);
> +
>   /*
>    * coresight_disable_path_from : Disable components in the given path beyond
>    * @nd in the list. If @nd is NULL, all the components, except the SOURCE are
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 76403530f33e..a9f14c075e91 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -247,5 +247,7 @@ void coresight_add_helper(struct coresight_device *csdev,
>   void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev);
>   struct coresight_device *coresight_get_percpu_sink(int cpu);
>   void coresight_disable_source(struct coresight_device *csdev, void *data);
> +void coresight_pause_source(struct coresight_device *csdev);
> +void coresight_resume_source(struct coresight_device *csdev);
>   
>   #endif
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 17276965ff1d..703e1b8dbe22 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -385,6 +385,8 @@ struct coresight_ops_link {
>    *		is associated to.
>    * @enable:	enables tracing for a source.
>    * @disable:	disables tracing for a source.
> + * @resume:	resumes tracing for a source.
> + * @pause:	pauses tracing for a source.
>    */
>   struct coresight_ops_source {
>   	int (*cpu_id)(struct coresight_device *csdev);
> @@ -392,6 +394,8 @@ struct coresight_ops_source {
>   		      enum cs_mode mode, struct coresight_trace_id_map *id_map);
>   	void (*disable)(struct coresight_device *csdev,
>   			struct perf_event *event);
> +	int (*resume)(struct coresight_device *csdev);
> +	void (*pause)(struct coresight_device *csdev);

Given these callbacks are for perf mode, does it make sense  to pass the 
parameters similar to enable() ? Also, should we rename these as :

resume_perf
pause_perf

To explicitly mark them perf mode specific ?

Suzuki

>   };
>   
>   /**