Date   

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


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

Anup Patel
 

Hi Neel,

 

You first question is already answered by Greg.

 

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

 

Regards,

Anup

 

From: tech-unixplatformspec@... <tech-unixplatformspec@...> On Behalf Of Neel Gala
Sent: 27 May 2021 23:05
To: Anup Patel <Anup.Patel@...>
Cc: Josh Scheid <jscheid@...>; 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

 

Hi,

 

Minor comments:

 

Should there be a mention that the user level "time" csr (0xC01) which is used by the rdtime pseudo-instruction will enable a read-only peek into the mtime register? Would this require change in Table-1 privilege mode accesses? as well?

 

Should there also be a suggestion/recommendation on how to disable a pending interrupt (typically by writing to mtimecmp)?

 

On Thu, May 27, 2021 at 10:45 AM Anup Patel <anup.patel@...> wrote:

Hi Josh,

 

I have created a GitHub PR addressing your comments. Please check if you are okay with this.

https://github.com/riscv/riscv-aclint/pull/2

 

Regards,

Anup

 

From: tech-aia@... <tech-aia@...> On Behalf Of Josh Scheid
Sent: 27 May 2021 01:58
To: Anup Patel <Anup.Patel@...>
Cc: tech-aia@...; tech-unixplatformspec@...; Atish Patra <Atish.Patra@...>; Greg Favor <gfavor@...>; Alistair Francis <Alistair.Francis@...>; Andrew Waterman <andrew@...>; John Hauser <jh.riscv@...>
Subject: Re: [tech-aia] RISC-V ACLINT specification is now hosted on RISC-V GitHub

 

Thanks for writing this up, Anup.

 

In https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc#24-synchronizing-multiple-mtimer-devices, the SW algorithm should include verifying the reference-target delta, retrying if the delta is out of bounds, and / or reporting failure to verify the synchronization is in bounds.

 

-Josh

 

 

On Tue, May 25, 2021 at 10:18 PM Anup Patel <anup.patel@...> wrote:

Hi All,

The RISC-V ACLINT specification is now hosted on RISC-V GitHub page:
https://github.com/riscv/riscv-aclint
https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc

Please review this at your end send feedback on AIA/Platform mailing
lists.

The RISC-V ACLINT specification is intended to be small and backward
compatible to the SiFive CLINT specification which makes existing
RISC-V platforms compliant with the RISC-V ACLINT specification.

Overall, from platforms specification perspective it complements
the RISC-V AIA specification by providing IPI and Timer functionality.

A complete functional implementation is available for QEMU RISC-V
along with OpenSBI and Linux RISC-V changes. Please refer, the
riscv_aclint_v1 branch in following repos:
https://github.com/avpatel/qemu.git
https://github.com/avpatel/opensbi.git
https://github.com/avpatel/linux.git

To enable ACLINT emulation on QEMU, use "-M virt,aclint=on"
instead of just "-M virt" in your QEMU command line. For now,
QEMU supports ACLINT only for virt machine.

Regards,
Anup






 

--

Neel Gala

 


[RESEND PATCH v6 2/2] contributors: Add Abner as contributor

Abner Chang
 

From: Abner Chang <abner.chang@...>

Signed-off-by: Abner Chang <renba.chang@...>
---
contributors.adoc | 1 +
1 file changed, 1 insertion(+)

diff --git a/contributors.adoc b/contributors.adoc
index 91cc46e..7bc731e 100644
--- a/contributors.adoc
+++ b/contributors.adoc
@@ -29,6 +29,7 @@ Alistair Francis,
Sunil V L,
Rahul Pathak,
Mayuresh Chitale
+Abner Chang

If you have contributed and are not listed, do let us know. We haven't
forgotten you, but we may have forgotten to edit this list.
--
2.19.0.windows.1


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

Abner Chang
 

From: Abner Chang <abner.chang@...>

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

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

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

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

On v3 commit,
- Address review comments.

On v2 commit,
- CLINT is not deprecated.

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

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

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

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

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 4418788..d2925ac 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -79,9 +79,24 @@ include::profiles.adoc[]
* Start Address

==== Interrupt Controller
-* AIA
-* PLIC + CLINT
-* Interrupt Assignments
+===== AIA[[AIA]]
+===== PLIC[DEPRECATED][[PLIC]]
+The Platform Level Interrupt Controller (PLIC) provides facilities to route
+the non-local interrupts to the external interrupt of a hart context
+with a given privilege mode in a given hart. The number of non-local interrupt
+sources supported by PLIC and how does each of them connect to the hart
+context is PLIC core implementation-specific. +
+(Refer to https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc[RISC-V PLIC Specification]
+for the implementation reference of PLIC operation parameters)
+
+===== ACLINT
+Linux-2020 platform requires the Advanced Core Local Interruptor (ACLINT,
+refer to
+https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc[RISC-V ACLINT Specification])
+to provide facilities to route inter-processor interrupt and Machine mode timer
+interrupt to each RISC-V processor hart.
+
+===== Interrupt Assignments

==== System Peripherals
* UART/Serial Console
@@ -406,8 +421,8 @@ Any RISC-V system that uses at least RV32/64G can meet the Embedded-2022
specification.

