On Tue, Apr 12, 2022 at 05:28:13PM +0000, Nathan Huckleberry wrote:
> diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
> index dc35eb0245c5..ac37e2f7ca84 100644
> --- a/arch/arm64/crypto/aes-modes.S
> +++ b/arch/arm64/crypto/aes-modes.S
> @@ -479,6 +479,140 @@ ST5( mov v3.16b, v4.16b )
> b .Lctrout
> AES_FUNC_END(aes_ctr_encrypt)
>
> + /*
> + * aes_xctr_encrypt(u8 out[], u8 const in[], u8 const rk[], int rounds,
> + * int bytes, u8 const ctr[], u8 finalbuf[], int
> + * byte_ctr)
> + */
> +
What is the 'finalbuf' parameter for? It is never used.
Why is byte_ctr an 'int' here but an 'unsigned int' in the .c file?
It looks like 'ctr' is actually the IV; perhaps it should be called 'iv' to
distinguish it from the byte_ctr?
As mentioned elsewhere, please don't have a line break between a parameter's
type and name.
Generally, comments and register aliases would be super helpful throughout the
code. As-is, this is much harder to read than the x86 version...
Also, this function is heavily duplicated with aes_ctr_encrypt. Did you
consider generating both from a single macro, like you did with the x86 version?
> + umov x12, vctr.d[0] /* keep ctr in reg */
/* keep first 8 bytes of IV in reg */
> + lsr x7, x7, #4
x7 needs to be w7, since it corresponds to a 32-bit parameter ('int byte_ctr').
The upper 32 bits of the register are not guaranteed to be zero.
> + sub x7, x11, #MAX_STRIDE
> + eor x7, x12, x7
> + ins v0.d[0], x7
> + sub x7, x11, #MAX_STRIDE - 1
> + sub x8, x11, #MAX_STRIDE - 2
> + eor x7, x7, x12
> + sub x9, x11, #MAX_STRIDE - 3
> + mov v1.d[0], x7
> + eor x8, x8, x12
> + eor x9, x9, x12
> +ST5( sub x10, x11, #MAX_STRIDE - 4)
> + mov v2.d[0], x8
> + eor x10, x10, x12
> + mov v3.d[0], x9
> +ST5( mov v4.d[0], x10 )
There seem to be some unnecessarily tight instruction dependencies here. E.g.,
the first 3 instructions are all sequential. Are there not enough free
registers to write it otherwise? I.e. do all the sub's first, then the eor's,
then the mov's.
The trailing parenthesis after #MAX_STRIDE - 4 should be indented another level.
As-is it looks like a typo.
Why does one place use 'ins' and the others use 'mov'?
- Eric
