Re: [PATCH v3 02/20] target/riscv: remove MISA properties from isa_edata_arr[]

[Alistair Francis]

On Thu, Mar 30, 2023 at 3:33 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The code that disables extensions if there's a priv version mismatch
> uses cpu->cfg.ext_N properties to do its job.
>
> We're aiming to not rely on cpu->cfg.ext_N props for MISA bits. Split
> the MISA related verifications in a new function, removing it from
> isa_edata_arr[].
>
> We're also erroring it out instead of disabling, making the cpu_init()
> function responsible for running an adequate priv spec for the MISA
> extensions it wants to use.
>
> Note that the RVV verification is being ignored since we're always have
> at least PRIV_VERSION_1_10_0.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

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

Alistair

> ---
>  target/riscv/cpu.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 2711d80e16..21c0c637e4 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -72,10 +72,11 @@ struct isa_ext_data {
>   * 4. Non-standard extensions (starts with 'X') must be listed after all
>   *    standard extensions. They must be separated from other multi-letter
>   *    extensions by an underscore.
> + *
> + * Single letter extensions are checked in riscv_cpu_validate_misa_priv()
> + * instead.
>   */
>  static const struct isa_ext_data isa_edata_arr[] = {
> -    ISA_EXT_DATA_ENTRY(h, false, PRIV_VERSION_1_12_0, ext_h),
> -    ISA_EXT_DATA_ENTRY(v, false, PRIV_VERSION_1_10_0, ext_v),
>      ISA_EXT_DATA_ENTRY(zicbom, true, PRIV_VERSION_1_12_0, ext_icbom),
>      ISA_EXT_DATA_ENTRY(zicboz, true, PRIV_VERSION_1_12_0, ext_icboz),
>      ISA_EXT_DATA_ENTRY(zicond, true, PRIV_VERSION_1_12_0, ext_zicond),
> @@ -1131,6 +1132,14 @@ static void riscv_cpu_sync_misa_cfg(CPURISCVState *env)
>      env->misa_ext = env->misa_ext_mask = ext;
>  }
>
> +static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp)
> +{
> +    if (riscv_has_ext(env, RVH) && env->priv_ver < PRIV_VERSION_1_12_0) {
> +        error_setg(errp, "H extension requires priv spec 1.12.0");
> +        return;
> +    }
> +}
> +
>  static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>  {
>      CPUState *cs = CPU(dev);
> @@ -1174,6 +1183,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>       */
>      riscv_cpu_sync_misa_cfg(env);
>
> +    riscv_cpu_validate_misa_priv(env, &local_err);
> +    if (local_err != NULL) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
>      /* Force disable extensions if priv spec version does not match */
>      for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
>          if (isa_ext_is_enabled(cpu, &isa_edata_arr[i]) &&
> --
> 2.39.2
>
>