[RFC PATCH v2] lib: Implement System Reset (SRST) SBI extension

Anup Patel Anup.Patel at wdc.com
Thu Oct 1 03:56:43 EDT 2020



> -----Original Message-----
> From: Sean Anderson <seanga2 at gmail.com>
> Sent: 30 September 2020 17:07
> To: Anup Patel <Anup.Patel at wdc.com>; Atish Patra
> <Atish.Patra at wdc.com>; Alistair Francis <Alistair.Francis at wdc.com>
> Cc: Anup Patel <anup at brainfault.org>; opensbi at 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 at 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 at 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,
> > +};
> >



More information about the opensbi mailing list