linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] arm64: Make write_sysreg_s() select XZR reliably
@ 2026-09-04 17:48 Sascha Bischoff
  2026-09-04 17:49 ` [PATCH 1/2] arm64: sysreg: Make write_sysreg_s() select XZR without optimisation Sascha Bischoff
  2026-09-04 17:49 ` [PATCH 2/2] arm64: tools: Fix GIC CDEOI instruction encoding Sascha Bischoff
  0 siblings, 2 replies; 4+ messages in thread
From: Sascha Bischoff @ 2026-09-04 17:48 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: nd, Catalin Marinas, will@kernel.org, Mark Rutland,
	lpieralisi@kernel.org, maz@kernel.org, oupton@kernel.org,
	kvmarm@lists.linux.dev, kvm@vger.kernel.org

Hi all,

Sashiko spotted the original tools-side CDEOI encoding issue while
reviewing the GICv5 KVM IRS series [1]. While updating the tools copy to
match the kernel header, I noticed that the existing kernel workaround
relies on constant propagation to recognise that its local value was
initialised with zero. With optimisation disabled, compilers instead
select an arbitrary general-purpose register.

Compiling the kernel or tools without optimisation is uncommon, but the
fix is small and removes this dependency entirely.

This matters for instructions such as GIC CDEOI, where Rt must select
XZR rather than a register containing zero. Patch 1 removes the
reliance on constant propagation and instead tests the macro argument
directly. Patch 2 mirrors the change in the tools copy, fixing the
GICv5 KVM selftest.

After these changes, both GCC and LLVM emit the correct encoding when
using -O0.

[1] https://lore.kernel.org/r/20260807173928.3F6731F000E9@smtp.kernel.org

Thanks,
Sascha

Sascha Bischoff (2):
  arm64: sysreg: Make write_sysreg_s() select XZR without optimisation
  arm64: tools: Fix GIC CDEOI instruction encoding

 arch/arm64/include/asm/sysreg.h       |  2 +-
 tools/arch/arm64/include/asm/sysreg.h | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.34.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] arm64: sysreg: Make write_sysreg_s() select XZR without optimisation
  2026-09-04 17:48 [PATCH 0/2] arm64: Make write_sysreg_s() select XZR reliably Sascha Bischoff
@ 2026-09-04 17:49 ` Sascha Bischoff
  2026-09-08 16:19   ` Mark Rutland
  2026-09-04 17:49 ` [PATCH 2/2] arm64: tools: Fix GIC CDEOI instruction encoding Sascha Bischoff
  1 sibling, 1 reply; 4+ messages in thread
From: Sascha Bischoff @ 2026-09-04 17:49 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: nd, Catalin Marinas, will@kernel.org, Mark Rutland,
	lpieralisi@kernel.org, maz@kernel.org, oupton@kernel.org,
	kvmarm@lists.linux.dev, kvm@vger.kernel.org

write_sysreg_s() tests whether its local __val variable is constant when
deciding whether to use XZR. Compilers only recognise that __val was
initialised with a constant zero after constant propagation.

With optimisation disabled, the fallback path therefore selects an
arbitrary general-purpose register. This is incorrect for instructions
such as GIC CDEOI, where Rt must be XZR rather than a register containing
zero.

Test the macro argument directly so that constant zero selects XZR
regardless of the optimisation level.

Fixes: e9ad390a4812 ("arm64/sysreg: Fix GIC CDEOI instruction encoding")
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 arch/arm64/include/asm/sysreg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 7aa08d59d4944..a80bede50f865 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -1204,7 +1204,7 @@
 #define write_sysreg_s(v, r) do {					\
 	u64 __val = (u64)(v);						\
 	u32 __maybe_unused __check_r = (u32)(r);			\
-	if (__builtin_constant_p(__val) && __val == 0)			\
+	if (__builtin_constant_p(v) && (u64)(v) == 0)			\
 		asm volatile(__msr_s(r, "xzr"));			\
 	else								\
 		asm volatile(__msr_s(r, "%x0") : : "r" (__val));	\
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] arm64: tools: Fix GIC CDEOI instruction encoding
  2026-09-04 17:48 [PATCH 0/2] arm64: Make write_sysreg_s() select XZR reliably Sascha Bischoff
  2026-09-04 17:49 ` [PATCH 1/2] arm64: sysreg: Make write_sysreg_s() select XZR without optimisation Sascha Bischoff
