Re: [RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension

[Vineet Gupta <vineetg@rivosinc.com>]

On 12/17/21 2:50 PM, Palmer Dabbelt wrote:
>>>
>>> According to
>>> <https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions>, 
>>> this
>>> was ratified in November so we should be able to update this to
>>> something like PRIV_SPEC_CLASS_1P12 (the V stuff was also ratified, but
>>> that's a different issue).
>>
>> I can understand that why Vineet are doing these changes, but I had a
>> quick talk with Kito this morning, and I think we also have three
>> issues for now,
>>
>> 1. The current draft ISA spec doesn't have any related changes, so if
>> we change the `h' as a single standard extension, then this will
>> conflict with the spec.
>
> Can you be specific about which specification you're talking about?   
> By my count there's more than a dozen now and it gets kind of tricky 
> to figure out which is which.
>
> I'm assuming you're talking about the user ISA spec, the last ratified 
> version I can find is 
> <https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf>, 
> which definately lists H as a prefix.

I think Nelson is referring to section 27.8 of above document which 
specifies "H" to be named as "S" which in turn suggests a scheme of 'S' 
prefix + alphabetical name + optional ver number.
>
>> 2. Even if we decide to change the `h' as a single extension rather
>> than the multiple prefixed keyword, then in what order should we place
>> h.  I expect the table 27.1 in the ISA spec will also mention the
>> order of the single h.
>
> I guess I'd just assumed this was called "H", given that's what we've 
> all been calling it for a long time, but after actually reading the 
> ratified specs I don't see that anywhere.
>
> The Hypervisor extension itself is ratified, via priv-1.12, but I 
> don't see anything in a ratified spec that describes what it's called 
> in ISA strings.  It's called "Hypervisor ISA, Version 1.0" in the 
> preface and "Hypervisor Extension, Version 1.0.0-rc" in the chapter 
> that defines it (both in the ratified priv-1.12).  Both of those claim 
> they're frozen, in contrast to the ratified user spec which calls out 
> extensions as ratified -- not sure if that's relevant, though, as I'm 
> generally pretty lost WRT the state of the 
> specifications/extensions/versions right now.
>
> The rules in the ratified user spec make it sound like this would 
> either be called "S*" or "H*" (depending on whether this is a 
> supervisor-mode extension or an ISA), but I can't find anything 
> pointing to where the second letter should be.  Also no idea what to 
> do with the version, assuming that RC suffix is canonical.
>
> Maybe I'm missing something?

Yeah it seems section 27.8 is the contention point.

>
>> 3. I never considered that an extension version may be controlled by
>> the privileged spec before, so in the riscv_get_default_ext_version,
>>
>>> >        if (strcmp (table[i].name, name) == 0
>>> >         && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
>>> > +           || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
>>> >             || table[i].isa_spec_class == *default_isa_spec))
>>
>> We will need to rewrite the related code, since the isa_spec_class may
>> be one of the PRIV_SPEC_CLASS_XXX in the future, so we will never
>> match it to the default_isa_spec...
>
> More that that: it now sounds like the hypervisor extension (and/or 
> ISA?) encoding for ISA strings changes based on the user specification 
> in play, despite it being defined in the priv specification.
>
> Certainly sounds like we should hold off until we get something 
> concrete on the naming scheme, as making something up on our own is 
> going to be a mess in the long run.

The naming scheme is already a mess - I'd suggest we get the H-code 
support in (adhering to existing naming scheme) and clean up the naming 
/ arch string / toggles when that gets fixed. No point in holding the 
development based on hypervisor for that - and having to hand code 
opcodes and csrs in the relevant projects.