On 10/27/20 4:20 PM, Atish Patra wrote:

On Thu, 2020-10-22 at 20:55 -0400, Sean Anderson wrote:

On 10/22/20 8:35 PM, Jonathan Behrens wrote:

On Thu, Oct 22, 2020 at 7:36 PM Sean Anderson via lists.riscv.org <
http://lists.riscv.org> <seanga2=gmail.com@...
<mailto:gmail.com@...>> wrote:

The initial state of software running in the SBI was not
specified. Particularly egregious was the omission of the
requirement that a0 contain the hartid of the hart. That
behavior was used to justify not including a function to get the
current hartid (e.g. in #25 [1]), so it should be specified.

Does it need to be specified in SBI specification or platform
specification ? Platform specification anyways should have a section
about state of machine while entering M/S mode.

I think this belongs in the SBI specification. There are multiple issues
about information expected to be in this ([1], [2]). In addition, some
concepts such as the HSM, or the placement of the hartid in a0 should be
addressed because they are included with the SBI specification.

I think it would be fine to just specify the value of a0 and the status
of harts as returned by the HSM extension *as long as* other state is
specified elsewhere. To give one example, the platform specification
could require that a device tree with particular nodes and properties be
present in a1, and this would be compatible with the additions I
proposed.

[1] https://github.com/riscv/riscv-sbi-doc/issues/25
[2] https://github.com/riscv/riscv-sbi-doc/issues/13

Keeping the same information in SBI specification may be redundant.

SBI is intended as an interface for supervisor mode software,
however, there is no technical reason why it cannot also be a
"UBI." This mode of operation is currently supported by OpenSBI.
I think it should be captured by this specification. There is a
slight conflict with the HSM extension, in that harts are
requires to start in S-mode. If a system only supports U-mode,
then it would be impossible to start harts with the HSM
extension, as it is currently written. If it is undesired to
reference U-mode, then those references can be removed (or split
off into a separate series).

This also specifies the return value of sbi_hart_status. This is
to avoid a "slow start" situation where a hart is part of an
execution environment, but starts much later than another hart.
This could cause confusing behavior where it appeared to be
stopped, and then start without a call to sbi_start_hart.
Perhaps this sort of behavior should be explicitly disallowed by
the HSM extension?

[1] https://github.com/riscv/riscv-sbi-doc/issues/25

Signed-off-by: Sean Anderson <seanga2@... <mailto:
seanga2@...>>
---

riscv-sbi.adoc | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index d8cba88..0df3870 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -56,6 +56,23 @@ the value of the `mhartid` CSR on particular
harts.
All harts in the execution environment must comply with the
link:https://riscv.org/technical/specifications/[RISC-V <
https://riscv.org/technical/specifications/%5BRISC-V> ISA
specification].

+== Initial State
+
+If a hart supports supervisor mode, then it should begin
execution in (virtual) +supervisor mode. Otherwise, harts shall
begin execution in (virtual) user mode.


How should the SBI user detect whether it is in supervisor mode or
user mode? Will this be indicated in the device tree?

Hm, I don't know. OpenSBI doesn't seem to provide this info to its
payloads (presumably whatever sets up fw_dynamic ensures that the
payload is intended for U-mode). Perhaps change

The stage prior to OpenSBI can specify this via fy_dynamic_info if
fw_dyanmic is used.

Ok, I'll leave this wording as-is.

+If a hart supports supervisor mode, then it should begin
execution in (virtual) +supervisor mode.

to

+If a hart supports supervisor mode, then it shall begin
execution in (virtual) +supervisor mode.

and then have the payload determine the mode by running
`sbi_probe_extension('s')`. Alternatively, the mode could be passed
in in a register.

+The entry point is unspecified, but must be the same for all
harts in the +execution environment.

+ +If a hart begins execution in supervisor mode, then the
`satp` CSR must be set +to 0, and the `sie` bit of the `sstatus`
CSR must be cleared. If a hart begins +execution in supervisor
mode, then the `uie` bit of the `ustatus` CSR must be +cleared.
In both cases, the `a0` register must be set to the *hartid*
+identifying the hart. The state of all other registers is
unspecified.


I believe current implementations all set/require that at
initialization register a1 holds the physical address of the
flattened device tree, so that should probably also be specified
here.

Do we want to specifically mandate an fdt?

IMHO, yes. Device tree should be one of the standard interface for
enumerating hardware. There can be others (ACPI/config) but there is a
lot of support for device tree in many OS/firmware/bootloader. I don't
see a point not standardizing device tree.

Having said that, this can be specified on profiles (the details are
under discussion). For example, a Linux distro profile can specify
whether it is DT based while other one is ACPI based.

Ok. So perhaps something like

The `a1` register must contain a pointer to a devicetree, or be set
to 0, indicating that no devicetree is supplied. The exact format of
such a devicetree is outside the scope of this specification.

I don't think a device tree should be specified here, since it's a
pretty complex structure. Alternatively, we could do something like

The `a1` register must contain a pointer to an implementation-defined
configuration structure, or be set to 0, indicating that no such
structure is supplied. The exact format of such a structure is outside
the scope of this specification, but is typically a devicetree.

Either way, I think the platform specification should specify the
details of such a device tree.

--Sean

On Thu, 2020-10-22 at 20:55 -0400, Sean Anderson wrote:

On 10/22/20 8:35 PM, Jonathan Behrens wrote:

On Thu, Oct 22, 2020 at 7:36 PM Sean Anderson via lists.riscv.org <
http://lists.riscv.org> <seanga2=gmail.com@...
<mailto:gmail.com@...>> wrote:

The initial state of software running in the SBI was not
specified.
Particularly egregious was the omission of the requirement that
a0
contain the hartid of the hart. That behavior was used to
justify not
including a function to get the current hartid (e.g. in #25
[1]), so it
should be specified.

Does it need to be specified in SBI specification or platform
specification ? Platform specification anyways should have a section
about state of machine while entering M/S mode.

Keeping the same information in SBI specification may be redundant.

SBI is intended as an interface for supervisor mode software,
however,
there is no technical reason why it cannot also be a "UBI."
This mode of
operation is currently supported by OpenSBI. I think it should
be
captured by this specification. There is a slight conflict with
the HSM
extension, in that harts are requires to start in S-mode. If a
system
only supports U-mode, then it would be impossible to start
harts with
the HSM extension, as it is currently written. If it is
undesired to
reference U-mode, then those references can be removed (or
split off
into a separate series).

This also specifies the return value of sbi_hart_status. This
is to
avoid a "slow start" situation where a hart is part of an
execution
environment, but starts much later than another hart. This
could cause
confusing behavior where it appeared to be stopped, and then
start
without a call to sbi_start_hart. Perhaps this sort of behavior
should
be explicitly disallowed by the HSM extension?

[1] https://github.com/riscv/riscv-sbi-doc/issues/25

Signed-off-by: Sean Anderson <seanga2@... <mailto:
seanga2@...>>
---

riscv-sbi.adoc | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index d8cba88..0df3870 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -56,6 +56,23 @@ the value of the `mhartid` CSR on particular
harts.
All harts in the execution environment must comply with the
link:https://riscv.org/technical/specifications/[RISC-V <
https://riscv.org/technical/specifications/%5BRISC-V> ISA
specification].

+== Initial State
+
+If a hart supports supervisor mode, then it should begin
execution in (virtual)
+supervisor mode. Otherwise, harts shall begin execution in
(virtual) user mode.


How should the SBI user detect whether it is in supervisor mode or
user mode? Will this be indicated in the device tree?

Hm, I don't know. OpenSBI doesn't seem to provide this info to its
payloads (presumably whatever sets up fw_dynamic ensures that the
payload is intended for U-mode). Perhaps change

The stage prior to OpenSBI can specify this via fy_dynamic_info if
fw_dyanmic is used.

+If a hart supports supervisor mode, then it should begin
execution in (virtual)
+supervisor mode.

to

+If a hart supports supervisor mode, then it shall begin
execution in (virtual)
+supervisor mode.

and then have the payload determine the mode by running
`sbi_probe_extension('s')`. Alternatively, the mode could be passed
in
in a register.

+The entry point is unspecified, but must be the same for all
harts in the
+execution environment.

+
+If a hart begins execution in supervisor mode, then the `satp`
CSR must be set
+to 0, and the `sie` bit of the `sstatus` CSR must be cleared.
If a hart begins
+execution in supervisor mode, then the `uie` bit of the
`ustatus` CSR must be
+cleared. In both cases, the `a0` register must be set to the
*hartid*
+identifying the hart. The state of all other registers is
unspecified.


I believe current implementations all set/require that at
initialization register a1 holds the physical address of the
flattened device tree, so that should probably also be specified
here.

Do we want to specifically mandate an fdt?

IMHO, yes. Device tree should be one of the standard interface for
enumerating hardware. There can be others (ACPI/config) but there is a
lot of support for device tree in many OS/firmware/bootloader. I don't
see a point not standardizing device tree.

Having said that, this can be specified on profiles (the details are
under discussion). For example, a Linux distro profile can specify
whether it is DT based while other one is ACPI based.

Some implementations may just
want to provide a RISC-V config. Other unices may have their own
configuration structure that they expect. One option would be to just
specify a "config" of some kind, and let payloads determine what type
of
config by reading the magic number. Alternatively, we could specify
the
config type via some other manner.

--Sean

--
Regards,
Atish

On 10/22/20 8:35 PM, Jonathan Behrens wrote:

On Thu, Oct 22, 2020 at 7:36 PM Sean Anderson via lists.riscv.org <http://lists.riscv.org> <seanga2=gmail.com@... <mailto:gmail.com@...>> wrote:

The initial state of software running in the SBI was not specified.
Particularly egregious was the omission of the requirement that a0
contain the hartid of the hart. That behavior was used to justify not
including a function to get the current hartid (e.g. in #25 [1]), so it
should be specified.

SBI is intended as an interface for supervisor mode software, however,
there is no technical reason why it cannot also be a "UBI." This mode of
operation is currently supported by OpenSBI. I think it should be
captured by this specification. There is a slight conflict with the HSM
extension, in that harts are requires to start in S-mode. If a system
only supports U-mode, then it would be impossible to start harts with
the HSM extension, as it is currently written. If it is undesired to
reference U-mode, then those references can be removed (or split off
into a separate series).

This also specifies the return value of sbi_hart_status. This is to
avoid a "slow start" situation where a hart is part of an execution
environment, but starts much later than another hart. This could cause
confusing behavior where it appeared to be stopped, and then start
without a call to sbi_start_hart. Perhaps this sort of behavior should
be explicitly disallowed by the HSM extension?

[1] https://github.com/riscv/riscv-sbi-doc/issues/25

Signed-off-by: Sean Anderson <seanga2@... <mailto:seanga2@...>>
---

riscv-sbi.adoc | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index d8cba88..0df3870 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -56,6 +56,23 @@ the value of the `mhartid` CSR on particular harts.
All harts in the execution environment must comply with the
link:https://riscv.org/technical/specifications/[RISC-V <https://riscv.org/technical/specifications/%5BRISC-V> ISA specification].

+== Initial State
+
+If a hart supports supervisor mode, then it should begin execution in (virtual)
+supervisor mode. Otherwise, harts shall begin execution in (virtual) user mode.


How should the SBI user detect whether it is in supervisor mode or user mode? Will this be indicated in the device tree?

Hm, I don't know. OpenSBI doesn't seem to provide this info to its
payloads (presumably whatever sets up fw_dynamic ensures that the
payload is intended for U-mode). Perhaps change

+If a hart supports supervisor mode, then it should begin execution in (virtual)
+supervisor mode.

to

+If a hart supports supervisor mode, then it shall begin execution in (virtual)
+supervisor mode.

and then have the payload determine the mode by running
`sbi_probe_extension('s')`. Alternatively, the mode could be passed in
in a register.

+The entry point is unspecified, but must be the same for all harts in the
+execution environment.

+
+If a hart begins execution in supervisor mode, then the `satp` CSR must be set
+to 0, and the `sie` bit of the `sstatus` CSR must be cleared. If a hart begins
+execution in supervisor mode, then the `uie` bit of the `ustatus` CSR must be
+cleared. In both cases, the `a0` register must be set to the *hartid*
+identifying the hart. The state of all other registers is unspecified.


I believe current implementations all set/require that at initialization register a1 holds the physical address of the flattened device tree, so that should probably also be specified here.

Do we want to specifically mandate an fdt? Some implementations may just
want to provide a RISC-V config. Other unices may have their own
configuration structure that they expect. One option would be to just
specify a "config" of some kind, and let payloads determine what type of
config by reading the magic number. Alternatively, we could specify the
config type via some other manner.

--Sean

The initial state of software running in the SBI was not specified.
Particularly egregious was the omission of the requirement that a0
contain the hartid of the hart. That behavior was used to justify not
including a function to get the current hartid (e.g. in #25 [1]), so it
should be specified.

SBI is intended as an interface for supervisor mode software, however,
there is no technical reason why it cannot also be a "UBI." This mode of
operation is currently supported by OpenSBI. I think it should be
captured by this specification. There is a slight conflict with the HSM
extension, in that harts are requires to start in S-mode. If a system
only supports U-mode, then it would be impossible to start harts with
the HSM extension, as it is currently written. If it is undesired to
reference U-mode, then those references can be removed (or split off
into a separate series).

This also specifies the return value of sbi_hart_status. This is to
avoid a "slow start" situation where a hart is part of an execution
environment, but starts much later than another hart. This could cause
confusing behavior where it appeared to be stopped, and then start
without a call to sbi_start_hart. Perhaps this sort of behavior should
be explicitly disallowed by the HSM extension?

[1] https://github.com/riscv/riscv-sbi-doc/issues/25

Signed-off-by: Sean Anderson <seanga2@...>
---

riscv-sbi.adoc | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index d8cba88..0df3870 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -56,6 +56,23 @@ the value of the `mhartid` CSR on particular harts.
All harts in the execution environment must comply with the
link:https://riscv.org/technical/specifications/[RISC-V ISA specification].

+== Initial State
+
+If a hart supports supervisor mode, then it should begin execution in (virtual)
+supervisor mode. Otherwise, harts shall begin execution in (virtual) user mode.
+The entry point is unspecified, but must be the same for all harts in the
+execution environment.
+
+If a hart begins execution in supervisor mode, then the `satp` CSR must be set
+to 0, and the `sie` bit of the `sstatus` CSR must be cleared. If a hart begins
+execution in supervisor mode, then the `uie` bit of the `ustatus` CSR must be
+cleared. In both cases, the `a0` register must be set to the *hartid*
+identifying the hart. The state of all other registers is unspecified.
+
+If the <<hsm-extension>> is present, all harts in the execution environment
+must have a status of `STARTED` or `START_REQUEST_PENDING` (as returned by
+`sbi_hart_status()`) when any hart first retires an instruction.
+
== Binary Encoding

All SBI functions share a single binary encoding, which facilitates the mixing
--
2.28.0