[PATCH v3 0/3] 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.
* Provide missing constants * Provide a system reset driver using the system reset extension. * Provide a UEFI runtime implementation for system reset
v3: add SBI_HSM_HART_STATUS_SUSPENDED, SBI_HSM_HART_STATUS_SUSPEND_PENDING, SBI_HSM_HART_STATUS_RESUME_PENDING v2: correct constants that were blindly copied from Linux
Heinrich Schuchardt (3): riscv: add missing SBI extension definitions cmd/sbi: use constants instead of numerical values sysreset: provide SBI based sysreset driver
MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- arch/riscv/include/asm/sbi.h | 41 +++++++++++++- arch/riscv/lib/sbi.c | 21 ++++++-- cmd/riscv/sbi.c | 30 +++++------ drivers/sysreset/Kconfig | 11 ++++ drivers/sysreset/Makefile | 1 + drivers/sysreset/sysreset_sbi.c | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 9 files changed, 193 insertions(+), 23 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
-- 2.30.2
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 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 --- v3: add SBI_HSM_HART_STATUS_SUSPENDED, SBI_HSM_HART_STATUS_SUSPEND_PENDING, SBI_HSM_HART_STATUS_RESUME_PENDING v2: correct constants that were blindly copied from Linux --- arch/riscv/include/asm/sbi.h | 40 ++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 53ca316180..e9caa78d17 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,42 @@ 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_VMID, + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA, +}; + +enum sbi_ext_hsm_fid { + SBI_EXT_HSM_HART_START = 0, + SBI_EXT_HSM_HART_STOP, + SBI_EXT_HSM_HART_STATUS, + SBI_EXT_HSM_HART_SUSPEND, +}; + +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, + SBI_HSM_HART_STATUS_SUSPENDED, + SBI_HSM_HART_STATUS_SUSPEND_PENDING, + SBI_HSM_HART_STATUS_RESUME_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.2
On Sun, Sep 5, 2021 at 4:38 PM 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 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
v3: add SBI_HSM_HART_STATUS_SUSPENDED, SBI_HSM_HART_STATUS_SUSPEND_PENDING, SBI_HSM_HART_STATUS_RESUME_PENDING v2: correct constants that were blindly copied from Linux
arch/riscv/include/asm/sbi.h | 40 ++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Use constants for extension IDs.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Sean Anderson seanga2@gmail.com Reviewed-by: Leo Yu-Chi Liang ycliang@andestech.com --- v3: no change --- 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.2
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Use constants for extension IDs.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Sean Anderson seanga2@gmail.com Reviewed-by: Leo Yu-Chi Liang ycliang@andestech.com
v3: no change
cmd/riscv/sbi.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Provide sysreset driver using the SBI system reset extension.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3: no change --- MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- 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 | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..88d7aa2bc7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b5..8e49b6d736 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,7 @@ #include <common.h> #include <cpu.h> #include <dm.h> +#include <dm/lists.h> #include <init.h> #include <log.h> #include <asm/encoding.h> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void)
int arch_early_init_r(void) { - return riscv_cpu_probe(); + int ret; + + ret = riscv_cpu_probe(); + if (ret) + return ret; + + if (IS_ENABLED(CONFIG_SYSRESET_SBI)) + device_bind_driver(gd->dm_root, "sbi-sysreset", + "sbi-sysreset", NULL); + + return 0; }
/** diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index e9caa78d17..69cddda245 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -85,6 +85,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 implementation 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..fec5a66515 --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,96 @@ +// 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; + + 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; + enum sbi_srst_reset_reason reset_reason; + + if (have_reset) + switch (type) { + 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: + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT; + break; + } + + 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_reason); + + while (1) + ; +} + +static int sbi_sysreset_probe(struct udevice *dev) +{ + have_reset = sbi_probe_extension(SBI_EXT_SRST); + if (have_reset) + return 0; + + log_warning("SBI has no system reset extension\n"); + return -ENOENT; +} + +static struct sysreset_ops sbi_sysreset_ops = { + .request = sbi_sysreset_request, +}; + +U_BOOT_DRIVER(sbi_sysreset) = { + .name = "sbi-sysreset", + .id = UCLASS_SYSRESET, + .ops = &sbi_sysreset_ops, + .probe = sbi_sysreset_probe, +}; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index dacc3b5881..36985fce2c 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -297,7 +297,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.2
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide sysreset driver using the SBI system reset extension.
This patch should be split into 2 patches, one for adding the sysreset DM driver, and the other one for EFI support.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: no change
MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- 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 | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..88d7aa2bc7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b5..8e49b6d736 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,7 @@ #include <common.h> #include <cpu.h> #include <dm.h> +#include <dm/lists.h> #include <init.h> #include <log.h> #include <asm/encoding.h> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void)
int arch_early_init_r(void) {
return riscv_cpu_probe();
int ret;ret = riscv_cpu_probe();if (ret)return ret;if (IS_ENABLED(CONFIG_SYSRESET_SBI))device_bind_driver(gd->dm_root, "sbi-sysreset","sbi-sysreset", NULL);return 0;}
/** diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index e9caa78d17..69cddda245 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -85,6 +85,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_V02select SYSRESET_CMD_POWEROFF if CMD_POWEROFFhelpEnable system reset and poweroff via the SBI system reset extension.If the SBI implementation 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..fec5a66515 --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,96 @@ +// 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;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;+}
Is there a better place for the EFI stuff?
+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;enum sbi_srst_reset_reason reset_reason;if (have_reset)switch (type) {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:reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;break;}if (reset_status == EFI_SUCCESS)reset_reason = SBI_SRST_RESET_REASON_NONE;elsereset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE;sbi_srst_reset(reset_type, reset_reason);while (1);+}
+static int sbi_sysreset_probe(struct udevice *dev) +{
have_reset = sbi_probe_extension(SBI_EXT_SRST);if (have_reset)return 0;log_warning("SBI has no system reset extension\n");return -ENOENT;+}
+static struct sysreset_ops sbi_sysreset_ops = {
.request = sbi_sysreset_request,+};
+U_BOOT_DRIVER(sbi_sysreset) = {
.name = "sbi-sysreset",.id = UCLASS_SYSRESET,.ops = &sbi_sysreset_ops,.probe = sbi_sysreset_probe,+}; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index dacc3b5881..36985fce2c 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -297,7 +297,7 @@ config EFI_HAVE_RUNTIME_RESET bool default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
SANDBOX || SYSRESET_X86
SANDBOX || SYSRESET_SBI || SYSRESET_X86config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM" --
Regards, Bin
On 9/5/21 1:59 PM, Bin Meng wrote:
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide sysreset driver using the SBI system reset extension.
This patch should be split into 2 patches, one for adding the sysreset DM driver, and the other one for EFI support.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: no change
MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- 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 | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..88d7aa2bc7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b5..8e49b6d736 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,7 @@ #include <common.h> #include <cpu.h> #include <dm.h> +#include <dm/lists.h> #include <init.h> #include <log.h> #include <asm/encoding.h> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void)
int arch_early_init_r(void) {
return riscv_cpu_probe();
int ret;ret = riscv_cpu_probe();if (ret)return ret;if (IS_ENABLED(CONFIG_SYSRESET_SBI))device_bind_driver(gd->dm_root, "sbi-sysreset","sbi-sysreset", NULL);return 0;}
/**
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index e9caa78d17..69cddda245 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -85,6 +85,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_V02select SYSRESET_CMD_POWEROFF if CMD_POWEROFFhelpEnable system reset and poweroff via the SBI system reset extension.If the SBI implementation 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..fec5a66515 --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,96 @@ +// 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;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;+}
Is there a better place for the EFI stuff?
Bin, thanks for reviewing the series.
This seems to be related to you comment above about splitting into two patches. We put have the same set of EFI runtime functions for system reset in:
drivers/sysreset/sysreset_x86.c drivers/firmware/psci.c arch/arm/mach-bcm283x/reset.c arch/arm/cpu/armv8/fsl-layerscape/cpu.c
My idea was that all SBI system reset related things should be in the same source file.
Where do you think would be the best place for RISC-V SBI calls at UEFI runtime?
Best regards
Heinrich
+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;enum sbi_srst_reset_reason reset_reason;if (have_reset)switch (type) {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:reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;break;}if (reset_status == EFI_SUCCESS)reset_reason = SBI_SRST_RESET_REASON_NONE;elsereset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE;sbi_srst_reset(reset_type, reset_reason);while (1);+}
+static int sbi_sysreset_probe(struct udevice *dev) +{
have_reset = sbi_probe_extension(SBI_EXT_SRST);if (have_reset)return 0;log_warning("SBI has no system reset extension\n");return -ENOENT;+}
+static struct sysreset_ops sbi_sysreset_ops = {
.request = sbi_sysreset_request,+};
+U_BOOT_DRIVER(sbi_sysreset) = {
.name = "sbi-sysreset",.id = UCLASS_SYSRESET,.ops = &sbi_sysreset_ops,.probe = sbi_sysreset_probe,+}; diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index dacc3b5881..36985fce2c 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -297,7 +297,7 @@ config EFI_HAVE_RUNTIME_RESET bool default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
SANDBOX || SYSRESET_X86
SANDBOX || SYSRESET_SBI || SYSRESET_X86config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM"
--
Regards, Bin
Hi Heinrich,
On Mon, Sep 6, 2021 at 12:50 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 1:59 PM, Bin Meng wrote:
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide sysreset driver using the SBI system reset extension.
This patch should be split into 2 patches, one for adding the sysreset DM driver, and the other one for EFI support.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: no change
MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- 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 | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..88d7aa2bc7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b5..8e49b6d736 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,7 @@ #include <common.h> #include <cpu.h> #include <dm.h> +#include <dm/lists.h> #include <init.h> #include <log.h> #include <asm/encoding.h> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void)
int arch_early_init_r(void) {
return riscv_cpu_probe();
int ret;ret = riscv_cpu_probe();if (ret)return ret;if (IS_ENABLED(CONFIG_SYSRESET_SBI))device_bind_driver(gd->dm_root, "sbi-sysreset","sbi-sysreset", NULL);return 0;}
/**
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index e9caa78d17..69cddda245 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -85,6 +85,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_V02select SYSRESET_CMD_POWEROFF if CMD_POWEROFFhelpEnable system reset and poweroff via the SBI system reset extension.If the SBI implementation 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..fec5a66515 --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,96 @@ +// 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;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;+}
Is there a better place for the EFI stuff?
Bin, thanks for reviewing the series.
This seems to be related to you comment above about splitting into two patches.
Yep.
We put have the same set of EFI runtime functions for system reset in:
drivers/sysreset/sysreset_x86.c drivers/firmware/psci.c arch/arm/mach-bcm283x/reset.c arch/arm/cpu/armv8/fsl-layerscape/cpu.c
I didn't realize that. But this does not look correct to me.
EFI reset should be independent of the system reset drivers. For example, on RISC-V SRST is optional so not every SBI implements this. On SiFive Unleashed, the "gpio-restart" driver is used for reset (see sysreset_gpio.c).
With the approach in this patch, we mandate the EFI reset goes through the SBI call hence it does not work on boards like Unleashed.
I believe we should drop the driver-specific efi_reset_system(), but move efi_reset_system() to a generic place in EFI loader, and re-implement it using DM APIs.
My idea was that all SBI system reset related things should be in the same source file.
Where do you think would be the best place for RISC-V SBI calls at UEFI runtime?
Regards, Bin
On 9/5/21 7:00 PM, Bin Meng wrote:
Hi Heinrich,
On Mon, Sep 6, 2021 at 12:50 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 1:59 PM, Bin Meng wrote:
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide sysreset driver using the SBI system reset extension.
This patch should be split into 2 patches, one for adding the sysreset DM driver, and the other one for EFI support.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: no change
MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- 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 | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..88d7aa2bc7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b5..8e49b6d736 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,7 @@ #include <common.h> #include <cpu.h> #include <dm.h> +#include <dm/lists.h> #include <init.h> #include <log.h> #include <asm/encoding.h> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void)
int arch_early_init_r(void) {
return riscv_cpu_probe();
int ret;ret = riscv_cpu_probe();if (ret)return ret;if (IS_ENABLED(CONFIG_SYSRESET_SBI))device_bind_driver(gd->dm_root, "sbi-sysreset","sbi-sysreset", NULL);return 0;}
/**
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index e9caa78d17..69cddda245 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -85,6 +85,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_V02select SYSRESET_CMD_POWEROFF if CMD_POWEROFFhelpEnable system reset and poweroff via the SBI system reset extension.If the SBI implementation 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..fec5a66515 --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,96 @@ +// 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;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;+}
Is there a better place for the EFI stuff?
Bin, thanks for reviewing the series.
This seems to be related to you comment above about splitting into two patches.
Yep.
We put have the same set of EFI runtime functions for system reset in:
drivers/sysreset/sysreset_x86.c drivers/firmware/psci.c arch/arm/mach-bcm283x/reset.c arch/arm/cpu/armv8/fsl-layerscape/cpu.c
I didn't realize that. But this does not look correct to me.
EFI reset should be independent of the system reset drivers. For example, on RISC-V SRST is optional so not every SBI implements this. On SiFive Unleashed, the "gpio-restart" driver is used for reset (see sysreset_gpio.c).
The EFI functions that you see in this patch are for the runtime, i.e. after ExitBootServices().
When you issue the reboot or poweroff command in Linux it will call the UEFI runtime. At this point all driver model code is gone. So we cannot call any DM U-Boot driver. This is why these two functions are marked as __efi_runtime.
The RISC-V platform specification clearly states that you should use the SBI system reset if it exists.
Before ExitBootServices() the U-Boot sysreset driver will be called which may be the SBI driver. U-Boot will iterate through all sysreset drivers until one actually resets the board.
With the approach in this patch, we mandate the EFI reset goes through the SBI call hence it does not work on boards like Unleashed.
On the Hifive Unmatched poweroff using OpenSBI just works fine. But OpenSBI does not have reset for the HiFive Unmatched due to a missing GPIO.
I have no HiFive Unleashed for testing but calling OpenSBI should work out of the box as the device-tree defines the GPIOs both for reboot and poweroff that the reset driver looks for in OpenSBI. Do you have access to an Unleashed board for testing?
For OpenSBI I created commit e928472e67f8 ("lib: utils: support both of gpio-poweroff, gpio-reset") which you will be needed for the Unleashed board. The patch is merged into upstream.
I believe we should drop the driver-specific efi_reset_system(), but move efi_reset_system() to a generic place in EFI loader, and re-implement it using DM APIs.
see above
Best regards
Heinrich
My idea was that all SBI system reset related things should be in the same source file.
Where do you think would be the best place for RISC-V SBI calls at UEFI runtime?
Regards, Bin
On Mon, Sep 6, 2021 at 1:21 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 7:00 PM, Bin Meng wrote:
Hi Heinrich,
On Mon, Sep 6, 2021 at 12:50 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 1:59 PM, Bin Meng wrote:
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide sysreset driver using the SBI system reset extension.
This patch should be split into 2 patches, one for adding the sysreset DM driver, and the other one for EFI support.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: no change
MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- 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 | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..88d7aa2bc7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b5..8e49b6d736 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,7 @@ #include <common.h> #include <cpu.h> #include <dm.h> +#include <dm/lists.h> #include <init.h> #include <log.h> #include <asm/encoding.h> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void)
int arch_early_init_r(void) {
return riscv_cpu_probe();
int ret;ret = riscv_cpu_probe();if (ret)return ret;if (IS_ENABLED(CONFIG_SYSRESET_SBI))device_bind_driver(gd->dm_root, "sbi-sysreset","sbi-sysreset", NULL);return 0;}
/**
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index e9caa78d17..69cddda245 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -85,6 +85,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_V02select SYSRESET_CMD_POWEROFF if CMD_POWEROFFhelpEnable system reset and poweroff via the SBI system reset extension.If the SBI implementation 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..fec5a66515 --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,96 @@ +// 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;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;+}
Is there a better place for the EFI stuff?
Bin, thanks for reviewing the series.
This seems to be related to you comment above about splitting into two patches.
Yep.
We put have the same set of EFI runtime functions for system reset in:
drivers/sysreset/sysreset_x86.c drivers/firmware/psci.c arch/arm/mach-bcm283x/reset.c arch/arm/cpu/armv8/fsl-layerscape/cpu.c
I didn't realize that. But this does not look correct to me.
EFI reset should be independent of the system reset drivers. For example, on RISC-V SRST is optional so not every SBI implements this. On SiFive Unleashed, the "gpio-restart" driver is used for reset (see sysreset_gpio.c).
The EFI functions that you see in this patch are for the runtime, i.e. after ExitBootServices().
When you issue the reboot or poweroff command in Linux it will call the UEFI runtime. At this point all driver model code is gone. So we cannot call any DM U-Boot driver. This is why these two functions are marked as __efi_runtime.
The mark of __efi_runtime is troublesome in some cases. I remember on x86 when booting from U-Boot EFI loader the Linux kernel calls EFI runtime services would hang which is because we don't mark the required functions with __efi_runtime in U-Boot. This is not a scalable solution I think. In the end we may have to mark all U-Boot functions to __efi_runtime.
The RISC-V platform specification clearly states that you should use the SBI system reset if it exists.
Before ExitBootServices() the U-Boot sysreset driver will be called which may be the SBI driver. U-Boot will iterate through all sysreset drivers until one actually resets the board.
With the approach in this patch, we mandate the EFI reset goes through the SBI call hence it does not work on boards like Unleashed.
On the Hifive Unmatched poweroff using OpenSBI just works fine. But OpenSBI does not have reset for the HiFive Unmatched due to a missing GPIO.
I have no HiFive Unleashed for testing but calling OpenSBI should work out of the box as the device-tree defines the GPIOs both for reboot and poweroff that the reset driver looks for in OpenSBI. Do you have access to an Unleashed board for testing?
For OpenSBI I created commit e928472e67f8 ("lib: utils: support both of gpio-poweroff, gpio-reset") which you will be needed for the Unleashed board. The patch is merged into upstream.
Regards, Bin
Am 6. September 2021 03:47:39 MESZ schrieb Bin Meng bmeng.cn@gmail.com:
On Mon, Sep 6, 2021 at 1:21 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 7:00 PM, Bin Meng wrote:
Hi Heinrich,
On Mon, Sep 6, 2021 at 12:50 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 1:59 PM, Bin Meng wrote:
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide sysreset driver using the SBI system reset extension.
This patch should be split into 2 patches, one for adding the sysreset DM driver, and the other one for EFI support.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: no change
MAINTAINERS | 1 + arch/riscv/cpu/cpu.c | 13 ++++- 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 | 96 +++++++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 2 +- 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS index 4cf0c33c5d..88d7aa2bc7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c index c894ac10b5..8e49b6d736 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -6,6 +6,7 @@ #include <common.h> #include <cpu.h> #include <dm.h> +#include <dm/lists.h> #include <init.h> #include <log.h> #include <asm/encoding.h> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void)
int arch_early_init_r(void) {
return riscv_cpu_probe();
int ret;ret = riscv_cpu_probe();if (ret)return ret;if (IS_ENABLED(CONFIG_SYSRESET_SBI))device_bind_driver(gd->dm_root, "sbi-sysreset","sbi-sysreset", NULL);return 0;}
/**
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index e9caa78d17..69cddda245 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -85,6 +85,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_V02select SYSRESET_CMD_POWEROFF if CMD_POWEROFFhelpEnable system reset and poweroff via the SBI system reset extension.If the SBI implementation 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..fec5a66515 --- /dev/null +++ b/drivers/sysreset/sysreset_sbi.c @@ -0,0 +1,96 @@ +// 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;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;+}
Is there a better place for the EFI stuff?
Bin, thanks for reviewing the series.
This seems to be related to you comment above about splitting into two patches.
Yep.
We put have the same set of EFI runtime functions for system reset in:
drivers/sysreset/sysreset_x86.c drivers/firmware/psci.c arch/arm/mach-bcm283x/reset.c arch/arm/cpu/armv8/fsl-layerscape/cpu.c
I didn't realize that. But this does not look correct to me.
EFI reset should be independent of the system reset drivers. For example, on RISC-V SRST is optional so not every SBI implements this. On SiFive Unleashed, the "gpio-restart" driver is used for reset (see sysreset_gpio.c).
The EFI functions that you see in this patch are for the runtime, i.e. after ExitBootServices().
When you issue the reboot or poweroff command in Linux it will call the UEFI runtime. At this point all driver model code is gone. So we cannot call any DM U-Boot driver. This is why these two functions are marked as __efi_runtime.
When using a system with multiple GiB of memory one could argue that freeing a MiB of boottime U-Boot memory at ExitBootServices is not worth the effort.
On the other side having a very small runtime to consider for security analysis also has its merrits.
Using platform independent reset methods like PSCI and SBI allows to limit the runtime code base. We should avoid board specific code.
But could you, please, answer my original question: Where do you want the SBI runtime code placed?
Best regards
Heinrich
The mark of __efi_runtime is troublesome in some cases. I remember on x86 when booting from U-Boot EFI loader the Linux kernel calls EFI runtime services would hang which is because we don't mark the required functions with __efi_runtime in U-Boot. This is not a scalable solution I think. In the end we may have to mark all U-Boot functions to __efi_runtime.
The RISC-V platform specification clearly states that you should use the SBI system reset if it exists.
Before ExitBootServices() the U-Boot sysreset driver will be called which may be the SBI driver. U-Boot will iterate through all sysreset drivers until one actually resets the board.
With the approach in this patch, we mandate the EFI reset goes through the SBI call hence it does not work on boards like Unleashed.
On the Hifive Unmatched poweroff using OpenSBI just works fine. But OpenSBI does not have reset for the HiFive Unmatched due to a missing GPIO.
I have no HiFive Unleashed for testing but calling OpenSBI should work out of the box as the device-tree defines the GPIOs both for reboot and poweroff that the reset driver looks for in OpenSBI. Do you have access to an Unleashed board for testing?
For OpenSBI I created commit e928472e67f8 ("lib: utils: support both of gpio-poweroff, gpio-reset") which you will be needed for the Unleashed board. The patch is merged into upstream.
Regards, Bin
On Mon, Sep 6, 2021 at 12:45 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 6. September 2021 03:47:39 MESZ schrieb Bin Meng bmeng.cn@gmail.com:
On Mon, Sep 6, 2021 at 1:21 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 7:00 PM, Bin Meng wrote:
Hi Heinrich,
On Mon, Sep 6, 2021 at 12:50 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 1:59 PM, Bin Meng wrote:
On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > Provide sysreset driver using the SBI system reset extension. >
This patch should be split into 2 patches, one for adding the sysreset DM driver, and the other one for EFI support.
> Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de > --- > v3: > no change > --- > MAINTAINERS | 1 + > arch/riscv/cpu/cpu.c | 13 ++++- > 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 | 96 +++++++++++++++++++++++++++++++++ > lib/efi_loader/Kconfig | 2 +- > 8 files changed, 140 insertions(+), 6 deletions(-) > create mode 100644 drivers/sysreset/sysreset_sbi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4cf0c33c5d..88d7aa2bc7 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c > index c894ac10b5..8e49b6d736 100644 > --- a/arch/riscv/cpu/cpu.c > +++ b/arch/riscv/cpu/cpu.c > @@ -6,6 +6,7 @@ > #include <common.h> > #include <cpu.h> > #include <dm.h> > +#include <dm/lists.h> > #include <init.h> > #include <log.h> > #include <asm/encoding.h> > @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void) > > int arch_early_init_r(void) > { > - return riscv_cpu_probe(); > + int ret; > + > + ret = riscv_cpu_probe(); > + if (ret) > + return ret; > + > + if (IS_ENABLED(CONFIG_SYSRESET_SBI)) > + device_bind_driver(gd->dm_root, "sbi-sysreset", > + "sbi-sysreset", NULL); > + > + return 0; > } > > /** > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index e9caa78d17..69cddda245 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 > --- a/drivers/sysreset/Kconfig > +++ b/drivers/sysreset/Kconfig > @@ -85,6 +85,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 implementation 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..fec5a66515 > --- /dev/null > +++ b/drivers/sysreset/sysreset_sbi.c > @@ -0,0 +1,96 @@ > +// 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; > + > + 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; > +}
Is there a better place for the EFI stuff?
Bin, thanks for reviewing the series.
This seems to be related to you comment above about splitting into two patches.
Yep.
We put have the same set of EFI runtime functions for system reset in:
drivers/sysreset/sysreset_x86.c drivers/firmware/psci.c arch/arm/mach-bcm283x/reset.c arch/arm/cpu/armv8/fsl-layerscape/cpu.c
I didn't realize that. But this does not look correct to me.
EFI reset should be independent of the system reset drivers. For example, on RISC-V SRST is optional so not every SBI implements this. On SiFive Unleashed, the "gpio-restart" driver is used for reset (see sysreset_gpio.c).
The EFI functions that you see in this patch are for the runtime, i.e. after ExitBootServices().
When you issue the reboot or poweroff command in Linux it will call the UEFI runtime. At this point all driver model code is gone. So we cannot call any DM U-Boot driver. This is why these two functions are marked as __efi_runtime.
When using a system with multiple GiB of memory one could argue that freeing a MiB of boottime U-Boot memory at ExitBootServices is not worth the effort.
On the other side having a very small runtime to consider for security analysis also has its merrits.
Agree, but my point is that current U-Boot EFI loader approach of marking runtime functions with __efi_runtime is not scalable. You have to trace down all callees to make sure they are marked with __efi_runtime by an __efi_runtime caller, like you did in this patch. But my test on EFI loader on x86 Linux in the past suggested that we missed something in the calling path.
Using platform independent reset methods like PSCI and SBI allows to limit the runtime code base. We should avoid board specific code.
There are platforms without PSCI and SBI but want EFI support.
But could you, please, answer my original question: Where do you want the SBI runtime code placed?
I still think we should implement the EFI reset via DM APIs, so as I mentioned the runtime codes can be put in the common efi_loader directory.
Regards, Bin
Am 6. September 2021 07:22:21 MESZ schrieb Bin Meng bmeng.cn@gmail.com:
On Mon, Sep 6, 2021 at 12:45 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 6. September 2021 03:47:39 MESZ schrieb Bin Meng bmeng.cn@gmail.com:
On Mon, Sep 6, 2021 at 1:21 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 7:00 PM, Bin Meng wrote:
Hi Heinrich,
On Mon, Sep 6, 2021 at 12:50 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 9/5/21 1:59 PM, Bin Meng wrote: > On Sun, Sep 5, 2021 at 4:38 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote: >> >> Provide sysreset driver using the SBI system reset extension. >> > > This patch should be split into 2 patches, one for adding the sysreset > DM driver, and the other one for EFI support. > >> Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de >> --- >> v3: >> no change >> --- >> MAINTAINERS | 1 + >> arch/riscv/cpu/cpu.c | 13 ++++- >> 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 | 96 +++++++++++++++++++++++++++++++++ >> lib/efi_loader/Kconfig | 2 +- >> 8 files changed, 140 insertions(+), 6 deletions(-) >> create mode 100644 drivers/sysreset/sysreset_sbi.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 4cf0c33c5d..88d7aa2bc7 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -1017,6 +1017,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/cpu/cpu.c b/arch/riscv/cpu/cpu.c >> index c894ac10b5..8e49b6d736 100644 >> --- a/arch/riscv/cpu/cpu.c >> +++ b/arch/riscv/cpu/cpu.c >> @@ -6,6 +6,7 @@ >> #include <common.h> >> #include <cpu.h> >> #include <dm.h> >> +#include <dm/lists.h> >> #include <init.h> >> #include <log.h> >> #include <asm/encoding.h> >> @@ -138,7 +139,17 @@ int arch_cpu_init_dm(void) >> >> int arch_early_init_r(void) >> { >> - return riscv_cpu_probe(); >> + int ret; >> + >> + ret = riscv_cpu_probe(); >> + if (ret) >> + return ret; >> + >> + if (IS_ENABLED(CONFIG_SYSRESET_SBI)) >> + device_bind_driver(gd->dm_root, "sbi-sysreset", >> + "sbi-sysreset", NULL); >> + >> + return 0; >> } >> >> /** >> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h >> index e9caa78d17..69cddda245 100644 >> --- a/arch/riscv/include/asm/sbi.h >> +++ b/arch/riscv/include/asm/sbi.h >> @@ -154,5 +154,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 ac77ffbc8b..6782331181 100644 >> --- a/drivers/sysreset/Kconfig >> +++ b/drivers/sysreset/Kconfig >> @@ -85,6 +85,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 implementation 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..fec5a66515 >> --- /dev/null >> +++ b/drivers/sysreset/sysreset_sbi.c >> @@ -0,0 +1,96 @@ >> +// 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; >> + >> + 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; >> +} > > Is there a better place for the EFI stuff?
Bin, thanks for reviewing the series.
This seems to be related to you comment above about splitting into two patches.
Yep.
We put have the same set of EFI runtime functions for system reset in:
drivers/sysreset/sysreset_x86.c drivers/firmware/psci.c arch/arm/mach-bcm283x/reset.c arch/arm/cpu/armv8/fsl-layerscape/cpu.c
I didn't realize that. But this does not look correct to me.
EFI reset should be independent of the system reset drivers. For example, on RISC-V SRST is optional so not every SBI implements this. On SiFive Unleashed, the "gpio-restart" driver is used for reset (see sysreset_gpio.c).
The EFI functions that you see in this patch are for the runtime, i.e. after ExitBootServices().
When you issue the reboot or poweroff command in Linux it will call the UEFI runtime. At this point all driver model code is gone. So we cannot call any DM U-Boot driver. This is why these two functions are marked as __efi_runtime.
When using a system with multiple GiB of memory one could argue that freeing a MiB of boottime U-Boot memory at ExitBootServices is not worth the effort.
On the other side having a very small runtime to consider for security analysis also has its merrits.
Agree, but my point is that current U-Boot EFI loader approach of marking runtime functions with __efi_runtime is not scalable. You have to trace down all callees to make sure they are marked with __efi_runtime by an __efi_runtime caller, like you did in this patch. But my test on EFI loader on x86 Linux in the past suggested that we missed something in the calling path.
Using platform independent reset methods like PSCI and SBI allows to limit the runtime code base. We should avoid board specific code.
There are platforms without PSCI and SBI but want EFI support.
But could you, please, answer my original question: Where do you want the SBI runtime code placed?
I still think we should implement the EFI reset via DM APIs, so as I mentioned the runtime codes can be put in the common efi_loader directory.
After ExitBootServices Linux calls SetVirtualAdressMap. The DM code would have to update all pointers it uses afterwards according to Linux' memory mapping.
So instead of spreading __efi_runtime you will be spreading pointer update code. This will lead to an increase of the U-Boot binary size for all boards and add complexity.
I am in doubt here.
Best regards
Heinrich
participants (2)
-
Bin Meng -
Heinrich Schuchardt