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