Date   

Re: [PATCH] riscv-sbi.adoc: Use 'an' before 'SBI'

atishp@...
 

On Fri, 2021-06-04 at 21:00 +0800, Bin Meng wrote:
%s/a SBI/an SBI/
%s/A SBI/An SBI/

Signed-off-by: Bin Meng <bmeng.cn@...>
---

 riscv-sbi.adoc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 11c30c3..b02332e 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -52,8 +52,8 @@ of the SBI follows the general RISC-V philosophy of
having a small core along
 with a set of optional modular extensions.
 
 The higher privilege software providing SBI interface to the
supervisor-mode
-software is referred to as a SBI implemenation or Supervisor Execution
-Environment (SEE). A SBI implementation (or SEE) can be platform
runtime
+software is referred to as an SBI implemenation or Supervisor
Execution
+Environment (SEE). An SBI implementation (or SEE) can be platform
runtime
 firmware executing in machine-mode (M-mode) (see below <<fig_intro1>>)
or
 it can be some hypervisor executing in hypervisor-mode (HS-mode) (see
below
 <<fig_intro2>>).
@@ -124,7 +124,7 @@ the specification simple and easily adaptable for
all RISC-V ISA types (i.e.
 RV32, RV64 and RV128). In case the data is defined as 32bit wide,
higher
 privilege software must ensure that it only uses 32 bit data only.
 
-If a SBI function needs to pass a list of harts to the higher
privilege mode,
+If an SBI function needs to pass a list of harts to the higher
privilege mode,
 it must use a hart mask as defined below. This is applicable to any
extensions
 defined in or after v0.2.
 
@@ -765,13 +765,13 @@ A platform can have multiple harts grouped into a
hierarchical topology
 groups (namely cores, clusters, nodes, etc) with separate platform
specific
 low-power states for each hierarchical group. These platform specific
 low-power states of hierarchial topology groups can be represented as
-platform specific suspend states of a hart. A SBI implementation can
+platform specific suspend states of a hart. An SBI implementation can
 utilize the suspend states of higher topology groups using one of the
 following approaches:
 
 . *Platform-coordinated:* In this approach, when a hart becomes idle
the
   supervisor-mode power-managment software will request deepest
suspend
-  state for the hart and higher topology groups. A SBI implementation
+  state for the hart and higher topology groups. An SBI implementation
   should choose a suspend state at higher topology group which is:
 .. Not deeper than the specified suspend state
 .. Wake-up latency is not higher than the wake-up latency of the
@@ -782,7 +782,7 @@ following approaches:
   the supervisor-mode power-managment software will always select
suspend
   state for the hart itself but it will select a suspend state for a
higher
   topology group only if the hart is the last running hart in the
group.
-  A SBI implementation should:
+  An SBI implementation should:
 .. Never choose a suspend state for higher topology group different
from
    the specified suspend state
 .. Always prefer most recent suspend state requested for higher
topology
Reviewed-by: Atish Patra <atish.patra@...>

--
Regards,
Atish


Re: [PATCH v6 1/2] riscv-platform-spec: PLIC and CLINT for Linux-2022 platform

atishp@...
 

On Sat, 2021-05-29 at 22:42 +0800, Abner Chang wrote:
From: Abner Chang <renba.chang@...>

Initial description of PLIC  CLINT section of Linux-2022 platform.

On v6 commit,
  Remove the changes in Embedded-2022 section.

On v5 commit,
- Remove CLINT from platform spec
- Require ACLINT on Linux2020 platform and have a link to
https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc.
- Remove Machine mode timer from previous patch because that is in
the scope of ACLINT
- For Embedded-2022 platform, mention Machine mode timer and refer to
ACLINT for the definition of registers

On v4 commit,
- PLIC section with [DEPRECATED] in Linux- 2022 chapter
- CLINT section in Linux- 2022 chapter for M-mode timer. We don't
mention
  IPI because AIA already supported it.
- In Embedded-2022 Machine mode timer section, CLINT is not
mandatory.
- Separate section in appendix for the Machine mode timer registers

On v3 commit,
- Address review comments.

On v2 commit,
- CLINT is not deprecated.

- Add a standalone section for Machine Mode Timer in System
Peripherals.
  Do you think this is a good place for Machine Mode Timer?
  @Mayuresh, please check if you are ok with this change, not sure if
this
  overlaps with your text or not (The timer setion). I can remove
this
  if you prefer to put this with your patch.

- In Embedded-2022, refer to Machine Mode Timer in System Peripherals
  section and CLINT in Linux-2022 Platform.
  @Alistair, is this ok?

On v1 commit,
- Not sure where to put the [DEPRECATED].
- Change the reference of PLIC in section 2.2.2. Interrupt Controller
to
  1.1.3.2 PLIC + CLINT section.

Signed-off-by: Abner Chang <renba.chang@...>
Cc: Alistair Francis <alistair.francis@...>
Cc: Sunil V L <sunilvl@...>
Cc: Mayuresh Chitale <mchitale@...>
---
 riscv-platform-spec.adoc | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 160c74a..c0ee75d 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -49,9 +49,24 @@ include::profiles.adoc[]
 * Start Address
 
 ==== Interrupt Controller
-* AIA
-* PLIC + CLINT
-* Interrupt Assignments
+===== AIA[[AIA]]
+===== PLIC[DEPRECATED][[PLIC]]
+The Platform Level Interrupt Controller (PLIC) provides facilities
to route
+the non-local interrupts to the external interrupt of a hart context
+with a given privilege mode in a given hart. The number of non-local
interrupt
+sources supported by PLIC and how does each of them connect to the
hart
+context is PLIC core implementation-specific. +
+(Refer to
https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc[RISC-V
 PLIC Specification]
IIRC, PLIC spec was never reviewed widely. As this group is more active
now, tt would be good to send it as a separate patch so we can do a
detailed review of that as well.

I am just concerned about semantics rather than technical details.

+for the implementation reference of PLIC operation parameters)
+
+===== ACLINT
+Linux-2020 platform 
We have now official names for the PLATFORM spec. We should refer to
that.

requires the Advanced Core Local Interruptor (ACLINT,
+refer to
+
https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc[RISC-V
 ACLINT Specification])
+to provide facilities to route inter-processor interrupt and Machine
mode timer
+interrupt to each RISC-V processor hart.
+
+===== Interrupt Assignments
 
 ==== System Peripherals
 * UART/Serial Console
@@ -289,8 +304,8 @@ Any RISC-V system that uses at least RV32/64G can
meet the Embedded-2022
 specification.
 
 ==== Interrupt Controller
-Embedded systems are recommended to use a spec compliant
-https://github.com/riscv/riscv-plic-spec[PLIC], a spec compliant
+Embedded systems are recommended to use a spec compliant
<<PLIC,PLIC>>,
+a spec compliant
 
https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc[CLIC]
 or both a CLIC and and PLIC.
 
--
Regards,
Atish


Re: [PATCH] Clarify that a SBI extension cannot be partially implemented

atishp@...
 

On Fri, 2021-06-04 at 20:48 +0800, Bin Meng wrote:
Hi Heinrich,

On Fri, Jun 4, 2021 at 8:13 PM Heinrich Schuchardt <
xypron.glpk@...> wrote:

a
On 6/4/21 11:57 AM, Bin Meng wrote:
Signed-off-by: Bin Meng <bmeng.cn@...>
---

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 11c30c3..8696f97 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -34,6 +34,7 @@ https://creativecommons.org/licenses/by/4.0/.
  * Improved SBI introduction secion
  * Improved documentation of SBI hart state managment extension
  * Added suspend function to SBI hart state managment extension
+* Clarified that a SBI extension cannot be partially implemented

  === Version 0.2

@@ -51,6 +52,11 @@ abstraction for platform (or hypervisor)
specific functionality. The design
  of the SBI follows the general RISC-V philosophy of having a
small core along
  with a set of optional modular extensions.

+SBI extensions as whole are optional but if a SBI <abc>
extension compliant
%s/a SBI/an SBI/ (as you will pronounce SBI as as-bee-aye)
Sure. Will send a new patch to fix other places in the same file.


+with SBI v0.X spec is implemented then all functions of SBI
<abc> extension
+as defined in SBI v0.X are assumed to be present. Basically, a
SBI extension
Can we do away with all the placeholders?

How about:

"SBI extensions as whole are optional but they shall not be
partially
implemented: If sbi_probe_extension() signals that an extension is
available, it must be implemented in total and conform to the SBI
version reported by sbi_get_spec_version()."
This one is more verbose but sounds better to me. May be we should just
explicitly say that "all functions belonging to that extension must be
implemented" similar to the below version.

How about:

If sbi_probe_extension() signals that an extension is available, all
functions that conform to the SBI version reported by
sbi_get_spec_version() must be implemented.

Regards,
Bin




--
Regards,
Atish


[PATCH] riscv-sbi.adoc: Use 'an' before 'SBI'

Bin Meng
 

%s/a SBI/an SBI/
%s/A SBI/An SBI/

Signed-off-by: Bin Meng <bmeng.cn@...>
---

riscv-sbi.adoc | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 11c30c3..b02332e 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -52,8 +52,8 @@ of the SBI follows the general RISC-V philosophy of having a small core along
with a set of optional modular extensions.

The higher privilege software providing SBI interface to the supervisor-mode
-software is referred to as a SBI implemenation or Supervisor Execution
-Environment (SEE). A SBI implementation (or SEE) can be platform runtime
+software is referred to as an SBI implemenation or Supervisor Execution
+Environment (SEE). An SBI implementation (or SEE) can be platform runtime
firmware executing in machine-mode (M-mode) (see below <<fig_intro1>>) or
it can be some hypervisor executing in hypervisor-mode (HS-mode) (see below
<<fig_intro2>>).
@@ -124,7 +124,7 @@ the specification simple and easily adaptable for all RISC-V ISA types (i.e.
RV32, RV64 and RV128). In case the data is defined as 32bit wide, higher
privilege software must ensure that it only uses 32 bit data only.

-If a SBI function needs to pass a list of harts to the higher privilege mode,
+If an SBI function needs to pass a list of harts to the higher privilege mode,
it must use a hart mask as defined below. This is applicable to any extensions
defined in or after v0.2.

@@ -765,13 +765,13 @@ A platform can have multiple harts grouped into a hierarchical topology
groups (namely cores, clusters, nodes, etc) with separate platform specific
low-power states for each hierarchical group. These platform specific
low-power states of hierarchial topology groups can be represented as
-platform specific suspend states of a hart. A SBI implementation can
+platform specific suspend states of a hart. An SBI implementation can
utilize the suspend states of higher topology groups using one of the
following approaches:

. *Platform-coordinated:* In this approach, when a hart becomes idle the
supervisor-mode power-managment software will request deepest suspend
- state for the hart and higher topology groups. A SBI implementation
+ state for the hart and higher topology groups. An SBI implementation
should choose a suspend state at higher topology group which is:
.. Not deeper than the specified suspend state
.. Wake-up latency is not higher than the wake-up latency of the
@@ -782,7 +782,7 @@ following approaches:
the supervisor-mode power-managment software will always select suspend
state for the hart itself but it will select a suspend state for a higher
topology group only if the hart is the last running hart in the group.
- A SBI implementation should:
+ An SBI implementation should:
.. Never choose a suspend state for higher topology group different from
the specified suspend state
.. Always prefer most recent suspend state requested for higher topology
--
2.25.1


Re: [PATCH] Clarify that a SBI extension cannot be partially implemented

Bin Meng
 

Hi Heinrich,

On Fri, Jun 4, 2021 at 8:13 PM Heinrich Schuchardt <xypron.glpk@...> wrote:

a
On 6/4/21 11:57 AM, Bin Meng wrote:
Signed-off-by: Bin Meng <bmeng.cn@...>
---

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 11c30c3..8696f97 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -34,6 +34,7 @@ https://creativecommons.org/licenses/by/4.0/.
* Improved SBI introduction secion
* Improved documentation of SBI hart state managment extension
* Added suspend function to SBI hart state managment extension
+* Clarified that a SBI extension cannot be partially implemented

=== Version 0.2

@@ -51,6 +52,11 @@ abstraction for platform (or hypervisor) specific functionality. The design
of the SBI follows the general RISC-V philosophy of having a small core along
with a set of optional modular extensions.

+SBI extensions as whole are optional but if a SBI <abc> extension compliant
%s/a SBI/an SBI/ (as you will pronounce SBI as as-bee-aye)
Sure. Will send a new patch to fix other places in the same file.


+with SBI v0.X spec is implemented then all functions of SBI <abc> extension
+as defined in SBI v0.X are assumed to be present. Basically, a SBI extension
Can we do away with all the placeholders?

How about:

"SBI extensions as whole are optional but they shall not be partially
implemented: If sbi_probe_extension() signals that an extension is
available, it must be implemented in total and conform to the SBI
version reported by sbi_get_spec_version()."
How about:

If sbi_probe_extension() signals that an extension is available, all
functions that conform to the SBI version reported by
sbi_get_spec_version() must be implemented.

Regards,
Bin


Re: [PATCH] Clarify that a SBI extension cannot be partially implemented

Heinrich Schuchardt
 

a
On 6/4/21 11:57 AM, Bin Meng wrote:
Signed-off-by: Bin Meng <bmeng.cn@...>
---

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 11c30c3..8696f97 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -34,6 +34,7 @@ https://creativecommons.org/licenses/by/4.0/.
* Improved SBI introduction secion
* Improved documentation of SBI hart state managment extension
* Added suspend function to SBI hart state managment extension
+* Clarified that a SBI extension cannot be partially implemented

=== Version 0.2

@@ -51,6 +52,11 @@ abstraction for platform (or hypervisor) specific functionality. The design
of the SBI follows the general RISC-V philosophy of having a small core along
with a set of optional modular extensions.

+SBI extensions as whole are optional but if a SBI <abc> extension compliant
%s/a SBI/an SBI/ (as you will pronounce SBI as as-bee-aye)

+with SBI v0.X spec is implemented then all functions of SBI <abc> extension
+as defined in SBI v0.X are assumed to be present. Basically, a SBI extension
Can we do away with all the placeholders?

How about:

"SBI extensions as whole are optional but they shall not be partially
implemented: If sbi_probe_extension() signals that an extension is
available, it must be implemented in total and conform to the SBI
version reported by sbi_get_spec_version()."

Best regards

Heinrich

+cannot be partially implemented.
+
The higher privilege software providing SBI interface to the supervisor-mode
software is referred to as a SBI implemenation or Supervisor Execution
Environment (SEE). A SBI implementation (or SEE) can be platform runtime


Re: [PATCH] Add direct memory access synchronize extension

Anup Patel
 

-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 04 June 2021 15:20
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Fri, Jun 4, 2021 at 5:33 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:50
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Fri, Jun 4, 2021 at 5:06 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:12
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Fri, Jun 4, 2021 at 4:26 PM Anup Patel <Anup.Patel@...>
wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 13:07
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add
direct memory access synchronize extension

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel
<Anup.Patel@...>
wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH]
Add direct memory access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel
<anup.patel@...>
wrote:

This patch adds SBI direct memory access synchronize
(DSYN)) extension which allows S-mode (or VS-mode)
software to explicitly synchronize memory with
assistance from the M-mode (or
HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index
79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@
https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with
Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?
We have not documented a detailed policy/plan/schedule for all
SBI spec versions.


What is the version supposed to be used for?
The set of functions (or definition of functions) provided by
a SBI extension can change over time so the SBI spec version
helps us differentiate this changes.

For example, SBI HSM extension defined in SBI v0.2 does not
include SBI HSM suspend call but the v0.3 does include SBI HSM
suspend call so the Linux CPUIDLE driver will check both SBI
spec version and availability of SBI HSM extension before
using SBI HSM
suspend call.

Any function not supported, OS can make the SBI call, and check
its return value against SBI_ERR_NOT_SUPPORTED. I don't believe
an arbitrary version number really helps here.
We can't blindly call a SBI function just to check if it is present or not.

For example, the SBI HSM suspend call will suspend the current CPU
and the CPU will not resume until some interrupt/resume event
happens.

If CPU is successfully suspended, then the function is implemented
by SBI firmware. I don't see why I need to care about the version number.
If suspend function is not available, then SBI_ERR_NOT_SUPPORTED is
returned.


That's why we need to use combination of SBI spec version and SBI
probe extension to know whether a particular SBI function is
available or not.

I would like to re-iterate that SBI extensions as whole are
optional but if a SBI <abc> extension compliant with SBI v0.X spec
is implemented then all functions of SBI <abc> extension as
defined in SBI v0.X are assumed to be present. Basically, a SBI
extension cannot be partially implemented.
Is this clearly documented?
Argh, this should have been documented in the introduction chapter.
I can send a patch.
Yes, please go ahead.





Also, newly defined SBI extensions won't be available on
firmware implementing older SBI spec version so S-mode
software should always probe SBI extensions based on SBI spec
version.

For example, SBI SRST extension will be available in only in
firmware implementing SBI v0.3 or higher.
Like you said, SRST extension can be probed. The version number
is not needed.
Checking both SBI spec version before doing SBI probe helps us
avoid unnecessary SBI probe.
Then why did we invent the probe function in the first place? We can
rely on SBI version anyway and maintain a big function matrix in the
OS, but as we introduce more and more extensions over time, I don't
think that's scalable.
Checking SBI spec version before doing SBI probe does not help much
compared to a simple probe without caring about version number.
SBI spec = calling convention + a set of SBI extension

SBI extension = a set of SBI functions

We have the SBI extension probing in SBI spec so that SBI
implementation can skip SBI extension for which some other HW
mechanism is available.

For example, SBI TIMER extension is not required when underlying HW
has RISC-V Sstc extension proposed by Greg
I know probe() can be helpful. I just don't see the value of using version
number to determine whether a certain SBI extension is avaiable.
Yes, using probe() along with spec version can only helps us save
few probe falls. Checking spec version is certainly not mandatory.

Regards,
Anup


Regards,
Bin




[PATCH] Clarify that a SBI extension cannot be partially implemented

Bin Meng
 

Signed-off-by: Bin Meng <bmeng.cn@...>
---

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 11c30c3..8696f97 100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -34,6 +34,7 @@ https://creativecommons.org/licenses/by/4.0/.
* Improved SBI introduction secion
* Improved documentation of SBI hart state managment extension
* Added suspend function to SBI hart state managment extension
+* Clarified that a SBI extension cannot be partially implemented

=== Version 0.2

@@ -51,6 +52,11 @@ abstraction for platform (or hypervisor) specific functionality. The design
of the SBI follows the general RISC-V philosophy of having a small core along
with a set of optional modular extensions.

+SBI extensions as whole are optional but if a SBI <abc> extension compliant
+with SBI v0.X spec is implemented then all functions of SBI <abc> extension
+as defined in SBI v0.X are assumed to be present. Basically, a SBI extension
+cannot be partially implemented.
+
The higher privilege software providing SBI interface to the supervisor-mode
software is referred to as a SBI implemenation or Supervisor Execution
Environment (SEE). A SBI implementation (or SEE) can be platform runtime
--
2.25.1


Re: [PATCH] Add direct memory access synchronize extension

Bin Meng
 

On Fri, Jun 4, 2021 at 5:33 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:50
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Fri, Jun 4, 2021 at 5:06 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:12
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Fri, Jun 4, 2021 at 4:26 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 13:07
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel <Anup.Patel@...>
wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add
direct memory access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel
<anup.patel@...>
wrote:

This patch adds SBI direct memory access synchronize
(DSYN)) extension which allows S-mode (or VS-mode)
software to explicitly synchronize memory with assistance
from the M-mode (or
HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index
79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@
https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?
We have not documented a detailed policy/plan/schedule for all SBI
spec versions.


What is the version supposed to be used for?
The set of functions (or definition of functions) provided by a
SBI extension can change over time so the SBI spec version helps
us differentiate this changes.

For example, SBI HSM extension defined in SBI v0.2 does not
include SBI HSM suspend call but the v0.3 does include SBI HSM
suspend call so the Linux CPUIDLE driver will check both SBI spec
version and availability of SBI HSM extension before using SBI HSM
suspend call.

Any function not supported, OS can make the SBI call, and check its
return value against SBI_ERR_NOT_SUPPORTED. I don't believe an
arbitrary version number really helps here.
We can't blindly call a SBI function just to check if it is present or not.

For example, the SBI HSM suspend call will suspend the current CPU and
the CPU will not resume until some interrupt/resume event happens.
If CPU is successfully suspended, then the function is implemented by SBI
firmware. I don't see why I need to care about the version number.
If suspend function is not available, then SBI_ERR_NOT_SUPPORTED is
returned.


That's why we need to use combination of SBI spec version and SBI
probe extension to know whether a particular SBI function is available
or not.

I would like to re-iterate that SBI extensions as whole are optional
but if a SBI <abc> extension compliant with SBI v0.X spec is
implemented then all functions of SBI <abc> extension as defined in
SBI v0.X are assumed to be present. Basically, a SBI extension cannot
be partially implemented.
Is this clearly documented?
Argh, this should have been documented in the introduction chapter.
I can send a patch.




Also, newly defined SBI extensions won't be available on firmware
implementing older SBI spec version so S-mode software should
always probe SBI extensions based on SBI spec version.

For example, SBI SRST extension will be available in only in
firmware implementing SBI v0.3 or higher.
Like you said, SRST extension can be probed. The version number is
not needed.
Checking both SBI spec version before doing SBI probe helps us avoid
unnecessary SBI probe.
Then why did we invent the probe function in the first place? We can rely on
SBI version anyway and maintain a big function matrix in the OS, but as we
introduce more and more extensions over time, I don't think that's scalable.
Checking SBI spec version before doing SBI probe does not help much
compared to a simple probe without caring about version number.
SBI spec = calling convention + a set of SBI extension

SBI extension = a set of SBI functions

We have the SBI extension probing in SBI spec so that SBI implementation
can skip SBI extension for which some other HW mechanism is available.

For example, SBI TIMER extension is not required when underlying HW
has RISC-V Sstc extension proposed by Greg
I know probe() can be helpful. I just don't see the value of using
version number to determine whether a certain SBI extension is
avaiable.

Regards,
Bin


Re: [PATCH] Add direct memory access synchronize extension

Anup Patel
 

-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:50
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Fri, Jun 4, 2021 at 5:06 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:12
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Fri, Jun 4, 2021 at 4:26 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 13:07
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel <Anup.Patel@...>
wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add
direct memory access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel
<anup.patel@...>
wrote:

This patch adds SBI direct memory access synchronize
(DSYN)) extension which allows S-mode (or VS-mode)
software to explicitly synchronize memory with assistance
from the M-mode (or
HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index
79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@
https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?
We have not documented a detailed policy/plan/schedule for all SBI
spec versions.


What is the version supposed to be used for?
The set of functions (or definition of functions) provided by a
SBI extension can change over time so the SBI spec version helps
us differentiate this changes.

For example, SBI HSM extension defined in SBI v0.2 does not
include SBI HSM suspend call but the v0.3 does include SBI HSM
suspend call so the Linux CPUIDLE driver will check both SBI spec
version and availability of SBI HSM extension before using SBI HSM
suspend call.

Any function not supported, OS can make the SBI call, and check its
return value against SBI_ERR_NOT_SUPPORTED. I don't believe an
arbitrary version number really helps here.
We can't blindly call a SBI function just to check if it is present or not.

For example, the SBI HSM suspend call will suspend the current CPU and
the CPU will not resume until some interrupt/resume event happens.
If CPU is successfully suspended, then the function is implemented by SBI
firmware. I don't see why I need to care about the version number.
If suspend function is not available, then SBI_ERR_NOT_SUPPORTED is
returned.


That's why we need to use combination of SBI spec version and SBI
probe extension to know whether a particular SBI function is available
or not.

I would like to re-iterate that SBI extensions as whole are optional
but if a SBI <abc> extension compliant with SBI v0.X spec is
implemented then all functions of SBI <abc> extension as defined in
SBI v0.X are assumed to be present. Basically, a SBI extension cannot
be partially implemented.
Is this clearly documented?
Argh, this should have been documented in the introduction chapter.



Also, newly defined SBI extensions won't be available on firmware
implementing older SBI spec version so S-mode software should
always probe SBI extensions based on SBI spec version.

For example, SBI SRST extension will be available in only in
firmware implementing SBI v0.3 or higher.
Like you said, SRST extension can be probed. The version number is
not needed.
Checking both SBI spec version before doing SBI probe helps us avoid
unnecessary SBI probe.
Then why did we invent the probe function in the first place? We can rely on
SBI version anyway and maintain a big function matrix in the OS, but as we
introduce more and more extensions over time, I don't think that's scalable.
Checking SBI spec version before doing SBI probe does not help much
compared to a simple probe without caring about version number.
SBI spec = calling convention + a set of SBI extension

SBI extension = a set of SBI functions

We have the SBI extension probing in SBI spec so that SBI implementation
can skip SBI extension for which some other HW mechanism is available.

For example, SBI TIMER extension is not required when underlying HW
has RISC-V Sstc extension proposed by Greg

Regards,
Anup


Regards,
Bin


Re: [PATCH] Add direct memory access synchronize extension

Nick Kossifidis
 

Στις 2021-06-03 18:13, Anup Patel έγραψε:
This patch adds SBI direct memory access synchronize (DSYN)) extension
which allows S-mode (or VS-mode) software to explicitly synchronize
memory with assistance from the M-mode (or HS-mode).
Signed-off-by: Anup Patel <anup.patel@...>
Thanks for working on this, it seems simple and clean, some thoughts:

a) I also prefer DMAS or something with DMA in the name, and fixed-sized arguments.

b) Device and CPU don't necessarily have the same view of the memory, we need to define that physical address is the address the CPU sees.

c) Custom CMOs may only accept virtual addresses instead of physical, in which case we'll need to switch them back to virtual in the firmware. Upon registration SBI may tell the OS if it accepts physical or virtual addresses and the OS can act accordingly (switch cpu_addr to physical or not).

d) Since these operations may also be implemented with custom instructions (instead of e.g. a register write somewhere) I agree that keeping the code in the firmware makes more sense than allowing custom instructions in the kernel, on the other hand these operations are supposed to be performed on S-mode and doing an ecall for them adds a bit of an overhead. This extension would be a good candidate for using the vDSO-like interface we discussed at some point. M-mode could share a code region with S-mode (both PMP and ePMP allow this scenario) and during registration of the extension, SBI will return the physical address of the region, its size and a set of offsets for the different functions in there (in this case only one function). I'm not very passionate about this, after all an ecall isn't that expensive and a DMA sync is not an operation that happens very frequently, but maybe it's a good opportunity to talk about this approach.

Regards,
Nick


Re: [PATCH] Add direct memory access synchronize extension

Bin Meng
 

On Fri, Jun 4, 2021 at 5:06 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:12
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Fri, Jun 4, 2021 at 4:26 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 13:07
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel <Anup.Patel@...>
wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel <anup.patel@...>
wrote:

This patch adds SBI direct memory access synchronize (DSYN))
extension which allows S-mode (or VS-mode) software to
explicitly synchronize memory with assistance from the M-mode (or
HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index
79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@
https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?
We have not documented a detailed policy/plan/schedule for all SBI
spec versions.


What is the version supposed to be used for?
The set of functions (or definition of functions) provided by a SBI
extension can change over time so the SBI spec version helps us
differentiate this changes.

For example, SBI HSM extension defined in SBI v0.2 does not include
SBI HSM suspend call but the v0.3 does include SBI HSM suspend call so
the Linux CPUIDLE driver will check both SBI spec version and
availability of SBI HSM extension before using SBI HSM suspend call.
Any function not supported, OS can make the SBI call, and check its return
value against SBI_ERR_NOT_SUPPORTED. I don't believe an arbitrary version
number really helps here.
We can't blindly call a SBI function just to check if it is present or not.

For example, the SBI HSM suspend call will suspend the current CPU
and the CPU will not resume until some interrupt/resume event
happens.
If CPU is successfully suspended, then the function is implemented by
SBI firmware. I don't see why I need to care about the version number.
If suspend function is not available, then SBI_ERR_NOT_SUPPORTED is
returned.


That's why we need to use combination of SBI spec version and
SBI probe extension to know whether a particular SBI function is
available or not.

I would like to re-iterate that SBI extensions as whole are optional
but if a SBI <abc> extension compliant with SBI v0.X spec is implemented
then all functions of SBI <abc> extension as defined in SBI v0.X are
assumed to be present. Basically, a SBI extension cannot be partially
implemented.
Is this clearly documented?


Also, newly defined SBI extensions won't be available on firmware
implementing older SBI spec version so S-mode software should always
probe SBI extensions based on SBI spec version.

For example, SBI SRST extension will be available in only in firmware
implementing SBI v0.3 or higher.
Like you said, SRST extension can be probed. The version number is not
needed.
Checking both SBI spec version before doing SBI probe helps us avoid
unnecessary SBI probe.
Then why did we invent the probe function in the first place? We can
rely on SBI version anyway and maintain a big function matrix in the
OS, but as we introduce more and more extensions over time, I don't
think that's scalable. Checking SBI spec version before doing SBI
probe does not help much compared to a simple probe without caring
about version number.

Regards,
Bin


Re: [PATCH] Add direct memory access synchronize extension

Anup Patel
 

-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 14:12
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Fri, Jun 4, 2021 at 4:26 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 13:07
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel <Anup.Patel@...>
wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel <anup.patel@...>
wrote:

This patch adds SBI direct memory access synchronize (DSYN))
extension which allows S-mode (or VS-mode) software to
explicitly synchronize memory with assistance from the M-mode (or
HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index
79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@
https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?
We have not documented a detailed policy/plan/schedule for all SBI
spec versions.


What is the version supposed to be used for?
The set of functions (or definition of functions) provided by a SBI
extension can change over time so the SBI spec version helps us
differentiate this changes.

For example, SBI HSM extension defined in SBI v0.2 does not include
SBI HSM suspend call but the v0.3 does include SBI HSM suspend call so
the Linux CPUIDLE driver will check both SBI spec version and
availability of SBI HSM extension before using SBI HSM suspend call.
Any function not supported, OS can make the SBI call, and check its return
value against SBI_ERR_NOT_SUPPORTED. I don't believe an arbitrary version
number really helps here.
We can't blindly call a SBI function just to check if it is present or not.

For example, the SBI HSM suspend call will suspend the current CPU
and the CPU will not resume until some interrupt/resume event
happens.

That's why we need to use combination of SBI spec version and
SBI probe extension to know whether a particular SBI function is
available or not.

I would like to re-iterate that SBI extensions as whole are optional
but if a SBI <abc> extension compliant with SBI v0.X spec is implemented
then all functions of SBI <abc> extension as defined in SBI v0.X are
assumed to be present. Basically, a SBI extension cannot be partially
implemented.


Also, newly defined SBI extensions won't be available on firmware
implementing older SBI spec version so S-mode software should always
probe SBI extensions based on SBI spec version.

For example, SBI SRST extension will be available in only in firmware
implementing SBI v0.3 or higher.
Like you said, SRST extension can be probed. The version number is not
needed.
Checking both SBI spec version before doing SBI probe helps us avoid
unnecessary SBI probe.

Regards,
Anup


Regards,
Bin


Re: [PATCH] Add direct memory access synchronize extension

Bin Meng
 

On Fri, Jun 4, 2021 at 4:26 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 13:07
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel <anup.patel@...>
wrote:

This patch adds SBI direct memory access synchronize (DSYN))
extension which allows S-mode (or VS-mode) software to explicitly
synchronize memory with assistance from the M-mode (or HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index
79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@ https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?
We have not documented a detailed policy/plan/schedule for all
SBI spec versions.


What is the version supposed to be used for?
The set of functions (or definition of functions) provided by a
SBI extension can change over time so the SBI spec version helps
us differentiate this changes.

For example, SBI HSM extension defined in SBI v0.2 does not include
SBI HSM suspend call but the v0.3 does include SBI HSM suspend call
so the Linux CPUIDLE driver will check both SBI spec version and
availability of SBI HSM extension before using SBI HSM suspend call.
Any function not supported, OS can make the SBI call, and check its
return value against SBI_ERR_NOT_SUPPORTED. I don't believe an
arbitrary version number really helps here.

Also, newly defined SBI extensions won't be available on firmware
implementing older SBI spec version so S-mode software should
always probe SBI extensions based on SBI spec version.

For example, SBI SRST extension will be available in only in
firmware implementing SBI v0.3 or higher.
Like you said, SRST extension can be probed. The version number is not needed.

Regards,
Bin


Re: [PATCH] Add direct memory access synchronize extension

Anup Patel
 

-----Original Message-----
From: Bin Meng <bmeng.cn@...>
Sent: 04 June 2021 13:07
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct
memory access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel <anup.patel@...>
wrote:

This patch adds SBI direct memory access synchronize (DSYN))
extension which allows S-mode (or VS-mode) software to explicitly
synchronize memory with assistance from the M-mode (or HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index
79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@ https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?
We have not documented a detailed policy/plan/schedule for all
SBI spec versions.


What is the version supposed to be used for?
The set of functions (or definition of functions) provided by a
SBI extension can change over time so the SBI spec version helps
us differentiate this changes.

For example, SBI HSM extension defined in SBI v0.2 does not include
SBI HSM suspend call but the v0.3 does include SBI HSM suspend call
so the Linux CPUIDLE driver will check both SBI spec version and
availability of SBI HSM extension before using SBI HSM suspend call.

Also, newly defined SBI extensions won't be available on firmware
implementing older SBI spec version so S-mode software should
always probe SBI extensions based on SBI spec version.

For example, SBI SRST extension will be available in only in
firmware implementing SBI v0.3 or higher.

Regards,
Anup


Regards,
Bin


Re: [PATCH] Add direct memory access synchronize extension

Bin Meng
 

On Thu, Jun 3, 2021 at 11:47 PM Anup Patel <Anup.Patel@...> wrote:



-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel <anup.patel@...> wrote:

This patch adds SBI direct memory access synchronize (DSYN)) extension
which allows S-mode (or VS-mode) software to explicitly synchronize
memory with assistance from the M-mode (or HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index 79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@ https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.
Do we have policies, or planning/schedule of versions?

What is the version supposed to be used for?

Regards,
Bin


Re: [PATCH 1/1] riscv-sbi.adoc: fix typos

Bin Meng
 

On Fri, Jun 4, 2021 at 5:17 AM Heinrich Schuchardt <xypron.glpk@...> wrote:

%s/secion/section/
%s/managment/management/
%s/implemenation/implementation/
%s/requestd/requested/
%s/hierarchial/hierarchical/
%s/inititated/initiated/
%s/recieves/receives/
%s/rententive/retentive/

Signed-off-by: Heinrich Schuchardt <xypron.glpk@...>
---
riscv-sbi.adoc | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
Reviewed-by: Bin Meng <bmeng.cn@...>


[PATCH 1/1] riscv-sbi.adoc: fix typos

Heinrich Schuchardt
 

%s/secion/section/
%s/managment/management/
%s/implemenation/implementation/
%s/requestd/requested/
%s/hierarchial/hierarchical/
%s/inititated/initiated/
%s/recieves/receives/
%s/rententive/retentive/

Signed-off-by: Heinrich Schuchardt <xypron.glpk@...>
=2D--
riscv-sbi.adoc | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
index 11c30c3..4b6f2c3 100644
=2D-- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -31,9 +31,9 @@ https://creativecommons.org/licenses/by/4.0/.

* Improved document styling and naming conventions
* Added SBI system reset extension
-* Improved SBI introduction secion
-* Improved documentation of SBI hart state managment extension
-* Added suspend function to SBI hart state managment extension
+* Improved SBI introduction section
+* Improved documentation of SBI hart state management extension
+* Added suspend function to SBI hart state management extension

=3D=3D=3D Version 0.2

@@ -52,7 +52,7 @@ of the SBI follows the general RISC-V philosophy of havi=
ng a small core along
with a set of optional modular extensions.

The higher privilege software providing SBI interface to the supervisor-m=
ode
-software is referred to as a SBI implemenation or Supervisor Execution
+software is referred to as a SBI implementation or Supervisor Execution
Environment (SEE). A SBI implementation (or SEE) can be platform runtime
firmware executing in machine-mode (M-mode) (see below <<fig_intro1>>) or
it can be some hypervisor executing in hypervisor-mode (HS-mode) (see bel=
ow
@@ -741,7 +741,7 @@ along with a unique **HSM state id** for each state:
in the **STOPPED** state.
| 4 | SUSPENDED | This hart is in a platform specific suspen=
d
(or low power) state.
-| 5 | SUSPEND_PENDING | The hart has requestd to put itself in a
+| 5 | SUSPEND_PENDING | The hart has requested to put itself in a
platform specific low power state from the
**STARTED** state and the SBI implementati=
on
is still working to get the hart in the
@@ -764,22 +764,22 @@ image::riscv-sbi-hsm.png[]
A platform can have multiple harts grouped into a hierarchical topology
groups (namely cores, clusters, nodes, etc) with separate platform specif=
ic
low-power states for each hierarchical group. These platform specific
-low-power states of hierarchial topology groups can be represented as
+low-power states of hierarchical topology groups can be represented as
platform specific suspend states of a hart. A SBI implementation can
utilize the suspend states of higher topology groups using one of the
following approaches:

. *Platform-coordinated:* In this approach, when a hart becomes idle the
- supervisor-mode power-managment software will request deepest suspend
+ supervisor-mode power-management software will request deepest suspend
state for the hart and higher topology groups. A SBI implementation
should choose a suspend state at higher topology group which is:
.. Not deeper than the specified suspend state
.. Wake-up latency is not higher than the wake-up latency of the
specified suspend state
-. *OS-inititated:* In this approach, the supervisor-mode power-managment
+. *OS-initiated:* In this approach, the supervisor-mode power-management
software will directly request a suspend state for higher topology grou=
p
after the last hart in that group becomes idle. When a hart becomes idl=
e,
- the supervisor-mode power-managment software will always select suspend
+ the supervisor-mode power-management software will always select suspen=
d
state for the hart itself but it will select a suspend state for a high=
er
topology group only if the hart is the last running hart in the group.
A SBI implementation should:
@@ -815,7 +815,7 @@ below.
|=3D=3D=3D

This call is asynchronous -- more specifically, the `sbi_hart_start()` ma=
y
-return before target hart starts executing as long as the SBI implemenati=
on
+return before target hart starts executing as long as the SBI implementat=
ion
is capable of ensuring the return code is accurate. It is recommended tha=
t
if the SBI implementation is a platform runtime firmware executing in
machine-mode (M-mode) then it MUST configure PMP and other M-mode state
@@ -915,13 +915,13 @@ struct sbiret sbi_hart_suspend(uint32_t suspend_type=
,
unsigned long opaque)
----

-Request the SBI implementation to put the calling hart in a platform spec=
fic
+Request the SBI implementation to put the calling hart in a platform spec=
ific
suspend (or low power) state specified by the `suspend_type` parameter. T=
he
hart will automatically come out of suspended state and resume normal
-execution when it recieves an interrupt or platform specific hardware eve=
nt.
+execution when it receives an interrupt or platform specific hardware eve=
nt.

The platform specific suspend states for a hart can be either retentive
-or non-rententive in nature. A retentive suspend state will preserve hart
+or non-retentive in nature. A retentive suspend state will preserve hart
register and CSR values for all privilege modes whereas a non-retentive
suspend state will not preserve hart register and CSR values.

=2D-
2.30.2


Re: [PATCH] Add direct memory access synchronize extension

Anup Patel
 

The SBI_DMA_SYNC_NONE if succeeds tells supervisor-mode that target memory regions is valid and DMA sync calls with other directions will go through.

 

In this first draft, I tried to keep various DMA directions in sync with DMA directions defined by Linux DMA mappings . I am fine to drop SBI_DMA_SYNC_NONE as well.

 

Regards,

Anup

 

From: tech-unixplatformspec@... <tech-unixplatformspec@...> On Behalf Of Jonathan Behrens
Sent: 03 June 2021 21:14
To: Bin Meng <bmeng.cn@...>
Cc: Anup Patel <Anup.Patel@...>; tech-unixplatformspec@...; Atish Patra <Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory access synchronize extension

 

Could you clarify what SBI_DMA_SYNC_NONE does and how it would help with debugging?

 

Jonathan

 

On Thu, Jun 3, 2021 at 11:32 AM Bin Meng via lists.riscv.org <bmeng.cn=gmail.com@...> wrote:

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel <anup.patel@...> wrote:
>
> This patch adds SBI direct memory access synchronize (DSYN)) extension
> which allows S-mode (or VS-mode) software to explicitly synchronize
> memory with assistance from the M-mode (or HS-mode).
>
> Signed-off-by: Anup Patel <anup.patel@...>
> ---
>  riscv-sbi.adoc | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
>
> diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc
> index 79d98a6..0eb2898 100644
> --- a/riscv-sbi.adoc
> +++ b/riscv-sbi.adoc
> @@ -27,6 +27,10 @@ https://creativecommons.org/licenses/by/4.0/.
>  [preface]
>  == Change Log
>
> +=== Version 0.4-rc0

What's our policy of bumping up versions?

> +
> +* Added direct memory access synchronize extension
> +
>  === Version 0.3-rc0

Is there no version 0.3, but just -rc0?

>
>  * Improved document styling and naming conventions
> @@ -1550,6 +1554,97 @@ The possible error codes returned in `sbiret.error` are shown in the
>  | sbi_pmu_counter_fw_read         | 0.3         | 5   | 0x504D55
>  |===
>
> +== Direct Memory Access Synchronize Extension (EID #0x4453594e "DSYN")

I am not sure DSYN is a good name. How about DMAS?

> +
> +A RISC-V platform will generally have direct memory access
> +(https://en.wikipedia.org/wiki/Direct_memory_access[DMA]) capable devices.
> +These DMA capable devices can sometimes be non-coherent with HART caches (i.e.
> +I/O non-coherent) hence requiring explicit cache flush and/or invalidate from
> +HART to synchronize memory with the DMA capable device. The SBI direct memory
> +access synchronize (DSYN) extension is an interface for supervisor-mode to
> +explicitly synchronize memory region with assistance from the machine-mode
> +(or hypervisor-mode).

Does RVI have plan to introduce cache instructions into the ISA?

This extension only makes sense if the cache instructions are not
allowed in less privileged mode.

> +
> +=== Function: DMA Synchronize (FID #0)
> +
> +[source, C]
> +----
> +struct sbiret sbi_dma_synchronize(unsigned long addr, unsigned long size,

For 32-bit system, "unsigned long" cannot represent a physical address
beyond 4GiB. I am not sure if size is an issue (> 4GiB) too.

> +                                  unsigned long direction)
> +----
> +
> +Synchronize a memory region for non-coherent DMA capable devices based on
> +`addr`, `size` and `direction` paramenters. The `addr` and `size` parameter

typo: parameters

> +represent the physical address and size of memory region whereas `direction`
> +parameter represents the direction of synchronization with possible values
> +shown in <<table_dma_sync_direction_list>> below.
> +
> +[#table_dma_sync_direction_list]
> +.DMA Synchronize Directions
> +[cols="4,1,5", width=95%, align="center", options="header"]
> +|===
> +| Direction Name             | Value | Description
> +| SBI_DMA_SYNC_BIDIRECTIONAL |     0 | Data direction isn't known. +
> +                                       +
> +                                       The DMA synchronization in this
> +                                       direction must be done: +
> +                                       * once before the memory region is
> +                                         handed off to the device. +
> +                                       * once before the memory region is
> +                                         accessed after being used by the
> +                                         device.
> +| SBI_DMA_SYNC_TO_DEVICE     |     1 | Data is going from the memory region
> +                                       to the device. +
> +                                       +
> +                                       The DMA synchronization in this
> +                                       direction must be done after the last
> +                                       modification of the memory region by
> +                                       the supervisor-mode and before region
> +                                       is handed off to the device.
> +| SBI_DMA_SYNC_FROM_DEVICE   |     2 | Data is coming from the device to
> +                                       the memory region. +
> +                                       +
> +                                       The DMA synchronization in this
> +                                       direction must be before the
> +                                       supervisor-mode accesses memory region
> +                                       that may have been updated by the
> +                                       device.
> +| SBI_DMA_SYNC_NONE          |     3 | No data direction. +
> +                                       +
> +                                       This is only for debugging and does
> +                                       not do any DMA synchronization.
> +| *RESERVED*                 |   > 3 | Reserved for future use
> +|===

These seem vague to me. I think the intention is to map cache
operations, and shouldn't cache management extension be a clearer
name?

> +
> +The possible error codes returned in `sbiret.error` are shown in the
> +<<table_dma_sync_errors>> below.
> +
> +[#table_dma_sync_errors]
> +.DMA Synchronize Errors
> +[cols="1,2", width=100%, align="center", options="header"]
> +|===
> +| Error code              | Description
> +| SBI_SUCCESS             | memory synchronized successfully.
> +| SBI_ERR_INVALID_PARAM   | `direction` is not valid.
> +| SBI_ERR_INVALID_ADDRESS | memory region pointed by `addr` and `size`
> +                            parameter is not valid possibly due to
> +                            following reasons: +
> +                            * It is not a valid physical address range. +
> +                            * The memory address range is prohibited by
> +                              PMP to access in supervisor-mode.
> +| SBI_ERR_FAILED          | memory synchroinzation failed for unknown reasons.

typo: synchronization

> +|===
> +
> +=== Function Listing
> +
> +[#table_dsyn_function_list]
> +.DSYN Function List
> +[cols="5,2,1,2", width=80%, align="center", options="header"]
> +|===
> +| Function Name                   | SBI Version | FID | EID
> +| sbi_dma_synchronize             | 0.4         | 0   | 0x4453594e
> +|===
> +
>  == Experimental SBI Extension Space (EIDs #0x08000000 - #0x08FFFFFF)
>
>  No management.
> --

Regards,
Bin





Re: [PATCH] Add direct memory access synchronize extension

Anup Patel
 

-----Original Message-----
From: tech-unixplatformspec@... <tech-
unixplatformspec@...> On Behalf Of Bin Meng
Sent: 03 June 2021 21:02
To: Anup Patel <Anup.Patel@...>
Cc: tech-unixplatformspec@...; Atish Patra
<Atish.Patra@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

On Thu, Jun 3, 2021 at 11:13 PM Anup Patel <anup.patel@...> wrote:

This patch adds SBI direct memory access synchronize (DSYN)) extension
which allows S-mode (or VS-mode) software to explicitly synchronize
memory with assistance from the M-mode (or HS-mode).

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

diff --git a/riscv-sbi.adoc b/riscv-sbi.adoc index 79d98a6..0eb2898
100644
--- a/riscv-sbi.adoc
+++ b/riscv-sbi.adoc
@@ -27,6 +27,10 @@ https://creativecommons.org/licenses/by/4.0/.
[preface]
== Change Log

+=== Version 0.4-rc0
What's our policy of bumping up versions?
This extension is meant for SBI v0.4 based on discussion with Atish.

We will be soon freezing SBI v0.3.


+
+* Added direct memory access synchronize extension
+
=== Version 0.3-rc0
Is there no version 0.3, but just -rc0?
SBI v0.3 will be released soon so that we can proceed with
Kernel patches.



* Improved document styling and naming conventions @@ -1550,6
+1554,97 @@ The possible error codes returned in `sbiret.error` are shown
in the
| sbi_pmu_counter_fw_read | 0.3 | 5 | 0x504D55
|===

+== Direct Memory Access Synchronize Extension (EID #0x4453594e
+"DSYN")
I am not sure DSYN is a good name. How about DMAS?
I am fine with both DSYN and DMAS names. Let's see what other people think.


+
+A RISC-V platform will generally have direct memory access
+(https://en.wikipedia.org/wiki/Direct_memory_access[DMA]) capable
devices.
+These DMA capable devices can sometimes be non-coherent with HART
caches (i.e.
+I/O non-coherent) hence requiring explicit cache flush and/or
+invalidate from HART to synchronize memory with the DMA capable
+device. The SBI direct memory access synchronize (DSYN) extension is
+an interface for supervisor-mode to explicitly synchronize memory
+region with assistance from the machine-mode (or hypervisor-mode).
Does RVI have plan to introduce cache instructions into the ISA?
There is Cache Maintenance Operation (CMO) TG but it is just a year old TG
and they are far from freezing. I am guess it will take few more years for CMO
extension to be frozen and Linux kernel will not take patches for draft
extensions.


This extension only makes sense if the cache instructions are not allowed in
less privileged mode.
This extension is like a stop-gap solution due to CMO extension being
unavailable. It is meant to abstract out platform specific non-standard
DMA sync mechanism.

Like all other SBI extension, this SBI extension is also optional and it
will be available only on platforms having custom / non-standard
cache operations.


+
+=== Function: DMA Synchronize (FID #0)
+
+[source, C]
+----
+struct sbiret sbi_dma_synchronize(unsigned long addr, unsigned long
+size,
For 32-bit system, "unsigned long" cannot represent a physical address
beyond 4GiB. I am not sure if size is an issue (> 4GiB) too.
Sure, we can make this uint64_t


+ unsigned long direction)
+----
+
+Synchronize a memory region for non-coherent DMA capable devices
+based on `addr`, `size` and `direction` paramenters. The `addr` and
+`size` parameter
typo: parameters
Okay will update.


+represent the physical address and size of memory region whereas
+`direction` parameter represents the direction of synchronization
+with possible values shown in <<table_dma_sync_direction_list>> below.
+
+[#table_dma_sync_direction_list]
+.DMA Synchronize Directions
+[cols="4,1,5", width=95%, align="center", options="header"]
+|===
+| Direction Name | Value | Description
+| SBI_DMA_SYNC_BIDIRECTIONAL | 0 | Data direction isn't known. +
+ +
+ The DMA synchronization in this
+ direction must be done: +
+ * once before the memory region is
+ handed off to the device. +
+ * once before the memory region is
+ accessed after being used by the
+ device.
+| SBI_DMA_SYNC_TO_DEVICE | 1 | Data is going from the memory
region
+ to the device. +
+ +
+ The DMA synchronization in this
+ direction must be done after the last
+ modification of the memory region by
+ the supervisor-mode and before region
+ is handed off to the device.
+| SBI_DMA_SYNC_FROM_DEVICE | 2 | Data is coming from the device
to
+ the memory region. +
+ +
+ The DMA synchronization in this
+ direction must be before the
+ supervisor-mode accesses memory region
+ that may have been updated by the
+ device.
+| SBI_DMA_SYNC_NONE | 3 | No data direction. +
+ +
+ This is only for debugging and does
+ not do any DMA synchronization.
+| *RESERVED* | > 3 | Reserved for future use
+|===
These seem vague to me. I think the intention is to map cache operations, and
shouldn't cache management extension be a clearer name?
No, the intention is only to map DMA sync operation.

The cache management operations need provide cache maintenance
for a particular cache-level in a cache hierarchy.


+
+The possible error codes returned in `sbiret.error` are shown in the
+<<table_dma_sync_errors>> below.
+
+[#table_dma_sync_errors]
+.DMA Synchronize Errors
+[cols="1,2", width=100%, align="center", options="header"]
+|===
+| Error code | Description
+| SBI_SUCCESS | memory synchronized successfully.
+| SBI_ERR_INVALID_PARAM | `direction` is not valid.
+| SBI_ERR_INVALID_ADDRESS | memory region pointed by `addr` and
+|`size`
+ parameter is not valid possibly due to
+ following reasons: +
+ * It is not a valid physical address range. +
+ * The memory address range is prohibited by
+ PMP to access in supervisor-mode.
+| SBI_ERR_FAILED | memory synchroinzation failed for unknown
reasons.

typo: synchronization
Okay will update.


+|===
+
+=== Function Listing
+
+[#table_dsyn_function_list]
+.DSYN Function List
+[cols="5,2,1,2", width=80%, align="center", options="header"]
+|===
+| Function Name | SBI Version | FID | EID
+| sbi_dma_synchronize | 0.4 | 0 | 0x4453594e
+|===
+
== Experimental SBI Extension Space (EIDs #0x08000000 - #0x08FFFFFF)

No management.
--
Regards,
Bin



Regards,
Anup

821 - 840 of 1847