Krishna Yenduri wrote: > Dan, > Sorry for the delayed review. My comments are below. > I haven't reviewed the license files. > >> Here's a review for AES optimization on OpenSolaris x86 64-bit: >> 5072963 Need an optimized AES implementation for amd64 >> http://dan.drydog.com/reviews/5072963-aes/ > > KY-1 usr/src/common/crypto/aes/aes_cbc_crypt.c > Lines 296-308 and others. > I don't think this is needed since bcopy() has optimization for > the word aligned case. Did you see any improved performance > with this change?
I ran encrypt(1) with a "borrowed" dtrace script and found bcopy was taking something like 1/4 of the total time. Looking further, I found several bcopy() calls in aes_cbc_crypt.c, but only 3 were being used for CBC mode. I replaced these 3 calls with direct assignments and shaved off another 5% from the execution time on x64. The same code also saved 5% on sparc sun4u. > > KY-2 The file usr/src/common/crypto/aes/aes_impl.c > is missing from this webrev. I believe you need to > modify that file so that the C routines for aes_encrypt_impl > and aes_decrypt_impl are not used. FIXED. Also added aes_impl.h. The files were in an earlier version of the webrev, but I think my "evil twin" removed them. Please review aes_impl.c and aes_impl.h. > > KY-3 usr/src/common/crypto/aes/amd64/aes_amd64.s > Does this assembly code follow the Solaris AMD64 ABI > for parameter passing conventions? I am asking this because the code > seems to have been originally developed for Windows OS. Yes. Gladman's AES code was originally implemented with a .ifdef to implement both the Linux/Solaris AMD64 ABI and the Windows ABI. I removed the Windows ABI code for clarity. -- Dan.Anderson at Sun.COM, Sun Microsystems, San Diego, CA, US +1-858-526-9418 or x55418, http://namefinder/NameFinder?nfquery=73024 -------------- next part -------------- A non-text attachment was scrubbed... Name: smime.p7s Type: application/x-pkcs7-signature Size: 3261 bytes Desc: S/MIME Cryptographic Signature URL: <http://mail.opensolaris.org/pipermail/security-discuss/attachments/20080611/d862971c/attachment.bin>
