Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to crypto_test app; STM32 crypto HW driver; CBC/CTR speed #2174

Merged
merged 3 commits into from
Mar 4, 2020

Conversation

utzig
Copy link
Member

@utzig utzig commented Jan 31, 2020

  • apps: crypto_test: add benchmarks for CBC/CTR
  • stm32: crypto: make driver obey syscfg HW support
  • crypto: use 32-bit XOR in CBC/CTR

@utzig utzig force-pushed the crypto-updates branch 3 times, most recently from 1ff621f to 5a652be Compare February 3, 2020 14:07
@utzig
Copy link
Member Author

utzig commented Feb 25, 2020

Could someone take a look?

inbuf32 = (uint32_t *)inbuf8;
outbuf32 = (uint32_t *)outbuf8;
for (i = 0; i < len / 4; i++) {
outbuf32[i] = inbuf32[i] ^ _out32[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what is the usage of this function but unaligned access here could crash Cortex-M0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed now, thanks!

}
}
}
printf("done in %lu ticks\n", os_time_get() - t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not use ticks to display time, you could not compare results of two different boards not knowing what tick mean on each one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing to different boards is not relevant IMO, what is being done here is to compare the HW acceleration speed to SW implementations using tinycrypt/mbedTLS.

1) The table generation script was updated to save a block of CBC and
   CTR encrypted buffers (plus IV/NONCE).
2) Add new routines to run CBC/CTR and count cycles.

Signed-off-by: Fabio Utzig <utzig@apache.org>
Allow disabling of HW support for CBC/CTR. This is a correctness change;
nobody should do this in practice unless for testing the SW fallbacks or
in the unlikely case that there is a BUG found in one of those
encryption/decryption modes.

Signed-off-by: Fabio Utzig <utzig@apache.org>
Using 32-bit XORs is around 10% faster with slightly increased code size
(for optimized target). For CBC always requires AES block length buffers
so just employ 32-bit XORs. For CTR due to the stream semantics, use
32-bit XOR when buffer is AES block length size, otherwise fallback to
byte-by-byte XOR.

Signed-off-by: Fabio Utzig <utzig@apache.org>
@apache-mynewt-bot
Copy link

Style check summary

Our coding style is here!

hw/drivers/crypto/src/crypto.c

@@ -77,11 +77,11 @@
             }
         } else {
 #endif
-            for (i = 0; i < len; i++) {
-                outbuf8[i] = inbuf8[i] ^ _out[i];
-            }
-#if defined(__ARM_FEATURE_UNALIGNED)
-        }
+        for (i = 0; i < len; i++) {
+            outbuf8[i] = inbuf8[i] ^ _out[i];
+        }
+#if defined(__ARM_FEATURE_UNALIGNED)
+    }
 #endif
 
         for (i = AES_BLOCK_LEN; i > 0; --i) {

Copy link
Contributor

@kasjer kasjer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@utzig utzig merged commit 6897f5d into apache:master Mar 4, 2020
@utzig utzig deleted the crypto-updates branch March 4, 2020 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants