xiaoxiang781216 commented on a change in pull request #5782:
URL: https://github.com/apache/incubator-nuttx/pull/5782#discussion_r840758665
##########
File path: arch/risc-v/src/Makefile
##########
@@ -24,6 +24,12 @@ ifeq ($(CONFIG_OPENSBI),y)
include opensbi/Make.defs
endif
+# Kernel runs in supervisor mode or machine mode ?
Review comment:
change the comment "Include supervisor specific source files"
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -69,48 +69,16 @@
exception_common:
addi sp, sp, -XCPTCONTEXT_SIZE
+ save_ctx sp
- REGSTORE x1, REG_X1(sp) /* ra */
-#ifdef RISCV_SAVE_GP
- REGSTORE x3, REG_X3(sp) /* gp */
-#endif
- REGSTORE x4, REG_X4(sp) /* tp */
- REGSTORE x5, REG_X5(sp) /* t0 */
- REGSTORE x6, REG_X6(sp) /* t1 */
- REGSTORE x7, REG_X7(sp) /* t2 */
- REGSTORE x8, REG_X8(sp) /* s0 */
- REGSTORE x9, REG_X9(sp) /* s1 */
- REGSTORE x10, REG_X10(sp) /* a0 */
- REGSTORE x11, REG_X11(sp) /* a1 */
- REGSTORE x12, REG_X12(sp) /* a2 */
- REGSTORE x13, REG_X13(sp) /* a3 */
- REGSTORE x14, REG_X14(sp) /* a4 */
- REGSTORE x15, REG_X15(sp) /* a5 */
- REGSTORE x16, REG_X16(sp) /* a6 */
- REGSTORE x17, REG_X17(sp) /* a7 */
- REGSTORE x18, REG_X18(sp) /* s2 */
- REGSTORE x19, REG_X19(sp) /* s3 */
- REGSTORE x20, REG_X20(sp) /* s4 */
- REGSTORE x21, REG_X21(sp) /* s5 */
- REGSTORE x22, REG_X22(sp) /* s6 */
- REGSTORE x23, REG_X23(sp) /* s7 */
- REGSTORE x24, REG_X24(sp) /* s8 */
- REGSTORE x25, REG_X25(sp) /* s9 */
- REGSTORE x26, REG_X26(sp) /* s10 */
- REGSTORE x27, REG_X27(sp) /* s11 */
- REGSTORE x28, REG_X28(sp) /* t3 */
- REGSTORE x29, REG_X29(sp) /* t4 */
- REGSTORE x30, REG_X30(sp) /* t5 */
- REGSTORE x31, REG_X31(sp) /* t6 */
-
- csrr s0, mstatus
- REGSTORE s0, REG_INT_CTX(sp) /* mstatus */
+ csrr s0, CSR_STATUS
+ REGSTORE s0, REG_INT_CTX(sp) /* status */
addi s0, sp, XCPTCONTEXT_SIZE
- REGSTORE s0, REG_X2(sp) /* original SP */
+ REGSTORE s0, REG_X2(sp) /* original SP */
Review comment:
move save sp into save_ctx
##########
File path: arch/risc-v/src/common/supervisor/riscv_syscall_dispatch.S
##########
@@ -0,0 +1,148 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_syscall_dispatch.S
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+.file "riscv_syscall_dispatch.S"
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <arch/mode.h>
+
+#include "riscv_exception_macros.S"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+ .globl riscv_syscall_dispatch
+
+/****************************************************************************
+ * Name: riscv_syscall_dispatch
+ *
+ * Description:
+ * Dispatch syscall from kernel
+ *
+ * C Function Prototype:
+ * void riscv_syscall_dispatch(void);
+ *
+ * Input Parameters:
+ * Assumes the context to return is already set up
+ *
+ * Returned Value:
+ * Return value of system call is returned into contex
+ *
+ * Assumptions:
+ * Task is running in privileged mode
+ *
+ ****************************************************************************/
+
+.type riscv_syscall_dispatch, function
+
+riscv_syscall_dispatch:
+
+ addi sp, sp, -XCPTCONTEXT_SIZE /* make room */
+ save_ctx sp /* save current context */
+
+ /* Mask interrupts and store the status register to context */
+
+ li s1, STATUS_IE /* move IE -> PIE */
+ csrrc s0, CSR_STATUS, s1
+ and s1, s0, s1 /* if (STATUS & IE) */
+ beqz s1, 1f
+ li s1, ~STATUS_IE /* clear IE */
+ and s0, s0, s1
+ li s1, STATUS_PIE /* set PIE */
+ or s0, s0, s1
+
+ 1:
+ /* Set previous privilege, we are in privileged mode now */
+
+ li s1, STATUS_PPP /* set previous privilege */
+ or s0, s0, s1
+ REGSTORE s0, REG_INT_CTX(sp) /* store status to context */
+
+ REGSTORE x1, REG_EPC(sp) /* save ra to epc */
+
+ addi s0, sp, XCPTCONTEXT_SIZE
+ REGSTORE s0, REG_SP(sp) /* original SP */
+
+#ifdef CONFIG_ARCH_FPU
+ mv a0, sp
+ jal x1, riscv_savefpu /* FP registers */
+#endif
+
+ mv a0, sp /* a0 = context */
+
+ /* Switch to interrupt stack */
+
+#if CONFIG_ARCH_INTERRUPTSTACK > 15
+#if IRQ_NSTACKS > 1
+ jal x1, riscv_mhartid /* get hartid */
+ li t0, (CONFIG_ARCH_INTERRUPTSTACK & ~15)
+ mul t0, a0, t0
+ la a0, g_intstacktop
+ sub sp, a0, t0
+#else
+ la sp, g_intstacktop
+#endif
+#endif
+
+ /* Run the handler */
+
+ jal x1, riscv_perform_syscall
Review comment:
let's call riscv_doirq and remove riscv_perform_syscall
##########
File path: arch/risc-v/include/syscall.h
##########
@@ -122,50 +124,17 @@
#define SYS_signal_handler_return (7)
#endif /* !CONFIG_BUILD_FLAT */
-/* sys_call macros **********************************************************/
-
-#ifndef __ASSEMBLY__
-
-/* Context switching system calls *******************************************/
-
-/* SYS call 0:
- *
- * int riscv_saveusercontext(uintptr_t *saveregs);
- *
- * Return:
- * 0: Normal Return
- * 1: Context Switch Return
- */
-
-#define riscv_saveusercontext(saveregs) \
- (int)sys_call1(SYS_save_context, (uintptr_t)(saveregs))
-
-/* SYS call 1:
- *
- * void riscv_fullcontextrestore(uintptr_t *restoreregs) noreturn_function;
- */
-
-#define riscv_fullcontextrestore(restoreregs) \
- sys_call1(SYS_restore_context, (uintptr_t)(restoreregs))
-
-/* SYS call 2:
- *
- * void riscv_switchcontext(uintptr_t **saveregs, uintptr_t *restoreregs);
- */
-
-#define riscv_switchcontext(saveregs, restoreregs) \
- sys_call2(SYS_switch_context, (uintptr_t)(saveregs),
(uintptr_t)(restoreregs))
-
-#ifdef CONFIG_BUILD_KERNEL
-/* SYS call 3:
- *
- * void riscv_syscall_return(void);
- */
-
-#define riscv_syscall_return() sys_call0(SYS_syscall_return)
-
+#if defined (CONFIG_ARCH_USE_S_MODE) && defined (__KERNEL__)
+# define ASM_SYS_CALL \
+ " addi sp, sp, -16\n" /* Make room */ \
+ REGSTORE " ra, 0(sp)\n" /* Save ra */ \
+ " jal ra, riscv_syscall_dispatch\n" /* Dispatch (modifies ra) */ \
Review comment:
how about we directly jump to riscv_syscall_dispatch by `j`? so we can
save 4 instructions and remove line 34.
##########
File path: arch/risc-v/src/common/riscv_fpu.S
##########
@@ -74,7 +75,7 @@
riscv_fpuconfig:
li a0, FS_INITIAL
- csrs mstatus, a0
+ csrs CSR_STATUS, a0
csrwi fcsr, 0
ret
Review comment:
how about change riscv_savefpu/riscv_restorefpu to macro or merge into
save_ctx and restore_ctx
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -69,48 +69,16 @@
exception_common:
addi sp, sp, -XCPTCONTEXT_SIZE
Review comment:
move the stack reserve to save_ctx?
##########
File path: arch/risc-v/src/common/riscv_exception_macros.S
##########
@@ -0,0 +1,130 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_exception_macros.S
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+.file "riscv_exception_macros.S"
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <arch/arch.h>
+#include <arch/irq.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: save_ctx
+ *
+ * Parameter:
+ * in - Pointer to where the save is performed (e.g. sp)
+ *
+ * Description:
+ * Save the common context registers (i.e. work / temp / etc).
+ *
+ ****************************************************************************/
+
+.macro save_ctx in
Review comment:
save_ctx->riscv_savectx
##########
File path: arch/risc-v/include/irq.h
##########
@@ -35,8 +35,10 @@
#include <arch/types.h>
Review comment:
remove the blank line
##########
File path: arch/risc-v/src/common/riscv_exception_macros.S
##########
@@ -0,0 +1,130 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_exception_macros.S
Review comment:
change riscv_exception_macros.S to riscv_macros.S since the macro is
used in the normal context too.
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -159,62 +132,27 @@ exception_common:
jal x1, riscv_restorefpu /* restore FPU context */
#endif
- /* If context switch is needed, return a new sp */
+ /* If context switch is needed, return a new sp */
mv sp, a0
- REGLOAD s0, REG_EPC(sp) /* restore mepc */
- csrw mepc, s0
- REGLOAD s0, REG_INT_CTX(sp) /* restore mstatus */
- csrw mstatus, s0
+ REGLOAD s0, REG_EPC(sp) /* restore sepc */
+ csrw CSR_EPC, s0
-#ifdef RISCV_SAVE_GP
- REGLOAD x3, REG_X3(sp) /* gp */
-#endif
- REGLOAD x4, REG_X4(sp) /* tp */
- REGLOAD x5, REG_X5(sp) /* t0 */
- REGLOAD x6, REG_X6(sp) /* t1 */
- REGLOAD x7, REG_X7(sp) /* t2 */
- REGLOAD x8, REG_X8(sp) /* s0 */
- REGLOAD x9, REG_X9(sp) /* s1 */
- REGLOAD x10, REG_X10(sp) /* a0 */
- REGLOAD x11, REG_X11(sp) /* a1 */
- REGLOAD x12, REG_X12(sp) /* a2 */
- REGLOAD x13, REG_X13(sp) /* a3 */
- REGLOAD x14, REG_X14(sp) /* a4 */
- REGLOAD x15, REG_X15(sp) /* a5 */
- REGLOAD x16, REG_X16(sp) /* a6 */
- REGLOAD x17, REG_X17(sp) /* a7 */
- REGLOAD x18, REG_X18(sp) /* s2 */
- REGLOAD x19, REG_X19(sp) /* s3 */
- REGLOAD x20, REG_X20(sp) /* s4 */
- REGLOAD x21, REG_X21(sp) /* s5 */
- REGLOAD x22, REG_X22(sp) /* s6 */
- REGLOAD x23, REG_X23(sp) /* s7 */
- REGLOAD x24, REG_X24(sp) /* s8 */
- REGLOAD x25, REG_X25(sp) /* s9 */
- REGLOAD x26, REG_X26(sp) /* s10 */
- REGLOAD x27, REG_X27(sp) /* s11 */
- REGLOAD x28, REG_X28(sp) /* t3 */
- REGLOAD x29, REG_X29(sp) /* t4 */
- REGLOAD x30, REG_X30(sp) /* t5 */
- REGLOAD x31, REG_X31(sp) /* t6 */
-
- REGLOAD x1, REG_X1(sp) /* ra */
-
- REGLOAD sp, REG_X2(sp) /* restore original sp */
-
- /* Return from Machine Interrupt */
-
- mret
-
-/************************************************************************************
- * Name: g_intstackalloc and g_intstacktop
-
************************************************************************************/
+ REGLOAD s0, REG_INT_CTX(sp) /* restore sstatus */
+ csrw CSR_STATUS, s0
+
+ load_ctx sp
-/************************************************************************************
+ REGLOAD sp, REG_SP(sp) /* restore original sp */
Review comment:
move sp restore into load_ctx
##########
File path: arch/risc-v/src/common/riscv_exception_macros.S
##########
@@ -0,0 +1,130 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_exception_macros.S
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+.file "riscv_exception_macros.S"
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <arch/arch.h>
+#include <arch/irq.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: save_ctx
+ *
+ * Parameter:
+ * in - Pointer to where the save is performed (e.g. sp)
+ *
+ * Description:
+ * Save the common context registers (i.e. work / temp / etc).
+ *
+ ****************************************************************************/
+
+.macro save_ctx in
+
+ REGSTORE x1, REG_X1(\in) /* ra */
+#ifdef RISCV_SAVE_GP
+ REGSTORE x3, REG_X3(\in) /* gp */
+#endif
+ REGSTORE x4, REG_X4(\in) /* tp */
+ REGSTORE x5, REG_X5(\in) /* t0 */
+ REGSTORE x6, REG_X6(\in) /* t1 */
+ REGSTORE x7, REG_X7(\in) /* t2 */
+ REGSTORE x8, REG_X8(\in) /* s0 */
+ REGSTORE x9, REG_X9(\in) /* s1 */
+ REGSTORE x10, REG_X10(\in) /* a0 */
+ REGSTORE x11, REG_X11(\in) /* a1 */
+ REGSTORE x12, REG_X12(\in) /* a2 */
+ REGSTORE x13, REG_X13(\in) /* a3 */
+ REGSTORE x14, REG_X14(\in) /* a4 */
+ REGSTORE x15, REG_X15(\in) /* a5 */
+ REGSTORE x16, REG_X16(\in) /* a6 */
+ REGSTORE x17, REG_X17(\in) /* a7 */
+ REGSTORE x18, REG_X18(\in) /* s2 */
+ REGSTORE x19, REG_X19(\in) /* s3 */
+ REGSTORE x20, REG_X20(\in) /* s4 */
+ REGSTORE x21, REG_X21(\in) /* s5 */
+ REGSTORE x22, REG_X22(\in) /* s6 */
+ REGSTORE x23, REG_X23(\in) /* s7 */
+ REGSTORE x24, REG_X24(\in) /* s8 */
+ REGSTORE x25, REG_X25(\in) /* s9 */
+ REGSTORE x26, REG_X26(\in) /* s10 */
+ REGSTORE x27, REG_X27(\in) /* s11 */
+ REGSTORE x28, REG_X28(\in) /* t3 */
+ REGSTORE x29, REG_X29(\in) /* t4 */
+ REGSTORE x30, REG_X30(\in) /* t5 */
+ REGSTORE x31, REG_X31(\in) /* t6 */
+
+.endm
+
+/****************************************************************************
+ * Name: load_ctx
+ *
+ * Parameter:
+ * out - Pointer to where the load is performed (e.g. sp)
+ *
+ * Description:
+ * Load the common context registers (i.e. work / temp / etc).
+ *
+ ****************************************************************************/
+
+.macro load_ctx out
Review comment:
riscv_loadctx
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -119,24 +87,29 @@ exception_common:
/* Setup arg0(exception cause), arg1(context) */
- csrr a0, mcause /* exception cause */
+ csrr a0, CSR_CAUSE /* exception cause */
mv a1, sp /* context = sp */
#if CONFIG_ARCH_INTERRUPTSTACK > 15
- /* Load mhartid (cpuid) */
- csrr s0, mhartid
+ /* Offset to hartid */
+
+ mv s0, a0 /* save cause */
+ jal x1, riscv_mhartid /* get hartid */
/* Switch to interrupt stack */
+
#if IRQ_NSTACKS > 1
li t0, (CONFIG_ARCH_INTERRUPTSTACK & ~15)
- mul t0, s0, t0
- la s0, g_intstacktop
- sub sp, s0, t0
+ mul t0, a0, t0
+ la a0, g_intstacktop
+ sub sp, a0, t0
#else
la sp, g_intstacktop
#endif
+ mv a0, s0 /* restore cause */
Review comment:
remove
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -119,24 +87,29 @@ exception_common:
/* Setup arg0(exception cause), arg1(context) */
- csrr a0, mcause /* exception cause */
+ csrr a0, CSR_CAUSE /* exception cause */
mv a1, sp /* context = sp */
#if CONFIG_ARCH_INTERRUPTSTACK > 15
- /* Load mhartid (cpuid) */
- csrr s0, mhartid
+ /* Offset to hartid */
+
+ mv s0, a0 /* save cause */
Review comment:
remove
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -119,24 +87,29 @@ exception_common:
/* Setup arg0(exception cause), arg1(context) */
- csrr a0, mcause /* exception cause */
+ csrr a0, CSR_CAUSE /* exception cause */
mv a1, sp /* context = sp */
#if CONFIG_ARCH_INTERRUPTSTACK > 15
- /* Load mhartid (cpuid) */
- csrr s0, mhartid
+ /* Offset to hartid */
+
+ mv s0, a0 /* save cause */
+ jal x1, riscv_mhartid /* get hartid */
Review comment:
move after line 102
##########
File path: arch/risc-v/src/common/supervisor/riscv_syscall_dispatch.S
##########
@@ -0,0 +1,148 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_syscall_dispatch.S
Review comment:
change to riscv_syscall_dispatch to riscv_dispatch_syscall like
riscv_dispatch_irq
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -119,24 +87,29 @@ exception_common:
/* Setup arg0(exception cause), arg1(context) */
- csrr a0, mcause /* exception cause */
+ csrr a0, CSR_CAUSE /* exception cause */
Review comment:
move line 88-91 after line 110 and remove line 111
##########
File path: arch/risc-v/src/common/supervisor/Make.defs
##########
@@ -0,0 +1,28 @@
+############################################################################
+# arch/risc-v/src/common/supervisor/Make.defs
+#
+# 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.
+#
+############################################################################
+
+# If the NuttX kernel runs in S-mode
+
+CMN_ASRCS += riscv_syscall_dispatch.S
+CMN_CSRCS += riscv_sbi.c riscv_perform_syscall.c
+CMN_CSRCS += riscv_percpu.c
Review comment:
move riscv_sbi.c and riscv_percpu.c to the next patch
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
