[PATCH 0/5] riscv: enable SBI system reset
The purpose of this series is to provide the UEFI ResetSystem() service at runtime on RISC-V systems.
With SBI v0.3 a system reset extension is available. This allows to implement reboot and poweroff in U-Boot in a system independent way.
OpenSBI already provides an implementation of the extension for the Kendryte K210 and other platforms.
* Provide a system reset driver using the system reset extension. * Enable the driver on the MAIX board. * Change the sbi command to use contants instead of raw numbers where applicable.
Heinrich Schuchardt (5): risv: add missing SBI extension definitions cmd/sbi: use constants instead of numerical values sysreset: provide SBI based sysreset driver pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON maix: enable SBI system reset for MAIX
MAINTAINERS | 1 + arch/riscv/include/asm/sbi.h | 37 +++++++++- arch/riscv/lib/sbi.c | 21 ++++-- board/sipeed/maix/maix.c | 4 ++ cmd/riscv/sbi.c | 30 ++++---- configs/sipeed_maix_smode_defconfig | 2 + drivers/pinctrl/Kconfig | 2 + drivers/sysreset/Kconfig | 7 ++ drivers/sysreset/Makefile | 1 + drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 11 files changed, 187 insertions(+), 22 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
-- 2.30.1
Add the System Reset Extension and the Hart State Management Extension definitions.
Add missing RFENCE Extension enum values.
The SBI 0.1 extension constants are needed for for the sbi command. Remove an #ifdef.
Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 53ca316180..c598bb11ce 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -12,7 +12,6 @@ #include <linux/types.h>
enum sbi_ext_id { -#ifdef CONFIG_SBI_V01 SBI_EXT_0_1_SET_TIMER = 0x0, SBI_EXT_0_1_CONSOLE_PUTCHAR = 0x1, SBI_EXT_0_1_CONSOLE_GETCHAR = 0x2, @@ -22,11 +21,12 @@ enum sbi_ext_id { SBI_EXT_0_1_REMOTE_SFENCE_VMA = 0x6, SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID = 0x7, SBI_EXT_0_1_SHUTDOWN = 0x8, -#endif SBI_EXT_BASE = 0x10, SBI_EXT_TIME = 0x54494D45, SBI_EXT_IPI = 0x735049, SBI_EXT_RFENCE = 0x52464E43, + SBI_EXT_HSM = 0x48534D, + SBI_EXT_SRST = 0x53525354, };
enum sbi_ext_base_fid { @@ -51,6 +51,38 @@ enum sbi_ext_rfence_fid { SBI_EXT_RFENCE_REMOTE_FENCE_I = 0, SBI_EXT_RFENCE_REMOTE_SFENCE_VMA, SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID, + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA, + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, +}; + +enum sbi_ext_hsm_fid { + SBI_EXT_HSM_HART_START = 0, + SBI_EXT_HSM_HART_STOP, + SBI_EXT_HSM_HART_STATUS, +}; + +enum sbi_hsm_hart_status { + SBI_HSM_HART_STATUS_STARTED = 0, + SBI_HSM_HART_STATUS_STOPPED, + SBI_HSM_HART_STATUS_START_PENDING, + SBI_HSM_HART_STATUS_STOP_PENDING, +}; + +enum sbi_ext_srst_fid { + SBI_EXT_SRST_RESET = 0, +}; + +enum sbi_srst_reset_type { + SBI_SRST_RESET_TYPE_SHUTDOWN = 0, + SBI_SRST_RESET_TYPE_COLD_REBOOT, + SBI_SRST_RESET_TYPE_WARM_REBOOT, +}; + +enum sbi_srst_reset_reason { + SBI_SRST_RESET_REASON_NONE = 0, + SBI_SRST_RESET_REASON_SYS_FAILURE, };
#ifdef CONFIG_SBI_V01 -- 2.30.1
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
Add the System Reset Extension and the Hart State Management Extension definitions.
Add missing RFENCE Extension enum values.
The SBI 0.1 extension constants are needed for for the sbi command. Remove an #ifdef.
Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 53ca316180..c598bb11ce 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -12,7 +12,6 @@ #include <linux/types.h>
enum sbi_ext_id { -#ifdef CONFIG_SBI_V01 SBI_EXT_0_1_SET_TIMER = 0x0, SBI_EXT_0_1_CONSOLE_PUTCHAR = 0x1, SBI_EXT_0_1_CONSOLE_GETCHAR = 0x2, @@ -22,11 +21,12 @@ enum sbi_ext_id { SBI_EXT_0_1_REMOTE_SFENCE_VMA = 0x6, SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID = 0x7, SBI_EXT_0_1_SHUTDOWN = 0x8, -#endif SBI_EXT_BASE = 0x10, SBI_EXT_TIME = 0x54494D45, SBI_EXT_IPI = 0x735049, SBI_EXT_RFENCE = 0x52464E43,
SBI_EXT_HSM = 0x48534D,
SBI_EXT_SRST = 0x53525354, };
enum sbi_ext_base_fid {
@@ -51,6 +51,38 @@ enum sbi_ext_rfence_fid { SBI_EXT_RFENCE_REMOTE_FENCE_I = 0, SBI_EXT_RFENCE_REMOTE_SFENCE_VMA, SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID,
- SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA,
- SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID,
- SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
- SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
Isn't this backwards? That is, we should have
+ SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID, + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
so the FIDs match.
--Sean
+};
+enum sbi_ext_hsm_fid {
- SBI_EXT_HSM_HART_START = 0,
- SBI_EXT_HSM_HART_STOP,
- SBI_EXT_HSM_HART_STATUS,
+};
+enum sbi_hsm_hart_status {
- SBI_HSM_HART_STATUS_STARTED = 0,
- SBI_HSM_HART_STATUS_STOPPED,
- SBI_HSM_HART_STATUS_START_PENDING,
- SBI_HSM_HART_STATUS_STOP_PENDING,
+};
+enum sbi_ext_srst_fid {
- SBI_EXT_SRST_RESET = 0,
+};
+enum sbi_srst_reset_type {
- SBI_SRST_RESET_TYPE_SHUTDOWN = 0,
- SBI_SRST_RESET_TYPE_COLD_REBOOT,
- SBI_SRST_RESET_TYPE_WARM_REBOOT,
+};
+enum sbi_srst_reset_reason {
SBI_SRST_RESET_REASON_NONE = 0,
SBI_SRST_RESET_REASON_SYS_FAILURE, };
#ifdef CONFIG_SBI_V01
-- 2.30.1
On 3/5/21 12:28 AM, Sean Anderson wrote:
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
Add the System Reset Extension and the Hart State Management Extension definitions.
Add missing RFENCE Extension enum values.
The SBI 0.1 extension constants are needed for for the sbi command. Remove an #ifdef.
Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 53ca316180..c598bb11ce 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -12,7 +12,6 @@ #include <linux/types.h>
enum sbi_ext_id { -#ifdef CONFIG_SBI_V01 SBI_EXT_0_1_SET_TIMER = 0x0, SBI_EXT_0_1_CONSOLE_PUTCHAR = 0x1, SBI_EXT_0_1_CONSOLE_GETCHAR = 0x2, @@ -22,11 +21,12 @@ enum sbi_ext_id { SBI_EXT_0_1_REMOTE_SFENCE_VMA = 0x6, SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID = 0x7, SBI_EXT_0_1_SHUTDOWN = 0x8, -#endif SBI_EXT_BASE = 0x10, SBI_EXT_TIME = 0x54494D45, SBI_EXT_IPI = 0x735049, SBI_EXT_RFENCE = 0x52464E43, + SBI_EXT_HSM = 0x48534D, + SBI_EXT_SRST = 0x53525354, };
enum sbi_ext_base_fid { @@ -51,6 +51,38 @@ enum sbi_ext_rfence_fid { SBI_EXT_RFENCE_REMOTE_FENCE_I = 0, SBI_EXT_RFENCE_REMOTE_SFENCE_VMA, SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID, + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA, + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
Isn't this backwards? That is, we should have
+ SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID, + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
so the FIDs match.
You are right. I should not have copied from Linux without checking.
Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc#78-functio...
Best regards
Heinrich
--Sean
+};
+enum sbi_ext_hsm_fid { + SBI_EXT_HSM_HART_START = 0, + SBI_EXT_HSM_HART_STOP, + SBI_EXT_HSM_HART_STATUS, +};
+enum sbi_hsm_hart_status { + SBI_HSM_HART_STATUS_STARTED = 0, + SBI_HSM_HART_STATUS_STOPPED, + SBI_HSM_HART_STATUS_START_PENDING, + SBI_HSM_HART_STATUS_STOP_PENDING, +};
+enum sbi_ext_srst_fid { + SBI_EXT_SRST_RESET = 0, +};
+enum sbi_srst_reset_type { + SBI_SRST_RESET_TYPE_SHUTDOWN = 0, + SBI_SRST_RESET_TYPE_COLD_REBOOT, + SBI_SRST_RESET_TYPE_WARM_REBOOT, +};
+enum sbi_srst_reset_reason { + SBI_SRST_RESET_REASON_NONE = 0, + SBI_SRST_RESET_REASON_SYS_FAILURE, };
#ifdef CONFIG_SBI_V01
2.30.1
On Fri, Mar 5, 2021 at 1:02 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Add the System Reset Extension and the Hart State Management Extension definitions.
Add missing RFENCE Extension enum values.
The SBI 0.1 extension constants are needed for for the sbi command. Remove an #ifdef.
Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
U-Boot does not need HSM.
Regards, Bin
On 3/6/21 12:59 AM, Bin Meng wrote:
On Fri, Mar 5, 2021 at 1:02 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Add the System Reset Extension and the Hart State Management Extension definitions.
Add missing RFENCE Extension enum values.
The SBI 0.1 extension constants are needed for for the sbi command. Remove an #ifdef.
Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
U-Boot does not need HSM.
(yet)
As far as I can tell, it's completely conforming for all harts but one to be put into the STOPPED state by the SBI (with the expectation that U-Boot or something else start them). Also, while we currently figure out which harts to start via device tree passed from SBI, that behavior is not standardized. Going forward, I we may see some SBI implementations which communicate this sort of info through HSM. Though this has not happened yet...
--Sean
Use constants for extension IDs.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- cmd/riscv/sbi.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c index 90c0811e14..65a2c93290 100644 --- a/cmd/riscv/sbi.c +++ b/cmd/riscv/sbi.c @@ -29,21 +29,21 @@ static struct sbi_imp implementations[] = { };
static struct sbi_ext extensions[] = { - { 0x00000000, "sbi_set_timer" }, - { 0x00000001, "sbi_console_putchar" }, - { 0x00000002, "sbi_console_getchar" }, - { 0x00000003, "sbi_clear_ipi" }, - { 0x00000004, "sbi_send_ipi" }, - { 0x00000005, "sbi_remote_fence_i" }, - { 0x00000006, "sbi_remote_sfence_vma" }, - { 0x00000007, "sbi_remote_sfence_vma_asid" }, - { 0x00000008, "sbi_shutdown" }, - { 0x00000010, "SBI Base Functionality" }, - { 0x54494D45, "Timer Extension" }, - { 0x00735049, "IPI Extension" }, - { 0x52464E43, "RFENCE Extension" }, - { 0x0048534D, "Hart State Management Extension" }, - { 0x53525354, "System Reset Extension" }, + { SBI_EXT_0_1_SET_TIMER, "sbi_set_timer" }, + { SBI_EXT_0_1_CONSOLE_PUTCHAR, "sbi_console_putchar" }, + { SBI_EXT_0_1_CONSOLE_GETCHAR, "sbi_console_getchar" }, + { SBI_EXT_0_1_CLEAR_IPI, "sbi_clear_ipi" }, + { SBI_EXT_0_1_SEND_IPI, "sbi_send_ipi" }, + { SBI_EXT_0_1_REMOTE_FENCE_I, "sbi_remote_fence_i" }, + { SBI_EXT_0_1_REMOTE_SFENCE_VMA, "sbi_remote_sfence_vma" }, + { SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID, "sbi_remote_sfence_vma_asid" }, + { SBI_EXT_0_1_SHUTDOWN, "sbi_shutdown" }, + { SBI_EXT_BASE, "SBI Base Functionality" }, + { SBI_EXT_TIME, "Timer Extension" }, + { SBI_EXT_IPI, "IPI Extension" }, + { SBI_EXT_RFENCE, "RFENCE Extension" }, + { SBI_EXT_HSM, "Hart State Management Extension" }, + { SBI_EXT_SRST, "System Reset Extension" }, };
static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc, -- 2.30.1
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
Use constants for extension IDs.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
cmd/riscv/sbi.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c index 90c0811e14..65a2c93290 100644 --- a/cmd/riscv/sbi.c +++ b/cmd/riscv/sbi.c @@ -29,21 +29,21 @@ static struct sbi_imp implementations[] = { };
static struct sbi_ext extensions[] = {
- { 0x00000000, "sbi_set_timer" },
- { 0x00000001, "sbi_console_putchar" },
- { 0x00000002, "sbi_console_getchar" },
- { 0x00000003, "sbi_clear_ipi" },
- { 0x00000004, "sbi_send_ipi" },
- { 0x00000005, "sbi_remote_fence_i" },
- { 0x00000006, "sbi_remote_sfence_vma" },
- { 0x00000007, "sbi_remote_sfence_vma_asid" },
- { 0x00000008, "sbi_shutdown" },
- { 0x00000010, "SBI Base Functionality" },
- { 0x54494D45, "Timer Extension" },
- { 0x00735049, "IPI Extension" },
- { 0x52464E43, "RFENCE Extension" },
- { 0x0048534D, "Hart State Management Extension" },
- { 0x53525354, "System Reset Extension" },
{ SBI_EXT_0_1_SET_TIMER, "sbi_set_timer" },
{ SBI_EXT_0_1_CONSOLE_PUTCHAR, "sbi_console_putchar" },
{ SBI_EXT_0_1_CONSOLE_GETCHAR, "sbi_console_getchar" },
{ SBI_EXT_0_1_CLEAR_IPI, "sbi_clear_ipi" },
{ SBI_EXT_0_1_SEND_IPI, "sbi_send_ipi" },
{ SBI_EXT_0_1_REMOTE_FENCE_I, "sbi_remote_fence_i" },
{ SBI_EXT_0_1_REMOTE_SFENCE_VMA, "sbi_remote_sfence_vma" },
{ SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID, "sbi_remote_sfence_vma_asid" },
{ SBI_EXT_0_1_SHUTDOWN, "sbi_shutdown" },
{ SBI_EXT_BASE, "SBI Base Functionality" },
{ SBI_EXT_TIME, "Timer Extension" },
{ SBI_EXT_IPI, "IPI Extension" },
{ SBI_EXT_RFENCE, "RFENCE Extension" },
{ SBI_EXT_HSM, "Hart State Management Extension" },
{ SBI_EXT_SRST, "System Reset Extension" }, };
static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
-- 2.30.1
Reviewed-by: Sean Anderson seanga2@gmail.com
On Thu, Mar 04, 2021 at 05:00:48PM +0000, Heinrich Schuchardt wrote:
Use constants for extension IDs.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
cmd/riscv/sbi.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c index 90c0811e14..65a2c93290 100644 --- a/cmd/riscv/sbi.c +++ b/cmd/riscv/sbi.c @@ -29,21 +29,21 @@ static struct sbi_imp implementations[] = { };
static struct sbi_ext extensions[] = {
- { 0x00000000, "sbi_set_timer" },
- { 0x00000001, "sbi_console_putchar" },
- { 0x00000002, "sbi_console_getchar" },
- { 0x00000003, "sbi_clear_ipi" },
- { 0x00000004, "sbi_send_ipi" },
- { 0x00000005, "sbi_remote_fence_i" },
- { 0x00000006, "sbi_remote_sfence_vma" },
- { 0x00000007, "sbi_remote_sfence_vma_asid" },
- { 0x00000008, "sbi_shutdown" },
- { 0x00000010, "SBI Base Functionality" },
- { 0x54494D45, "Timer Extension" },
- { 0x00735049, "IPI Extension" },
- { 0x52464E43, "RFENCE Extension" },
- { 0x0048534D, "Hart State Management Extension" },
- { 0x53525354, "System Reset Extension" },
- { SBI_EXT_0_1_SET_TIMER, "sbi_set_timer" },
- { SBI_EXT_0_1_CONSOLE_PUTCHAR, "sbi_console_putchar" },
- { SBI_EXT_0_1_CONSOLE_GETCHAR, "sbi_console_getchar" },
- { SBI_EXT_0_1_CLEAR_IPI, "sbi_clear_ipi" },
- { SBI_EXT_0_1_SEND_IPI, "sbi_send_ipi" },
- { SBI_EXT_0_1_REMOTE_FENCE_I, "sbi_remote_fence_i" },
- { SBI_EXT_0_1_REMOTE_SFENCE_VMA, "sbi_remote_sfence_vma" },
- { SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID, "sbi_remote_sfence_vma_asid" },
- { SBI_EXT_0_1_SHUTDOWN, "sbi_shutdown" },
- { SBI_EXT_BASE, "SBI Base Functionality" },
- { SBI_EXT_TIME, "Timer Extension" },
- { SBI_EXT_IPI, "IPI Extension" },
- { SBI_EXT_RFENCE, "RFENCE Extension" },
- { SBI_EXT_HSM, "Hart State Management Extension" },
- { SBI_EXT_SRST, "System Reset Extension" },
};
static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
2.30.1
Reviewed-by: Leo Yu-Chi Liang ycliang@andestech.com
Provide sysreset driver using the SBI system reset extension.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- MAINTAINERS | 1 + arch/riscv/include/asm/sbi.h | 1 + arch/riscv/lib/sbi.c | 21 +++++-- drivers/sysreset/Kconfig | 11 ++++ drivers/sysreset/Makefile | 1 + drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 7 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index de499940e5..192db06ff9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -985,6 +985,7 @@ T: git https://source.denx.de/u-boot/custodians/u-boot-riscv.git F: arch/riscv/ F: cmd/riscv/ F: doc/usage/sbi.rst +F: drivers/sysreset/sysreset_sbi.c F: drivers/timer/andes_plmt_timer.c F: drivers/timer/sifive_clint_timer.c F: tools/prelink-riscv.c diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index c598bb11ce..058e2e23a8 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value); long sbi_get_spec_version(void); int sbi_get_impl_id(void); int sbi_probe_extension(int ext); +void sbi_srst_reset(unsigned long type, unsigned long reason);
#endif diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 77845a73ca..8508041f2a 100644 --- a/arch/riscv/lib/sbi.c +++ b/arch/riscv/lib/sbi.c @@ -8,13 +8,14 @@ */
#include <common.h> +#include <efi_loader.h> #include <asm/encoding.h> #include <asm/sbi.h>
-struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, - unsigned long arg1, unsigned long arg2, - unsigned long arg3, unsigned long arg4, - unsigned long arg5) +struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long arg0, + unsigned long arg1, unsigned long arg2, + unsigned long arg3, unsigned long arg4, + unsigned long arg5) { struct sbiret ret;
@@ -108,6 +109,18 @@ int sbi_probe_extension(int extid) return -ENOTSUPP; }
+/** + * sbi_srst_reset() - invoke system reset extension + * + * @type: type of reset + * @reason: reason for reset + */ +void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long reason) +{ + sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason, + 0, 0, 0, 0); +} + #ifdef CONFIG_SBI_V01
/** diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig index 0e5c7c9971..05a7e26300 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -79,6 +79,17 @@ config SYSRESET_PSCI Enable PSCI SYSTEM_RESET function call. To use this, PSCI firmware must be running on your system.
+config SYSRESET_SBI + bool "Enable support for SBI System Reset" + depends on RISCV_SMODE && SBI_V02 + select SYSRESET_CMD_POWEROFF if CMD_POWEROFF + help + Enable system reset and poweroff via the SBI system reset extension. + If the SBI implemention provides the extension, is board specific. + The extension was introduced in version 0.3 of the SBI specification. + The SBI system reset driver supports the UEFI ResetSystem() service + at runtime. + config SYSRESET_SOCFPGA bool "Enable support for Intel SOCFPGA family" depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10) diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile index de81c399d7..8e00be0779 100644 --- a/drivers/sysreset/Makefile +++ b/drivers/sysreset/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o +obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o diff --git a/drivers/sysreset/sysreset_sbi.c b/drivers/sysreset/sysreset_sbi.c new file mode 100644 index 0000000000..87fbc3ea3e --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2021, Heinrich Schuchardt xypron.glpk@gmx.de + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <efi_loader.h> +#include <log.h> +#include <sysreset.h> +#include <asm/sbi.h> + +static long __efi_runtime_data have_reset; + +static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t type) +{ + enum sbi_srst_reset_type reset_type; + + if (!have_reset) + return -ENOENT; + + switch (type) { + case SYSRESET_WARM: + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT; + break; + case SYSRESET_COLD: + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT; + break; + case SYSRESET_POWER_OFF: + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN; + break; + default: + log_err("SBI has no system reset extension\n"); + return -ENOSYS; + } + + sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE); + + return -EINPROGRESS; +} + +efi_status_t efi_reset_system_init(void) +{ + return EFI_SUCCESS; +} + +void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type, + efi_status_t reset_status, + unsigned long data_size, + void *reset_data) +{ + enum sbi_srst_reset_type reset_type; + + if (have_reset) + switch (type) { + case SYSRESET_WARM: + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT; + break; + case SYSRESET_COLD: + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT; + break; + case SYSRESET_POWER_OFF: + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN; + break; + default: + goto hang; + } + + sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE); + +hang: + while (1) + ; +} + +static int sbi_sysreset_probe(struct udevice *dev) +{ + have_reset = sbi_probe_extension(SBI_EXT_SRST); + if (have_reset) + return 0; + + log_err("SBI has no system reset extension\n"); + return -ENOENT; +} + +static const struct udevice_id sbi_sysreset_ids[] = { + { .compatible = "riscv" }, + { } +}; + +static struct sysreset_ops sbi_sysreset_ops = { + .request = sbi_sysreset_request, +}; + +U_BOOT_DRIVER(sbi_sysreset) = { + .name = "sbi-sysreset", + .id = UCLASS_SYSRESET, + .of_match = sbi_sysreset_ids, + .ops = &sbi_sysreset_ops, + .probe = sbi_sysreset_probe, +}; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e729f727df..7e76435339 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET bool default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \ - SANDBOX || SYSRESET_X86 + SANDBOX || SYSRESET_SBI || SYSRESET_X86
config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM" -- 2.30.1
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
Provide sysreset driver using the SBI system reset extension.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
MAINTAINERS | 1 + arch/riscv/include/asm/sbi.h | 1 + arch/riscv/lib/sbi.c | 21 +++++-- drivers/sysreset/Kconfig | 11 ++++ drivers/sysreset/Makefile | 1 + drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 7 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index de499940e5..192db06ff9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -985,6 +985,7 @@ T: git https://source.denx.de/u-boot/custodians/u-boot-riscv.git F: arch/riscv/ F: cmd/riscv/ F: doc/usage/sbi.rst +F: drivers/sysreset/sysreset_sbi.c F: drivers/timer/andes_plmt_timer.c F: drivers/timer/sifive_clint_timer.c F: tools/prelink-riscv.c diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index c598bb11ce..058e2e23a8 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value); long sbi_get_spec_version(void); int sbi_get_impl_id(void); int sbi_probe_extension(int ext); +void sbi_srst_reset(unsigned long type, unsigned long reason);
#endif diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 77845a73ca..8508041f2a 100644 --- a/arch/riscv/lib/sbi.c +++ b/arch/riscv/lib/sbi.c @@ -8,13 +8,14 @@ */
#include <common.h> +#include <efi_loader.h> #include <asm/encoding.h> #include <asm/sbi.h>
-struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
unsigned long arg1, unsigned long arg2,unsigned long arg3, unsigned long arg4,unsigned long arg5)+struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long arg0,
unsigned long arg1, unsigned long arg2,unsigned long arg3, unsigned long arg4,unsigned long arg5)@@ -108,6 +109,18 @@ int sbi_probe_extension(int extid) return -ENOTSUPP; }
+/**
- sbi_srst_reset() - invoke system reset extension
- @type: type of reset
- @reason: reason for reset
- */
+void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long reason) +{
- sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason,
0, 0, 0, 0);+}
#ifdef CONFIG_SBI_V01
/**
diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig index 0e5c7c9971..05a7e26300 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -79,6 +79,17 @@ config SYSRESET_PSCI Enable PSCI SYSTEM_RESET function call. To use this, PSCI firmware must be running on your system.
+config SYSRESET_SBI
- bool "Enable support for SBI System Reset"
- depends on RISCV_SMODE && SBI_V02
- select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
- help
Enable system reset and poweroff via the SBI system reset extension.If the SBI implemention provides the extension, is board specific.The extension was introduced in version 0.3 of the SBI specification.The SBI system reset driver supports the UEFI ResetSystem() serviceat runtime.- config SYSRESET_SOCFPGA bool "Enable support for Intel SOCFPGA family" depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10)
diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile index de81c399d7..8e00be0779 100644 --- a/drivers/sysreset/Makefile +++ b/drivers/sysreset/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o +obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o diff --git a/drivers/sysreset/sysreset_sbi.c b/drivers/sysreset/sysreset_sbi.c new file mode 100644 index 0000000000..87fbc3ea3e --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2021, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <efi_loader.h> +#include <log.h> +#include <sysreset.h> +#include <asm/sbi.h>
+static long __efi_runtime_data have_reset;
+static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t type) +{
- enum sbi_srst_reset_type reset_type;
- if (!have_reset)
return -ENOENT;
Is this necessary? This should never be called if there is no reset, since we just fail to probe.
- switch (type) {
- case SYSRESET_WARM:
reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;break;- case SYSRESET_COLD:
reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;break;- case SYSRESET_POWER_OFF:
reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;break;- default:
log_err("SBI has no system reset extension\n");return -ENOSYS;- }
- sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
- return -EINPROGRESS;
+}
+efi_status_t efi_reset_system_init(void) +{
- return EFI_SUCCESS;
+}
+void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type,
efi_status_t reset_status,unsigned long data_size,void *reset_data)
As an aside, is there a reason why the generic (weak) efi_reset_system does not just call sysreset_walk_halt(type)?
+{
- enum sbi_srst_reset_type reset_type;
- if (have_reset)
switch (type) {case SYSRESET_WARM:reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;break;case SYSRESET_COLD:reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;break;case SYSRESET_POWER_OFF:reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;break;default:goto hang;
Why do we hang by default? For comparison, sysreset_x86 has
if (reset_type == EFI_RESET_COLD || reset_type == EFI_RESET_PLATFORM_SPECIFIC) value = SYS_RST | RST_CPU | FULL_RST; else /* assume EFI_RESET_WARM since we cannot return an error */ value = SYS_RST | RST_CPU;
- }
- sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
Should we instead do
enum sbi_srst_reset_reason reset_reason; if (reset_status == EFI_SUCCESS) reset_reason = SBI_SRST_RESET_REASON_NONE; else reset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE; sbi_srst_reset(reset_type, reset_status == EFI_SUCCESS ? SBI_SRST_RESET_REASON_NONE : SBI_SRST_RESET_REASON_SYS_FAILURE);
since we have reset status available?
+hang:
- while (1)
;+}
+static int sbi_sysreset_probe(struct udevice *dev) +{
- have_reset = sbi_probe_extension(SBI_EXT_SRST);
- if (have_reset)
return 0;- log_err("SBI has no system reset extension\n");
Is this an error? It's perfectly normal for SBI implementations to lack support for some extensions. Perhaps a warning/info would be better.
- return -ENOENT;
+}
+static const struct udevice_id sbi_sysreset_ids[] = {
- { .compatible = "riscv" },
This compatible string is already in-use for RISC-V CPUs. And if this isn't supposed to have a device tree binding, do we even need of_match?
- { }
+};
+static struct sysreset_ops sbi_sysreset_ops = {
- .request = sbi_sysreset_request,
+};
+U_BOOT_DRIVER(sbi_sysreset) = {
- .name = "sbi-sysreset",
- .id = UCLASS_SYSRESET,
- .of_match = sbi_sysreset_ids,
- .ops = &sbi_sysreset_ops,
- .probe = sbi_sysreset_probe,
+}; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e729f727df..7e76435339 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET bool default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
SANDBOX || SYSRESET_X86
SANDBOX || SYSRESET_SBI || SYSRESET_X86
As a general note, perhaps it is better to set this from other Kconfigs? How long will this list get?
--Sean
config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM" -- 2.30.1
On 3/5/21 12:50 AM, Sean Anderson wrote:
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
Provide sysreset driver using the SBI system reset extension.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
MAINTAINERS | 1 + arch/riscv/include/asm/sbi.h | 1 + arch/riscv/lib/sbi.c | 21 +++++-- drivers/sysreset/Kconfig | 11 ++++ drivers/sysreset/Makefile | 1 + drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 7 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index de499940e5..192db06ff9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -985,6 +985,7 @@ T: git https://source.denx.de/u-boot/custodians/u-boot-riscv.git F: arch/riscv/ F: cmd/riscv/ F: doc/usage/sbi.rst +F: drivers/sysreset/sysreset_sbi.c F: drivers/timer/andes_plmt_timer.c F: drivers/timer/sifive_clint_timer.c F: tools/prelink-riscv.c diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index c598bb11ce..058e2e23a8 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value); long sbi_get_spec_version(void); int sbi_get_impl_id(void); int sbi_probe_extension(int ext); +void sbi_srst_reset(unsigned long type, unsigned long reason);
#endif diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 77845a73ca..8508041f2a 100644 --- a/arch/riscv/lib/sbi.c +++ b/arch/riscv/lib/sbi.c @@ -8,13 +8,14 @@ */
#include <common.h> +#include <efi_loader.h> #include <asm/encoding.h> #include <asm/sbi.h>
-struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, - unsigned long arg1, unsigned long arg2, - unsigned long arg3, unsigned long arg4, - unsigned long arg5) +struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long arg0, + unsigned long arg1, unsigned long arg2, + unsigned long arg3, unsigned long arg4, + unsigned long arg5) { struct sbiret ret;
@@ -108,6 +109,18 @@ int sbi_probe_extension(int extid) return -ENOTSUPP; }
+/**
- sbi_srst_reset() - invoke system reset extension
- @type: type of reset
- @reason: reason for reset
- */
+void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long reason) +{ + sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason, + 0, 0, 0, 0); +}
#ifdef CONFIG_SBI_V01
/** diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig index 0e5c7c9971..05a7e26300 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -79,6 +79,17 @@ config SYSRESET_PSCI Enable PSCI SYSTEM_RESET function call. To use this, PSCI firmware must be running on your system.
+config SYSRESET_SBI + bool "Enable support for SBI System Reset" + depends on RISCV_SMODE && SBI_V02 + select SYSRESET_CMD_POWEROFF if CMD_POWEROFF + help + Enable system reset and poweroff via the SBI system reset extension. + If the SBI implemention provides the extension, is board specific. + The extension was introduced in version 0.3 of the SBI specification. + The SBI system reset driver supports the UEFI ResetSystem() service + at runtime.
config SYSRESET_SOCFPGA bool "Enable support for Intel SOCFPGA family" depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10) diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile index de81c399d7..8e00be0779 100644 --- a/drivers/sysreset/Makefile +++ b/drivers/sysreset/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o +obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o diff --git a/drivers/sysreset/sysreset_sbi.c b/drivers/sysreset/sysreset_sbi.c new file mode 100644 index 0000000000..87fbc3ea3e --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2021, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <efi_loader.h> +#include <log.h> +#include <sysreset.h> +#include <asm/sbi.h>
+static long __efi_runtime_data have_reset;
+static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t type) +{ + enum sbi_srst_reset_type reset_type;
+ if (!have_reset) + return -ENOENT;
Is this necessary? This should never be called if there is no reset, since we just fail to probe.
Thank you for reviewing.
Yes, we can skip it.
+ switch (type) { + case SYSRESET_WARM: + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT; + break; + case SYSRESET_COLD: + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT; + break; + case SYSRESET_POWER_OFF: + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN; + break; + default: + log_err("SBI has no system reset extension\n"); + return -ENOSYS; + }
+ sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
+ return -EINPROGRESS; +}
+efi_status_t efi_reset_system_init(void) +{ + return EFI_SUCCESS; +}
+void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type, + efi_status_t reset_status, + unsigned long data_size, + void *reset_data)
As an aside, is there a reason why the generic (weak) efi_reset_system does not just call sysreset_walk_halt(type)?
efi_reset_system is invoked at UEFI runtime. Most functions of U-Boot are no longer in memory after ExitBootServices(). Only functions in the __efi_runtime section may be called.
+{ + enum sbi_srst_reset_type reset_type;
+ if (have_reset) + switch (type) { + case SYSRESET_WARM: + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT; + break; + case SYSRESET_COLD: + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT; + break; + case SYSRESET_POWER_OFF: + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN; + break; + default: + goto hang;
Why do we hang by default? For comparison, sysreset_x86 has
if (reset_type == EFI_RESET_COLD || reset_type == EFI_RESET_PLATFORM_SPECIFIC) value = SYS_RST | RST_CPU | FULL_RST; else /* assume EFI_RESET_WARM since we cannot return an error */ value = SYS_RST | RST_CPU;
ok
+ }
+ sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
Should we instead do
enum sbi_srst_reset_reason reset_reason; if (reset_status == EFI_SUCCESS) reset_reason = SBI_SRST_RESET_REASON_NONE; else reset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE; sbi_srst_reset(reset_type, reset_status == EFI_SUCCESS ? SBI_SRST_RESET_REASON_NONE : SBI_SRST_RESET_REASON_SYS_FAILURE);
since we have reset status available?
Makes sense.
+hang: + while (1) + ; +}
+static int sbi_sysreset_probe(struct udevice *dev) +{ + have_reset = sbi_probe_extension(SBI_EXT_SRST); + if (have_reset) + return 0;
+ log_err("SBI has no system reset extension\n");
Is this an error? It's perfectly normal for SBI implementations to lack support for some extensions. Perhaps a warning/info would be better.
We shouldn't have chosen this driver in the configuration if the SBI does not support it.
I will change this to a warning.
+ return -ENOENT; +}
+static const struct udevice_id sbi_sysreset_ids[] = { + { .compatible = "riscv" },
This compatible string is already in-use for RISC-V CPUs. And if this isn't supposed to have a device tree binding, do we even need of_match?
I discussed this with Simon in
https://lists.denx.de/pipermail/u-boot/2021-March/443270.html
Instead of adding code to the board files we should have a device-tree node. A reasonable compatible string would be "riscv,sbi-sysreset".
+ { } +};
+static struct sysreset_ops sbi_sysreset_ops = { + .request = sbi_sysreset_request, +};
+U_BOOT_DRIVER(sbi_sysreset) = { + .name = "sbi-sysreset", + .id = UCLASS_SYSRESET, + .of_match = sbi_sysreset_ids, + .ops = &sbi_sysreset_ops, + .probe = sbi_sysreset_probe, +}; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e729f727df..7e76435339 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET bool default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \ - SANDBOX || SYSRESET_X86 + SANDBOX || SYSRESET_SBI || SYSRESET_X86
As a general note, perhaps it is better to set this from other Kconfigs? How long will this list get?
This seems to be a matter of taste.
Best regards
Heinrich
--Sean
config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM" -- 2.30.1
On 3/6/21 2:18 AM, Heinrich Schuchardt wrote:
On 3/5/21 12:50 AM, Sean Anderson wrote:
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
Provide sysreset driver using the SBI system reset extension.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
MAINTAINERS | 1 + arch/riscv/include/asm/sbi.h | 1 + arch/riscv/lib/sbi.c | 21 +++++-- drivers/sysreset/Kconfig | 11 ++++ drivers/sysreset/Makefile | 1 + drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 7 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index de499940e5..192db06ff9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -985,6 +985,7 @@ T: git https://source.denx.de/u-boot/custodians/u-boot-riscv.git F: arch/riscv/ F: cmd/riscv/ F: doc/usage/sbi.rst +F: drivers/sysreset/sysreset_sbi.c F: drivers/timer/andes_plmt_timer.c F: drivers/timer/sifive_clint_timer.c F: tools/prelink-riscv.c diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index c598bb11ce..058e2e23a8 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value); long sbi_get_spec_version(void); int sbi_get_impl_id(void); int sbi_probe_extension(int ext); +void sbi_srst_reset(unsigned long type, unsigned long reason);
#endif diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 77845a73ca..8508041f2a 100644 --- a/arch/riscv/lib/sbi.c +++ b/arch/riscv/lib/sbi.c @@ -8,13 +8,14 @@ */
#include <common.h> +#include <efi_loader.h> #include <asm/encoding.h> #include <asm/sbi.h>
-struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
unsigned long arg1, unsigned long arg2,unsigned long arg3, unsigned long arg4,unsigned long arg5)+struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long arg0,
unsigned long arg1, unsigned long arg2,unsigned long arg3, unsigned long arg4,unsigned long arg5)@@ -108,6 +109,18 @@ int sbi_probe_extension(int extid) return -ENOTSUPP; }
+/**
- sbi_srst_reset() - invoke system reset extension
- @type: type of reset
- @reason: reason for reset
- */
+void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long reason) +{
- sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason,
0, 0, 0, 0);+}
#ifdef CONFIG_SBI_V01
/**
diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig index 0e5c7c9971..05a7e26300 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -79,6 +79,17 @@ config SYSRESET_PSCI Enable PSCI SYSTEM_RESET function call. To use this, PSCI firmware must be running on your system.
+config SYSRESET_SBI
- bool "Enable support for SBI System Reset"
- depends on RISCV_SMODE && SBI_V02
- select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
- help
Enable system reset and poweroff via the SBI system resetextension.
If the SBI implemention provides the extension, is board specific.The extension was introduced in version 0.3 of the SBIspecification.
The SBI system reset driver supports the UEFI ResetSystem()service
at runtime.- config SYSRESET_SOCFPGA bool "Enable support for Intel SOCFPGA family" depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 ||
TARGET_SOCFPGA_ARRIA10) diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile index de81c399d7..8e00be0779 100644 --- a/drivers/sysreset/Makefile +++ b/drivers/sysreset/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o +obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o diff --git a/drivers/sysreset/sysreset_sbi.c b/drivers/sysreset/sysreset_sbi.c new file mode 100644 index 0000000000..87fbc3ea3e --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2021, Heinrich Schuchardt xypron.glpk@gmx.de
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <efi_loader.h> +#include <log.h> +#include <sysreset.h> +#include <asm/sbi.h>
+static long __efi_runtime_data have_reset;
+static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t type) +{
- enum sbi_srst_reset_type reset_type;
- if (!have_reset)
return -ENOENT;Is this necessary? This should never be called if there is no reset, since we just fail to probe.
Thank you for reviewing.
Yes, we can skip it.
- switch (type) {
- case SYSRESET_WARM:
reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;break;- case SYSRESET_COLD:
reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;break;- case SYSRESET_POWER_OFF:
reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;break;- default:
log_err("SBI has no system reset extension\n");return -ENOSYS;- }
- sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
- return -EINPROGRESS;
+}
+efi_status_t efi_reset_system_init(void) +{
- return EFI_SUCCESS;
+}
+void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type,
efi_status_t reset_status,unsigned long data_size,void *reset_data)As an aside, is there a reason why the generic (weak) efi_reset_system does not just call sysreset_walk_halt(type)?
efi_reset_system is invoked at UEFI runtime. Most functions of U-Boot are no longer in memory after ExitBootServices(). Only functions in the __efi_runtime section may be called.
+{
- enum sbi_srst_reset_type reset_type;
- if (have_reset)
switch (type) {case SYSRESET_WARM:reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;break;case SYSRESET_COLD:reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;break;case SYSRESET_POWER_OFF:reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;break;default:goto hang;Why do we hang by default? For comparison, sysreset_x86 has
if (reset_type == EFI_RESET_COLD || reset_type == EFI_RESET_PLATFORM_SPECIFIC) value = SYS_RST | RST_CPU | FULL_RST; else /* assume EFI_RESET_WARM since we cannot return an error */ value = SYS_RST | RST_CPU;ok
- }
- sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
Should we instead do
enum sbi_srst_reset_reason reset_reason; if (reset_status == EFI_SUCCESS) reset_reason = SBI_SRST_RESET_REASON_NONE; else reset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE; sbi_srst_reset(reset_type, reset_status == EFI_SUCCESS ?SBI_SRST_RESET_REASON_NONE : SBI_SRST_RESET_REASON_SYS_FAILURE);
since we have reset status available?
Makes sense.
+hang:
- while (1)
;+}
+static int sbi_sysreset_probe(struct udevice *dev) +{
- have_reset = sbi_probe_extension(SBI_EXT_SRST);
- if (have_reset)
return 0;- log_err("SBI has no system reset extension\n");
Is this an error? It's perfectly normal for SBI implementations to lack support for some extensions. Perhaps a warning/info would be better.
We shouldn't have chosen this driver in the configuration if the SBI does not support it.
Yes, but we would like to use the same config for differing SBI implementations (e.g. say a vendor's SBI implementation and SBI).
I will change this to a warning.
Great.
- return -ENOENT;
+}
+static const struct udevice_id sbi_sysreset_ids[] = {
- { .compatible = "riscv" },
This compatible string is already in-use for RISC-V CPUs. And if this isn't supposed to have a device tree binding, do we even need of_match?
I discussed this with Simon in
https://lists.denx.de/pipermail/u-boot/2021-March/443270.html
Instead of adding code to the board files we should have a device-tree node. A reasonable compatible string would be "riscv,sbi-sysreset".
Ok, so will patch 5 have the board parts dropped?
--Sean
- { }
+};
+static struct sysreset_ops sbi_sysreset_ops = {
- .request = sbi_sysreset_request,
+};
+U_BOOT_DRIVER(sbi_sysreset) = {
- .name = "sbi-sysreset",
- .id = UCLASS_SYSRESET,
- .of_match = sbi_sysreset_ids,
- .ops = &sbi_sysreset_ops,
- .probe = sbi_sysreset_probe,
+}; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e729f727df..7e76435339 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET bool default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
SANDBOX || SYSRESET_X86
SANDBOX || SYSRESET_SBI || SYSRESET_X86As a general note, perhaps it is better to set this from other Kconfigs? How long will this list get?
This seems to be a matter of taste.
Best regards
Heinrich
--Sean
config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM" -- 2.30.1
Select missing Kconfig dependencies.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- drivers/pinctrl/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 77fb851114..2a859ab5d0 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -294,6 +294,8 @@ config ASPEED_AST2500_PINCTRL config PINCTRL_K210 bool "Kendryte K210 Fully-Programmable Input/Output Array driver" depends on DM && PINCTRL_GENERIC + select REGMAP + select SYSCON help Support pin multiplexing on the K210. The "FPIOA" can remap any supported function to any multifunctional IO pin. It can also perform -- 2.30.1
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
Select missing Kconfig dependencies.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
drivers/pinctrl/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 77fb851114..2a859ab5d0 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -294,6 +294,8 @@ config ASPEED_AST2500_PINCTRL config PINCTRL_K210 bool "Kendryte K210 Fully-Programmable Input/Output Array driver" depends on DM && PINCTRL_GENERIC
- select REGMAP
- select SYSCON help Support pin multiplexing on the K210. The "FPIOA" can remap any supported function to any multifunctional IO pin. It can also perform
-- 2.30.1
Reviewed-by: Sean Anderson seanga2@gmail.com
On Thu, Mar 04, 2021 at 05:00:50PM +0000, Heinrich Schuchardt wrote:
Select missing Kconfig dependencies.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
drivers/pinctrl/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 77fb851114..2a859ab5d0 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -294,6 +294,8 @@ config ASPEED_AST2500_PINCTRL config PINCTRL_K210 bool "Kendryte K210 Fully-Programmable Input/Output Array driver" depends on DM && PINCTRL_GENERIC
- select REGMAP
- select SYSCON help Support pin multiplexing on the K210. The "FPIOA" can remap any supported function to any multifunctional IO pin. It can also perform
-- 2.30.1
Reviewed-by: Leo Yu-Chi Liang ycliang@andestech.com
When running in S-mode we can use the SBI system reset extension to provide the system reset.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- board/sipeed/maix/maix.c | 5 +++++ configs/sipeed_maix_smode_defconfig | 2 ++ 2 files changed, 7 insertions(+)
diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c index cbcb23cf5c..388eddee6e 100644 --- a/board/sipeed/maix/maix.c +++ b/board/sipeed/maix/maix.c @@ -6,6 +6,7 @@ #include <common.h> #include <clk.h> #include <dm.h> +#include <dm/lists.h> #include <fdt_support.h> #include <asm/io.h>
@@ -37,5 +38,9 @@ int board_init(void) return ret; }
+ if (IS_ENABLED(CONFIG_SYSRESET_SBI)) + ret = device_bind_driver(gd->dm_root, "sbi-sysreset", + "sbi-sysreset", NULL); + return 0; } diff --git a/configs/sipeed_maix_smode_defconfig b/configs/sipeed_maix_smode_defconfig index 2516bb7258..aa95893feb 100644 --- a/configs/sipeed_maix_smode_defconfig +++ b/configs/sipeed_maix_smode_defconfig @@ -7,4 +7,6 @@ CONFIG_STACK_SIZE=0x100000 # CONFIG_NET is not set # CONFIG_INPUT is not set # CONFIG_DM_ETH is not set +CONFIG_SYSRESET_SBI=y +# CONFIG_SYSRESET_SYSCON is not set # CONFIG_EFI_UNICODE_CAPITALIZATION is not set -- 2.30.1
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
When running in S-mode we can use the SBI system reset extension to provide the system reset.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
board/sipeed/maix/maix.c | 5 +++++ configs/sipeed_maix_smode_defconfig | 2 ++ 2 files changed, 7 insertions(+)
diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c index cbcb23cf5c..388eddee6e 100644 --- a/board/sipeed/maix/maix.c +++ b/board/sipeed/maix/maix.c @@ -6,6 +6,7 @@ #include <common.h> #include <clk.h> #include <dm.h> +#include <dm/lists.h> #include <fdt_support.h> #include <asm/io.h>
@@ -37,5 +38,9 @@ int board_init(void) return ret; }
- if (IS_ENABLED(CONFIG_SYSRESET_SBI))
ret = device_bind_driver(gd->dm_root, "sbi-sysreset","sbi-sysreset", NULL);
Besides that this isn't a device-tree driver, shouldn't this live in arch_early_init_r or similar? This isn't really board-specific.
--Sean
return 0; } diff --git a/configs/sipeed_maix_smode_defconfig b/configs/sipeed_maix_smode_defconfig index 2516bb7258..aa95893feb 100644 --- a/configs/sipeed_maix_smode_defconfig +++ b/configs/sipeed_maix_smode_defconfig @@ -7,4 +7,6 @@ CONFIG_STACK_SIZE=0x100000 # CONFIG_NET is not set # CONFIG_INPUT is not set # CONFIG_DM_ETH is not set +CONFIG_SYSRESET_SBI=y +# CONFIG_SYSRESET_SYSCON is not set
# CONFIG_EFI_UNICODE_CAPITALIZATION is not set
2.30.1
On 3/5/21 12:24 AM, Sean Anderson wrote:
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
When running in S-mode we can use the SBI system reset extension to provide the system reset.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
board/sipeed/maix/maix.c | 5 +++++ configs/sipeed_maix_smode_defconfig | 2 ++ 2 files changed, 7 insertions(+)
diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c index cbcb23cf5c..388eddee6e 100644 --- a/board/sipeed/maix/maix.c +++ b/board/sipeed/maix/maix.c @@ -6,6 +6,7 @@ #include <common.h> #include <clk.h> #include <dm.h> +#include <dm/lists.h> #include <fdt_support.h> #include <asm/io.h>
@@ -37,5 +38,9 @@ int board_init(void) return ret; }
+ if (IS_ENABLED(CONFIG_SYSRESET_SBI)) + ret = device_bind_driver(gd->dm_root, "sbi-sysreset", + "sbi-sysreset", NULL);
Besides that this isn't a device-tree driver, shouldn't this live in arch_early_init_r or similar? This isn't really board-specific.
Using arch_early_init_r() seems to be a viable alternative to putting some dummy node into each device-tree.
Best regards
Heinrich
--Sean
return 0; } diff --git a/configs/sipeed_maix_smode_defconfig b/configs/sipeed_maix_smode_defconfig index 2516bb7258..aa95893feb 100644 --- a/configs/sipeed_maix_smode_defconfig +++ b/configs/sipeed_maix_smode_defconfig @@ -7,4 +7,6 @@ CONFIG_STACK_SIZE=0x100000 # CONFIG_NET is not set # CONFIG_INPUT is not set # CONFIG_DM_ETH is not set +CONFIG_SYSRESET_SBI=y +# CONFIG_SYSRESET_SYSCON is not set # CONFIG_EFI_UNICODE_CAPITALIZATION is not set -- 2.30.1
participants (4)
-
Bin Meng -
Heinrich Schuchardt -
Leo Liang -
Sean Anderson