[PATCH 3/5] sysreset: provide SBI based sysreset driver
Sean Anderson
seanga2 at gmail.com
Fri Mar 5 00:50:17 CET 2021
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 at 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 at 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
>
More information about the U-Boot
mailing list