==== Interrupt Controller
-Embedded systems are recommended to use a spec compliant
-https://github.com/riscv/riscv-plic-spec[PLIC], a spec compliant
+Embedded systems are recommended to use a spec compliant <<PLIC,PLIC>>,
+a spec compliant
https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc[CLIC]
or both a CLIC and and PLIC.

--
2.19.0.windows.1


[PATCH v6 2/2] contributors: Add Abner as contributor

Abner Chang
 

From: Abner Chang <renba.chang@...>

Signed-off-by: Abner Chang <renba.chang@...>
---
contributors.adoc | 1 +
1 file changed, 1 insertion(+)

diff --git a/contributors.adoc b/contributors.adoc
index 3d19411..73b99c4 100644
--- a/contributors.adoc
+++ b/contributors.adoc
@@ -27,6 +27,7 @@ Andrew Waterman,
Kumar Sankaran,
Alistair Francis,
Sunil V L.
+Abner Chang

If you have contributed and are not listed, do let us know. We haven't
forgotten you, but we may have forgotten to edit this list.
--
2.19.0.windows.1


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

Abner Chang
 

From: Abner Chang <renba.chang@...>

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

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

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

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

On v3 commit,
- Address review comments.

On v2 commit,
- CLINT is not deprecated.

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

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

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

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

diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index 160c74a..c0ee75d 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -49,9 +49,24 @@ include::profiles.adoc[]
* Start Address

==== Interrupt Controller
-* AIA
-* PLIC + CLINT
-* Interrupt Assignments
+===== AIA[[AIA]]
+===== PLIC[DEPRECATED][[PLIC]]
+The Platform Level Interrupt Controller (PLIC) provides facilities to route
+the non-local interrupts to the external interrupt of a hart context
+with a given privilege mode in a given hart. The number of non-local interrupt
+sources supported by PLIC and how does each of them connect to the hart
+context is PLIC core implementation-specific. +
+(Refer to https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc[RISC-V PLIC Specification]
+for the implementation reference of PLIC operation parameters)
+
+===== ACLINT
+Linux-2020 platform requires the Advanced Core Local Interruptor (ACLINT,
+refer to
+https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc[RISC-V ACLINT Specification])
+to provide facilities to route inter-processor interrupt and Machine mode timer
+interrupt to each RISC-V processor hart.
+
+===== Interrupt Assignments

==== System Peripherals
* UART/Serial Console
@@ -289,8 +304,8 @@ Any RISC-V system that uses at least RV32/64G can meet the Embedded-2022
specification.

==== Interrupt Controller
-Embedded systems are recommended to use a spec compliant
-https://github.com/riscv/riscv-plic-spec[PLIC], a spec compliant
+Embedded systems are recommended to use a spec compliant <<PLIC,PLIC>>,
+a spec compliant
https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc[CLIC]
or both a CLIC and and PLIC.

--
2.19.0.windows.1


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

Josh Scheid
 

On Fri, May 28, 2021 at 4:01 PM Greg Favor <gfavor@...> wrote:

More recent ARM SBSA requires a 1 GHz counter resolution, but does not place any requirement on the actual measurable 'time" resolution (i.e. a minimum update frequency).  So one can have 1 ns resolution in the 'time' counter value, but the actual measurable granularity could be just 1 us.

Upping the standard counter resolution seems of little or secondary value.  It's the actual maximum granularity or resolution of measurable time that matters.  Which no one in RISC-V (or ARM SBSA) seems willing or wanting to require actual 1 ns resolution to time measurements.


Right, I'm attempting to refer to the effective timer resolution as opposed to the apparent timer unit period.

I'm just making explicit what this relaxation implies about capabilities in real systems.  Platforms should separately consider whether or not to allow this relaxation based on a threshold for effective timer resolution which must be available on compliant implementations of that platform.

-Josh


[PATCH v1 2/2] Platform debug requirements

Paul Donahue
 

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


[PATCH v1 1/2] Updating changelog

Paul Donahue
 

Updating changelog

Signed-off-by: Paul Donahue pdonahue@...
---
changelog.adoc | 2 ++
1 file changed, 2 insertions(+)

diff --git a/changelog.adoc b/changelog.adoc
index 7ec1b1f..cc69971 100644
--- a/changelog.adoc
+++ b/changelog.adoc
@@ -8,6 +8,8 @@
## Change Log
### version 0.2-rc0
+* 2021-05-20:
+** Platform requirements for Debug
* 2021-05-19:
** Base boot and runtime requirements - Initial commit
* 2021-04-08:
--=20
2.25.1


[PATCH v1 0/2] Updating contributors

Paul Donahue
 

Adding myself to list of contributors.

Signed-off-by: Paul Donahue pdonahue@...
---
contributors.adoc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contributors.adoc b/contributors.adoc
index 08b16f0..13dcf59 100644
--- a/contributors.adoc
+++ b/contributors.adoc
@@ -27,7 +27,8 @@ Andrew Waterman,
Kumar Sankaran,
Alistair Francis,
Sunil V L,
-Rahul Pathak
+Rahul Pathak,
+Paul Donahue
If you have contributed and are not listed, do let us know. We haven't
forgotten you, but we may have forgotten to edit this list.
--
2.25.1

841 - 860 of 1847