Skip to contentSkip to conversation

lib/crypto: x86/chacha: Simplify chacha_dosimd()

PATCH Applied Eric Biggers 2 participants 3 messages Nov 4 – Nov 7

Subsystems (2)
lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

PATCH 1/1 lib/crypto: x86/chacha: Simplify chacha_dosimd()

- Use if-else statements to make the logic clearer, eliminate excessive not-taken branches for certain lengths (e.g., 16-block aligned lengths), and consolidate a lot of the counter increments.

- Eliminate the unnecessary chacha_advance() function. DIV_ROUND_UP(bytes, CHACHA_BLOCK_SIZE) is just as good. The min() clamp did nothing.

- Don't pass a length greater than N blocks to the assembly functions that process N blocks. They all accepted it and just processed the first N blocks anyway, but there's no need to rely on that.

The behavior is unchanged for every length.

---
 lib/crypto/x86/chacha.h | 128 +++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 74 deletions(-)
lib/crypto/x86/chacha.h +54 -74
index ed39b3fdda84..57d644f89431 100644
@@ -48,101 +48,81 @@ static __ro_after_init DEFINE_STATIC_KEY_FALSE(chacha_use_avx2);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(chacha_use_avx512vl);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(chacha_use_avx512);
 
-static size_t chacha_advance(size_t len, size_t maxblocks)
-{
-	len = min(len, maxblocks * CHACHA_BLOCK_SIZE);
-	return round_up(len, CHACHA_BLOCK_SIZE) / CHACHA_BLOCK_SIZE;
-}
-
 static void chacha_dosimd(struct chacha_state *state, u8 *dst, const u8 *src,
 			  size_t bytes, int nrounds)
 {
-	if (static_branch_likely(&chacha_use_avx512)) {
-		while (bytes >= CHACHA_BLOCK_SIZE * 16) {
-			chacha_16block_xor_avx512(state, dst, src, bytes,
-						  nrounds);
-			bytes -= CHACHA_BLOCK_SIZE * 16;
-			src += CHACHA_BLOCK_SIZE * 16;
-			dst += CHACHA_BLOCK_SIZE * 16;
-			state->x[12] += 16;
-		}
-		if (bytes > CHACHA_BLOCK_SIZE * 8) {
-			chacha_16block_xor_avx512(state, dst, src, bytes,
-						  nrounds);
-			state->x[12] += chacha_advance(bytes, 16);
-			return;
-		}
-	}
-
 	if (static_branch_likely(&chacha_use_avx512vl)) {
-		while (bytes >= CHACHA_BLOCK_SIZE * 8) {
-			chacha_8block_xor_avx512vl(state, dst, src, bytes,
-						   nrounds);
-			bytes -= CHACHA_BLOCK_SIZE * 8;
-			src += CHACHA_BLOCK_SIZE * 8;
-			dst += CHACHA_BLOCK_SIZE * 8;
-			state->x[12] += 8;
+		if (static_branch_likely(&chacha_use_avx512)) {
+			while (bytes >= CHACHA_BLOCK_SIZE * 16) {
+				chacha_16block_xor_avx512(
+					state, dst, src, CHACHA_BLOCK_SIZE * 16,
+					nrounds);
+				bytes -= CHACHA_BLOCK_SIZE * 16;
+				src += CHACHA_BLOCK_SIZE * 16;
+				dst += CHACHA_BLOCK_SIZE * 16;
+				state->x[12] += 16;
+			}
+			if (bytes > CHACHA_BLOCK_SIZE * 8) {
+				chacha_16block_xor_avx512(state, dst, src,
+							  bytes, nrounds);
+				state->x[12] +=
+					DIV_ROUND_UP(bytes, CHACHA_BLOCK_SIZE);
+				return;
+			}
+		} else {
+			while (bytes >= CHACHA_BLOCK_SIZE * 8) {
+				chacha_8block_xor_avx512vl(
+					state, dst, src, CHACHA_BLOCK_SIZE * 8,
+					nrounds);
+				bytes -= CHACHA_BLOCK_SIZE * 8;
+				src += CHACHA_BLOCK_SIZE * 8;
+				dst += CHACHA_BLOCK_SIZE * 8;
+				state->x[12] += 8;
+			}
 		}
-		if (bytes > CHACHA_BLOCK_SIZE * 4) {
+		if (bytes > CHACHA_BLOCK_SIZE * 4)
 			chacha_8block_xor_avx512vl(state, dst, src, bytes,
 						   nrounds);
-			state->x[12] += chacha_advance(bytes, 8);
-			return;
-		}
-		if (bytes > CHACHA_BLOCK_SIZE * 2) {
+		else if (bytes > CHACHA_BLOCK_SIZE * 2)
 			chacha_4block_xor_avx512vl(state, dst, src, bytes,
 						   nrounds);
-			state->x[12] += chacha_advance(bytes, 4);
-			return;
-		}
-		if (bytes) {
+		else if (bytes)
 			chacha_2block_xor_avx512vl(state, dst, src, bytes,
 						   nrounds);
-			state->x[12] += chacha_advance(bytes, 2);
-			return;
-		}
-	}
-
-	if (static_branch_likely(&chacha_use_avx2)) {
+		state->x[12] += DIV_ROUND_UP(bytes, CHACHA_BLOCK_SIZE);
+	} else if (static_branch_likely(&chacha_use_avx2)) {
 		while (bytes >= CHACHA_BLOCK_SIZE * 8) {
-			chacha_8block_xor_avx2(state, dst, src, bytes, nrounds);
+			chacha_8block_xor_avx2(state, dst, src,
+					       CHACHA_BLOCK_SIZE * 8, nrounds);
 			bytes -= CHACHA_BLOCK_SIZE * 8;
 			src += CHACHA_BLOCK_SIZE * 8;
 			dst += CHACHA_BLOCK_SIZE * 8;
 			state->x[12] += 8;
 		}
-		if (bytes > CHACHA_BLOCK_SIZE * 4) {
+		if (bytes > CHACHA_BLOCK_SIZE * 4)
 			chacha_8block_xor_avx2(state, dst, src, bytes, nrounds);
-			state->x[12] += chacha_advance(bytes, 8);
-			return;
-		}
-		if (bytes > CHACHA_BLOCK_SIZE * 2) {
+		else if (bytes > CHACHA_BLOCK_SIZE * 2)
 			chacha_4block_xor_avx2(state, dst, src, bytes, nrounds);
-			state->x[12] += chacha_advance(bytes, 4);
-			return;
-		}
-		if (bytes > CHACHA_BLOCK_SIZE) {
+		else if (bytes > CHACHA_BLOCK_SIZE)
 			chacha_2block_xor_avx2(state, dst, src, bytes, nrounds);
-			state->x[12] += chacha_advance(bytes, 2);
-			return;
+		else if (bytes)
+			chacha_block_xor_ssse3(state, dst, src, bytes, nrounds);
+		state->x[12] += DIV_ROUND_UP(bytes, CHACHA_BLOCK_SIZE);
+	} else {
+		while (bytes >= CHACHA_BLOCK_SIZE * 4) {
+			chacha_4block_xor_ssse3(state, dst, src,
+						CHACHA_BLOCK_SIZE * 4, nrounds);
+			bytes -= CHACHA_BLOCK_SIZE * 4;
+			src += CHACHA_BLOCK_SIZE * 4;
+			dst += CHACHA_BLOCK_SIZE * 4;
+			state->x[12] += 4;
 		}
-	}
-
-	while (bytes >= CHACHA_BLOCK_SIZE * 4) {
-		chacha_4block_xor_ssse3(state, dst, src, bytes, nrounds);
-		bytes -= CHACHA_BLOCK_SIZE * 4;
-		src += CHACHA_BLOCK_SIZE * 4;
-		dst += CHACHA_BLOCK_SIZE * 4;
-		state->x[12] += 4;
-	}
-	if (bytes > CHACHA_BLOCK_SIZE) {
-		chacha_4block_xor_ssse3(state, dst, src, bytes, nrounds);
-		state->x[12] += chacha_advance(bytes, 4);
-		return;
-	}
-	if (bytes) {
-		chacha_block_xor_ssse3(state, dst, src, bytes, nrounds);
-		state->x[12]++;
+		if (bytes > CHACHA_BLOCK_SIZE)
+			chacha_4block_xor_ssse3(state, dst, src, bytes,
+						nrounds);
+		else if (bytes)
+			chacha_block_xor_ssse3(state, dst, src, bytes, nrounds);
+		state->x[12] += DIV_ROUND_UP(bytes, CHACHA_BLOCK_SIZE);
 	}
 }
 

base-commit: 9fbbbaefd5f95551da18426c200cf41e1b4255a1
prerequisite-patch-id: 890b33260038bff2e3adfe7867af5919666677fc
prerequisite-patch-id: 36cadf8c358a9778d745637c00368df82a93ce0d
prerequisite-patch-id: b9ef92f4abcdfe8946d645db12187f6293a46381
prerequisite-patch-id: 85904a5ef5aa8a48c311b2e87ed4d2403bd9e351

Signature · 1 line
2.56.0
Tested-by on v1 1/1
#1 lore raw

Tested on Zen 5 (Ryzen 9 9950X) in a KVM guest, on top of the bzhi, vzeroupper, interleaving and keystream-XOR patches. KUnit chacha suite + benchmark (64..16384 B, 3 runs each), compared with the tree without this patch:

  path                 CPU setup              KUnit  perf
  -------------------  ---------------------  -----  ------------
  AVX-512, zmm         native                 pass   -0.4..+0.6%
  AVX-512VL, ymm only  spoofed SKX (6/0x55)   pass   -0.3..+0.1%
  AVX2                 clearcpuid=avx512f     pass   -0.9..+1.3%
  SSSE3                clearcpuid=avx2        pass   -0.7..+0.3%

Generated assembly is unchanged.

Signature · 5 lines
Antonio Quartulli

CEO and Co-Founder
Mandelbit Srl
https://www.mandelbit.com
author maintainer Applied
#2 lore raw
In reply to Eric Biggers
Quoted 12 lines · Eric Biggers

- Use if-else statements to make the logic clearer, eliminate excessive not-taken branches for certain lengths (e.g., 16-block aligned lengths), and consolidate a lot of the counter increments.

- Eliminate the unnecessary chacha_advance() function. DIV_ROUND_UP(bytes, CHACHA_BLOCK_SIZE) is just as good. The min() clamp did nothing.

- Don't pass a length greater than N blocks to the assembly functions that process N blocks. They all accepted it and just processed the first N blocks anyway, but there's no need to rely on that.

Quoted 6 lines

The behavior is unchanged for every length.

---
 lib/crypto/x86/chacha.h | 128 +++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 74 deletions(-)

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-next

- Eric