xiaoxiang781216 commented on a change in pull request #5782:
URL: https://github.com/apache/incubator-nuttx/pull/5782#discussion_r837771274



##########
File path: arch/risc-v/src/common/riscv_internal.h
##########
@@ -283,6 +285,60 @@ int riscv_pause_handler(int irq, void *c, void *arg);
 
 uintptr_t riscv_mhartid(void);
 
+#ifdef CONFIG_ARCH_USE_S_MODE
+/* If kernel runs in Supervisor mode, declare proper function prototypes,
+ * this is because it is not possible to ecall from S mode to S mode
+ */
+
+int riscv_saveusercontext(uintptr_t *saveregs);
+void riscv_fullcontextrestore(uintptr_t *restoreregs) noreturn_function;
+void riscv_switchcontext(uintptr_t **saveregs, uintptr_t *restoreregs);
+void riscv_syscall_return(void);
+void riscv_syscall_dispatch(void) noreturn_function;
+
+#else
+
+/* Context switching via system calls ***************************************/
+
+/* SYS call 0:
+ *
+ * int riscv_saveusercontext(uintptr_t *saveregs);
+ *
+ * Return:
+ * 0: Normal Return
+ * 1: Context Switch Return
+ */
+
+#define riscv_saveusercontext(saveregs) \
+  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)

Review comment:
       since ksys_callx is merged into sys_callx, all context switch related 
macros could forward to syscallx directly regardless which mode these are 
running in.

##########
File path: arch/risc-v/src/common/supervisor/riscv_vectors.S
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_vectors.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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+  .section .text
+  .balign  8
+  .global  __trap_vec
+  .global  __irq_vec
+
+/****************************************************************************
+ * Name: __trap_vec
+ *
+ * Description:
+ *   All M-mode exceptions and interrupts will be handled from here.
+ *
+ ****************************************************************************/
+
+__trap_vec:

Review comment:
       could be shared with the machine version?

##########
File path: arch/risc-v/src/Makefile
##########
@@ -24,10 +24,19 @@ ifeq ($(CONFIG_OPENSBI),y)
 include opensbi/Make.defs
 endif
 
+# Kernel runs in supervisor mode or machine mode ?
+
+ifeq ($(CONFIG_ARCH_USE_S_MODE),y)
+ARCH_CMN_MODE_DIR = supervisor

Review comment:
       change to ARCH_MODDIR like ARCH_SRCDIR

##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -179,6 +187,31 @@ int riscv_swint(int irq, void *context, void *arg)
   riscv_registerdump(regs);
 #endif
 
+  /* Handle the syscall */
+
+  regs = riscv_handle_syscall(regs);
+
+  /* Report what happened.  That might difficult in the case of a context
+   * switch
+   */
+
+#ifdef CONFIG_DEBUG_SYSCALL_INFO

Review comment:
       should we move the log at the begin and end to riscv_handle_syscall?

##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -144,7 +148,11 @@ static void dispatch_syscall(void)
      " addi sp, sp, 4\n"          /* Destroy the stack frame */
      " mv   a2, a0\n"             /* a2=Save return value in a0 */
      " li   a0, 3\n"              /* a0=SYS_syscall_return (3) */
+#ifdef CONFIG_ARCH_USE_S_MODE

Review comment:
       look like we can merge 32bit and 64bit implementation into one by always 
subtracting 16B.

##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -179,6 +187,31 @@ int riscv_swint(int irq, void *context, void *arg)
   riscv_registerdump(regs);
 #endif
 
+  /* Handle the syscall */
+
+  regs = riscv_handle_syscall(regs);
+
+  /* Report what happened.  That might difficult in the case of a context
+   * switch
+   */
+
+#ifdef CONFIG_DEBUG_SYSCALL_INFO
+  if (regs != CURRENT_REGS)
+    {
+      svcinfo("SWInt Return: Context switch!\n");
+      riscv_registerdump((const uintptr_t *)CURRENT_REGS);
+    }
+  else
+    {
+      svcinfo("SWInt Return: %d\n", regs[REG_A0]);
+    }
+#endif
+
+  return OK;
+}
+
+void *riscv_handle_syscall(uintptr_t *regs)

Review comment:
       how about change riscv_handle_syscall to riscv_do_swint?

##########
File path: arch/risc-v/src/common/supervisor/riscv_exception_common.S
##########
@@ -0,0 +1,137 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_exception_common.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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <arch/irq.h>
+
+#include "riscv_exception_macros.S"
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: exception_common
+ *
+ * Description:
+ *   Handles delegated interrupts in S-mode interrupt handler.
+ *
+ ****************************************************************************/
+
+  .section .text
+  .global exception_common
+  .align  8
+
+exception_common:

Review comment:
       what's the real difference make the machine and supervisor mode can't 
share the same code base? I suppose that the macros in mode.h can unify the 
difference.

##########
File path: arch/risc-v/src/common/supervisor/riscv_context.S
##########
@@ -0,0 +1,234 @@
+/****************************************************************************
+ * arch/risc-v/src/common/supervisor/riscv_context.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_context.S"
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <arch/mode.h>
+
+#include "riscv_exception_macros.S"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+    .globl  riscv_saveusercontext
+    .globl  riscv_fullcontextrestore
+    .globl  riscv_switchcontext
+    .globl  riscv_syscall_return
+
+/****************************************************************************
+ * Name: riscv_saveusercontext
+ *
+ * Description:
+ *   Save user context, partially destroys the caller's context
+ *
+ * C Function Prototype:
+ *   int riscv_saveusercontext(uintptr_t *saveregs);
+ *
+ * Input Parameters:
+ *   saveregs - Context to save
+ *
+ * Returned Value:
+ *   0 on context switch
+ *   1 on no context switch
+ *
+ * Assumptions:
+ *   Global interrupts disabled by the caller.
+ *
+ ****************************************************************************/
+
+.type riscv_saveusercontext, function
+
+riscv_saveusercontext:

Review comment:
       can we reuse the syscall implementation like this:
   
riscv_saveusercontext->sys_call1->riscv_syscall_dispatch->riscv_handle_syscall.
   So the machine and supervisor mode share all most code base.
   




-- 
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-unsubscr...@nuttx.apache.org

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


Reply via email to