[PATCH] perf: arm_spe: Add barrier before enabling profiling buffer

[James Clark]

On 02/02/2026 7:14 pm, Leo Yan wrote:
> On Mon, Feb 02, 2026 at 06:57:11PM +0000, Will Deacon wrote:
> 
> [...]
> 
> 
>>>> I'm not sure I follow your logic as to why both ISBs are required, but
>>>> I'd have thought that if perf_aux_output_begin() fails when called from
>>>> arm_spe_perf_aux_output_begin() in the irqhandler, we need the ISB
>>>> because we're going to clear pmblimitr_el1 to 0 and that surely has
>>>> to be ordered before clearing pmbsr?
>>>
>>> I think the ISB after arm_spe_perf_aux_output_begin() in the irq
>>> handler is required for both the failure and success cases.
>>>
>>> For a normal maintenance interrupt, an ISB is inserted between writing
>>> PMBLIMITR_EL1 and PMBSR_EL1 to ensure that a valid limit write is
>>> visible before tracing restarts.  This ensures that the following
>>> conditions are safely met:
>>>
>>>    "While the Profiling Buffer is enabled, profiling is not stopped, and
>>>     Discard mode is not enabled, all of the following must be true:
>>>
>>>     The current write pointer must be at least one sample record below
>>>     the write limit pointer.
>>>
>>>     PMBPTR_EL1.PTR[63:56] must equal PMBLIMITR_EL1.LIMIT[63:56],
>>>     regardless of the value of the applicable TBI bit."
>>
>> Hmm, so let's say we've executed the first ISB. At that point, the
>> Profiling Buffer is disabled (PMBLIMITR_EL1.E = 0) and profiling is
>> stopped (PMBSR_EL1.S = 1).
> 
> This is not true.  PMBLIMITR_EL1.E is always 1 during interrupt
> handling.
> 
>> If we *don't* have the second ISB then either
>> PMBLIMITR_EL1 is written first or PMBSR_EL1 is written first. But the
>> text you quoted will only come into effect once they've both happened,
>> right? In which case, why does the order matter for the success case?
> 
> Yes, both PMBLIMITR_EL1.E == 1 and PMBSR_EL1.S == 0 must be true to
> enable tracing.
> 
> However, the tricky part is that PMBLIMITR_EL1.E remains 1 throughout
> the sequence.  Writing PMBLIMITR_EL1 effectively only sets the limit,
> while clearing PMBSR_EL1 is the distinct step that enables tracing.
> 
> Thanks,
> Leo

I think Leo is correct that the old isb() is still needed. I removed it 
under the assumption that PMBLIMITR_EL1.E was unset in the interrupt 
handler. Possibly because the previous version re-arranged the handler 
to do that.

If PMBLIMITR_EL1.E is set, we have to make sure clearing PMBSR_EL1 comes 
last as it's the thing that defines the point where both pointers must 
be correct by.