On Sat, Oct 28, 2023 at 6:56 PM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
> RVG behaves like a profile: a single flag enables a set of bits. Right
> now we're considering user choice when handling RVG and zicsr/zifencei
> and ignoring user choice on MISA bits.
>
> We'll add user warnings for profiles when the user disables its
> mandatory extensions in the next patch. We'll do the same thing with RVG
> now to keep consistency between RVG and profile handling.
>
> First and foremost, create a new RVG only helper to avoid clogging
> riscv_cpu_validate_set_extensions(). We do not want to annoy users with
> RVG warnings like we did in the past (see 9b9741c38f), thus we'll only
> warn if RVG was user set and the user disabled a RVG extension in the
> command line.
>
> For every RVG MISA bit (IMAFD), zicsr and zifencei, the logic then
> becomes:
>
> - if enabled, do nothing;
> - if disabled and not user set, enable it;
> - if disabled and user set, throw a warning that it's a RVG mandatory
> extension.
>
> This same logic will be used for profiles in the next patch.
>
> Note that this is a behavior change, where we would error out if the
> user disabled either zicsr or zifencei. As long as users are explicitly
> disabling things in the command line we'll let them have a go at it, at
> least in this step. We'll error out later in the validation if needed.
>
> Other notable changes from the previous RVG code:
>
> - use riscv_cpu_write_misa_bit() instead of manually updating both
> env->misa_ext and env->misa_ext_mask;
>
> - set zicsr and zifencei directly. We're already checking if they
> were user set and priv version will never fail for these
> extensions, making cpu_cfg_ext_auto_update() redundant.
>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajo...@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/tcg/tcg-cpu.c | 73 +++++++++++++++++++++++++-------------
> 1 file changed, 48 insertions(+), 25 deletions(-)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 3a96b1f476..953e8432d6 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -42,6 +42,12 @@ static bool cpu_cfg_ext_is_user_set(uint32_t ext_offset)
> GUINT_TO_POINTER(ext_offset));
> }
>
> +static bool cpu_misa_ext_is_user_set(uint32_t misa_bit)
> +{
> + return g_hash_table_contains(misa_ext_user_opts,
> + GUINT_TO_POINTER(misa_bit));
> +}
> +
> static void cpu_cfg_ext_add_user_opt(uint32_t ext_offset, bool value)
> {
> g_hash_table_insert(multi_ext_user_opts, GUINT_TO_POINTER(ext_offset),
> @@ -303,6 +309,46 @@ static void riscv_cpu_validate_named_features(RISCVCPU
> *cpu)
> riscv_cpu_validate_zic64b(cpu);
> }
>
> +static void riscv_cpu_validate_g(RISCVCPU *cpu)
> +{
> + const char *warn_msg = "RVG mandates disabled extension %s";
> + uint32_t g_misa_bits[] = {RVI, RVM, RVA, RVF, RVD};
> + bool send_warn = cpu_misa_ext_is_user_set(RVG);
> +
> + for (int i = 0; i < ARRAY_SIZE(g_misa_bits); i++) {
> + uint32_t bit = g_misa_bits[i];
> +
> + if (riscv_has_ext(&cpu->env, bit)) {
> + continue;
> + }
> +
> + if (!cpu_misa_ext_is_user_set(bit)) {
> + riscv_cpu_write_misa_bit(cpu, bit, true);
> + continue;
> + }
> +
> + if (send_warn) {
> + warn_report(warn_msg, riscv_get_misa_ext_name(bit));
> + }
> + }
> +
> + if (!cpu->cfg.ext_zicsr) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zicsr))) {
> + cpu->cfg.ext_zicsr = true;
> + } else if (send_warn) {
> + warn_report(warn_msg, "zicsr");
> + }
> + }
> +
> + if (!cpu->cfg.ext_zifencei) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zifencei))) {
> + cpu->cfg.ext_zifencei = true;
> + } else if (send_warn) {
> + warn_report(warn_msg, "zifencei");
> + }
> + }
> +}
> +
> /*
> * Check consistency between chosen extensions while setting
> * cpu->cfg accordingly.
> @@ -312,31 +358,8 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
> Error **errp)
> CPURISCVState *env = &cpu->env;
> Error *local_err = NULL;
>
> - /* Do some ISA extension error checking */
> - if (riscv_has_ext(env, RVG) &&
> - !(riscv_has_ext(env, RVI) && riscv_has_ext(env, RVM) &&
> - riscv_has_ext(env, RVA) && riscv_has_ext(env, RVF) &&
> - riscv_has_ext(env, RVD) &&
> - cpu->cfg.ext_zicsr && cpu->cfg.ext_zifencei)) {
> -
> - if (cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zicsr)) &&
> - !cpu->cfg.ext_zicsr) {
> - error_setg(errp, "RVG requires Zicsr but user set Zicsr to
> false");
> - return;
> - }
> -
> - if (cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zifencei)) &&
> - !cpu->cfg.ext_zifencei) {
> - error_setg(errp, "RVG requires Zifencei but user set "
> - "Zifencei to false");
> - return;
> - }
> -
> - cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zicsr), true);
> - cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zifencei), true);
> -
> - env->misa_ext |= RVI | RVM | RVA | RVF | RVD;
> - env->misa_ext_mask |= RVI | RVM | RVA | RVF | RVD;
> + if (riscv_has_ext(env, RVG)) {
> + riscv_cpu_validate_g(cpu);
> }
>
> if (riscv_has_ext(env, RVI) && riscv_has_ext(env, RVE)) {
> --
> 2.41.0
>
>
