[PATCH v3 3/3] sysreset: provide SBI based sysreset driver
Bin Meng
bmeng.cn at gmail.com
Sun Sep 5 19:00:30 CEST 2021
Hi Heinrich,
On Mon, Sep 6, 2021 at 12:50 AM Heinrich Schuchardt <xypron.glpk at 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 at 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 at 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 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;
> >> +
> >> + 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
More information about the U-Boot
mailing list