Date   

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


Re: [PATCH] Add direct memory access synchronize extension

Jonathan Behrens <behrensj@...>
 

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

Bin Meng
 

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


[PATCH] Add direct memory access synchronize extension

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@...>
---
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
+
+* Added direct memory access synchronize extension
+
=== Version 0.3-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")
+
+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).
+
+=== Function: DMA Synchronize (FID #0)
+
+[source, C]
+----
+struct sbiret sbi_dma_synchronize(unsigned long addr, unsigned long size,
+ 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
+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
+|===
+
+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.
+|===
+
+=== 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.
--
2.25.1


Next Platform HSC Meeting on Fri Jun 4 2021 8AM PST

Kumar Sankaran
 

Hi All,
The next platform HSC meeting is scheduled on Fri Jun 4th at 8AM PST.
This is a new slot only for this week due to Monday being a holiday in the
US for Memorial Day and Wed being the RISC-V tool chain event.

Here are the details:

Agenda and minutes kept on the github wiki:
https://github.com/riscv/riscv-platform-specs/wiki

Here are the slides:
https://docs.google.com/presentation/d/1eaobawWTZqsIadaTab77LZ0WC_IHi7yuG1Sm0x3d9jc/edit#slide=id.gd675c2d261_0_0

Meeting info
Zoom meeting: https://zoom.us/j/2786028446
Passcode: 901897

Or iPhone one-tap :
US: +16465588656,,2786028466# or +16699006833,,2786028466# Or Telephone:
Dial(for higher quality, dial a number based on your current location):
US: +1 646 558 8656 or +1 669 900 6833
Meeting ID: 278 602 8446
International numbers available:
https://zoom.us/zoomconference?m=_R0jyyScMETN7-xDLLRkUFxRAP07A-_

Regards
Kumar


Re: [PATCH v1 2/2] Platform debug requirements

Paul Donahue
 

As architected, it setting up breakpoints/watchpoints would have to be handled via SBI.  It's possible to trap to M mode and then trampoline back to S as you say.  Another option is to have M mode use medeleg[3] to cause all triggers and ebreak instructions to go directly to S mode (assuming M mode doesn't want to handle any debug traps at all).

The ability to have direct self-hosted debug capability for S (and VS) has definitely crossed my mind but that's beyond the current scope.  I don't think that anyone has really spent a lot of time thinking about the details of how that might work.


Thanks,

-Paul

On Tue, Jun 1, 2021 at 12:40 PM Jonathan Behrens <behrensj@...> wrote:
That was an interesting read. Do you know what the thinking is on providing self-hosted debug functionality to S-mode? Is this something that's expected to be done via SBI? I think it might be possible to wire something up that lets S-mode set breakpoints/watchpoints with SBI calls and then when those events trigger has it trampoline from M-mode back into S-mode.

Jonathan

On Tue, Jun 1, 2021 at 2:48 PM Paul Donahue via lists.riscv.org <pdonahue=ventanamicro.com@...> wrote:
Chapter 5 of the debug spec covers triggers that are used for self-hosted and/or external debug.  The action field of each trigger must be programmed according to table 5.1 to either trap or halt.


This is also true in the ratified 0.13.2 debug spec from 2019, though it was easy to be misled by that document's title ("RISC-V External Debug Support").


Thanks,

-Paul

On Mon, May 31, 2021 at 6:10 PM Jonathan Behrens <behrensj@...> wrote:
Could you link to documentation about self-hosted debug? The only proposed specifications I've seen are for external debug support.

Thanks,
Jonathan

On Fri, May 28, 2021 at 7:47 PM Paul Donahue via lists.riscv.org <pdonahue=ventanamicro.com@...> wrote:
Signed-off-by: Paul Donahue pdonahue@...
---
riscv-platform-spec.adoc | 101 +++++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 98783c8..e5a1236 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -73,6 +73,93 @@ include::profiles.adoc[]
** ASID
 * Debug
+==== Debug
+The Linux base platform requirements are -
+
+- Implement resethaltreq
+  * Rationale: Debugging immediately out of reset is a useful debug tool and
+    is required by item 5 in chapter 3. The resethaltreq mechanism provides a
+    standard way to do this.
+- Implement the program buffer
+  * Rationale: The program buffer is easier for most implementations than
+    abstract access.
+  * Rationale: Debuggers need to be able to insert ebreak instructions into
+    memory and make sure that the ebreak is visible to subsequent instruction
+    fetches.  Abstract access has no support for fence.i (or similar
+    mechanisms).
+- abstractcs.relaxedpriv must be 0
+  * Rationale: Doing otherwise is a potential security problem.
+- abstractauto must be implemented
+  * Rationale: autoexecprogbuf allows faster instruction-stuffing
+  * Rationale: autoexecdata allows fast read/write of a region of memory
+- dcsr.mprven must be tied to 1
+  * Rationale: Emulating two-stage table walks and PMP checks and endianness
+    swapping is a heavy burden on the debugger.
+- In textra, sselect must support the value 0 and either value 1 or 2 (or
+both).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular user context and a way to ignore user context.
+- If textra.sselect=1 is supported, the number of implemented bits of svalue
+must be at least the number of implemented bits of scontext.
+  * Rationale: This allows matching on every possible scontext.
+- If textra.sselect=2 is supported, the number of implemented bits of svalue
+must be at least ASIDLEN.
+  * Rationale: This allows matching on every possible ASID.
+- In textra, mhselect must support the value 0.  If the H extension is
+supported then mhselect must also support either values 1 and 5 or values 2
+and 6 (or all four).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular guest context and a way to ignore guest context.
+- If textra.mhselect=1,5 are supported and if H is the number of implemented
+bits of hcontext then, unless all bits of mhvalue are implemented, at least
+H-1 bits of mhvalue must be implemented.
+  * Rationale: This allows matching on every possible hcontext (up to the limit
+    of the field width).  It is H-1 bits instead of H because mhselect[2]
+    provides one bit.
+- If textra.mhselect=2,6 are supported, the number of implemented bits of
+mhvalue must be at least VMIDLEN-1.
+  * Rationale: This allows matching on every possible VMID.  It is VMIDLEN-1
+    instead of VMIDLEN because mhselect[2] provides one bit.
+- Implement at least four mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: The debugger needs breakpoints and 4 is a sufficient baseline.
+- Implement at least four mcontrol6 triggers that can support matching on load
+ and store addresses (select=0, match=0, and all combinations of load/store)
+ with timing=0 and full support for mode filtering (vs, vu, m, s, u) for all
+ supported modes and support for textra as above.
+  * Rationale: The debugger needs watchpoints and 4 is a sufficient baseline.
+- Implement at least one trigger capable of icount and support for textra as
+above.
+  * Rationale: Self-hosted single step needs this
+- Implement at least one trigger capable of etrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch exceptions.
+- Implement at least one trigger capable of itrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch interrupts.
+- The minimum trigger requirements must be met for action=0 and for action=1
+(possibly by the same triggers)
+  * Rationale: The intent is to have full support for external debug and full
+    support for self-hosted debug (though not necessarily at the same time).
+    This can be provided via the same set of triggers or separate sets of
+    triggers. External debug support for icount is unnecessary due to dcsr.step
+    and is therefore called out separately.
+- For implementations with multiple cores, support for at least one halt group
+and one resume group (in addition to group 0)
+  * Rationale: Allows stopping all harts (approximately) simultaneously which
+    is useful for debugging MP software
+- dcsr.stepie must support the 0 setting.  It is optional to support the 1
+setting.
+  * Rationale: It is not generally useful to step into interrupt
handlers.
+- dcsr.stopcount and dcsr.stoptime must be supported and the reset value of
+each must be 1
+  * Rationale: The architecture has strict requirements on minstret which may
+    be perturbed by an external debugger in a way that's visible to software.
+    The default should allow code that's sensitive to these requirements to be
+    debugged.
+
==== Memory Map
* Virtual Memory
* sv39/sv48/sv57
@@ -166,6 +253,20 @@ Spec should be ratified*]
file for each HART ?? (*TBD*)
- IOMMU with support for memory resident interrupt files ?? (*TBD*)
+==== Debug
+The Linux server platform requirements are all of the above plus:
+
+- Implement at least six mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: Other architectures have found that 4 breakpoints are
+    insufficient in more capable systems and recommend 6.
+- If system bus access is implemented then accesses must be coherent with
+respect to all harts connected to the DM
+  * Rationale: Debuggers must be able to view memory coherently
+
+
==== Boot and Runtime Requirements
=====  Firmware
The boot and system firmware for the RV64I server platforms required to be
--
2.25.1






Re: [PATCH v1 2/2] Platform debug requirements

Jonathan Behrens <behrensj@...>
 

That was an interesting read. Do you know what the thinking is on providing self-hosted debug functionality to S-mode? Is this something that's expected to be done via SBI? I think it might be possible to wire something up that lets S-mode set breakpoints/watchpoints with SBI calls and then when those events trigger has it trampoline from M-mode back into S-mode.

Jonathan


On Tue, Jun 1, 2021 at 2:48 PM Paul Donahue via lists.riscv.org <pdonahue=ventanamicro.com@...> wrote:
Chapter 5 of the debug spec covers triggers that are used for self-hosted and/or external debug.  The action field of each trigger must be programmed according to table 5.1 to either trap or halt.


This is also true in the ratified 0.13.2 debug spec from 2019, though it was easy to be misled by that document's title ("RISC-V External Debug Support").


Thanks,

-Paul

On Mon, May 31, 2021 at 6:10 PM Jonathan Behrens <behrensj@...> wrote:
Could you link to documentation about self-hosted debug? The only proposed specifications I've seen are for external debug support.

Thanks,
Jonathan

On Fri, May 28, 2021 at 7:47 PM Paul Donahue via lists.riscv.org <pdonahue=ventanamicro.com@...> wrote:
Signed-off-by: Paul Donahue pdonahue@...
---
riscv-platform-spec.adoc | 101 +++++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 98783c8..e5a1236 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -73,6 +73,93 @@ include::profiles.adoc[]
** ASID
 * Debug
