riscv/kernel_stack: Use kernel stack to store the user context

If a kernel stack exists, use that whenever the user process is in
privileged mode, i.e. running an exception or in system call. Previously
the exception context was stored into the user's stack, which is not ideal.

Why?

1. Because the exception entry status (REG_INT_CTX) is needed by the
   kernel, and this is now in user memory which requires that the correct
   user mappings are active when it is accessed.

2. The user must currently account for the exception stack frame (which
   is BIG) in its own stack allocation. Moving the exception context save
   to the kernel stack offloads this responsibility from the user to the
   kernel, which is IMO the correct behavior.

3. The kernel access to user memory is currently allowed without condition,
   however this is not ideal either. The privileged mode status CSR allows
   blocking access to user memory via the STATUS_SUM-bit, which should be
   disabled by default and only enabled when access to user space is really
   needed. This patch allows implementing such features.
This commit is contained in:
Ville Juven 2023-05-17 10:31:11 +03:00 committed by archer
parent a636edcbe4
commit 7bc8e59cce
9 changed files with 200 additions and 43 deletions

View File

@ -557,6 +557,7 @@ struct xcptcontext
uintptr_t *ustkptr; /* Saved user stack pointer */
uintptr_t *kstack; /* Allocate base of the (aligned) kernel stack */
uintptr_t *ktopstk; /* Top of kernel stack */
uintptr_t *kstkptr; /* Saved kernel stack pointer */
#endif
#endif

View File

@ -97,7 +97,7 @@ CMN_CSRCS += riscv_mmu.c
endif
ifeq ($(CONFIG_ARCH_KERNEL_STACK),y)
CMN_CSRCS += riscv_addrenv_kstack.c
CMN_CSRCS += riscv_addrenv_kstack.c riscv_ksp.c
endif
ifeq ($(CONFIG_ARCH_ADDRENV),y)

View File

@ -32,24 +32,22 @@
#include "chip.h"
#include "riscv_percpu.h"
#include "riscv_macros.S"
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
/* Using address environments currently require that a common interrupt stack
* is in place. This is needed because during context switch the procedure
* that swaps the active address environment is dependent on a stack, which
* must be a 'neutral' stack.
*
* Another option would be to use a per-process kernel stack, but full
* support for this is not yet in place, so use the common IRQ stack instead.
/* Using address environments requires that the per-process kernel stack is
* enabled. Using user stack to run exception and/or kernel code is a very
* very bad idea, thus enforce the kernel stack
*/
#ifdef CONFIG_ARCH_ADDRENV
# if CONFIG_ARCH_INTERRUPTSTACK == 0 && !defined(CONFIG_ARCH_KERNEL_STACK)
# error "IRQ or kernel stack is needed for swapping address environments"
# ifndef CONFIG_ARCH_KERNEL_STACK
# error "Kernel stack is needed for handling exceptions"
# endif
#endif
@ -78,13 +76,38 @@
exception_common:
#ifdef CONFIG_ARCH_KERNEL_STACK
/* Take the kernel stack into use */
csrrw a0, CSR_SCRATCH, a0
REGSTORE sp, RISCV_PERCPU_USP(a0)
REGLOAD sp, RISCV_PERCPU_KSP(a0)
REGSTORE x0, RISCV_PERCPU_KSP(a0)
bnez sp, 1f
/* No kernel stack, exception comes from kernel */
REGLOAD sp, RISCV_PERCPU_USP(a0)
1:
/* Restore the per-cpu structure */
csrrw a0, CSR_SCRATCH, a0
#endif
addi sp, sp, -XCPTCONTEXT_SIZE
save_ctx sp
csrr s0, CSR_STATUS
REGSTORE s0, REG_INT_CTX(sp) /* status */
#ifdef CONFIG_ARCH_KERNEL_STACK
csrr s0, CSR_SCRATCH
REGLOAD s0, RISCV_PERCPU_USP(s0)
#else
addi s0, sp, XCPTCONTEXT_SIZE
#endif
REGSTORE s0, REG_X2(sp) /* original SP */
csrr s0, CSR_EPC
@ -133,6 +156,22 @@ exception_common:
REGLOAD s0, REG_INT_CTX(sp) /* restore sstatus */
csrw CSR_STATUS, s0
#ifdef CONFIG_ARCH_KERNEL_STACK
/* Returning to userspace ? */
li s1, STATUS_PPP
and s0, s0, s1
bnez s0, 1f
/* Set the next task's kernel stack to the scratch area */
jal x1, riscv_current_ksp
csrr s0, CSR_SCRATCH
REGSTORE a0, RISCV_PERCPU_KSP(s0)
1:
#endif
load_ctx sp
REGLOAD sp, REG_SP(sp) /* restore original sp */

View File

