From: Andrew Jones <ajones@ventanamicro.com>
To: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
alistair.francis@wdc.com, bmeng@tinylab.org,
liweiwei@iscas.ac.cn, zhiwei_liu@linux.alibaba.com,
palmer@rivosinc.com
Subject: Re: [PATCH v8 05/19] target/riscv/tcg: update priv_ver on user_set extensions
Date: Thu, 2 Nov 2023 14:48:47 +0100 [thread overview]
Message-ID: <20231102-492bf4aa8761af0d8549ca3b@orel> (raw)
In-Reply-To: <05eb78d6-7563-4c5a-bae7-5ca14f4e91a2@ventanamicro.com>
On Thu, Nov 02, 2023 at 10:42:25AM -0300, Daniel Henrique Barboza wrote:
>
>
> On 11/2/23 06:47, Andrew Jones wrote:
> > On Wed, Nov 01, 2023 at 05:41:50PM -0300, Daniel Henrique Barboza wrote:
> > > We'll add a new bare CPU type that won't have any default priv_ver. This
> > > means that the CPU will default to priv_ver = 0, i.e. 1.10.0.
> > >
> > > At the same we'll allow these CPUs to enable extensions at will, but
> > > then, if the extension has a priv_ver newer than 1.10, we'll end up
> > > disabling it. Users will then need to manually set priv_ver to something
> > > other than 1.10 to enable the extensions they want, which is not ideal.
> > >
> > > Change the setter() of multi letter extensions to allow user enabled
> > > extensions to bump the priv_ver of the CPU. This will make it
> > > conveniente for users to enable extensions for CPUs that doesn't set a
> > > default priv_ver.
> > >
> > > This change does not affect any existing CPU: vendor CPUs does not allow
> > > extensions to be enabled, and generic CPUs are already set to priv_ver
> > > LATEST.
> > >
> > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > > ---
> > > target/riscv/tcg/tcg-cpu.c | 20 ++++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > >
> > > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> > > index f54069d06f..b88fce98a4 100644
> > > --- a/target/riscv/tcg/tcg-cpu.c
> > > +++ b/target/riscv/tcg/tcg-cpu.c
> > > @@ -114,6 +114,22 @@ static int cpu_cfg_ext_get_min_version(uint32_t ext_offset)
> > > g_assert_not_reached();
> > > }
> > > +static void cpu_validate_multi_ext_priv_ver(CPURISCVState *env,
> > > + uint32_t ext_offset)
> > > +{
> > > + int ext_priv_ver;
> > > +
> > > + if (env->priv_ver == PRIV_VERSION_LATEST) {
> > > + return;
> > > + }
> > > +
> > > + ext_priv_ver = cpu_cfg_ext_get_min_version(ext_offset);
> > > +
> > > + if (env->priv_ver < ext_priv_ver) {
> > > + env->priv_ver = ext_priv_ver;
> > > + }
> >
> > This will ignore user input. If the user, for example, does
> >
> > -cpu rv64,priv_spec=v1.10.0,zicbom=on
>
>
> This won't ignore user input because "priv_spec=v1.10.0" will be evaluated during
> finalize() time, in riscv_cpu_validate_priv_spec(). This change I made was made
> with this behavior in mind.
>
> In the case you mentioned, this will happen:
>
> $ ./build/qemu-system-riscv64 -M virt -cpu rv64,priv_spec=v1.10.0,zicbom=on
> qemu-system-riscv64: H extension requires priv spec 1.12.0
>
> This happened because, although 'zicbom=true' would bump priv ver to 1.12.0 if needed
> (it isn't - rv64 is already set to LATEST), "priv_spec=v1.10.0" is evaluated during
> finalize() time and the CPU priv_ver is set to 1.10 before our validation step.
>
> This means that doesn't matter where the 'priv_spec' option is in the command line,
> it'll always be honored.
Oh, good. Sorry for the noise! But maybe a comment above the priv_ver bump
saying something along the lines of what you wrote here would help ease
the minds of those that cross this in the future (including the mind of my
future self after I've forgotten this again)
>
>
> >
> > then, afaict, priv_spec will be silently bumped to 1.12. I think we should
> > error out in that case instead. And, we should also error out for
> >
> > -cpu rv64,zicbom=on,priv_spec=v1.10.0
> >
> > which means we should know when priv_spec is either
> >
> > - its default value
> > - has been bumped by an extension
> > - has been explicitly set by the user
>
> >
> > > +}
> > > +
> > > static void cpu_cfg_ext_auto_update(RISCVCPU *cpu, uint32_t ext_offset,
> > > bool value)
> > > {
> > > @@ -829,6 +845,10 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> > > return;
> > > }
> > > + if (value) {
> > > + cpu_validate_multi_ext_priv_ver(&cpu->env, multi_ext_cfg->offset);
> > > + }
> >
> > Some misa extensions also have priv spec version dependencies. See
> > riscv_cpu_validate_misa_priv()
>
> Yeah. I'll add this same mechanic to RVH, the only MISA bit we have that has a priv_ver
> limitation. Thanks,
Thanks,
drew
next prev parent reply other threads:[~2023-11-02 13:49 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-01 20:41 [PATCH v8 00/19] rv64i CPU, RVA22U64 profile support Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 01/19] target/riscv: create TYPE_RISCV_VENDOR_CPU Daniel Henrique Barboza
2023-11-02 2:32 ` Alistair Francis
2023-11-01 20:41 ` [PATCH v8 02/19] target/riscv/tcg: do not use "!generic" CPU checks Daniel Henrique Barboza
2023-11-02 2:38 ` Alistair Francis
2023-11-01 20:41 ` [PATCH v8 03/19] target/riscv/cpu.c: set satp_max_supported in cpu_riscv_set_satp() Daniel Henrique Barboza
2023-11-02 9:24 ` Andrew Jones
2023-11-02 12:53 ` Daniel Henrique Barboza
2023-11-02 13:11 ` Andrew Jones
2023-11-01 20:41 ` [PATCH v8 04/19] target/riscv/cpu.c: set satp_mode_max MBARE during satp_finalize() Daniel Henrique Barboza
2023-11-02 9:32 ` Andrew Jones
2023-11-01 20:41 ` [PATCH v8 05/19] target/riscv/tcg: update priv_ver on user_set extensions Daniel Henrique Barboza
2023-11-02 9:47 ` Andrew Jones
2023-11-02 13:42 ` Daniel Henrique Barboza
2023-11-02 13:48 ` Andrew Jones [this message]
2023-11-01 20:41 ` [PATCH v8 06/19] target/riscv: add rv64i CPU Daniel Henrique Barboza
2023-11-02 9:59 ` Andrew Jones
2023-11-02 14:23 ` Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 07/19] target/riscv: add zicbop extension flag Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 08/19] target/riscv/tcg: add 'zic64b' support Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 09/19] riscv-qmp-cmds.c: expose named features in cpu_model_expansion Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 10/19] target/riscv: add rva22u64 profile definition Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 11/19] target/riscv/kvm: add 'rva22u64' flag as unavailable Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 12/19] target/riscv/tcg: add user flag for profile support Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 13/19] target/riscv/tcg: add MISA user options hash Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 14/19] target/riscv/tcg: add riscv_cpu_write_misa_bit() Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 15/19] target/riscv/tcg: handle profile MISA bits Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 16/19] target/riscv/tcg: add hash table insert helpers Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 17/19] target/riscv/tcg: honor user choice for G MISA bits Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 18/19] target/riscv/tcg: validate profiles during finalize Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 19/19] riscv-qmp-cmds.c: add profile flags in cpu-model-expansion Daniel Henrique Barboza
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231102-492bf4aa8761af0d8549ca3b@orel \
--to=ajones@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=bmeng@tinylab.org \
--cc=dbarboza@ventanamicro.com \
--cc=liweiwei@iscas.ac.cn \
--cc=palmer@rivosinc.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).