Date   

Re: [RISC-V] [software] Add SBI extension space for firmware code base implementation

Jonathan Behrens <behrensj@...>
 

What I was suggesting is something like:

> Implementation specific SBI extension Space, Extension IDs 0x0A000000 through 0x0AFFFFFF. Low bits from SBI Implementation ID. 

In this scheme, each SBI implementation would have one extension reserved for it: BBL gets 0x0A000000, OpenSBI gets 0x0A000001, Xvisor gets 0x0A000002, KVM gets 0x0A000003, etc. That might not seem like a lot of space, but each extension can have up to 2^32 different functions (including ones for version number discovery, etc.) so it shouldn't actually be limiting.

As a side note, I don't think edk2 has an SBI Implementation ID assigned yet. You should just be able to ask for one and get it.

Jonathan


On Fri, Apr 10, 2020 at 2:23 AM Chang, Abner (HPS SW/FW Technologist) <abner.chang@...> wrote:


> -----Original Message-----
> From: tech-unixplatformspec@... [mailto:tech-
> unixplatformspec@...] On Behalf Of Atish Patra
> Sent: Friday, April 10, 2020 11:32 AM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>;
> behrensj@...
> Cc: Schaefer, Daniel (DualStudy) <daniel.schaefer@...>; tech-
> unixplatformspec@...; Anup Patel <Anup.Patel@...>
> Subject: Re: [RISC-V] [tech-unixplatformspec] [RISC-V] [software] Add SBI
> extension space for firmware code base implementation
>
> On Fri, 2020-04-10 at 01:08 +0000, Chang, Abner (HPS SW/FW
> Technologist) wrote:
> > Got it, Software ML removed.
> >
> > Johnathan, do you mean to define it as below?
> > Firmware Base Extension, Extension ID: 0xxxxxxxxx  (FWBE) , and the
> > SBI functions definition is firmware code base implementation-
> > specific.
> >
>
> I prefer this one as well. But you should pick any value within experimental
> range.
Not quite follow this, why pick up one from experimental range?  The extension ID must be in the range of 0x0800000-0x08ffffff?

I am not sure about the purpose of the extension but if it has the
> potential to be a generic SBI extension for firmwares, you should propose it
> to the Unix Platform working group. We can add it to the official spec.
For those SBI extension which is generic to all firmware code bases, then we should just propose it to the official SBI spec and don’t not have to go with Firmware Base Extension.
Firmware Base Extension is classified to those firmware code base specific SBI, for example to load an edk2 driver into M-mode managed memory and executed in M-mode which is behaved as secured system manage mode driver. So the detail Firmware Base Extension would be defined in the separate spec and not part of official sbi spec. Occupied an SBI extension ID is to avoid conflicts.

>
> > This also works for me and better than to reserve a range of IDs.
> > Thanks
> > Abner
> >
> > From: software@... [mailto:software@...] On
> > Behalf Of Jonathan Behrens
> > Sent: Friday, April 10, 2020 12:39 AM
> > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
> > Cc: Atish Patra <Atish.Patra@...>; Anup Patel
> <anup.patel@...
> > >; Schaefer, Daniel (DualStudy) <daniel.schaefer@...>;
> > software@...; tech-unixplatformspec@...
> > Subject: Re: [RISC-V] [software] Add SBI extension space for firmware
> > code base implementation
> >
> > I think tech-unixplatformspec@... might be the right list?
> >
> > To address the PR itself, I'd personally prefer to have this space
> > allocated based on SBI implementation IDs analogously to how the
> > vendor space is allocated. It also probably makes sense to pick a
> > different address range so experimental extension space doesn't have
> > to move.
> >
> > Jonathan
> >
> > On Thu, Apr 9, 2020 at 4:39 AM Abner Chang via lists.riscv.org <
> > abner.chang=hpe.com@...> wrote:
> > > Seems opensbi ML is no longer exist? Use software@...
> > > insrtead.
> > >
> > > From: Chang, Abner (HPS SW/FW Technologist)
> > > Sent: Thursday, April 9, 2020 4:23 PM
> > > To: Atish Patra <Atish.Patra@...>; Anup Patel <
> > > anup.patel@...>
> > > Cc: opensbi@...; Schaefer, Daniel (DualStudy) <
> > > daniel.schaefer@...>
> > > Subject: Add SBI extension space for firmware code base
> > > implementation
> > >
> > > Hi Atish and Anup,
> > > We are working on some edk2-specific SBI extension which intends to
> > > be used by upper layer edk2 drivers. We originally consider to use
> > > Vendor Extension space however vendor may have its own proprietary
> > > SBI extension as well. In order to prevent from the conflicts of SBI
> > > extension space, we propose to have a range for firmware code base.
> > > The changes look like the PR below,
> > >
> > > https://github.com/riscv/riscv-sbi-doc/pull/43
> > >
> > > How do you think?
> > > Thanks
> > > Abner
> >
> >
>
> --
> Regards,
> Atish
>
>


Re: [RISC-V] [software] Add SBI extension space for firmware code base implementation

Abner Chang <abner.chang@...>
 

-----Original Message-----
From: tech-unixplatformspec@... [mailto:tech-
unixplatformspec@...] On Behalf Of Atish Patra
Sent: Friday, April 10, 2020 11:32 AM
To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>;
behrensj@...
Cc: Schaefer, Daniel (DualStudy) <daniel.schaefer@...>; tech-
unixplatformspec@...; Anup Patel <Anup.Patel@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [RISC-V] [software] Add SBI
extension space for firmware code base implementation

On Fri, 2020-04-10 at 01:08 +0000, Chang, Abner (HPS SW/FW
Technologist) wrote:
Got it, Software ML removed.

Johnathan, do you mean to define it as below?
Firmware Base Extension, Extension ID: 0xxxxxxxxx (FWBE) , and the
SBI functions definition is firmware code base implementation-
specific.
I prefer this one as well. But you should pick any value within experimental
range.
Not quite follow this, why pick up one from experimental range? The extension ID must be in the range of 0x0800000-0x08ffffff?

I am not sure about the purpose of the extension but if it has the
potential to be a generic SBI extension for firmwares, you should propose it
to the Unix Platform working group. We can add it to the official spec.
For those SBI extension which is generic to all firmware code bases, then we should just propose it to the official SBI spec and don’t not have to go with Firmware Base Extension.
Firmware Base Extension is classified to those firmware code base specific SBI, for example to load an edk2 driver into M-mode managed memory and executed in M-mode which is behaved as secured system manage mode driver. So the detail Firmware Base Extension would be defined in the separate spec and not part of official sbi spec. Occupied an SBI extension ID is to avoid conflicts.


This also works for me and better than to reserve a range of IDs.
Thanks
Abner

From: software@... [mailto:software@...] On
Behalf Of Jonathan Behrens
Sent: Friday, April 10, 2020 12:39 AM
To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
Cc: Atish Patra <Atish.Patra@...>; Anup Patel
<anup.patel@...
; Schaefer, Daniel (DualStudy) <daniel.schaefer@...>;
software@...; tech-unixplatformspec@...
Subject: Re: [RISC-V] [software] Add SBI extension space for firmware
code base implementation

I think tech-unixplatformspec@... might be the right list?

To address the PR itself, I'd personally prefer to have this space
allocated based on SBI implementation IDs analogously to how the
vendor space is allocated. It also probably makes sense to pick a
different address range so experimental extension space doesn't have
to move.

Jonathan

On Thu, Apr 9, 2020 at 4:39 AM Abner Chang via lists.riscv.org <
abner.chang=hpe.com@...> wrote:
Seems opensbi ML is no longer exist? Use software@...
insrtead.

From: Chang, Abner (HPS SW/FW Technologist)
Sent: Thursday, April 9, 2020 4:23 PM
To: Atish Patra <Atish.Patra@...>; Anup Patel <
anup.patel@...>
Cc: opensbi@...; Schaefer, Daniel (DualStudy) <
daniel.schaefer@...>
Subject: Add SBI extension space for firmware code base
implementation

Hi Atish and Anup,
We are working on some edk2-specific SBI extension which intends to
be used by upper layer edk2 drivers. We originally consider to use
Vendor Extension space however vendor may have its own proprietary
SBI extension as well. In order to prevent from the conflicts of SBI
extension space, we propose to have a range for firmware code base.
The changes look like the PR below,

https://github.com/riscv/riscv-sbi-doc/pull/43

How do you think?
Thanks
Abner
--
Regards,
Atish


Re: [RISC-V] [software] Add SBI extension space for firmware code base implementation

atishp@...
 

On Fri, 2020-04-10 at 01:08 +0000, Chang, Abner (HPS SW/FW
Technologist) wrote:
Got it, Software ML removed.

Johnathan, do you mean to define it as below?
Firmware Base Extension, Extension ID: 0xxxxxxxxx (FWBE) , and the
SBI functions definition is firmware code base implementation-
specific.
I prefer this one as well. But you should pick any value within
experimental range. I am not sure about the purpose of the extension
but if it has the potential to be a generic SBI extension for
firmwares, you should propose it to the Unix Platform working group. We
can add it to the official spec.

This also works for me and better than to reserve a range of IDs.
Thanks
Abner

From: software@... [mailto:software@...] On
Behalf Of Jonathan Behrens
Sent: Friday, April 10, 2020 12:39 AM
To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
Cc: Atish Patra <Atish.Patra@...>; Anup Patel <anup.patel@...
; Schaefer, Daniel (DualStudy) <daniel.schaefer@...>;
software@...; tech-unixplatformspec@...
Subject: Re: [RISC-V] [software] Add SBI extension space for firmware
code base implementation

I think tech-unixplatformspec@... might be the right
list?

To address the PR itself, I'd personally prefer to have this space
allocated based on SBI implementation IDs analogously to how the
vendor space is allocated. It also probably makes sense to pick a
different address range so experimental extension space doesn't have
to move.

Jonathan

On Thu, Apr 9, 2020 at 4:39 AM Abner Chang via lists.riscv.org <
abner.chang=hpe.com@...> wrote:
Seems opensbi ML is no longer exist? Use software@...
insrtead.

From: Chang, Abner (HPS SW/FW Technologist)
Sent: Thursday, April 9, 2020 4:23 PM
To: Atish Patra <Atish.Patra@...>; Anup Patel <
anup.patel@...>
Cc: opensbi@...; Schaefer, Daniel (DualStudy) <
daniel.schaefer@...>
Subject: Add SBI extension space for firmware code base
implementation

Hi Atish and Anup,
We are working on some edk2-specific SBI extension which intends to
be used by upper layer edk2 drivers. We originally consider to use
Vendor Extension space however vendor may have its own proprietary
SBI extension as well. In order to prevent from the conflicts of
SBI extension space, we propose to have a range for firmware code
base. The changes look like the PR below,

https://github.com/riscv/riscv-sbi-doc/pull/43

How do you think?
Thanks
Abner
--
Regards,
Atish


Re: [RISC-V] [software] Add SBI extension space for firmware code base implementation

Abner Chang <abner.chang@...>
 

Got it, Software ML removed.

 

Johnathan, do you mean to define it as below?

Firmware Base Extension, Extension ID: 0xxxxxxxxx  (FWBE) , and the SBI functions definition is firmware code base implementation-specific.

 

This also works for me and better than to reserve a range of IDs.

Thanks

Abner

 

From: software@... [mailto:software@...] On Behalf Of Jonathan Behrens
Sent: Friday, April 10, 2020 12:39 AM
To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
Cc: Atish Patra <Atish.Patra@...>; Anup Patel <anup.patel@...>; Schaefer, Daniel (DualStudy) <daniel.schaefer@...>; software@...; tech-unixplatformspec@...
Subject: Re: [RISC-V] [software] Add SBI extension space for firmware code base implementation

 

I think tech-unixplatformspec@... might be the right list?

 

To address the PR itself, I'd personally prefer to have this space allocated based on SBI implementation IDs analogously to how the vendor space is allocated. It also probably makes sense to pick a different address range so experimental extension space doesn't have to move.

 

Jonathan

 

On Thu, Apr 9, 2020 at 4:39 AM Abner Chang via lists.riscv.org <abner.chang=hpe.com@...> wrote:

Seems opensbi ML is no longer exist? Use software@... insrtead.

 

From: Chang, Abner (HPS SW/FW Technologist)
Sent: Thursday, April 9, 2020 4:23 PM
To: Atish Patra <Atish.Patra@...>; Anup Patel <anup.patel@...>
Cc: opensbi@...; Schaefer, Daniel (DualStudy) <daniel.schaefer@...>
Subject: Add SBI extension space for firmware code base implementation

 

Hi Atish and Anup,

We are working on some edk2-specific SBI extension which intends to be used by upper layer edk2 drivers. We originally consider to use Vendor Extension space however vendor may have its own proprietary SBI extension as well. In order to prevent from the conflicts of SBI extension space, we propose to have a range for firmware code base. The changes look like the PR below,

 

https://github.com/riscv/riscv-sbi-doc/pull/43

 

How do you think?

Thanks

Abner


Re: [RISC-V] [software] Add SBI extension space for firmware code base implementation

Jonathan Behrens <behrensj@...>
 

