[PATCH] Add direct memory access synchronize extension


Allen Baum
 

Arch-test should be involved also.
It is (more than) a bit  complicated because CMOs are instructions that affect non-architectural bits of an implementation 
- so it's unclear what it even means to have an architectural test, much less how to write one.
The framework and tests are currently only handling deterministic archtecutral state and 
The definition of done has an architectural test component, and a proof of concept component.
The CMOs can only do the proof-of-concept part because of the above.

On Tue, Jun 15, 2021 at 4:33 PM David Kruckemyer <dkruckemyer@...> wrote:
Hi all,

My apologies as I just got wind of this discussion (I was unable to attend the last few CMO TG meetings due to travel). I think we should sync up on the CMO TG and SBI/platform efforts since there seems to be a bit of disconnect.

Regarding the CMO TG goals, we have intended to get a basic subset of operations into the profile/platform specifications for this year. The "phase 1" status is listed here:


Though honestly, a bit of this is out of date already, so expect some clarification in the coming days (just need to do some terminology cleanup).

Please do not hesitate to reach out to me with any questions (or to post questions to the the CMO TG mailing list: tech-cmo@... )

Cheers,
David


On Mon, Jun 7, 2021 at 2:35 AM Nick Kossifidis <mick@...> wrote:
Στις 2021-06-07 07:03, Anup Patel έγραψε:
>
> Let's have a simple SBI DMA sync extension in SBI v0.4 spec.
>
> The shared code pages between M-mode and S-mode will have it's own
> Challenges and we will have to define more stuff in SBI spec to support
> this (see above).
>

Totally agree with you, I just thought it'd be a good opportunity to
bring this up so that we can discuss it at some point, let's have
something that works and we can optimize it later on.

> It seems CMO extension might freeze sooner than we think (others can
> comment on this). If CMO extension is frozen by year end then we can
> trap-n-emulate CMO instructions instead of SBI DMA sync extension. If
> it does not freeze by year end then we will have to go ahead with
> SBI DMA sync extension as stop-gap solution.
>

The CMOs TG has a meeting today, I'll try and join and ask for updates
on this.






David Kruckemyer
 

FWIW, our (the CMO TG's) priorities are in order as follows:

- Zicbom (maintenance)
- Zicboz (zero)
- Zicbop (prefetch)

We happen to have provisional opcodes for both Zicbom and Zicboz (mostly since they occupy the same real estate).

The primary goal now is to take our overly general spec and distill it down into the three extensions and limit it to the Phase 1 material. Volunteers to help out with that would be greatly appreciated.... :)

Cheers,
David


On Tue, Jun 15, 2021 at 7:32 PM Anup Patel <Anup.Patel@...> wrote:

Hi Paul,

 

Everyone over here is well aware of the importance of fast-tracking basic CMO instructions and getting it frozen soon. The CMO group is also aware of their priorities so we should let them tackle this instead of proposing how they should work.

 

As mentioned quite a few time in this email thread, the SBI DMA sync is only a stop-gap solution (or backup plan) to tackle Linux RISC-V patch acceptance policy if we don’t get basic CMO instructions soon. We would certainly like to avoid SBI DMA sync extension if possible. In fact, we have not included SBI DMA sync extension in the recently frozen SBI v0.3-rc1 spec which will be released next month.

 

It is certainly possible to have basic CMO instructions frozen by 2021 year end. If this happens then we will discard SBI DMA sync proposal and emulate basic CMO instructions in OpenSBI for BeagleV and Allwinner D1 boards. In fact, Atish is still figuring out ways to avoid both SBI DMA sync and CMO instructions for at least BeagleV if that is possible.

 

Regards,

Anup

 

From: Paul Walmsley <paul.walmsley@...>
Sent: 16 June 2021 05:29
To: David Kruckemyer <dkruckemyer@...>; Nick Kossifidis <mick@...>
Cc: Anup Patel <Anup.Patel@...>; Atish Patra <Atish.Patra@...>; tech-unixplatformspec@...; Palmer Dabbelt <palmerdabbelt@...>; Palmer Dabbelt <palmer@...>; tech-cmo@...; John Ingalls <john.ingalls@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory access synchronize extension

 

It would be ideal if the CMO group could focus on fast-tracking the Cache Block Maintenance Operations for Phase 1 and get opcodes assigned, and this part of the specification frozen.  The maintenance operations are mandatory for non-CPU-cache-coherent peripheral DMA to work correctly; that's why these should be completed first.   As far as I can tell, prefetch and zeroing are strictly optimizations, so it would be best if these could be delayed to a Phase 2 -- which could be developed in parallel while Phase 1 goes through the opcode committee, etc. 

 

Then the SBI sync extension should be superfluous. It would be ideal if we could avoid having multiple mechanisms for the same operations.

 

For this to work, though, the CMO group needs to move on the block maintenance instructions quickly. 

 

 

- Paul

 

 

On 6/15/21 4:33 PM, David Kruckemyer wrote:

Hi all,

 

My apologies as I just got wind of this discussion (I was unable to attend the last few CMO TG meetings due to travel). I think we should sync up on the CMO TG and SBI/platform efforts since there seems to be a bit of disconnect.

 

Regarding the CMO TG goals, we have intended to get a basic subset of operations into the profile/platform specifications for this year. The "phase 1" status is listed here:

 

 

Though honestly, a bit of this is out of date already, so expect some clarification in the coming days (just need to do some terminology cleanup).

 

Please do not hesitate to reach out to me with any questions (or to post questions to the the CMO TG mailing list: tech-cmo@... )

 

Cheers,

David

 

 

On Mon, Jun 7, 2021 at 2:35 AM Nick Kossifidis <mick@...> wrote:

Στις 2021-06-07 07:03, Anup Patel έγραψε:
>
> Let's have a simple SBI DMA sync extension in SBI v0.4 spec.
>
> The shared code pages between M-mode and S-mode will have it's own
> Challenges and we will have to define more stuff in SBI spec to support
> this (see above).
>

Totally agree with you, I just thought it'd be a good opportunity to
bring this up so that we can discuss it at some point, let's have
something that works and we can optimize it later on.

> It seems CMO extension might freeze sooner than we think (others can
> comment on this). If CMO extension is frozen by year end then we can
> trap-n-emulate CMO instructions instead of SBI DMA sync extension. If
> it does not freeze by year end then we will have to go ahead with
> SBI DMA sync extension as stop-gap solution.
>

The CMOs TG has a meeting today, I'll try and join and ask for updates
on this.





Anup Patel
 

Hi Paul,

 

Everyone over here is well aware of the importance of fast-tracking basic CMO instructions and getting it frozen soon. The CMO group is also aware of their priorities so we should let them tackle this instead of proposing how they should work.

 

As mentioned quite a few time in this email thread, the SBI DMA sync is only a stop-gap solution (or backup plan) to tackle Linux RISC-V patch acceptance policy if we don’t get basic CMO instructions soon. We would certainly like to avoid SBI DMA sync extension if possible. In fact, we have not included SBI DMA sync extension in the recently frozen SBI v0.3-rc1 spec which will be released next month.

 

It is certainly possible to have basic CMO instructions frozen by 2021 year end. If this happens then we will discard SBI DMA sync proposal and emulate basic CMO instructions in OpenSBI for BeagleV and Allwinner D1 boards. In fact, Atish is still figuring out ways to avoid both SBI DMA sync and CMO instructions for at least BeagleV if that is possible.

 

Regards,

Anup

 

From: Paul Walmsley <paul.walmsley@...>
Sent: 16 June 2021 05:29
To: David Kruckemyer <dkruckemyer@...>; Nick Kossifidis <mick@...>
Cc: Anup Patel <Anup.Patel@...>; Atish Patra <Atish.Patra@...>; tech-unixplatformspec@...; Palmer Dabbelt <palmerdabbelt@...>; Palmer Dabbelt <palmer@...>; tech-cmo@...; John Ingalls <john.ingalls@...>
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory access synchronize extension

 

It would be ideal if the CMO group could focus on fast-tracking the Cache Block Maintenance Operations for Phase 1 and get opcodes assigned, and this part of the specification frozen.  The maintenance operations are mandatory for non-CPU-cache-coherent peripheral DMA to work correctly; that's why these should be completed first.   As far as I can tell, prefetch and zeroing are strictly optimizations, so it would be best if these could be delayed to a Phase 2 -- which could be developed in parallel while Phase 1 goes through the opcode committee, etc. 

 

Then the SBI sync extension should be superfluous. It would be ideal if we could avoid having multiple mechanisms for the same operations.

 

For this to work, though, the CMO group needs to move on the block maintenance instructions quickly. 

 

 

- Paul

 

 

On 6/15/21 4:33 PM, David Kruckemyer wrote:

Hi all,

 

My apologies as I just got wind of this discussion (I was unable to attend the last few CMO TG meetings due to travel). I think we should sync up on the CMO TG and SBI/platform efforts since there seems to be a bit of disconnect.

 

Regarding the CMO TG goals, we have intended to get a basic subset of operations into the profile/platform specifications for this year. The "phase 1" status is listed here:

 

 

Though honestly, a bit of this is out of date already, so expect some clarification in the coming days (just need to do some terminology cleanup).

 

Please do not hesitate to reach out to me with any questions (or to post questions to the the CMO TG mailing list: tech-cmo@... )

 

Cheers,

David

 

 

On Mon, Jun 7, 2021 at 2:35 AM Nick Kossifidis <mick@...> wrote:

Στις 2021-06-07 07:03, Anup Patel έγραψε:
>
> Let's have a simple SBI DMA sync extension in SBI v0.4 spec.
>
> The shared code pages between M-mode and S-mode will have it's own
> Challenges and we will have to define more stuff in SBI spec to support
> this (see above).
>

Totally agree with you, I just thought it'd be a good opportunity to
bring this up so that we can discuss it at some point, let's have
something that works and we can optimize it later on.

> It seems CMO extension might freeze sooner than we think (others can
> comment on this). If CMO extension is frozen by year end then we can
> trap-n-emulate CMO instructions instead of SBI DMA sync extension. If
> it does not freeze by year end then we will have to go ahead with
> SBI DMA sync extension as stop-gap solution.
>

The CMOs TG has a meeting today, I'll try and join and ask for updates
on this.





Paul Walmsley
 

It would be ideal if the CMO group could focus on fast-tracking the Cache Block Maintenance Operations for Phase 1 and get opcodes assigned, and this part of the specification frozen.  The maintenance operations are mandatory for non-CPU-cache-coherent peripheral DMA to work correctly; that's why these should be completed first.   As far as I can tell, prefetch and zeroing are strictly optimizations, so it would be best if these could be delayed to a Phase 2 -- which could be developed in parallel while Phase 1 goes through the opcode committee, etc. 


Then the SBI sync extension should be superfluous. It would be ideal if we could avoid having multiple mechanisms for the same operations.


For this to work, though, the CMO group needs to move on the block maintenance instructions quickly. 



- Paul



On 6/15/21 4:33 PM, David Kruckemyer wrote:

Hi all,

My apologies as I just got wind of this discussion (I was unable to attend the last few CMO TG meetings due to travel). I think we should sync up on the CMO TG and SBI/platform efforts since there seems to be a bit of disconnect.

Regarding the CMO TG goals, we have intended to get a basic subset of operations into the profile/platform specifications for this year. The "phase 1" status is listed here:


Though honestly, a bit of this is out of date already, so expect some clarification in the coming days (just need to do some terminology cleanup).

Please do not hesitate to reach out to me with any questions (or to post questions to the the CMO TG mailing list: tech-cmo@... )

Cheers,
David


On Mon, Jun 7, 2021 at 2:35 AM Nick Kossifidis <mick@...> wrote:
Στις 2021-06-07 07:03, Anup Patel έγραψε:
>
> Let's have a simple SBI DMA sync extension in SBI v0.4 spec.
>
> The shared code pages between M-mode and S-mode will have it's own
> Challenges and we will have to define more stuff in SBI spec to support
> this (see above).
>

Totally agree with you, I just thought it'd be a good opportunity to
bring this up so that we can discuss it at some point, let's have
something that works and we can optimize it later on.

> It seems CMO extension might freeze sooner than we think (others can
> comment on this). If CMO extension is frozen by year end then we can
> trap-n-emulate CMO instructions instead of SBI DMA sync extension. If
> it does not freeze by year end then we will have to go ahead with
> SBI DMA sync extension as stop-gap solution.
>

The CMOs TG has a meeting today, I'll try and join and ask for updates
on this.






David Kruckemyer
 

Hi all,

My apologies as I just got wind of this discussion (I was unable to attend the last few CMO TG meetings due to travel). I think we should sync up on the CMO TG and SBI/platform efforts since there seems to be a bit of disconnect.

Regarding the CMO TG goals, we have intended to get a basic subset of operations into the profile/platform specifications for this year. The "phase 1" status is listed here:


Though honestly, a bit of this is out of date already, so expect some clarification in the coming days (just need to do some terminology cleanup).

Please do not hesitate to reach out to me with any questions (or to post questions to the the CMO TG mailing list: tech-cmo@... )

Cheers,
David


On Mon, Jun 7, 2021 at 2:35 AM Nick Kossifidis <mick@...> wrote:
Στις 2021-06-07 07:03, Anup Patel έγραψε:
>
> Let's have a simple SBI DMA sync extension in SBI v0.4 spec.
>
> The shared code pages between M-mode and S-mode will have it's own
> Challenges and we will have to define more stuff in SBI spec to support
> this (see above).
>

Totally agree with you, I just thought it'd be a good opportunity to
bring this up so that we can discuss it at some point, let's have
something that works and we can optimize it later on.

> It seems CMO extension might freeze sooner than we think (others can
> comment on this). If CMO extension is frozen by year end then we can
> trap-n-emulate CMO instructions instead of SBI DMA sync extension. If
> it does not freeze by year end then we will have to go ahead with
> SBI DMA sync extension as stop-gap solution.
>

The CMOs TG has a meeting today, I'll try and join and ask for updates
on this.






Nick Kossifidis
 

Στις 2021-06-07 07:03, Anup Patel έγραψε:
Let's have a simple SBI DMA sync extension in SBI v0.4 spec.
The shared code pages between M-mode and S-mode will have it's own
Challenges and we will have to define more stuff in SBI spec to support
this (see above).
Totally agree with you, I just thought it'd be a good opportunity to bring this up so that we can discuss it at some point, let's have something that works and we can optimize it later on.

It seems CMO extension might freeze sooner than we think (others can
comment on this). If CMO extension is frozen by year end then we can
trap-n-emulate CMO instructions instead of SBI DMA sync extension. If
it does not freeze by year end then we will have to go ahead with
SBI DMA sync extension as stop-gap solution.
The CMOs TG has a meeting today, I'll try and join and ask for updates on this.


@guoren
 

Thx Anup,

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
+
+* 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





Reviewed-by: Guo Ren <guoren@...>

It's Okay for us, I'll follow that. May I write a new version of Linux
implementation with the framework, or you've begun preparing that
Linux patches?

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/


Anup Patel
 

-----Original Message-----
From: Nick Kossifidis <mick@...>
Sent: 05 June 2021 19:32
To: Atish Patra <Atish.Patra@...>
Cc: mick@...; Anup Patel <Anup.Patel@...>; tech-
unixplatformspec@...
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH] Add direct memory
access synchronize extension

Στις 2021-06-04 23:01, Atish Patra έγραψε:

The firmware code will still be executed while the priv mode is S-mode
right ?

Wouldn't that violate the priv spec ?
M-mode can share a code region with S-mode using PMP/ePMP and let S-
mode map that region as executable on its address space. With the current
PMP M-mode can define a region as RX for example and both M-mode and
S/U-mode will have RX permissions there, with ePMP M-mode can share a
code region with S/U-mode that can be RX for M-mode and just X for S/U-
mode. I'm obviously talking about small code snippets without any
dependencies and references to external symbols etc. A function that flushes
the cache for example can be written in such a way.
It's not that simple. Providing shared executable code pages from M-mode
to S-mode means:
1) We will have to define ABI for entry/exit of functions in this shared
2) Define a format of function table offset which M-mode can export to
S-mode in the shared code pages itself.


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.

That's what I am thinking. The only additional cost is just a "ecall
and mret".

IMO, there will be noticeable difference in performance in vDSO-like
interface where S-mode is trying to read something that M-mode
provides. Thus, the base function list are likely candidates [1].
However, the OS makes those SBI calls once during the boot. Thus, it
wouldn't benefit that much.
I was thinking that as part of the extension, we can have an SBI call that would
return the address/length of the shared code region (in physical memory) and
offsets for each function within that region. The OS will do the SBI call upon
registering that SBI extension and will just use the provided function pointers
to directly execute code from the shared region. If we are looking for a
scenario with a high rate of syncs (lots of packets per second) there will be a
noticeable performance difference between a function call and an SBI call, on
the other had on such scenarios I'd expect to use the coherent API instead of
the non-coherent one.
Let's have a simple SBI DMA sync extension in SBI v0.4 spec.

The shared code pages between M-mode and S-mode will have it's own
Challenges and we will have to define more stuff in SBI spec to support
this (see above).

It seems CMO extension might freeze sooner than we think (others can
comment on this). If CMO extension is frozen by year end then we can
trap-n-emulate CMO instructions instead of SBI DMA sync extension. If
it does not freeze by year end then we will have to go ahead with
SBI DMA sync extension as stop-gap solution.

Regards,
Anup


Nick Kossifidis
 

Στις 2021-06-04 23:01, Atish Patra έγραψε:
The firmware code will still be executed while the priv mode is S-mode
right ?
Wouldn't that violate the priv spec ?
M-mode can share a code region with S-mode using PMP/ePMP and let S-mode map that region as executable on its address space. With the current PMP M-mode can define a region as RX for example and both M-mode and S/U-mode will have RX permissions there, with ePMP M-mode can share a code region with S/U-mode that can be RX for M-mode and just X for S/U-mode. I'm obviously talking about small code snippets without any dependencies and references to external symbols etc. A function that flushes the cache for example can be written in such a way.

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.
That's what I am thinking. The only additional cost is just a "ecall
and mret".
IMO, there will be noticeable difference in performance in vDSO-like
interface where S-mode is trying to read something that M-mode
provides. Thus, the base function list are likely candidates [1].
However, the OS makes those SBI calls once during the boot. Thus, it
wouldn't benefit that much.
I was thinking that as part of the extension, we can have an SBI call that would return the address/length of the shared code region (in physical memory) and offsets for each function within that region. The OS will do the SBI call upon registering that SBI extension and will just use the provided function pointers to directly execute code from the shared region. If we are looking for a scenario with a high rate of syncs (lots of packets per second) there will be a noticeable performance difference between a function call and an SBI call, on the other had on such scenarios I'd expect to use the coherent API instead of the non-coherent one.


atishp@...
 

On Fri, 2021-06-04 at 12:31 +0300, Nick Kossifidis wrote:
Στις 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.
Agreed.

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). 
The firmware code will still be executed while the priv mode is S-mode
right ?

Wouldn't that violate the priv spec ?


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.

That's what I am thinking. The only additional cost is just a "ecall
and mret".

IMO, there will be noticeable difference in performance in vDSO-like
interface where S-mode is trying to read something that M-mode
provides. Thus, the base function list are likely candidates [1].
However, the OS makes those SBI calls once during the boot. Thus, it
wouldn't benefit that much.


[1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc#function-listing

Regards,
Nick
--
Regards,
Atish


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




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


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


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


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


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


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


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


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


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