[PATCH v2 04/10] lib: sbi: Introduce trap context

Anup Patel anup at brainfault.org
Sun Mar 17 04:23:31 PDT 2024


On Wed, Mar 13, 2024 at 10:28 AM Xiang W <wxjstz at 126.com> wrote:
>
> 在 2024-03-12星期二的 15:57 +0530,Anup Patel写道:
> > Club the struct sbi_trap_regs and struct sbi_trap_info a new
> > struct sbi_trap_context (aka trap context) which must be saved
> > by low-level trap handler before calling sbi_trap_handler().
> >
> > To track nested traps, the struct sbi_scratch points to the current
> > trap context and the trap context has pointer to pervious context
> > of previous trap.
> >
> > Signed-off-by: Anup Patel <apatel at ventanamicro.com>
> > ---
> >  firmware/fw_base.S     | 62 ++++++++++++++++++++++++++++++++++--------
> >  include/sbi/sbi_trap.h | 29 +++++++++++++++++++-
> >  lib/sbi/sbi_trap.c     | 58 +++++++++++++++++----------------------
> >  3 files changed, 103 insertions(+), 46 deletions(-)
> >
> > diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> > index 539fd1d..967cca5 100644
> > --- a/firmware/fw_base.S
> > +++ b/firmware/fw_base.S
> > @@ -446,14 +446,12 @@ _start_warm:
> >
> >       /* Setup trap handler */
> >       lla     a4, _trap_handler
> > -#if __riscv_xlen == 32
> >       csrr    a5, CSR_MISA
> >       srli    a5, a5, ('H' - 'A')
> >       andi    a5, a5, 0x1
> > -     beq     a5, zero, _skip_trap_handler_rv32_hyp
> > -     lla     a4, _trap_handler_rv32_hyp
> > -_skip_trap_handler_rv32_hyp:
> > -#endif
> > +     beq     a5, zero, _skip_trap_handler_hyp
> > +     lla     a4, _trap_handler_hyp
> > +_skip_trap_handler_hyp:
> >       csrw    CSR_MTVEC, a4
> >
> >       /* Initialize SBI runtime */
> > @@ -564,10 +562,10 @@ memcmp:
> >       xor     t0, tp, t0
> >
> >       /* Save original SP on exception stack */
> > -     REG_S   sp, (SBI_TRAP_REGS_OFFSET(sp) - SBI_TRAP_REGS_SIZE)(t0)
> > +     REG_S   sp, (SBI_TRAP_REGS_OFFSET(sp) - SBI_TRAP_CONTEXT_SIZE)(t0)
> >
> > -     /* Set SP to exception stack and make room for trap registers */
> > -     add     sp, t0, -(SBI_TRAP_REGS_SIZE)
> > +     /* Set SP to exception stack and make room for trap context */
> > +     add     sp, t0, -(SBI_TRAP_CONTEXT_SIZE)
> >
> >       /* Restore T0 from scratch space */
> >       REG_L   t0, SBI_SCRATCH_TMP0_OFFSET(tp)
> > @@ -627,6 +625,32 @@ memcmp:
> >       REG_S   t6, SBI_TRAP_REGS_OFFSET(t6)(sp)
> >  .endm
> >
> > +.macro       TRAP_SAVE_INFO have_mstatush have_h_extension
> > +     csrr    t0, CSR_MCAUSE
> > +     REG_S   t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(cause))(sp)
> > +     csrr    t0, CSR_MTVAL
> > +     REG_S   t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval))(sp)
> > +.if \have_h_extension
> > +     csrr    t0, CSR_MTVAL2
> > +     REG_S   t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval2))(sp)
> > +     csrr    t0, CSR_MTINST
> > +     REG_S   t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tinst))(sp)
> > +     .if \have_mstatush
> > +     csrr    t0, CSR_MSTATUSH
> > +     srli    t0, t0, MSTATUSH_GVA_SHIFT
> > +     .else
> > +     csrr    t0, CSR_MSTATUS
> > +     srli    t0, t0, MSTATUS_GVA_SHIFT
> > +     .endif
> > +     and     t0, t0, 0x1
> > +.else
> > +     REG_S   zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval2))(sp)
> > +     REG_S   zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tinst))(sp)
> > +     add     t0, zero, zero
> > +.endif
> > +     REG_S   t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(gva))(sp)
> > +.endm
> > +
> >  .macro       TRAP_CALL_C_ROUTINE
> >       /* Call C routine */
> >       add     a0, sp, zero
> > @@ -696,6 +720,8 @@ _trap_handler:
> >
> >       TRAP_SAVE_GENERAL_REGS_EXCEPT_SP_T0
> >
> > +     TRAP_SAVE_INFO 0 0
> > +
> >       TRAP_CALL_C_ROUTINE
> >
> >       TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
> > @@ -706,27 +732,39 @@ _trap_handler:
> >
> >       mret
> >
> > -#if __riscv_xlen == 32
> >       .section .entry, "ax", %progbits
> >       .align 3
> > -     .globl _trap_handler_rv32_hyp
> > -_trap_handler_rv32_hyp:
> > +     .globl _trap_handler_hyp
> > +_trap_handler_hyp:
> >       TRAP_SAVE_AND_SETUP_SP_T0
> >
> > +#if __riscv_xlen == 32
> >       TRAP_SAVE_MEPC_MSTATUS 1
> > +#else
> > +     TRAP_SAVE_MEPC_MSTATUS 0
> > +#endif
> >
> >       TRAP_SAVE_GENERAL_REGS_EXCEPT_SP_T0
> >
> > +#if __riscv_xlen == 32
> > +     TRAP_SAVE_INFO 1 1
> > +#else
> > +     TRAP_SAVE_INFO 0 1
> > +#endif
> > +
> >       TRAP_CALL_C_ROUTINE
> >
> >       TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
> >
> > +#if __riscv_xlen == 32
> >       TRAP_RESTORE_MEPC_MSTATUS 1
> > +#else
> > +     TRAP_RESTORE_MEPC_MSTATUS 0
> > +#endif
> >
> >       TRAP_RESTORE_A0_T0
> >
> >       mret
> > -#endif
> >
> >       .section .entry, "ax", %progbits
> >       .align 3
> > diff --git a/include/sbi/sbi_trap.h b/include/sbi/sbi_trap.h
> > index a6032ab..f7c170e 100644
> > --- a/include/sbi/sbi_trap.h
> > +++ b/include/sbi/sbi_trap.h
> > @@ -112,9 +112,15 @@
> >  /** Size (in bytes) of sbi_trap_info */
> >  #define SBI_TRAP_INFO_SIZE SBI_TRAP_INFO_OFFSET(last)
> >
> > +/** Size (in bytes) of sbi_trap_context */
> > +#define SBI_TRAP_CONTEXT_SIZE (SBI_TRAP_REGS_SIZE + \
> > +                            SBI_TRAP_INFO_SIZE + \
> > +                            __SIZEOF_POINTER__)
> > +
> >  #ifndef __ASSEMBLER__
> >
> >  #include <sbi/sbi_types.h>
> > +#include <sbi/sbi_scratch.h>
> >
> >  /** Representation of register state at time of trap/interrupt */
> >  struct sbi_trap_regs {
> > @@ -204,6 +210,16 @@ struct sbi_trap_info {
> >       unsigned long gva;
> >  };
> >
> > +/** Representation of trap context saved on stack */
> > +struct sbi_trap_context {
> > +     /** Register state */
> > +     struct sbi_trap_regs regs;
> > +     /** Trap details */
> > +     struct sbi_trap_info trap;
> > +     /** Pointer to previous trap context */
> > +     struct sbi_trap_context *prev_context;
> > +};
> > +
> >  static inline unsigned long sbi_regs_gva(const struct sbi_trap_regs *regs)
> >  {
> >       /*
> > @@ -223,7 +239,18 @@ static inline unsigned long sbi_regs_gva(const struct sbi_trap_regs *regs)
> >  int sbi_trap_redirect(struct sbi_trap_regs *regs,
> >                     const struct sbi_trap_info *trap);
> >
> > -struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs);
> > +static inline struct sbi_trap_context *sbi_trap_get_context(struct sbi_scratch *scratch)
> > +{
> > +     return (scratch) ? (void *)scratch->trap_context : NULL;
> > +}
> > +
> > +static inline void sbi_trap_set_context(struct sbi_scratch *scratch,
> > +                                     struct sbi_trap_context *tcntx)
> > +{
> > +     scratch->trap_context = (unsigned long)tcntx;
> > +}
> > +
> > +struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx);
> >
> >  #endif
> >
> > diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
> > index b059c4b..4dbda06 100644
> > --- a/lib/sbi/sbi_trap.c
> > +++ b/lib/sbi/sbi_trap.c
> > @@ -24,18 +24,18 @@
> >  #include <sbi/sbi_trap.h>
> >
> >  static void __noreturn sbi_trap_error(const char *msg, int rc,
> > -                                   ulong mcause, ulong mtval, ulong mtval2,
> > -                                   ulong mtinst, struct sbi_trap_regs *regs)
> > +                                   const struct sbi_trap_info *trap,
> > +                                   const struct sbi_trap_regs *regs)
> >  {
> >       u32 hartid = current_hartid();
> >
> >       sbi_printf("%s: hart%d: %s (error %d)\n", __func__, hartid, msg, rc);
> >       sbi_printf("%s: hart%d: mcause=0x%" PRILX " mtval=0x%" PRILX "\n",
> > -                __func__, hartid, mcause, mtval);
> > +                __func__, hartid, trap->cause, trap->tval);
> >       if (misa_extension('H')) {
> >               sbi_printf("%s: hart%d: mtval2=0x%" PRILX
> >                          " mtinst=0x%" PRILX "\n",
> > -                        __func__, hartid, mtval2, mtinst);
> > +                        __func__, hartid, trap->tval2, trap->tinst);
> >       }
> >       sbi_printf("%s: hart%d: mepc=0x%" PRILX " mstatus=0x%" PRILX "\n",
> >                  __func__, hartid, regs->mepc, regs->mstatus);
> > @@ -258,20 +258,20 @@ static int sbi_trap_aia_irq(struct sbi_trap_regs *regs, ulong mcause)
> >   * 6. Stack pointer (SP) is setup for current HART
> >   * 7. Interrupts are disabled in MSTATUS CSR
> >   *
> > - * @param regs pointer to register state
> > + * @param tcntx pointer to trap context
> >   */
> > -struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
> > +struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
> >  {
> >       int rc = SBI_ENOTSUPP;
> >       const char *msg = "trap handler failed";
> > -     ulong mcause = csr_read(CSR_MCAUSE);
> > -     ulong mtval = csr_read(CSR_MTVAL), mtval2 = 0, mtinst = 0;
> > -     struct sbi_trap_info trap;
> > +     struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> > +     const struct sbi_trap_info *trap = &tcntx->trap;
> > +     struct sbi_trap_regs *regs = &tcntx->regs;
> > +     ulong mcause = tcntx->trap.cause;
> >
> > -     if (misa_extension('H')) {
> > -             mtval2 = csr_read(CSR_MTVAL2);
> > -             mtinst = csr_read(CSR_MTINST);
> > -     }
> > +     /* Update trap context pointer */
> > +     tcntx->prev_context = sbi_trap_get_context(scratch);
> If the exception comes from S mode, tcntx will point to the same memory address.
> At this time prev_context will be the same as tcntx and needs to be avoided.

When we come from S/U-mode, the trap context pointer on scratch is NULL
hence prev_context will be NULL as well.

Also, when we go back to S/U-mode, the trap context pointer on scratch is
set to prev_context again (i.e. NULL).

Regards,
Anup

>
> Regards,
> Xiang W
> > +     sbi_trap_set_context(scratch, tcntx);
> >
> >       if (mcause & (1UL << (__riscv_xlen - 1))) {
> >               if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
> > @@ -279,32 +279,23 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
> >                       rc = sbi_trap_aia_irq(regs, mcause);
> >               else
> >                       rc = sbi_trap_nonaia_irq(regs, mcause);
> > -             if (rc) {
> > -                     msg = "unhandled local interrupt";
> > -                     goto trap_error;
> > -             }
> > -             return regs;
> > +             msg = "unhandled local interrupt";
> > +             goto trap_done;
> >       }
> > -     /* Original trap_info */
> > -     trap.cause = mcause;
> > -     trap.tval  = mtval;
> > -     trap.tval2 = mtval2;
> > -     trap.tinst = mtinst;
> > -     trap.gva   = sbi_regs_gva(regs);
> >
> >       switch (mcause) {
> >       case CAUSE_ILLEGAL_INSTRUCTION:
> > -             rc  = sbi_illegal_insn_handler(mtval, regs);
> > +             rc  = sbi_illegal_insn_handler(tcntx->trap.tval, regs);
> >               msg = "illegal instruction handler failed";
> >               break;
> >       case CAUSE_MISALIGNED_LOAD:
> >               sbi_pmu_ctr_incr_fw(SBI_PMU_FW_MISALIGNED_LOAD);
> > -             rc  = sbi_misaligned_load_handler(regs, &trap);
> > +             rc  = sbi_misaligned_load_handler(regs, trap);
> >               msg = "misaligned load handler failed";
> >               break;
> >       case CAUSE_MISALIGNED_STORE:
> >               sbi_pmu_ctr_incr_fw(SBI_PMU_FW_MISALIGNED_STORE);
> > -             rc  = sbi_misaligned_store_handler(regs, &trap);
> > +             rc  = sbi_misaligned_store_handler(regs, trap);
> >               msg = "misaligned store handler failed";
> >               break;
> >       case CAUSE_SUPERVISOR_ECALL:
> > @@ -314,23 +305,24 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
> >               break;
> >       case CAUSE_LOAD_ACCESS:
> >               sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_LOAD);
> > -             rc  = sbi_load_access_handler(regs, &trap);
> > +             rc  = sbi_load_access_handler(regs, trap);
> >               msg = "load fault handler failed";
> >               break;
> >       case CAUSE_STORE_ACCESS:
> >               sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_STORE);
> > -             rc  = sbi_store_access_handler(regs, &trap);
> > +             rc  = sbi_store_access_handler(regs, trap);
> >               msg = "store fault handler failed";
> >               break;
> >       default:
> >               /* If the trap came from S or U mode, redirect it there */
> >               msg = "trap redirect failed";
> > -             rc  = sbi_trap_redirect(regs, &trap);
> > +             rc  = sbi_trap_redirect(regs, trap);
> >               break;
> >       }
> >
> > -trap_error:
> > +trap_done:
> >       if (rc)
> > -             sbi_trap_error(msg, rc, mcause, mtval, mtval2, mtinst, regs);
> > -     return regs;
> > +             sbi_trap_error(msg, rc, trap, regs);
> > +     sbi_trap_set_context(scratch, tcntx->prev_context);
> > +     return tcntx;
> >  }
> > --
> > 2.34.1
> >
> >
>



More information about the opensbi mailing list