MAX32665 and MAX32666 TPU HW and ARM ASM Crypto Callback Support #7777

night1rider · 2024-07-23T06:28:22Z

Description

MAX32665 and MAX32666 TPU HW

This adds support for the TPU HW acceleration on the MAX32665 and MAX32666 from Analog Devices
Code and Limitations where determined from the User Guide UG6971; Rev 3; 2/2022

Maxim SDK can be found here

Setup to run using these defines to run:

#define WOLFSSL_MAX3266X
#define WOLFSSL_MAX3266X_OLD - only needed if with old SDK
#define MAX3266X_RTC - only setup for new sdk and not old for baremetal benchmarking
#define WOLFSSL_SP_MATH_ALL

ARM ASM Crypto Callback support

Added in Ability to use Callbacks with ARM ASM with the following algos

AES ECB: 128, 192, 256
AES CBC: 128, 192, 256
SHA-1
SHA-256
SHA-384
SHA-512

Tested with MAX32666 dev board with these settings
ARM ASM Settings:

#define WOLFSSL_ARMASM
#define WOLFSSL_ARMASM_INLINE
#define WOLFSSL_ARMASM_NO_HW_CRYPTO
#define WOLFSSL_ARMASM_NO_NEON
#define WOLFSSL_ARM_ARCH 7
#define GCM_TABLE

Testing

wolfCrypt Test and wolfCrypt Benchmark on Baremetal with the MAX32666FTHR with new SDK
wolfCrypt Test and wolfCrypt Benchmark on FreeRTOS with custom hardware with MAX32666 with old SDK

Checklist

HW Acceleration

Side Items

Jenkins Test Nightly using the MAX32666FTHR
Update wolfSSL Manual
Update Readme in wolfssl/wolfcrypt/port/maxim/

night1rider · 2024-07-23T16:10:57Z

Bug with MAA report here for visibility: analogdevicesinc/msdk#1089
I will be looking to file a ticket on Analogs website as well.

night1rider · 2024-07-23T16:32:54Z

I do plan on fixing, max3266x.c and max3266x.h to use proper wolfSSL types. Have had some compile issues when using byte instead of char type

wolfcrypt/src/port/maxim/max3266x.c

dgarske · 2024-07-23T17:15:05Z

wolfcrypt/src/port/maxim/max3266x.c

@@ -0,0 +1,921 @@
+/* max3266x.c
+ *
+ * Copyright (C) 2006-2023 wolfSSL Inc.


dgarske · 2024-07-23T17:15:23Z

wolfcrypt/src/port/maxim/max3266x.c

+
+#include <wolfssl/wolfcrypt/settings.h>
+
+#warning "Inside max3266x.c"


Please remove these.

Removed, leftover from debugging

dgarske · 2024-07-23T17:15:27Z

wolfcrypt/src/port/maxim/max3266x.c

+
+#warning "Inside max3266x.c"
+#if defined(WOLFSSL_MAX32665) ||  defined(WOLFSSL_MAX32666)
+#warning "Inside max3266x.c define"


Please remove these.

dgarske · 2024-07-23T17:15:47Z

wolfcrypt/src/port/maxim/max3266x.c

+#endif
+
+#include <stdint.h>
+#include <wolfssl/internal.h>


Do you really need internal.h?

No, I will need #include <stdarg.h> for the int wc_MXC_MAA_Fallback(unsigned int count, ...) function.

dgarske · 2024-07-23T17:27:26Z

wolfssl/wolfcrypt/port/maxim/max3266x.h

+    int wc_MXC_TPU_Init(void);
+    int wc_MXC_TPU_Shutdown(void);
+    int wc_MXC_error(int *ret); /* Convert Errors to wolfCrypt Codes */
+    //int wc_MXC_AesSetKey(Aes* aes, byte* userKey, unsigned int keylen);


Dead, remove?

dgarske · 2024-07-23T17:27:39Z

wolfssl/wolfcrypt/port/maxim/max3266x.h

