Re: RFC: QEMU rv64/rv32 CPU, 'max' CPU and profile support

[Andrew Jones]

On Thu, Sep 14, 2023 at 11:09:57AM -0700, Palmer Dabbelt wrote:
> On Thu, 14 Sep 2023 08:08:40 PDT (-0700), ajones@ventanamicro.com wrote:
> > On Thu, Sep 14, 2023 at 07:52:10AM -0700, Andrea Bolognani wrote:
> >> On Mon, Sep 11, 2023 at 07:01:38PM -0300, Daniel Henrique Barboza wrote:
> >> > On 9/11/23 16:46, Daniel Henrique Barboza wrote:
> >> > > On 9/8/23 11:10, Andrew Jones wrote:
> >> > > > On Fri, Sep 08, 2023 at 03:28:12AM -0700, Andrea Bolognani wrote:
> >> > > > > Wouldn't that be exposed as a named vendor CPU rather than 
> >> > > > > something
> >> > > > > that users have to assemble themselves by layering profiles?
> >> > > >
> >> > > > Combing multiple profiles should be considered as creating a union 
> >> > > > of the
> >> > > > mandatory extensions each profile represents, rather than as 
> >> > > > layering.
> >> > > >
> >> > > > Let's take the hypothetical case of C being dropped from rva24u64 as 
> >> > > > an
> >> > > > example. When specifying both rva22u64 and rva24u64
> >> > > > (-cpu min64,rva22u64=on,rva24u64=on) the user will get C enabled 
> >> > > > without
> >> > > > even having to know about it, whereas if the user could only select 
> >> > > > the
> >> > > > latest profile (-cpu rva24u64), C would not get enabled unless the 
> >> > > > user
> >> > > > was aware that it needed to be explicitly enabled (-cpu 
> >> > > > rva24u64,c=on).
> >>
> >> Right, that's what I had in mind when I was talking about layering.
> >> Using the term union is probably more appropriate though :)
> >>
> >> > > > > If you're working on defining a new CPU model and need fine-grained
> >> > > > > control over the exact set of extensions, then you can already do
> >> > > > > that by explicitly flipping each one of them on or off, and adding
> >> > > > > the ability to layer profiles doesn't add much value except 
> >> > > > > possibly
> >> > > > > removing a bit of verbosity. Not particularly compelling IMO.
> >> > > >
> >> > > > I think it's compelling, because the extension lists that profiles 
> >> > > > provide
> >> > > > are long and mostly uninteresting. For example, how often do we want 
> >> > > > to
> >> > > > type and think about imafd_Zicsr_Zifencei? I think we'd mostly 
> >> > > > rather take
> >> > > > those for granted, and we can, because we just specify 'g' instead. 
> >> > > > Indeed
> >> > > > the profile spec even points out that using profiles as a way to 
> >> > > > deal with
> >> > > > "unwieldy" ISA strings is another motivation for them. A RISC-V QEMU 
> >> > > > CPU
> >> > > > command line is effectively an ISA string, so I think it's 
> >> > > > appropriate to
> >> > > > apply profiles to it as well.
> >>
> >> I'm not sure this will save a lot of typing unless you're enabling a
> >> lot of profiles which are possibly far apart from each other in terms
> >> of release year, but I also don't have a very strong opposition to
> >> the approach. As you say, we are already effectively implementing it
> >> with virtual extensions like "g", so extending the applicability
> >> further to cover profiles is not too much of a stretch.
> >>
> >> I think libvirt will probably need to learn what set of extensions
> >> each virtual extension/profile maps to, but since the definition will
> >> come directly from actual specifications that shouldn't be a big
> >> problem.
> >>
> >> > > > >    (This also gets really interesting when you start thinking about
> >> > > > >    extensions that are optional for the profile... Maybe we need
> >> > > > >    something like '-cpu rva23u64,optional-extensions=off' so that
> >> > > > >    testing against the baseline for a profile doesn't require 
> >> > > > > combing
> >> > > > >    through the spec and disabling extensions manually?)
> >> > > >
> >> > > > I don't think the profiles (whether they are CPU virtual extensions 
> >> > > > or
> >> > > > CPU models) should enable the optional extensions by default. When
> >> > > > software targets a profile (or set of profiles with the virtual
> >> > > > extension approach) then it should never assume the optional 
> >> > > > extensions
> >> > > > will be present. Developers should need to consciously include those
> >> > > > extensions when creating the QEMU platform used for testing.
> >>
> >> Fair enough.
> >>
> >> Should there be a way to explicitly enable all possible optional
> >> extensions, then? To allow the developer to verify that the
> >> application works correctly on a minimal implementation of the
> >> profile, but also takes advantage of optional extensions if they are
> >> present? Or would that be achieved by enabling the relevant
> >> extensions manually? Or perhaps by using the max CPU model instead?
> >
> > We could add a variant of each profile which represents everything the
> > profile mentions. That could maybe be held off until the need arises,
> > though, since 'max' may satisfy most users which aren't concerned about
> > which extensions are mandatory vs. optional.
> 
> At the risk of adding to the bikeshedding here:  IMO we should start 
> with a small set of CPU types that we know are useful.  We're not going 
> to be able to cover everything, hence the options to turn on/off 
> extensions/features.  Anything we add needs to go through the 
> deprecation process if we remove it, though, so it's a bit of a 
> headache.
>

I agree. I think we need a 'max' since it'll be the one primarily used by
developers and CI. Then, we also need a way to build a specific cpu which
may not have a vendor name yet or ever be assigned one. That means we need
a "base", which we've tried out different names for (rv32/rv64,
min, rv32i/rv64i, ...), but they're all the same thing. The "base" cpu
type and the "max" cpu type should never need to be depreciated. We
may find it useful to introduce alias cpu types for particular uses of
them, e.g. max64 == 'max,xlen64=on', but we can wait and see before
introducing those.

Defining profiles as "virtual extensions", which represent collections of
extensions, avoids creating new cpu types for each of them. We just
create new properties. I wouldn't expect to ever need to deprecate those
properties, since they'd only be a list of extensions and a cpu finalize
function which validates that all the mandatory extensions remain enabled
and are compatible with everything else that got enabled.

Thanks,
drew