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
+** Upon illegal instruction fault taken into S-mode, the `stval` CSR must
+ provide faulting instruction bits
** Platforms is allowed to operate only in little-endian mode i.e.
implementations must hardwire the mstatus.MBE field to 0.
+** 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
+*** Upon virtual instruction fault taken into S-mode, the `stval` CSR must
+ provide faulting instruction bits
+*** 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.

[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

--
2.25.1

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

On 06/08/21, 1:14 PM, "tech-unixplatformspec@... on behalf of Heinrich Schuchardt" <tech-unixplatformspec@... on behalf of heinrich.schuchardt@...> wrote:

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.

[Anup] While I agree that we should not re-define exact details written
into the mtval CSR but we have to explicitly state which exception we
are talking about because the optionality is only for certain exceptions.
[Anup] Ideally all these optionality in RISC-V privileged spec should be
mandated by the RISC-V profiles spec. Until that happens, the ISA
requirements over here only complements the missing details in the
RISC-V profiles spec.

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

[Anup] There was quite a bit of discussion in separate email thread where
it was concluded to include the statement way it is not. The grammar error
should be definitely fixed.
[Anup] For first release of RISC-V platform spec, only little-endian mode
is allowed. Future release of RISC-V platform spec will certainly allow
big-endian mode but we need software support first (i.e. big-endian
kernel and distro/rootfs).

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

[Anup] Like mentioned above, we need to at least state the exception/trap which
has the optionality.

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

[Anup] Okay, let me re-word this as well.

Regards,
Anup


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

On 8/7/21 8:16 AM, Greg Favor wrote:

On Fri, Aug 6, 2021 at 10:41 PM Anup Patel <anup.patel@... <mailto:anup.patel@...>> wrote:
    I think it should be enough to require that the mstatus.MBE
field is set
    to 0 and not hardwired.
[Anup] There was quite a bit of discussion in separate email thread
where
it was concluded to include the statement way it is. The grammar

Hello Anup,

do you have a link to the thread?

Best regards

Heinrich

error
should be definitely fixed.
It now strikes me that requiring 2022-compliant platforms to hardware MBE to zero will result in future platform specs - that allow MBE to be writeable - to not be compatible with the 2022 platform spec.  This I believe goes against the intent that a platform that is compliant with a given year's platform spec should inherently also be compatible with all earlier years of the platform spec (modulo any DEPRECATED features).
If this intent is correct, then it seems like we must not require MBE to be hardwired and instead need to say that it must be set to 0.
Greg
[Anup] For first release of RISC-V platform spec, only little-endian
mode
is allowed. Future release of RISC-V platform spec will certainly allow
big-endian mode but we need software support first (i.e. big-endian
kernel and distro/rootfs).