linux-kernel - Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

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: <e315b489-2ebf-4ab6-8a60-347f1b1dca16@arm.com>
Date: Tue, 2 Dec 2025 11:15:47 +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>,
 Anshuman Khandual <anshuman.khandual@....com>,
 Yeoreum Yun <yeoreum.yun@....com>, Will Deacon <will@...nel.org>,
 Mark Rutland <mark.rutland@....com>, Tamas Petz <tamas.petz@....com>,
 Tamas Zsoldos <tamas.zsoldos@....com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
 Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

On 01/12/2025 11:21, Leo Yan wrote:
> Rather than spreading AUX flag setting in different functions, use
> trbe_get_fault_act() as a central place for setting the flag.
> 
> Later we will support WRAP mode with continuous trace, so the WRAP
> event does not necessarily cause the trace discontinuity, change to
> check the stop status instead.
> 
> Signed-off-by: Leo Yan <leo.yan@....com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 38 +++++++++++++---------------
>   1 file changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 28e2bfa68074f19ccaa4a737d00af577aea818fe..b06885a08e082fd34f68d9588518807b5c47c86e 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -265,25 +265,6 @@ static void trbe_reset_local(struct trbe_cpudata *cpudata)
>   	write_sysreg_s(0, SYS_TRBSR_EL1);
>   }
>   
> -static void trbe_report_wrap_event(struct perf_output_handle *handle)
> -{
> -	/*
> -	 * Mark the buffer to indicate that there was a WRAP event by
> -	 * setting the COLLISION flag. This indicates to the user that
> -	 * the TRBE trace collection was stopped without stopping the
> -	 * ETE and thus there might be some amount of trace that was
> -	 * lost between the time the WRAP was detected and the IRQ
> -	 * was consumed by the CPU.
> -	 *
> -	 * Setting the TRUNCATED flag would move the event to STOPPED
> -	 * state unnecessarily, even when there is space left in the
> -	 * ring buffer. Using the COLLISION flag doesn't have this side
> -	 * effect. We only set TRUNCATED flag when there is no space
> -	 * left in the ring buffer.
> -	 */
> -	perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
> -}
> -
>   static void trbe_truncate_event(struct perf_output_handle *handle)
>   {
>   	struct trbe_buf *buf = etm_perf_sink_config(handle);
> @@ -687,6 +668,23 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
>   		goto out_fatal;
>   	}
>   
> +	/*
> +	 * Mark the buffer to indicate that there was a WRAP event by
> +	 * setting the COLLISION flag. This indicates to the user that
> +	 * the TRBE trace collection was stopped without stopping the
> +	 * ETE and thus there might be some amount of trace that was
> +	 * lost between the time the WRAP was detected and the IRQ
> +	 * was consumed by the CPU.
> +	 *
> +	 * Setting the TRUNCATED flag would move the event to STOPPED
> +	 * state unnecessarily, even when there is space left in the
> +	 * ring buffer. Using the COLLISION flag doesn't have this side
> +	 * effect. We only set TRUNCATED flag when there is no space
> +	 * left in the ring buffer.
> +	 */
> +	if (!is_trbe_running(trbsr))

Is this really required ? There is a WARN_ON(is_trbe_running(trbsr))
above ?

> +		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);

Also, setting this would unnecessarily mark COLLISION while stopping the
TRBE buffer normally (when called from arm_trbe_update_buffer), when
there is no WRAP event ?


> +
>   	if (is_trbe_wrap(trbsr))
>   		return TRBE_FAULT_ACT_WRAP;

Couldn't this be :

	/* Move the big fat comment here */
	if (is_trbe_wrap(trbsr)) {
		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
		return TRBE_FAULT_ACT_WRAP;
	}

Suzuki