You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "pussuw (via GitHub)" <gi...@apache.org> on 2023/03/10 10:06:56 UTC
[GitHub] [nuttx] pussuw opened a new pull request, #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
pussuw opened a new pull request, #8787:
URL: https://github.com/apache/nuttx/pull/8787
## Summary
Provide a stripped down, native SBI version for NuttX. This is in no way supposed to be compatible with OpenSBI specification, nor does it implement almost any of the features required by the OpenSBI specification. This is provided as-is for faster integration of new RISC-V kernel mode targets.
What does it handle then ?
- Starts the system in M mode
- Makes handover from M -> S mode NuttX
- Machine timer access
- Machine mode exceptions (just a trap, no logic here)
## Impact
None, if not taken into use. If used, RISC-V kernel mode targets can be compiled natively from the NuttX tree, with obvious limitations regarding SBI functionality.
## Testing
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] [nuttx] pussuw commented on pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#issuecomment-1463573402
I'm providing this here as it seems like there is interest in having a native SBI implementation to make kernel mode development faster for RISC-V.
This feature is open for discussion and if this is seen as something unnecessary / unwanted / too minimalistic / not good enough, I'll abandon the PR and keep this in my private repository.
Like said in the comment, I provide this as-is and shall not touch or extend / maintain its functionality, unless there is an obvious bug found during this PR which I will happily fix.
--
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] [nuttx] pussuw commented on pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#issuecomment-1463912705
Still an issue with the defconfig file :(
Will fix next monday
--
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] [nuttx] pkarashchenko commented on a diff in pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#discussion_r1220213540
##########
arch/risc-v/src/nuttsbi/sbi_start.c:
##########
@@ -0,0 +1,125 @@
+/****************************************************************************
+ * arch/risc-v/src/nuttsbi/sbi_start.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/irq.h>
+
+#include <assert.h>
+
+#include "riscv_internal.h"
+
+#include "sbi_internal.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static void __trap_vec_tmp(void) noreturn_function;
+static void __trap_vec_tmp(void)
+{
+ for (; ; )
+ {
+ asm("WFI");
+ }
+}
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+extern void __start_s(void);
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+void sbi_start(void)
+{
+ uintptr_t reg;
+ uintptr_t hartid;
+
+ /* Read hart ID */
+
+ hartid = READ_CSR(mhartid);
+
+ /* Set mscratch, mtimer */
+
+ sbi_mscratch_assign(hartid);
+ sbi_init_mtimer(MTIMER_TIME_BASE, MTIMER_CMP_BASE);
+
+ /* Setup system to enter S-mode */
+
+ reg = READ_CSR(mstatus);
+ reg &= ~MSTATUS_MPPM; /* Clear MPP */
+ reg &= ~MSTATUS_MPIE; /* Clear MPIE */
+ reg &= ~MSTATUS_TW; /* Do not trap WFI */
+ reg &= ~MSTATUS_TSR; /* Do not trap sret */
+ reg &= ~MSTATUS_TVM; /* Allow satp writes from S-mode */
+ reg |= MSTATUS_SUM; /* Allow supervisor to access user memory */
+ reg |= MSTATUS_MPPS; /* Set next context = supervisor */
+
+ /* Setup next context */
+
+ WRITE_CSR(mstatus, reg);
+
+ /* Setup a temporary S-mode interrupt vector */
+
+ WRITE_CSR(stvec, __trap_vec_tmp);
+
+ /* Delegate interrupts */
+
+ reg = (MIP_SSIP | MIP_STIP | MIP_SEIP);
+ WRITE_CSR(mideleg, reg);
+
+ /* Delegate exceptions (all of them) */
+
+ reg = ((1 << RISCV_IRQ_IAMISALIGNED) |
Review Comment:
Outer `()` are redundant
##########
arch/risc-v/src/nuttsbi/sbi_start.c:
##########
@@ -0,0 +1,125 @@
+/****************************************************************************
+ * arch/risc-v/src/nuttsbi/sbi_start.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/irq.h>
+
+#include <assert.h>
+
+#include "riscv_internal.h"
+
+#include "sbi_internal.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static void __trap_vec_tmp(void) noreturn_function;
+static void __trap_vec_tmp(void)
+{
+ for (; ; )
+ {
+ asm("WFI");
+ }
+}
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+extern void __start_s(void);
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+void sbi_start(void)
+{
+ uintptr_t reg;
+ uintptr_t hartid;
+
+ /* Read hart ID */
+
+ hartid = READ_CSR(mhartid);
+
+ /* Set mscratch, mtimer */
+
+ sbi_mscratch_assign(hartid);
+ sbi_init_mtimer(MTIMER_TIME_BASE, MTIMER_CMP_BASE);
+
+ /* Setup system to enter S-mode */
+
+ reg = READ_CSR(mstatus);
+ reg &= ~MSTATUS_MPPM; /* Clear MPP */
+ reg &= ~MSTATUS_MPIE; /* Clear MPIE */
+ reg &= ~MSTATUS_TW; /* Do not trap WFI */
+ reg &= ~MSTATUS_TSR; /* Do not trap sret */
+ reg &= ~MSTATUS_TVM; /* Allow satp writes from S-mode */
+ reg |= MSTATUS_SUM; /* Allow supervisor to access user memory */
+ reg |= MSTATUS_MPPS; /* Set next context = supervisor */
+
+ /* Setup next context */
+
+ WRITE_CSR(mstatus, reg);
+
+ /* Setup a temporary S-mode interrupt vector */
+
+ WRITE_CSR(stvec, __trap_vec_tmp);
+
+ /* Delegate interrupts */
+
+ reg = (MIP_SSIP | MIP_STIP | MIP_SEIP);
+ WRITE_CSR(mideleg, reg);
+
+ /* Delegate exceptions (all of them) */
+
+ reg = ((1 << RISCV_IRQ_IAMISALIGNED) |
+ (1 << RISCV_IRQ_INSTRUCTIONPF) |
+ (1 << RISCV_IRQ_LOADPF) |
+ (1 << RISCV_IRQ_STOREPF) |
+ (1 << RISCV_IRQ_ECALLU));
+ WRITE_CSR(medeleg, reg);
+
+ /* Enable access to all counters for S- and U-mode */
+
+ WRITE_CSR(mcounteren, UINT32_C(~0));
+ WRITE_CSR(scounteren, UINT32_C(~0));
+
+ /* Set program counter to __start_s */
+
+ WRITE_CSR(mepc, __start_s);
+
+ /* Open everything for PMP */
+
+ WRITE_CSR(pmpaddr0, -1);
+ WRITE_CSR(pmpcfg0, (PMPCFG_A_NAPOT | PMPCFG_R | PMPCFG_W | PMPCFG_X));
+
+ /* Then jump to the S-mode start function */
+
+ register long r0 asm("a0") = (long) hartid;
Review Comment:
`(long) hartid;` -> `(long)hartid;`
##########
arch/risc-v/src/nuttsbi/sbi_mexception.c:
##########
@@ -0,0 +1,37 @@
+/****************************************************************************
+ * arch/risc-v/src/nuttsbi/sbi_mexception.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdint.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+void sbi_mexception(uintptr_t mcause, uintptr_t *mepc)
+{
+ (void) mcause;
+ (void) mepc;
Review Comment:
`UNUSED`?
##########
arch/risc-v/src/nuttsbi/sbi_start.c:
##########
@@ -0,0 +1,125 @@
+/****************************************************************************
+ * arch/risc-v/src/nuttsbi/sbi_start.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/compiler.h>
+#include <nuttx/irq.h>
+
+#include <assert.h>
+
+#include "riscv_internal.h"
+
+#include "sbi_internal.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static void __trap_vec_tmp(void) noreturn_function;
+static void __trap_vec_tmp(void)
+{
+ for (; ; )
+ {
+ asm("WFI");
+ }
+}
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+extern void __start_s(void);
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+void sbi_start(void)
+{
+ uintptr_t reg;
+ uintptr_t hartid;
+
+ /* Read hart ID */
+
+ hartid = READ_CSR(mhartid);
+
+ /* Set mscratch, mtimer */
+
+ sbi_mscratch_assign(hartid);
+ sbi_init_mtimer(MTIMER_TIME_BASE, MTIMER_CMP_BASE);
+
+ /* Setup system to enter S-mode */
+
+ reg = READ_CSR(mstatus);
+ reg &= ~MSTATUS_MPPM; /* Clear MPP */
+ reg &= ~MSTATUS_MPIE; /* Clear MPIE */
+ reg &= ~MSTATUS_TW; /* Do not trap WFI */
+ reg &= ~MSTATUS_TSR; /* Do not trap sret */
+ reg &= ~MSTATUS_TVM; /* Allow satp writes from S-mode */
+ reg |= MSTATUS_SUM; /* Allow supervisor to access user memory */
+ reg |= MSTATUS_MPPS; /* Set next context = supervisor */
+
+ /* Setup next context */
+
+ WRITE_CSR(mstatus, reg);
+
+ /* Setup a temporary S-mode interrupt vector */
+
+ WRITE_CSR(stvec, __trap_vec_tmp);
+
+ /* Delegate interrupts */
+
+ reg = (MIP_SSIP | MIP_STIP | MIP_SEIP);
+ WRITE_CSR(mideleg, reg);
+
+ /* Delegate exceptions (all of them) */
+
+ reg = ((1 << RISCV_IRQ_IAMISALIGNED) |
+ (1 << RISCV_IRQ_INSTRUCTIONPF) |
+ (1 << RISCV_IRQ_LOADPF) |
+ (1 << RISCV_IRQ_STOREPF) |
+ (1 << RISCV_IRQ_ECALLU));
+ WRITE_CSR(medeleg, reg);
+
+ /* Enable access to all counters for S- and U-mode */
+
+ WRITE_CSR(mcounteren, UINT32_C(~0));
+ WRITE_CSR(scounteren, UINT32_C(~0));
+
+ /* Set program counter to __start_s */
+
+ WRITE_CSR(mepc, __start_s);
+
+ /* Open everything for PMP */
+
+ WRITE_CSR(pmpaddr0, -1);
+ WRITE_CSR(pmpcfg0, (PMPCFG_A_NAPOT | PMPCFG_R | PMPCFG_W | PMPCFG_X));
Review Comment:
no need to wrap value with `()`
--
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] [nuttx] pussuw commented on a diff in pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#discussion_r1133049239
##########
arch/risc-v/src/common/supervisor/riscv_sbi.c:
##########
@@ -87,13 +93,17 @@ static inline uintptr_t sbi_ecall(unsigned int extid, unsigned int fid,
void riscv_sbi_set_timer(uint64_t stime_value)
{
+#ifdef CONFIG_NUTTSBI
+ sbi_mcall_set_timer(stime_value);
Review Comment:
It is not trivial, I think this level of abstraction is good enough.
`sbi_mcall_set_timer` implements / uses the custom ecall interface. The reason is, I don't want to make any expectations that the NuttX native SBI will ever conform to OpenSBI specification.
I still strongly suggest using OpenSBI, but integrating it can be quite a burden. This is why I made this simple / minimal SBI, because it made (and makes) developing the kernel mode much faster.
--
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] [nuttx] xiaoxiang781216 merged pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #8787:
URL: https://github.com/apache/nuttx/pull/8787
--
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] [nuttx] pussuw commented on pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#issuecomment-1463909723
> Hi @pussuw the information you included here in the PR template is way better than information in the commit log or even that inside the Kconfig. So, I suggest you to include it in a new entry in our Documentation/, this way people will find it more easily.
Thanks, I try my best. This stuff has been in my private repo for a long time so I did not even look at the commit messages. Anyhow, I'll start incorporating motivation into my commit messages too.
In any case, if someone wants to know how this pr works, the info is at least here.
--
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] [nuttx] xiaoxiang781216 commented on a diff in pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#discussion_r1133038964
##########
arch/risc-v/src/common/supervisor/riscv_sbi.c:
##########
@@ -87,13 +93,17 @@ static inline uintptr_t sbi_ecall(unsigned int extid, unsigned int fid,
void riscv_sbi_set_timer(uint64_t stime_value)
{
+#ifdef CONFIG_NUTTSBI
+ sbi_mcall_set_timer(stime_value);
Review Comment:
can we make nuttx sbi use the same ecall interface?
--
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] [nuttx] acassis commented on pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#issuecomment-1463888888
Hi @pussuw the information you included here in the PR template is way better than information in the commit log or even that inside the Kconfig. So, I suggest you to include it in a new entry in our Documentation/, this way people will find it more easily.
--
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] [nuttx] acassis commented on pull request #8787: risc-v/kernel mode: Add a stripped down, native version of SBI for NuttX
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #8787:
URL: https://github.com/apache/nuttx/pull/8787#issuecomment-1463920309
> > Hi @pussuw the information you included here in the PR template is way better than information in the commit log or even that inside the Kconfig. So, I suggest you to include it in a new entry in our Documentation/, this way people will find it more easily.
>
> Thanks, I try my best. This stuff has been in my private repo for a long time so I did not even look at the commit messages. Anyhow, I'll start incorporating motivation into my commit messages too.
>
> In any case, if someone wants to know how this pr works, the info is at least here.
Yes, I think this is important to have a Documentation explaining about it, about motivation and why to use it instead o OpenSBI, etc.
--
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