[edk2-devel] [PATCH 7/7] [PATCH v2] UefiCpuPkg: BaseRiscV64CpuExceptionHandlerLib: clean up exception handling

Tuan Phan tphan at ventanamicro.com
Thu Mar 2 05:17:55 UTC 2023


On Wed, Mar 1, 2023 at 8:59 PM Tuan Phan via groups.io <tphan=
ventanamicro.com at groups.io> wrote:

>
>
> On Thu, Feb 23, 2023 at 3:55 PM Andrei Warkentin <
> andrei.warkentin at intel.com> wrote:
>
>> RegisterCpuInterruptHandler did not allow setting
>> exception handlers for anything beyond the timer IRQ.
>> Beyond that, it didn't meet the spec around handling
>> of inputs.
>>
>> RiscVSupervisorModeTrapHandler now will invoke
>> set handlers for both exceptions and interrupts.
>> Two arrays of handlers are maintained - one for exceptions
>> and one for interrupts.
>>
>> For unhandled traps, RiscVSupervisorModeTrapHandler dumps
>> state using the now implemented DumpCpuContext.
>>
>> For EFI_SYSTEM_CONTEXT_RISCV64, extend this with the trapped
>> PC address (SEPC), just like on AArch64 (ELR). This is
>> necessary for X86EmulatorPkg to work as it allows a trap
>> handler to return execution to a different place. Add
>> SSTATUS/STVAL as well, at least for debugging purposes. There
>> is no value in hiding this.
>>
>> Fix nested exception handling. Handler code should not
>> be saving SIE (the value is saved in SSTATUS.SPIE) or
>> directly restored (that's done by SRET). Save and
>> restore the entire SSTATUS and STVAL, too.
>>
>> Cc: Sunil V L <sunilvl at ventanamicro.com>
>> Cc: Daniel Schaefer <git at danielschaefer.me>
>> Signed-off-by: Andrei Warkentin <andrei.warkentin at intel.com>
>> ---
>>  MdePkg/Include/Protocol/DebugSupport.h        |  32 ++-
>>  .../CpuExceptionHandlerLib.h                  |  11 +-
>>  UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c         |   6 +-
>>  .../CpuExceptionHandlerLib.c                  | 232 ++++++++++++++++--
>>  .../SupervisorTrapHandler.S                   |  17 +-
>>  5 files changed, 254 insertions(+), 44 deletions(-)
>>
>> diff --git a/MdePkg/Include/Protocol/DebugSupport.h
>> b/MdePkg/Include/Protocol/DebugSupport.h
>> index 2b0ae2d1577b..9742663619c5 100644
>> --- a/MdePkg/Include/Protocol/DebugSupport.h
>> +++ b/MdePkg/Include/Protocol/DebugSupport.h
>> @@ -613,11 +613,34 @@ typedef struct {
>>  #define EXCEPT_RISCV_STORE_AMO_ACCESS_FAULT        7
>>  #define EXCEPT_RISCV_ENV_CALL_FROM_UMODE           8
>>  #define EXCEPT_RISCV_ENV_CALL_FROM_SMODE           9
>> -#define EXCEPT_RISCV_ENV_CALL_FROM_HMODE           10
>> +#define EXCEPT_RISCV_ENV_CALL_FROM_VS_MODE         10
>>  #define EXCEPT_RISCV_ENV_CALL_FROM_MMODE           11
>> +#define EXCEPT_RISCV_INST_ACCESS_PAGE_FAULT        12
>> +#define EXCEPT_RISCV_LOAD_ACCESS_PAGE_FAULT        13
>> +#define EXCEPT_RISCV_14                            14
>> +#define EXCEPT_RISCV_STORE_ACCESS_PAGE_FAULT       15
>> +#define EXCEPT_RISCV_16                            16
>> +#define EXCEPT_RISCV_17                            17
>> +#define EXCEPT_RISCV_18                            18
>> +#define EXCEPT_RISCV_19                            19
>> +#define EXCEPT_RISCV_INST_GUEST_PAGE_FAULT         20
>> +#define EXCEPT_RISCV_LOAD_GUEST_PAGE_FAULT         21
>> +#define EXCEPT_RISCV_VIRTUAL_INSTRUCTION           22
>> +#define EXCEPT_RISCV_STORE_GUEST_PAGE_FAULT        23
>> +#define EXCEPT_RISCV_MAX_EXCEPTIONS
>> (EXCEPT_RISCV_STORE_GUEST_PAGE_FAULT)
>>
>> -#define EXCEPT_RISCV_SOFTWARE_INT  0x0
>> -#define EXCEPT_RISCV_TIMER_INT     0x1
>> +///
>> +/// RISC-V processor exception types for interrupts.
>> +///
>> +#define EXCEPT_RISCV_IS_IRQ(x)     ((x & 0x8000000000000000UL) != 0)
>> +#define EXCEPT_RISCV_IRQ_INDEX(x)  (x & 0x7FFFFFFFFFFFFFFFUL)
>> +#define EXCEPT_RISCV_IRQ_0                 0x8000000000000000UL
>> +#define EXCEPT_RISCV_IRQ_SOFT_FROM_SMODE   0x8000000000000001UL
>> +#define EXCEPT_RISCV_IRQ_SOFT_FROM_VSMODE  0x8000000000000002UL
>> +#define EXCEPT_RISCV_IRQ_SOFT_FROM_MMODE   0x8000000000000003UL
>> +#define EXCEPT_RISCV_IRQ_4                 0x8000000000000004UL
>> +#define EXCEPT_RISCV_IRQ_TIMER_FROM_SMODE  0x8000000000000005UL
>> +#define EXCEPT_RISCV_MAX_IRQS
>> (EXCEPT_RISCV_IRQ_INDEX(EXCEPT_RISCV_IRQ_TIMER_FROM_SMODE))
>>
>> There is no difference in terms of processing interrupt or exception
> type. It is cleaner if we treat interrupts and exceptions the same. So no
> need to detect if an event is interrupt or exception.
>
>
>>  typedef struct {
>>    UINT64    X0;
>> @@ -652,6 +675,9 @@ typedef struct {
>>    UINT64    X29;
>>    UINT64    X30;
>>    UINT64    X31;
>> +  UINT64    SEPC;
>> +  UINT32    SSTATUS;
>> +  UINT32    STVAL;
>>  } EFI_SYSTEM_CONTEXT_RISCV64;
>>
>>  //
>> diff --git
>> a/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.h
>> b/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.h
>> index 30f47e87552b..9b7e1304dd3b 100644
>> ---
>> a/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.h
>> +++
>> b/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.h
>> @@ -59,7 +59,7 @@ SupervisorModeTrap (
>>  #define SMODE_TRAP_REGS_t6       31
>>  #define SMODE_TRAP_REGS_sepc     32
>>  #define SMODE_TRAP_REGS_sstatus  33
>> -#define SMODE_TRAP_REGS_sie      34
>> +#define SMODE_TRAP_REGS_stval    34
>>  #define SMODE_TRAP_REGS_last     35
>>
>>  #define SMODE_TRAP_REGS_OFFSET(x)  ((SMODE_TRAP_REGS_##x) *
>> __SIZEOF_POINTER__)
>> @@ -68,7 +68,7 @@ SupervisorModeTrap (
>>  #pragma pack(1)
>>  typedef struct {
>>    //
>> -  // Below are follow the format of EFI_SYSTEM_CONTEXT
>> +  // Below follow the format of EFI_SYSTEM_CONTEXT.
>>    //
>>    UINT64    zero;
>>    UINT64    ra;
>> @@ -102,14 +102,9 @@ typedef struct {
>>    UINT64    t4;
>>    UINT64    t5;
>>    UINT64    t6;
>> -  //
>> -  // Below are the additional information to
>> -  // EFI_SYSTEM_CONTEXT, private to supervisor mode trap
>> -  // and not public to EFI environment.
>> -  //
>>    UINT64    sepc;
>>    UINT64    sstatus;
>> -  UINT64    sie;
>> +  UINT64    stval;
>>  } SMODE_TRAP_REGISTERS;
>>  #pragma pack()
>>
>> diff --git a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
>> b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
>> index 50f57a9780f0..ab280d3b2d7d 100644
>> --- a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
>> +++ b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
>> @@ -276,7 +276,11 @@ TimerDriverInitialize (
>>    //
>>    // Install interrupt handler for RISC-V Timer.
>>    //
>> -  Status = mCpu->RegisterInterruptHandler (mCpu, EXCEPT_RISCV_TIMER_INT,
>> TimerInterruptHandler);
>> +  Status = mCpu->RegisterInterruptHandler (
>> +                   mCpu,
>> +                   EXCEPT_RISCV_IRQ_TIMER_FROM_SMODE,
>> +                   TimerInterruptHandler
>> +                   );
>>    ASSERT_EFI_ERROR (Status);
>>
>>    //
>> diff --git
>> a/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c
>> b/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c
>> index f1ee7d236aec..1beaf5ac513e 100644
>> ---
>> a/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c
>> +++
>> b/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c
>> @@ -11,11 +11,162 @@
>>  #include <Library/CpuExceptionHandlerLib.h>
>>  #include <Library/DebugLib.h>
>>  #include <Library/BaseLib.h>
>> +#include <Library/SerialPortLib.h>
>> +#include <Library/PrintLib.h>
>>  #include <Register/RiscV64/RiscVEncoding.h>
>> -
>>  #include "CpuExceptionHandlerLib.h"
>>
>> -STATIC EFI_CPU_INTERRUPT_HANDLER  mInterruptHandlers[2];
>> +//
>> +// Define the maximum message length
>> +//
>> +#define MAX_DEBUG_MESSAGE_LENGTH  0x100
>> +
>> +STATIC EFI_CPU_INTERRUPT_HANDLER
>> mExceptionHandlers[EXCEPT_RISCV_MAX_EXCEPTIONS + 1];
>> +STATIC EFI_CPU_INTERRUPT_HANDLER  mIrqHandlers[EXCEPT_RISCV_MAX_IRQS +
>> 1];
>> +
>> +STATIC CONST CHAR8  mExceptionOrIrqUnknown[]
>> = "Unknown";
>> +STATIC CONST CHAR8  *mExceptionNameStr[EXCEPT_RISCV_MAX_EXCEPTIONS + 1]
>> = {
>> +  "EXCEPT_RISCV_INST_MISALIGNED",
>> +  "EXCEPT_RISCV_INST_ACCESS_FAULT",
>> +  "EXCEPT_RISCV_ILLEGAL_INST",
>> +  "EXCEPT_RISCV_BREAKPOINT",
>> +  "EXCEPT_RISCV_LOAD_ADDRESS_MISALIGNED",
>> +  "EXCEPT_RISCV_LOAD_ACCESS_FAULT",
>> +  "EXCEPT_RISCV_STORE_AMO_ADDRESS_MISALIGNED",
>> +  "EXCEPT_RISCV_STORE_AMO_ACCESS_FAULT",
>> +  "EXCEPT_RISCV_ENV_CALL_FROM_UMODE",
>> +  "EXCEPT_RISCV_ENV_CALL_FROM_SMODE",
>> +  "EXCEPT_RISCV_ENV_CALL_FROM_VS_MODE",
>> +  "EXCEPT_RISCV_ENV_CALL_FROM_MMODE",
>> +  "EXCEPT_RISCV_INST_ACCESS_PAGE_FAULT",
>> +  "EXCEPT_RISCV_LOAD_ACCESS_PAGE_FAULT",
>> +  "EXCEPT_RISCV_14",
>> +  "EXCEPT_RISCV_STORE_ACCESS_PAGE_FAULT",
>> +  "EXCEPT_RISCV_16",
>> +  "EXCEPT_RISCV_17",
>> +  "EXCEPT_RISCV_18",
>> +  "EXCEPT_RISCV_19",
>> +  "EXCEPT_RISCV_INST_GUEST_PAGE_FAULT",
>> +  "EXCEPT_RISCV_LOAD_GUEST_PAGE_FAULT",
>> +  "EXCEPT_RISCV_VIRTUAL_INSTRUCTION",
>> +  "EXCEPT_RISCV_STORE_GUEST_PAGE_FAULT"
>> +};
>> +
>> +STATIC CONST CHAR8  *mIrqNameStr[EXCEPT_RISCV_MAX_IRQS + 1] = {
>> +  "EXCEPT_RISCV_IRQ_0",
>> +  "EXCEPT_RISCV_IRQ_SOFT_FROM_SMODE",
>> +  "EXCEPT_RISCV_IRQ_SOFT_FROM_VSMODE",
>> +  "EXCEPT_RISCV_IRQ_SOFT_FROM_MMODE",
>> +  "EXCEPT_RISCV_IRQ_4",
>> +  "EXCEPT_RISCV_IRQ_TIMER_FROM_SMODE",
>> +};
>> +
>> +/**
>> +  Prints a message to the serial port.
>> +
>> +  @param  Format      Format string for the message to print.
>> +  @param  ...         Variable argument list whose contents are accessed
>> +                      based on the format string specified by Format.
>> +
>> +**/
>> +STATIC
>> +VOID
>> +EFIAPI
>> +InternalPrintMessage (
>> +  IN  CONST CHAR8  *Format,
>> +  ...
>> +  )
>> +{
>> +  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
>> +  VA_LIST  Marker;
>> +
>> +  //
>> +  // Convert the message to an ASCII String
>> +  //
>> +  VA_START (Marker, Format);
>> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
>> +  VA_END (Marker);
>> +
>> +  //
>> +  // Send the print string to a Serial Port
>> +  //
>> +  SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
>> +}
>> +
>> +/**
>> +  Get ASCII format string exception name by exception type.
>> +
>> +  @param ExceptionType  Exception type.
>> +
>> +  @return  ASCII format string exception name.
>> +**/
>> +STATIC
>> +CONST CHAR8 *
>> +GetExceptionNameStr (
>> +  IN EFI_EXCEPTION_TYPE  ExceptionType
>> +  )
>> +{
>> +  if (EXCEPT_RISCV_IS_IRQ (ExceptionType)) {
>> +    if (EXCEPT_RISCV_IRQ_INDEX (ExceptionType) > EXCEPT_RISCV_MAX_IRQS) {
>> +      return mExceptionOrIrqUnknown;
>> +    }
>> +
>> +    return mIrqNameStr[EXCEPT_RISCV_IRQ_INDEX (ExceptionType)];
>> +  }
>> +
>> +  if (ExceptionType > EXCEPT_RISCV_MAX_EXCEPTIONS) {
>> +    return mExceptionOrIrqUnknown;
>> +  }
>> +
>> +  return mExceptionNameStr[ExceptionType];
>> +}
>> +
>> +/**
>> +  Display CPU information. This can be called by 3rd-party handlers
>> +  set by RegisterCpuInterruptHandler.
>> +
>> +  @param ExceptionType  Exception type.
>> +  @param SystemContext  Pointer to EFI_SYSTEM_CONTEXT.
>> +**/
>> +VOID
>> +EFIAPI
>> +DumpCpuContext (
>> +  IN EFI_EXCEPTION_TYPE  ExceptionType,
>> +  IN EFI_SYSTEM_CONTEXT  SystemContext
>> +  )
>> +{
>> +  UINTN                 Printed = 0;
>> +  SMODE_TRAP_REGISTERS  *Regs   = (SMODE_TRAP_REGISTERS *)
>> +                                  SystemContext.SystemContextRiscV64;
>> +
>> +  InternalPrintMessage (
>> +    "!!!! RISCV64 Exception Type - %016x(%a) !!!!\n",
>> +    ExceptionType,
>> +    GetExceptionNameStr (ExceptionType)
>> +    );
>> +
>> +  #define REGS()
>>   \
>> +  REG (t0); REG (t1); REG (t2); REG (t3); REG (t4); REG (t5); REG (t6); \
>> +  REG (s0); REG (s1); REG (s2); REG (s3); REG (s4); REG (s5); REG (s6); \
>> +  REG (s7); REG (s8); REG (s9); REG (s10); REG (s11);                   \
>> +  REG (a0); REG (a1); REG (a2); REG (a3); REG (a4); REG (a5); REG (a6); \
>> +  REG (a7);                                                             \
>> +  REG (zero); REG (ra); REG (sp); REG (gp); REG (tp);                   \
>> +  REG (sepc); REG (sstatus); REG (stval);
>> +
>> +  #define REG(x)  do {                                      \
>> +    InternalPrintMessage ("%7a = 0x%017lx%c", #x, Regs->x,  \
>> +                          (++Printed % 2) ? L'\t' : L'\n'); \
>> +  } while (0);
>> +
>> +  REGS ();
>> +  if (Printed % 2) {
>> +    InternalPrintMessage ("\n");
>> +  }
>> +
>>
> I would like to only print registers at DEBUG build only. If you look at
> ARM implementation, register and stack
> dump are only enabled at DEBUG build.
> Printing them all at release version can be at security risk. Anyone
> wanting to debug an issue likely builds a debug version.
>
> +  #undef REG
>> +  #undef REGS
>> +}
>>
>>  /**
>>    Initializes all CPU exceptions entries and provides the default
>> exception handlers.
>> @@ -47,34 +198,59 @@ InitializeCpuExceptionHandlers (
>>    Registers a function to be called from the processor interrupt handler.
>>
>>    This function registers and enables the handler specified by
>> InterruptHandler for a processor
>> -  interrupt or exception type specified by InterruptType. If
>> InterruptHandler is NULL, then the
>> -  handler for the processor interrupt or exception type specified by
>> InterruptType is uninstalled.
>> +  interrupt or exception type specified by ExceptionType. If
>> InterruptHandler is NULL, then the
>> +  handler for the processor interrupt or exception type specified by
>> ExceptionType is uninstalled.
>>    The installed handler is called once for each processor interrupt or
>> exception.
>>    NOTE: This function should be invoked after
>> InitializeCpuExceptionHandlers() or
>>    InitializeCpuInterruptHandlers() invoked, otherwise EFI_UNSUPPORTED
>> returned.
>>
>> -  @param[in]  InterruptType     Defines which interrupt or exception to
>> hook.
>> +  @param[in]  ExceptionType     Defines which interrupt or exception to
>> hook.
>>    @param[in]  InterruptHandler  A pointer to a function of type
>> EFI_CPU_INTERRUPT_HANDLER that is called
>>                                  when a processor interrupt occurs. If
>> this parameter is NULL, then the handler
>>                                  will be uninstalled.
>>
>>    @retval EFI_SUCCESS           The handler for the processor interrupt
>> was successfully installed or uninstalled.
>> -  @retval EFI_ALREADY_STARTED   InterruptHandler is not NULL, and a
>> handler for InterruptType was
>> +  @retval EFI_ALREADY_STARTED   InterruptHandler is not NULL, and a
>> handler for ExceptionType was
>>                                  previously installed.
>> -  @retval EFI_INVALID_PARAMETER InterruptHandler is NULL, and a handler
>> for InterruptType was not
>> +  @retval EFI_INVALID_PARAMETER InterruptHandler is NULL, and a handler
>> for ExceptionType was not
>>                                  previously installed.
>> -  @retval EFI_UNSUPPORTED       The interrupt specified by InterruptType
>> is not supported,
>> +  @retval EFI_UNSUPPORTED       The interrupt specified by ExceptionType
>> is not supported,
>>                                  or this function is not supported.
>>  **/
>>  EFI_STATUS
>>  EFIAPI
>>  RegisterCpuInterruptHandler (
>> -  IN EFI_EXCEPTION_TYPE         InterruptType,
>> +  IN EFI_EXCEPTION_TYPE         ExceptionType,
>>    IN EFI_CPU_INTERRUPT_HANDLER  InterruptHandler
>>    )
>>  {
>> -  DEBUG ((DEBUG_INFO, "%a: Type:%x Handler: %x\n", __FUNCTION__,
>> InterruptType, InterruptHandler));
>> -  mInterruptHandlers[InterruptType] = InterruptHandler;
>> +  DEBUG ((DEBUG_INFO, "%a: Type:%x Handler: %x\n", __FUNCTION__,
>> ExceptionType, InterruptHandler));
>> +  if (EXCEPT_RISCV_IS_IRQ (ExceptionType)) {
>> +    if (EXCEPT_RISCV_IRQ_INDEX (ExceptionType) > EXCEPT_RISCV_MAX_IRQS) {
>> +      return EFI_UNSUPPORTED;
>> +    }
>> +
>> +    if (mIrqHandlers[EXCEPT_RISCV_IRQ_INDEX (ExceptionType)] != NULL) {
>> +      return EFI_ALREADY_STARTED;
>> +    } else if (InterruptHandler == NULL) {
>> +      return EFI_INVALID_PARAMETER;
>> +    }
>> +
>> +    mIrqHandlers[EXCEPT_RISCV_IRQ_INDEX (ExceptionType)] =
>> InterruptHandler;
>> +  } else {
>> +    if (ExceptionType > EXCEPT_RISCV_MAX_EXCEPTIONS) {
>> +      return EFI_UNSUPPORTED;
>> +    }
>> +
>> +    if (mExceptionHandlers[ExceptionType] != NULL) {
>> +      return EFI_ALREADY_STARTED;
>> +    } else if (InterruptHandler == NULL) {
>> +      return EFI_INVALID_PARAMETER;
>> +    }
>> +
>> +    mExceptionHandlers[ExceptionType] = InterruptHandler;
>> +  }
>> +
>>
> As my first comment, this code can be clearer if the interrupt and
> exception are the same.
>
>    return EFI_SUCCESS;
>>  }
>>
>> @@ -113,21 +289,31 @@ RiscVSupervisorModeTrapHandler (
>>    SMODE_TRAP_REGISTERS  *SmodeTrapReg
>>    )
>>  {
>> -  UINTN               SCause;
>> +  EFI_EXCEPTION_TYPE  ExceptionType;
>>    EFI_SYSTEM_CONTEXT  RiscVSystemContext;
>>
>>    RiscVSystemContext.SystemContextRiscV64 = (EFI_SYSTEM_CONTEXT_RISCV64
>> *)SmodeTrapReg;
>> -  //
>> -  // Check scasue register.
>> -  //
>> -  SCause = (UINTN)RiscVGetSupervisorTrapCause ();
>> -  if ((SCause & (1UL << (sizeof (UINTN) * 8- 1))) != 0) {
>> -    //
>> -    // This is interrupt event.
>> -    //
>> -    SCause &= ~(1UL << (sizeof (UINTN) * 8- 1));
>> -    if ((SCause == IRQ_S_TIMER) &&
>> (mInterruptHandlers[EXCEPT_RISCV_TIMER_INT] != NULL)) {
>> -      mInterruptHandlers[EXCEPT_RISCV_TIMER_INT](EXCEPT_RISCV_TIMER_INT,
>> RiscVSystemContext);
>> +  ExceptionType                           =
>> (UINTN)RiscVGetSupervisorTrapCause ();
>> +
>> +  if (EXCEPT_RISCV_IS_IRQ (ExceptionType)) {
>> +    UINTN  IrqIndex = EXCEPT_RISCV_IRQ_INDEX (ExceptionType);
>> +
>> +    if ((IrqIndex <= EXCEPT_RISCV_MAX_IRQS) &&
>> +        (mIrqHandlers[IrqIndex] != NULL))
>> +    {
>> +      mIrqHandlers[IrqIndex](ExceptionType, RiscVSystemContext);
>> +      return;
>>      }
>> +  } else {
>> +    if ((ExceptionType <= EXCEPT_RISCV_MAX_EXCEPTIONS) &&
>> +        (mExceptionHandlers[ExceptionType] != 0))
>> +    {
>> +      mExceptionHandlers[ExceptionType](ExceptionType,
>> RiscVSystemContext);
>> +      return;
>> +    }
>> +  }
>> +
>> +  DumpCpuContext (ExceptionType, RiscVSystemContext);
>> +  while (1) {
>>
> Would it be better with "do{} while (1)"?
>
CpuDeadLoop() even better.

>    }
>>  }
>> diff --git
>> a/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/SupervisorTrapHandler.S
>> b/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/SupervisorTrapHandler.S
>> index 649c4c5becf4..45070b5cda04 100644
>> ---
>> a/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/SupervisorTrapHandler.S
>> +++
>> b/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/SupervisorTrapHandler.S
>> @@ -20,14 +20,14 @@ SupervisorModeTrap:
>>    sd    t0, SMODE_TRAP_REGS_OFFSET(t0)(sp)
>>
>>    csrr  t0, CSR_SSTATUS
>> -  and   t0, t0, (SSTATUS_SIE | SSTATUS_SPIE)
>>    sd    t0, SMODE_TRAP_REGS_OFFSET(sstatus)(sp)
>>    csrr  t0, CSR_SEPC
>>    sd    t0, SMODE_TRAP_REGS_OFFSET(sepc)(sp)
>> -  csrr  t0, CSR_SIE
>> -  sd    t0, SMODE_TRAP_REGS_OFFSET(sie)(sp)
>> +  csrr  t0, CSR_STVAL
>> +  sd    t0, SMODE_TRAP_REGS_OFFSET(stval)(sp)
>>    ld    t0, SMODE_TRAP_REGS_OFFSET(t0)(sp)
>>
>> +  sd    zero, SMODE_TRAP_REGS_OFFSET(zero)(sp)
>>    sd    ra, SMODE_TRAP_REGS_OFFSET(ra)(sp)
>>    sd    gp, SMODE_TRAP_REGS_OFFSET(gp)(sp)
>>    sd    tp, SMODE_TRAP_REGS_OFFSET(tp)(sp)
>> @@ -59,6 +59,7 @@ SupervisorModeTrap:
>>    sd    t6, SMODE_TRAP_REGS_OFFSET(t6)(sp)
>>
>>    /* Call to Supervisor mode trap handler in CpuExceptionHandlerLib.c */
>> +  mv    a0, sp
>>    call  RiscVSupervisorModeTrapHandler
>>
>>    /* Restore all general regisers except SP */
>> @@ -66,6 +67,7 @@ SupervisorModeTrap:
>>    ld    gp, SMODE_TRAP_REGS_OFFSET(gp)(sp)
>>    ld    tp, SMODE_TRAP_REGS_OFFSET(tp)(sp)
>>    ld    t2, SMODE_TRAP_REGS_OFFSET(t2)(sp)
>> +  ld    t1, SMODE_TRAP_REGS_OFFSET(t1)(sp)
>>    ld    s0, SMODE_TRAP_REGS_OFFSET(s0)(sp)
>>    ld    s1, SMODE_TRAP_REGS_OFFSET(s1)(sp)
>>    ld    a0, SMODE_TRAP_REGS_OFFSET(a0)(sp)
>> @@ -93,13 +95,10 @@ SupervisorModeTrap:
>>
>>    ld    t0, SMODE_TRAP_REGS_OFFSET(sepc)(sp)
>>    csrw  CSR_SEPC, t0
>> -  ld    t0, SMODE_TRAP_REGS_OFFSET(sie)(sp)
>> -  csrw  CSR_SIE, t0
>> -  csrr  t0, CSR_SSTATUS
>> -  ld    t1, SMODE_TRAP_REGS_OFFSET(sstatus)(sp)
>> -  or    t0, t0, t1
>> +  ld    t0, SMODE_TRAP_REGS_OFFSET(sstatus)(sp)
>>    csrw  CSR_SSTATUS, t0
>> -  ld    t1, SMODE_TRAP_REGS_OFFSET(t1)(sp)
>> +  ld    t0, SMODE_TRAP_REGS_OFFSET(stval)(sp)
>> +  csrw  CSR_STVAL, t0
>>    ld    t0, SMODE_TRAP_REGS_OFFSET(t0)(sp)
>>    addi  sp, sp, SMODE_TRAP_REGS_SIZE
>>    sret
>> --
>> 2.25.1
>>
>>
>>
>>
>>
>>
>> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100618): https://edk2.groups.io/g/devel/message/100618
Mute This Topic: https://groups.io/mt/97196087/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20230301/bc2f0bd1/attachment-0001.htm>


More information about the edk2-devel-archive mailing list