Re: [PATCH v2 21/23] coresight: trbe: Read TRBSR after prohibiting and flushing trace

[James Clark <james.clark@linaro.org>]

On 16/10/2026 17:55, Leo Yan wrote:
> The IRQ handler needs a stable TRBSR_EL1 value before classifying the
> interrupt. If the PE can still generate trace, or if pending trace has
> not been flushed, TRBSR can still change while the handler is sampling
> it. This is especially important for TRBSR.S bit, which is used to
> decide whether the TRBE has stopped.
> 
> Move the drain buffer into arm_trbe_prohibit_trace(), so the handler
> first prohibits trace and flushes trace before reading TRBSR_EL1. Once
> this has been done, the IRQ path no longer needs the unconditional
> trbe_drain_and_disable_local(); it only needs to disable TRBE before
> handling the stopped-buffer status.
> 
> Refactor the IRQ exit path accordingly: restore TRFCR on non-truncated
> exits, while the truncated path still runs the perf IRQ work to shut
> down the event.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 40 +++++++++++++++++++---------
>   1 file changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index c83c440d4ac13c346bc2a7f9093951c186c90157..36f143a15e680b28aa6ca6a43ad7d3ee134693be 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -1207,24 +1207,34 @@ static bool is_perf_trbe(struct perf_output_handle *handle)
>   	return true;
>   }
>   
> -static u64 cpu_prohibit_trace(void)
> +static u64 arm_trbe_prohibit_trace(void)
>   {
>   	u64 trfcr = read_trfcr();
>   
>   	/* Prohibit tracing at EL0 & the kernel EL */
>   	write_trfcr(trfcr & ~(TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE));
> +
> +	/* Ensure the program-flow trace is flushed */
> +	trbe_drain_buffer();
> +
>   	/* Return the original value of the TRFCR */
>   	return trfcr;
>   }
>   
> +static void arm_trbe_allow_trace(u64 trfcr)
> +{
> +	/* Allow tracing at EL0 & the kernel EL */
> +	write_trfcr(trfcr);
> +}
> +
>   static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>   {
>   	struct trbe_cpudata *cpudata = dev;
>   	struct perf_output_handle *handle = cpudata->handle;
>   	enum trbe_fault_action act;
> -	u64 status;
>   	bool truncated = false;
> -	u64 trfcr;
> +	u64 status, trfcr;
> +	irqreturn_t ret;
>   
>   	if (!handle || !perf_get_aux(handle))
>   		return IRQ_NONE;
> @@ -1232,22 +1242,25 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>   	if (!is_perf_trbe(handle))
>   		return IRQ_NONE;
>   
> +	/* Prohibit the CPU from tracing before we disable the TRBE */
> +	trfcr = arm_trbe_prohibit_trace();
> +
>   	/* Reads to TRBSR_EL1 is fine when TRBE is active */
>   	status = read_sysreg_s(SYS_TRBSR_EL1);

I think sashiko is right, you need an isb() between 
arm_trbe_prohibit_trace() and the status read, otherwise it can still be 
stale.

>   	/*
>   	 * If the pending IRQ was handled by update_buffer callback
>   	 * we have nothing to do here.
>   	 */
> -	if (!is_trbe_irq(status))
> -		return IRQ_NONE;
> +	if (!is_trbe_irq(status)) {
> +		ret = IRQ_NONE;
> +		goto out;
> +	}
>   
> -	/* Prohibit the CPU from tracing before we disable the TRBE */
> -	trfcr = cpu_prohibit_trace();
>   	/*
>   	 * Ensure the trace is visible to the CPUs and
>   	 * any external aborts have been resolved.
>   	 */
> -	trbe_drain_and_disable_local(cpudata);
> +	set_trbe_disabled(cpudata);
>   	clr_trbe_irq();
>   	isb();
>   
> @@ -1272,12 +1285,15 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev)
>   	 * Otherwise, restore the trace filter controls to
>   	 * allow the tracing.
>   	 */
> -	if (truncated)
> +	if (truncated) {
>   		irq_work_run();
> -	else
> -		write_trfcr(trfcr);
> +		return IRQ_HANDLED;
> +	}
>   
> -	return IRQ_HANDLED;
> +	ret = IRQ_HANDLED;
> +out:
> +	arm_trbe_allow_trace(trfcr);
> +	return ret;
>   }
>   
>   static int arm_trbe_save(struct coresight_device *csdev)
>