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 2022/01/24 08:50:18 UTC

[GitHub] [incubator-nuttx] pussuw opened a new pull request #5322: Mpfs build protected

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


   ## Summary
   Implements CONFIG_BUILD_PROTECTED for MPFS target. The kernel-/userspace separation is done with MMU by mapping the user space memory with vaddr=paddr mapping. The nuttx kernel runs in M-mode (flat addressing).
   
   The PMP implementation is extended to check for incoming parameters, including the existence of implementation specific features of the PMP. An access check query is provided to see if the PMP is already configured and whether the access rights are set correctly or not.
   
   Upon boot, the PMP access is tested and the user space is opened if not done already. If PMP is configured and access to the user space is revoked, the system halts (via assert) as nothing can be done at that point.
   
   ## Impact
   Adds 'CONFIG_BUILD_PROTECTED' option for MPFS
   
   ## Testing
   Tested with icicle board, ran ostest and created a custom 'poke' application to test that the MMU mappings work. Poking mapped memory works (no fault is triggered) and poking unmapped memory triggers an immediate fault.
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5322: Mpfs build protected

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


   @pussuw look like we can merge many small patch into large one


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] acassis commented on a change in pull request #5322: Mpfs build protected

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



##########
File path: arch/risc-v/src/mpfs/mpfs_userspace.c
##########
@@ -192,9 +192,8 @@ static void configure_mpu(void)
   /* Configure the PMP to permit user-space access to its ROM and RAM.
    *
    * Note: PMP by default revokes access, thus if different privilege modes
-   * are in use (mstatus.mprv is set), the the user space _must_ be granted
-   * access here, otherwise an exception will fire when the user space task
-   * is started.
+   * are in use, the the user space _must_ be granted access here, otherwise

Review comment:
       ```suggestion
      * are in use, the user space _must_ be granted access here, otherwise
   ```




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] pussuw commented on a change in pull request #5322: Mpfs build protected

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



##########
File path: arch/risc-v/src/common/riscv_pmp.c
##########
@@ -644,9 +644,19 @@ int riscv_check_pmp_access(uintptr_t attr, uintptr_t base, uintptr_t size)
           continue;
         }
 
-      /* Does this address range match ? */
-
-      if (base >= entry.base || end <= entry.end)
+      /* Does this address range match ? Take partial matches into account.

Review comment:
       No problem, I preserved the local history but I can also squash the parts you mentioned.

##########
File path: boards/risc-v/mpfs/icicle/scripts/kernel-space.ld
##########
@@ -86,13 +86,13 @@ SECTIONS
         *(.gnu.linkonce.b.*)
         *(.gnu.linkonce.sb.*)
         *(COMMON)
-        . = ALIGN(4);
-        _ebss = ABSOLUTE(.);
     } > ksram
 
     /* Page tables here, align to 4K boundary */
     .pgtables : ALIGN(0x1000) {
         *(.pgtables)
+         . = ALIGN(4);

Review comment:
       No problem, I preserved the local history but I can also squash the parts you mentioned.

##########
File path: arch/risc-v/src/mpfs/mpfs_userspace.c
##########
@@ -192,9 +192,8 @@ static void configure_mpu(void)
   /* Configure the PMP to permit user-space access to its ROM and RAM.
    *
    * Note: PMP by default revokes access, thus if different privilege modes
-   * are in use (mstatus.mprv is set), the the user space _must_ be granted
-   * access here, otherwise an exception will fire when the user space task
-   * is started.
+   * are in use, the user space _must_ be granted access here, otherwise

Review comment:
       No problem, I preserved the local history but I can also squash the parts you mentioned.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5322: Mpfs build protected

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



##########
File path: arch/risc-v/src/common/riscv_pmp.c
##########
@@ -644,9 +644,19 @@ int riscv_check_pmp_access(uintptr_t attr, uintptr_t base, uintptr_t size)
           continue;
         }
 
-      /* Does this address range match ? */
-
-      if (base >= entry.base || end <= entry.end)
+      /* Does this address range match ? Take partial matches into account.

Review comment:
       should we merge into previous patch

##########
File path: boards/risc-v/mpfs/icicle/scripts/kernel-space.ld
##########
@@ -86,13 +86,13 @@ SECTIONS
         *(.gnu.linkonce.b.*)
         *(.gnu.linkonce.sb.*)
         *(COMMON)
-        . = ALIGN(4);
-        _ebss = ABSOLUTE(.);
     } > ksram
 
     /* Page tables here, align to 4K boundary */
     .pgtables : ALIGN(0x1000) {
         *(.pgtables)
+         . = ALIGN(4);

Review comment:
       merge to previous patch?

##########
File path: arch/risc-v/src/mpfs/mpfs_userspace.c
##########
@@ -192,9 +192,8 @@ static void configure_mpu(void)
   /* Configure the PMP to permit user-space access to its ROM and RAM.
    *
    * Note: PMP by default revokes access, thus if different privilege modes
-   * are in use (mstatus.mprv is set), the the user space _must_ be granted
-   * access here, otherwise an exception will fire when the user space task
-   * is started.
+   * are in use, the user space _must_ be granted access here, otherwise

Review comment:
       merge with previous patch?

##########
File path: arch/risc-v/src/common/riscv_pmp.c
##########
@@ -44,6 +58,361 @@
       reg |= attr << (offset * PMP_CFG_BITS_CNT); \
     } while(0);
 
+#define PMP_READ_REGION_FROM_REG(region, reg) \
+  ({ \
+    uintptr_t tmp = READ_CSR(reg); \
+    tmp >>= ((region % PMP_CFG_CNT_IN_REG) * PMP_CFG_BITS_CNT); \
+    tmp &= PMP_CFG_FLAG_MASK; \
+    tmp; \
+  })
+
+#ifndef min
+#define min(a,b) ((a) < (b) ? (a) : (b))
+#endif
+
+#ifndef max
+#define max(a,b) ((a) > (b) ? (a) : (b))
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* Helper structure for handling a PMP entry */
+
+struct pmp_entry_s
+{
+  uintptr_t base;   /* Base address of region */
+  uintptr_t end;    /* End address of region */
+  uintptr_t size;   /* Region size */
+  uint8_t   mode;   /* Address matching mode */
+  uint8_t   rwx;    /* Access rights */
+};
+
+typedef struct pmp_entry_s pmp_entry_t;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pmp_check_addrmatch_type
+ *
+ * Description:
+ *   Test if an address matching type is supported by the architecture.
+ *
+ * Input Parameters:
+ *   type - The type to test.
+ *
+ * Returned Value:
+ *   true if it is, false otherwise.
+ *
+ ****************************************************************************/
+
+static bool pmp_check_addrmatch_type(uintptr_t type)
+{
+  /* Parameter is potentially unused */
+
+  (void) type;

Review comment:
       UNUSED(type)




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] pussuw commented on pull request #5322: Mpfs build protected

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


   I squashed about half of the commits, please let me know if you wish for me to squash more!


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] pussuw commented on pull request #5322: Mpfs build protected

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


   I squashed about half of the commits, please let me know if you wish for me to squash more!


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] pussuw commented on a change in pull request #5322: Mpfs build protected

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