I think tech-unixplatformspec@... might be the right list?

To address the PR itself, I'd personally prefer to have this space allocated based on SBI implementation IDs analogously to how the vendor space is allocated. It also probably makes sense to pick a different address range so experimental extension space doesn't have to move.

Jonathan


On Thu, Apr 9, 2020 at 4:39 AM Abner Chang via lists.riscv.org <abner.chang=hpe.com@...> wrote:

Seems opensbi ML is no longer exist? Use software@... insrtead.

 

From: Chang, Abner (HPS SW/FW Technologist)
Sent: Thursday, April 9, 2020 4:23 PM
To: Atish Patra <Atish.Patra@...>; Anup Patel <anup.patel@...>
Cc: opensbi@...; Schaefer, Daniel (DualStudy) <daniel.schaefer@...>
Subject: Add SBI extension space for firmware code base implementation

 

Hi Atish and Anup,

We are working on some edk2-specific SBI extension which intends to be used by upper layer edk2 drivers. We originally consider to use Vendor Extension space however vendor may have its own proprietary SBI extension as well. In order to prevent from the conflicts of SBI extension space, we propose to have a range for firmware code base. The changes look like the PR below,

 

https://github.com/riscv/riscv-sbi-doc/pull/43

 

How do you think?

Thanks

Abner


Re: [PATCH v3] Add system reboot extension

Bin Meng
 

On Mon, Apr 6, 2020 at 11:27 AM Anup Patel <anup.patel@...> wrote:

This patch adds SBI v0.2 compliant system reboot extension. It defines
the sbi_system_reboot function which does different things based on
reboot_type parameter:
1. shutdown - Power-off the entire system
2. cold reboot - Power-cycle the entire system
3. warm reboot - Power-cycle only parts of system based on SOC vendor
design choices

The sbi_system_reboot function defined here is also a replacement of
SBI v0.1 shutdown function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-sbi.adoc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
A general comment: it would be good to put the reason why a new
extension is introduced, to solve what issue.

Reviewed-by: Bin Meng <bmeng.cn@...>


[PATCH v3] Add system reboot extension

Anup Patel
 

This patch adds SBI v0.2 compliant system reboot extension. It defines
the sbi_system_reboot function which does different things based on
reboot_type parameter:
1. shutdown - Power-off the entire system
2. cold reboot - Power-cycle the entire system
3. warm reboot - Power-cycle only parts of system based on SOC vendor
design choices

The sbi_system_reboot function defined here is also a replacement of
SBI v0.1 shutdown function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-sbi.adoc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 8137686..a55a996 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -615,6 +615,53 @@ state of the hart at the time of return value verification.
| sbi_hart_get_status | 2 | 0x48534D
|===

+== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
+
+The System Reboot Extension provides a function that allow the supervisor
+software to request system-level reboot or shutdown.
+
+[source, C]
+----
+struct sbiret sbi_system_reboot(unsigned long reboot_type)
+----
+
+Reboot the system based on provided reboot type. This is a synchronous call
+and does not return if it succeeds.
+
+The reboot_type parameter is 32 bit wide and has following possible values:
+
+[cols="<,>",options="header,compact"]
+|===
+| Value | Description
+| 0x00000000 | Shutdown
+| 0x00000001 | Cold reboot
+| 0x00000002 | Warm reboot
+| 0x00000003 - 0xEFFFFFFF | Reserved for future use
+| 0xF0000000 - 0xFFFFFFFF | Vendor or platform specific reboot type
+| 0x100000000 - 2^XELN-1 | Reserved and unusable on RV32
+|===
+
+Cold reboot results in complete power cycle of the entire system while warm
+reboot depends on SOC vendor design choices.
+
+*Returns* one of the following possible SBI error codes through sbiret.error
+upon failure.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code | Description
+| SBI_ERR_INVALID_PARAM | `reboot_type` is not valid.
+| SBI_ERR_FAILED | Reboot request failed for unknown reasons.
+|===
+
+=== SRBT Function Listing
+
+[cols="<,,>",options="header,compact"]
+|===
+| Function Name | Function ID | Extension ID
+| sbi_system_reboot | 0 | 0x53524254
+|===
+
== Experimental SBI Extension Space, Extension IDs 0x0800000 through 0x08FFFFFF

No management.
--
2.17.1


Re: [PATCH v2] Add system reboot extension

Anup Patel
 

Sure, I will fix this in next revision.

 

Thanks for catching.

 

Regards,

Anup

 

From: tech-unixplatformspec@... <tech-unixplatformspec@...> On Behalf Of Jonathan Behrens
Sent: 03 April 2020 20:18
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra <Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH v2] Add system reboot extension

 

> The System Reboot Extension provides a set of functions that allow the

 

nit: "a set of functions" -> "a function"

 

On Fri, Apr 3, 2020 at 1:36 AM Anup Patel via lists.riscv.org <anup.patel=wdc.com@...> wrote:

This patch adds SBI v0.2 compliant system reboot extension. It defines
the sbi_system_reboot function which does different things based on
reboot_type parameter:
1. shutdown    - Power-off the entire system
2. cold reboot - Power-cycle the entire system
3. warm reboot - Power-cycle only parts of system based on SOC vendor
                 design choices

The sbi_system_reboot function defined here is also an replacement of
SBI v0.1 shutdown function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
 riscv-sbi.adoc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 8137686..d93a5c2 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -615,6 +615,53 @@ state of the hart at the time of return value verification.
 | sbi_hart_get_status          |           2 |     0x48534D
 |===

+== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
+
+The System Reboot Extension provides a set of functions that allow the
+supervisor software to request system-level reboot or shutdown.
+
+[source, C]
+----
+struct sbiret sbi_system_reboot(unsigned long reboot_type)
+----
+
+Reboot the system based on provided reboot type. This is a synchronous call
+and does not return if it succeeds.
+
+The reboot_type parameter is 32 bit wide and has following possible values:
+
+[cols="<,>",options="header,compact"]
+|===
+| Value                        | Description
+| 0x00000000                   | Shutdown
+| 0x00000001                   | Cold reboot
+| 0x00000002                   | Warm reboot
+| 0x00000003 - 0xEFFFFFFF      | Reserved for future use
+| 0xF0000000 - 0xFFFFFFFF      | Vendor or platform specific reboot type
+| 0x100000000 - 2^XELN-1       | Reserved and unusable on RV32
+|===
+
+Cold reboot results in complete power cycle of the entire system while
+warm reboot depends on SOC vendor design choices.
+
+*Returns* one of the following possible SBI error codes through sbiret.error
+upon failure.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code                | Description
+| SBI_ERR_INVALID_PARAM     | `reboot_type` is not valid.
+| SBI_ERR_FAILED            | Reboot request failed for unknown reasons.
+|===
+
+=== SRBT Function Listing
+
+[cols="<,,>",options="header,compact"]
+|===
+| Function Name                 | Function ID | Extension ID
+| sbi_system_reboot                    |           0 |   0x53524254
+|===
+
 == Experimental SBI Extension Space, Extension IDs 0x0800000 through 0x08FFFFFF

 No management.
