Re: [PATCH v2 1/1] BaseLib:Fix RISC-V Supervisor mode (S-Mode) trap handler reentry issue.

["Liming Gao" <liming.gao@intel.com>]

Acked-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Abner Chang <abner.chang@hpe.com> 
Sent: 2020年7月22日 16:54
To: devel@edk2.groups.io
Cc: abner.chang@hpe.com; Daniel Schaefer <daniel.schaefer@hpe.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>
Subject: [PATCH v2 1/1] BaseLib:Fix RISC-V Supervisor mode (S-Mode) trap handler reentry issue.

While RISC-V hart is trapped into S-Mode, the S-Mode interrupt CSR (SIE) is disabled by RISC-V hart. However the (SIE) is enabled again by RestoreTPL, this causes the second S-Mode trap is triggered by the machine mode (M-Mode)timer interrupt redirection. The SRET instruction clear Supervisor Previous Privilege (SPP) to zero (User mode) in the second S-Mode interrupt according to the RISC-V spec. Above brings hart to the user mode (U-Mode) when execute SRET in the nested S-Mode interrupt handler because SPP is set to User Mode in the second interrupt. Afterward, system runs in U-Mode and any accesses to S-Mode CSR causes the invalid instruction exception.

Signed-off-by: Abner Chang <abner.chang@hpe.com>
Reviewed-by: Daniel Schaefer <daniel.schaefer@hpe.com>

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
---
 .../Library/BaseLib/RiscV64/RiscVInterrupt.S  | 45 ++++++++++++++++---
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S b/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S
index 766fcfb9cb..e821124781 100644
--- a/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S
+++ b/MdePkg/Library/BaseLib/RiscV64/RiscVInterrupt.S
@@ -12,21 +12,52 @@ ASM_GLOBAL ASM_PFX(RiscVDisableSupervisorModeInterrupts)
 ASM_GLOBAL ASM_PFX(RiscVEnableSupervisorModeInterrupt) ASM_GLOBAL ASM_PFX(RiscVGetSupervisorModeInterrupts) -# define  MSTATUS_SIE    0x00000002-# define  CSR_SSTATUS    0x100+#define  SSTATUS_SIE                 0x00000002+#define  CSR_SSTATUS                 0x100+#define  SSTATUS_SPP_BIT_POSITION    8 +//+// This routine disables supervisor mode interrupt+// ASM_PFX(RiscVDisableSupervisorModeInterrupts):-  li   a1, MSTATUS_SIE-  csrc CSR_SSTATUS, a1+  add   sp, sp, -(__SIZEOF_POINTER__)+  sd    a1, (sp)+  li    a1, SSTATUS_SIE+  csrc  CSR_SSTATUS, a1+  ld    a1, (sp)+  add   sp, sp, (__SIZEOF_POINTER__)   ret +//+// This routine enables supervisor mode interrupt+// ASM_PFX(RiscVEnableSupervisorModeInterrupt):-  li   a1, MSTATUS_SIE-  csrs CSR_SSTATUS, a1+  add   sp, sp, -2*(__SIZEOF_POINTER__)+  sd    a0, (0*__SIZEOF_POINTER__)(sp)+  sd    a1, (1*__SIZEOF_POINTER__)(sp)++  csrr  a0, CSR_SSTATUS+  and   a0, a0, (1 << SSTATUS_SPP_BIT_POSITION)+  bnez  a0, InTrap      // We are in supervisor mode (SMode)+                        // trap handler.+                        // Skip enabling SIE becasue SIE+                        // is set to disabled by RISC-V hart+                        // when the trap takes hart to SMode.++  li    a1, SSTATUS_SIE+  csrs  CSR_SSTATUS, a1+InTrap:+  ld    a0, (0*__SIZEOF_POINTER__)(sp)+  ld    a1, (1*__SIZEOF_POINTER__)(sp)+  add   sp, sp, 2*(__SIZEOF_POINTER__)   ret +//+// This routine returns supervisor mode interrupt+// status.+// ASM_PFX(RiscVGetSupervisorModeInterrupts):   csrr a0, CSR_SSTATUS-  andi a0, a0, MSTATUS_SIE+  andi a0, a0, SSTATUS_SIE   ret -- 
2.25.0