+==== Debug
+The Linux base platform requirements are -
+
+- Implement resethaltreq
+  * Rationale: Debugging immediately out of reset is a useful debug tool and
+    is required by item 5 in chapter 3. The resethaltreq mechanism provides a
+    standard way to do this.
+- Implement the program buffer
+  * Rationale: The program buffer is easier for most implementations than
+    abstract access.
+  * Rationale: Debuggers need to be able to insert ebreak instructions into
+    memory and make sure that the ebreak is visible to subsequent instruction
+    fetches.  Abstract access has no support for fence.i (or similar
+    mechanisms).
+- abstractcs.relaxedpriv must be 0
+  * Rationale: Doing otherwise is a potential security problem.
+- abstractauto must be implemented
+  * Rationale: autoexecprogbuf allows faster instruction-stuffing
+  * Rationale: autoexecdata allows fast read/write of a region of memory
+- dcsr.mprven must be tied to 1
+  * Rationale: Emulating two-stage table walks and PMP checks and endianness
+    swapping is a heavy burden on the debugger.
+- In textra, sselect must support the value 0 and either value 1 or 2 (or
+both).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular user context and a way to ignore user context.
+- If textra.sselect=1 is supported, the number of implemented bits of svalue
+must be at least the number of implemented bits of scontext.
+  * Rationale: This allows matching on every possible scontext.
+- If textra.sselect=2 is supported, the number of implemented bits of svalue
+must be at least ASIDLEN.
+  * Rationale: This allows matching on every possible ASID.
+- In textra, mhselect must support the value 0.  If the H extension is
+supported then mhselect must also support either values 1 and 5 or values 2
+and 6 (or all four).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular guest context and a way to ignore guest context.
+- If textra.mhselect=1,5 are supported and if H is the number of implemented
+bits of hcontext then, unless all bits of mhvalue are implemented, at least
+H-1 bits of mhvalue must be implemented.
+  * Rationale: This allows matching on every possible hcontext (up to the limit
+    of the field width).  It is H-1 bits instead of H because mhselect[2]
+    provides one bit.
+- If textra.mhselect=2,6 are supported, the number of implemented bits of
+mhvalue must be at least VMIDLEN-1.
+  * Rationale: This allows matching on every possible VMID.  It is VMIDLEN-1
+    instead of VMIDLEN because mhselect[2] provides one bit.
+- Implement at least four mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: The debugger needs breakpoints and 4 is a sufficient baseline.
+- Implement at least four mcontrol6 triggers that can support matching on load
+ and store addresses (select=0, match=0, and all combinations of load/store)
+ with timing=0 and full support for mode filtering (vs, vu, m, s, u) for all
+ supported modes and support for textra as above.
+  * Rationale: The debugger needs watchpoints and 4 is a sufficient baseline.
+- Implement at least one trigger capable of icount and support for textra as
+above.
+  * Rationale: Self-hosted single step needs this
+- Implement at least one trigger capable of etrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch exceptions.
+- Implement at least one trigger capable of itrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch interrupts.
+- The minimum trigger requirements must be met for action=0 and for action=1
+(possibly by the same triggers)
+  * Rationale: The intent is to have full support for external debug and full
+    support for self-hosted debug (though not necessarily at the same time).
+    This can be provided via the same set of triggers or separate sets of
+    triggers. External debug support for icount is unnecessary due to dcsr.step
+    and is therefore called out separately.
+- For implementations with multiple cores, support for at least one halt group
+and one resume group (in addition to group 0)
+  * Rationale: Allows stopping all harts (approximately) simultaneously which
+    is useful for debugging MP software
+- dcsr.stepie must support the 0 setting.  It is optional to support the 1
+setting.
+  * Rationale: It is not generally useful to step into interrupt
handlers.
+- dcsr.stopcount and dcsr.stoptime must be supported and the reset value of
+each must be 1
+  * Rationale: The architecture has strict requirements on minstret which may
+    be perturbed by an external debugger in a way that's visible to software.
+    The default should allow code that's sensitive to these requirements to be
+    debugged.
+
==== Memory Map
* Virtual Memory
* sv39/sv48/sv57
@@ -166,6 +253,20 @@ Spec should be ratified*]
file for each HART ?? (*TBD*)
- IOMMU with support for memory resident interrupt files ?? (*TBD*)
+==== Debug
+The Linux server platform requirements are all of the above plus:
+
+- Implement at least six mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: Other architectures have found that 4 breakpoints are
+    insufficient in more capable systems and recommend 6.
+- If system bus access is implemented then accesses must be coherent with
+respect to all harts connected to the DM
+  * Rationale: Debuggers must be able to view memory coherently
+
+
==== Boot and Runtime Requirements
=====  Firmware
The boot and system firmware for the RV64I server platforms required to be
--
2.25.1






