Re: [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks

[Alistair Francis <alistair23@gmail.com>]

On Sun, May 15, 2022 at 12:56 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> We should separate "check" and "configure" steps as possible.
> This commit separates both steps except vector/Zfinx-related checks.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f910a41407..5ab246bf63 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -630,14 +630,27 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>
> +        if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> +            error_setg(errp, "Zve32f/Zve64f extensions require F extension");
> +            return;
> +        }
> +
> +        /* Set the ISA extensions, checks should have happened above */
>          if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
>              cpu->cfg.ext_zhinxmin) {
>              cpu->cfg.ext_zfinx = true;
>          }
>
> -        if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
> -            error_setg(errp, "Zfinx extension requires Zicsr");
> -            return;
> +        if (cpu->cfg.ext_zfinx) {
> +            if (!cpu->cfg.ext_icsr) {
> +                error_setg(errp, "Zfinx extension requires Zicsr");
> +                return;
> +            }
> +            if (cpu->cfg.ext_f) {
> +                error_setg(errp,
> +                    "Zfinx cannot be supported together with F extension");
> +                return;
> +            }
>          }
>
>          if (cpu->cfg.ext_zk) {
> @@ -663,7 +676,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              cpu->cfg.ext_zksh = true;
>          }
>
> -        /* Set the ISA extensions, checks should have happened above */
>          if (cpu->cfg.ext_i) {
>              ext |= RVI;
>          }
> @@ -734,20 +746,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              }
>              set_vext_version(env, vext_version);
>          }
> -        if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> -            error_setg(errp, "Zve32f/Zve64f extension depends upon RVF.");
> -            return;
> -        }
>          if (cpu->cfg.ext_j) {
>              ext |= RVJ;
>          }
> -        if (cpu->cfg.ext_zfinx && ((ext & (RVF | RVD)) || cpu->cfg.ext_zfh ||
> -                                   cpu->cfg.ext_zfhmin)) {
> -            error_setg(errp,
> -                    "'Zfinx' cannot be supported together with 'F', 'D', 'Zfh',"
> -                    " 'Zfhmin'");
> -            return;
> -        }
>
>          set_misa(env, env->misa_mxl, ext);
>      }
> --
> 2.34.1
>