Fix issues with H5 SPI master driver. Fixes for TPM NCS's. Cleanup dead code.

Author: dgarske

Makefile

@@ -531,6 +531,8 @@ cppcheck:
 	cppcheck -f --enable=warning --enable=portability \
 		--suppress="ctunullpointer" --suppress="nullPointer" \
 		--suppress="objectIndex" --suppress="comparePointers" \
+		--suppress="subtractPointers" --suppress="intToPointerCast" \
+		--check-level=exhaustive \
 		--error-exitcode=89 --std=c89 src/*.c hal/*.c hal/spi/*.c hal/uart/*.c
 
 otp: tools/keytools/otp/otp-keystore-primer.bin FORCE

hal/spi/spi_drv_stm32.c

@@ -116,7 +116,7 @@ void RAMFUNCTION stm_gpio_config(uint32_t base, uint32_t pin, uint32_t mode,
     GPIO_OSPD(base) |= (speed << (pin * 2));
 
 #if defined (__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U)
-
+    /* TODO: Consider setting GPIO_SECCFGR(base) */
 #endif
 }
 
@@ -375,7 +375,7 @@ int qspi_transfer(uint8_t fmode, const uint8_t cmd,
 #if defined(SPI_FLASH) || defined(WOLFBOOT_TPM)
 uint8_t RAMFUNCTION spi_read(void)
 {
-    while ((SPI1_SR & SPI_SR_RX_NOTEMPTY) == 0);
+    while (!(SPI1_SR & SPI_SR_RX_NOTEMPTY));
 #ifdef SPI1_RXDR
     return SPI1_RXDR;
 #else
@@ -385,12 +385,12 @@ uint8_t RAMFUNCTION spi_read(void)
 
 void RAMFUNCTION spi_write(const char byte)
 {
-    int i;
-    while ((SPI1_SR & SPI_SR_TX_EMPTY) == 0);
+    while (!(SPI1_SR & SPI_SR_TX_EMPTY));
+
 #ifdef SPI1_TXDR
-    SPI1_TXDR = byte;
+    SPI1_TXDR = (uint8_t)byte;
 #else
-    SPI1_DR = byte;
+    SPI1_DR = (uint8_t)byte;
 #endif
 }
 #endif /* SPI_FLASH || WOLFBOOT_TPM */
@@ -496,17 +496,20 @@ void RAMFUNCTION spi_init(int polarity, int phase)
         /* Configure SPI1 for master mode */
         SPI1_CR1 &= ~SPI_CR1_SPI_EN;
     #if defined(TARGET_stm32h5)
+        /* Clear any faults in the status register */
+        SPI1_IFCR = (SPI_IFCR_SUSPC | SPI_IFCR_MODFC | SPI_IFCR_TIFREC |
+                     SPI_IFCR_OVRC | SPI_IFCR_UDRC);
+
         /* baud rate 2 (hclk/8), data size (8-bits), CRC Size (8-bits),
          * FIFO threshold level (1-data) */
         SPI1_CFG1 = (
             ((2 & SPI_CFG1_BAUDRATE_MASK) << SPI_CFG1_BAUDRATE_SHIFT) |
             ((7 & SPI_CFG1_CRCSIZE_MASK) << SPI_CFG1_CRCSIZE_SHIFT) |
             ((0 & SPI_CFG1_FTHLV_MASK) << SPI_CFG1_FTHLV_SHIFT) |
             ((7 & SPI_CFG1_DSIZE_MASK) << SPI_CFG1_DSIZE_SHIFT));
-        SPI1_CFG2 = SPI_CRF2_MASTER | SPI_CFG2_SSOE |
+        SPI1_CFG2 = SPI_CFG2_MASTER | SPI_CFG2_SSOE |
             (polarity << SPI_CFG2_CLOCK_POL_SHIFT) |
             (phase << SPI_CFG2_CLOCK_PHASE_SHIFT);
-        SPI1_CR1 |= SPI_CR1_CSTART; /* use continuous start mode */
     #else
         #ifndef TARGET_stm32l0 /* use existing/default baud for L0 */
         /* Baud rate 5 (hclk/6), data size 8 bits */
@@ -520,6 +523,10 @@ void RAMFUNCTION spi_init(int polarity, int phase)
     #endif
 
         SPI1_CR1 |= SPI_CR1_SPI_EN; /* Enable SPI */
+
+    #ifdef SPI_CR1_CSTART
+        SPI1_CR1 |= SPI_CR1_CSTART; /* use continuous start mode */
+    #endif
 #endif /* SPI_FLASH || WOLFBOOOT_TPM */
     }
 }

