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/04/06 12:13:24 UTC

[GitHub] [incubator-nuttx] pussuw opened a new pull request, #5993: MPFS: Prepare support for S-mode

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

   - Access to PLIC via S-mode registers
   - Access to IRQs via S-mode registers / definitions
   - Initialize S-mode registers upon boot
   - Initialize per CPU area before nx_start
   
   NOTE: S-mode requires a companion SW (SBI) which is not yet implemented,
   thus S-mode is not usable as is, yet.NOTE: S
   
   ## Summary
   Prepares MPFS for running NuttX in S-mode
   ## Impact
   MPFS only
   ## Testing
   MPFS icicle:knsh
   


-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844219676


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   should we just call this function only once.



##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   should we just call this function only once?



-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844262947


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   The check is correct, hart0 for us at least runs the bootloader. Also, hart0 does not have supervisor mode at all, or FPU, this is why we check for hart0 in many places.
   
   Need to somehow decide which hart 1-4 does the boot, and use that number here. But that comes when / if SMP support is added.



-- 
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 #5993: MPFS: Prepare support for S-mode

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

   > @xiaoxiang781216 the S-mode register initializations were missing, mpfs_head.S file changes are new since your approval. Can you please re-check ?
   
   Let's finish riscv_percpu_init first. Other change in this patch looks good.


-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844288701


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   Initializing the module is still necessary for S-mode even when using 1 hart, because the memory for the percpu elements lives there. The percpu structure is used to initialize sscratch which contains hartid (which cannot be read in S-mode by `csrr reg, mhartid`), as well as the IRQ stack (only 1 at the moment, but it is ready for SMP too).
   
   This works and is absolutely necessary when #ifdef CONFIG_ARCH_USE_S_MODE is true.



-- 
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 #5993: MPFS: Prepare support for S-mode

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


-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844284825


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   If so, let's remove riscv_percpu_init which is unnecessary but complicate the code.



-- 
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 #5993: MPFS: Prepare support for S-mode

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

   @xiaoxiang781216 the S-mode register initializations were missing, mpfs_head.S file changes are new since your approval. Can you please re-check ?


-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844633684


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   Let's talk riscv_percpu_init in #5985



-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844258266


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   If so, the check should be if (mhartid == 0) to avoid the double initialization



-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844288701


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   Initializing the module is still necessary for S-mode even when using 1 hart, because the memory for the percpu elements lives there. The percpu structure is used to initialize sscratch which contains hartid (which cannot be read in S-mode), as well as the IRQ stack (only 1 at the moment, but it is ready for SMP too).
   
   This works and is absolutely necessary when #ifdef CONFIG_ARCH_USE_S_MODE is true.



-- 
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 diff in pull request #5993: MPFS: Prepare support for S-mode

Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #5993:
URL: https://github.com/apache/incubator-nuttx/pull/5993#discussion_r844226143


##########
arch/risc-v/src/mpfs/mpfs_start.c:
##########
@@ -169,6 +175,16 @@ void __mpfs_start(uint64_t mhartid)
 
   mpfs_boardinitialize();
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+  /* Initialize the per CPU areas */
+
+  if (mhartid != 0)
+    {
+      riscv_percpu_init();

Review Comment:
   Yes, only the boot hart should do this, but there is no logic for selecting one yet. Need to fix this later if/when SMP is supported.



-- 
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 #5993: MPFS: Prepare support for S-mode

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

   Call to percpu init 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.

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

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