From: Mark Rutland <mark.rutland@arm.com>
To: James Clark <james.clark@linaro.org>
Cc: Will Deacon <will@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Anshuman Khandual <Anshuman.Khandual@arm.com>,
Rob Herring <Rob.Herring@arm.com>,
Suzuki Poulose <Suzuki.Poulose@arm.com>,
Robin Murphy <Robin.Murphy@arm.com>, Leo Yan <leo.yan@arm.com>,
linux-arm-kernel@lists.infradead.org,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
Michael Williams <Michael.Williams@arm.com>,
Alexandru Elisei <Alexandru.Elisei@arm.com>
Subject: Re: [PATCH] perf: arm_spe: Add barrier before enabling profiling buffer
Date: Thu, 19 Feb 2026 14:30:41 +0000 [thread overview]
Message-ID: <aZcekSK5fGEkw_1P@J2N7QTR9R3.cambridge.arm.com> (raw)
In-Reply-To: <2d5f6207-afa2-4880-b34d-f03b5ebf3b5f@linaro.org>
On Thu, Feb 19, 2026 at 02:15:22PM +0000, James Clark wrote:
> On 19/02/2026 2:03 pm, Will Deacon wrote:
> > On Thu, Feb 19, 2026 at 01:51:26PM +0000, James Clark wrote:
> > > On 19/02/2026 12:57 pm, Will Deacon wrote:
> > > > On Thu, Feb 19, 2026 at 12:08:27PM +0000, James Clark wrote:
> > > > > I'm back to drag this up again. So I think all of the above discussion
> > > > > relies on the ordering given by the indirect read needed for the "might
> > > > > ignore a direct write..." part. But it's _might_ ignore a direct write, it's
> > > > > possible for an implementation to not do that, so there are two possible
> > > > > implementations:
> > > > >
> > > > > #1 Where there is an indirect read to give the write ignore outcome
> > > > > #2 Where there is no write ignore outcome so it doesn't require an
> > > > > indirect read
> > > > >
> > > > > For #2 there's nothing to force the ordering. We're writing to two different
> > > > > registers (PMBPTR_EL1 and PMBLIMITR_EL1) and we have to have the
> > > > > PMBLIMITR_EL1 write come second for the buffer to be considered configured
> > > > > correctly. For example if the old value of PMBPTR_EL1 is higher than the new
> > > > > PMBLIMITR_EL1 and the write to PMBLIMITR_EL1 happens first then it's
> > > > > misconfigured. That's why we think we need the isb() here.
> > > >
> > > > I thought profiling was disabled in these cases, so why is it
> > > > misconfigured?
> > > >
> > > > If it is misconfigured, what can go wrong given that we're either stopped
> > > > or pmscr is clear?
> > > >
> > > > Will
> > >
> > > It is stopped in the interrupt handler because PMBSR_EL1.S = 1. But in
> > > arm_spe_pmu_start(), PMBSR_EL1.S = 0 so it's not stopped. And with
> > > PMBPTR_EL1 still being set to the value from the last session it could be
> > > higher than PMBLIMITR_EL1.
> > >
> > > PMSCR_EL1 doesn't affect SPEProfilingStopped(), it's only:
> > >
> > > boolean stopped = (PMBSR_EL1.S == '1');
> > >
> > > The conditions for when the buffer needs to be configured correctly from
> > > R24557 are:
> > >
> > > ProfilingBufferEnabled() && !SPEProfilingStopped() &&
> > > PMBLIMITR_EL1.FM != DISCARD
> > >
> > > I think even if PMSCR_EL1 is clear you can still get a buffer management
> > > error, even if no samples were going to be written into the buffer. It just
> > > says:
> > >
> > > 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.
> >
> > ... but doesn't that mean that R24557 is a breaking change to the
> > architecture? The current Arm ARM doesn't appear to require this,
> > existing software doesn't honour it so why should we hack extra barriers
> > into Linux?
> >
> > Will
>
> Yes I suppose it is. The current Arm ARM doesn't require it, but R24577 is
> in the "known issues" document for the Arm ARM, so it's saying the ARM is
> incorrect here and you shouldn't trust what it says.
>
> Arm Architecture Reference Manual for A-profile architecture: Known
> issues
>
> This document includes the Known Issues for the following documents:
>
> Arm Architecture Reference Manual for A-profile architecture
> (DDI0487)
>
> Presumably the fix will make it into the Arm ARM eventually.
While that document is called "Known issues" it really means "Key
changes since the last ARM ARM". The items in that are not all bug
fixes.
In this case, R24577 is intended to be a relaxation (that's what the "R"
stands for), which breaks existing widely-deployed SW. It fails to meet
the usual acceptability requirements for a relaxation.
IMO this should not have made it into the architecture as-is.
> I suppose there is a chance that this could be deleted from the known issues
> and not make it into a future Arm ARM. TBH I have no experience or feeling
> to say how likely that would be.
Broken changes have made it into the architecture before, and have been
subsequently reverted. In general, we try to avoid that in the first
place.
I will raise this internally within Arm.
Mark.
prev parent reply other threads:[~2026-02-19 14:30 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-23 16:03 James Clark
2026-01-30 20:24 ` Leo Yan
2026-02-02 16:53 ` Will Deacon
2026-02-02 18:42 ` Leo Yan
2026-02-02 18:57 ` Will Deacon
2026-02-02 19:14 ` Leo Yan
2026-02-03 9:29 ` James Clark
2026-02-03 9:32 ` Will Deacon
2026-02-02 19:03 ` Will Deacon
2026-02-03 10:46 ` James Clark
2026-02-03 11:07 ` Will Deacon
2026-02-06 9:50 ` James Clark
2026-02-19 12:08 ` James Clark
2026-02-19 12:57 ` Will Deacon
2026-02-19 13:51 ` James Clark
2026-02-19 14:03 ` Will Deacon
2026-02-19 14:15 ` James Clark
2026-02-19 14:30 ` Mark Rutland [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aZcekSK5fGEkw_1P@J2N7QTR9R3.cambridge.arm.com \
--to=mark.rutland@arm.com \
--cc=Alexandru.Elisei@arm.com \
--cc=Anshuman.Khandual@arm.com \
--cc=Michael.Williams@arm.com \
--cc=Rob.Herring@arm.com \
--cc=Robin.Murphy@arm.com \
--cc=Suzuki.Poulose@arm.com \
--cc=catalin.marinas@arm.com \
--cc=james.clark@linaro.org \
--cc=leo.yan@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®