Re: [PATCH v3 3/6] target/riscv/tcg: add user flag for profile support

[Andrew Jones]

On Fri, Oct 20, 2023 at 07:39:48PM -0300, Daniel Henrique Barboza wrote:
> The TCG emulation implements all the extensions described in the
> RVA22U64 profile, both mandatory and optional. The mandatory extensions
> will be enabled via the profile flag. We'll leave the optional
> extensions to be enabled by hand.
> 
> Given that this is the first profile we're implementing in TCG we'll
> need some ground work first:
> 
> - all profiles declared in riscv_profiles[] will be exposed to users.
> TCG is the main accelerator we're considering when adding profile
> support in QEMU, so for now it's safe to assume that all profiles in
> riscv_profiles[] will be relevant to TCG;
> 
> - we'll not support user profile settings for vendor CPUs. The flags
> will still be exposed but users won't be able to change them. The idea
> is that vendor CPUs in the future can enable profiles internally in
> their cpu_init() functions, showing to the external world that the CPU
> supports a certain profile. But users won't be able to enable/disable
> it;
> 
> - Setting a profile to 'true' means 'enable all mandatory extensions of
> this profile, setting it to 'false' means disabling all its mandatory
> extensions. Disabling a profile is discouraged for regular use and will
> issue an user warning. User choices for individual extensions will take
> precedence, i.e. enabling a profile will not enable extensions that the
> user set to 'false', and vice-versa. This will make us independent of
> left-to-right ordering in the QEMU command line, i.e. the following QEMU
> command lines:
> 
> -cpu rv64,zicbom=false,rva22u64=true,Zifencei=false
> -cpu rv64,zicbom=false,Zifencei=false,rva22u64=true
> -cpu rv64,rva22u64=true,zicbom=false,Zifencei=false
> 
> They mean the same thing: "enable all mandatory extensions of the
> rva22u64 profile while keeping zicbom and Zifencei disabled".

Hmm, I'm not sure I agree with special-casing profiles like this. I think
the left-to-right processing should be consistent for all. I'm also not
sure we should always warn when disabling a profile. For example, if a
user does

 -cpu rv64,rva22u64=true,rva22u64=false

then they'll get a warning, even though all they're doing is restoring the
cpu model. While that looks like an odd thing to do, a script may be
adding the rva22u64=true and the rva22u64=false is the user input which
undoes what the script did.

As far as warnings go, it'd be nice to warn when mandatory profile
extensions are disabled from an enabled profile. Doing that might be
useful for debug, but users which do it without being aware they're
"breaking" the profile may learn from that warning. Note, the warning
should only come when the profile is actually enabled and when the
extension would actually be disabled, i.e.

 -cpu rv64,rva22u64=true,c=off

should warn

 -cpu rv64,c=off,rva22u64=true

should not warn (rva22u64 overrides c=off since it's to the right)

 -cpu rv64,rva22u64=true,rva22u64=false,c=off

should not warn (rva22u64 is not enabled)

And,

 -cpu rv64,rva22u64=true,rva24u64=false

should warn for each extension which is mandatory in both profiles.

Thanks,
drew