linux-kernel - Re: [PATCH v2 5/8] coresight: etm: Add an attribute for updating buffer

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: <d8b1cf1f-1996-4d9c-9f1a-fad556f91577@arm.com>
Date: Mon, 10 Mar 2025 13:29:26 +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 5/8] coresight: etm: Add an attribute for updating
 buffer

Hi Leo

On 10/03/2025 10:49, Leo Yan wrote:
> Add an attribute for updating buffer when the AUX trace is paused.  And
> populate the value to the 'update_buf_on_pause' flag during the AUX
> setting up.

Do we need this attribute in the uAPI ? Could we do this by default for
sinks without interrupt ? This definitely improves the quality of trace
collected for such sinks and the driver can transparently do this.

Cheers
Suzuki

> 
> If the AUX pause operation is attached to a PMU counter, when the
> counter is overflow and if the PMU interrupt in an NMI, then AUX pause
> operation will be triggered in the NMI context.  On the other hand, the
> per CPU sink has its own interrupt handling.  Thus, there will be a race
> condition between the updating buffer in NMI and sink's interrupt
> handler.
> 
> To avoid the race condition, this commit disallows updating buffer on
> AUX pause for the per CPU sink.  Currently, this is only applied for
> TRBE.
> 
> Signed-off-by: Leo Yan <leo.yan@....com>
> ---
>   .../hwtracing/coresight/coresight-etm-perf.c  | 20 +++++++++++++++++++
>   .../hwtracing/coresight/coresight-etm-perf.h  |  2 ++
>   include/linux/coresight-pmu.h                 |  1 +
>   3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 29d52386ffbb..d759663a1f7d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -62,6 +62,8 @@ PMU_FORMAT_ATTR(contextid1,	"config:" __stringify(ETM_OPT_CTXTID));
>   PMU_FORMAT_ATTR(contextid2,	"config:" __stringify(ETM_OPT_CTXTID2));
>   PMU_FORMAT_ATTR(timestamp,	"config:" __stringify(ETM_OPT_TS));
>   PMU_FORMAT_ATTR(retstack,	"config:" __stringify(ETM_OPT_RETSTK));
> +PMU_FORMAT_ATTR(update_buf_on_pause,
> +		"config:" __stringify(ETM_OPT_UPDATE_BUF_ON_PAUSE));
>   /* preset - if sink ID is used as a configuration selector */
>   PMU_FORMAT_ATTR(preset,		"config:0-3");
>   /* Sink ID - same for all ETMs */
> @@ -103,6 +105,7 @@ static struct attribute *etm_config_formats_attr[] = {
>   	&format_attr_configid.attr,
>   	&format_attr_branch_broadcast.attr,
>   	&format_attr_cc_threshold.attr,
> +	&format_attr_update_buf_on_pause.attr,
>   	NULL,
>   };
>   
> @@ -434,6 +437,23 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
>   	if (!sink)
>   		goto err;
>   
> +	/* Populate the flag for updating buffer on AUX pause */
> +	event_data->update_buf_on_pause =
> +		!!(event->attr.config & BIT(ETM_OPT_UPDATE_BUF_ON_PAUSE));
> +
> +	if (event_data->update_buf_on_pause) {
> +		/*
> +		 * The per CPU sink has own interrupt handling, it might have
> +		 * race condition with updating buffer on AUX trace pause if
> +		 * it is invoked from NMI.  To avoid the race condition,
> +		 * disallows updating buffer for the per CPU sink case.
> +		 */
> +		if (coresight_is_percpu_sink(sink)) {
> +			dev_err(&sink->dev, "update_buf_on_pause is not permitted.\n");
> +			goto err;
> +		}
> +	}
> +
>   	/* If we don't have any CPUs ready for tracing, abort */
>   	cpu = cpumask_first(mask);
>   	if (cpu >= nr_cpu_ids)
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
> index 744531158d6b..52b9385f8c11 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.h
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
> @@ -51,6 +51,7 @@ struct etm_filters {
>    * @aux_hwid_done:	Whether a CPU has emitted the TraceID packet or not.
>    * @snk_config:		The sink configuration.
>    * @cfg_hash:		The hash id of any coresight config selected.
> + * @update_buf_on_pause: The flag to indicate updating buffer on AUX pause.
>    * @path:		An array of path, each slot for one CPU.
>    */
>   struct etm_event_data {
> @@ -59,6 +60,7 @@ struct etm_event_data {
>   	cpumask_t aux_hwid_done;
>   	void *snk_config;
>   	u32 cfg_hash;
> +	bool update_buf_on_pause;
>   	struct list_head * __percpu *path;
>   };
>   
> diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h
> index 89b0ac0014b0..04147e30c2f2 100644
> --- a/include/linux/coresight-pmu.h
> +++ b/include/linux/coresight-pmu.h
> @@ -35,6 +35,7 @@
>   #define ETM_OPT_CTXTID2		15
>   #define ETM_OPT_TS		28
>   #define ETM_OPT_RETSTK		29
> +#define ETM_OPT_UPDATE_BUF_ON_PAUSE	30
>   
>   /* ETMv4 CONFIGR programming bits for the ETM OPTs */
>   #define ETM4_CFG_BIT_BB         3