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