diff mbox series

[v2,18/19] crypto: x86 - standardize not loaded prints

Message ID 20221012215931.3896-19-elliott@hpe.com
State New
Headers show
Series [RFC,1/7] rcu: correct CONFIG_EXT_RCU_CPU_STALL_TIMEOUT descriptions | expand

Commit Message

Elliott, Robert (Servers) Oct. 12, 2022, 9:59 p.m. UTC
Standardize the prints that additional required CPU features are not
present along with the main CPU features (e.g., OSXSAVE is not
present along with AVX).

Although modules are not supposed to print unless loaded and
active, these are existing exceptions.

Signed-off-by: Robert Elliott <elliott@hpe.com>
---
 arch/x86/crypto/aegis128-aesni-glue.c      | 4 +++-
 arch/x86/crypto/aria_aesni_avx_glue.c      | 4 ++--
 arch/x86/crypto/camellia_aesni_avx2_glue.c | 5 +++--
 arch/x86/crypto/camellia_aesni_avx_glue.c  | 5 +++--
 arch/x86/crypto/cast5_avx_glue.c           | 3 ++-
 arch/x86/crypto/cast6_avx_glue.c           | 3 ++-
 arch/x86/crypto/crc32-pclmul_glue.c        | 4 +++-
 arch/x86/crypto/nhpoly1305-avx2-glue.c     | 4 +++-
 arch/x86/crypto/serpent_avx2_glue.c        | 8 +++++---
 arch/x86/crypto/serpent_avx_glue.c         | 3 ++-
 arch/x86/crypto/sm3_avx_glue.c             | 7 ++++---
 arch/x86/crypto/sm4_aesni_avx2_glue.c      | 5 +++--
 arch/x86/crypto/sm4_aesni_avx_glue.c       | 5 +++--
 arch/x86/crypto/twofish_avx_glue.c         | 3 ++-
 14 files changed, 40 insertions(+), 23 deletions(-)

Comments

Jason A. Donenfeld Oct. 13, 2022, 12:42 a.m. UTC | #1
On Wed, Oct 12, 2022 at 04:59:30PM -0500, Robert Elliott wrote:
> Standardize the prints that additional required CPU features are not
> present along with the main CPU features (e.g., OSXSAVE is not
> present along with AVX).
> 
> Although modules are not supposed to print unless loaded and
> active, these are existing exceptions.

Another comma splice. But also, don't do this. No need to clutter dmesg.
`lsmod` is the job for this.

Jason
Elliott, Robert (Servers) Oct. 13, 2022, 10:20 p.m. UTC | #2
> -----Original Message-----
> From: Jason A. Donenfeld <Jason@zx2c4.com>
> Sent: Wednesday, October 12, 2022 7:43 PM
> To: Elliott, Robert (Servers) <elliott@hpe.com>
> Cc: herbert@gondor.apana.org.au; davem@davemloft.net;
> tim.c.chen@linux.intel.com; ap420073@gmail.com; ardb@kernel.org; linux-
> crypto@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 18/19] crypto: x86 - standardize not loaded prints
> 
> On Wed, Oct 12, 2022 at 04:59:30PM -0500, Robert Elliott wrote:
> > Standardize the prints that additional required CPU features are not
> > present along with the main CPU features (e.g., OSXSAVE is not
> > present along with AVX).
> >
> > Although modules are not supposed to print unless loaded and
> > active, these are existing exceptions.
> 
> Another comma splice. But also, don't do this. No need to clutter dmesg.
> `lsmod` is the job for this.

If module loading fails, modprobe gets back one errno value
and converts that to a string, but has no other clue what
is wrong.

The modprobe man page refers to dmesg:
  ... modprobe does not do anything to the module itself: the work of 
  resolving symbols and understanding parameters is done inside the
  kernel. So module failure is sometimes accompanied by a kernel
  message: see dmesg(8).

If the error happens to be -ENOENT, modprobe specifically recommends
looking at dmesg:
  modprobe: ERROR: could not insert 'tcrypt': Unknown symbol in module, or unknown parameter (see dmesg)

A device table mismatch can be determined by comparing the modinfo
aliases for the module to /sys/devices/system/cpu/modalias:

cpu:type:x86,ven0000fam0006mod0055:feature:,0000,0001,0002,0003,0004,0005,0006,0007,0008,0009,000B,000C,000D,000E,000F,0010,0011,0013,0015,0016,0017,0018,0019,001A,001B,001C,001D,001F,002B,0034,003A,003B,003D,0068,006A,006B,006C,006D,006F,0070,0072,0074,0075,0076,0078,0079,007C,0080,0081,0082,0083,0084,0085,0086,0087,0088,0089,008B,008C,008D,008E,008F,0091,0092,0093,0094,0095,0096,0097,0098,0099,009A,009B,009C,009D,009E,00C0,00C5,00C8,00E1,00E3,00E4,00E6,00E7,00EA,00F0,00F1,00F2,00F3,00F5,00F9,00FA,00FB,00FE,00FF,0100,0101,0102,0103,0104,0111,0120,0121,0123,0125,0126,0127,0128,0129,012A,012C,012D,012E,012F,0130,0131,0132,0133,0134,0137,0138,0139,013C,013E,013F,0140,0141,0142,0143,0160,0161,0162,0163,0164,0165,0171,01C0,01C1,01C2,01C4,01C5,01C6,01C7,01C9,01CB,0203,0204,020B,024A,025A,025B,025C,025D,025F

modinfo aesni-intel:
alias:          cpu:type:x86,ven*fam*mod*:feature:*0099*

so I'm comfortable not printing that one.

The checks for other combinations of features (e.g., sha512
also requiring BMI2) and for CPU extended features are not
so obvious. Nothing in modinfo explains what the module is
looking for, and nothing records what it didn't like. There
are currently 32 prints in the directory, either explaining
errors or explaining which optional features have been
enabled.

The modprobe manpage doesn't promise what log level will
explain the problem, so we could print them with pr_debug
so they're only available if you figure out how to enable
dynamic debug for the module.

The positive messages about which optional features are
engaged could be reported as read-only module parameters.
Elliott, Robert (Servers) Nov. 10, 2022, 10:06 p.m. UTC | #3
> The positive messages about which optional features are
> engaged could be reported as read-only module parameters.

I've experimented with this approach.

There are two constructions for the modules:
1. modules that enable different behavior in the drivers
  (e.g., aesni_intel enabling avx or avx2 within each driver)

  I named these parameters "engaged_<feature>"
 
2. modules that register separate drivers for different behavior
  (e.g., sha1 registering separate drivers for avx2, avx, and ssse3)

  I named these parameters "registered_<feature>"

It looks like this:

$ grep -Hs . /sys/module/*/parameters/engaged*
/sys/module/aesni_intel/parameters/engaged_avx:1
/sys/module/aesni_intel/parameters/engaged_avx2:1
/sys/module/aria_aesni_avx_x86_64/parameters/engaged_gfni:0
/sys/module/chacha_x86_64/parameters/engaged_avx2:1
/sys/module/chacha_x86_64/parameters/engaged_avx512:1
/sys/module/crc32c_intel/parameters/engaged_pclmulqdq:1
/sys/module/curve25519_x86_64/parameters/engaged_adx:1
/sys/module/libblake2s_x86_64/parameters/engaged_avx512:1
/sys/module/libblake2s_x86_64/parameters/engaged_ssse3:1
/sys/module/poly1305_x86_64/parameters/engaged_avx:1
/sys/module/poly1305_x86_64/parameters/engaged_avx2:1
/sys/module/poly1305_x86_64/parameters/engaged_avx512:0

with modinfo descriptions like:
parm:           engaged_avx2:Using x86 instruction set extensions: AVX2 (for GCM mode) (int)
parm:           engaged_avx:Using x86 instruction set extensions: AVX (for CTR and GCM modes) (int)

$ grep -Hs . /sys/module/*/parameters/registered*
/sys/module/sha1_ssse3/parameters/registered_avx:1
/sys/module/sha1_ssse3/parameters/registered_avx2:1
/sys/module/sha1_ssse3/parameters/registered_shani:0
/sys/module/sha1_ssse3/parameters/registered_ssse3:1
/sys/module/sha256_ssse3/parameters/registered_avx:1
/sys/module/sha256_ssse3/parameters/registered_avx2:1
/sys/module/sha256_ssse3/parameters/registered_shani:0
/sys/module/sha256_ssse3/parameters/registered_ssse3:1
/sys/module/sha512_ssse3/parameters/registered_avx:1
/sys/module/sha512_ssse3/parameters/registered_avx2:1
/sys/module/sha512_ssse3/parameters/registered_ssse3:1