Re: [PATCH v1 2/2] Platform debug requirements

Paul Donahue
 

Chapter 5 of the debug spec covers triggers that are used for self-hosted and/or external debug.  The action field of each trigger must be programmed according to table 5.1 to either trap or halt.


This is also true in the ratified 0.13.2 debug spec from 2019, though it was easy to be misled by that document's title ("RISC-V External Debug Support").


Thanks,

-Paul

On Mon, May 31, 2021 at 6:10 PM Jonathan Behrens <behrensj@...> wrote:
Could you link to documentation about self-hosted debug? The only proposed specifications I've seen are for external debug support.

Thanks,
Jonathan

On Fri, May 28, 2021 at 7:47 PM Paul Donahue via lists.riscv.org <pdonahue=ventanamicro.com@...> wrote:
Signed-off-by: Paul Donahue pdonahue@...
---
riscv-platform-spec.adoc | 101 +++++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 98783c8..e5a1236 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -73,6 +73,93 @@ include::profiles.adoc[]
** ASID
 * Debug
+==== Debug
+The Linux base platform requirements are -
+
+- Implement resethaltreq
+  * Rationale: Debugging immediately out of reset is a useful debug tool and
+    is required by item 5 in chapter 3. The resethaltreq mechanism provides a
+    standard way to do this.
+- Implement the program buffer
+  * Rationale: The program buffer is easier for most implementations than
+    abstract access.
+  * Rationale: Debuggers need to be able to insert ebreak instructions into
+    memory and make sure that the ebreak is visible to subsequent instruction
+    fetches.  Abstract access has no support for fence.i (or similar
+    mechanisms).
+- abstractcs.relaxedpriv must be 0
+  * Rationale: Doing otherwise is a potential security problem.
+- abstractauto must be implemented
+  * Rationale: autoexecprogbuf allows faster instruction-stuffing
+  * Rationale: autoexecdata allows fast read/write of a region of memory
+- dcsr.mprven must be tied to 1
+  * Rationale: Emulating two-stage table walks and PMP checks and endianness
+    swapping is a heavy burden on the debugger.
+- In textra, sselect must support the value 0 and either value 1 or 2 (or
+both).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular user context and a way to ignore user context.
+- If textra.sselect=1 is supported, the number of implemented bits of svalue
+must be at least the number of implemented bits of scontext.
+  * Rationale: This allows matching on every possible scontext.
+- If textra.sselect=2 is supported, the number of implemented bits of svalue
+must be at least ASIDLEN.
+  * Rationale: This allows matching on every possible ASID.
+- In textra, mhselect must support the value 0.  If the H extension is
+supported then mhselect must also support either values 1 and 5 or values 2
+and 6 (or all four).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular guest context and a way to ignore guest context.
+- If textra.mhselect=1,5 are supported and if H is the number of implemented
+bits of hcontext then, unless all bits of mhvalue are implemented, at least
+H-1 bits of mhvalue must be implemented.
+  * Rationale: This allows matching on every possible hcontext (up to the limit
+    of the field width).  It is H-1 bits instead of H because mhselect[2]
+    provides one bit.
+- If textra.mhselect=2,6 are supported, the number of implemented bits of
+mhvalue must be at least VMIDLEN-1.
+  * Rationale: This allows matching on every possible VMID.  It is VMIDLEN-1
+    instead of VMIDLEN because mhselect[2] provides one bit.
+- Implement at least four mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: The debugger needs breakpoints and 4 is a sufficient baseline.
+- Implement at least four mcontrol6 triggers that can support matching on load
+ and store addresses (select=0, match=0, and all combinations of load/store)
+ with timing=0 and full support for mode filtering (vs, vu, m, s, u) for all
+ supported modes and support for textra as above.
+  * Rationale: The debugger needs watchpoints and 4 is a sufficient baseline.
+- Implement at least one trigger capable of icount and support for textra as
+above.
+  * Rationale: Self-hosted single step needs this
+- Implement at least one trigger capable of etrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch exceptions.
+- Implement at least one trigger capable of itrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch interrupts.
+- The minimum trigger requirements must be met for action=0 and for action=1
+(possibly by the same triggers)
+  * Rationale: The intent is to have full support for external debug and full
+    support for self-hosted debug (though not necessarily at the same time).
+    This can be provided via the same set of triggers or separate sets of
+    triggers. External debug support for icount is unnecessary due to dcsr.step
+    and is therefore called out separately.
+- For implementations with multiple cores, support for at least one halt group
+and one resume group (in addition to group 0)
+  * Rationale: Allows stopping all harts (approximately) simultaneously which
+    is useful for debugging MP software
+- dcsr.stepie must support the 0 setting.  It is optional to support the 1
+setting.
+  * Rationale: It is not generally useful to step into interrupt
handlers.
+- dcsr.stopcount and dcsr.stoptime must be supported and the reset value of
+each must be 1
+  * Rationale: The architecture has strict requirements on minstret which may
+    be perturbed by an external debugger in a way that's visible to software.
+    The default should allow code that's sensitive to these requirements to be
+    debugged.
+
==== Memory Map
* Virtual Memory
* sv39/sv48/sv57
@@ -166,6 +253,20 @@ Spec should be ratified*]
file for each HART ?? (*TBD*)
- IOMMU with support for memory resident interrupt files ?? (*TBD*)
+==== Debug
+The Linux server platform requirements are all of the above plus:
+
+- Implement at least six mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: Other architectures have found that 4 breakpoints are
+    insufficient in more capable systems and recommend 6.
+- If system bus access is implemented then accesses must be coherent with
+respect to all harts connected to the DM
+  * Rationale: Debuggers must be able to view memory coherently
+
+
==== Boot and Runtime Requirements
=====  Firmware
The boot and system firmware for the RV64I server platforms required to be
--
2.25.1






Re: [PATCH v1 2/2] Platform debug requirements

Jonathan Behrens <behrensj@...>
 

Could you link to documentation about self-hosted debug? The only proposed specifications I've seen are for external debug support.

Thanks,
Jonathan


On Fri, May 28, 2021 at 7:47 PM Paul Donahue via lists.riscv.org <pdonahue=ventanamicro.com@...> wrote:
Signed-off-by: Paul Donahue pdonahue@...
---
riscv-platform-spec.adoc | 101 +++++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 98783c8..e5a1236 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -73,6 +73,93 @@ include::profiles.adoc[]
** ASID
 * Debug
+==== Debug
+The Linux base platform requirements are -
+
+- Implement resethaltreq
+  * Rationale: Debugging immediately out of reset is a useful debug tool and
+    is required by item 5 in chapter 3. The resethaltreq mechanism provides a
+    standard way to do this.
+- Implement the program buffer
+  * Rationale: The program buffer is easier for most implementations than
+    abstract access.
+  * Rationale: Debuggers need to be able to insert ebreak instructions into
+    memory and make sure that the ebreak is visible to subsequent instruction
+    fetches.  Abstract access has no support for fence.i (or similar
+    mechanisms).
+- abstractcs.relaxedpriv must be 0
+  * Rationale: Doing otherwise is a potential security problem.
+- abstractauto must be implemented
+  * Rationale: autoexecprogbuf allows faster instruction-stuffing
+  * Rationale: autoexecdata allows fast read/write of a region of memory
+- dcsr.mprven must be tied to 1
+  * Rationale: Emulating two-stage table walks and PMP checks and endianness
+    swapping is a heavy burden on the debugger.
+- In textra, sselect must support the value 0 and either value 1 or 2 (or
+both).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular user context and a way to ignore user context.
+- If textra.sselect=1 is supported, the number of implemented bits of svalue
+must be at least the number of implemented bits of scontext.
+  * Rationale: This allows matching on every possible scontext.
+- If textra.sselect=2 is supported, the number of implemented bits of svalue
+must be at least ASIDLEN.
+  * Rationale: This allows matching on every possible ASID.
+- In textra, mhselect must support the value 0.  If the H extension is
+supported then mhselect must also support either values 1 and 5 or values 2
+and 6 (or all four).
+  * Rationale: There must be some way to limit triggers to only match in a
+    particular guest context and a way to ignore guest context.
+- If textra.mhselect=1,5 are supported and if H is the number of implemented
+bits of hcontext then, unless all bits of mhvalue are implemented, at least
+H-1 bits of mhvalue must be implemented.
+  * Rationale: This allows matching on every possible hcontext (up to the limit
+    of the field width).  It is H-1 bits instead of H because mhselect[2]
+    provides one bit.
+- If textra.mhselect=2,6 are supported, the number of implemented bits of
+mhvalue must be at least VMIDLEN-1.
+  * Rationale: This allows matching on every possible VMID.  It is VMIDLEN-1
+    instead of VMIDLEN because mhselect[2] provides one bit.
+- Implement at least four mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: The debugger needs breakpoints and 4 is a sufficient baseline.
+- Implement at least four mcontrol6 triggers that can support matching on load
+ and store addresses (select=0, match=0, and all combinations of load/store)
+ with timing=0 and full support for mode filtering (vs, vu, m, s, u) for all
+ supported modes and support for textra as above.
+  * Rationale: The debugger needs watchpoints and 4 is a sufficient baseline.
+- Implement at least one trigger capable of icount and support for textra as
+above.
+  * Rationale: Self-hosted single step needs this
+- Implement at least one trigger capable of etrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch exceptions.
+- Implement at least one trigger capable of itrigger and support for textra as
+above.
+  * Rationale: Debuggers need to be able to catch interrupts.
+- The minimum trigger requirements must be met for action=0 and for action=1
+(possibly by the same triggers)
+  * Rationale: The intent is to have full support for external debug and full
+    support for self-hosted debug (though not necessarily at the same time).
+    This can be provided via the same set of triggers or separate sets of
+    triggers. External debug support for icount is unnecessary due to dcsr.step
+    and is therefore called out separately.
+- For implementations with multiple cores, support for at least one halt group
+and one resume group (in addition to group 0)
+  * Rationale: Allows stopping all harts (approximately) simultaneously which
+    is useful for debugging MP software
+- dcsr.stepie must support the 0 setting.  It is optional to support the 1
+setting.
+  * Rationale: It is not generally useful to step into interrupt
handlers.
+- dcsr.stopcount and dcsr.stoptime must be supported and the reset value of
+each must be 1
+  * Rationale: The architecture has strict requirements on minstret which may
+    be perturbed by an external debugger in a way that's visible to software.
+    The default should allow code that's sensitive to these requirements to be
+    debugged.
+
==== Memory Map
* Virtual Memory
* sv39/sv48/sv57
@@ -166,6 +253,20 @@ Spec should be ratified*]
file for each HART ?? (*TBD*)
- IOMMU with support for memory resident interrupt files ?? (*TBD*)
+==== Debug
+The Linux server platform requirements are all of the above plus:
+
+- Implement at least six mcontrol6 triggers that can support matching on PC
+(select=0, execute=1, match=0) with timing=0 and full support for mode
+filtering (vs, vu, m, s, u) for all supported modes and support for textra as
+above.
+  * Rationale: Other architectures have found that 4 breakpoints are
+    insufficient in more capable systems and recommend 6.
+- If system bus access is implemented then accesses must be coherent with
+respect to all harts connected to the DM
+  * Rationale: Debuggers must be able to view memory coherently
+
+
==== Boot and Runtime Requirements
=====  Firmware
The boot and system firmware for the RV64I server platforms required to be
--
2.25.1






