Hi Herbert,
the updated patch addresses almost all your comments.
Changes since last time:
* Removed data_len and fallback_driver_name from struct padlock_sha_ctx
* PADLOCK_CRA_PRIORITY moved to padlock.h
* #define CTX(tfm) converted into inline function ctx()
* Changed padlock_htonl_bloc() renamed, changed to use swab32(),
doesn't work in-place anymore (i.e. removed some memcpy() as well)
* Return -ENOMEM from cra_init if get_free_page() failed.
* Plus a few minor changes.
>> +#include "padlock.h"
>> +
>> +#define PADLOCK_CRA_PRIORITY 300
>
> Perhaps this macro can be moved into padlock.h since it's shared
> with AES?
Right, patch follows.
> Please use u8/u16/u32/u64 instead of uintX_t for kernel code. Until
> there is a change in general kernel policy it's better to stick with
> the prevailing coding style.
I prefer to keep uintX_t - quick grep reveals these are not uncommon in
the kernel. After all - linux people always rant about standards and
uint32_t *is* a standard while u32 is not (Plus I get a nice green syntax
highlighting for the *standard* types ;-)
>> +void padlock_do_sha1(const char *in, char *out, int count)
>> +{
>> + /* We can't store directly to *out as it
>> + * doesn't have to be aligned. But who cares,
>> + * it's only a few bytes... */
>> + char buf[128+16];
>
> Does it really need 128 bytes?
Yes, CPU temporarily stores some data in there (IIRC in case the input
buffer is not 16 Bytes-aligned it realigns it there for the SSE input
microcode).
> It's also better to put this in the
> ctx since there it's easier to get the required alignment (see what
> I did to the padlock AES driver).
>
> Also the output buffer is guaranteed to be aligned because you've
> set the alignment mask.
This buffer is only used during the final hashing phase so it could easily
be on the stack. I didn't want to carry it around in CTX for each TFM.
OTOH if I change the engine later to work without fallbacks I'll need to
store the intermediate results. I'll make this change once needed.
Any other comments?
Michal
-
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
