Re: [PATCH v2 00/10] riscv: RVA22U64 profile support

[Andrew Jones <ajones@ventanamicro.com>]

On Tue, Oct 17, 2023 at 01:55:47PM +1000, Alistair Francis wrote:
> On Mon, Oct 16, 2023 at 7:03 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Thu, Oct 12, 2023 at 04:07:50PM -0300, Daniel Henrique Barboza wrote:
> > >
> > >
> > > On 10/11/23 00:01, Alistair Francis wrote:
> > > > On Sat, Oct 7, 2023 at 12:23 AM Daniel Henrique Barboza
> > > > <dbarboza@ventanamicro.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > Several design changes were made in this version after the reviews and
> > > > > feedback in the v1 [1]. The high-level summary is:
> > > > >
> > > > > - we'll no longer allow users to set profile flags for vendor CPUs. If
> > > > >    we're to adhere to the current policy of not allowing users to enable
> > > > >    extensions for vendor CPUs, the profile support would become a
> > > > >    glorified way of checking if the vendor CPU happens to support a
> > > > >    specific profile. If a future vendor CPU supports a profile the CPU
> > > > >    can declare it manually in its cpu_init() function, the flag will
> > > > >    still be set, but users can't change it;
> > > > >
> > > > > - disabling a profile will now disable all the mandatory extensions from
> > > > >    the CPU;
> > > >
> > > > What happens if you enable one profile and disable a different one?
> > >
> > > With this implementation as is the profiles will be evaluated by the order they're
> > > declared in riscv_cpu_profiles[]. Which isn't exactly ideal since we're exchanging
> > > a left-to-right ordering in the command line by an arbitrary order that we happened
> > > to set in the code.
> > >
> > > I can make some tweaks to make the profiles sensible to left-to-right order between
> > > them, while keeping regular extension with higher priority. e.g.:
> > >
> > >
> > > -cpu rv64,zicbom=true,profileA=false,profileB=true,zicboz=false
> > > -cpu rv64,profileA=false,zicbom=true,zicboz=false,profileB=true
> > > -cpu rv64,profileA=false,profileB=true,zicbom=true,zicboz=false
> > >
> > > These would all do the same thing: "keeping zicbom=true and zicboz=false, disable profileA
> > > and then enable profile B"
> > >
> > > Switching the profiles order would have a different result:
> > >
> > > -cpu rv64,profileB=true,profileA=false,zicbom=true,zicboz=false
> > >
> > > "keeping zicbom=true and zicboz=false, enable profile B and then disable profile A"
> > >
> > >
> > > I'm happy to hear any other alternative/ideas. We'll either deal with some left-to-right
> > > ordering w.r.t profiles or deal with an internal profile commit ordering. TBH I think
> > > it's sensible to demand left-to-right command line ordering for profiles only.
> >
> > left-to-right ordering is how the rest of QEMU properties work and scripts
> > depend on it. For example, one can do -cpu $MODEL,$DEFAULT_PROPS,$MORE_PROPS
> > where $MORE_PROPS can not only add more props but also override default
> > props (DEFAULT_PROPS='foo=off', MORE_PROPS='foo=on' - foo will be on).
> > left-to-right also works with multiple -cpu parameters, i.e. -cpu
> > $MODEL,$DEFAULT_PROPS -cpu $MODEL,$MY_PROPS will replace default props
> > with my props.
> 
> That seems like the way to go then
> 
> >
> > I don't think profiles should be treated special with regard to this. They
> > should behave the same as any property. If one does
> > profileA=off,profileB=on and there are overlapping extensions then a
> 
> But what does this mean? What intent is the user saying here?
> 
> For example if a user says:
> 
>     RVA22U64=off,RVA24U64=on
> 
> They only want the extensions that were added in RVA24U64? What about
> G and the standard extensions?

Disabling a profile is certainly odd, because I wouldn't expect profiles
to be used with any CPU type other than a base type, i.e. they should be
used to enable extensions on a barebones CPU model, which means setting
them off would be noops.  And, if a profile is set off for a cpu model
where extensions are set either by the model or by previous profile and
extension properties, then it also seems like an odd use, but that's at
least consistent with how other properties would work, so I'm not sure we
need to forbid it.

> 
> To me it just seems really strange to have more than 1 profile.
> Profiles are there to help software and users have common platforms.
> Why would a user want to mix-n-match them

I think it's possible users will want to describe platforms which are
compatible with more than one profile, e.g. RVA22U64=on,RVA24U64=on.

The example I gave Andrea about this was that C may get demoted from
mandatory to optional in later profiles. If a platform is built which
conforms to an older profile with C and to the later profile where C
is only optional, then enabling both profiles will ensure that C is
enabled, whereas only enabling the later profile will not, and then
C must be added manually after inspecting the older profile to see
what will be missed. OIOW, the burden of individual extension management
will still be present if only single profiles may be enabled at a time.
(And, even if the later profile was a strict superset of the older one,
then, if a user wants to explicitly describe a platform which claims
compatibility with both profiles, they probably shouldn't be punished
for it, even if the resulting extension enablement would be equivalent
to only enabling the later profile.)

Thanks,
drew