with modinfo descriptions like:
parm:           registered_shani:Registered crypto driver using x86 instruction set extensions: SHA-NI (int)
parm:           registered_ssse3:Registered crypto driver using x86 instruction set extensions: SSSE3 (int)
parm:           registered_avx:Registered crypto driver using x86 instruction set extensions: AVX (int)
parm:           registered_avx2:Registered crypto driver using x86 instruction set extensions: AVX2 (int)

That would eliminate these prints in aesni_intel, so all the
modules load silently (but you can figure out what they're
doing if needed).

	pr_info("AVX2 version of gcm_enc/dec engaged.\n");
	pr_info("AVX version of gcm_enc/dec engaged.\n");
	pr_info("SSE version of gcm_enc/dec engaged.\n");
	pr_info("AES CTR mode by8 optimization enabled\n");
diff mbox series

Patch

diff --git a/arch/x86/crypto/aegis128-aesni-glue.c b/arch/x86/crypto/aegis128-aesni-glue.c
index e8eaf79ef220..aa94b9f8703c 100644
--- a/arch/x86/crypto/aegis128-aesni-glue.c
+++ b/arch/x86/crypto/aegis128-aesni-glue.c
@@ -281,8 +281,10 @@  static int __init crypto_aegis128_aesni_module_init(void)
 
 	if (!boot_cpu_has(X86_FEATURE_XMM2) ||
 	    !boot_cpu_has(X86_FEATURE_AES) ||
-	    !cpu_has_xfeatures(XFEATURE_MASK_SSE, NULL))
+	    !cpu_has_xfeatures(XFEATURE_MASK_SSE, NULL)) {
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (SSE2, AESNI) not supported\n");
 		return -ENODEV;
+	}
 
 	ret = simd_register_aeads_compat(&crypto_aegis128_aesni_alg, 1,
 					  &simd_alg);
diff --git a/arch/x86/crypto/aria_aesni_avx_glue.c b/arch/x86/crypto/aria_aesni_avx_glue.c
index d58fb995a266..24982450a125 100644
--- a/arch/x86/crypto/aria_aesni_avx_glue.c
+++ b/arch/x86/crypto/aria_aesni_avx_glue.c
@@ -176,13 +176,13 @@  static int __init aria_avx_init(void)
 	if (!boot_cpu_has(X86_FEATURE_AVX) ||
 	    !boot_cpu_has(X86_FEATURE_AES) ||
 	    !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
-		pr_info("AVX or AES-NI instructions are not detected.\n");
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (AVX, AES-NI, OSXSAVE) not supported\n");
 		return -ENODEV;
 	}
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU extended feature '%s' is not supported\n", feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/camellia_aesni_avx2_glue.c b/arch/x86/crypto/camellia_aesni_avx2_glue.c
index e6c4ed1e40d2..bc6862077984 100644
--- a/arch/x86/crypto/camellia_aesni_avx2_glue.c
+++ b/arch/x86/crypto/camellia_aesni_avx2_glue.c
@@ -123,13 +123,14 @@  static int __init camellia_aesni_init(void)
 	    !boot_cpu_has(X86_FEATURE_AVX2) ||
 	    !boot_cpu_has(X86_FEATURE_AES) ||
 	    !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
