From: Mark Rutland <mark.rutland@arm•com>
To: Mark Brown <broonie@kernel•org>
Cc: linux-arm-kernel@lists•infradead.org, kvmarm@lists•linux.dev,
catalin.marinas@arm•com, james.morse@arm•com, maz@kernel•org,
oupton@kernel•org, tabba@google•com, will@kernel•org
Subject: Re: [PATCH 11/18] arm64: fpsimd: Split FPSR/FPCR from SVE save/restore
Date: Wed, 27 May 2026 14:51:13 +0100 [thread overview]
Message-ID: <ahb20fXkdWI7SljE@J2N7QTR9R3.cambridge.arm.com> (raw)
In-Reply-To: <0b72df66-2914-4537-811e-2080e0a7ce6c@sirena.org.uk>
On Tue, May 26, 2026 at 05:28:21PM +0100, Mark Brown wrote:
> On Thu, May 21, 2026 at 02:25:49PM +0100, Mark Rutland wrote:
> > Regardless of whether the vector registers are saved in FPSIMD or SVE
> > format, we store FPSR and FPCR in user_fpsimd_state::{fpsr,fpcr}.
>
> ...
>
> > Note that the SVE assembly sequence for restoring FPCR uses an
> > unconditional write to FPCR. The plain FPSIMD assembly sequence has used
> > a conditional write to FPCR since 2014 in commit:
>
> > 5959e25729a5 ("arm64: fpsimd: avoid restoring fpcr if the contents haven't change")
>
> > ... but this was not followed for the SVE restore assembly implemented
> > in 2017 in commit:
>
> > 1fc5dce78ad1 ("arm64/sve: Low-level SVE architectural state manipulation functions")
>
> > ... so I've assumed that this doesn't actually matter in practice, and
> > implemented the C version matching the existing SVE assembly.
>
> > For the moment, fpsimd_save_state() and fpsimd_load_state() are left
> > as-is with their own logic to save/restore FPSR and FPCR. This will be
> > unified in subsequent patches.
>
> There is a possibility that it only matters for older, FPSIMD only CPUs
> or just that nobody got round to benchmarking this on physical CPUs with
> SVE and in fact a similar optimisation is also useful there.
All of that might be true, but that doesn't change my assessment that
this doesn't seem to matter in practice, and given that the overall goal
of this series is to *simplify* things, I'd much rather err towards that
than hypothetical performance concerns.
> I'm a bit wary of dropping the optimisation without any verification
> of the performance impact, but equally I'm not aware of a specific
> benchmark that showed the impact or even if there was one in the first
> place. The changelog sounds like the optimisation might've been
> written based on inspection alone, I don't know if anyone will
> remember more than a decade later.
From what I remember, the changes in commit 5959e25729a5 were made based
on intuition, inspired by a contemporary retrospective change to the
architecture that made FPCR self-synchronizing. Previously the
architecture required a context synchronization event for the write to
take effect, but implementations happened to be stronger.
The conditional write isn't necessarily a win, because the cost of
recovering from a branch mispredict can be much larger than the cost of
micro-architectural mechanisms to ensure that FPCR is
self-synchronizing.
> Having said all that given that a conditional update is simple to
> implement in C it seems safer to add one in the SVE path than to drop
> it from the FPSIMD path.
I agree that if we need to, it would be simple to add this.
For now I'm going to leave this as-is given the rationale I originally
provided. This patch specifically doesn't change the existing behaviour.
I don't think this matters in practice, we haven't consistently applied
this approach to FPCR (or other similar registers), and omitting this
makes the code simpler.
Mark.
next prev parent reply other threads:[~2026-05-27 13:51 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-21 13:25 [PATCH 00/18] arm64+KVM: FPSIMD/SVE/SME cleanups Mark Rutland
2026-05-21 13:25 ` [PATCH 01/18] KVM: arm64: Don't include <asm/fpsimdmacros.h> Mark Rutland
2026-05-26 14:18 ` Mark Brown
2026-05-27 10:10 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 02/18] KVM: arm64: Don't override FFR save/restore argument Mark Rutland
2026-05-26 14:27 ` Mark Brown
2026-05-27 10:16 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 03/18] KVM: arm64: pkvm: Save host FPMR in host cpu context Mark Rutland
2026-05-27 10:29 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 04/18] KVM: arm64: pkvm: Remove struct cpu_sve_state Mark Rutland
2026-05-27 11:58 ` Vladimir Murzin
2026-05-27 16:02 ` Mark Rutland
2026-05-27 16:11 ` Vladimir Murzin
2026-05-28 15:09 ` Mark Rutland
2026-05-28 15:12 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 05/18] arm64: fpsimd: Fold sve_init_regs() into do_sve_acc() Mark Rutland
2026-05-26 15:28 ` Mark Brown
2026-05-27 12:05 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 06/18] arm64: fpsimd: Remove sve_set_vq() and sme_set_vq() Mark Rutland
2026-05-26 15:42 ` Mark Brown
2026-05-27 12:50 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 07/18] arm64: fpsimd: Use assembler for SVE instructions Mark Rutland
2026-05-26 15:43 ` Mark Brown
2026-05-27 12:58 ` Vladimir Murzin
2026-05-27 16:10 ` Mark Rutland
2026-05-21 13:25 ` [PATCH 08/18] arm64: fpsimd: Use assembler for baseline SME instructions Mark Rutland
2026-05-26 15:45 ` Mark Brown
2026-05-27 13:06 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 09/18] arm64: fpsimd: Move sve_get_vl() and sme_get_vl() inline Mark Rutland
2026-05-26 15:47 ` Mark Brown
2026-05-27 13:18 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 10/18] arm64: sysreg: Add FPCR and FPSR Mark Rutland
2026-05-26 15:55 ` Mark Brown
2026-05-26 16:51 ` Mark Rutland
2026-05-26 16:54 ` Mark Brown
2026-05-21 13:25 ` [PATCH 11/18] arm64: fpsimd: Split FPSR/FPCR from SVE save/restore Mark Rutland
2026-05-26 16:28 ` Mark Brown
2026-05-27 13:51 ` Mark Rutland [this message]
2026-05-27 14:13 ` Mark Brown
2026-05-27 16:13 ` Mark Rutland
2026-05-27 13:44 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 12/18] arm64: fpsimd: Move fpsimd save/restore inline Mark Rutland
2026-05-26 16:44 ` Mark Brown
2026-05-28 16:15 ` Mark Rutland
2026-05-28 16:39 ` Mark Brown
2026-05-27 14:49 ` Vladimir Murzin
2026-05-27 15:34 ` Mark Rutland
2026-05-27 16:13 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 13/18] arm64: fpsimd: Use opaque type for SVE state Mark Rutland
2026-05-26 16:53 ` Mark Brown
2026-05-28 9:45 ` Vladimir Murzin
2026-05-28 16:25 ` Mark Rutland
2026-05-21 13:25 ` [PATCH 14/18] arm64: fpsimd: Use opaque type for SME state Mark Rutland
2026-05-26 16:56 ` Mark Brown
2026-05-28 9:51 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 15/18] arm64: fpsimd: Move SVE save/restore inline Mark Rutland
2026-05-27 12:29 ` Mark Brown
2026-05-28 10:39 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 16/18] arm64: fpsimd: Move sve_flush_live() inline Mark Rutland
2026-05-27 12:54 ` Mark Brown
2026-05-27 16:23 ` Mark Rutland
2026-05-28 10:49 ` Vladimir Murzin
2026-05-21 13:25 ` [PATCH 17/18] arm64: fpsimd: Move SME save/restore inline Mark Rutland
2026-05-26 14:08 ` Mark Rutland
2026-05-26 14:39 ` Vladimir Murzin
2026-05-26 15:28 ` Mark Rutland
2026-05-26 16:38 ` Mark Rutland
2026-05-27 9:00 ` Vladimir Murzin
2026-05-29 9:10 ` Mark Rutland
2026-05-28 12:30 ` Vladimir Murzin
2026-05-28 14:39 ` Mark Rutland
2026-05-21 13:25 ` [PATCH 18/18] arm64: fpsimd: Remove <asm/fpsimdmacros.h> Mark Rutland
2026-05-28 13:10 ` Vladimir Murzin
2026-05-27 8:07 ` [PATCH 00/18] arm64+KVM: FPSIMD/SVE/SME cleanups Marc Zyngier
2026-05-27 10:32 ` Mark Rutland
2026-05-27 14:36 ` Will Deacon
2026-05-28 13:21 ` Vladimir Murzin
2026-05-28 16:28 ` Mark Rutland
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=ahb20fXkdWI7SljE@J2N7QTR9R3.cambridge.arm.com \
--to=mark.rutland@arm$(echo .)com \
--cc=broonie@kernel$(echo .)org \
--cc=catalin.marinas@arm$(echo .)com \
--cc=james.morse@arm$(echo .)com \
--cc=kvmarm@lists$(echo .)linux.dev \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=maz@kernel$(echo .)org \
--cc=oupton@kernel$(echo .)org \
--cc=tabba@google$(echo .)com \
--cc=will@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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