[edk2 0/7] v3 Assorted fixes to core RISC-V and RiscVVirt

[edk2 0/7] v3 Assorted fixes to core RISC-V and RiscVVirt

[Andrei Warkentin]

Dear all,

Please find the following patches for your reviewing pleasure. Sunil
suggested I put these into a single patch set, instead of sending
out these separately.

This patch set is also available at https://github.com/andreiw/edk2-rv-wip/tree/patchset-1

Andrei Warkentin (7):
  OvmfPkg: RiscVVirt: add SATA support
  MdePkg: BasePeCoffLib: Allow AArch64 and x64 images in
    ImageFormatSupported
  MdePkg: BaseLib: don't log in RISCV InternalSwitchStack
  MdePkg: BaseCpuLib: Fix RISCV CpuSleep symbol name.
  MdeModulePkg: Dxe: add RISCV64 to mMachineTypeInfo
  UefiCpuPkg: CpuTimerDxeRiscV64: fix tick duration accounting
  UefiCpuPkg: BaseRiscV64CpuExceptionHandlerLib: clean up exception
    handling

 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc                                           |   7 +
 OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf                                           |   7 +
 MdePkg/Include/Protocol/DebugSupport.h                                        |  32 ++-
 UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.h |  11 +-
 MdeModulePkg/Core/Dxe/Image/Image.c                                           |   3 +-
 MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c                          |   8 -
 MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c                           |   9 +-
 UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c                                         |  39 ++--
 UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c | 235 ++++++++++++++++++--
 MdePkg/Library/BaseCpuLib/RiscV/Cpu.S                                         |   4 +-
 UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/SupervisorTrapHandler.S  |  17 +-
 11 files changed, 302 insertions(+), 70 deletions(-)

-- 
2.25.1

[edk2 1/7] OvmfPkg: RiscVVirt: add SATA support

[Andrei Warkentin]

Tested with a PCIe pass-thru'd AHCI controller.

Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc | 7 +++++++
 OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
index 054d511138c3..28d9af4d79b9 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
@@ -426,6 +426,13 @@ [Components]
   MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 
+  #
+  # SATA
+  #
+  MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
+  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
+  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
+
   #
   # NVME Driver
   #
diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
index 6072c37af03d..354c9271d10c 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
@@ -137,6 +137,13 @@ [FV.DXEFV]
 INF  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
 INF  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 
+#
+# SATA
+#
+INF  MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
+INF  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
+INF  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
+
 #
 # NVME Driver
 #
-- 
2.25.1

[edk2 2/7] MdePkg: BasePeCoffLib: Allow AArch64 and x64 images in ImageFormatSupported

[Andrei Warkentin]