##########
File path: arch/risc-v/src/common/riscv_pmp.c
##########
@@ -44,6 +58,361 @@
       reg |= attr << (offset * PMP_CFG_BITS_CNT); \
     } while(0);
 
+#define PMP_READ_REGION_FROM_REG(region, reg) \
+  ({ \
+    uintptr_t tmp = READ_CSR(reg); \
+    tmp >>= ((region % PMP_CFG_CNT_IN_REG) * PMP_CFG_BITS_CNT); \
+    tmp &= PMP_CFG_FLAG_MASK; \
+    tmp; \
+  })
+
+#ifndef min
+#define min(a,b) ((a) < (b) ? (a) : (b))
+#endif
+
+#ifndef max
+#define max(a,b) ((a) > (b) ? (a) : (b))
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* Helper structure for handling a PMP entry */
+
+struct pmp_entry_s
+{
+  uintptr_t base;   /* Base address of region */
+  uintptr_t end;    /* End address of region */
+  uintptr_t size;   /* Region size */
+  uint8_t   mode;   /* Address matching mode */
+  uint8_t   rwx;    /* Access rights */
+};
+
+typedef struct pmp_entry_s pmp_entry_t;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pmp_check_addrmatch_type
+ *
+ * Description:
+ *   Test if an address matching type is supported by the architecture.
+ *
+ * Input Parameters:
+ *   type - The type to test.
+ *
+ * Returned Value:
+ *   true if it is, false otherwise.
+ *
+ ****************************************************************************/
+
+static bool pmp_check_addrmatch_type(uintptr_t type)
+{
+  /* Parameter is potentially unused */
+
+  (void) type;

Review comment:
       Sure




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5322: Mpfs build protected

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


   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5322: Mpfs build protected

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


   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] pussuw commented on a change in pull request #5322: Mpfs build protected

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



