Re: [PATCH v3 1/6] Additional requirements for H-extension


Heinrich Schuchardt
 

On 8/6/21 8:18 AM, Anup Patel wrote:
To have a meaningful H-extension support, both OS/A-base and
OS/A-server platforms must comply with additional requirements
for H-extension.
Signed-off-by: Anup Patel <anup.patel@...>
---
riscv-platform-spec.adoc | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
index a24281f..9d9fbc7 100644
--- a/riscv-platform-spec.adoc
+++ b/riscv-platform-spec.adoc
@@ -118,8 +118,22 @@ The M platform has the following extensions:
value stored to that location. (That is, the fetched instruction is not an
unpredictable value, nor is it a hybrid of the bytes of the old and new
values.)
+** Upon illegal instruction fault taken into M-mode, the `mtval` CSR must
+ provide faulting instruction bits
Please, add period at end of sentence.

According to the privileged architecture specification:

<cite>
If this feature is provided, after an illegal instruction trap, mtval will contain the shortest of:
• the actual faulting instruction
• the first ILEN bits of the faulting instruction
• the first XLEN bits of the faulting instruction
</cite>

We should not redefine what is already another specification.

I sugest the following:

** When a trap is taken into M-mode exception-specific information must be written to the mtval register.

+** Upon illegal instruction fault taken into S-mode, the `stval` CSR must
+ provide faulting instruction bits
** When a trap is taken into S-mode exception-specific information must be written to the stval register.

** Platforms is allowed to operate only in little-endian mode i.e.
implementations must hardwire the mstatus.MBE field to 0.
This mixes plural (platforms) and singular (is).

"operate in little-endian mode" is distinct to "hardwire mstatus.MBE field to 0". Why should a platform not support big endian mode when it is running in a context that is beyond the scope of this specification?

I think it should be enough to require that the mstatus.MBE field is set to 0 and not hardwired.

+** If the H-extension is implemented then the OS-A platform must comply with
+ the following additional requirements:
+*** Upon virtual instruction fault taken into M-mode, the `mtval` CSR must
+ provide faulting instruction bits
This is already covered by the general requirement above.

+*** Upon virtual instruction fault taken into S-mode, the `stval` CSR must
+ provide faulting instruction bits
This is already covered by the general requirement above.

+*** Upon guest page faults taken into HS-mode, the `htval` CSR must provide
+ the guest physical address right shifted by 2.
+*** Upon guest page faults taken into M-mode, the `mtval2` CSR must provide
+ the guest physical address right shifted by 2.
That and how the trap information shall be written to htval and mtval2 is already defined in the H-extension. We should not redefine it here. How about:

"The platform must implement writing trap information to htval and mtval2 in accordance with the hypervisor extension."


Best regards

Heinrich

[sidebar]
--
@@ -448,9 +462,13 @@ base with the additional requirements as below.
==== Architecture
The platforms which conform to server extension are required to implement +
-- RV64 support
-- RISC-V H ISA extension
-- VMID support
+* RV64 support
+* RISC-V ISA H-extension with following additional requirements:
+** VMID support
+** Upon load/store/AMO exceptions taken into HS-mode, the `htinst` CSR must
+ provide transformed standard instruction.
+** Upon load/store/AMO exceptions taken into M-mode, the `mtinst` CSR must
+ provide transformed standard instruction.
==== PMU

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