-		pr_info("AVX2 or AES-NI instructions are not detected.\n");
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (AVX, AVX2, AESNI, OSXSAVE) not supported\n");
 		return -ENODEV;
 	}
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/camellia_aesni_avx_glue.c b/arch/x86/crypto/camellia_aesni_avx_glue.c
index 6a9eadf0fe90..96e7e1accb6c 100644
--- a/arch/x86/crypto/camellia_aesni_avx_glue.c
+++ b/arch/x86/crypto/camellia_aesni_avx_glue.c
@@ -121,13 +121,14 @@  static int __init camellia_aesni_init(void)
 	if (!boot_cpu_has(X86_FEATURE_AVX) ||
 	    !boot_cpu_has(X86_FEATURE_AES) ||
 	    !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
-		pr_info("AVX or AES-NI instructions are not detected.\n");
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (AVX, AESNI, OSXSAVE) not supported\n");
 		return -ENODEV;
 	}
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/cast5_avx_glue.c b/arch/x86/crypto/cast5_avx_glue.c
index b5ae17c3ac53..89650fffb550 100644
--- a/arch/x86/crypto/cast5_avx_glue.c
+++ b/arch/x86/crypto/cast5_avx_glue.c
@@ -114,7 +114,8 @@  static int __init cast5_init(void)
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+		feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/cast6_avx_glue.c b/arch/x86/crypto/cast6_avx_glue.c
index d1c14a5f80d7..d69f62ac9553 100644
--- a/arch/x86/crypto/cast6_avx_glue.c
+++ b/arch/x86/crypto/cast6_avx_glue.c
@@ -114,7 +114,8 @@  static int __init cast6_init(void)
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/crc32-pclmul_glue.c b/arch/x86/crypto/crc32-pclmul_glue.c
index c56d3d3ab0a0..4cf86f8f9428 100644
--- a/arch/x86/crypto/crc32-pclmul_glue.c
+++ b/arch/x86/crypto/crc32-pclmul_glue.c
@@ -192,8 +192,10 @@  static int __init crc32_pclmul_mod_init(void)
 {
 	int ret;
 
-	if (!x86_match_cpu(module_cpu_ids))
+	if (!x86_match_cpu(module_cpu_ids)) {
+		pr_info("CPU-optimized crypto module not loaded, required CPU feature (PCLMULQDQ) not supported\n");
 		return -ENODEV;
+	}
 
 	ret = crypto_register_shash(&alg);
 	if (!ret)
diff --git a/arch/x86/crypto/nhpoly1305-avx2-glue.c b/arch/x86/crypto/nhpoly1305-avx2-glue.c
index 2dc7b618771f..834bf64bb160 100644
--- a/arch/x86/crypto/nhpoly1305-avx2-glue.c
+++ b/arch/x86/crypto/nhpoly1305-avx2-glue.c
@@ -74,8 +74,10 @@  static int __init nhpoly1305_mod_init(void)
 		return -ENODEV;
 
 	if (!boot_cpu_has(X86_FEATURE_AVX2) ||
-	    !boot_cpu_has(X86_FEATURE_OSXSAVE))
+	    !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (AVX2, OSXSAVE) not supported\n");
 		return -ENODEV;
+	}
 
 	ret = crypto_register_shash(&nhpoly1305_alg);
 	if (!ret)
diff --git a/arch/x86/crypto/serpent_avx2_glue.c b/arch/x86/crypto/serpent_avx2_glue.c
index bf59addaf804..4bd59ccea69a 100644
--- a/arch/x86/crypto/serpent_avx2_glue.c
+++ b/arch/x86/crypto/serpent_avx2_glue.c
@@ -112,13 +112,15 @@  static int __init serpent_avx2_init(void)
 
 		return -ENODEV;
 
