Re: [PATCH] AES for CryptoAPI - i586-optimized

Re: [PATCH] AES for CryptoAPI - i586-optimized

[Andi Kleen]

Christoph Hellwig <hch@infradead.org> writes:

> On Tue, Jul 08, 2003 at 05:27:55PM +0200, Fruhwirth Clemens wrote:
> > 
> > Due to the recent discussion about the asm-optimized version of AES which is
> > included in loop-AES, I'd like to point out that I've ported this
> > implementation - Dr. Brian Gladman's btw. - to CryptoAPI a long time ago.
> 
> Cool, that means we just need to hash out the framework for optimized
> implementations now..

It's not cool. Pentium Classic tuning is quite useless for PPro+
The Pentium Classic had a quite weird pipeline and code optimized
for it tends to be suboptimal for more modern designs.

I didn't benchmark, but I suspect the C version compiled with a good compiler
for the correct CPU is faster than that on a modern CPU.

-Andi

Re: [PATCH] AES for CryptoAPI - i586-optimized

[Jeff Garzik]

On Tue, Jul 08, 2003 at 07:12:27PM +0200, Andi Kleen wrote:
> Christoph Hellwig <hch@infradead.org> writes:
> 
> > On Tue, Jul 08, 2003 at 05:27:55PM +0200, Fruhwirth Clemens wrote:
> > > 
> > > Due to the recent discussion about the asm-optimized version of AES which is
> > > included in loop-AES, I'd like to point out that I've ported this
> > > implementation - Dr. Brian Gladman's btw. - to CryptoAPI a long time ago.
> > 
> > Cool, that means we just need to hash out the framework for optimized
> > implementations now..
> 
> It's not cool. Pentium Classic tuning is quite useless for PPro+
> The Pentium Classic had a quite weird pipeline and code optimized
> for it tends to be suboptimal for more modern designs.
> 
> I didn't benchmark, but I suspect the C version compiled with a good compiler
> for the correct CPU is faster than that on a modern CPU.

I agree 100% with what you state here... but at the same time I was
thinking it would be nice to merge, mainly as an example of asm support
if nothing else.

We really need to see benchmarks before merging, too, to see (as you
state) if the C compiler still does a better job.

	Jeff

Re: [PATCH] AES for CryptoAPI - i586-optimized

[Andi Kleen]

On Tue, 8 Jul 2003 13:28:48 -0400
Jeff Garzik <jgarzik@pobox.com> wrote:


> I agree 100% with what you state here... but at the same time I was
> thinking it would be nice to merge, mainly as an example of asm support
> if nothing else.

There already is an example for optimized variants - the z990 implementation.

-Andi

Re: [PATCH] AES for CryptoAPI - i586-optimized

[Jason Papadopoulos]

 > There already is an example for optimized variants - the z990 
implementation.
 >
 > -Andi

My Master's thesis involved developing custom AES implementations
for the Alpha EV6, UltraSPARC, and P3.

www.boo.net/~jasonp

Source is included, but only the encrypt was implemented.

Hope this helps,
jasonp

[PATCH] AES for CryptoAPI - i586-optimized

[Fruhwirth Clemens]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]


Due to the recent discussion about the asm-optimized version of AES which is
included in loop-AES, I'd like to point out that I've ported this
implementation - Dr. Brian Gladman's btw. - to CryptoAPI a long time ago.

http://therapy.endorphin.org/patches/aes-i586-asm-2.5.58.diff

The demand for this patch will rise quickly if cryptoloop is included :) 

Regards, Clemens

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [PATCH] AES for CryptoAPI - i586-optimized

[Christoph Hellwig]

On Tue, Jul 08, 2003 at 05:27:55PM +0200, Fruhwirth Clemens wrote:
> 
> Due to the recent discussion about the asm-optimized version of AES which is
> included in loop-AES, I'd like to point out that I've ported this
> implementation - Dr. Brian Gladman's btw. - to CryptoAPI a long time ago.

Cool, that means we just need to hash out the framework for optimized
implementations now..

A few more comments:

> diff -r --new-file -u crypto/Kconfig ../linux-2.5.58/crypto/Kconfig
> --- crypto/Kconfig	Thu Feb  6 13:53:47 2003
> +++ ../linux-2.5.58/crypto/Kconfig	Tue Feb  4 00:54:18 2003
> @@ -119,6 +119,26 @@
>  
>  	  See http://csrc.nist.gov/encryption/aes/ for more information.
>  
> +config CRYPTO_AES_586
> +	tristate "AES cipher algorithms (586)"
> +	depends on CRYPTO

Should also depend on CONFIG_X86 && !CONFIG_X86_64

> +$(obj)/aes-i586.o: $(obj)/aes-i586-asm.o crypto/aes-i586-glue.o
> +	$(LD) -r $(obj)/aes-i586-asm.o $(obj)/aes-i586-glue.o -o $(obj)/aes-i586.o

That's not how kernel makesfile work.  It should be something like

aes-i586-y		:= aes-i586-asm.o aes-i586-glue.o

> +// THE CIPHER INTERFACE

Please use C-style comments.

> +	if(key_length != 16 && key_length != 24 && key_length != 32)
> +	{

Should be

	if (key_length != 16 && key_length != 24 && key_length != 32) {

> +MODULE_DESCRIPTION("Rijndael (AES) Cipher Algorithm");
> +MODULE_LICENSE("Dual BSD/GPL");

MODULE_AUTHOR is missing.  Also the description should mention that
this is an optimized assembly version.