+    extern "C" {
+#endif
+
+    int wc_MXC_TPU_Init(void);


Please mark internal functions with WOLFSSL_LOCAL

dgarske · 2024-07-23T17:27:56Z

wolfssl/wolfcrypt/port/maxim/max3266x.h

+        typedef wc_MXC_Sha wc_Sha256;
+        #define WC_SHA256_TYPE_DEFINED
+
+        // Define the SHA-256 digest for an empty string as a constant byte array


Only C style comment /* */

dgarske · 2024-07-23T17:28:55Z

wolfssl/wolfcrypt/port/maxim/max3266x.h

+        #define WC_SHA256_TYPE_DEFINED
+
+        // Define the SHA-256 digest for an empty string as a constant byte array
+        static const unsigned char MXC_EMPTY_DIGEST_SHA256[32] =


Use WC_SHA256_DIGEST_SIZE. Please use byte array { 0xe3, 0xb0, ...};

dgarske · 2024-07-23T17:29:14Z

wolfssl/wolfcrypt/port/maxim/max3266x.h

+
+#if defined(MAX3266X_MATH)
+
+    int hw_mulmod();


Add arguments... They are not ().

Yes you are right! This is part of the same reason I am using char instead of byte, I am having issues with including wolfssl types and structs to max3266x.h. I am exploring a better solution and this is a temp solution to make sure the implementation works (HW works as intended).

dgarske · 2024-07-25T16:17:31Z

wolfcrypt/src/aes.c

@@ -2888,6 +2892,15 @@ static WARN_UNUSED_RESULT int wc_AesEncrypt(
    }
 #endif

+#if defined(MAX3266X_AES)
+    word32 keySize;
+    if(wc_AesGetKeySize(aes, &keySize) == 0){


NIT: Please add space if (

Updated if statements in aes.c and max3266x.c

dgarske · 2024-07-25T16:18:16Z

wolfcrypt/src/aes.c

+            return BAD_LENGTH_E;
+        }
+#endif
+        if (blocks == 0)


Yea, this was a leftover copy paste, removed the blocks variable, and using sz instead.

dgarske · 2024-07-25T16:18:29Z

wolfcrypt/src/aes.c

+        if (blocks == 0)
+            return 0;
+
+        iv      = (byte*)aes->reg;


NIT: Reduce space after iv.

dgarske · 2024-07-25T16:19:38Z

wolfcrypt/src/port/maxim/max3266x.c

+    #define MXC_WORD_SIZE               SP_WORD_SIZE
+#endif
+
+#define MXC_MAA_MAX_SIZE (2048)/MXC_WORD_SIZE


#define MXC_MAA_MAX_SIZE (2048 / MXC_WORD_SIZE)

dgarske · 2024-07-25T16:20:11Z

wolfcrypt/src/port/maxim/max3266x.c

+    #if defined(WOLFSSL_MAX3266X_OLD)
+        MXC_TPU_Shutdown(); /* Is a void return in older SDK */
+    #else
+        if(MXC_TPU_Shutdown(MXC_SYS_PERIPH_CLOCK_TRNG) != EXIT_SUCCESS){


NIT: if ( and ) {. Please reduce indent of 69-76 4 spaces.

updated and fixed indenting

dgarske · 2024-07-25T16:26:42Z

wolfcrypt/src/port/maxim/max3266x.c

+    mp_int multiplicand;
+    multiplicand.dp[0] = 0x01;
+    multiplicand.used = 1;
+    WOLFSSL_MSG("Preparing exptmod MAA HW Call");


Again consider a new local file debugging macro for this level of verbosity.

wolfcrypt/src/port/maxim/max3266x.c

dgarske · 2024-07-25T16:27:19Z

wolfcrypt/src/port/maxim/max3266x.c

+
+int hw_sqrmod(mp_int* base, mp_int* mod, mp_int* result)
+{
+        if (base->used == 0) {


NIT: Fix indent.

dgarske · 2024-07-25T16:27:34Z

wolfcrypt/src/port/maxim/max3266x.c

+    else{
+        return wc_MXC_MAA_sqrmod(base, mod, result);
+    }
+


NIT: Remove extra line.

dgarske · 2024-07-25T16:27:57Z

wolfcrypt/src/port/maxim/max3266x.c

+}
+
+/* Function to provide the current time as a double */
+double wc_MXC_RTC_Time(void) {


NIT: Functions should have starting/ending brace on their own line.

was overlooked, fixed.

night1rider · 2024-07-26T15:42:07Z

Jenkins retest this please

dgarske · 2024-07-26T22:07:34Z

wolfcrypt/src/aes.c

@@ -3611,6 +3624,15 @@ static WARN_UNUSED_RESULT int wc_AesDecrypt(
    } /* else !wc_esp32AesSupportedKeyLen for ESP32 */
 #endif

+#if defined(MAX3266X_AES)
+    word32 keySize;


Mixed declaration. Please move this top top of function and macro guard.

updated for wc_AesDecrypt and wc_AesEncrypt

dgarske · 2024-07-26T22:09:09Z

wolfcrypt/src/port/maxim/max3266x.c

+int wc_MXC_MAA_adjustUsed(unsigned int *array, unsigned int length)
+{
+    int lastNonZeroIndex = -1; /* Track the last non-zero index */
+    for (int i = 0; i < length; i++) {


Mixed declaration. Move the variable declarations to top.

dgarske · 2024-07-26T22:09:26Z

wolfcrypt/src/port/maxim/max3266x.c

+    va_start(args, count);
+    unsigned int largest = va_arg(args, unsigned int);
+
+    for (int i = 1; i < count; i++) {


Mixed declaration. Move the variable declarations to top.

dgarske · 2024-07-26T22:09:54Z

wolfcrypt/src/port/maxim/max3266x.c

+    /* Create an array to compare values to to check edge for error edge case */
+    mp_digit *zero_tmp = XMALLOC(multiplier->size*(sizeof(mp_digit)), NULL,
+                                    DYNAMIC_TYPE_TMP_BUFFER);
+    XMEMSET(zero_tmp, 0x00, multiplier->size*(sizeof(mp_digit)));


NIT: Please use (multiplier->size*sizeof(mp_digit))

dgarske · 2024-07-26T22:10:14Z

wolfcrypt/src/port/maxim/max3266x.c

+    }
+
+    /* Create an array to compare values to to check edge for error edge case */
+    mp_digit *zero_tmp = XMALLOC(multiplier->size*(sizeof(mp_digit)), NULL,


Need explicit cast to mp_digit* and move variable declaration to top

dgarske · 2024-09-18T21:48:36Z

wolfssl/wolfcrypt/port/maxim/max3266x-cryptocb.h

+    #define WOLFSSL_MAX3266X_DEVID 9
+#endif
+#ifndef MAX_CRYPTO_DEVID_CALLBACKS
+    #define MAX_CRYPTO_DEVID_CALLBACKS WOLFSSL_MAX3266X_DEVID


This macro specifies the maximum number of concurrent callbacks to register. I don't think you need to change or set this. Also you don't need to include this header in cryptocb.c.

removed, and removed inclusion of the header in cryptocb.c

dgarske · 2024-09-18T21:50:23Z

wolfcrypt/src/sha.c

@@ -1003,7 +1014,11 @@ if (sha->ctx.mode == ESP32_SHA_HW) {
    /* we'll always reset state upon exit and return the error code from above,
     * which may cause fall back to SW if HW is busy. we do not return result
     * of initSha here */
-    (void)InitSha(sha); /* reset state */
+    #ifdef WOLF_CRYPTO_CB /* Use to reset state but keep devId info */


This change should not be required. The InitSha should not be resetting heap hint or devId.

dgarske · 2024-09-18T22:29:58Z

wolfcrypt/src/sha256.c

@@ -1732,7 +1740,11 @@ static int InitSha256(wc_Sha256* sha256)
    #endif
        XMEMCPY(hash, sha256->digest, WC_SHA256_DIGEST_SIZE);

+    #ifdef WOLF_CRYPTO_CB /* Use to reset state but keep devId info */


Consider fixing InitSha256 to not wipe out devId... and move the sha256->devId = wc_CryptoCb_DefaultDevID(); login into wc_InitSha256_ex.

Removed the setting of the devId in InitSha256(), It doesn't seem like we need to add sha256->devId = wc_CryptoCb_DefaultDevID(); to wc_InitSha256_ex() as this function expects a devId argument passed to it.

I think it can be removed too. This was originally added to handle the cases where Sha256 didn't have a devId like CalcHashId. However those one-shot hash API's like wc_Sha256Hash now have this logic built-in. devId = wc_CryptoCb_DefaultDevID();

dgarske · 2024-09-19T21:31:45Z

wolfcrypt/src/aes.c

@@ -82,6 +82,17 @@ block cipher mechanism that uses n-bit binary string parameter key with 128-bits
    #include <wolfssl/wolfcrypt/port/psa/psa.h>
 #endif

+#if defined(WOLFSSL_MAX3266X) || defined(WOLFSSL_MAX3266X_OLD)
+    #include <wolfssl/wolfcrypt/port/maxim/max3266x.h>
+#ifdef WOLF_CRYPTO_CB


Should this be gated on MAX3266X_CB?

Updated to MAX3266X_CB for consistency

dgarske · 2024-09-19T21:37:46Z

wolfssl/wolfcrypt/aes.h

+#if (defined(WOLFSSL_MAX3266X) || defined(WOLFSSL_MAX3266X_OLD)) && \
+        defined(WOLF_CRYPTO_CB)
+    /* Need backup key for MXC CB */
+    word32 cb_key[60];


This can be removed... Already code in struct to support raw key. See devKey

Removed and setup to use devKey

dgarske · 2024-09-19T21:38:07Z

wolfcrypt/src/aes.c

@@ -4102,8 +4168,12 @@ static void AesSetKey_C(Aes* aes, const byte* key, word32 keySz, int dir)
    unsigned int i = 0;

    XMEMCPY(rk, key, keySz);
+#ifdef MAX3266X_CB /* Copies needed values to use later if CB is used */
+    XMEMCPY(aes->cb_key, key, keySz);


Not required. See devKey. Use this for crypto callbacks.

dgarske · 2024-09-19T21:38:17Z

wolfcrypt/src/aes.c

@@ -4543,11 +4613,14 @@ static void AesSetKey_C(Aes* aes, const byte* key, word32 keySz, int dir)
 #endif

        XMEMCPY(aes->key, userKey, keylen);
+#ifdef MAX3266X_CB /* Copy Key for CB for use later if needed */
+        XMEMCMP(aes->cb_key, userKey, keylen);


Not required. See devKey. Use this for crypto callbacks.

wolfcrypt/src/port/arm/armv8-aes.c

…a1, and Sha224

…MAA call update for MAX3266X Port.

… Fix SP SHA CB Bug

wolfssl/wolfcrypt/wc_port.h

night1rider · 2024-09-20T21:58:41Z

Jenkins retest this please

night1rider self-assigned this Jul 23, 2024

night1rider mentioned this pull request Jul 23, 2024

MAX32665 and MAX32666 SDK TPU/MAA Bug analogdevicesinc/msdk#1089

Closed

dgarske requested changes Jul 23, 2024

View reviewed changes

night1rider force-pushed the MAX32666-port branch 11 times, most recently from 18752ce to 30a2feb Compare July 25, 2024 04:54

dgarske requested changes Jul 25, 2024

View reviewed changes

night1rider force-pushed the MAX32666-port branch 11 times, most recently from c00c33c to 56b585f Compare July 25, 2024 22:55

dgarske requested changes Jul 26, 2024

View reviewed changes

dgarske requested changes Sep 18, 2024

View reviewed changes

dgarske removed their assignment Sep 18, 2024

night1rider force-pushed the MAX32666-port branch from f2ccfb3 to e7ad571 Compare September 19, 2024 20:28

night1rider assigned dgarske Sep 19, 2024

dgarske requested changes Sep 19, 2024

View reviewed changes

night1rider changed the title ~~Initial PR for MAX32665 and MAX32666 TPU HW Support~~ MAX32665 and MAX32666 TPU HW and ARM ASM Crypto Callback Support Sep 19, 2024

night1rider force-pushed the MAX32666-port branch 2 times, most recently from 3a8578f to 23ea92e Compare September 19, 2024 22:54

dgarske previously approved these changes Sep 20, 2024

View reviewed changes

dgarske removed their assignment Sep 20, 2024

msi-debian and others added 7 commits September 20, 2024 09:42

Initial PR for MAX32665 and MAX32666 TPU HW Support

2e8cf39

Addressing PR comments typos and cleanup and support HAVE_AES_ECB, Sh…

d714e55

…a1, and Sha224

Adding SHA-384/512 support, Null Checks, RNG Health Test for HW, and …

fe7987f

…MAA call update for MAX3266X Port.

Addressing Feedback, Adding Null Checks and Mutex Around TRNG

8f8b4e6

Making so hw mutex define is not needed

a7ef540

Expanding mutexing and Adding in AES Callbacks for HW

1cb324a

Crypto Callback Support for ARM ASM: AES-ECB/CBC, SHA-1/256/384/512 +…

9881edf

… Fix SP SHA CB Bug

night1rider force-pushed the MAX32666-port branch from 23ea92e to 9881edf Compare September 20, 2024 15:43

night1rider removed their assignment Sep 20, 2024

JacobBarthelmeh requested changes Sep 20, 2024

View reviewed changes

wolfssl/wolfcrypt/wc_port.h Outdated Show resolved Hide resolved

JacobBarthelmeh assigned night1rider and unassigned JacobBarthelmeh and wolfSSL-Bot Sep 20, 2024

Making HW Mutex Functions Private Api

1ffcf40

night1rider dismissed dgarske’s stale review via 1ffcf40 September 20, 2024 21:21

night1rider assigned JacobBarthelmeh and unassigned night1rider Sep 20, 2024

JacobBarthelmeh approved these changes Sep 20, 2024

View reviewed changes

JacobBarthelmeh merged commit 554d52b into wolfSSL:master Sep 20, 2024
135 checks passed


		#include <wolfssl/wolfcrypt/settings.h>

		#warning "Inside max3266x.c"

MAX32665 and MAX32666 TPU HW and ARM ASM Crypto Callback Support #7777

MAX32665 and MAX32666 TPU HW and ARM ASM Crypto Callback Support #7777

Conversation

night1rider commented Jul 23, 2024 • edited Loading

Description