-	if (!boot_cpu_has(X86_FEATURE_AVX2) || !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
-		pr_info("AVX2 instructions are not detected.\n");
+	if (!boot_cpu_has(X86_FEATURE_AVX2) ||
+	    !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (AVX2, OSXSAVE) not supported\n");
 		return -ENODEV;
 	}
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/serpent_avx_glue.c b/arch/x86/crypto/serpent_avx_glue.c
index 7b0c02a61552..853b48677d2b 100644
--- a/arch/x86/crypto/serpent_avx_glue.c
+++ b/arch/x86/crypto/serpent_avx_glue.c
@@ -121,7 +121,8 @@  static int __init serpent_init(void)
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/sm3_avx_glue.c b/arch/x86/crypto/sm3_avx_glue.c
index 532f07b05745..5250fee79147 100644
--- a/arch/x86/crypto/sm3_avx_glue.c
+++ b/arch/x86/crypto/sm3_avx_glue.c
@@ -131,18 +131,19 @@  static int __init sm3_avx_mod_init(void)
 		return -ENODEV;
 
 	if (!boot_cpu_has(X86_FEATURE_AVX)) {
-		pr_info("AVX instruction are not detected.\n");
+		pr_info("CPU-optimized crypto module not loaded, required CPU feature (AVX) not supported\n");
 		return -ENODEV;
 	}
 
 	if (!boot_cpu_has(X86_FEATURE_BMI2)) {
-		pr_info("BMI2 instruction are not detected.\n");
+		pr_info("CPU-optimized crypto module not loaded, required CPU feature (BMI2) not supported\n");
 		return -ENODEV;
 	}
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/sm4_aesni_avx2_glue.c b/arch/x86/crypto/sm4_aesni_avx2_glue.c
index 42819ee5d36d..cdd7ca92ca61 100644
--- a/arch/x86/crypto/sm4_aesni_avx2_glue.c
+++ b/arch/x86/crypto/sm4_aesni_avx2_glue.c
@@ -152,13 +152,14 @@  static int __init sm4_init(void)
 	    !boot_cpu_has(X86_FEATURE_AVX2) ||
 	    !boot_cpu_has(X86_FEATURE_AES) ||
 	    !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
-		pr_info("AVX2 or AES-NI instructions are not detected.\n");
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (AVX, AVX2, AESNI, OSXSAVE) not supported\n");
 		return -ENODEV;
 	}
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/sm4_aesni_avx_glue.c b/arch/x86/crypto/sm4_aesni_avx_glue.c
index 8a25376d341f..a2ae3d1e0a4a 100644
--- a/arch/x86/crypto/sm4_aesni_avx_glue.c
+++ b/arch/x86/crypto/sm4_aesni_avx_glue.c
@@ -468,13 +468,14 @@  static int __init sm4_init(void)
 	if (!boot_cpu_has(X86_FEATURE_AVX) ||
 	    !boot_cpu_has(X86_FEATURE_AES) ||
 	    !boot_cpu_has(X86_FEATURE_OSXSAVE)) {
-		pr_info("AVX or AES-NI instructions are not detected.\n");
+		pr_info("CPU-optimized crypto module not loaded, all required CPU features (AVX, AESNI, OSXSAVE) not supported\n");
 		return -ENODEV;
 	}
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM,
 				&feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}
 
diff --git a/arch/x86/crypto/twofish_avx_glue.c b/arch/x86/crypto/twofish_avx_glue.c
index ccf016bf6ef2..70167dd01816 100644
--- a/arch/x86/crypto/twofish_avx_glue.c
+++ b/arch/x86/crypto/twofish_avx_glue.c
@@ -123,7 +123,8 @@  static int __init twofish_init(void)
 		return -ENODEV;
 
 	if (!cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM, &feature_name)) {
-		pr_info("CPU feature '%s' is not supported.\n", feature_name);
+		pr_info("CPU-optimized crypto module not loaded, CPU extended feature '%s' is not supported\n",
+			feature_name);
 		return -ENODEV;
 	}