Re: [U-Boot] [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
Subject: [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs.
It is important to note that there is no equivalent S-mode CSR for misa and mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to emulate misa and mhartid CSR read.
In-future, we will have more patches to avoid accessing misa and mhartid CSRs from S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 5 +++++ arch/riscv/cpu/start.S | 33 ++++++++++++++++++++++++++++ arch/riscv/include/asm/encoding.h | 2 ++ arch/riscv/lib/interrupts.c | 36 +++++++++++++++++++++++-------- 4 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -55,6 +55,11 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
+config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 15e1b8199a..704190f946 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -41,10 +41,18 @@ _start: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) la t0, trap_entry +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
/* mask all interrupts */+#ifdef CONFIG_RISCV_SMODE
csrw sie, zero+#else csrw mie, zero +#endif
/* Enable cache */ jal icache_enable@@ -166,7 +174,11 @@ fix_rela_dyn: */ la t0, trap_entry add t0, t0, t6 +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
clear_bss: la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ @@ -238,17 +250,34 @@ trap_entry: SREG x29, 29*REGBYTES(sp) SREG x30, 30*REGBYTES(sp) SREG x31, 31*REGBYTES(sp) +#ifdef CONFIG_RISCV_SMODE
csrr a0, scausecsrr a1, sepc+#else csrr a0, mcause csrr a1, mepc +#endif mv a2, sp jal handle_trap +#ifdef CONFIG_RISCV_SMODE
csrw sepc, a0+#else csrw mepc, a0 +#endif
+#ifdef CONFIG_RISCV_SMODE +/*
- Remain in S-mode after sret
- */
li t0, SSTATUS_SPPcsrs sstatus, t0+#else /*
- Remain in M-mode after mret
*/ li t0, MSTATUS_MPP csrs mstatus, t0 +#endif
It only the difference between s and m in csr. The code seem to be duplicate too much. Can you implement it in macro ? or consider to separate it in different .S file
It may too complex to maintain in the future.
B.R Rick
LREG x1, 1*REGBYTES(sp) LREG x2, 2*REGBYTES(sp) LREG x3, 3*REGBYTES(sp)@@ -281,4 +310,8 @@ trap_entry: LREG x30, 30*REGBYTES(sp) LREG x31, 31*REGBYTES(sp) addi sp, sp, 32*REGBYTES +#ifdef CONFIG_RISCV_SMODE
sret+#else mret +#endif diff --git a/arch/riscv/include/asm/encoding.h b/arch/riscv/include/asm/encoding.h index 9ea50ce640..153f5af2ff 100644 --- a/arch/riscv/include/asm/encoding.h +++ b/arch/riscv/include/asm/encoding.h @@ -143,6 +143,8 @@ # define MCAUSE_CAUSE MCAUSE32_CAUSE #endif
+#define SCAUSE_INT MCAUSE_INT
#define RISCV_PGSHIFT 12 #define RISCV_PGSIZE BIT(RISCV_PGSHIFT)
diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 903a1c4cd5..8793f233d0 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -34,17 +34,35 @@ int disable_interrupts(void) return 0; }
-ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs) +ulong handle_trap(ulong cause, ulong epc, struct pt_regs *regs) {
ulong is_int;
ulong is_irq, irq;
is_int = (mcause & MCAUSE_INT);if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT))external_interrupt(0); /* handle_m_ext_interrupt */else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER))timer_interrupt(0); /* handle_m_timer_interrupt */else_exit_trap(mcause, epc, regs);+#ifdef CONFIG_RISCV_SMODE
is_irq = (cause & SCAUSE_INT);irq = (cause & ~SCAUSE_INT);+#else
is_irq = (cause & MCAUSE_INT);irq = (cause & ~MCAUSE_INT);+#endif
if (is_irq) {switch (irq) {case IRQ_M_EXT:case IRQ_S_EXT:external_interrupt(0); /* handle external interrupt */break;case IRQ_M_TIMER:case IRQ_S_TIMER:timer_interrupt(0); /* handle timer interrupt */break;default:_exit_trap(cause, epc, regs);break;};} else {_exit_trap(cause, epc, regs);} return epc;}
2.17.1
On Tue, Nov 27, 2018 at 12:09 PM Rick Chen rickchen36@gmail.com wrote:
Subject: [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs.
It is important to note that there is no equivalent S-mode CSR for misa and mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to emulate misa and mhartid CSR read.
In-future, we will have more patches to avoid accessing misa and mhartid CSRs from S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 5 +++++ arch/riscv/cpu/start.S | 33 ++++++++++++++++++++++++++++ arch/riscv/include/asm/encoding.h | 2 ++ arch/riscv/lib/interrupts.c | 36 +++++++++++++++++++++++-------- 4 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -55,6 +55,11 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
+config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 15e1b8199a..704190f946 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -41,10 +41,18 @@ _start: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) la t0, trap_entry +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
/* mask all interrupts */+#ifdef CONFIG_RISCV_SMODE
csrw sie, zero+#else csrw mie, zero +#endif
/* Enable cache */ jal icache_enable@@ -166,7 +174,11 @@ fix_rela_dyn: */ la t0, trap_entry add t0, t0, t6 +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
clear_bss: la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ @@ -238,17 +250,34 @@ trap_entry: SREG x29, 29*REGBYTES(sp) SREG x30, 30*REGBYTES(sp) SREG x31, 31*REGBYTES(sp) +#ifdef CONFIG_RISCV_SMODE
csrr a0, scausecsrr a1, sepc+#else csrr a0, mcause csrr a1, mepc +#endif mv a2, sp jal handle_trap +#ifdef CONFIG_RISCV_SMODE
csrw sepc, a0+#else csrw mepc, a0 +#endif
+#ifdef CONFIG_RISCV_SMODE +/*
- Remain in S-mode after sret
- */
li t0, SSTATUS_SPPcsrs sstatus, t0+#else /*
- Remain in M-mode after mret
*/ li t0, MSTATUS_MPP csrs mstatus, t0 +#endif
It only the difference between s and m in csr. The code seem to be duplicate too much. Can you implement it in macro ? or consider to separate it in different .S file
It may too complex to maintain in the future.
Here are few reasons why not to do this: 1. We don't have same set of CSRs for M-mode and S-mode. For certain, M-mode CSRs we don't have any corresponding S-mode CSRs. 2. Number of CSRs for each mode are really few so there won't be much #ifdef in code. Having separate .S will be total overkill and too much code duplication. 3. Using macro would mean we use incomplete CSR names examples: "status" for "mstatus"/"sstatus", "epc" for "mepc"/"sepc", etc. This means we cannot grep code for use of a CSR. I think this is less readable compared to using #ifdef
Despite above reasons, above can always be attempted as separate patch.
Regards, Anup
On 27.11.18 07:52, Anup Patel wrote:
On Tue, Nov 27, 2018 at 12:09 PM Rick Chen rickchen36@gmail.com wrote:
Subject: [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs.
It is important to note that there is no equivalent S-mode CSR for misa and mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to emulate misa and mhartid CSR read.
In-future, we will have more patches to avoid accessing misa and mhartid CSRs from S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 5 +++++ arch/riscv/cpu/start.S | 33 ++++++++++++++++++++++++++++ arch/riscv/include/asm/encoding.h | 2 ++ arch/riscv/lib/interrupts.c | 36 +++++++++++++++++++++++-------- 4 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -55,6 +55,11 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
+config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 15e1b8199a..704190f946 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -41,10 +41,18 @@ _start: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) la t0, trap_entry +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
/* mask all interrupts */+#ifdef CONFIG_RISCV_SMODE
csrw sie, zero+#else csrw mie, zero +#endif
/* Enable cache */ jal icache_enable@@ -166,7 +174,11 @@ fix_rela_dyn: */ la t0, trap_entry add t0, t0, t6 +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
clear_bss: la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ @@ -238,17 +250,34 @@ trap_entry: SREG x29, 29*REGBYTES(sp) SREG x30, 30*REGBYTES(sp) SREG x31, 31*REGBYTES(sp) +#ifdef CONFIG_RISCV_SMODE
csrr a0, scausecsrr a1, sepc+#else csrr a0, mcause csrr a1, mepc +#endif mv a2, sp jal handle_trap +#ifdef CONFIG_RISCV_SMODE
csrw sepc, a0+#else csrw mepc, a0 +#endif
+#ifdef CONFIG_RISCV_SMODE +/*
- Remain in S-mode after sret
- */
li t0, SSTATUS_SPPcsrs sstatus, t0+#else /*
- Remain in M-mode after mret
*/ li t0, MSTATUS_MPP csrs mstatus, t0 +#endif
It only the difference between s and m in csr. The code seem to be duplicate too much. Can you implement it in macro ? or consider to separate it in different .S file
It may too complex to maintain in the future.
Here are few reasons why not to do this:
- We don't have same set of CSRs for M-mode and S-mode. For
certain, M-mode CSRs we don't have any corresponding S-mode CSRs. 2. Number of CSRs for each mode are really few so there won't be much #ifdef in code. Having separate .S will be total overkill and too much code duplication. 3. Using macro would mean we use incomplete CSR names examples: "status" for "mstatus"/"sstatus", "epc" for "mepc"/"sepc", etc. This means we cannot grep code for use of a CSR. I think this is less readable compared to using #ifdef
Despite above reasons, above can always be attempted as separate patch.
On AArch64, we determine the current EL during runtime and then branch to respective labels for different ELs (see the switch_el macro for asm as well as current_el() for C).
Wouldn't it make sense to attempt something similar with S-mode, so that the same binary can run either in M or in S depending on how it was started?
Alex
On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf agraf@suse.de wrote:
On 27.11.18 07:52, Anup Patel wrote:
On Tue, Nov 27, 2018 at 12:09 PM Rick Chen rickchen36@gmail.com wrote:
Subject: [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs.
It is important to note that there is no equivalent S-mode CSR for misa and mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to emulate misa and mhartid CSR read.
In-future, we will have more patches to avoid accessing misa and mhartid CSRs from S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 5 +++++ arch/riscv/cpu/start.S | 33 ++++++++++++++++++++++++++++ arch/riscv/include/asm/encoding.h | 2 ++ arch/riscv/lib/interrupts.c | 36 +++++++++++++++++++++++-------- 4 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -55,6 +55,11 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
+config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 15e1b8199a..704190f946 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -41,10 +41,18 @@ _start: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) la t0, trap_entry +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
/* mask all interrupts */+#ifdef CONFIG_RISCV_SMODE
csrw sie, zero+#else csrw mie, zero +#endif
/* Enable cache */ jal icache_enable@@ -166,7 +174,11 @@ fix_rela_dyn: */ la t0, trap_entry add t0, t0, t6 +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
clear_bss: la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ @@ -238,17 +250,34 @@ trap_entry: SREG x29, 29*REGBYTES(sp) SREG x30, 30*REGBYTES(sp) SREG x31, 31*REGBYTES(sp) +#ifdef CONFIG_RISCV_SMODE
csrr a0, scausecsrr a1, sepc+#else csrr a0, mcause csrr a1, mepc +#endif mv a2, sp jal handle_trap +#ifdef CONFIG_RISCV_SMODE
csrw sepc, a0+#else csrw mepc, a0 +#endif
+#ifdef CONFIG_RISCV_SMODE +/*
- Remain in S-mode after sret
- */
li t0, SSTATUS_SPPcsrs sstatus, t0+#else /*
- Remain in M-mode after mret
*/ li t0, MSTATUS_MPP csrs mstatus, t0 +#endif
It only the difference between s and m in csr. The code seem to be duplicate too much. Can you implement it in macro ? or consider to separate it in different .S file
It may too complex to maintain in the future.
Here are few reasons why not to do this:
- We don't have same set of CSRs for M-mode and S-mode. For
certain, M-mode CSRs we don't have any corresponding S-mode CSRs. 2. Number of CSRs for each mode are really few so there won't be much #ifdef in code. Having separate .S will be total overkill and too much code duplication. 3. Using macro would mean we use incomplete CSR names examples: "status" for "mstatus"/"sstatus", "epc" for "mepc"/"sepc", etc. This means we cannot grep code for use of a CSR. I think this is less readable compared to using #ifdef
Despite above reasons, above can always be attempted as separate patch.
On AArch64, we determine the current EL during runtime and then branch to respective labels for different ELs (see the switch_el macro for asm as well as current_el() for C).
Wouldn't it make sense to attempt something similar with S-mode, so that the same binary can run either in M or in S depending on how it was started?
As-per my understand, there is no direct way of knowing current execution mode in RISC-V at runtime.
Of course, software can step from M to S/U using mret OR from S to U using sret. Further software running in U-mode can do ecall to S-mode and S-mode can do ecall to M-mode.
Software is generally written for a particular execution mode (M, S, or U).
Regards, Anup
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午8:38寫道：
On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf agraf@suse.de wrote:
On 27.11.18 07:52, Anup Patel wrote:
On Tue, Nov 27, 2018 at 12:09 PM Rick Chen rickchen36@gmail.com wrote:
Subject: [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs.
It is important to note that there is no equivalent S-mode CSR for misa and mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to emulate misa and mhartid CSR read.
In-future, we will have more patches to avoid accessing misa and mhartid CSRs from S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 5 +++++ arch/riscv/cpu/start.S | 33 ++++++++++++++++++++++++++++ arch/riscv/include/asm/encoding.h | 2 ++ arch/riscv/lib/interrupts.c | 36 +++++++++++++++++++++++-------- 4 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -55,6 +55,11 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
+config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 15e1b8199a..704190f946 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -41,10 +41,18 @@ _start: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) la t0, trap_entry +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
/* mask all interrupts */+#ifdef CONFIG_RISCV_SMODE
csrw sie, zero+#else csrw mie, zero +#endif
/* Enable cache */ jal icache_enable@@ -166,7 +174,11 @@ fix_rela_dyn: */ la t0, trap_entry add t0, t0, t6 +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
clear_bss: la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ @@ -238,17 +250,34 @@ trap_entry: SREG x29, 29*REGBYTES(sp) SREG x30, 30*REGBYTES(sp) SREG x31, 31*REGBYTES(sp) +#ifdef CONFIG_RISCV_SMODE
csrr a0, scausecsrr a1, sepc+#else csrr a0, mcause csrr a1, mepc +#endif mv a2, sp jal handle_trap +#ifdef CONFIG_RISCV_SMODE
csrw sepc, a0+#else csrw mepc, a0 +#endif
+#ifdef CONFIG_RISCV_SMODE +/*
- Remain in S-mode after sret
- */
li t0, SSTATUS_SPPcsrs sstatus, t0+#else /*
- Remain in M-mode after mret
*/ li t0, MSTATUS_MPP csrs mstatus, t0 +#endif
It only the difference between s and m in csr. The code seem to be duplicate too much. Can you implement it in macro ? or consider to separate it in different .S file
It may too complex to maintain in the future.
Here are few reasons why not to do this:
- We don't have same set of CSRs for M-mode and S-mode. For
certain, M-mode CSRs we don't have any corresponding S-mode CSRs. 2. Number of CSRs for each mode are really few so there won't be much #ifdef in code. Having separate .S will be total overkill and too much code duplication. 3. Using macro would mean we use incomplete CSR names examples: "status" for "mstatus"/"sstatus", "epc" for "mepc"/"sepc", etc. This means we cannot grep code for use of a CSR. I think this is less readable compared to using #ifdef
Despite above reasons, above can always be attempted as separate patch.
Hi Anup
I mean it is not a good way to switch s and m mode like that #ifdef CONFIG_RISCV_SMODE csrw sepc, a0 #else csrw mepc, a0 #endif
You can use ## to get stvec ot mtvec in different CONFIG It can be refered to some kernel code as below: glue.h #define ____glue(name,fn) name##fn #define __glue(name,fn) ____glue(name,fn)
glue-proc.h #ifdef CONFIG_CPU_ARM720T #define CPU_NAME cpu_arm720 #else #define CPU_NAME cpu_arm7tdmi #endif
#define cpu_proc_init __glue(CPU_NAME,_proc_init)
Then It will compile as cpu_arm720_proc_init or cpu_arm7tdmi_proc_init in different configuration. stvec and mtvce can be implement by the same way.
B.R Rick
On AArch64, we determine the current EL during runtime and then branch to respective labels for different ELs (see the switch_el macro for asm as well as current_el() for C).
Wouldn't it make sense to attempt something similar with S-mode, so that the same binary can run either in M or in S depending on how it was started?
As-per my understand, there is no direct way of knowing current execution mode in RISC-V at runtime.
Of course, software can step from M to S/U using mret OR from S to U using sret. Further software running in U-mode can do ecall to S-mode and S-mode can do ecall to M-mode.
Software is generally written for a particular execution mode (M, S, or U).
Regards, Anup
On Fri, Nov 30, 2018 at 12:34 PM Rick Chen rickchen36@gmail.com wrote:
Anup Patel anup@brainfault.org 於 2018年11月27日 週二 下午8:38寫道：
On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf agraf@suse.de wrote:
On 27.11.18 07:52, Anup Patel wrote:
On Tue, Nov 27, 2018 at 12:09 PM Rick Chen rickchen36@gmail.com wrote:
> Subject: [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode > > This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this > opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs. > > It is important to note that there is no equivalent S-mode CSR for misa and > mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to > emulate misa and mhartid CSR read. > > In-future, we will have more patches to avoid accessing misa and mhartid CSRs > from S-mode. > > Signed-off-by: Anup Patel anup@brainfault.org > Reviewed-by: Bin Meng bmeng.cn@gmail.com > Tested-by: Bin Meng bmeng.cn@gmail.com > Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de > --- > arch/riscv/Kconfig | 5 +++++ > arch/riscv/cpu/start.S | 33 ++++++++++++++++++++++++++++ > arch/riscv/include/asm/encoding.h | 2 ++ > arch/riscv/lib/interrupts.c | 36 +++++++++++++++++++++++-------- > 4 files changed, 67 insertions(+), 9 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 > 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -55,6 +55,11 @@ config RISCV_ISA_C > config RISCV_ISA_A > def_bool y > > +config RISCV_SMODE > + bool "Run in S-Mode" > + help > + Enable this option to build U-Boot for RISC-V S-Mode > + > config 32BIT > bool > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index > 15e1b8199a..704190f946 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -41,10 +41,18 @@ _start: > li t0, CONFIG_SYS_SDRAM_BASE > SREG a2, 0(t0) > la t0, trap_entry > +#ifdef CONFIG_RISCV_SMODE > + csrw stvec, t0 > +#else > csrw mtvec, t0 > +#endif > > /* mask all interrupts */ > +#ifdef CONFIG_RISCV_SMODE > + csrw sie, zero > +#else > csrw mie, zero > +#endif > > /* Enable cache */ > jal icache_enable > @@ -166,7 +174,11 @@ fix_rela_dyn: > */ > la t0, trap_entry > add t0, t0, t6 > +#ifdef CONFIG_RISCV_SMODE > + csrw stvec, t0 > +#else > csrw mtvec, t0 > +#endif > > clear_bss: > la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ > @@ -238,17 +250,34 @@ trap_entry: > SREG x29, 29*REGBYTES(sp) > SREG x30, 30*REGBYTES(sp) > SREG x31, 31*REGBYTES(sp) > +#ifdef CONFIG_RISCV_SMODE > + csrr a0, scause > + csrr a1, sepc > +#else > csrr a0, mcause > csrr a1, mepc > +#endif > mv a2, sp > jal handle_trap > +#ifdef CONFIG_RISCV_SMODE > + csrw sepc, a0 > +#else > csrw mepc, a0 > +#endif > > +#ifdef CONFIG_RISCV_SMODE > +/* > + * Remain in S-mode after sret > + */ > + li t0, SSTATUS_SPP > + csrs sstatus, t0 > +#else > /* > * Remain in M-mode after mret > */ > li t0, MSTATUS_MPP > csrs mstatus, t0 > +#endif
It only the difference between s and m in csr. The code seem to be duplicate too much. Can you implement it in macro ? or consider to separate it in different .S file
It may too complex to maintain in the future.
Here are few reasons why not to do this:
- We don't have same set of CSRs for M-mode and S-mode. For
certain, M-mode CSRs we don't have any corresponding S-mode CSRs. 2. Number of CSRs for each mode are really few so there won't be much #ifdef in code. Having separate .S will be total overkill and too much code duplication. 3. Using macro would mean we use incomplete CSR names examples: "status" for "mstatus"/"sstatus", "epc" for "mepc"/"sepc", etc. This means we cannot grep code for use of a CSR. I think this is less readable compared to using #ifdef
Despite above reasons, above can always be attempted as separate patch.
Hi Anup
I mean it is not a good way to switch s and m mode like that #ifdef CONFIG_RISCV_SMODE csrw sepc, a0 #else csrw mepc, a0 #endif
You can use ## to get stvec ot mtvec in different CONFIG It can be refered to some kernel code as below: glue.h #define ____glue(name,fn) name##fn #define __glue(name,fn) ____glue(name,fn)
glue-proc.h #ifdef CONFIG_CPU_ARM720T #define CPU_NAME cpu_arm720 #else #define CPU_NAME cpu_arm7tdmi #endif
#define cpu_proc_init __glue(CPU_NAME,_proc_init)
Then It will compile as cpu_arm720_proc_init or cpu_arm7tdmi_proc_init in different configuration. stvec and mtvce can be implement by the same way.
I had understood your suggestion previously.
Though I am not fan of this approach, I will change it anyway.
Thanks, Anup
participants (3)
-
Alexander Graf -
Anup Patel -
Rick Chen