Re: [PATCH v2 2/3] target/riscv/cpu.c: create flag for ziccrse

[Andrew Jones <ajones@ventanamicro.com>]

On Fri, Feb 21, 2025 at 11:18:33AM -0300, Daniel Henrique Barboza wrote:
> At this moment ziccrse is a TCG always enabled named feature for
> priv_ver > 1.11 that has no exclusive flag. In the next patch we'll make
> the KVM driver update ziccrse as well, turning it on/off depending on
> host settings, but for that we'll need an ext_ziccrse flag in the CPU
> state.
> 
> Create an exclusive flag for it like we do with other named features.
> As with any named features we already have, it won't be exposed to
> users. TCG will keep the same restiction for it (always enabled if
> has_priv_1_11 is true) and KVM will be free to turn it on/off as
> required.

Reading this as "KVM can choose" makes it sound wrong, since KVM can't
choose. However, KVM will turn it on/off depending on whether this
extension is/isn't present. So reading it as "TCG always has it on, but
KVM will turn it off when the extension isn't available", makes more
sense.

> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu.c         | 3 ++-
>  target/riscv/cpu_cfg.h     | 3 +++
>  target/riscv/tcg/tcg-cpu.c | 2 ++
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 522d6584e4..fc4632ce36 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -105,7 +105,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(ziccamoa, PRIV_VERSION_1_11_0, has_priv_1_11),
>      ISA_EXT_DATA_ENTRY(ziccif, PRIV_VERSION_1_11_0, has_priv_1_11),
>      ISA_EXT_DATA_ENTRY(zicclsm, PRIV_VERSION_1_11_0, has_priv_1_11),
> -    ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, has_priv_1_11),
> +    ISA_EXT_DATA_ENTRY(ziccrse, PRIV_VERSION_1_11_0, ext_ziccrse),
>      ISA_EXT_DATA_ENTRY(zicfilp, PRIV_VERSION_1_12_0, ext_zicfilp),
>      ISA_EXT_DATA_ENTRY(zicfiss, PRIV_VERSION_1_13_0, ext_zicfiss),
>      ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
> @@ -1749,6 +1749,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
>      MULTI_EXT_CFG_BOOL("zic64b", ext_zic64b, true),
>      MULTI_EXT_CFG_BOOL("ssstateen", ext_ssstateen, true),
>      MULTI_EXT_CFG_BOOL("sha", ext_sha, true),
> +    MULTI_EXT_CFG_BOOL("ziccrse", ext_ziccrse, true),
>  
>      { },
>  };
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 3f3c1118c0..8a843482cc 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -166,6 +166,9 @@ struct RISCVCPUConfig {
>      bool has_priv_1_12;
>      bool has_priv_1_11;
>  
> +    /* Always enabled for TCG if has_priv_1_11 */
> +    bool ext_ziccrse;
> +
>      /* Vendor-specific custom extensions */
>      bool ext_xtheadba;
>      bool ext_xtheadbb;
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index ea8d77d06a..c93612b1da 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -360,6 +360,8 @@ static void riscv_cpu_update_named_features(RISCVCPU *cpu)
>  
>      cpu->cfg.ext_sha = riscv_has_ext(&cpu->env, RVH) &&
>                         cpu->cfg.ext_ssstateen;
> +
> +    cpu->cfg.ext_ziccrse = cpu->cfg.has_priv_1_11;
>  }
>  
>  static void riscv_cpu_validate_g(RISCVCPU *cpu)
> -- 
> 2.48.1
>

Other than my hangup on the commit message,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>