On 4/13/21 7:09 PM, Bjorn Andersson wrote:
On Tue 13 Apr 17:44 CDT 2021, Thara Gopinath wrote:
[..]
Yes, given that you just typecast things as you do it should just work
to move the typecast to the qce_cpu_to_be32p_array().
But as I said, this would indicate that what is cpu_to_be32() should
have been be32_to_cpu() (both performs the same swap, it's just a matter
of what type goes in and what type goes out).
Looking at the other uses of qce_cpu_to_be32p_array() I suspect that
it's the same situation there, so perhaps introduce a new function
qce_be32_to_cpu() in this patchset (that returns number of words
converted) and then look into the existing users after that?
Hi!
I have sent out the v2 with the new function. To me, it does look
cleaner. So thanks!
[..]
+ if (IS_SHA_HMAC(rctx->flags)) {
+ /* Write default authentication iv */
+ if (IS_SHA1_HMAC(rctx->flags)) {
+ auth_ivsize = SHA1_DIGEST_SIZE;
+ memcpy(authiv, std_iv_sha1, auth_ivsize);
+ } else if (IS_SHA256_HMAC(rctx->flags)) {
+ auth_ivsize = SHA256_DIGEST_SIZE;
+ memcpy(authiv, std_iv_sha256, auth_ivsize);
+ }
+ authiv_words = auth_ivsize / sizeof(u32);
+ qce_write_array(qce, REG_AUTH_IV0, (u32 *)authiv, authiv_words);
AUTH_IV0 is affected by the little endian configuration, does this imply
that IS_SHA_HMAC() and IS_CCM() are exclusive bits of rctx->flags? If so
I think it would be nice if you grouped the conditionals in a way that
made that obvious when reading the function.
So yes IS_SHA_HMAC() and IS_CCM() are exclusive bits of rctx->flags.
AUTH_IVn is 0 for ccm and has initial value for HMAC algorithms. I don't
understand the confusion here.
I'm just saying that writing is as below would have made it obvious to
me that IS_SHA_HMAC() and IS_CCM() are exclusive:
So regardless of the mode, it is a good idea to clear the IV registers
which happens above in
qce_clear_array(qce, REG_AUTH_IV0, 16);
This is important becasue the size of IV varies between HMAC(SHA1) and
HMAC(SHA256) and we don't want any previous bits sticking around.
For CCM after the above step we don't do anything with AUTH_IV where as for
SHA_HMAC we have to go and program the registers. I can split it into
if (IS_SHA_HMAC(flags)) {
...
} else {
...
}
but both snippets will have the above line code clearing the IV register and
the if part will have more stuff actually programming these registers.. Is
that what you are looking for ?
I didn't find an answer quickly to the question if the two where
mutually exclusive and couldn't determine from the code flow either. But
my comment seems to stem from my misunderstanding that the little endian
bit was dependent on IS_CCM().
That said, if the logic really is "do this for IS_SHA_HMAC(), otherwise
do that", then if else makes sense.
So, the logic is really. do "clearing of IV" in all cases. Do setting of
initial IV only for HMAC. I tried the if..else like you said. It did not
look correct to duplicate the code clearing the IV. So I have added
comments around this section hopefully making it clearer. Do take a look
and let me know. I can rework it further if you think so.
Regards,
Bjorn
--
Warm Regards
Thara
