You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/11/05 12:09:47 UTC

[GitHub] [incubator-nuttx] donghengqaz opened a new pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

donghengqaz opened a new pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224


   ## Summary
   
   Add SPI Flash hardware encryption I/O support.
   
   ## Impact
   
   ## Testing
   
   Add board level testing code `void esp32_spiflash_encrypt_test(void)`. When enable option `ESP32_SPIFLASH_ENCRYPTION_TEST`, board initialization will run this.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-722463677


   But this warning could be fixed:
   ···
   esp32_spiflash.c:174:1: error: Too many blank lines
   ···


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r522581666



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -862,108 +974,380 @@ static int esp32_writedata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
           bytes = MIN(bytes, SPI_FLASH_WRITE_BUF_SIZE);
         }
 
-      regval = addr & 0xffffff;
-      regval |= bytes << ESP_ROM_SPIFLASH_BYTES_LEN;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      memcpy(tmp_buf, &buffer[off], bytes);
 
-      for (i = 0; i < bytes; i += 4)
-        {
-          res = MIN(4, bytes - i);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_writeonce(priv, addr, tmp_buf, bytes);
+      esp32_spiflash_opdone(priv, flags, me);
 
-          spi_memcpy(&tmp, tmpbuff, res);
-          spi_set_reg(priv, SPI_W0_OFFSET + i, tmp);
-          tmpbuff += res;
+      if (ret)
+        {
+          return ret;
         }
 
-      spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
-      spi_set_reg(priv, SPI_CMD_OFFSET, SPI_FLASH_PP);
-      while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
+      addr += bytes;
+      size -= bytes;
+      off += bytes;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: esp32_writedata
+ *
+ * Description:
+ *   Write plaintext data to SPI Flash at designated address by SPI Flash
+ *   hardware encryption, and written data in SPI Flash is ciphertext.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   0 if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_writedata_encrypted(
+  FAR struct esp32_spiflash_s *priv,
+  uint32_t addr,
+  const uint8_t *buffer,
+  uint32_t size)
+{
+  int i;
+  int blocks;
+  int ret = OK;
+  int me;
+  uint32_t flags;
+  uint32_t tmp_buf[SPI_FLASH_ENCRYPT_WORDS];
+
+  if (addr % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: address=0x%x is not %d-byte align\n",
+           addr, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  if (size % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: size=%u is not %d-byte align\n",
+           size, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  blocks = size / SPI_FLASH_ENCRYPT_UNIT_SIZE;
+
+  for (i = 0; i < blocks; i++)
+    {
+      memcpy(tmp_buf, buffer, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+
+      flags = esp32_spiflash_opstart(priv, &me);
+      esp_rom_spiflash_write_encrypted_enable();
+
+      ret = esp_rom_spiflash_prepare_encrypted_data(addr, tmp_buf);
+      if (ret)
         {
-          ;
+          ferr("ERROR: Failed to prepare encrypted data\n");
+          goto exit;
         }
 
-      if (esp32_wait_idle(priv) != OK)
+      ret = esp32_writeonce(priv, addr, tmp_buf,
+                            SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      if (ret)
         {
-          esp32_spiflash_opdone(priv, flags, me);
-          return -EIO;
+          ferr("ERROR: Failed to write encrypted data @ 0x%x\n", addr);
+          goto exit;
         }
 
-      addr += bytes;
-      size -= bytes;
-
+      esp_rom_spiflash_write_encrypted_disable();
       esp32_spiflash_opdone(priv, flags, me);
+
+      addr += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      buffer += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      size -= SPI_FLASH_ENCRYPT_UNIT_SIZE;
     }
 
+  return 0;
+
+exit:
+  esp_rom_spiflash_write_encrypted_disable();
+  esp32_spiflash_opdone(priv, flags, me);
+
   return ret;
 }
 
 /****************************************************************************
  * Name: esp32_readdata
  *
  * Description:
- *   Read data from SPI Flash at designated address.
+ *   Read max 64 byte data data from SPI Flash at designated address.
+ *
+ *   ESP32 can read max 64 byte once transmission by hardware.
  *
  * Input Parameters:
  *   spi    - ESP32 SPI Flash chip data
  *   addr   - target address
  *   buffer - data buffer pointer
- *   size   - data number
+ *   size   - data number by bytes
  *
  * Returned Value:
  *   OK if success or a negative value if fail.
  *
  ****************************************************************************/
 
-static int esp32_readdata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
-                          uint8_t *buffer, uint32_t size)
+static int IRAM_ATTR esp32_readonce(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint32_t *buffer,
+                                    uint32_t size)
 {
   uint32_t regval;
   uint32_t i;
-  uint32_t bytes;
-  uint32_t tmp;
-  uint32_t res;
-  uint8_t  *tmpbuff = buffer;
-  int me;
-  uint32_t flags;
+
+  if (size > SPI_FLASH_READ_BUF_SIZE)
+    {
+      return -EINVAL;
+    }
 
   if (esp32_wait_idle(priv) != OK)
     {
       return -EIO;
     }
 
-  while (size > 0)
+  regval = ((size << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
+  spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
+
+  regval = addr << 8;
+  spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+
+  spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
+  spi_set_reg(priv, SPI_CMD_OFFSET, SPI_USR);
+  while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
     {
-      flags = esp32_spiflash_opstart(priv, &me);
+      ;
+    }
+
+  for (i = 0; i < (size / 4); i++)
+    {
+      buffer[i] = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+    }
+
+  if (size & 0x3)
+    {
+      regval = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+      memcpy(&buffer[i], &regval, size & 0x3);
+    }
+
+  return OK;
+}
 
+/****************************************************************************
+ * Name: esp32_readdata
+ *
+ * Description:
+ *   Read data from SPI Flash at designated address.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   OK if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_readdata(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint8_t *buffer,
+                                    uint32_t size)
+{
+  int ret;
+  int me;
+  uint32_t flags;
+  uint32_t off = 0;
+  uint32_t bytes;
+  uint32_t tmp_buf[SPI_FLASH_READ_WORDS];
+
+  while (size > 0)
+    {
       bytes = MIN(size, SPI_FLASH_READ_BUF_SIZE);
-      regval = ((bytes << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
-      spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
 
-      regval = addr << 8;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_readonce(priv, addr, tmp_buf, bytes);

Review comment:
       When selecting `LIBC_ARCH_MEMCPY `, Nuttx will disable standard `memcpy` and use ESP32 ROM's `memcpy` which is safe as in IRAM, then it will work as esp-idf. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r522470258



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -862,108 +974,380 @@ static int esp32_writedata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
           bytes = MIN(bytes, SPI_FLASH_WRITE_BUF_SIZE);
         }
 
-      regval = addr & 0xffffff;
-      regval |= bytes << ESP_ROM_SPIFLASH_BYTES_LEN;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      memcpy(tmp_buf, &buffer[off], bytes);
 
-      for (i = 0; i < bytes; i += 4)
-        {
-          res = MIN(4, bytes - i);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_writeonce(priv, addr, tmp_buf, bytes);
+      esp32_spiflash_opdone(priv, flags, me);
 
-          spi_memcpy(&tmp, tmpbuff, res);
-          spi_set_reg(priv, SPI_W0_OFFSET + i, tmp);
-          tmpbuff += res;
+      if (ret)
+        {
+          return ret;
         }
 
-      spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
-      spi_set_reg(priv, SPI_CMD_OFFSET, SPI_FLASH_PP);
-      while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
+      addr += bytes;
+      size -= bytes;
+      off += bytes;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: esp32_writedata
+ *
+ * Description:
+ *   Write plaintext data to SPI Flash at designated address by SPI Flash
+ *   hardware encryption, and written data in SPI Flash is ciphertext.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   0 if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_writedata_encrypted(
+  FAR struct esp32_spiflash_s *priv,
+  uint32_t addr,
+  const uint8_t *buffer,
+  uint32_t size)
+{
+  int i;
+  int blocks;
+  int ret = OK;
+  int me;
+  uint32_t flags;
+  uint32_t tmp_buf[SPI_FLASH_ENCRYPT_WORDS];
+
+  if (addr % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: address=0x%x is not %d-byte align\n",
+           addr, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  if (size % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: size=%u is not %d-byte align\n",
+           size, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  blocks = size / SPI_FLASH_ENCRYPT_UNIT_SIZE;
+
+  for (i = 0; i < blocks; i++)
+    {
+      memcpy(tmp_buf, buffer, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+
+      flags = esp32_spiflash_opstart(priv, &me);
+      esp_rom_spiflash_write_encrypted_enable();
+
+      ret = esp_rom_spiflash_prepare_encrypted_data(addr, tmp_buf);
+      if (ret)
         {
-          ;
+          ferr("ERROR: Failed to prepare encrypted data\n");
+          goto exit;
         }
 
-      if (esp32_wait_idle(priv) != OK)
+      ret = esp32_writeonce(priv, addr, tmp_buf,
+                            SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      if (ret)
         {
-          esp32_spiflash_opdone(priv, flags, me);
-          return -EIO;
+          ferr("ERROR: Failed to write encrypted data @ 0x%x\n", addr);
+          goto exit;
         }
 
-      addr += bytes;
-      size -= bytes;
-
+      esp_rom_spiflash_write_encrypted_disable();
       esp32_spiflash_opdone(priv, flags, me);
+
+      addr += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      buffer += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      size -= SPI_FLASH_ENCRYPT_UNIT_SIZE;
     }
 
+  return 0;
+
+exit:
+  esp_rom_spiflash_write_encrypted_disable();
+  esp32_spiflash_opdone(priv, flags, me);
+
   return ret;
 }
 
 /****************************************************************************
  * Name: esp32_readdata
  *
  * Description:
- *   Read data from SPI Flash at designated address.
+ *   Read max 64 byte data data from SPI Flash at designated address.
+ *
+ *   ESP32 can read max 64 byte once transmission by hardware.
  *
  * Input Parameters:
  *   spi    - ESP32 SPI Flash chip data
  *   addr   - target address
  *   buffer - data buffer pointer
- *   size   - data number
+ *   size   - data number by bytes
  *
  * Returned Value:
  *   OK if success or a negative value if fail.
  *
  ****************************************************************************/
 
-static int esp32_readdata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
-                          uint8_t *buffer, uint32_t size)
+static int IRAM_ATTR esp32_readonce(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint32_t *buffer,
+                                    uint32_t size)
 {
   uint32_t regval;
   uint32_t i;
-  uint32_t bytes;
-  uint32_t tmp;
-  uint32_t res;
-  uint8_t  *tmpbuff = buffer;
-  int me;
-  uint32_t flags;
+
+  if (size > SPI_FLASH_READ_BUF_SIZE)
+    {
+      return -EINVAL;
+    }
 
   if (esp32_wait_idle(priv) != OK)
     {
       return -EIO;
     }
 
-  while (size > 0)
+  regval = ((size << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
+  spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
+
+  regval = addr << 8;
+  spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+
+  spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
+  spi_set_reg(priv, SPI_CMD_OFFSET, SPI_USR);
+  while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
     {
-      flags = esp32_spiflash_opstart(priv, &me);
+      ;
+    }
+
+  for (i = 0; i < (size / 4); i++)
+    {
+      buffer[i] = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+    }
+
+  if (size & 0x3)
+    {
+      regval = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+      memcpy(&buffer[i], &regval, size & 0x3);
+    }
+
+  return OK;
+}
 
+/****************************************************************************
+ * Name: esp32_readdata
+ *
+ * Description:
+ *   Read data from SPI Flash at designated address.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   OK if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_readdata(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint8_t *buffer,
+                                    uint32_t size)
+{
+  int ret;
+  int me;
+  uint32_t flags;
+  uint32_t off = 0;
+  uint32_t bytes;
+  uint32_t tmp_buf[SPI_FLASH_READ_WORDS];
+
+  while (size > 0)
+    {
       bytes = MIN(size, SPI_FLASH_READ_BUF_SIZE);
-      regval = ((bytes << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
-      spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
 
-      regval = addr << 8;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_readonce(priv, addr, tmp_buf, bytes);

Review comment:
       Is it in IRAM?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r522589727



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -1044,24 +1427,12 @@ static ssize_t esp32_read(FAR struct mtd_dev_s *dev, off_t offset,
                           size_t nbytes, FAR uint8_t *buffer)
 {
   int ret;
-  uint8_t *tmpbuff = buffer;
   FAR struct esp32_spiflash_s *priv = MTD2PRIV(dev);
 
 #ifdef CONFIG_ESP32_SPIFLASH_DEBUG
   finfo("esp32_read(%p, 0x%x, %d, %p)\n", dev, offset, nbytes, buffer);
 #endif
 
-#ifdef CONFIG_XTENSA_USE_SEPARATE_IMEM
-  if (esp32_ptr_extram(buffer))
-    {
-      tmpbuff = xtensa_imm_malloc(nbytes);
-      if (tmpbuff == NULL)
-        {
-          return (ssize_t)-ENOMEM;
-        }
-    }
-#endif
-

Review comment:
       Yes, the process that reading data from registers into local buffer and copy data from local buffer into user buffer is in critical sections.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 merged pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-723702632


   > > @donghengqaz please run nxstyle to fix the errors or leave a comment if they are part of the API.
   > 
   > These errors are from using functions that are inside the ROM, we can't change them.
   
   @Ouss4 
   Perhaps, you can change the symbol name in boards/xtensa/esp32/esp32-core/scripts/esp32_rom.ld
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r521901902



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -1044,24 +1427,12 @@ static ssize_t esp32_read(FAR struct mtd_dev_s *dev, off_t offset,
                           size_t nbytes, FAR uint8_t *buffer)
 {
   int ret;
-  uint8_t *tmpbuff = buffer;
   FAR struct esp32_spiflash_s *priv = MTD2PRIV(dev);
 
 #ifdef CONFIG_ESP32_SPIFLASH_DEBUG
   finfo("esp32_read(%p, 0x%x, %d, %p)\n", dev, offset, nbytes, buffer);
 #endif
 
-#ifdef CONFIG_XTENSA_USE_SEPARATE_IMEM
-  if (esp32_ptr_extram(buffer))
-    {
-      tmpbuff = xtensa_imm_malloc(nbytes);
-      if (tmpbuff == NULL)
-        {
-          return (ssize_t)-ENOMEM;
-        }
-    }
-#endif
-

Review comment:
       Why these were removed?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] acassis commented on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-723609138


   Hi @donghengqaz there are some conflicts with esp32_spiflash.c because recent modifications, please update it and submit again.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r522581666



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -862,108 +974,380 @@ static int esp32_writedata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
           bytes = MIN(bytes, SPI_FLASH_WRITE_BUF_SIZE);
         }
 
-      regval = addr & 0xffffff;
-      regval |= bytes << ESP_ROM_SPIFLASH_BYTES_LEN;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      memcpy(tmp_buf, &buffer[off], bytes);
 
-      for (i = 0; i < bytes; i += 4)
-        {
-          res = MIN(4, bytes - i);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_writeonce(priv, addr, tmp_buf, bytes);
+      esp32_spiflash_opdone(priv, flags, me);
 
-          spi_memcpy(&tmp, tmpbuff, res);
-          spi_set_reg(priv, SPI_W0_OFFSET + i, tmp);
-          tmpbuff += res;
+      if (ret)
+        {
+          return ret;
         }
 
-      spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
-      spi_set_reg(priv, SPI_CMD_OFFSET, SPI_FLASH_PP);
-      while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
+      addr += bytes;
+      size -= bytes;
+      off += bytes;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: esp32_writedata
+ *
+ * Description:
+ *   Write plaintext data to SPI Flash at designated address by SPI Flash
+ *   hardware encryption, and written data in SPI Flash is ciphertext.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   0 if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_writedata_encrypted(
+  FAR struct esp32_spiflash_s *priv,
+  uint32_t addr,
+  const uint8_t *buffer,
+  uint32_t size)
+{
+  int i;
+  int blocks;
+  int ret = OK;
+  int me;
+  uint32_t flags;
+  uint32_t tmp_buf[SPI_FLASH_ENCRYPT_WORDS];
+
+  if (addr % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: address=0x%x is not %d-byte align\n",
+           addr, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  if (size % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: size=%u is not %d-byte align\n",
+           size, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  blocks = size / SPI_FLASH_ENCRYPT_UNIT_SIZE;
+
+  for (i = 0; i < blocks; i++)
+    {
+      memcpy(tmp_buf, buffer, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+
+      flags = esp32_spiflash_opstart(priv, &me);
+      esp_rom_spiflash_write_encrypted_enable();
+
+      ret = esp_rom_spiflash_prepare_encrypted_data(addr, tmp_buf);
+      if (ret)
         {
-          ;
+          ferr("ERROR: Failed to prepare encrypted data\n");
+          goto exit;
         }
 
-      if (esp32_wait_idle(priv) != OK)
+      ret = esp32_writeonce(priv, addr, tmp_buf,
+                            SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      if (ret)
         {
-          esp32_spiflash_opdone(priv, flags, me);
-          return -EIO;
+          ferr("ERROR: Failed to write encrypted data @ 0x%x\n", addr);
+          goto exit;
         }
 
-      addr += bytes;
-      size -= bytes;
-
+      esp_rom_spiflash_write_encrypted_disable();
       esp32_spiflash_opdone(priv, flags, me);
+
+      addr += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      buffer += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      size -= SPI_FLASH_ENCRYPT_UNIT_SIZE;
     }
 
+  return 0;
+
+exit:
+  esp_rom_spiflash_write_encrypted_disable();
+  esp32_spiflash_opdone(priv, flags, me);
+
   return ret;
 }
 
 /****************************************************************************
  * Name: esp32_readdata
  *
  * Description:
- *   Read data from SPI Flash at designated address.
+ *   Read max 64 byte data data from SPI Flash at designated address.
+ *
+ *   ESP32 can read max 64 byte once transmission by hardware.
  *
  * Input Parameters:
  *   spi    - ESP32 SPI Flash chip data
  *   addr   - target address
  *   buffer - data buffer pointer
- *   size   - data number
+ *   size   - data number by bytes
  *
  * Returned Value:
  *   OK if success or a negative value if fail.
  *
  ****************************************************************************/
 
-static int esp32_readdata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
-                          uint8_t *buffer, uint32_t size)
+static int IRAM_ATTR esp32_readonce(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint32_t *buffer,
+                                    uint32_t size)
 {
   uint32_t regval;
   uint32_t i;
-  uint32_t bytes;
-  uint32_t tmp;
-  uint32_t res;
-  uint8_t  *tmpbuff = buffer;
-  int me;
-  uint32_t flags;
+
+  if (size > SPI_FLASH_READ_BUF_SIZE)
+    {
+      return -EINVAL;
+    }
 
   if (esp32_wait_idle(priv) != OK)
     {
       return -EIO;
     }
 
-  while (size > 0)
+  regval = ((size << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
+  spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
+
+  regval = addr << 8;
+  spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+
+  spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
+  spi_set_reg(priv, SPI_CMD_OFFSET, SPI_USR);
+  while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
     {
-      flags = esp32_spiflash_opstart(priv, &me);
+      ;
+    }
+
+  for (i = 0; i < (size / 4); i++)
+    {
+      buffer[i] = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+    }
+
+  if (size & 0x3)
+    {
+      regval = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+      memcpy(&buffer[i], &regval, size & 0x3);
+    }
+
+  return OK;
+}
 
+/****************************************************************************
+ * Name: esp32_readdata
+ *
+ * Description:
+ *   Read data from SPI Flash at designated address.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   OK if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_readdata(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint8_t *buffer,
+                                    uint32_t size)
+{
+  int ret;
+  int me;
+  uint32_t flags;
+  uint32_t off = 0;
+  uint32_t bytes;
+  uint32_t tmp_buf[SPI_FLASH_READ_WORDS];
+
+  while (size > 0)
+    {
       bytes = MIN(size, SPI_FLASH_READ_BUF_SIZE);
-      regval = ((bytes << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
-      spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
 
-      regval = addr << 8;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_readonce(priv, addr, tmp_buf, bytes);

Review comment:
       When selecting `LIBC_ARCH_MEMCPY `, Nuttx will disable standard `memcpy` and use ROM's `memcpy`, then it will work as esp-idf. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-722427773


   > @donghengqaz please run nxstyle to fix the errors or leave a comment if they are part of the API.
   
   These errors are from using functions that are inside the ROM, we can't change them.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r522581666



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -862,108 +974,380 @@ static int esp32_writedata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
           bytes = MIN(bytes, SPI_FLASH_WRITE_BUF_SIZE);
         }
 
-      regval = addr & 0xffffff;
-      regval |= bytes << ESP_ROM_SPIFLASH_BYTES_LEN;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      memcpy(tmp_buf, &buffer[off], bytes);
 
-      for (i = 0; i < bytes; i += 4)
-        {
-          res = MIN(4, bytes - i);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_writeonce(priv, addr, tmp_buf, bytes);
+      esp32_spiflash_opdone(priv, flags, me);
 
-          spi_memcpy(&tmp, tmpbuff, res);
-          spi_set_reg(priv, SPI_W0_OFFSET + i, tmp);
-          tmpbuff += res;
+      if (ret)
+        {
+          return ret;
         }
 
-      spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
-      spi_set_reg(priv, SPI_CMD_OFFSET, SPI_FLASH_PP);
-      while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
+      addr += bytes;
+      size -= bytes;
+      off += bytes;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: esp32_writedata
+ *
+ * Description:
+ *   Write plaintext data to SPI Flash at designated address by SPI Flash
+ *   hardware encryption, and written data in SPI Flash is ciphertext.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   0 if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_writedata_encrypted(
+  FAR struct esp32_spiflash_s *priv,
+  uint32_t addr,
+  const uint8_t *buffer,
+  uint32_t size)
+{
+  int i;
+  int blocks;
+  int ret = OK;
+  int me;
+  uint32_t flags;
+  uint32_t tmp_buf[SPI_FLASH_ENCRYPT_WORDS];
+
+  if (addr % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: address=0x%x is not %d-byte align\n",
+           addr, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  if (size % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: size=%u is not %d-byte align\n",
+           size, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  blocks = size / SPI_FLASH_ENCRYPT_UNIT_SIZE;
+
+  for (i = 0; i < blocks; i++)
+    {
+      memcpy(tmp_buf, buffer, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+
+      flags = esp32_spiflash_opstart(priv, &me);
+      esp_rom_spiflash_write_encrypted_enable();
+
+      ret = esp_rom_spiflash_prepare_encrypted_data(addr, tmp_buf);
+      if (ret)
         {
-          ;
+          ferr("ERROR: Failed to prepare encrypted data\n");
+          goto exit;
         }
 
-      if (esp32_wait_idle(priv) != OK)
+      ret = esp32_writeonce(priv, addr, tmp_buf,
+                            SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      if (ret)
         {
-          esp32_spiflash_opdone(priv, flags, me);
-          return -EIO;
+          ferr("ERROR: Failed to write encrypted data @ 0x%x\n", addr);
+          goto exit;
         }
 
-      addr += bytes;
-      size -= bytes;
-
+      esp_rom_spiflash_write_encrypted_disable();
       esp32_spiflash_opdone(priv, flags, me);
+
+      addr += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      buffer += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      size -= SPI_FLASH_ENCRYPT_UNIT_SIZE;
     }
 
+  return 0;
+
+exit:
+  esp_rom_spiflash_write_encrypted_disable();
+  esp32_spiflash_opdone(priv, flags, me);
+
   return ret;
 }
 
 /****************************************************************************
  * Name: esp32_readdata
  *
  * Description:
- *   Read data from SPI Flash at designated address.
+ *   Read max 64 byte data data from SPI Flash at designated address.
+ *
+ *   ESP32 can read max 64 byte once transmission by hardware.
  *
  * Input Parameters:
  *   spi    - ESP32 SPI Flash chip data
  *   addr   - target address
  *   buffer - data buffer pointer
- *   size   - data number
+ *   size   - data number by bytes
  *
  * Returned Value:
  *   OK if success or a negative value if fail.
  *
  ****************************************************************************/
 
-static int esp32_readdata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
-                          uint8_t *buffer, uint32_t size)
+static int IRAM_ATTR esp32_readonce(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint32_t *buffer,
+                                    uint32_t size)
 {
   uint32_t regval;
   uint32_t i;
-  uint32_t bytes;
-  uint32_t tmp;
-  uint32_t res;
-  uint8_t  *tmpbuff = buffer;
-  int me;
-  uint32_t flags;
+
+  if (size > SPI_FLASH_READ_BUF_SIZE)
+    {
+      return -EINVAL;
+    }
 
   if (esp32_wait_idle(priv) != OK)
     {
       return -EIO;
     }
 
-  while (size > 0)
+  regval = ((size << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
+  spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
+
+  regval = addr << 8;
+  spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+
+  spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
+  spi_set_reg(priv, SPI_CMD_OFFSET, SPI_USR);
+  while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
     {
-      flags = esp32_spiflash_opstart(priv, &me);
+      ;
+    }
+
+  for (i = 0; i < (size / 4); i++)
+    {
+      buffer[i] = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+    }
+
+  if (size & 0x3)
+    {
+      regval = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+      memcpy(&buffer[i], &regval, size & 0x3);
+    }
+
+  return OK;
+}
 
+/****************************************************************************
+ * Name: esp32_readdata
+ *
+ * Description:
+ *   Read data from SPI Flash at designated address.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   OK if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_readdata(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint8_t *buffer,
+                                    uint32_t size)
+{
+  int ret;
+  int me;
+  uint32_t flags;
+  uint32_t off = 0;
+  uint32_t bytes;
+  uint32_t tmp_buf[SPI_FLASH_READ_WORDS];
+
+  while (size > 0)
+    {
       bytes = MIN(size, SPI_FLASH_READ_BUF_SIZE);
-      regval = ((bytes << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
-      spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
 
-      regval = addr << 8;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_readonce(priv, addr, tmp_buf, bytes);

Review comment:
       When selecting `LIBC_ARCH_MEMCPY `, Nuttx will disable standard `memcpy` and use ROM's `memcpy` which is safe as in IRAM, then it will work as esp-idf. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r521928395



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -1044,24 +1427,12 @@ static ssize_t esp32_read(FAR struct mtd_dev_s *dev, off_t offset,
                           size_t nbytes, FAR uint8_t *buffer)
 {
   int ret;
-  uint8_t *tmpbuff = buffer;
   FAR struct esp32_spiflash_s *priv = MTD2PRIV(dev);
 
 #ifdef CONFIG_ESP32_SPIFLASH_DEBUG
   finfo("esp32_read(%p, 0x%x, %d, %p)\n", dev, offset, nbytes, buffer);
 #endif
 
-#ifdef CONFIG_XTENSA_USE_SEPARATE_IMEM
-  if (esp32_ptr_extram(buffer))
-    {
-      tmpbuff = xtensa_imm_malloc(nbytes);
-      if (tmpbuff == NULL)
-        {
-          return (ssize_t)-ENOMEM;
-        }
-    }
-#endif
-

Review comment:
       For example reading SPI Flash data, We should copy data from SPI data registers to user buffers, to avoid the buffer address is not aligned by 4 bytes or other case like it is from extram, I firstly copy these data from registers into function's local buffer and then copy data from local buffer into user buffer. This operation maybe a little rough, but it is useful. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-722463677


   But this warning could be fixed:
   ```
   esp32_spiflash.c:174:1: error: Too many blank lines
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r521946905



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -862,108 +974,380 @@ static int esp32_writedata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
           bytes = MIN(bytes, SPI_FLASH_WRITE_BUF_SIZE);
         }
 
-      regval = addr & 0xffffff;
-      regval |= bytes << ESP_ROM_SPIFLASH_BYTES_LEN;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      memcpy(tmp_buf, &buffer[off], bytes);
 
-      for (i = 0; i < bytes; i += 4)
-        {
-          res = MIN(4, bytes - i);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_writeonce(priv, addr, tmp_buf, bytes);
+      esp32_spiflash_opdone(priv, flags, me);
 
-          spi_memcpy(&tmp, tmpbuff, res);
-          spi_set_reg(priv, SPI_W0_OFFSET + i, tmp);
-          tmpbuff += res;
+      if (ret)
+        {
+          return ret;
         }
 
-      spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
-      spi_set_reg(priv, SPI_CMD_OFFSET, SPI_FLASH_PP);
-      while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
+      addr += bytes;
+      size -= bytes;
+      off += bytes;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: esp32_writedata
+ *
+ * Description:
+ *   Write plaintext data to SPI Flash at designated address by SPI Flash
+ *   hardware encryption, and written data in SPI Flash is ciphertext.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   0 if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_writedata_encrypted(
+  FAR struct esp32_spiflash_s *priv,
+  uint32_t addr,
+  const uint8_t *buffer,
+  uint32_t size)
+{
+  int i;
+  int blocks;
+  int ret = OK;
+  int me;
+  uint32_t flags;
+  uint32_t tmp_buf[SPI_FLASH_ENCRYPT_WORDS];
+
+  if (addr % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: address=0x%x is not %d-byte align\n",
+           addr, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  if (size % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: size=%u is not %d-byte align\n",
+           size, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  blocks = size / SPI_FLASH_ENCRYPT_UNIT_SIZE;
+
+  for (i = 0; i < blocks; i++)
+    {
+      memcpy(tmp_buf, buffer, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+
+      flags = esp32_spiflash_opstart(priv, &me);
+      esp_rom_spiflash_write_encrypted_enable();
+
+      ret = esp_rom_spiflash_prepare_encrypted_data(addr, tmp_buf);
+      if (ret)
         {
-          ;
+          ferr("ERROR: Failed to prepare encrypted data\n");
+          goto exit;
         }
 
-      if (esp32_wait_idle(priv) != OK)
+      ret = esp32_writeonce(priv, addr, tmp_buf,
+                            SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      if (ret)
         {
-          esp32_spiflash_opdone(priv, flags, me);
-          return -EIO;
+          ferr("ERROR: Failed to write encrypted data @ 0x%x\n", addr);
+          goto exit;
         }
 
-      addr += bytes;
-      size -= bytes;
-
+      esp_rom_spiflash_write_encrypted_disable();
       esp32_spiflash_opdone(priv, flags, me);
+
+      addr += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      buffer += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      size -= SPI_FLASH_ENCRYPT_UNIT_SIZE;
     }
 
+  return 0;
+
+exit:
+  esp_rom_spiflash_write_encrypted_disable();
+  esp32_spiflash_opdone(priv, flags, me);
+
   return ret;
 }
 
 /****************************************************************************
  * Name: esp32_readdata
  *
  * Description:
- *   Read data from SPI Flash at designated address.
+ *   Read max 64 byte data data from SPI Flash at designated address.
+ *
+ *   ESP32 can read max 64 byte once transmission by hardware.
  *
  * Input Parameters:
  *   spi    - ESP32 SPI Flash chip data
  *   addr   - target address
  *   buffer - data buffer pointer
- *   size   - data number
+ *   size   - data number by bytes
  *
  * Returned Value:
  *   OK if success or a negative value if fail.
  *
  ****************************************************************************/
 
-static int esp32_readdata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
-                          uint8_t *buffer, uint32_t size)
+static int IRAM_ATTR esp32_readonce(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint32_t *buffer,
+                                    uint32_t size)
 {
   uint32_t regval;
   uint32_t i;
-  uint32_t bytes;
-  uint32_t tmp;
-  uint32_t res;
-  uint8_t  *tmpbuff = buffer;
-  int me;
-  uint32_t flags;
+
+  if (size > SPI_FLASH_READ_BUF_SIZE)
+    {
+      return -EINVAL;
+    }
 
   if (esp32_wait_idle(priv) != OK)
     {
       return -EIO;
     }
 
-  while (size > 0)
+  regval = ((size << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
+  spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
+
+  regval = addr << 8;
+  spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+
+  spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
+  spi_set_reg(priv, SPI_CMD_OFFSET, SPI_USR);
+  while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
     {
-      flags = esp32_spiflash_opstart(priv, &me);
+      ;
+    }
+
+  for (i = 0; i < (size / 4); i++)
+    {
+      buffer[i] = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+    }
+
+  if (size & 0x3)
+    {
+      regval = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+      memcpy(&buffer[i], &regval, size & 0x3);
+    }
+
+  return OK;
+}
 
+/****************************************************************************
+ * Name: esp32_readdata
+ *
+ * Description:
+ *   Read data from SPI Flash at designated address.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   OK if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_readdata(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint8_t *buffer,
+                                    uint32_t size)
+{
+  int ret;
+  int me;
+  uint32_t flags;
+  uint32_t off = 0;
+  uint32_t bytes;
+  uint32_t tmp_buf[SPI_FLASH_READ_WORDS];
+
+  while (size > 0)
+    {
       bytes = MIN(size, SPI_FLASH_READ_BUF_SIZE);
-      regval = ((bytes << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
-      spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
 
-      regval = addr << 8;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_readonce(priv, addr, tmp_buf, bytes);

Review comment:
        `esp32_spiflash_opstart` will disable the cache and `esp32_readonce` is using standard `memcpy`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea commented on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
jerpelea commented on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-722342887


   @donghengqaz please run nxstyle to fix the erros 
   
   esp32_spiflash.c:155:5: error: Mixed case identifier found
   esp32_spiflash.c:156:5: error: Mixed case identifier found
   esp32_spiflash.c:157:5: error: Mixed case identifier found
   esp32_spiflash.c:407:2: error: Mixed case identifier found
   esp32_spiflash.c:409:2: error: Mixed case identifier found
   esp32_spiflash.c:439:2: error: Mixed case identifier found
   esp32_spiflash.c:440:2: error: Mixed case identifier found
   esp32_spiflash.c:442:2: error: Mixed case identifier found
   esp32_spiflash.c:443:2: error: Mixed case identifier found
   esp32_soc.h:456:11: error: Mixed case identifier found
   esp32_spiflash.c:174:1: error: Too many blank lines
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r521948523



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -1044,24 +1427,12 @@ static ssize_t esp32_read(FAR struct mtd_dev_s *dev, off_t offset,
                           size_t nbytes, FAR uint8_t *buffer)
 {
   int ret;
-  uint8_t *tmpbuff = buffer;
   FAR struct esp32_spiflash_s *priv = MTD2PRIV(dev);
 
 #ifdef CONFIG_ESP32_SPIFLASH_DEBUG
   finfo("esp32_read(%p, 0x%x, %d, %p)\n", dev, offset, nbytes, buffer);
 #endif
 
-#ifdef CONFIG_XTENSA_USE_SEPARATE_IMEM
-  if (esp32_ptr_extram(buffer))
-    {
-      tmpbuff = xtensa_imm_malloc(nbytes);
-      if (tmpbuff == NULL)
-        {
-          return (ssize_t)-ENOMEM;
-        }
-    }
-#endif
-

Review comment:
       Did you create smaller critical sections around copying from local buffer to user buffer? (I made a comment somewhere else were I think it's related to this.)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#discussion_r522033505



##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -862,108 +974,380 @@ static int esp32_writedata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
           bytes = MIN(bytes, SPI_FLASH_WRITE_BUF_SIZE);
         }
 
-      regval = addr & 0xffffff;
-      regval |= bytes << ESP_ROM_SPIFLASH_BYTES_LEN;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      memcpy(tmp_buf, &buffer[off], bytes);
 
-      for (i = 0; i < bytes; i += 4)
-        {
-          res = MIN(4, bytes - i);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_writeonce(priv, addr, tmp_buf, bytes);
+      esp32_spiflash_opdone(priv, flags, me);
 
-          spi_memcpy(&tmp, tmpbuff, res);
-          spi_set_reg(priv, SPI_W0_OFFSET + i, tmp);
-          tmpbuff += res;
+      if (ret)
+        {
+          return ret;
         }
 
-      spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
-      spi_set_reg(priv, SPI_CMD_OFFSET, SPI_FLASH_PP);
-      while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
+      addr += bytes;
+      size -= bytes;
+      off += bytes;
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: esp32_writedata
+ *
+ * Description:
+ *   Write plaintext data to SPI Flash at designated address by SPI Flash
+ *   hardware encryption, and written data in SPI Flash is ciphertext.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   0 if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_writedata_encrypted(
+  FAR struct esp32_spiflash_s *priv,
+  uint32_t addr,
+  const uint8_t *buffer,
+  uint32_t size)
+{
+  int i;
+  int blocks;
+  int ret = OK;
+  int me;
+  uint32_t flags;
+  uint32_t tmp_buf[SPI_FLASH_ENCRYPT_WORDS];
+
+  if (addr % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: address=0x%x is not %d-byte align\n",
+           addr, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  if (size % SPI_FLASH_ENCRYPT_UNIT_SIZE)
+    {
+      ferr("ERROR: size=%u is not %d-byte align\n",
+           size, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      return -EINVAL;
+    }
+
+  blocks = size / SPI_FLASH_ENCRYPT_UNIT_SIZE;
+
+  for (i = 0; i < blocks; i++)
+    {
+      memcpy(tmp_buf, buffer, SPI_FLASH_ENCRYPT_UNIT_SIZE);
+
+      flags = esp32_spiflash_opstart(priv, &me);
+      esp_rom_spiflash_write_encrypted_enable();
+
+      ret = esp_rom_spiflash_prepare_encrypted_data(addr, tmp_buf);
+      if (ret)
         {
-          ;
+          ferr("ERROR: Failed to prepare encrypted data\n");
+          goto exit;
         }
 
-      if (esp32_wait_idle(priv) != OK)
+      ret = esp32_writeonce(priv, addr, tmp_buf,
+                            SPI_FLASH_ENCRYPT_UNIT_SIZE);
+      if (ret)
         {
-          esp32_spiflash_opdone(priv, flags, me);
-          return -EIO;
+          ferr("ERROR: Failed to write encrypted data @ 0x%x\n", addr);
+          goto exit;
         }
 
-      addr += bytes;
-      size -= bytes;
-
+      esp_rom_spiflash_write_encrypted_disable();
       esp32_spiflash_opdone(priv, flags, me);
+
+      addr += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      buffer += SPI_FLASH_ENCRYPT_UNIT_SIZE;
+      size -= SPI_FLASH_ENCRYPT_UNIT_SIZE;
     }
 
+  return 0;
+
+exit:
+  esp_rom_spiflash_write_encrypted_disable();
+  esp32_spiflash_opdone(priv, flags, me);
+
   return ret;
 }
 
 /****************************************************************************
  * Name: esp32_readdata
  *
  * Description:
- *   Read data from SPI Flash at designated address.
+ *   Read max 64 byte data data from SPI Flash at designated address.
+ *
+ *   ESP32 can read max 64 byte once transmission by hardware.
  *
  * Input Parameters:
  *   spi    - ESP32 SPI Flash chip data
  *   addr   - target address
  *   buffer - data buffer pointer
- *   size   - data number
+ *   size   - data number by bytes
  *
  * Returned Value:
  *   OK if success or a negative value if fail.
  *
  ****************************************************************************/
 
-static int esp32_readdata(FAR struct esp32_spiflash_s *priv, uint32_t addr,
-                          uint8_t *buffer, uint32_t size)
+static int IRAM_ATTR esp32_readonce(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint32_t *buffer,
+                                    uint32_t size)
 {
   uint32_t regval;
   uint32_t i;
-  uint32_t bytes;
-  uint32_t tmp;
-  uint32_t res;
-  uint8_t  *tmpbuff = buffer;
-  int me;
-  uint32_t flags;
+
+  if (size > SPI_FLASH_READ_BUF_SIZE)
+    {
+      return -EINVAL;
+    }
 
   if (esp32_wait_idle(priv) != OK)
     {
       return -EIO;
     }
 
-  while (size > 0)
+  regval = ((size << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
+  spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
+
+  regval = addr << 8;
+  spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+
+  spi_set_reg(priv, SPI_RD_STATUS_OFFSET, 0);
+  spi_set_reg(priv, SPI_CMD_OFFSET, SPI_USR);
+  while (spi_get_reg(priv, SPI_CMD_OFFSET) != 0)
     {
-      flags = esp32_spiflash_opstart(priv, &me);
+      ;
+    }
+
+  for (i = 0; i < (size / 4); i++)
+    {
+      buffer[i] = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+    }
+
+  if (size & 0x3)
+    {
+      regval = spi_get_reg(priv, SPI_W0_OFFSET + i * 4);
+      memcpy(&buffer[i], &regval, size & 0x3);
+    }
+
+  return OK;
+}
 
+/****************************************************************************
+ * Name: esp32_readdata
+ *
+ * Description:
+ *   Read data from SPI Flash at designated address.
+ *
+ * Input Parameters:
+ *   spi    - ESP32 SPI Flash chip data
+ *   addr   - target address
+ *   buffer - data buffer pointer
+ *   size   - data number
+ *
+ * Returned Value:
+ *   OK if success or a negative value if fail.
+ *
+ ****************************************************************************/
+
+static int IRAM_ATTR esp32_readdata(FAR struct esp32_spiflash_s *priv,
+                                    uint32_t addr,
+                                    uint8_t *buffer,
+                                    uint32_t size)
+{
+  int ret;
+  int me;
+  uint32_t flags;
+  uint32_t off = 0;
+  uint32_t bytes;
+  uint32_t tmp_buf[SPI_FLASH_READ_WORDS];
+
+  while (size > 0)
+    {
       bytes = MIN(size, SPI_FLASH_READ_BUF_SIZE);
-      regval = ((bytes << 3) - 1) << SPI_USR_MISO_DBITLEN_S;
-      spi_set_reg(priv, SPI_MISO_DLEN_OFFSET, regval);
 
-      regval = addr << 8;
-      spi_set_reg(priv, SPI_ADDR_OFFSET, regval);
+      flags = esp32_spiflash_opstart(priv, &me);
+      ret = esp32_readonce(priv, addr, tmp_buf, bytes);

Review comment:
       I have added option to select `LIB_ARCH_MEMCPY`  when selecting ARCH_CHIP_ESP32. https://github.com/apache/incubator-nuttx/blob/b63c0863b27d1dd884a47594fa6a59f203de47eb/arch/xtensa/Kconfig#L21




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] jerpelea edited a comment on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
jerpelea edited a comment on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-722342887


   @donghengqaz please run nxstyle to fix the errors or leave a comment if they are part of the API. 
   
   esp32_spiflash.c:155:5: error: Mixed case identifier found
   esp32_spiflash.c:156:5: error: Mixed case identifier found
   esp32_spiflash.c:157:5: error: Mixed case identifier found
   esp32_spiflash.c:407:2: error: Mixed case identifier found
   esp32_spiflash.c:409:2: error: Mixed case identifier found
   esp32_spiflash.c:439:2: error: Mixed case identifier found
   esp32_spiflash.c:440:2: error: Mixed case identifier found
   esp32_spiflash.c:442:2: error: Mixed case identifier found
   esp32_spiflash.c:443:2: error: Mixed case identifier found
   esp32_soc.h:456:11: error: Mixed case identifier found
   esp32_spiflash.c:174:1: error: Too many blank lines
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] donghengqaz commented on pull request #2224: xtensa/esp32: Add SPI Flash hardware encryption I/O support

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on pull request #2224:
URL: https://github.com/apache/incubator-nuttx/pull/2224#issuecomment-724573471


   > Hi @donghengqaz there are some conflicts with esp32_spiflash.c because recent modifications, please update it and submit again.
   
   Hi @acassis The conflicts are fixed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org