@ -32,6 +32,7 @@
#include <nuttx/tls.h>
#include <arch/irq.h>
#include "addrenv.h"
#include "riscv_internal.h"
/****************************************************************************
@ -56,6 +57,7 @@ void up_initial_state(struct tcb_s *tcb)
{
struct xcptcontext *xcp = &tcb->xcp;
uintptr_t regval;
uintptr_t topstack;
#ifdef CONFIG_ARCH_KERNEL_STACK
uintptr_t *kstack = xcp->kstack;
#endif
@ -88,13 +90,22 @@ void up_initial_state(struct tcb_s *tcb)
return;
}
topstack = (uintptr_t)tcb->stack_base_ptr + tcb->adj_stack_size;
#ifdef CONFIG_ARCH_KERNEL_STACK
xcp->kstack = kstack;
/* Use the process kernel stack to store context for user processes */
if (kstack)
{
xcp->kstack = kstack;
xcp->ustkptr = (uintptr_t *)topstack;
topstack = (uintptr_t)kstack + ARCH_KERNEL_STACKSIZE;
xcp->ktopstk = (uintptr_t *)topstack;
xcp->kstkptr = xcp->ktopstk;
}
#endif
xcp->regs = (uintptr_t *)(
(uintptr_t)tcb->stack_base_ptr + tcb->adj_stack_size - XCPTCONTEXT_SIZE);
xcp->regs = (uintptr_t *)(topstack - XCPTCONTEXT_SIZE);
memset(xcp->regs, 0, XCPTCONTEXT_SIZE);
/* Save the initial stack pointer. Hmmm.. the stack is set to the very
@ -103,8 +114,7 @@ void up_initial_state(struct tcb_s *tcb)
* only the start function would do that and we have control over that one
*/
xcp->regs[REG_SP] = (uintptr_t)tcb->stack_base_ptr +
tcb->adj_stack_size;
xcp->regs[REG_SP] = topstack;
/* Save the task entry point */

View File

@ -0,0 +1,36 @@
/****************************************************************************
* arch/risc-v/src/common/riscv_ksp.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 <stdint.h>
#include <nuttx/sched.h>
/****************************************************************************
* Public Functions
****************************************************************************/
uintptr_t riscv_current_ksp(void)
{
return (uintptr_t)nxsched_self()->xcp.kstkptr;
}

View File

@ -192,3 +192,35 @@ uintptr_t riscv_percpu_get_irqstack(void)
return ((riscv_percpu_t *)scratch)->irq_stack;
}
/****************************************************************************
* Name: riscv_percpu_set_kstack
*
* Description:
* Set current kernel stack, so it can be taken quickly into use when a
* trap is taken. Do this whenever a new process moves to running state.
*
* Input Parameters:
* ksp - Pointer to the kernel stack
*
* Returned Value:
* None
*
****************************************************************************/
void riscv_percpu_set_kstack(uintptr_t ksp)
{
irqstate_t flags;
uintptr_t scratch;
/* This must be done with interrupts disabled */
flags = enter_critical_section();
scratch = READ_CSR(CSR_SCRATCH);
DEBUGASSERT(scratch >= (uintptr_t) &g_percpu &&
scratch < (uintptr_t) &g_percpu + sizeof(g_percpu));
((riscv_percpu_t *)scratch)->ksp = ksp;
leave_critical_section(flags);
}

View File

@ -50,6 +50,8 @@
#define RISCV_PERCPU_LIST (0 * INT_REG_SIZE)
#define RISCV_PERCPU_HARTID (1 * INT_REG_SIZE)
#define RISCV_PERCPU_IRQSTACK (2 * INT_REG_SIZE)
#define RISCV_PERCPU_USP (3 * INT_REG_SIZE)
#define RISCV_PERCPU_KSP (4 * INT_REG_SIZE)
#ifndef __ASSEMBLY__
@ -68,6 +70,8 @@ struct riscv_percpu_s
struct riscv_percpu_s *next; /* For sl list linkage */
uintptr_t hartid; /* Hart ID */
uintptr_t irq_stack; /* Interrupt stack */
uintptr_t usp; /* Area to store user sp */
uintptr_t ksp; /* Area to load kernel sp */
};
typedef struct riscv_percpu_s riscv_percpu_t;
@ -116,5 +120,22 @@ uintptr_t riscv_percpu_get_hartid(void);
uintptr_t riscv_percpu_get_irqstack(void);
/****************************************************************************
* Name: riscv_percpu_set_kstack
*
* Description:
* Set current kernel stack, so it can be taken quickly into use when a
* trap is taken.
*
* Input Parameters:
* ksp - Pointer to the kernel stack
*
* Returned Value:
* None
*
****************************************************************************/
void riscv_percpu_set_kstack(uintptr_t ksp);
#endif /* __ASSEMBLY__ */
#endif /* __ARCH_RISC_V_SRC_COMMON_RISCV_PERCPU_H */

