You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by na...@apache.org on 2020/10/21 16:42:10 UTC

[mynewt-core] branch master updated: da1469x: Fix DMA Multiplexer register access

This is an automated email from the ASF dual-hosted git repository.

naveenkaje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git


The following commit(s) were added to refs/heads/master by this push:
     new 7168056  da1469x: Fix DMA Multiplexer register access
     new 5d35328  Merge pull request #2392 from nkaje/Fix_DMA_Crypto_Access_rebased
7168056 is described below

commit 71680567422fe7ee5261a222d265d1e8ced53f05
Author: Naveen Kaje <na...@juul.com>
AuthorDate: Wed Oct 21 08:37:31 2020 -0500

    da1469x: Fix DMA Multiplexer register access
    
    Modify the crypto driver to acquire/release the
    DMA channel being used. As the acquire/release
    modifies a global variable and DMA_REQ_MUX_REG, which
    are shared resources, these accesses need to be done
    in a critical section.
    
    The macros MCU_DMA_SET_MUX/MCU_DMA_GET_MUX are reworked
    to generate the pattern for the pair of channels as documented
    in the datasheet.
    
    Configure DMA_REQ_MUX_REG within a critical section to avoid
    race conditions. Use common macros provided by the DMA
    driver to make changes to this register. It is to be noted that this
    lock usage can be quite heavy when any given DMA channel needs to be
    accessed for long periods of time and this should be addressed.
    
    Signed-off-by: Naveen Kaje <na...@juul.com>
---
 .../crypto/crypto_da1469x/src/crypto_da1469x.c     | 30 +++++++++-------
 hw/mcu/dialog/da1469x/src/da1469x_dma.c            | 41 ++++++++++++++--------
 2 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/hw/drivers/crypto/crypto_da1469x/src/crypto_da1469x.c b/hw/drivers/crypto/crypto_da1469x/src/crypto_da1469x.c
index f8d55b6..9d93e4d 100644
--- a/hw/drivers/crypto/crypto_da1469x/src/crypto_da1469x.c
+++ b/hw/drivers/crypto/crypto_da1469x/src/crypto_da1469x.c
@@ -64,7 +64,7 @@ da1469x_has_support(struct crypto_dev *crypto, uint8_t op, uint16_t algo,
 static void
 do_dma_key_tx(const void *key, uint16_t keylen)
 {
-    DMA_Type *dma_regs = DMA;
+    struct da1469x_dma_regs *dma_regs;
     /* DMA len: keylen in bits, bus width is 4 */
     keylen >>= 5;
 
@@ -72,18 +72,24 @@ do_dma_key_tx(const void *key, uint16_t keylen)
     da1469x_clock_amba_enable(CRG_TOP_CLK_AMBA_REG_OTP_ENABLE_Msk);
     da1469x_otp_set_mode(OTPC_MODE_READ);
 
-    /* securely DMA hardware key from secret storage to QSPI decrypt engine */
-    dma_regs->DMA_REQ_MUX_REG |= 0xf000;
-    dma_regs->DMA7_LEN_REG = keylen;
+    /*
+     * Securely DMA hardware key using channel #7 from secret storage to decrypt engine
+     * The channel #7 is a secure channel and used for crypto operations only. It should
+     * always return the correct address.
+     */
+    dma_regs = da1469x_dma_acquire_single(7);
+    assert(dma_regs);
+    dma_regs->DMA_LEN_REG = keylen;
     /* program source and destination addresses */
-    dma_regs->DMA7_A_START_REG = (uint32_t)key;
-    dma_regs->DMA7_B_START_REG = (uint32_t)&AES_HASH->CRYPTO_KEYS_START;
-    dma_regs->DMA7_CTRL_REG = DMA_DMA7_CTRL_REG_AINC_Msk |
-                              DMA_DMA7_CTRL_REG_BINC_Msk |
-                              (MCU_DMA_BUS_WIDTH_4B << DMA_DMA7_CTRL_REG_BW_Pos) |
-                              DMA_DMA7_CTRL_REG_DMA_ON_Msk;
-    while (dma_regs->DMA7_IDX_REG != keylen);
-
+    dma_regs->DMA_A_START_REG = (uint32_t)key;
+    dma_regs->DMA_B_START_REG = (uint32_t)&AES_HASH->CRYPTO_KEYS_START;
+    dma_regs->DMA_CTRL_REG = DMA_DMA7_CTRL_REG_AINC_Msk |
+                             DMA_DMA7_CTRL_REG_BINC_Msk |
+                             (MCU_DMA_BUS_WIDTH_4B << DMA_DMA7_CTRL_REG_BW_Pos) |
+                             DMA_DMA7_CTRL_REG_DMA_ON_Msk;
+    while (dma_regs->DMA_IDX_REG != keylen);
+
+    da1469x_dma_release_channel(dma_regs);
     /* set OTP to standby and turn off clock */
     da1469x_otp_set_mode(OTPC_MODE_STBY);
     da1469x_clock_amba_disable(CRG_TOP_CLK_AMBA_REG_OTP_ENABLE_Msk);
diff --git a/hw/mcu/dialog/da1469x/src/da1469x_dma.c b/hw/mcu/dialog/da1469x/src/da1469x_dma.c
index 0dc4c28..e7318c0 100644
--- a/hw/mcu/dialog/da1469x/src/da1469x_dma.c
+++ b/hw/mcu/dialog/da1469x/src/da1469x_dma.c
@@ -31,15 +31,21 @@
 #define MCU_DMA_CHAN_MAX            (8)
 #endif
 
-#define MCU_DMA_SET_MUX(_cidx, _periph)             \
-    do {                                            \
-        DMA->DMA_REQ_MUX_REG =                      \
-            (DMA->DMA_REQ_MUX_REG &                 \
-             ~(0xf << ((_cidx) * 2))) |             \
-            ((_periph) << ((_cidx) * 2));           \
-    } while (0)
-#define MCU_DMA_GET_MUX(_cidx)                      \
-    ((DMA->DMA_REQ_MUX_REG >> ((_cidx) * 2)) & 0xf)
+#define DA1469X_DMA_MUX_SHIFT(_cidx) (((_cidx) >> 1) * 4)
+
+/*
+ * Mux Bits 0:3, 4:7, 8:11, 12:15 correspond to selection in
+ * channels 0,1  2,3, 4,5 & 6,7. Use the shift macro above to
+ * determine the mask.
+ */
+#define MCU_DMA_GET_MUX(_cidx)                                                       \
+    ((DMA->DMA_REQ_MUX_REG >> DA1469X_DMA_MUX_SHIFT(_cidx)) & 0xf)
+
+#define MCU_DMA_SET_MUX(_cidx, _periph)                                              \
+    DMA->DMA_REQ_MUX_REG =                                                           \
+        ((DMA->DMA_REQ_MUX_REG & ~(0xf << DA1469X_DMA_MUX_SHIFT(_cidx)))             \
+         | ((_periph) & 0xf) << DA1469X_DMA_MUX_SHIFT(_cidx));                       \
+
 
 struct da1469x_dma_interrupt_cfg {
     da1469x_dma_interrupt_cb cb;
@@ -117,17 +123,19 @@ da1469x_dma_init(void)
 struct da1469x_dma_regs *
 da1469x_dma_acquire_single(int cidx)
 {
-    struct da1469x_dma_regs *chan;
+    struct da1469x_dma_regs *chan = NULL;
+    int sr;
 
     assert(cidx < MCU_DMA_CHAN_MAX);
 
+    OS_ENTER_CRITICAL(sr);
     if (cidx < 0) {
         cidx = find_free_single();
         if (cidx < 0) {
-            return NULL;
+            goto end_single;
         }
     } else if (g_da1469x_dma_acquired & (1 << cidx)) {
-        return NULL;
+        goto end_single;
     }
 
     if (!g_da1469x_dma_acquired) {
@@ -147,6 +155,8 @@ da1469x_dma_acquire_single(int cidx)
 
     chan->DMA_CTRL_REG &= ~DMA_DMA0_CTRL_REG_DREQ_MODE_Msk;
 
+end_single:
+    OS_EXIT_CRITICAL(sr);
     return chan;
 }
 
@@ -154,7 +164,7 @@ int
 da1469x_dma_acquire_periph(int cidx, uint8_t periph,
                            struct da1469x_dma_regs *chans[2])
 {
-    assert(cidx < MCU_DMA_CHAN_MAX);
+    assert(cidx < MCU_DMA_CHAN_MAX && periph < MCU_DMA_PERIPH_NONE);
 
     if (cidx < 0) {
         cidx = find_free_pair();
@@ -189,16 +199,18 @@ int
 da1469x_dma_release_channel(struct da1469x_dma_regs *chan)
 {
     int cidx = MCU_DMA_CHAN2CIDX(chan);
+    int sr;
 
     assert(cidx >= 0 && cidx < MCU_DMA_CHAN_MAX);
 
+    OS_ENTER_CRITICAL(sr);
     /*
      * If corresponding pair for this channel is configured for triggering from
      * peripheral, we'll use lower of channel index.
      *
      * Only channels 0-7 can use pairs for peripherals.
      */
-    if (cidx < 8 && MCU_DMA_GET_MUX(cidx) != MCU_DMA_PERIPH_NONE) {
+    if (cidx < 8 && MCU_DMA_GET_MUX(cidx) < MCU_DMA_PERIPH_NONE) {
         cidx &= 0xfe;
         chan = MCU_DMA_CIDX2CHAN(cidx);
 
@@ -232,6 +244,7 @@ da1469x_dma_release_channel(struct da1469x_dma_regs *chan)
         da1469x_pd_release(MCU_PD_DOMAIN_SYS);
     }
 
+    OS_EXIT_CRITICAL(sr);
     return 0;
 }