Re: [PATCH v3 1/6] Additional requirements for H-extension
On 8/6/21 8:18 AM, Anup Patel wrote:
To have a meaningful H-extension support, both OS/A-base andPlease, add period at end of sentence.
According to the privileged architecture specification:
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
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** 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.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 withThis is already covered by the general requirement above.
+*** Upon virtual instruction fault taken into S-mode, the `stval` CSR mustThis is already covered by the general requirement above.
+*** Upon guest page faults taken into HS-mode, the `htval` CSR must provideThat 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."