RE: [PATCH] lib/crypto: blake2s: fix a CFI failure

[David Laight]

From: Ard Biesheuvel
> Sent: 19 January 2022 12:19
...
> -               (*compress)(state, in, nblocks - 1, BLAKE2S_BLOCK_SIZE);
> +               if (IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S))
> +                       (*compress)(state, in, nblocks - 1, BLAKE2S_BLOCK_SIZE);
> +               else
> +                       blake2s_compress_generic(state, in, nblocks - 1,
> +                                                BLAKE2S_BLOCK_SIZE);

Isn't that a candidate for a 'static call' ?

And, maybe all these inlined functions should be real functions?
No point having all the bloat on every call site.
Much better to call a real function and used the cached instructions.

Although, having looked at the source and the generated code for
x86-64 and arm64 I'm not sure I'd want to try to generate
optimised assembler for it.
(Unless you can a instruction that does exactly what the code wants.)

Basically the compiler can merge the instructions for 4 of the
G() expansions so that they can execute in parallel on a multi-issue
cpu. Doing that by hand will be error prone.
Each G() expansion is pretty much a register dependency chain,
not much chance of parallel execution.

There are clearly optimisations for the top/bottom of the loop.
But they can be done to the generic C version.

The real problem is lack of registers - the code needs 16 for the
v[] array plus a few extras.
So some have to spill to stack.

The unrolled code is about 1200 instructions on arm64 and x86-64.
Each of the 10 rounds reads all 16 of the u32 input values.
So that is about 8 (1200/160) instructions for each read.
Which means there is plenty of memory bandwidth for other
reads.

So 'rolling up' the rounds - which adds in the blake2s_sigma[]
reads could easily be 'almost free'.
Certainly on x86 where you are just (well should be just) adding an
extra memory uop for each input buffer reads.

I'm not sure the 8 G() calls can be folded into two sets of 4
while still getting the compiler to interleave the generated code.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)