##########
File path: arch/risc-v/src/common/riscv_pmp.c
##########
@@ -44,6 +58,361 @@
       reg |= attr << (offset * PMP_CFG_BITS_CNT); \
     } while(0);
 
+#define PMP_READ_REGION_FROM_REG(region, reg) \
+  ({ \
+    uintptr_t tmp = READ_CSR(reg); \
+    tmp >>= ((region % PMP_CFG_CNT_IN_REG) * PMP_CFG_BITS_CNT); \
+    tmp &= PMP_CFG_FLAG_MASK; \
+    tmp; \
+  })
+
+#ifndef min
+#define min(a,b) ((a) < (b) ? (a) : (b))
+#endif
+
+#ifndef max
+#define max(a,b) ((a) > (b) ? (a) : (b))
+#endif
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* Helper structure for handling a PMP entry */
+
+struct pmp_entry_s
+{
+  uintptr_t base;   /* Base address of region */
+  uintptr_t end;    /* End address of region */
+  uintptr_t size;   /* Region size */
+  uint8_t   mode;   /* Address matching mode */
+  uint8_t   rwx;    /* Access rights */
+};
+
+typedef struct pmp_entry_s pmp_entry_t;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pmp_check_addrmatch_type
+ *
+ * Description:
+ *   Test if an address matching type is supported by the architecture.
+ *
+ * Input Parameters:
+ *   type - The type to test.
+ *
+ * Returned Value:
+ *   true if it is, false otherwise.
+ *
+ ****************************************************************************/
+
+static bool pmp_check_addrmatch_type(uintptr_t type)
+{
+  /* Parameter is potentially unused */
+
+  (void) type;

Review comment:
       Sure

##########
File path: arch/risc-v/src/common/riscv_pmp.c
##########
@@ -644,9 +644,19 @@ int riscv_check_pmp_access(uintptr_t attr, uintptr_t base, uintptr_t size)
           continue;
         }
 
-      /* Does this address range match ? */
-
-      if (base >= entry.base || end <= entry.end)
+      /* Does this address range match ? Take partial matches into account.

Review comment:
       No problem, I preserved the local history but I can also squash the parts you mentioned.

##########
File path: boards/risc-v/mpfs/icicle/scripts/kernel-space.ld
##########
@@ -86,13 +86,13 @@ SECTIONS
         *(.gnu.linkonce.b.*)
         *(.gnu.linkonce.sb.*)
         *(COMMON)
-        . = ALIGN(4);
-        _ebss = ABSOLUTE(.);
     } > ksram
 
     /* Page tables here, align to 4K boundary */
     .pgtables : ALIGN(0x1000) {
         *(.pgtables)
+         . = ALIGN(4);

Review comment:
       No problem, I preserved the local history but I can also squash the parts you mentioned.

##########
File path: arch/risc-v/src/mpfs/mpfs_userspace.c
##########
@@ -192,9 +192,8 @@ static void configure_mpu(void)
   /* Configure the PMP to permit user-space access to its ROM and RAM.
    *
    * Note: PMP by default revokes access, thus if different privilege modes
-   * are in use (mstatus.mprv is set), the the user space _must_ be granted
-   * access here, otherwise an exception will fire when the user space task
-   * is started.
+   * are in use, the user space _must_ be granted access here, otherwise

Review comment:
       No problem, I preserved the local history but I can also squash the parts you mentioned.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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