linux-kernel - Re: [PATCH] perf: arm_spe: Add barrier before enabling profiling 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: <95e205af-3dec-48c7-8a0d-293629f1551b@linaro.org>
Date: Tue, 3 Feb 2026 09:29:56 +0000
From: James Clark <james.clark@...aro.org>
To: Leo Yan <leo.yan@....com>, Will Deacon <will@...nel.org>
Cc: Mark Rutland <mark.rutland@....com>,
 Catalin Marinas <catalin.marinas@....com>,
 Alexandru Elisei <Alexandru.Elisei@....com>,
 Anshuman Khandual <Anshuman.Khandual@....com>,
 Rob Herring <Rob.Herring@....com>, Suzuki Poulose <Suzuki.Poulose@....com>,
 Robin Murphy <Robin.Murphy@....com>, linux-arm-kernel@...ts.infradead.org,
 linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf: arm_spe: Add barrier before enabling profiling
 buffer



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 Proﬁling Buﬀer is enabled, proﬁling 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.