hal/spi/spi_drv_stm32.h

@@ -433,12 +433,14 @@
 #define SPI1_CFG1     (*(volatile uint32_t *)(SPI1_BASE + 0x08))
 #define SPI1_CFG2     (*(volatile uint32_t *)(SPI1_BASE + 0x0C))
 #define SPI1_SR       (*(volatile uint32_t *)(SPI1_BASE + 0x14))
+#define SPI1_IFCR     (*(volatile uint32_t *)(SPI1_BASE + 0x18))
 #define SPI1_TXDR     (*(volatile uint8_t *)(SPI1_BASE + 0x20))
 #define SPI1_RXDR     (*(volatile uint8_t *)(SPI1_BASE + 0x30))
 
 #define SPI_CR1_SPI_EN              (1 << 0)
-#define SPI_CR1_CSTART              (1 << 9) /* Continous start */
-
+#define SPI_CR1_MASRX               (1 << 8)  /* master automatic suspension in Receive mode */
+#define SPI_CR1_CSTART              (1 << 9)  /* Continous start */
+#define SPI_CR1_SSI                 (1 << 12) /* Internal slave select signal input level */
 #define SPI_CFG1_DSIZE_MASK         (0x1F)
 #define SPI_CFG1_DSIZE_SHIFT        (0)
 #define SPI_CFG1_FTHLV_MASK         (0x1F)
@@ -448,18 +450,28 @@
 #define SPI_CFG1_BAUDRATE_MASK      (0x07)
 #define SPI_CFG1_BAUDRATE_SHIFT     (28)
 
-#define SPI_CRF2_MASTER             (1 << 22)
+#define SPI_CFG2_MASTER             (1 << 22)
 #define SPI_CFG2_LSBFIRST           (1 << 23)
 #define SPI_CFG2_CLOCK_PHASE_SHIFT  (24)
 #define SPI_CFG2_CLOCK_POL_SHIFT    (25)
 #define SPI_CFG2_SSM                (1 << 26)
 #define SPI_CFG2_SSOE               (1 << 29)
-#define SPI_CFG2_HW_CRC_EN          (1 << 29)
-#define SPI_CFG2_COMM_MASK          (0x3) /* 0=full duplex, 1=simplex tx, 2=simplex rx, 3=half duplex */
+#define SPI_CFG2_SSOM               (1 << 30)
+#define SPI_CFG2_AFCNTR             (1 << 31) /* alternate function GPIOs control */
+#define SPI_CFG2_COMM_MASK          (0x3)     /* 0=full duplex, 1=simplex tx, 2=simplex rx, 3=half duplex */
 #define SPI_CFG2_COMM_SHIFT         (17)
 
-#define SPI_SR_RX_NOTEMPTY          (1UL << 0)
-#define SPI_SR_TX_EMPTY             (1UL << 1)
+#define SPI_IFCR_SUSPC              (1 << 11)
+#define SPI_IFCR_MODFC              (1 << 9)
+#define SPI_IFCR_TIFREC             (1 << 8)
+#define SPI_IFCR_CRCEC              (1 << 7)
+#define SPI_IFCR_OVRC               (1 << 6)
+#define SPI_IFCR_UDRC               (1 << 5)
+#define SPI_IFCR_TXTFC              (1 << 4)
+#define SPI_IFCR_EOTC               (1 << 3)
+
+#define SPI_SR_RX_NOTEMPTY          (1 << 0)
+#define SPI_SR_TX_EMPTY             (1 << 1)
 
 #else
 

include/tpm.h

@@ -57,8 +57,10 @@ int wolfBoot_tpm2_clear(void);
 /* API's that are callable from non-secure code */
 int CSME_NSE_API wolfBoot_tpm2_caps(WOLFTPM2_CAPS* caps);
 int CSME_NSE_API wolfBoot_tpm2_get_handles(TPM_HANDLE handle, TPML_HANDLE* handles);
