linux-kernel - Re: [PATCH] lib/crc: make the CPU feature static keys __ro_after

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250417125318.12521F12-hca@linux.ibm.com>
Date: Thu, 17 Apr 2025 14:53:18 +0200
From: Heiko Carstens <hca@...ux.ibm.com>
To: Eric Biggers <ebiggers@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        Ard Biesheuvel <ardb@...nel.org>, linux-arm-kernel@...ts.infradead.org,
        loongarch@...ts.linux.dev, linux-mips@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, sparclinux@...r.kernel.org,
        linux-s390@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH] lib/crc: make the CPU feature static keys __ro_after_init

On Sun, Apr 13, 2025 at 08:43:50AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@...gle.com>
> 
> All of the CRC library's CPU feature static_keys are initialized by
> initcalls and never change afterwards, so there's no need for them to be
> in the regular .data section.  Put them in .data..ro_after_init instead.
> 
> Signed-off-by: Eric Biggers <ebiggers@...gle.com>
> ---
> 
> I'm planning to take this via the crc tree.
> 
>  arch/arm/lib/crc-t10dif-glue.c       | 4 ++--
>  arch/arm/lib/crc32-glue.c            | 4 ++--
>  arch/arm64/lib/crc-t10dif-glue.c     | 4 ++--
>  arch/loongarch/lib/crc32-loongarch.c | 2 +-
>  arch/mips/lib/crc32-mips.c           | 2 +-
>  arch/powerpc/lib/crc-t10dif-glue.c   | 2 +-
>  arch/powerpc/lib/crc32-glue.c        | 2 +-
>  arch/s390/lib/crc32-glue.c           | 2 +-


Acked-by: Heiko Carstens <hca@...ux.ibm.com> # s390

I just realized that we can get rid of the static key in the s390
piece and end up with slightly better code. Could you add the patch
below to your tree, please? If this would go via the s390 tree this
would result in a merge conflict, which is unnecessary.

>From 6c4c0ca6fe87e43acf6192f1afc0a6346db994f4 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <hca@...ux.ibm.com>
Date: Thu, 17 Apr 2025 14:30:56 +0200
Subject: [PATCH] s390/crc32: Remove have_vxrs static key

Replace the have_vxrs static key with a cpu_has_vx() call.  cpu_has_vx()
resolves into a compile time constant (true) if the kernel is compiled for
z13 or newer. Otherwise it generates an unconditional one instruction
branch, which is patched based on CPU alternatives.

In any case the generated code is at least as good as before and avoids
static key handling.

Signed-off-by: Heiko Carstens <hca@...ux.ibm.com>
---
 arch/s390/lib/crc32-glue.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/s390/lib/crc32-glue.c b/arch/s390/lib/crc32-glue.c
index 8f20a8e595c3..649ed7e8b99c 100644
--- a/arch/s390/lib/crc32-glue.c
+++ b/arch/s390/lib/crc32-glue.c
@@ -18,8 +18,6 @@
 #define VX_ALIGNMENT		16L
 #define VX_ALIGN_MASK		(VX_ALIGNMENT - 1)
 
-static __ro_after_init DEFINE_STATIC_KEY_FALSE(have_vxrs);
-
 /*
  * DEFINE_CRC32_VX() - Define a CRC-32 function using the vector extension
  *
@@ -34,8 +32,7 @@ static __ro_after_init DEFINE_STATIC_KEY_FALSE(have_vxrs);
 		unsigned long prealign, aligned, remaining;		    \
 		DECLARE_KERNEL_FPU_ONSTACK16(vxstate);			    \
 									    \
-		if (datalen < VX_MIN_LEN + VX_ALIGN_MASK ||		    \
-		    !static_branch_likely(&have_vxrs))			    \
+		if (datalen < VX_MIN_LEN + VX_ALIGN_MASK || !cpu_has_vx())  \
 			return ___crc32_sw(crc, data, datalen);		    \
 									    \
 		if ((unsigned long)data & VX_ALIGN_MASK) {		    \
@@ -66,8 +63,6 @@ DEFINE_CRC32_VX(crc32c_arch, crc32c_le_vgfm_16, crc32c_base)
 
 static int __init crc32_s390_init(void)
 {
-	if (cpu_have_feature(S390_CPU_FEATURE_VXRS))
-		static_branch_enable(&have_vxrs);
 	return 0;
 }
 arch_initcall(crc32_s390_init);
@@ -79,10 +74,11 @@ module_exit(crc32_s390_exit);
 
 u32 crc32_optimizations(void)
 {
-	if (static_key_enabled(&have_vxrs))
+	if (cpu_has_vx()) {
 		return CRC32_LE_OPTIMIZATION |
 		       CRC32_BE_OPTIMIZATION |
 		       CRC32C_OPTIMIZATION;
+	}
 	return 0;
 }
 EXPORT_SYMBOL(crc32_optimizations);
-- 
2.45.2