Re: [PATCH 14/16] target/riscv: adapt 'riscv_isa_string' for KVM

[Andrew Jones]

On Tue, May 30, 2023 at 04:46:21PM -0300, Daniel Henrique Barboza wrote:
> KVM is not using the same attributes as TCG, i.e. it doesn't use
> isa_edata_arr[]. Add a new kvm_riscv_isa_string_ext() helper that does
> basically the same thing, but using KVM internals instead.
> 
> The decision to add this helper target/riscv/kvm.c is to foster the
> separation between KVM and TCG logic, while still using
> riscv_isa_string_ext() from target/riscv/cpu.c to retrieve the string
> to not overcomplicate things.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu.c       |  5 +++++
>  target/riscv/kvm.c       | 19 +++++++++++++++++++
>  target/riscv/kvm_riscv.h |  2 ++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 3c348049a3..ec1d0c621a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1956,6 +1956,11 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
> **isa_str,
>      char *new = *isa_str;
>      int i;
>  
> +    if (riscv_running_KVM()) {
> +        kvm_riscv_isa_string_ext(cpu, isa_str, max_str_len);
> +        return;
> +    }
> +
>      for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
>          if (cpu->env.priv_ver >= isa_edata_arr[i].min_version &&
>              isa_ext_is_enabled(cpu, &isa_edata_arr[i])) {
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index b4193a10d8..675e18df3b 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -320,6 +320,25 @@ static void kvm_riscv_add_cpu_user_properties(Object 
> *cpu_obj)
>      }
>  }
>  
> +void kvm_riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
> +                              int max_str_len)
> +{
> +    char *old = *isa_str;
> +    char *new = *isa_str;
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
> +        RISCVCPUMultiExtConfig *multi_ext_cfg = &kvm_multi_ext_cfgs[i];
> +        if (kvm_cpu_cfg_get(cpu, multi_ext_cfg)) {
> +            new = g_strconcat(old, "_", multi_ext_cfg->name, NULL);
> +            g_free(old);
> +            old = new;
> +        }
> +    }
> +
> +    *isa_str = new;
> +}
> +
>  static int kvm_riscv_get_regs_core(CPUState *cs)
>  {
>      int ret = 0;
> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h
> index e3ba935808..1a12efa8db 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -20,6 +20,8 @@
>  #define QEMU_KVM_RISCV_H
>  
>  void kvm_riscv_init_user_properties(Object *cpu_obj);
> +void kvm_riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
> +                              int max_str_len);
>  void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
>  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
>  
> -- 
> 2.40.1
> 
>

Hmm, more duplication. I think we need an abstraction which can support
both TCG and KVM extension lists. Allowing functions like
riscv_isa_string_ext() to be shared for them.

Thanks,
drew