Hi Elliott, Robert
Thank you so much for your review!
On 8/25/22 04:35, Elliott, Robert (Servers) wrote:
>> -----Original Message-----
>> From: Taehee Yoo <ap420073@xxxxxxxxx>
>> Sent: Wednesday, August 24, 2022 10:59 AM
>> Subject: [PATCH 2/3] crypto: aria-avx: add AES-NI/AVX/x86_64 assembler
>> implementation of aria cipher
>>
> ...
>
>> +#include "ecb_cbc_helpers.h"
>> +
>> +asmlinkage void aria_aesni_avx_crypt_16way(const u32 *rk, u8 *dst,
>> + const u8 *src, int rounds);
>> +
>> +static int ecb_do_encrypt(struct skcipher_request *req, const u32
*rkey)
>> +{
>> + struct aria_ctx *ctx =
>> crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
>> + struct skcipher_walk walk;
>> + unsigned int nbytes;
>> + int err;
>> +
>> + err = skcipher_walk_virt(&walk, req, false);
>> +
>> + while ((nbytes = walk.nbytes) > 0) {
>> + const u8 *src = walk.src.virt.addr;
>> + u8 *dst = walk.dst.virt.addr;
>> +
>> + kernel_fpu_begin();
>> + while (nbytes >= ARIA_AVX_BLOCK_SIZE) {
>> + aria_aesni_avx_crypt_16way(rkey, dst, src, ctx->rounds);
>> + dst += ARIA_AVX_BLOCK_SIZE;
>> + src += ARIA_AVX_BLOCK_SIZE;
>> + nbytes -= ARIA_AVX_BLOCK_SIZE;
>> + }
>> + while (nbytes >= ARIA_BLOCK_SIZE) {
>> + aria_encrypt(ctx, dst, src);
>> + dst += ARIA_BLOCK_SIZE;
>> + src += ARIA_BLOCK_SIZE;
>> + nbytes -= ARIA_BLOCK_SIZE;
>> + }
>> + kernel_fpu_end();
>
> Is aria_encrypt() an FPU function that belongs between kernel_fpu_begin()
> and kernel_fpu_end()?
>
> Several drivers (camellia, serpent, twofish, cast5, cast6) use
ECB_WALK_START,
> ECB_BLOCK, and ECB_WALK_END macros for these loops. Those macros do
only call
> kernel_fpu_end() at the very end. That requires the functions to be
structured
> with (ctx, dst, src) arguments, not (rkey, dst, src, rounds).
>
aria_{encrypt | decrypt}() are not FPU functions.
I think if it uses the ECB macro, aria_{encrypt | decrypt}() will be
still in the FPU context.
So, I will get them out of the kernel FPU context.
Thank you so much!
Taehee Yoo
