On Fri, Oct 13, 2023 at 5:07 AM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
>
>
> On 10/11/23 00:01, Alistair Francis wrote:
> > On Sat, Oct 7, 2023 at 12:23 AM Daniel Henrique Barboza
> > <dbarb...@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
I think we should just not allow this.
I don't understand why a user would want this and what they mean here.
What if profileA and profileB have overlapping extensions?
Alistair
>
> 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.
>
>
> Thanks,
>
>
> Daniel
>
>
>
>
>
> >
> > Alistair
> >
> >>
> >> - the profile logic was moved to realize() time in a step we're calling
> >> 'commit profile'. This allows us to enable/disable profile extensions
> >> after considering user input in other individual extensions. The
> >> result is that we don't care about the order in which the profile flag
> >> was set in comparison with other extensions in the command line, i.e.
> >> the following lines are equal:
> >>
> >> -cpu rv64,zicbom=false,rva22u64=true,Zifencei=false
> >>
> >> -cpu rv64,rva22u64=true,zicbom=false,Zifencei=false
> >>
> >> and they mean 'enable the rva22u64 profile while keeping zicbom and
> >> Zifencei disabled'.
> >>
> >>
> >> Other minor changes were needed as result of these design changes. E.g.
> >> we're now having to track MISA extensions set by users (patch 7),
> >> something that we were doing only for multi-letter extensions.
> >>
> >> Changes from v1:
> >> - patch 6 from v1 ("target/riscv/kvm: add 'rva22u64' flag as unavailable"):
> >> - moved up to patch 4
> >> - patch 5 from v1("target/riscv/tcg-cpu.c: enable profile support for
> >> vendor CPUs"):
> >> - dropped
> >> - patch 6 (new):
> >> - add riscv_cpu_commit_profile()
> >> - patch 7 (new):
> >> - add user choice hash for MISA extensions
> >> - patch 9 (new):
> >> - handle MISA bits user choice when commiting profiles
> >> - patch 8 and 10 (new):
> >> - helpers to avoid code repetition
> >> - v1 link:
> >> https://lore.kernel.org/qemu-riscv/20230926194951.183767-1-dbarb...@ventanamicro.com/
> >>
> >>
> >> Daniel Henrique Barboza (10):
> >> target/riscv/cpu.c: add zicntr extension flag
> >> target/riscv/cpu.c: add zihpm extension flag
> >> target/riscv: add rva22u64 profile definition
> >> target/riscv/kvm: add 'rva22u64' flag as unavailable
> >> target/riscv/tcg: add user flag for profile support
> >> target/riscv/tcg: commit profiles during realize()
> >> target/riscv/tcg: add MISA user options hash
> >> target/riscv/tcg: add riscv_cpu_write_misa_bit()
> >> target/riscv/tcg: handle MISA bits on profile commit
> >> target/riscv/tcg: add hash table insert helpers
> >>
> >> target/riscv/cpu.c | 29 +++++++
> >> target/riscv/cpu.h | 12 +++
> >> target/riscv/cpu_cfg.h | 2 +
> >> target/riscv/kvm/kvm-cpu.c | 7 +-
> >> target/riscv/tcg/tcg-cpu.c | 165 +++++++++++++++++++++++++++++++++----
> >> 5 files changed, 197 insertions(+), 18 deletions(-)
> >>
> >> --
> >> 2.41.0
> >>
> >>