--
2.17.1




Re: [PATCH v2] Add system reboot extension

Jonathan Behrens <behrensj@...>
 

> The System Reboot Extension provides a set of functions that allow the

nit: "a set of functions" -> "a function"


On Fri, Apr 3, 2020 at 1:36 AM Anup Patel via lists.riscv.org <anup.patel=wdc.com@...> wrote:
This patch adds SBI v0.2 compliant system reboot extension. It defines
the sbi_system_reboot function which does different things based on
reboot_type parameter:
1. shutdown    - Power-off the entire system
2. cold reboot - Power-cycle the entire system
3. warm reboot - Power-cycle only parts of system based on SOC vendor
                 design choices

The sbi_system_reboot function defined here is also an replacement of
SBI v0.1 shutdown function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
 riscv-sbi.adoc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 8137686..d93a5c2 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -615,6 +615,53 @@ state of the hart at the time of return value verification.
 | sbi_hart_get_status          |           2 |     0x48534D
 |===

+== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
+
+The System Reboot Extension provides a set of functions that allow the
+supervisor software to request system-level reboot or shutdown.
+
+[source, C]
+----
+struct sbiret sbi_system_reboot(unsigned long reboot_type)
+----
+
+Reboot the system based on provided reboot type. This is a synchronous call
+and does not return if it succeeds.
+
+The reboot_type parameter is 32 bit wide and has following possible values:
+
+[cols="<,>",options="header,compact"]
+|===
+| Value                        | Description
+| 0x00000000                   | Shutdown
+| 0x00000001                   | Cold reboot
+| 0x00000002                   | Warm reboot
+| 0x00000003 - 0xEFFFFFFF      | Reserved for future use
+| 0xF0000000 - 0xFFFFFFFF      | Vendor or platform specific reboot type
+| 0x100000000 - 2^XELN-1       | Reserved and unusable on RV32
+|===
+
+Cold reboot results in complete power cycle of the entire system while
+warm reboot depends on SOC vendor design choices.
+
+*Returns* one of the following possible SBI error codes through sbiret.error
+upon failure.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code                | Description
+| SBI_ERR_INVALID_PARAM     | `reboot_type` is not valid.
+| SBI_ERR_FAILED            | Reboot request failed for unknown reasons.
+|===
+
+=== SRBT Function Listing
+
+[cols="<,,>",options="header,compact"]
+|===
+| Function Name                 | Function ID | Extension ID
+| sbi_system_reboot                    |           0 |   0x53524254
+|===
+
 == Experimental SBI Extension Space, Extension IDs 0x0800000 through 0x08FFFFFF

 No management.
--
2.17.1





[PATCH v2] Add system reboot extension

Anup Patel
 

This patch adds SBI v0.2 compliant system reboot extension. It defines
the sbi_system_reboot function which does different things based on
reboot_type parameter:
1. shutdown - Power-off the entire system
2. cold reboot - Power-cycle the entire system
3. warm reboot - Power-cycle only parts of system based on SOC vendor
design choices

The sbi_system_reboot function defined here is also an replacement of
SBI v0.1 shutdown function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-sbi.adoc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 8137686..d93a5c2 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -615,6 +615,53 @@ state of the hart at the time of return value verification.
| sbi_hart_get_status | 2 | 0x48534D
|===

+== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
+
+The System Reboot Extension provides a set of functions that allow the
+supervisor software to request system-level reboot or shutdown.
+
+[source, C]
+----
+struct sbiret sbi_system_reboot(unsigned long reboot_type)
+----
+
+Reboot the system based on provided reboot type. This is a synchronous call
+and does not return if it succeeds.
+
+The reboot_type parameter is 32 bit wide and has following possible values:
+
+[cols="<,>",options="header,compact"]
+|===
+| Value | Description
+| 0x00000000 | Shutdown
+| 0x00000001 | Cold reboot
+| 0x00000002 | Warm reboot
+| 0x00000003 - 0xEFFFFFFF | Reserved for future use
+| 0xF0000000 - 0xFFFFFFFF | Vendor or platform specific reboot type
+| 0x100000000 - 2^XELN-1 | Reserved and unusable on RV32
+|===
+
+Cold reboot results in complete power cycle of the entire system while
+warm reboot depends on SOC vendor design choices.
+
+*Returns* one of the following possible SBI error codes through sbiret.error
+upon failure.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code | Description
+| SBI_ERR_INVALID_PARAM | `reboot_type` is not valid.
+| SBI_ERR_FAILED | Reboot request failed for unknown reasons.
+|===
+
+=== SRBT Function Listing
+
+[cols="<,,>",options="header,compact"]
+|===
+| Function Name | Function ID | Extension ID
+| sbi_system_reboot | 0 | 0x53524254
+|===
+
== Experimental SBI Extension Space, Extension IDs 0x0800000 through 0x08FFFFFF

No management.
--
2.17.1


Re: [PATCH 0/1] SBI: Introduce Physical Memory Protection Extension

Jonathan Behrens <behrensj@...>
 

There are different sorts of security threats to keep in mind. I'm generally not a fan of threat models where the "person who legally purchased the computer" is the threat, but that is admittedly a case hardware vendors often care about. Another security concern for many organizations is basically the reverse: somebody has attached an extra device or some DRAM with unauthorized M-mode code that the operating system software doesn't know about, and now it is being used to exfiltrate data from the system. This isn't necessarily the place to try to resolve this sort of threat, but it is worth keeping in mind that decisions we make don't make it even harder to address.

Jonathan

