Re: [PATCH 19/20] target/riscv: add 'tcg_supported' class property

[Andrew Jones <ajones@ventanamicro.com>]

On Fri, Aug 25, 2023 at 10:08:52AM -0300, Daniel Henrique Barboza wrote:
> This property indicates if a CPU supports TCG acceleration. All CPUs but
> the 'host' CPU supports it.
> 
> The error in tcg_cpu_realizefn() can now be made generic in case more
> non-TCG CPUs are added in the future.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu-qom.h     |  1 +
>  target/riscv/cpu.c         | 10 ++++++++++
>  target/riscv/cpu.h         |  1 +
>  target/riscv/tcg/tcg-cpu.c |  7 +++++--
>  4 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
> index 7c76dc0dcc..e86b76f9fe 100644
> --- a/target/riscv/cpu-qom.h
> +++ b/target/riscv/cpu-qom.h
> @@ -71,5 +71,6 @@ struct RISCVCPUClass {
>      ResettablePhases parent_phases;
>  
>      bool user_extension_properties;
> +    bool tcg_supported;
>  };
>  #endif /* RISCV_CPU_QOM_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6817f94c2c..f749ea2a2e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -625,6 +625,14 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
>  }
>  #endif
>  
> +char *riscv_cpu_get_name(RISCVCPUClass *rcc)
> +{
> +    const char *typename = object_class_get_name(OBJECT_CLASS(rcc));
> +
> +    return g_strndup(typename,
> +                     strlen(typename) - strlen("-" TYPE_RISCV_CPU));
                                                  RISCV_CPU_TYPE_SUFFIX

Could also add the assert like x86 has in x86_cpu_class_get_model_name()

> +}
> +
>  static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
>  {
>      ObjectClass *oc;
> @@ -1637,6 +1645,7 @@ static void riscv_dynamic_cpu_class_init(ObjectClass *c, void *data)
>      RISCVCPUClass *rcc = RISCV_CPU_CLASS(c);
>  
>      rcc->user_extension_properties = true;
> +    rcc->tcg_supported = true;

Rather than add this tcg_supported to most (all but 'host'?) cpus, what
about doing what x86 does and create an 'accel_uses_host_cpuid()' function
which is checked in realize?

>  }
>  
>  static void riscv_vendor_cpu_class_init(ObjectClass *c, void *data)
> @@ -1644,6 +1653,7 @@ static void riscv_vendor_cpu_class_init(ObjectClass *c, void *data)
>      RISCVCPUClass *rcc = RISCV_CPU_CLASS(c);
>  
>      rcc->user_extension_properties = false;
> +    rcc->tcg_supported = true;
>  }
>  
>  #define DEFINE_DYNAMIC_CPU(type_name, initfn) \
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4254f04684..1e6ecf52ee 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -732,6 +732,7 @@ typedef struct isa_ext_data {
>  extern const RISCVIsaExtData isa_edata_arr[];
>  
>  void riscv_add_satp_mode_properties(Object *obj);
> +char *riscv_cpu_get_name(RISCVCPUClass *rcc);
>  
>  /* CSR function table */
>  extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 6c91978920..a13796c597 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -554,11 +554,14 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>  static bool tcg_cpu_realizefn(CPUState *cs, Error **errp)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> +    RISCVCPUClass *rcc = RISCV_CPU_GET_CLASS(cpu);
>      CPURISCVState *env = &cpu->env;
>      Error *local_err = NULL;
>  
> -    if (object_dynamic_cast(OBJECT(cpu), TYPE_RISCV_CPU_HOST)) {
> -        error_setg(errp, "'host' CPU is not compatible with TCG acceleration");
> +    if (!rcc->tcg_supported) {
> +        g_autofree char *name = riscv_cpu_get_name(rcc);
> +        error_setg(errp, "'%s' CPU is not compatible with TCG acceleration",
> +                   name);
>          return false;
>      }
>  
> -- 
> 2.41.0
> 
>

Thanks,
drew