Access to seed CSR from VS/VU modes

[JamesKenneyImperas]

I'm trying to pin down exactly the proposed behaviour for accessing the seed CSR from VU/VS modes, as there seems to be a conflict between the text in the specification and what was said in issue #134. The specification says this:

The seed CSR is never directly available from virtual (VS or VU) modes. A read-write instruction causes a virtual instruction Exception (while a read-only instruction always causes an illegal instruction Exception.)

A comment in #134 said this:

When a trap occurs due to an attempt to access seed from VS or
VU mode, the specific exception should depend on the type of access
and the value of the sseed bit of mseccfg. If the attempted access
is read-only (never allowed), an illegal instruction exception must be
raised. Else, following the rules for virtual instruction exceptions
laid down by the hypervisor extension, if mseccfg.sseed = 1, a virtual
instruction exception should be raised, and if mseccfg.sseed = 0, an
illegal instruction exception should be raised.

These don't seem to be saying the same thing. Which is correct? And does the value of mseccfg.useed affect anything?

Thanks.

[ben-marshall]

@mjosaarinen - I think this might be one for you?

[mjosaarinen]

The difference seems to be what kind of an exception occurs in the case of a read-write in VS/VU when mseccfg.sseed = 0. The spec says that it always causes a Virtual Instruction Exception and John said that it causes an illegal instruction exception. From the viewpoint of writing virtualization systems, it would feel awkward if it depended on a global flag like that; I'd think things that can possibly be legal in some virtual system can be handled via the virtual instruction exception. If seccfg.sseed is a Kernel configuration flag, as it may well be, then the hypervisor would need to implement two handlers to manage the gues depending on the particular configuration. I'm going to ask John about this.

[JamesKenneyImperas]

FYI, I think the relevant part of the Hypervisor Extension that discusses this might be 5.6 traps, which says among other things:

When V=1, a virtual instruction exception (code 22) is normally raised instead of an illegal instruction exception if the attempted instruction is HS-qualified but is prevented from executing when V=1 due to insufficient privilege or because the instruction is expressly disabled by a supervisor or hypervisor CSR such as scounteren or hcounteren. An instruction is HS-qualified if it would be valid to execute in HS-mode (for some values of the instruction’s register operands), assuming fields TSR and TVM of CSR mstatus are both zero.

In this case, when mseccfg.sseed=1, the instruction could be considered HS-qualified because it would execute in HS mode, but it has insufficient privilege for VS mode. When mseccfg.sseed=0, it would not be HS-qualified. It isn't clear to me what the rules would be for VU accesses with respect to mseccfg.useed though...

[mjosaarinen]

Yeah, it certainly seems easier to just always handle "potentially allowed" virtual things via the virtual instruction exception, rather than make it dependant on a global variable like mseccfg. The handling of illegal instructions and virtual exceptions are notably different:

"It is not unusual that hypervisors must emulate the instructions that raise virtual instruction exceptions, to support nested hypervisors or for other reasons. Machine level is expected ordinarily to delegate virtual instruction traps directly to HS-level, whereas illegal instruction traps are likely to be processed first in M-mode before being conditionally delegated (by software) to HS-level. Consequently, virtual instruction traps are expected typically to be handled faster than illegal instruction traps."

This makes the virtualization of something simple like an entropy source a nightmare if the global setting happens to be mseccfg.sseed=0. (Sect 5.6.1)

[mjosaarinen]

Anyway, here's a brief rationale for the spec being the way it is wrt VS/VU and mseccfg.sseed/useed. https://groups.google.com/a/groups.riscv.org/g/isa-dev/c/wHsZ986slaU/m/h98Sb8J-AAAJ

[ben-marshall]

@JamesKenneyImperas - just checking this latest change is enough for you to go on? If not, let us know and we'll discuss it some more. Otherwise, feel free to close this issue.

[JamesKenneyImperas]

Hello Ben,

Yes, this is good, already implemented and tested in riscvOVPsim.

Thanks,

James.