@ 2026-09-04 17:49 ` Sascha Bischoff
  1 sibling, 0 replies; 4+ messages in thread
From: Sascha Bischoff @ 2026-09-04 17:49 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: nd, Catalin Marinas, will@kernel.org, Mark Rutland,
	lpieralisi@kernel.org, maz@kernel.org, oupton@kernel.org,
	kvmarm@lists.linux.dev, kvm@vger.kernel.org, Sashiko

The tools copy of write_sysreg_s() relies on the "rZ" constraint with
the "%x0" modifier to select XZR for a constant zero. LLVM does not
understand this combination and can select another general-purpose
register instead.

The GICv5 PPI selftest emits CDEOI through the tools copy. Its Rt field
must select XZR, so using another register results in unpredictable
behaviour.

Mirror the kernel implementation so that a constant zero selects XZR
regardless of the optimisation level.

Fixes: 0a9f38bf612b ("KVM: arm64: selftests: Introduce a minimal GICv5 PPI selftest")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260807173928.3F6731F000E9@smtp.kernel.org
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 tools/arch/arm64/include/asm/sysreg.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/arch/arm64/include/asm/sysreg.h b/tools/arch/arm64/include/asm/sysreg.h
index f75efe98e9df3..342790d13f788 100644
--- a/tools/arch/arm64/include/asm/sysreg.h
+++ b/tools/arch/arm64/include/asm/sysreg.h
@@ -1152,10 +1152,19 @@
 	__val;								\
 })
 
+/*
+ * The "Z" constraint combined with the "%x0" template should be enough
+ * to force XZR generation if (v) is a constant 0 value but LLVM does not
+ * yet understand that modifier/constraint combo so a conditional is required
+ * to nudge the compiler into using XZR as a source for a 0 constant value.
+ */
 #define write_sysreg_s(v, r) do {					\
 	u64 __val = (u64)(v);						\
 	u32 __maybe_unused __check_r = (u32)(r);			\
-	asm volatile(__msr_s(r, "%x0") : : "rZ" (__val));		\
+	if (__builtin_constant_p(v) && (u64)(v) == 0)			\
+		asm volatile(__msr_s(r, "xzr"));			\
+	else								\
+		asm volatile(__msr_s(r, "%x0") : : "r" (__val));	\
 } while (0)
 
 /*
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] arm64: sysreg: Make write_sysreg_s() select XZR without optimisation
  2026-09-04 17:49 ` [PATCH 1/2] arm64: sysreg: Make write_sysreg_s() select XZR without optimisation Sascha Bischoff
@ 2026-09-08 16:19   ` Mark Rutland
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Rutland @ 2026-09-08 16:19 UTC (permalink / raw)
  To: Sascha Bischoff
  Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, nd, Catalin Marinas,
	will@kernel.org, lpieralisi@kernel.org, maz@kernel.org,
	oupton@kernel.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org

On Fri, Sep 04, 2026 at 06:49:25PM +0100, Sascha Bischoff wrote:
> write_sysreg_s() tests whether its local __val variable is constant when
> deciding whether to use XZR. Compilers only recognise that __val was
> initialised with a constant zero after constant propagation.
> 
> With optimisation disabled, the fallback path therefore selects an
> arbitrary general-purpose register. This is incorrect for instructions
> such as GIC CDEOI, where Rt must be XZR rather than a register containing
> zero.

Which CONFIG_* option needs to be selected for that to happen?

I'm struggling to see how other usage of __builtin_constant_p() wouldn't
blow up in such a configuration, so knowing that would help to
investigate and test.

> Test the macro argument directly so that constant zero selects XZR
> regardless of the optimisation level.
> 
> Fixes: e9ad390a4812 ("arm64/sysreg: Fix GIC CDEOI instruction encoding")
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  arch/arm64/include/asm/sysreg.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 7aa08d59d4944..a80bede50f865 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -1204,7 +1204,7 @@
>  #define write_sysreg_s(v, r) do {					\
>  	u64 __val = (u64)(v);						\
>  	u32 __maybe_unused __check_r = (u32)(r);			\
> -	if (__builtin_constant_p(__val) && __val == 0)			\
> +	if (__builtin_constant_p(v) && (u64)(v) == 0)			\

It's not safe to use 'v' here, since 'v' could be an expression with
side-effects, and we must not evaluate it multiple times.

I think we should just enxcode the GIC instructions manually. While they
are SYS/SYSL instructions, they *aren't* MSR/MRS, and using
write_sysreg_s() is a hack.

Mark.

>  		asm volatile(__msr_s(r, "xzr"));			\
>  	else								\
>  		asm volatile(__msr_s(r, "%x0") : : "r" (__val));	\
> -- 
> 2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-08 16:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-04 17:48 [PATCH 0/2] arm64: Make write_sysreg_s() select XZR reliably Sascha Bischoff
2026-09-04 17:49 ` [PATCH 1/2] arm64: sysreg: Make write_sysreg_s() select XZR without optimisation Sascha Bischoff
2026-09-08 16:19   ` Mark Rutland
2026-09-04 17:49 ` [PATCH 2/2] arm64: tools: Fix GIC CDEOI instruction encoding Sascha Bischoff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).