View File

@ -264,6 +264,14 @@ int riscv_swint(int irq, void *context, void *arg)
* unprivileged mode.
*/
#ifdef CONFIG_ARCH_KERNEL_STACK
/* Set the user stack pointer as we are about to return to user */
struct tcb_s *tcb = nxsched_self();
regs[REG_SP] = (uintptr_t)tcb->xcp.ustkptr;
tcb->xcp.ustkptr = NULL;
#endif
#if defined (CONFIG_BUILD_PROTECTED)
/* Use the nxtask_startup trampoline function */
@ -302,6 +310,14 @@ int riscv_swint(int irq, void *context, void *arg)
* unprivileged mode.
*/
#ifdef CONFIG_ARCH_KERNEL_STACK
/* Set the user stack pointer as we are about to return to user */
struct tcb_s *tcb = nxsched_self();
regs[REG_SP] = (uintptr_t)tcb->xcp.ustkptr;
tcb->xcp.ustkptr = NULL;
#endif
regs[REG_EPC] = (uintptr_t)regs[REG_A1]; /* startup */
/* Change the parameter ordering to match the expectation of the
@ -371,18 +387,13 @@ int riscv_swint(int irq, void *context, void *arg)
{
uintptr_t usp;
DEBUGASSERT(rtcb->xcp.kstkptr == NULL);
/* Store the current kernel stack pointer so it is not lost */
rtcb->xcp.kstkptr = (uintptr_t *)regs[REG_SP];
/* Copy "info" into user stack */
if (rtcb->xcp.sigdeliver)
{
usp = rtcb->xcp.saved_regs[REG_SP];
}
else
{
usp = rtcb->xcp.regs[REG_SP];
}
usp = rtcb->xcp.saved_regs[REG_SP];
/* Create a frame for info and copy the kernel info */
@ -391,9 +402,8 @@ int riscv_swint(int irq, void *context, void *arg)
/* Now set the updated SP and user copy of "info" to A2 */
rtcb->xcp.kstkptr = (uintptr_t *)regs[REG_SP];
regs[REG_SP] = usp;
regs[REG_A2] = usp;
regs[REG_SP] = usp;
regs[REG_A2] = usp;
}
#endif
}
@ -423,9 +433,8 @@ int riscv_swint(int irq, void *context, void *arg)
rtcb->xcp.sigreturn = 0;
#ifdef CONFIG_ARCH_KERNEL_STACK
/* We must enter here be using the user stack. We need to switch
* to back to the kernel user stack before returning to the kernel
* mode signal trampoline.
/* We must restore the original kernel stack pointer before
* returning to the kernel mode signal trampoline.
*/
if (rtcb->xcp.kstack != NULL)
@ -433,7 +442,7 @@ int riscv_swint(int irq, void *context, void *arg)
DEBUGASSERT(rtcb->xcp.kstkptr != NULL);
regs[REG_SP] = (uintptr_t)rtcb->xcp.kstkptr;
rtcb->xcp.kstkptr = NULL;
rtcb->xcp.kstkptr = rtcb->xcp.ktopstk;
}
#endif
}
@ -488,22 +497,15 @@ int riscv_swint(int irq, void *context, void *arg)
#endif
#ifdef CONFIG_ARCH_KERNEL_STACK
/* If this is the first SYSCALL and if there is an allocated
* kernel stack, then switch to the kernel stack.
/* If this is the first level system call, we must store the user
* stack pointer so it doesn't get lost.
*/
if (index == 0 && rtcb->xcp.kstack != NULL)
{
DEBUGASSERT(rtcb->xcp.ustkptr == NULL);
rtcb->xcp.ustkptr = (uintptr_t *)regs[REG_SP];
if (rtcb->xcp.kstkptr != NULL)
{
regs[REG_SP] = (uintptr_t)rtcb->xcp.kstkptr;
}
else
{
regs[REG_SP] = (uintptr_t)rtcb->xcp.kstack +
ARCH_KERNEL_STACKSIZE;
}
regs[REG_SP] = (uintptr_t)regs;
}
#endif
}

View File

@ -146,6 +146,22 @@ sys_call6:
1:
csrw CSR_STATUS, s0
#ifdef CONFIG_ARCH_KERNEL_STACK
/* Returning to userspace ? */
li s1, STATUS_PPP
and s0, s0, s1
bnez s0, 1f
/* Set the next task's kernel stack to the scratch area */
jal x1, riscv_current_ksp
csrr s0, CSR_SCRATCH
REGSTORE a0, RISCV_PERCPU_KSP(s0)
1:
#endif
load_ctx sp
REGLOAD sp, REG_SP(sp) /* restore original sp */