ARM64 and X64 may allow such foreign images to be used when
driver implementing EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL is
present.

Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Daniel Schaefer <git@danielschaefer.me>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
index adbfe9ccf580..0e9ee395dc26 100644
--- a/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
+++ b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
@@ -104,7 +104,14 @@ PeCoffLoaderImageFormatSupported (
   IN  UINT16  Machine
   )
 {
-  if (Machine ==  IMAGE_FILE_MACHINE_RISCV64) {
+  /*
+   * ARM64 and X64 may allow such foreign images to be used when
+   * a driver implementing EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL is
+   * present.
+   */
+  if (Machine == IMAGE_FILE_MACHINE_RISCV64 ||
+      Machine == IMAGE_FILE_MACHINE_ARM64 ||
+      Machine == IMAGE_FILE_MACHINE_X64) {
     return TRUE;
   }
 
-- 
2.25.1

[edk2 3/7] MdePkg: BaseLib: don't log in RISCV InternalSwitchStack

[Andrei Warkentin]

InternalSwitchStack may be called with a TPL high
enough for a DebugLib implementation to assert.

Other arch implementations don't log either.

Cc: Daniel Schaefer <git@danielschaefer.me>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
index cf646e498aba..b78424c16383 100644
--- a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
+++ b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
@@ -44,14 +44,6 @@ InternalSwitchStack (
 {
   BASE_LIBRARY_JUMP_BUFFER  JumpBuffer;
 
-  DEBUG ((
-    DEBUG_INFO,
-    "RISC-V InternalSwitchStack Entry:%x Context1:%x Context2:%x NewStack%x\n", \
-    EntryPoint,
-    Context1,
-    Context2,
-    NewStack
-    ));
   JumpBuffer.RA = (UINTN)EntryPoint;
   JumpBuffer.SP = (UINTN)NewStack - sizeof (VOID *);
   JumpBuffer.S0 = (UINT64)(UINTN)Context1;
-- 
2.25.1

[edk2 4/7] MdePkg: BaseCpuLib: Fix RISCV CpuSleep symbol name.

[Andrei Warkentin]

CpuSleep, not _CpuSleep.

Cc: Daniel Schaefer <git@danielschaefer.me>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 MdePkg/Library/BaseCpuLib/RiscV/Cpu.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BaseCpuLib/RiscV/Cpu.S b/MdePkg/Library/BaseCpuLib/RiscV/Cpu.S
index 375b91d31427..d6560087e080 100644
--- a/MdePkg/Library/BaseCpuLib/RiscV/Cpu.S
+++ b/MdePkg/Library/BaseCpuLib/RiscV/Cpu.S
@@ -10,9 +10,9 @@
 .align 3
 .section .text
 
-.global ASM_PFX(_CpuSleep)
+.global ASM_PFX(CpuSleep)
 
-ASM_PFX(_CpuSleep):
+ASM_PFX(CpuSleep):
     wfi
     ret
 
-- 
2.25.1

[edk2 5/7] MdeModulePkg: Dxe: add RISCV64 to mMachineTypeInfo

[Andrei Warkentin]

This fixes messages like:
"Image type AARCH64 can't be loaded on <Unknown> UEFI system"

Cc: Daniel Schaefer <git@danielschaefer.me>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 06cc6744b8c6..8704ebea9a7c 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -81,7 +81,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED MACHINE_TYPE_INFO  mMachineTypeInfo[] = {
   { EFI_IMAGE_MACHINE_IA64,           L"IA64"    },
   { EFI_IMAGE_MACHINE_X64,            L"X64"     },
   { EFI_IMAGE_MACHINE_ARMTHUMB_MIXED, L"ARM"     },
-  { EFI_IMAGE_MACHINE_AARCH64,        L"AARCH64" }
+  { EFI_IMAGE_MACHINE_AARCH64,        L"AARCH64" },
+  { EFI_IMAGE_MACHINE_RISCV64,        L"RISCV64" },
 };
 
 UINT16  mDxeCoreImageMachineType = 0;
-- 
2.25.1

[edk2 6/7] [PATCH v2] UefiCpuPkg: CpuTimerDxeRiscV64: fix tick duration accounting

[Andrei Warkentin]

The TimerDxe implementation doesn't account for the physical
time passed due to timer handler execution or (perhaps even
more importantly) time spent with interrupts masked.

Other implementations (e.g. like the Arm one) do. If the
timer tick is always incremented at a fixed rate, then
you can slow down UEFI's perception of time by running
long sections of code in a critical section.

Cc: Daniel Schaefer <git@danielschaefer.me>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c | 33 +++++++++++---------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
index db153f715e60..50f57a9780f0 100644
--- a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
+++ b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
@@ -40,7 +40,8 @@ STATIC EFI_TIMER_NOTIFY  mTimerNotifyFunction;
 //
 // The current period of the timer interrupt
 //
-STATIC UINT64  mTimerPeriod = 0;
+STATIC UINT64  mTimerPeriod     = 0;
+STATIC UINT64  mLastPeriodStart = 0;
 
 /**
   Timer Interrupt Handler.
@@ -56,25 +57,31 @@ TimerInterruptHandler (
   )
 {
   EFI_TPL  OriginalTPL;
-  UINT64   RiscvTimer;
+  UINT64   PeriodStart = RiscVReadTimer ();
 
   OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
   if (mTimerNotifyFunction != NULL) {
-    mTimerNotifyFunction (mTimerPeriod);
+    //
+    // For any number of reasons, the ticks could be coming
+    // in slower than mTimerPeriod. For example, some code
+    // is doing a *lot* of stuff inside an EFI_TPL_HIGH
+    // critical section, and this should not cause the EFI
+    // time to increment slower. So when we take an interrupt,
+    // account for the actual time passed.
+    //
+    mTimerNotifyFunction (PeriodStart - mLastPeriodStart);
   }
 
-  RiscVDisableTimerInterrupt (); // Disable SMode timer int
-  RiscVClearPendingTimerInterrupt ();
   if (mTimerPeriod == 0) {
+    RiscVDisableTimerInterrupt ();
     gBS->RestoreTPL (OriginalTPL);
-    RiscVDisableTimerInterrupt (); // Disable SMode timer int
     return;
   }
 
-  RiscvTimer               = RiscVReadTimer ();
-  SbiSetTimer (RiscvTimer += mTimerPeriod);
+  mLastPeriodStart          = PeriodStart;
+  SbiSetTimer (PeriodStart += mTimerPeriod);
+  RiscVEnableTimerInterrupt (); // enable SMode timer int
   gBS->RestoreTPL (OriginalTPL);
-  RiscVEnableTimerInterrupt (); // enable SMode timer int
 }
 
 /**
@@ -154,8 +161,6 @@ TimerDriverSetTimerPeriod (
   IN UINT64                   TimerPeriod
   )
 {
-  UINT64  RiscvTimer;
-
   DEBUG ((DEBUG_INFO, "TimerDriverSetTimerPeriod(0x%lx)\n", TimerPeriod));
 
   if (TimerPeriod == 0) {
@@ -164,9 +169,9 @@ TimerDriverSetTimerPeriod (
     return EFI_SUCCESS;
   }
 
-  mTimerPeriod = TimerPeriod / 10; // convert unit from 100ns to 1us
-  RiscvTimer   = RiscVReadTimer ();
-  SbiSetTimer (RiscvTimer + mTimerPeriod);
+  mTimerPeriod     = TimerPeriod / 10; // convert unit from 100ns to 1us
+  mLastPeriodStart = RiscVReadTimer ();
+  SbiSetTimer (mLastPeriodStart + mTimerPeriod);
 
   mCpu->EnableInterrupt (mCpu);
   RiscVEnableTimerInterrupt (); // enable SMode timer int
-- 
2.25.1

[edk2 7/7] [PATCH v3] UefiCpuPkg: BaseRiscV64CpuExceptionHandlerLib: clean up exception handling

[Andrei Warkentin]

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: Daniel Schaefer <git@danielschaefer.me>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 MdePkg/Include/Protocol/DebugSupport.h                                        |  32 ++-
 UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.h |  11 +-
 UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c                                         |   6 +-
 UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c | 235 ++++++++++++++++++--
 UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/SupervisorTrapHandler.S  |  17 +-
 5 files changed, 257 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))
 
 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..80869a1c1bff 100644
--- a/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c
+++ b/UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c
@@ -11,11 +11,166 @@
 #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)
+    );
+
+  DEBUG_CODE_BEGIN ();
+
+  #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");
+  }
+
+  #undef REG
+  #undef REGS
+
+  DEBUG_CODE_END ();
+}
 
 /**
   Initializes all CPU exceptions entries and provides the default exception handlers.
@@ -47,34 +202,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;
+  }
+
   return EFI_SUCCESS;
 }
 
@@ -113,21 +293,30 @@ 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);
+  CpuDeadLoop ();
 }
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

Re: [edk2-devel] [edk2 0/7] v3 Assorted fixes to core RISC-V and RiscVVirt

[Sunil V L]

On Mon, Mar 06, 2023 at 03:26:08PM -0600, Andrei Warkentin wrote:
> Dear all,
> 
> Please find the following patches for your reviewing pleasure. Sunil
> suggested I put these into a single patch set, instead of sending
> out these separately.
> 
> This patch set is also available at https://github.com/andreiw/edk2-rv-wip/tree/patchset-1
> 
> Andrei Warkentin (7):
>   OvmfPkg: RiscVVirt: add SATA support
>   MdePkg: BasePeCoffLib: Allow AArch64 and x64 images in
>     ImageFormatSupported
>   MdePkg: BaseLib: don't log in RISCV InternalSwitchStack
>   MdePkg: BaseCpuLib: Fix RISCV CpuSleep symbol name.
>   MdeModulePkg: Dxe: add RISCV64 to mMachineTypeInfo
>   UefiCpuPkg: CpuTimerDxeRiscV64: fix tick duration accounting
>   UefiCpuPkg: BaseRiscV64CpuExceptionHandlerLib: clean up exception
>     handling
> 
>  OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc                                           |   7 +
>  OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf                                           |   7 +
>  MdePkg/Include/Protocol/DebugSupport.h                                        |  32 ++-
>  UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.h |  11 +-
>  MdeModulePkg/Core/Dxe/Image/Image.c                                           |   3 +-
>  MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c                          |   8 -
>  MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c                           |   9 +-
>  UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c                                         |  39 ++--
>  UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/CpuExceptionHandlerLib.c | 235 ++++++++++++++++++--
>  MdePkg/Library/BaseCpuLib/RiscV/Cpu.S                                         |   4 +-
>  UefiCpuPkg/Library/BaseRiscV64CpuExceptionHandlerLib/SupervisorTrapHandler.S  |  17 +-
>  11 files changed, 302 insertions(+), 70 deletions(-)
> 

Thanks. Merged as part of PR #4118