linux-kernel - Re: [RFC PATCH crypto] AES: Add support to Intel AES-NI instructions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081215090745.GA4991@Chamillionaire.breakpoint.cc>
Date:	Mon, 15 Dec 2008 10:07:45 +0100
From:	Sebastian Andrzej Siewior <linux-crypto@...breakpoint.cc>
To:	Huang Ying <ying.huang@...el.com>
Cc:	Herbert Xu <herbert@...dor.apana.org.au>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
	"mingo@...e.hu" <mingo@...e.hu>,
	"tglx@...utronix.de" <tglx@...utronix.de>
Subject: Re: [RFC PATCH crypto] AES: Add support to Intel AES-NI
	instructions

* Huang Ying | 2008-12-15 10:19:02 [+0800]:

>> Nice work. A few things:
>> - Did you rename the "old" x86 functions to avoid a clash?
>>   So you bypass the crypto layer in case you can't handle the operation.
>>   Does this improve the performace or just saves key strokes? Not sure
>>   what the best sollution could be....
>
>The general x86 implementation is used as the fall back for new AES-NI
>based implementation. Because AES-NI can not be used in kernel soft_irq
>context. If crypto layer is used to access general x86 implementation,
>we will have tfm_ctx alignment issue, because AES-NI need tfm_ctx to be
>16 byte aligned.
>
>> - unless I've read the code too fast, it does not work if someone sets the
>>   key in user context and starts an encryption in softirq context.
>
>Oh, I should use struct crypto_aes_ctx instead of define struct aes_ctx.
>The tfm_ctx definition is different. I will fix this. Except that, is
>there any other issue?
Now I see what you have done. You are sharing the same tfm between your
aes version and the "old" asm version. Both assume different private
data (aes_ctx vs crypto_aes_ctx) so this should not work.

>> - aes_ctx is somehow bad. You are using this for a function and a
>>   struct. An Intel prefix would be nice (in case of the struct). On a
>>   second thought, any reason why you can't use crypto_aes_ctx?
>
>Yes. I will use that. The only issue is that AES-NI need scheduled key
>to be 16 bytes aligned, so we need move key_length in struct
>crypto_aes_ctx to the end of the struct.
You have to it if you want to bypass the crypto layer and call asm
functions directly and I'm not sure whether bypassing the crypto layer
is a good thing. Both asm routines (the 32bit and 64bit) assume that
keylen ist at +0 followed by enc key, dec key. Ach and they don't do the
ALIGN thing.

Herbert what do you thing?

>> - Is this an Intel thing or is going to be part of X86 and also
>>   available to others (like mmx). In that case the Intel prefix may be
>>   "wrong".
>
>Now it is an Intel thing. But in the future it may become part of x86.
>Intel named it as AES-NI (AES New Instructions), but name like
>aes_ni_aes_set_key is not good too. Does aes_ni_set_key sound better?
The latter is fine. I just want to avoid having several and different
aes_set_key() around.

>> - does the cpu support more than just pure aes e.g. block modes? In case
>>   it does not, does the perfomance improve if you implement lets say
>>   cbc(aes) and do the xor with sse in order to save a few
>>   kernel_fpu_begin() calls? I'm just asking because I saw a similar
>>   thing and PowerPC and the AltiVec unit. Maybe it is cheap on x86 :)
>
>Yes. AES-NI can benefit not only block modes. And the pipeline
>implementation of AES-NI can benefit cbc(aes) decryption and ctr(aes)
>even more (described in detail in white paper). We will work on that.
Ah okay. So depending on how expensive kernel_fpu_begin() really is it
might be slower for just the cipher (i.e. in xts(aes) where xts calls
you for every 16bytes).

>
>Best Regards,
>Huang Ying
>

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/