-const char* CSME_NSE_API wolfBoot_tpm2_get_alg_name(TPM_ALG_ID alg);
-const char* CSME_NSE_API wolfBoot_tpm2_get_rc_string(int rc);
+const char* CSME_NSE_API wolfBoot_tpm2_get_alg_name(TPM_ALG_ID alg,
+    char* name, int name_sz);
+const char* CSME_NSE_API wolfBoot_tpm2_get_rc_string(int rc,
+    char* error, int error_sz);
 TPM_RC CSME_NSE_API wolfBoot_tpm2_get_capability(GetCapability_In* in, GetCapability_Out* out);
 
 #if defined(WOLFBOOT_TPM_VERIFY) || defined(WOLFBOOT_TPM_SEAL)

include/wolfboot/wolfboot.h

@@ -38,14 +38,14 @@ extern "C" {
 
 
 #ifndef RAMFUNCTION
-#if defined(__WOLFBOOT) && defined(RAM_CODE)
-#  if defined(ARCH_ARM)
-#    define RAMFUNCTION __attribute__((used,section(".ramcode"),long_call))
+#  if defined(__WOLFBOOT) && defined(RAM_CODE)
+#    if defined(ARCH_ARM)
+#      define RAMFUNCTION __attribute__((used,section(".ramcode"),long_call))
+#    else
+#      define RAMFUNCTION __attribute__((used,section(".ramcode")))
+#    endif
 #  else
-#    define RAMFUNCTION __attribute__((used,section(".ramcode")))
-#  endif
-#else
-# define RAMFUNCTION
+#   define RAMFUNCTION
 #endif
 #endif
 
@@ -84,7 +84,7 @@ extern "C" {
 
 #ifndef XALIGNED_STACK
     /* Don't enforce stack alignment on IAR */
-    #if defined (__IAR_SYSTEMS_ICC__)
+    #if defined(__IAR_SYSTEMS_ICC__)
         #define XALIGNED_STACK(x)
     #else
         #define XALIGNED_STACK(x) XALIGNED(x)

src/image.c

@@ -36,6 +36,7 @@
 
 #include "loader.h"
 #include "image.h"
+#include "wolfboot/wolfboot.h"
 #include "hal.h"
 #include "spi_drv.h"
 #include "printf.h"

src/tpm.c

@@ -1137,24 +1137,49 @@ static int wolfRNG_GetSeedCB(OS_Seed* os, uint8_t* seed, uint32_t sz)
 /* API's that are callable from non-secure code */
 int CSME_NSE_API wolfBoot_tpm2_caps(WOLFTPM2_CAPS* caps)
 {
-    memset(&caps, 0, sizeof(caps));
+    memset(caps, 0, sizeof(*caps));
     return wolfTPM2_GetCapabilities(&wolftpm_dev, caps);
 }
 
 int CSME_NSE_API wolfBoot_tpm2_get_handles(TPM_HANDLE handle, TPML_HANDLE* handles)
 {
-    memset(&handles, 0, sizeof(handles));
+    memset(handles, 0, sizeof(*handles));
     return wolfTPM2_GetHandles(handle, handles);
 }
 
-const char* CSME_NSE_API wolfBoot_tpm2_get_alg_name(TPM_ALG_ID alg)
+const char* CSME_NSE_API wolfBoot_tpm2_get_alg_name(TPM_ALG_ID alg,
+    char* name, int name_sz)
 {
-    return TPM2_GetAlgName(alg);
+    const char* s_name;
+    if (name == NULL || name_sz <= 0) {
+        return NULL;
+    }
+    s_name = TPM2_GetAlgName(alg);
+    if (s_name != NULL && name != NULL && name_sz > 0) {
+        strncpy(name, s_name, name_sz - 1);
+        name[name_sz - 1] = '\0';
+    }
+    else {
+        strcpy(name, "Unknown");
+    }
+    return (const char*)name;
 }
 
-const char* CSME_NSE_API wolfBoot_tpm2_get_rc_string(int rc)
+const char* CSME_NSE_API wolfBoot_tpm2_get_rc_string(int rc, char* error, int error_sz)
 {
-    return TPM2_GetRCString(rc);
+    const char* s_error;
+    if (error == NULL || error_sz <= 0) {
+        return NULL;
+    }
+    s_error = TPM2_GetRCString(rc);
+    if (s_error != NULL && error != NULL && error_sz > 0) {
+        strncpy(error, s_error, error_sz - 1);
+        error[error_sz - 1] = '\0';
+    }
+    else {
+        strcpy(error, "Unknown");
+    }
+    return (const char*)error;
 }
 
 TPM_RC CSME_NSE_API wolfBoot_tpm2_get_capability(GetCapability_In* in, GetCapability_Out* out)

src/update_flash.c

@@ -27,7 +27,6 @@
 #include "image.h"
 #include "hal.h"
 #include "spi_flash.h"
-#include "wolfboot/wolfboot.h"
 #include "target.h"
 
 #include "delta.h"

test-app/app_stm32h5.c

@@ -677,6 +677,7 @@ static int TPM2_PCRs_Print(void)
     GetCapability_In  capIn;
     GetCapability_Out capOut;
     TPML_PCR_SELECTION* pcrSel;
+    char buffer[24];
 
     /* List available PCR's */
     XMEMSET(&capIn, 0, sizeof(capIn));
@@ -686,9 +687,11 @@ static int TPM2_PCRs_Print(void)
     rc = wolfBoot_tpm2_get_capability(&capIn, &capOut);
     if (rc == TPM_RC_SUCCESS) {
         pcrSel = &capOut.capabilityData.data.assignedPCR;
-        printf("Assigned PCR's:\n");
+        printf("Assigned PCR's:\r\n");
         for (pcrCount=0; pcrCount < (int)pcrSel->count; pcrCount++) {
-            printf("\t%s: ", wolfBoot_tpm2_get_alg_name(pcrSel->pcrSelections[pcrCount].hash));
+
+            printf("\t%s: ", wolfBoot_tpm2_get_alg_name(
+                pcrSel->pcrSelections[pcrCount].hash, buffer, sizeof(buffer)));
             for (pcrIndex=0;
                 pcrIndex<pcrSel->pcrSelections[pcrCount].sizeofSelect*8;
                 pcrIndex++) {
@@ -697,7 +700,7 @@ static int TPM2_PCRs_Print(void)
                     printf(" %d", pcrIndex);
                 }
             }
-            printf("\n");
+            printf("\r\n");
         }
     }
     return rc;
@@ -708,13 +711,14 @@ static int cmd_tpm_info(const char *args)
     int rc;
     WOLFTPM2_CAPS caps;
     TPML_HANDLE handles;
+    char error[100];
 
-    printf("Get TPM 2.0 module information\n");
+    printf("Get TPM 2.0 module information\r\n");
 
     rc = wolfBoot_tpm2_caps(&caps);
     if (rc == 0) {
         printf("Mfg %s (%d), Vendor %s, Fw %u.%u (0x%x), "
-            "FIPS 140-2 %d, CC-EAL4 %d\n",
+            "FIPS 140-2 %d, CC-EAL4 %d\r\n",
             caps.mfgStr, caps.mfg, caps.vendorStr, caps.fwVerMajor,
             caps.fwVerMinor, caps.fwVerVendor, caps.fips140_2, caps.cc_eal4);
     }
@@ -723,16 +727,17 @@ static int cmd_tpm_info(const char *args)
     rc = wolfBoot_tpm2_get_handles(PERSISTENT_FIRST, &handles);
     if (rc >= 0) {
         int i;
-        printf("Found %d persistent handles\n", rc);
+        printf("Found %d persistent handles\r\n", rc);
         for (i=0; i<(int)handles.count; i++) {
-            printf("\tHandle 0x%x\n", (unsigned int)handles.handle[i]);
+            printf("\tHandle 0x%x\r\n", (unsigned int)handles.handle[i]);
         }
     }
 
     /* Print the available PCR's */
     rc = TPM2_PCRs_Print();
     if (rc != 0) {
-        printf("TPM error 0x%x: %s\n", rc, wolfBoot_tpm2_get_rc_string(rc));
+        printf("TPM error 0x%x: %s\r\n",
+            rc, wolfBoot_tpm2_get_rc_string(rc, error, sizeof(error)));
     }
 
     return rc;

test-app/app_stm32l5.c

@@ -134,8 +134,6 @@ void extra_led_off(void)
     GPIOB_BSRR |= (1 << (LED_EXTRA_PIN + 16));
 }
 
-static char CaBuf[2048];
-static uint8_t my_pubkey[200];
 
 extern int ecdsa_sign_verify(int devId);