Re: [tech-aia] RISC-V ACLINT specification is now hosted on RISC-V GitHub

Greg Favor
 

On Sun, May 30, 2021 at 5:15 AM Anup Patel <Anup.Patel@...> wrote:

Regarding second question/suggestion, I agree we should explicitly state how to disable a pending interrupt.


More properly, this should be a non-normative sentence added to the Machine Timer Registers section in the Priv spec - that notes that setting mtimecmp to the max value effectively disables generation of a timer interrupt.

Greg

 


Re: [tech-aia] RISC-V ACLINT specification is now hosted on RISC-V GitHub

Jonathan Behrens <behrensj@...>
 

I would suggest just removing the mtime synchronization code sample, and replacing it with a reminder that clocks have to be synchronized. There's nothing all that subtle about the code so I don't think folks would have a huge issue figuring it out themselves if they happen to be in the super specific case where it is needed.

Jonathan

On Sun, May 30, 2021 at 8:26 AM Anup Patel <Anup.Patel@...> wrote:

The mtime synchronization code sample is only for reference. A platform can have it’s own way (hardware/software) of synchronizing MTIME registers. The code sample shows a simple approach to synchronize MTIME registers assuming MTIMER devices are running at same frequency and clock drift is taken care in hardware.

 

I think we need some text to explicitly state expectations around clock drift handling.

 

