linux-kernel - Re: [PATCH] perf: arm_spe: Ensure trace is disabled when TRUNCATED flag is set

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: <aQi44CZOiwhI19dd@willie-the-truck>
Date: Mon, 3 Nov 2025 14:14:56 +0000
From: Will Deacon <will@...nel.org>
To: Leo Yan <leo.yan@....com>
Cc: Mark Rutland <mark.rutland@....com>,
	Alexandru Elisei <alexandru.elisei@....com>,
	James Clark <james.clark@...aro.org>,
	linux-arm-kernel@...ts.infradead.org,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf: arm_spe: Ensure trace is disabled when TRUNCATED
 flag is set

On Wed, Oct 15, 2025 at 07:23:37PM +0100, Leo Yan wrote:
> The TRUNCATED flag can be set in the following cases:
> 
>  1. When data loss is detected (PMBSR_EL1.DL == 0b1).
>  2. When arm_spe_perf_aux_output_begin() fails to find a valid limit
>     because it runs out of free space.
> 
> Currently, only the first case invokes irq_work_run() to execute the
> event disable callback, while the second case does not.
> 
> Move the call to irq_work_run() later with checking the TRUNCATED flag,
> so that both cases are handled and redundant calls are avoided.
> 
> Signed-off-by: Leo Yan <leo.yan@....com>
> ---
>  drivers/perf/arm_spe_pmu.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> index fa50645feddadbea5dc1e404f80f62cf5aa96fd4..05ba977f75d607a1d524f68694db9ea18a6ef20c 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
> @@ -731,12 +731,6 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
>  	if (act == SPE_PMU_BUF_FAULT_ACT_SPURIOUS)
>  		return IRQ_NONE;
>  
> -	/*
> -	 * Ensure perf callbacks have completed, which may disable the
> -	 * profiling buffer in response to a TRUNCATION flag.
> -	 */
> -	irq_work_run();
> -
>  	switch (act) {
>  	case SPE_PMU_BUF_FAULT_ACT_FATAL:
>  		/*
> @@ -765,6 +759,15 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
>  		break;
>  	}
>  
> +	/*
> +	 * The TRUNCATED flag is set when data loss is detected by PMBSR_EL1.DL,
> +	 * or arm_spe_perf_aux_output_begin() sets the flag if runs out of free
> +	 * space. Ensure that all perf callbacks have completed for disabling
> +	 * the profiling buffer.
> +	 */
> +	if (handle->aux_flags & PERF_AUX_FLAG_TRUNCATED)
> +		irq_work_run();

I'm not sure about this. afaict, perf_aux_output_begin() can fail
(return NULL) without setting the TRUNCATED flag but with a disable
pending in irq work. It also feels a little fragile moving the existing
irq_work_run() call as that could easily break in future if e.g. irq
work is used for other manipulation of the buffer controls.

Given that this isn't a fast path, isn't it simpler and safer just to
do something like the below?

Will

--->8

diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index fa50645fedda..8a38d69e9600 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -758,6 +758,10 @@ static irqreturn_t arm_spe_pmu_irq_handler(int irq, void *dev)
                if (!(handle->aux_flags & PERF_AUX_FLAG_TRUNCATED)) {
                        arm_spe_perf_aux_output_begin(handle, event);
                        isb();
+
+                       /* Insightful comment here */
+                       if (event->hw.state)
+                               irq_work_run();
                }
                break;
        case SPE_PMU_BUF_FAULT_ACT_SPURIOUS: