On Mon, Feb 02, 2026 at 04:53:57PM +0000, Will Deacon wrote:

[...]

quoted

quoted

To avoid redundant isb()s in the IRQ handler, remove the isb() between
the PMBLIMITR_EL1 write and SYS_PMBSR_EL1 as it doesn't matter which
order these happen in now that all the previous configuration is covered
by the new isb().

The isb() in the interrupt handler is useful and should not be removed.

See the sequence in the interrupt handler:

    arm_spe_perf_aux_output_begin() {
        write_sysreg_s(base, SYS_PMBPTR_EL1);

        // Ensure the write pointer is ordered
        isb();

        write_sysreg_s(limit, SYS_PMBLIMITR_EL1);
    }

    // Ensure the limit pointer is ordered
    isb();

    // Profiling is enabled:
    //   (PMBLIMITR_EL1.E==1) && (PMBSR_EL1.S==0) && (not discard mode)
    write_sysreg_s(0, SYS_PMBSR_EL1);

The first isb() ensures that the write pointer update is ordered.  The
second isb() ensures that the limit pointer is visible before profiling
is enabled by clearing the PMBSR_EL1.S bit.  When handling a normal
maintenance interrupt, PMBSR_EL1.S is set by the SPE to stop tracing,
while PMBLIMITR_EL1.E remains set.  Clearing PMBSR_EL1.S therefore
effectively re-enables profiling.

Since the second isb() is a synchronization for both the write pointer
and the limit pointer before profiling is enabled, it could be argued
that the first isb() is redundant in the interrupt handling.  However,
the first isb() is crucial for the arm_spe_pmu_start() case, and keeping
it in the interrupt handler does no harm and simplifies code maintenance.

In short, if preserves the second isb() instead of removing it, the
change looks good to me.

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."

Thanks,
Leo