Regards,

Anup

 

From: Jonathan Behrens <behrensj@...>
Sent: 29 May 2021 04:37
To: jscheid@...
Cc: Anup Patel <Anup.Patel@...>; tech-aia@...; tech-unixplatformspec@...; Atish Patra <Atish.Patra@...>; Greg Favor <gfavor@...>; Alistair Francis <Alistair.Francis@...>; Andrew Waterman <andrew@...>; John Hauser <jh.riscv@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [tech-aia] RISC-V ACLINT specification is now hosted on RISC-V GitHub

 

If you're interested in reading about pure software approaches to clock synchronization, I recommend looking at https://www.usenix.org/conference/nsdi18/presentation/geng. They focus on synchronizing clocks between different servers in the same datacenter which if anything is a harder problem.

 

The main issue I see with the mtime synchronization code sample in the linked spec is that it doesn't do anything to address clock drift. The linked paper claims that CPU clocks can drift by 10 microseconds/second or more, so unless you have dedicated hardware to prevent drift (in which case why are you bothering with software at all?) you are very rapidly going to observe time readings that differ by more than one tick even if they start out consistent.

 

Jonathan

 

On Fri, May 28, 2021 at 5:44 PM Josh Scheid via lists.riscv.org <jscheid=ventanamicro.com@...> wrote:

One other issue with the "mtime" synchronization by SW approach is that this effectively places an upper limit on the achievable timer unit resolution.  It'd be some equation based on the ordered access latency of the reference and target resources, perhaps.

 

Has this been explicitly considered?  What is the expected upper limit and where should the platform be moving towards in the future?  Would further work be needed to enable >=1GHz timer resolution?

 

-Josh


Re: [tech-aia] RISC-V ACLINT specification is now hosted on RISC-V GitHub

Anup Patel
 

The mtime synchronization code sample is only for reference. A platform can have it’s own way (hardware/software) of synchronizing MTIME registers. The code sample shows a simple approach to synchronize MTIME registers assuming MTIMER devices are running at same frequency and clock drift is taken care in hardware.

 

I think we need some text to explicitly state expectations around clock drift handling.

 

Regards,

Anup

 

From: Jonathan Behrens <behrensj@...>
Sent: 29 May 2021 04:37
To: jscheid@...
Cc: Anup Patel <Anup.Patel@...>; tech-aia@...; tech-unixplatformspec@...; Atish Patra <Atish.Patra@...>; Greg Favor <gfavor@...>; Alistair Francis <Alistair.Francis@...>; Andrew Waterman <andrew@...>; John Hauser <jh.riscv@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [tech-aia] RISC-V ACLINT specification is now hosted on RISC-V GitHub

 

If you're interested in reading about pure software approaches to clock synchronization, I recommend looking at https://www.usenix.org/conference/nsdi18/presentation/geng. They focus on synchronizing clocks between different servers in the same datacenter which if anything is a harder problem.

 

The main issue I see with the mtime synchronization code sample in the linked spec is that it doesn't do anything to address clock drift. The linked paper claims that CPU clocks can drift by 10 microseconds/second or more, so unless you have dedicated hardware to prevent drift (in which case why are you bothering with software at all?) you are very rapidly going to observe time readings that differ by more than one tick even if they start out consistent.

 

Jonathan

 

On Fri, May 28, 2021 at 5:44 PM Josh Scheid via lists.riscv.org <jscheid=ventanamicro.com@...> wrote:

One other issue with the "mtime" synchronization by SW approach is that this effectively places an upper limit on the achievable timer unit resolution.  It'd be some equation based on the ordered access latency of the reference and target resources, perhaps.

 

Has this been explicitly considered?  What is the expected upper limit and where should the platform be moving towards in the future?  Would further work be needed to enable >=1GHz timer resolution?

 

-Josh

821 - 840 of 1836