(sorry for the duplicate... my other address isn't subscribed to the mailing list)

On Wed, Apr 1, 2020 at 9:48 PM Nick Kossifidis <mick@...> wrote:
Hello all,

>
> Let's continue the discussion here as it has a wider audience than
> github PR. I have also cc'd all the possible stakeholders.
>
> The other alternatives propsed so far
>
> 1. Just parse the deveice tree node in S-mode software.
>
> Pros: No additional SBI extensions are required.
> Cons: U-Boot is responsible for copying the reserved-memory node from
> the DT passed by OpenSBI and set it to the final DT if it is a
> different one. OpenSBI also need to provide the DT where the previous
> stage (FSBL/U-Boot SPL) doesn't provide a DT.
>
> IMHO, this is not a very difficult problem to solve. The U-Boot
> implementation is available here (just ~40 lines of code).
> https://patchwork.ozlabs.org/patch/1254740/
>
> 2. Trap-n-Emulate PMP CSR reads from S-mode. The details are available
> here.
>
> https://github.com/riscv/riscv-sbi-doc/pull/37#issuecomment-596944084
>
> Pros: No SBI extension or prior DTB fixup required in U-Boot.
> Cons: As PMP extensions proposals are already in place, this may need
> to be extended. Privilege spec may need to be modified to explicitly
> say that M-mode can provide PMP csr emulation.
>
>
> Any thoughts ?


The assumption that there'll always be a PMP rule for marking the
firmware's memory isn't always true. M-mode can access any region
without a matching PMP rule so basically the firmware can be anywhere
and work fine, without any PMP rule marking its region. With the current
PMP spec this is probably the best approach since it will also prevent
S-mode from ever accessing this region (S-mode will fail if there is no
matching PMP rule). Why waste a rule for the firmware's region to deny
any access from S-mode, when you can have the same result by not putting
that rule on PMP ? Even if that assumption was always true, what about
systems without PMP, systems with a different number of PMP regions, or
systems with custom PMP-equivalent solutions ? Trap-n-emulate is not
that generic (e.g. in the case of different number of PMP regions). Any
interface from S-mode to PMP or a PMP-equivalent mechanism, should be
more abstract than this IMHO.

We should also consider the security implications of exposing the PMP
configuration to a less privileged mode. It's one thing to let S-mode
know about the DRAM regions that it can't touch because the firmware is
there, and another to reveal the full PMP configuration. PMP may contain
settings that S-mode doesn't even know about, devices that are not on
the device tree for example or DRAM regions that are outside the device
tree's /memory node range (so S-mode won't touch them anyway). I'm not a
fan of security through obscurity but there are good reasons to keep PMP
configuration accessible only to M-mode (in addition to having a more
abstract API with S-mode to be able to handle the various system
configurations / implementations). We can have S-mode request M-mode to
protect specific physical regions that S-mode manages through PMP (or a
vendor-specific PMP-equivalent mechanism) but anything more than that
doesn't make much sense to me.

If we just want to prevent S-mode from touching firmware's memory we can
either exclude its region from the /memory node on the device tree, or
let the firmware modify the device tree it passes on to the next-stage
boot loader and add a reserved-memory node. I think that's the best
approach since it's the most generic one and it's standards-compliant,
plus the device-tree is platform-specific anyway, even if it's included
on the kernel blob directly without passing through OpenSBI or whatever
boot loader is there, we can always add there a reserved region, or
modify the /memory node manually.

Regards,
Nick


Re: [PATCH] Add system reboot extension

Anup Patel
 

The QEMU “SiFive Test Finisher” device has following issues:

  1. It is not a dedicated reboot/shutdown device. In fact, this device is meant to report test PASS or FAIL to QEMU users.
  2. It does not distinguish between “warm-reboot” and “cold-reboot”.
  3. There is no well-defined spec for “SiFive Test Finisher” device so we first need a spec for this device with improved reboot/shutdown functionality. Even if a spec for “SiFive Test Finisher” is available still it is not guaranteed that all RISC-V SOC vendors will implement it. In fact, SiFive FU540 SOC does not have “SiFive Test Finisher” device.
  4. Due to missing spec, I am not sure which all Hypervisors will be willing to emulate it for Guest/VM

 

Regards,

Anup

 

From: tech-unixplatformspec@... <tech-unixplatformspec@...> On Behalf Of Jonathan Behrens
Sent: 01 April 2020 22:43
To: Anup Patel <Anup.Patel@...>
Cc: Bin Meng <bmeng.cn@...>; tech-unixplatformspec@...; Atish Patra <Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add system reboot extension

 

 

 

On Wed, Apr 1, 2020 at 12:35 PM Anup Patel via Lists.Riscv.Org <anup.patel=wdc.com@...> wrote:



> -----Original Message-----
> From: Bin Meng <bmeng.cn@...>
> Sent: 01 April 2020 10:29
> To: Anup Patel <Anup.Patel@...>
> Cc: tech-unixplatformspec@...; Atish Patra
> <Atish.Patra@...>
> Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add system reboot
> extension
>
> On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:
> >
> > This patch adds SBI v0.2 compliant system reboot extension. It defines
> > two functions:
> > 1. sbi_reboot - A system reboot call with reboot type as parameter 2.
> > sbi_shutdown - A system shutdown/poweroff call
> >
> > The sbi_shutdown function defined here replaces SBI v0.1 shutdown
> > function.
> >
> > Signed-off-by: Atish Patra <atish.patra@...>
> > Signed-off-by: Anup Patel <anup.patel@...>
> > ---
> >  riscv-sbi.adoc | 64
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
>
> One generic comment, pretty much similar to the SBI PMP extension I
> proposed, is that why is this necessary to introduce a new SBI extension to
> support reboot and shutdown?
>
> Do these functionalities have to be operated from M-mode?

There are two cases:

1. If a system is partitioned between secure and non-secure world then we
cannot allow non-secure S-mode software to shutdown/reboot the system
without secure S-mode software knowing about it. The SBI shutdown/reboot
calls help M-mode software (OpenSBI) to mediate the shutdown/reboot
request coming from non-secure S-mode software.

2. In virtualization world, we don't have a standard way to shutdown/reboot
Guest/VM across architectures. To tackle this, we generally have architecture
specific hypercall for shutdown/reboot. For RISC-V, we don't want each
hypervisor coming up with its own hypercalls so we standardize these as
SBI calls (This rationale is similar to ARM PSCI shutdown/reboot calls).

 

Doesn't QEMU already have a "SiFive Test Finisher" for this purpose?

 

Regards,
Anup



Re: [PATCH 0/1] SBI: Introduce Physical Memory Protection Extension

Nick Kossifidis
 

Hello all,

Let's continue the discussion here as it has a wider audience than
github PR. I have also cc'd all the possible stakeholders.
The other alternatives propsed so far
1. Just parse the deveice tree node in S-mode software.
Pros: No additional SBI extensions are required.
Cons: U-Boot is responsible for copying the reserved-memory node from
the DT passed by OpenSBI and set it to the final DT if it is a
different one. OpenSBI also need to provide the DT where the previous
stage (FSBL/U-Boot SPL) doesn't provide a DT.
IMHO, this is not a very difficult problem to solve. The U-Boot
implementation is available here (just ~40 lines of code).
https://patchwork.ozlabs.org/patch/1254740/
2. Trap-n-Emulate PMP CSR reads from S-mode. The details are available
here.
https://github.com/riscv/riscv-sbi-doc/pull/37#issuecomment-596944084
Pros: No SBI extension or prior DTB fixup required in U-Boot.
Cons: As PMP extensions proposals are already in place, this may need
to be extended. Privilege spec may need to be modified to explicitly
say that M-mode can provide PMP csr emulation.
Any thoughts ?

