Skip to content

Commit

Permalink
core: riscv: Ensure XSTATUS is restored before XIE
Browse files Browse the repository at this point in the history
In previous implementation, we found some accidental interrupts during
entering user mode and resuming of thread. We fixed it by clearing
XSTATUS.XIE, which is global interrupt enable bit, to ensure there are
no interrrupts.

Now we found the better solution: restore XSTATUS before restoring XIE.
This can ensure the global interrupt bit is cleared before we restore
individual interrupt bits in XIE.

Signed-off-by: Alvin Chang <alvinga@andestech.com>
  • Loading branch information
gagachang committed Sep 10, 2024
1 parent 9dc93b3 commit 0e75590
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 33 deletions.
5 changes: 2 additions & 3 deletions core/arch/riscv/kernel/thread_optee_abi_rv.S
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,8 @@ FUNC thread_std_abi_entry , :
/* Save return value */
mv s0, a0

/* Disable all interrupts */
csrc CSR_XSTATUS, CSR_XSTATUS_IE

/* Mask all maskable exceptions before switching to temporary stack */
csrw CSR_XIE, x0
/* Switch to temporary stack */
jal thread_get_tmp_sp
mv sp, a0
Expand Down
58 changes: 28 additions & 30 deletions core/arch/riscv/kernel/thread_rv.S
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,12 @@ native_interrupt_from_kernel:
/* Restore XEPC */
load_xregs sp, THREAD_CTX_REG_EPC, REG_T0
csrw CSR_XEPC, t0
/* Restore XIE */
load_xregs sp, THREAD_CTX_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Restore XSTATUS */
load_xregs sp, THREAD_CTX_REG_STATUS, REG_T0
csrw CSR_XSTATUS, t0
/* Restore XIE */
load_xregs sp, THREAD_CTX_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Set scratch as thread_core_local */
csrw CSR_XSCRATCH, tp
/* Restore all GPRs */
Expand Down Expand Up @@ -246,12 +246,12 @@ set_sp:
/* Restore XEPC */
load_xregs sp, THREAD_ABT_REG_EPC, REG_T0
csrw CSR_XEPC, t0
/* Restore XIE */
load_xregs sp, THREAD_ABT_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Restore XSTATUS */
load_xregs sp, THREAD_ABT_REG_STATUS, REG_T0
csrw CSR_XSTATUS, t0
/* Restore XIE */
load_xregs sp, THREAD_ABT_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Set scratch as thread_core_local */
csrw CSR_XSCRATCH, tp

Expand Down Expand Up @@ -385,12 +385,12 @@ native_interrupt_from_user:
/* Restore XEPC */
load_xregs sp, THREAD_CTX_REG_EPC, REG_T0
csrw CSR_XEPC, t0
/* Restore XIE */
load_xregs sp, THREAD_CTX_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Restore XSTATUS */
load_xregs sp, THREAD_CTX_REG_STATUS, REG_T0
csrw CSR_XSTATUS, t0
/* Restore XIE */
load_xregs sp, THREAD_CTX_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Set scratch as thread_core_local */
csrw CSR_XSCRATCH, tp
/* Restore all GPRs */
Expand Down Expand Up @@ -475,13 +475,15 @@ ecall_from_user:
/* Restore XEPC */
load_xregs sp, THREAD_SCALL_REG_EPC, REG_T0
csrw CSR_XEPC, t0
/* Restore XIE */
load_xregs sp, THREAD_SCALL_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Restore XSTATUS */
load_xregs sp, THREAD_SCALL_REG_STATUS, REG_T0
csrw CSR_XSTATUS, t0
/* Restore XIE */
load_xregs sp, THREAD_SCALL_REG_IE, REG_T0
csrw CSR_XIE, t0

/* Check previous privilege mode by status.SPP */
csrr t0, CSR_XSTATUS
b_if_prev_priv_is_u t0, 1f
/*
* We are going to XRET to kernel mode.
Expand Down Expand Up @@ -588,19 +590,20 @@ abort_from_user:
/* Restore XEPC */
load_xregs sp, THREAD_ABT_REG_EPC, REG_T0
csrw CSR_XEPC, t0
/* Restore XIE */
load_xregs sp, THREAD_ABT_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Restore XSTATUS */
load_xregs sp, THREAD_ABT_REG_STATUS, REG_T0
csrw CSR_XSTATUS, t0
/* Restore XIE */
load_xregs sp, THREAD_ABT_REG_IE, REG_T0
csrw CSR_XIE, t0

/* Update core local flags */
lw a0, THREAD_CORE_LOCAL_FLAGS(tp)
srli a0, a0, THREAD_CLF_SAVED_SHIFT
sw a0, THREAD_CORE_LOCAL_FLAGS(tp)

/* Check previous privilege mode by status.SPP */
csrr t0, CSR_XSTATUS
b_if_prev_priv_is_u t0, 1f
/*
* We are going to XRET to kernel mode.
Expand Down Expand Up @@ -659,9 +662,6 @@ END_FUNC thread_unwind_user_mode
* uint32_t *exit_status1);
*/
FUNC __thread_enter_user_mode , :
/* Disable kernel mode exceptions first */
csrc CSR_XSTATUS, CSR_XSTATUS_IE

/*
* Create and fill in the struct thread_user_mode_rec
*/
Expand Down Expand Up @@ -693,15 +693,15 @@ FUNC __thread_enter_user_mode , :
/* Move struct thread_ctx_regs *regs to sp to reduce code size */
mv sp, a0

/* Set user ie */
load_xregs sp, THREAD_CTX_REG_IE, REG_S0
csrw CSR_XIE, s0
/* Set user status */
load_xregs sp, THREAD_CTX_REG_STATUS, REG_S0
csrw CSR_XSTATUS, s0
/* Set exception program counter */
load_xregs sp, THREAD_CTX_REG_EPC, REG_S0
csrw CSR_XEPC, s0
/* Set user status */
load_xregs sp, THREAD_CTX_REG_STATUS, REG_S0
csrw CSR_XSTATUS, s0
/* Set user ie */
load_xregs sp, THREAD_CTX_REG_IE, REG_S0
csrw CSR_XIE, s0
/* Load the rest of the general purpose registers */
load_xregs sp, THREAD_CTX_REG_RA, REG_RA
load_xregs sp, THREAD_CTX_REG_GP, REG_GP
Expand All @@ -719,23 +719,21 @@ END_FUNC __thread_enter_user_mode

/* void thread_resume(struct thread_ctx_regs *regs) */
FUNC thread_resume , :
/* Disable global interrupts first */
csrc CSR_XSTATUS, CSR_XSTATUS_IE

/* Move struct thread_ctx_regs *regs to sp to reduce code size */
mv sp, a0

/* Restore epc */
load_xregs sp, THREAD_CTX_REG_EPC, REG_T0
csrw CSR_XEPC, t0
/* Restore ie */
load_xregs sp, THREAD_CTX_REG_IE, REG_T0
csrw CSR_XIE, t0
/* Restore status */
load_xregs sp, THREAD_CTX_REG_STATUS, REG_T0
csrw CSR_XSTATUS, t0
/* Restore ie */
load_xregs sp, THREAD_CTX_REG_IE, REG_T0
csrw CSR_XIE, t0

/* Check if previous privilege mode by status.SPP */
csrr t0, CSR_XSTATUS
b_if_prev_priv_is_u t0, 1f
/* Set scratch as zero to indicate that we are in kernel mode */
csrw CSR_XSCRATCH, zero
Expand Down

0 comments on commit 0e75590

Please sign in to comment.