Re: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types

[Zhao Liu <zhao1.liu@linux.intel.com>]

Hi Philippe,

On Fri, Mar 15, 2024 at 02:08:54PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 15 Mar 2024 14:08:54 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-9.1 06/21] target/i386: Make X86_CPU common to new
>  I386_CPU / X86_64_CPU types
> X-Mailer: git-send-email 2.41.0
> 
> "target/foo/cpu-qom.h" can not use any target specific definitions.
> 
> Currently "target/i386/cpu-qom.h" defines TYPE_X86_CPU depending
> on the i386/x86_64 build type. This doesn't scale in a heterogeneous
> context where we need to access both types concurrently.

Does this mean that there would be a TCG case contains both 64-bit and
32-bit i386 core? ;-)

> In order to do that, introduce the new I386_CPU / X86_64_CPU
> types, both inheriting a common TYPE_X86_CPU base type.
> 
> Keep the current "base" and "max" CPU types as 32 or 64-bit,
> depending on the binary built.
> 
> Adapt the cpu-plug-test, since the 'base' architecture is now
> common to both 32/64-bit x86 targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/i386/cpu-qom.h       | 16 ++++++++++------
>  target/i386/cpu.c           | 20 ++++++++++++++++++--
>  tests/qtest/cpu-plug-test.c |  2 +-
>  3 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
> index d4e216d000..de28d7ea20 100644
> --- a/target/i386/cpu-qom.h
> +++ b/target/i386/cpu-qom.h
> @@ -1,5 +1,5 @@
>  /*
> - * QEMU x86 CPU
> + * QEMU x86 CPU QOM header (target agnostic)
>   *
>   * Copyright (c) 2012 SUSE LINUX Products GmbH
>   *
> @@ -22,14 +22,18 @@
>  
>  #include "hw/core/cpu.h"
>  
> -#ifdef TARGET_X86_64
> -#define TYPE_X86_CPU "x86_64-cpu"
> -#else
> -#define TYPE_X86_CPU "i386-cpu"
> -#endif
> +#define TYPE_X86_CPU    "x86-cpu"
> +#define TYPE_I386_CPU   "i386-cpu"
> +#define TYPE_X86_64_CPU "x86_64-cpu"
>  
>  OBJECT_DECLARE_CPU_TYPE(X86CPU, X86CPUClass, X86_CPU)
>  
> +OBJECT_DECLARE_CPU_TYPE(I386CPU, X86CPUClass, I386_CPU)
> +OBJECT_DECLARE_CPU_TYPE(X86_64CPU, X86CPUClass, X86_64_CPU)
> +
> +#define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
> +#define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
> +

X86_CPU_TYPE_NAME seems to be duplicated because the following line is
the existing X86_CPU_TYPE_NAME definition.

>  #define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU
>  #define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)
>  
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ebf555f50f..07f64c1ea5 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8057,12 +8057,28 @@ static const TypeInfo x86_cpu_types[] = {
>          .class_size     = sizeof(X86CPUClass),
>          .class_init     = x86_cpu_common_class_init,
>      }, {
> -        .name           = X86_CPU_TYPE_NAME("base"),
> +        .name           = TYPE_I386_CPU,
>          .parent         = TYPE_X86_CPU,
> +        .abstract       = true,
> +    }, {
> +        .name           = TYPE_X86_64_CPU,
> +        .parent         = TYPE_X86_CPU,
> +        .abstract       = true,
> +    }, {

Should TYPE_I386_CPU/TYPE_X86_64_CPU be also wrapped with TARGET_X86_64?

Otherwise, we would keep the 32-bit CPU type definition of TYPE_I386_CPU
in the 64-bit case.

> +        .name           = X86_CPU_TYPE_NAME("base"),
> +#ifdef TARGET_X86_64
> +        .parent         = TYPE_X86_64_CPU,
> +#else
> +        .parent         = TYPE_I386_CPU,
> +#endif
>          .class_init     = x86_cpu_base_class_init,
>      }, {
>          .name           = X86_CPU_TYPE_NAME("max"),
> -        .parent         = TYPE_X86_CPU,
> +#ifdef TARGET_X86_64
> +        .parent         = TYPE_X86_64_CPU,
> +#else
> +        .parent         = TYPE_I386_CPU,
> +#endif
>          .instance_init  = max_x86_cpu_initfn,
>          .class_init     = max_x86_cpu_class_init,
>      }