The assumption that there'll always be a PMP rule for marking the firmware's memory isn't always true. M-mode can access any region without a matching PMP rule so basically the firmware can be anywhere and work fine, without any PMP rule marking its region. With the current PMP spec this is probably the best approach since it will also prevent S-mode from ever accessing this region (S-mode will fail if there is no matching PMP rule). Why waste a rule for the firmware's region to deny any access from S-mode, when you can have the same result by not putting that rule on PMP ? Even if that assumption was always true, what about systems without PMP, systems with a different number of PMP regions, or systems with custom PMP-equivalent solutions ? Trap-n-emulate is not that generic (e.g. in the case of different number of PMP regions). Any interface from S-mode to PMP or a PMP-equivalent mechanism, should be more abstract than this IMHO.

We should also consider the security implications of exposing the PMP configuration to a less privileged mode. It's one thing to let S-mode know about the DRAM regions that it can't touch because the firmware is there, and another to reveal the full PMP configuration. PMP may contain settings that S-mode doesn't even know about, devices that are not on the device tree for example or DRAM regions that are outside the device tree's /memory node range (so S-mode won't touch them anyway). I'm not a fan of security through obscurity but there are good reasons to keep PMP configuration accessible only to M-mode (in addition to having a more abstract API with S-mode to be able to handle the various system configurations / implementations). We can have S-mode request M-mode to protect specific physical regions that S-mode manages through PMP (or a vendor-specific PMP-equivalent mechanism) but anything more than that doesn't make much sense to me.

If we just want to prevent S-mode from touching firmware's memory we can either exclude its region from the /memory node on the device tree, or let the firmware modify the device tree it passes on to the next-stage boot loader and add a reserved-memory node. I think that's the best approach since it's the most generic one and it's standards-compliant, plus the device-tree is platform-specific anyway, even if it's included on the kernel blob directly without passing through OpenSBI or whatever boot loader is there, we can always add there a reserved region, or modify the /memory node manually.

Regards,
Nick


Re: [PATCH] Add system reboot extension

Jonathan Behrens <behrensj@...>
 



On Wed, Apr 1, 2020 at 12:35 PM Anup Patel via Lists.Riscv.Org <anup.patel=wdc.com@...> wrote:


> -----Original Message-----
> From: Bin Meng <bmeng.cn@...>
> Sent: 01 April 2020 10:29
> To: Anup Patel <Anup.Patel@...>
> Cc: tech-unixplatformspec@...; Atish Patra
> <Atish.Patra@...>
> Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add system reboot
> extension
>
> On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:
> >
> > This patch adds SBI v0.2 compliant system reboot extension. It defines
> > two functions:
> > 1. sbi_reboot - A system reboot call with reboot type as parameter 2.
> > sbi_shutdown - A system shutdown/poweroff call
> >
> > The sbi_shutdown function defined here replaces SBI v0.1 shutdown
> > function.
> >
> > Signed-off-by: Atish Patra <atish.patra@...>
> > Signed-off-by: Anup Patel <anup.patel@...>
> > ---
> >  riscv-sbi.adoc | 64
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
>
> One generic comment, pretty much similar to the SBI PMP extension I
> proposed, is that why is this necessary to introduce a new SBI extension to
> support reboot and shutdown?
>
> Do these functionalities have to be operated from M-mode?

There are two cases:

1. If a system is partitioned between secure and non-secure world then we
cannot allow non-secure S-mode software to shutdown/reboot the system
without secure S-mode software knowing about it. The SBI shutdown/reboot
calls help M-mode software (OpenSBI) to mediate the shutdown/reboot
request coming from non-secure S-mode software.

2. In virtualization world, we don't have a standard way to shutdown/reboot
Guest/VM across architectures. To tackle this, we generally have architecture
specific hypercall for shutdown/reboot. For RISC-V, we don't want each
hypervisor coming up with its own hypercalls so we standardize these as
SBI calls (This rationale is similar to ARM PSCI shutdown/reboot calls).

Doesn't QEMU already have a "SiFive Test Finisher" for this purpose?

Regards,
Anup




Re: [PATCH] Add system reboot extension

Anup Patel
 

-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 01 April 2020 10:29
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add system reboot
extension

On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:

This patch adds SBI v0.2 compliant system reboot extension. It defines
two functions:
1. sbi_reboot - A system reboot call with reboot type as parameter 2.
sbi_shutdown - A system shutdown/poweroff call

The sbi_shutdown function defined here replaces SBI v0.1 shutdown
function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-sbi.adoc | 64
++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
One generic comment, pretty much similar to the SBI PMP extension I
proposed, is that why is this necessary to introduce a new SBI extension to
support reboot and shutdown?

Do these functionalities have to be operated from M-mode?
There are two cases:

1. If a system is partitioned between secure and non-secure world then we
cannot allow non-secure S-mode software to shutdown/reboot the system
without secure S-mode software knowing about it. The SBI shutdown/reboot
calls help M-mode software (OpenSBI) to mediate the shutdown/reboot
request coming from non-secure S-mode software.

2. In virtualization world, we don't have a standard way to shutdown/reboot
Guest/VM across architectures. To tackle this, we generally have architecture
specific hypercall for shutdown/reboot. For RISC-V, we don't want each
hypervisor coming up with its own hypercalls so we standardize these as
SBI calls (This rationale is similar to ARM PSCI shutdown/reboot calls).

Regards,
Anup


Re: [PATCH] Add system reboot extension

Anup Patel
 

I am fine with merging shutdown into reboot call but I am not 100% convinced.

 

Regarding the typo, I will update it.

 

Regards,

Anup

 

From: tech-unixplatformspec@... <tech-unixplatformspec@...> On Behalf Of Jonathan Behrens
Sent: 31 March 2020 19:49
To: Bin Meng <bmeng.cn@...>
Cc: Anup Patel <Anup.Patel@...>; tech-unixplatformspec@...
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add system reboot extension

 

Could this just be one function that had cold reboot, warm reboot, and shutdown all as options?

 

Also:

"This is a synchronous call and is not expected to return if succeeds." -> "This is a synchronous call and does not return if it succeeds"

 

Jonathan

 

On Tue, Mar 31, 2020 at 3:31 AM Bin Meng via Lists.Riscv.Org <bmeng.cn=gmail.com@...> wrote:

On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:
>
> This patch adds SBI v0.2 compliant system reboot extension. It defines
> two functions:
> 1. sbi_reboot - A system reboot call with reboot type as parameter
> 2. sbi_shutdown - A system shutdown/poweroff call
>
> The sbi_shutdown function defined here replaces SBI v0.1 shutdown
> function.
>
> Signed-off-by: Atish Patra <atish.patra@...>
> Signed-off-by: Anup Patel <anup.patel@...>
> ---
>  riscv-sbi.adoc | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
> index 8137686..a39e362 100644
> --- a/riscv-sbi.adoc
> +++ b/riscv-sbi.adoc
> @@ -615,6 +615,70 @@ state of the hart at the time of return value verification.
>  | sbi_hart_get_status          |           2 |     0x48534D
>  |===
>
> +== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
> +
> +The System Reboot Extension provides a set of functions that allow the
> +supervisor software to request system-level reboot or shutdown.
> +
> +[source, C]
> +----
> +struct sbiret sbi_system_reboot(unsigned long reboot_type)
> +----
> +
> +Reboot the system based on provided reboot type. This is a synchronous call
> +and is not expected to return if succeeds.
> +
> +The reboot_type parameter is 32 bit wide and has following possible values:
> +
> +[cols="<,>",options="header,compact"]
> +|===
> +| Value                        | Description
> +| 0x00000000                   | Cold reboot
> +| 0x00000001                   | Warm reboot
> +| 0x00000002 - 0xEFFFFFFF      | Reserved for future use
> +| 0xF0000000 - 0xFFFFFFFF      | Vendor or platform specific reboot type
> +| 0x100000000 - 2^XELN-1       | Reserved for RV64/RV128

The words are misleading, as it could indicate that the above types
only apply to RV32.

> +|===
> +
> +*Returns* one of the following possible SBI error codes through sbiret.error
> +upon failure.
> +
> +Cold reboot results in complete power cycle of the entire system while
> +warm reboot depends on SOC vendor design choices.
> +
> +[cols="<,>",options="header,compact"]
> +|===
> +| Error code                | Description
> +| SBI_ERR_INVALID_PARAM     | `reboot_type` is not valid.
> +| SBI_ERR_FAILED            | Reboot request failed for unknown reasons.
> +|===
> +
> +[source, C]
> +----
> +struct sbiret sbi_system_shutdown()
> +----
> +
> +Shutdown the system. This is a synchronous call and is not expected to
> +return if succeeds.
> +
> +*Returns* one of the following possible SBI error codes through sbiret.error
> +upon failure.
> +
> +[cols="<,>",options="header,compact"]
> +|===
> +| Error code                | Description
> +| SBI_ERR_FAILED            | The start request failed for unknown reasons.

The shutdown request

> +|===
> +
> +=== SRBT Function Listing
> +
> +[cols="<,,>",options="header,compact"]
> +|===
> +| Function Name                 | Function ID | Extension ID
> +| sbi_system_reboot                    |           0 |   0x53524254
> +| sbi_system_shutdown                  |           1 |   0x53524254
> +|===
> +
>  == Experimental SBI Extension Space, Extension IDs 0x0800000 through 0x08FFFFFF
>
>  No management.

Otherwise, looks good to me.

Regards,
Bin



Re: [PATCH] Add system reboot extension

Anup Patel
 

-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 31 March 2020 13:01
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add system reboot
extension

On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:

This patch adds SBI v0.2 compliant system reboot extension. It defines
two functions:
1. sbi_reboot - A system reboot call with reboot type as parameter 2.
sbi_shutdown - A system shutdown/poweroff call

The sbi_shutdown function defined here replaces SBI v0.1 shutdown
function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-sbi.adoc | 64
++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index 8137686..a39e362
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -615,6 +615,70 @@ state of the hart at the time of return value
verification.
| sbi_hart_get_status | 2 | 0x48534D
|===

+== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
+
+The System Reboot Extension provides a set of functions that allow
+the supervisor software to request system-level reboot or shutdown.
+
+[source, C]
+----
+struct sbiret sbi_system_reboot(unsigned long reboot_type)
+----
+
+Reboot the system based on provided reboot type. This is a
+synchronous call and is not expected to return if succeeds.
+
+The reboot_type parameter is 32 bit wide and has following possible
values:
+
+[cols="<,>",options="header,compact"]
+|===
+| Value | Description
+| 0x00000000 | Cold reboot
+| 0x00000001 | Warm reboot
+| 0x00000002 - 0xEFFFFFFF | Reserved for future use
+| 0xF0000000 - 0xFFFFFFFF | Vendor or platform specific reboot type
+| 0x100000000 - 2^XELN-1 | Reserved for RV64/RV128
The words are misleading, as it could indicate that the above types only apply
to RV32.
Sure, I will update.


+|===
+
+*Returns* one of the following possible SBI error codes through
+sbiret.error upon failure.
+
+Cold reboot results in complete power cycle of the entire system
+while warm reboot depends on SOC vendor design choices.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code | Description
+| SBI_ERR_INVALID_PARAM | `reboot_type` is not valid.
+| SBI_ERR_FAILED | Reboot request failed for unknown reasons.
+|===
+
+[source, C]
+----
+struct sbiret sbi_system_shutdown()
+----
+
+Shutdown the system. This is a synchronous call and is not expected
+to return if succeeds.
+
+*Returns* one of the following possible SBI error codes through
+sbiret.error upon failure.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code | Description
+| SBI_ERR_FAILED | The start request failed for unknown reasons.
The shutdown request
Sure, I will update.


+|===
+
+=== SRBT Function Listing
+
+[cols="<,,>",options="header,compact"]
+|===
+| Function Name | Function ID | Extension ID
+| sbi_system_reboot | 0 | 0x53524254
+| sbi_system_shutdown | 1 | 0x53524254
+|===
+
== Experimental SBI Extension Space, Extension IDs 0x0800000 through
0x08FFFFFF

No management.
Otherwise, looks good to me.

Regards,
Bin

Regards,
Anup


Re: [PATCH] Add system reboot extension

Bin Meng
 

On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:

This patch adds SBI v0.2 compliant system reboot extension. It defines
two functions:
1. sbi_reboot - A system reboot call with reboot type as parameter
2. sbi_shutdown - A system shutdown/poweroff call

The sbi_shutdown function defined here replaces SBI v0.1 shutdown
function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-sbi.adoc | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
One generic comment, pretty much similar to the SBI PMP extension I
proposed, is that why is this necessary to introduce a new SBI
extension to support reboot and shutdown?

Do these functionalities have to be operated from M-mode?

Regards,
Bin


Re: [PATCH] Add system reboot extension

Jonathan Behrens <behrensj@...>
 

Could this just be one function that had cold reboot, warm reboot, and shutdown all as options?

Also:
"This is a synchronous call and is not expected to return if succeeds." -> "This is a synchronous call and does not return if it succeeds"

Jonathan


On Tue, Mar 31, 2020 at 3:31 AM Bin Meng via Lists.Riscv.Org <bmeng.cn=gmail.com@...> wrote:
On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:
>
> This patch adds SBI v0.2 compliant system reboot extension. It defines
> two functions:
> 1. sbi_reboot - A system reboot call with reboot type as parameter
> 2. sbi_shutdown - A system shutdown/poweroff call
>
> The sbi_shutdown function defined here replaces SBI v0.1 shutdown
> function.
>
> Signed-off-by: Atish Patra <atish.patra@...>
> Signed-off-by: Anup Patel <anup.patel@...>
> ---
>  riscv-sbi.adoc | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
> index 8137686..a39e362 100644
> --- a/riscv-sbi.adoc
> +++ b/riscv-sbi.adoc
> @@ -615,6 +615,70 @@ state of the hart at the time of return value verification.
>  | sbi_hart_get_status          |           2 |     0x48534D
>  |===
>
> +== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
> +
> +The System Reboot Extension provides a set of functions that allow the
> +supervisor software to request system-level reboot or shutdown.
> +
> +[source, C]
> +----
> +struct sbiret sbi_system_reboot(unsigned long reboot_type)
> +----
> +
> +Reboot the system based on provided reboot type. This is a synchronous call
> +and is not expected to return if succeeds.
> +
> +The reboot_type parameter is 32 bit wide and has following possible values:
> +
> +[cols="<,>",options="header,compact"]
> +|===
> +| Value                        | Description
> +| 0x00000000                   | Cold reboot
> +| 0x00000001                   | Warm reboot
> +| 0x00000002 - 0xEFFFFFFF      | Reserved for future use
> +| 0xF0000000 - 0xFFFFFFFF      | Vendor or platform specific reboot type
> +| 0x100000000 - 2^XELN-1       | Reserved for RV64/RV128

The words are misleading, as it could indicate that the above types
only apply to RV32.

> +|===
> +
> +*Returns* one of the following possible SBI error codes through sbiret.error
> +upon failure.
> +
> +Cold reboot results in complete power cycle of the entire system while
> +warm reboot depends on SOC vendor design choices.
> +
> +[cols="<,>",options="header,compact"]
> +|===
> +| Error code                | Description
> +| SBI_ERR_INVALID_PARAM     | `reboot_type` is not valid.
> +| SBI_ERR_FAILED            | Reboot request failed for unknown reasons.
> +|===
> +
> +[source, C]
> +----
> +struct sbiret sbi_system_shutdown()
> +----
> +
> +Shutdown the system. This is a synchronous call and is not expected to
> +return if succeeds.
> +
> +*Returns* one of the following possible SBI error codes through sbiret.error
> +upon failure.
> +
> +[cols="<,>",options="header,compact"]
> +|===
> +| Error code                | Description
> +| SBI_ERR_FAILED            | The start request failed for unknown reasons.

The shutdown request

> +|===
> +
> +=== SRBT Function Listing
> +
> +[cols="<,,>",options="header,compact"]
> +|===
> +| Function Name                 | Function ID | Extension ID
> +| sbi_system_reboot                    |           0 |   0x53524254
> +| sbi_system_shutdown                  |           1 |   0x53524254
> +|===
> +
>  == Experimental SBI Extension Space, Extension IDs 0x0800000 through 0x08FFFFFF
>
>  No management.

Otherwise, looks good to me.

Regards,
Bin




Re: [PATCH] Add system reboot extension

Bin Meng
 

On Tue, Mar 31, 2020 at 12:13 PM Anup Patel <anup.patel@...> wrote:

This patch adds SBI v0.2 compliant system reboot extension. It defines
two functions:
1. sbi_reboot - A system reboot call with reboot type as parameter
2. sbi_shutdown - A system shutdown/poweroff call

The sbi_shutdown function defined here replaces SBI v0.1 shutdown
function.

Signed-off-by: Atish Patra <atish.patra@...>
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-sbi.adoc | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 8137686..a39e362 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -615,6 +615,70 @@ state of the hart at the time of return value verification.
| sbi_hart_get_status | 2 | 0x48534D
|===

+== System Reboot Extension, Extension ID: 0x53524254 (SRBT)
+
+The System Reboot Extension provides a set of functions that allow the
+supervisor software to request system-level reboot or shutdown.
+
+[source, C]
+----
+struct sbiret sbi_system_reboot(unsigned long reboot_type)
+----
+
+Reboot the system based on provided reboot type. This is a synchronous call
+and is not expected to return if succeeds.
+
+The reboot_type parameter is 32 bit wide and has following possible values:
+
+[cols="<,>",options="header,compact"]
+|===
+| Value | Description
+| 0x00000000 | Cold reboot
+| 0x00000001 | Warm reboot
+| 0x00000002 - 0xEFFFFFFF | Reserved for future use
+| 0xF0000000 - 0xFFFFFFFF | Vendor or platform specific reboot type
+| 0x100000000 - 2^XELN-1 | Reserved for RV64/RV128
The words are misleading, as it could indicate that the above types
only apply to RV32.

+|===
+
+*Returns* one of the following possible SBI error codes through sbiret.error
+upon failure.
+
+Cold reboot results in complete power cycle of the entire system while
+warm reboot depends on SOC vendor design choices.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code | Description
+| SBI_ERR_INVALID_PARAM | `reboot_type` is not valid.
+| SBI_ERR_FAILED | Reboot request failed for unknown reasons.
+|===
+
+[source, C]
+----
+struct sbiret sbi_system_shutdown()
+----
+
+Shutdown the system. This is a synchronous call and is not expected to
+return if succeeds.
+
+*Returns* one of the following possible SBI error codes through sbiret.error
+upon failure.
+
+[cols="<,>",options="header,compact"]
+|===
+| Error code | Description
+| SBI_ERR_FAILED | The start request failed for unknown reasons.
The shutdown request

+|===
+
+=== SRBT Function Listing
+
+[cols="<,,>",options="header,compact"]
+|===
+| Function Name | Function ID | Extension ID
+| sbi_system_reboot | 0 | 0x53524254
+| sbi_system_shutdown | 1 | 0x53524254
+|===
+
== Experimental SBI Extension Space, Extension IDs 0x0800000 through 0x08FFFFFF

No management.
Otherwise, looks good to me.

Regards,
Bin

1821 - 1840 of 1847