* [PATCH 1/1] BaseLib:Fix RISC-V Supervisor mode (S-Mode) trap handler reentry issue.
@ 2020-07-16 14:10 Abner Chang
2020-07-22 7:44 ` Daniel Schaefer
0 siblings, 1 reply; 2+ messages in thread
From: Abner Chang @ 2020-07-16 14:10 UTC (permalink / raw)
To: devel
Cc: abner.chang, Michael D Kinney, Liming Gao, Daniel Schaefer,
Leif Lindholm
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>
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>
Signed-off-by: Abner Chang <abner.chang@hpe.com>
---
.../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..87b3468fc7 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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 1/1] BaseLib:Fix RISC-V Supervisor mode (S-Mode) trap handler reentry issue.
2020-07-16 14:10 [PATCH 1/1] BaseLib:Fix RISC-V Supervisor mode (S-Mode) trap handler reentry issue Abner Chang
@ 2020-07-22 7:44 ` Daniel Schaefer
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Schaefer @ 2020-07-22 7:44 UTC (permalink / raw)
To: Abner Chang, devel; +Cc: Michael D Kinney, Liming Gao, Leif Lindholm
On 7/16/20 4:10 PM, Abner Chang wrote:
> 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>
>
> 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>
> Signed-off-by: Abner Chang <abner.chang@hpe.com>
> ---
> .../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..87b3468fc7 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
Please start this define at the beginning of the line.
>
> +//
> +// 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
Good! Let's not lose those register values.
> +//
> +// 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
because
> + // 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
>
>
Looks good otherwise :) With the the small changes:
Reviewed-by: Daniel Schaefer <daniel.schaefer@hpe.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-22 7:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-16 14:10 [PATCH 1/1] BaseLib:Fix RISC-V Supervisor mode (S-Mode) trap handler reentry issue Abner Chang
2020-07-22 7:44 ` Daniel Schaefer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox