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

Join tech-unixplatformspec@lists.riscv.org to automatically receive all group messages.