| Message ID | 20200926065321.807213-1-anup.patel@wdc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,v2] lib: Implement System Reset (SRST) SBI extension | expand |
On 9/26/20 2:53 AM, Anup Patel wrote: > The SBI SRST extension specification is in draft state and available > in srbt_v1 branch of: https://github.com/avpatel/riscv-sbi-doc. > > It allows to S-mode software to request system shutdown, cold reboot, > and warm reboot. > > This patch provides implementation of SBI SRST extension. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > Changes since v1: > - Updated patch as-per latest SBI SRST extension draft spec where > we have only one SBI call with "reset_type" parameter > --- > include/sbi/sbi_ecall.h | 1 + > include/sbi/sbi_ecall_interface.h | 8 ++++++ > lib/sbi/objects.mk | 1 + > lib/sbi/sbi_ecall.c | 3 ++ > lib/sbi/sbi_ecall_srst.c | 48 +++++++++++++++++++++++++++++++ > 5 files changed, 61 insertions(+) > create mode 100644 lib/sbi/sbi_ecall_srst.c > > diff --git a/include/sbi/sbi_ecall.h b/include/sbi/sbi_ecall.h > index 3273ba6..1ef86e2 100644 > --- a/include/sbi/sbi_ecall.h > +++ b/include/sbi/sbi_ecall.h > @@ -37,6 +37,7 @@ extern struct sbi_ecall_extension ecall_rfence; > extern struct sbi_ecall_extension ecall_ipi; > extern struct sbi_ecall_extension ecall_vendor; > extern struct sbi_ecall_extension ecall_hsm; > +extern struct sbi_ecall_extension ecall_srst; > > u16 sbi_ecall_version_major(void); > > diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h > index af30500..d9c2519 100644 > --- a/include/sbi/sbi_ecall_interface.h > +++ b/include/sbi/sbi_ecall_interface.h > @@ -27,6 +27,7 @@ > #define SBI_EXT_IPI 0x735049 > #define SBI_EXT_RFENCE 0x52464E43 > #define SBI_EXT_HSM 0x48534D > +#define SBI_EXT_SRST 0x53525354 > > /* SBI function IDs for BASE extension*/ > #define SBI_EXT_BASE_GET_SPEC_VERSION 0x0 > @@ -62,6 +63,13 @@ > #define SBI_HSM_HART_STATUS_START_PENDING 0x2 > #define SBI_HSM_HART_STATUS_STOP_PENDING 0x3 > > +/* SBI function IDs for SRST extension */ > +#define SBI_EXT_SRST_RESET 0x0 > + > +#define SBI_SRST_RESET_TYPE_SHUTDOWN 0x0 > +#define SBI_SRST_RESET_TYPE_COLD_REBOOT 0x1 > +#define SBI_SRST_RESET_TYPE_WARM_REBOOT 0x2 > + > #define SBI_SPEC_VERSION_MAJOR_OFFSET 24 > #define SBI_SPEC_VERSION_MAJOR_MASK 0x7f > #define SBI_SPEC_VERSION_MINOR_MASK 0xffffff > diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk > index fa808a0..6ec1154 100644 > --- a/lib/sbi/objects.mk > +++ b/lib/sbi/objects.mk > @@ -20,6 +20,7 @@ libsbi-objs-y += sbi_ecall_base.o > libsbi-objs-y += sbi_ecall_hsm.o > libsbi-objs-y += sbi_ecall_legacy.o > libsbi-objs-y += sbi_ecall_replace.o > +libsbi-objs-y += sbi_ecall_srst.o > libsbi-objs-y += sbi_ecall_vendor.o > libsbi-objs-y += sbi_emulate_csr.o > libsbi-objs-y += sbi_fifo.o > diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c > index 64c9933..6d41cff 100644 > --- a/lib/sbi/sbi_ecall.c > +++ b/lib/sbi/sbi_ecall.c > @@ -167,6 +167,9 @@ int sbi_ecall_init(void) > if (ret) > return ret; > ret = sbi_ecall_register_extension(&ecall_hsm); > + if (ret) > + return ret; > + ret = sbi_ecall_register_extension(&ecall_srst); > if (ret) > return ret; > ret = sbi_ecall_register_extension(&ecall_legacy); > diff --git a/lib/sbi/sbi_ecall_srst.c b/lib/sbi/sbi_ecall_srst.c > new file mode 100644 > index 0000000..64c228f > --- /dev/null > +++ b/lib/sbi/sbi_ecall_srst.c > @@ -0,0 +1,48 @@ > +/* > + * SPDX-License-Identifier: BSD-2-Clause > + * > + * Copyright (c) 2020 Western Digital Corporation or its affiliates. > + * > + * Authors: > + * Anup Patel <anup.patel@wdc.com> > + */ > + > +#include <sbi/sbi_ecall.h> > +#include <sbi/sbi_ecall_interface.h> > +#include <sbi/sbi_error.h> > +#include <sbi/sbi_platform.h> > +#include <sbi/sbi_system.h> > + > +static int sbi_ecall_srst_handler(unsigned long extid, unsigned long funcid, > + unsigned long *args, unsigned long *out_val, > + struct sbi_trap_info *out_trap) > +{ > + int ret = 0; > + > + if (funcid == SBI_EXT_SRST_RESET) { > + switch (args[0]) { > + case SBI_SRST_RESET_TYPE_SHUTDOWN: > + sbi_system_reset(SBI_PLATFORM_RESET_SHUTDOWN); Shouldn't we check a return value here? Some platforms may not support all types of resets, and may want to return SBI_ENOTSUPP. For example, the K210 only supports warm resets. --Sean > + break; > + case SBI_SRST_RESET_TYPE_COLD_REBOOT: > + sbi_system_reset(SBI_PLATFORM_RESET_COLD); > + break; > + case SBI_SRST_RESET_TYPE_WARM_REBOOT: > + sbi_system_reset(SBI_PLATFORM_RESET_WARM); > + break; > + default: > + ret = SBI_EINVAL; > + break; > + }; > + } else { > + ret = SBI_ENOTSUPP; > + } > + > + return ret; > +} > + > +struct sbi_ecall_extension ecall_srst = { > + .extid_start = SBI_EXT_SRST, > + .extid_end = SBI_EXT_SRST, > + .handle = sbi_ecall_srst_handler, > +}; >
> -----Original Message----- > From: Sean Anderson <seanga2@gmail.com> > Sent: 30 September 2020 17:07 > To: Anup Patel <Anup.Patel@wdc.com>; Atish Patra > <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com> > Cc: Anup Patel <anup@brainfault.org>; opensbi@lists.infradead.org > Subject: Re: [RFC PATCH v2] lib: Implement System Reset (SRST) SBI > extension > > On 9/26/20 2:53 AM, Anup Patel wrote: > > The SBI SRST extension specification is in draft state and available > > in srbt_v1 branch of: https://github.com/avpatel/riscv-sbi-doc. > > > > It allows to S-mode software to request system shutdown, cold reboot, > > and warm reboot. > > > > This patch provides implementation of SBI SRST extension. > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > --- > > Changes since v1: > > - Updated patch as-per latest SBI SRST extension draft spec where > > we have only one SBI call with "reset_type" parameter > > --- > > include/sbi/sbi_ecall.h | 1 + > > include/sbi/sbi_ecall_interface.h | 8 ++++++ > > lib/sbi/objects.mk | 1 + > > lib/sbi/sbi_ecall.c | 3 ++ > > lib/sbi/sbi_ecall_srst.c | 48 +++++++++++++++++++++++++++++++ > > 5 files changed, 61 insertions(+) > > create mode 100644 lib/sbi/sbi_ecall_srst.c > > > > diff --git a/include/sbi/sbi_ecall.h b/include/sbi/sbi_ecall.h index > > 3273ba6..1ef86e2 100644 > > --- a/include/sbi/sbi_ecall.h > > +++ b/include/sbi/sbi_ecall.h > > @@ -37,6 +37,7 @@ extern struct sbi_ecall_extension ecall_rfence; > > extern struct sbi_ecall_extension ecall_ipi; extern struct > > sbi_ecall_extension ecall_vendor; extern struct sbi_ecall_extension > > ecall_hsm; > > +extern struct sbi_ecall_extension ecall_srst; > > > > u16 sbi_ecall_version_major(void); > > > > diff --git a/include/sbi/sbi_ecall_interface.h > > b/include/sbi/sbi_ecall_interface.h > > index af30500..d9c2519 100644 > > --- a/include/sbi/sbi_ecall_interface.h > > +++ b/include/sbi/sbi_ecall_interface.h > > @@ -27,6 +27,7 @@ > > #define SBI_EXT_IPI 0x735049 > > #define SBI_EXT_RFENCE 0x52464E43 > > #define SBI_EXT_HSM 0x48534D > > +#define SBI_EXT_SRST 0x53525354 > > > > /* SBI function IDs for BASE extension*/ > > #define SBI_EXT_BASE_GET_SPEC_VERSION 0x0 > > @@ -62,6 +63,13 @@ > > #define SBI_HSM_HART_STATUS_START_PENDING 0x2 > > #define SBI_HSM_HART_STATUS_STOP_PENDING 0x3 > > > > +/* SBI function IDs for SRST extension */ > > +#define SBI_EXT_SRST_RESET 0x0 > > + > > +#define SBI_SRST_RESET_TYPE_SHUTDOWN 0x0 > > +#define SBI_SRST_RESET_TYPE_COLD_REBOOT 0x1 > > +#define SBI_SRST_RESET_TYPE_WARM_REBOOT 0x2 > > + > > #define SBI_SPEC_VERSION_MAJOR_OFFSET 24 > > #define SBI_SPEC_VERSION_MAJOR_MASK 0x7f > > #define SBI_SPEC_VERSION_MINOR_MASK 0xffffff > > diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk index > > fa808a0..6ec1154 100644 > > --- a/lib/sbi/objects.mk > > +++ b/lib/sbi/objects.mk > > @@ -20,6 +20,7 @@ libsbi-objs-y += sbi_ecall_base.o libsbi-objs-y += > > sbi_ecall_hsm.o libsbi-objs-y += sbi_ecall_legacy.o libsbi-objs-y += > > sbi_ecall_replace.o > > +libsbi-objs-y += sbi_ecall_srst.o > > libsbi-objs-y += sbi_ecall_vendor.o > > libsbi-objs-y += sbi_emulate_csr.o > > libsbi-objs-y += sbi_fifo.o > > diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c index > > 64c9933..6d41cff 100644 > > --- a/lib/sbi/sbi_ecall.c > > +++ b/lib/sbi/sbi_ecall.c > > @@ -167,6 +167,9 @@ int sbi_ecall_init(void) > > if (ret) > > return ret; > > ret = sbi_ecall_register_extension(&ecall_hsm); > > + if (ret) > > + return ret; > > + ret = sbi_ecall_register_extension(&ecall_srst); > > if (ret) > > return ret; > > ret = sbi_ecall_register_extension(&ecall_legacy); > > diff --git a/lib/sbi/sbi_ecall_srst.c b/lib/sbi/sbi_ecall_srst.c new > > file mode 100644 index 0000000..64c228f > > --- /dev/null > > +++ b/lib/sbi/sbi_ecall_srst.c > > @@ -0,0 +1,48 @@ > > +/* > > + * SPDX-License-Identifier: BSD-2-Clause > > + * > > + * Copyright (c) 2020 Western Digital Corporation or its affiliates. > > + * > > + * Authors: > > + * Anup Patel <anup.patel@wdc.com> > > + */ > > + > > +#include <sbi/sbi_ecall.h> > > +#include <sbi/sbi_ecall_interface.h> > > +#include <sbi/sbi_error.h> > > +#include <sbi/sbi_platform.h> > > +#include <sbi/sbi_system.h> > > + > > +static int sbi_ecall_srst_handler(unsigned long extid, unsigned long > funcid, > > + unsigned long *args, unsigned long > *out_val, > > + struct sbi_trap_info *out_trap) { > > + int ret = 0; > > + > > + if (funcid == SBI_EXT_SRST_RESET) { > > + switch (args[0]) { > > + case SBI_SRST_RESET_TYPE_SHUTDOWN: > > + > sbi_system_reset(SBI_PLATFORM_RESET_SHUTDOWN); > > Shouldn't we check a return value here? Some platforms may not support all > types of resets, and may want to return SBI_ENOTSUPP. For example, the > K210 only supports warm resets. The sbi_system_reset() is implemented as noreturn function. We have three cases here: 1. A platform has none of the SBI system reset types supported 2. A platform has few of the SBI system reset types supported 3. A platform has all SBI system reset types supported For case#1, we will simply halt all HARTs in WFI. For case#2, we will halt all HARTs in WFI when a particular SBI system reset type is not supported by platform. Basically, current sbi_system_reset() implementation ensures that we never return for a valid reset_type irrespective whether underlying platform supports it or not. The only case where SBI SRST RESET call fails is when provided reset_type is invalid. Regards, Anup > > --Sean > > > + break; > > + case SBI_SRST_RESET_TYPE_COLD_REBOOT: > > + sbi_system_reset(SBI_PLATFORM_RESET_COLD); > > + break; > > + case SBI_SRST_RESET_TYPE_WARM_REBOOT: > > + sbi_system_reset(SBI_PLATFORM_RESET_WARM); > > + break; > > + default: > > + ret = SBI_EINVAL; > > + break; > > + }; > > + } else { > > + ret = SBI_ENOTSUPP; > > + } > > + > > + return ret; > > +} > > + > > +struct sbi_ecall_extension ecall_srst = { > > + .extid_start = SBI_EXT_SRST, > > + .extid_end = SBI_EXT_SRST, > > + .handle = sbi_ecall_srst_handler, > > +}; > >
On 10/1/20 3:56 AM, Anup Patel wrote: > > >> -----Original Message----- >> From: Sean Anderson <seanga2@gmail.com> >> Sent: 30 September 2020 17:07 >> To: Anup Patel <Anup.Patel@wdc.com>; Atish Patra >> <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com> >> Cc: Anup Patel <anup@brainfault.org>; opensbi@lists.infradead.org >> Subject: Re: [RFC PATCH v2] lib: Implement System Reset (SRST) SBI >> extension >> >> On 9/26/20 2:53 AM, Anup Patel wrote: >>> The SBI SRST extension specification is in draft state and available >>> in srbt_v1 branch of: https://github.com/avpatel/riscv-sbi-doc. >>> >>> It allows to S-mode software to request system shutdown, cold reboot, >>> and warm reboot. >>> >>> This patch provides implementation of SBI SRST extension. >>> >>> Signed-off-by: Anup Patel <anup.patel@wdc.com> >>> --- >>> Changes since v1: >>> - Updated patch as-per latest SBI SRST extension draft spec where >>> we have only one SBI call with "reset_type" parameter >>> --- >>> include/sbi/sbi_ecall.h | 1 + >>> include/sbi/sbi_ecall_interface.h | 8 ++++++ >>> lib/sbi/objects.mk | 1 + >>> lib/sbi/sbi_ecall.c | 3 ++ >>> lib/sbi/sbi_ecall_srst.c | 48 +++++++++++++++++++++++++++++++ >>> 5 files changed, 61 insertions(+) >>> create mode 100644 lib/sbi/sbi_ecall_srst.c >>> >>> diff --git a/include/sbi/sbi_ecall.h b/include/sbi/sbi_ecall.h index >>> 3273ba6..1ef86e2 100644 >>> --- a/include/sbi/sbi_ecall.h >>> +++ b/include/sbi/sbi_ecall.h >>> @@ -37,6 +37,7 @@ extern struct sbi_ecall_extension ecall_rfence; >>> extern struct sbi_ecall_extension ecall_ipi; extern struct >>> sbi_ecall_extension ecall_vendor; extern struct sbi_ecall_extension >>> ecall_hsm; >>> +extern struct sbi_ecall_extension ecall_srst; >>> >>> u16 sbi_ecall_version_major(void); >>> >>> diff --git a/include/sbi/sbi_ecall_interface.h >>> b/include/sbi/sbi_ecall_interface.h >>> index af30500..d9c2519 100644 >>> --- a/include/sbi/sbi_ecall_interface.h >>> +++ b/include/sbi/sbi_ecall_interface.h >>> @@ -27,6 +27,7 @@ >>> #define SBI_EXT_IPI 0x735049 >>> #define SBI_EXT_RFENCE 0x52464E43 >>> #define SBI_EXT_HSM 0x48534D >>> +#define SBI_EXT_SRST 0x53525354 >>> >>> /* SBI function IDs for BASE extension*/ >>> #define SBI_EXT_BASE_GET_SPEC_VERSION 0x0 >>> @@ -62,6 +63,13 @@ >>> #define SBI_HSM_HART_STATUS_START_PENDING 0x2 >>> #define SBI_HSM_HART_STATUS_STOP_PENDING 0x3 >>> >>> +/* SBI function IDs for SRST extension */ >>> +#define SBI_EXT_SRST_RESET 0x0 >>> + >>> +#define SBI_SRST_RESET_TYPE_SHUTDOWN 0x0 >>> +#define SBI_SRST_RESET_TYPE_COLD_REBOOT 0x1 >>> +#define SBI_SRST_RESET_TYPE_WARM_REBOOT 0x2 >>> + >>> #define SBI_SPEC_VERSION_MAJOR_OFFSET 24 >>> #define SBI_SPEC_VERSION_MAJOR_MASK 0x7f >>> #define SBI_SPEC_VERSION_MINOR_MASK 0xffffff >>> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk index >>> fa808a0..6ec1154 100644 >>> --- a/lib/sbi/objects.mk >>> +++ b/lib/sbi/objects.mk >>> @@ -20,6 +20,7 @@ libsbi-objs-y += sbi_ecall_base.o libsbi-objs-y += >>> sbi_ecall_hsm.o libsbi-objs-y += sbi_ecall_legacy.o libsbi-objs-y += >>> sbi_ecall_replace.o >>> +libsbi-objs-y += sbi_ecall_srst.o >>> libsbi-objs-y += sbi_ecall_vendor.o >>> libsbi-objs-y += sbi_emulate_csr.o >>> libsbi-objs-y += sbi_fifo.o >>> diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c index >>> 64c9933..6d41cff 100644 >>> --- a/lib/sbi/sbi_ecall.c >>> +++ b/lib/sbi/sbi_ecall.c >>> @@ -167,6 +167,9 @@ int sbi_ecall_init(void) >>> if (ret) >>> return ret; >>> ret = sbi_ecall_register_extension(&ecall_hsm); >>> + if (ret) >>> + return ret; >>> + ret = sbi_ecall_register_extension(&ecall_srst); >>> if (ret) >>> return ret; >>> ret = sbi_ecall_register_extension(&ecall_legacy); >>> diff --git a/lib/sbi/sbi_ecall_srst.c b/lib/sbi/sbi_ecall_srst.c new >>> file mode 100644 index 0000000..64c228f >>> --- /dev/null >>> +++ b/lib/sbi/sbi_ecall_srst.c >>> @@ -0,0 +1,48 @@ >>> +/* >>> + * SPDX-License-Identifier: BSD-2-Clause >>> + * >>> + * Copyright (c) 2020 Western Digital Corporation or its affiliates. >>> + * >>> + * Authors: >>> + * Anup Patel <anup.patel@wdc.com> >>> + */ >>> + >>> +#include <sbi/sbi_ecall.h> >>> +#include <sbi/sbi_ecall_interface.h> >>> +#include <sbi/sbi_error.h> >>> +#include <sbi/sbi_platform.h> >>> +#include <sbi/sbi_system.h> >>> + >>> +static int sbi_ecall_srst_handler(unsigned long extid, unsigned long >> funcid, >>> + unsigned long *args, unsigned long >> *out_val, >>> + struct sbi_trap_info *out_trap) { >>> + int ret = 0; >>> + >>> + if (funcid == SBI_EXT_SRST_RESET) { >>> + switch (args[0]) { >>> + case SBI_SRST_RESET_TYPE_SHUTDOWN: >>> + >> sbi_system_reset(SBI_PLATFORM_RESET_SHUTDOWN); >> >> Shouldn't we check a return value here? Some platforms may not support all >> types of resets, and may want to return SBI_ENOTSUPP. For example, the >> K210 only supports warm resets. > > The sbi_system_reset() is implemented as noreturn function. > > We have three cases here: > 1. A platform has none of the SBI system reset types supported > 2. A platform has few of the SBI system reset types supported > 3. A platform has all SBI system reset types supported > > For case#1, we will simply halt all HARTs in WFI. > > For case#2, we will halt all HARTs in WFI when a > particular SBI system reset type is not supported by platform. > > Basically, current sbi_system_reset() implementation ensures that > we never return for a valid reset_type irrespective whether underlying > platform supports it or not. I think that should be changed. There must be a way to discover which reset types are supported by the platform. For example, say that I want to reset the system, or shut it down only if there is no way to reset it. With this API, I would have no way to do this. If I try a cold reset and it is not available, the platform will "shut down" anyway, even if there is a warm reset available. --Sean > The only case where SBI SRST RESET call fails is when provided > reset_type is invalid. > > Regards, > Anup > >> >> --Sean >> >>> + break; >>> + case SBI_SRST_RESET_TYPE_COLD_REBOOT: >>> + sbi_system_reset(SBI_PLATFORM_RESET_COLD); >>> + break; >>> + case SBI_SRST_RESET_TYPE_WARM_REBOOT: >>> + sbi_system_reset(SBI_PLATFORM_RESET_WARM); >>> + break; >>> + default: >>> + ret = SBI_EINVAL; >>> + break; >>> + }; >>> + } else { >>> + ret = SBI_ENOTSUPP; >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +struct sbi_ecall_extension ecall_srst = { >>> + .extid_start = SBI_EXT_SRST, >>> + .extid_end = SBI_EXT_SRST, >>> + .handle = sbi_ecall_srst_handler, >>> +}; >>> >
> -----Original Message----- > From: Sean Anderson <seanga2@gmail.com> > Sent: 02 October 2020 21:05 > To: Anup Patel <Anup.Patel@wdc.com>; Atish Patra > <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com> > Cc: Anup Patel <anup@brainfault.org>; opensbi@lists.infradead.org > Subject: Re: [RFC PATCH v2] lib: Implement System Reset (SRST) SBI > extension > > On 10/1/20 3:56 AM, Anup Patel wrote: > > > > > >> -----Original Message----- > >> From: Sean Anderson <seanga2@gmail.com> > >> Sent: 30 September 2020 17:07 > >> To: Anup Patel <Anup.Patel@wdc.com>; Atish Patra > >> <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com> > >> Cc: Anup Patel <anup@brainfault.org>; opensbi@lists.infradead.org > >> Subject: Re: [RFC PATCH v2] lib: Implement System Reset (SRST) SBI > >> extension > >> > >> On 9/26/20 2:53 AM, Anup Patel wrote: > >>> The SBI SRST extension specification is in draft state and available > >>> in srbt_v1 branch of: https://github.com/avpatel/riscv-sbi-doc. > >>> > >>> It allows to S-mode software to request system shutdown, cold > >>> reboot, and warm reboot. > >>> > >>> This patch provides implementation of SBI SRST extension. > >>> > >>> Signed-off-by: Anup Patel <anup.patel@wdc.com> > >>> --- > >>> Changes since v1: > >>> - Updated patch as-per latest SBI SRST extension draft spec where > >>> we have only one SBI call with "reset_type" parameter > >>> --- > >>> include/sbi/sbi_ecall.h | 1 + > >>> include/sbi/sbi_ecall_interface.h | 8 ++++++ > >>> lib/sbi/objects.mk | 1 + > >>> lib/sbi/sbi_ecall.c | 3 ++ > >>> lib/sbi/sbi_ecall_srst.c | 48 +++++++++++++++++++++++++++++++ > >>> 5 files changed, 61 insertions(+) > >>> create mode 100644 lib/sbi/sbi_ecall_srst.c > >>> > >>> diff --git a/include/sbi/sbi_ecall.h b/include/sbi/sbi_ecall.h index > >>> 3273ba6..1ef86e2 100644 > >>> --- a/include/sbi/sbi_ecall.h > >>> +++ b/include/sbi/sbi_ecall.h > >>> @@ -37,6 +37,7 @@ extern struct sbi_ecall_extension ecall_rfence; > >>> extern struct sbi_ecall_extension ecall_ipi; extern struct > >>> sbi_ecall_extension ecall_vendor; extern struct sbi_ecall_extension > >>> ecall_hsm; > >>> +extern struct sbi_ecall_extension ecall_srst; > >>> > >>> u16 sbi_ecall_version_major(void); > >>> > >>> diff --git a/include/sbi/sbi_ecall_interface.h > >>> b/include/sbi/sbi_ecall_interface.h > >>> index af30500..d9c2519 100644 > >>> --- a/include/sbi/sbi_ecall_interface.h > >>> +++ b/include/sbi/sbi_ecall_interface.h > >>> @@ -27,6 +27,7 @@ > >>> #define SBI_EXT_IPI 0x735049 > >>> #define SBI_EXT_RFENCE 0x52464E43 > >>> #define SBI_EXT_HSM 0x48534D > >>> +#define SBI_EXT_SRST 0x53525354 > >>> > >>> /* SBI function IDs for BASE extension*/ > >>> #define SBI_EXT_BASE_GET_SPEC_VERSION 0x0 > >>> @@ -62,6 +63,13 @@ > >>> #define SBI_HSM_HART_STATUS_START_PENDING 0x2 > >>> #define SBI_HSM_HART_STATUS_STOP_PENDING 0x3 > >>> > >>> +/* SBI function IDs for SRST extension */ > >>> +#define SBI_EXT_SRST_RESET 0x0 > >>> + > >>> +#define SBI_SRST_RESET_TYPE_SHUTDOWN 0x0 > >>> +#define SBI_SRST_RESET_TYPE_COLD_REBOOT 0x1 > >>> +#define SBI_SRST_RESET_TYPE_WARM_REBOOT 0x2 > >>> + > >>> #define SBI_SPEC_VERSION_MAJOR_OFFSET 24 > >>> #define SBI_SPEC_VERSION_MAJOR_MASK 0x7f > >>> #define SBI_SPEC_VERSION_MINOR_MASK 0xffffff > >>> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk index > >>> fa808a0..6ec1154 100644 > >>> --- a/lib/sbi/objects.mk > >>> +++ b/lib/sbi/objects.mk > >>> @@ -20,6 +20,7 @@ libsbi-objs-y += sbi_ecall_base.o libsbi-objs-y > >>> += sbi_ecall_hsm.o libsbi-objs-y += sbi_ecall_legacy.o > >>> libsbi-objs-y += sbi_ecall_replace.o > >>> +libsbi-objs-y += sbi_ecall_srst.o > >>> libsbi-objs-y += sbi_ecall_vendor.o libsbi-objs-y += > >>> sbi_emulate_csr.o libsbi-objs-y += sbi_fifo.o diff --git > >>> a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c index 64c9933..6d41cff > >>> 100644 > >>> --- a/lib/sbi/sbi_ecall.c > >>> +++ b/lib/sbi/sbi_ecall.c > >>> @@ -167,6 +167,9 @@ int sbi_ecall_init(void) > >>> if (ret) > >>> return ret; > >>> ret = sbi_ecall_register_extension(&ecall_hsm); > >>> + if (ret) > >>> + return ret; > >>> + ret = sbi_ecall_register_extension(&ecall_srst); > >>> if (ret) > >>> return ret; > >>> ret = sbi_ecall_register_extension(&ecall_legacy); > >>> diff --git a/lib/sbi/sbi_ecall_srst.c b/lib/sbi/sbi_ecall_srst.c new > >>> file mode 100644 index 0000000..64c228f > >>> --- /dev/null > >>> +++ b/lib/sbi/sbi_ecall_srst.c > >>> @@ -0,0 +1,48 @@ > >>> +/* > >>> + * SPDX-License-Identifier: BSD-2-Clause > >>> + * > >>> + * Copyright (c) 2020 Western Digital Corporation or its affiliates. > >>> + * > >>> + * Authors: > >>> + * Anup Patel <anup.patel@wdc.com> > >>> + */ > >>> + > >>> +#include <sbi/sbi_ecall.h> > >>> +#include <sbi/sbi_ecall_interface.h> #include <sbi/sbi_error.h> > >>> +#include <sbi/sbi_platform.h> #include <sbi/sbi_system.h> > >>> + > >>> +static int sbi_ecall_srst_handler(unsigned long extid, unsigned > >>> +long > >> funcid, > >>> + unsigned long *args, unsigned long > >> *out_val, > >>> + struct sbi_trap_info *out_trap) { > >>> + int ret = 0; > >>> + > >>> + if (funcid == SBI_EXT_SRST_RESET) { > >>> + switch (args[0]) { > >>> + case SBI_SRST_RESET_TYPE_SHUTDOWN: > >>> + > >> sbi_system_reset(SBI_PLATFORM_RESET_SHUTDOWN); > >> > >> Shouldn't we check a return value here? Some platforms may not > >> support all types of resets, and may want to return SBI_ENOTSUPP. For > >> example, the > >> K210 only supports warm resets. > > > > The sbi_system_reset() is implemented as noreturn function. > > > > We have three cases here: > > 1. A platform has none of the SBI system reset types supported 2. A > > platform has few of the SBI system reset types supported 3. A platform > > has all SBI system reset types supported > > > > For case#1, we will simply halt all HARTs in WFI. > > > > For case#2, we will halt all HARTs in WFI when a particular SBI system > > reset type is not supported by platform. > > > > Basically, current sbi_system_reset() implementation ensures that we > > never return for a valid reset_type irrespective whether underlying > > platform supports it or not. > > I think that should be changed. There must be a way to discover which reset > types are supported by the platform. For example, say that I want to reset > the system, or shut it down only if there is no way to reset it. With this API, I > would have no way to do this. If I try a cold reset and it is not available, the > platform will "shut down" anyway, even if there is a warm reset available. This is a good suggestion. The SBI SRST RESET call should return error when reset_type is not supported by the platform. We should first update the SBI SRST extension draft and then update this patch. (Refer, https://github.com/riscv/riscv-sbi-doc/pull/39) Regards, Anup > > --Sean > > > The only case where SBI SRST RESET call fails is when provided > > reset_type is invalid. > > > > Regards, > > Anup > > > >> > >> --Sean > >> > >>> + break; > >>> + case SBI_SRST_RESET_TYPE_COLD_REBOOT: > >>> + sbi_system_reset(SBI_PLATFORM_RESET_COLD); > >>> + break; > >>> + case SBI_SRST_RESET_TYPE_WARM_REBOOT: > >>> + sbi_system_reset(SBI_PLATFORM_RESET_WARM); > >>> + break; > >>> + default: > >>> + ret = SBI_EINVAL; > >>> + break; > >>> + }; > >>> + } else { > >>> + ret = SBI_ENOTSUPP; > >>> + } > >>> + > >>> + return ret; > >>> +} > >>> + > >>> +struct sbi_ecall_extension ecall_srst = { > >>> + .extid_start = SBI_EXT_SRST, > >>> + .extid_end = SBI_EXT_SRST, > >>> + .handle = sbi_ecall_srst_handler, > >>> +}; > >>> > >
diff --git a/include/sbi/sbi_ecall.h b/include/sbi/sbi_ecall.h index 3273ba6..1ef86e2 100644 --- a/include/sbi/sbi_ecall.h +++ b/include/sbi/sbi_ecall.h @@ -37,6 +37,7 @@ extern struct sbi_ecall_extension ecall_rfence; extern struct sbi_ecall_extension ecall_ipi; extern struct sbi_ecall_extension ecall_vendor; extern struct sbi_ecall_extension ecall_hsm; +extern struct sbi_ecall_extension ecall_srst; u16 sbi_ecall_version_major(void); diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h index af30500..d9c2519 100644 --- a/include/sbi/sbi_ecall_interface.h +++ b/include/sbi/sbi_ecall_interface.h @@ -27,6 +27,7 @@ #define SBI_EXT_IPI 0x735049 #define SBI_EXT_RFENCE 0x52464E43 #define SBI_EXT_HSM 0x48534D +#define SBI_EXT_SRST 0x53525354 /* SBI function IDs for BASE extension*/ #define SBI_EXT_BASE_GET_SPEC_VERSION 0x0 @@ -62,6 +63,13 @@ #define SBI_HSM_HART_STATUS_START_PENDING 0x2 #define SBI_HSM_HART_STATUS_STOP_PENDING 0x3 +/* SBI function IDs for SRST extension */ +#define SBI_EXT_SRST_RESET 0x0 + +#define SBI_SRST_RESET_TYPE_SHUTDOWN 0x0 +#define SBI_SRST_RESET_TYPE_COLD_REBOOT 0x1 +#define SBI_SRST_RESET_TYPE_WARM_REBOOT 0x2 + #define SBI_SPEC_VERSION_MAJOR_OFFSET 24 #define SBI_SPEC_VERSION_MAJOR_MASK 0x7f #define SBI_SPEC_VERSION_MINOR_MASK 0xffffff diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk index fa808a0..6ec1154 100644 --- a/lib/sbi/objects.mk +++ b/lib/sbi/objects.mk @@ -20,6 +20,7 @@ libsbi-objs-y += sbi_ecall_base.o libsbi-objs-y += sbi_ecall_hsm.o libsbi-objs-y += sbi_ecall_legacy.o libsbi-objs-y += sbi_ecall_replace.o +libsbi-objs-y += sbi_ecall_srst.o libsbi-objs-y += sbi_ecall_vendor.o libsbi-objs-y += sbi_emulate_csr.o libsbi-objs-y += sbi_fifo.o diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c index 64c9933..6d41cff 100644 --- a/lib/sbi/sbi_ecall.c +++ b/lib/sbi/sbi_ecall.c @@ -167,6 +167,9 @@ int sbi_ecall_init(void) if (ret) return ret; ret = sbi_ecall_register_extension(&ecall_hsm); + if (ret) + return ret; + ret = sbi_ecall_register_extension(&ecall_srst); if (ret) return ret; ret = sbi_ecall_register_extension(&ecall_legacy); diff --git a/lib/sbi/sbi_ecall_srst.c b/lib/sbi/sbi_ecall_srst.c new file mode 100644 index 0000000..64c228f --- /dev/null +++ b/lib/sbi/sbi_ecall_srst.c @@ -0,0 +1,48 @@ +/* + * SPDX-License-Identifier: BSD-2-Clause + * + * Copyright (c) 2020 Western Digital Corporation or its affiliates. + * + * Authors: + * Anup Patel <anup.patel@wdc.com> + */ + +#include <sbi/sbi_ecall.h> +#include <sbi/sbi_ecall_interface.h> +#include <sbi/sbi_error.h> +#include <sbi/sbi_platform.h> +#include <sbi/sbi_system.h> + +static int sbi_ecall_srst_handler(unsigned long extid, unsigned long funcid, + unsigned long *args, unsigned long *out_val, + struct sbi_trap_info *out_trap) +{ + int ret = 0; + + if (funcid == SBI_EXT_SRST_RESET) { + switch (args[0]) { + case SBI_SRST_RESET_TYPE_SHUTDOWN: + sbi_system_reset(SBI_PLATFORM_RESET_SHUTDOWN); + break; + case SBI_SRST_RESET_TYPE_COLD_REBOOT: + sbi_system_reset(SBI_PLATFORM_RESET_COLD); + break; + case SBI_SRST_RESET_TYPE_WARM_REBOOT: + sbi_system_reset(SBI_PLATFORM_RESET_WARM); + break; + default: + ret = SBI_EINVAL; + break; + }; + } else { + ret = SBI_ENOTSUPP; + } + + return ret; +} + +struct sbi_ecall_extension ecall_srst = { + .extid_start = SBI_EXT_SRST, + .extid_end = SBI_EXT_SRST, + .handle = sbi_ecall_srst_handler, +};
The SBI SRST extension specification is in draft state and available in srbt_v1 branch of: https://github.com/avpatel/riscv-sbi-doc. It allows to S-mode software to request system shutdown, cold reboot, and warm reboot. This patch provides implementation of SBI SRST extension. Signed-off-by: Anup Patel <anup.patel@wdc.com> --- Changes since v1: - Updated patch as-per latest SBI SRST extension draft spec where we have only one SBI call with "reset_type" parameter --- include/sbi/sbi_ecall.h | 1 + include/sbi/sbi_ecall_interface.h | 8 ++++++ lib/sbi/objects.mk | 1 + lib/sbi/sbi_ecall.c | 3 ++ lib/sbi/sbi_ecall_srst.c | 48 +++++++++++++++++++++++++++++++ 5 files changed, 61 insertions(+) create mode 100644 lib/sbi/sbi_ecall_srst.c