Re: [PATCH 02/35] target/arm: Move aesmc and aesimc tables to crypto/aes.c

[Philippe Mathieu-Daudé]

On 3/6/23 04:33, Richard Henderson wrote:
> We do not currently have a table in crypto/ for
> just MixColumns.  Move both tables for consistency.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/crypto/aes.h           |   6 ++
>   crypto/aes.c                   | 142 ++++++++++++++++++++++++++++++++
>   target/arm/tcg/crypto_helper.c | 143 ++-------------------------------
>   3 files changed, 153 insertions(+), 138 deletions(-)


>       union CRYPTO_STATE st = { .l = { rm[0], rm[1] } };
> +    const uint32_t *mc = decrypt ? AES_imc_rot : AES_mc_rot;
>       int i;
>   
>       for (i = 0; i < 16; i += 4) {
>           CR_ST_WORD(st, i >> 2) =
> -            mc[decrypt][CR_ST_BYTE(st, i)] ^
> -            rol32(mc[decrypt][CR_ST_BYTE(st, i + 1)], 8) ^
> -            rol32(mc[decrypt][CR_ST_BYTE(st, i + 2)], 16) ^
> -            rol32(mc[decrypt][CR_ST_BYTE(st, i + 3)], 24);
> +            mc[CR_ST_BYTE(st, i)] ^
> +            rol32(mc[CR_ST_BYTE(st, i + 1)], 8) ^
> +            rol32(mc[CR_ST_BYTE(st, i + 2)], 16) ^
> +            rol32(mc[CR_ST_BYTE(st, i + 3)], 24);

Matter of style, (since you are changing these lines), I find starting
the lines with the ^ operator clearer to review:

            mc[CR_ST_BYTE(st, i)]
            ^ rol32(mc[CR_ST_BYTE(st, i + 1)], 8)
            ^ rol32(mc[CR_ST_BYTE(st, i + 2)], 16)
            ^ rol32(mc[CR_ST_BYTE(st, i + 3)], 24);

Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>       }
>   
>       rd[0] = st.l[0];