|
| From: | Philippe Mathieu-Daudé |
| Subject: | Re: [PATCH v2 12/16] target: Move ArchCPUClass definition to 'cpu.h' |
| Date: | Mon, 6 Nov 2023 15:58:24 +0100 |
| User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 |
On 20/10/23 10:03, Zhao Liu wrote:
Hi Philippe, On Fri, Oct 13, 2023 at 04:01:11PM +0200, Philippe Mathieu-Daudé wrote:Date: Fri, 13 Oct 2023 16:01:11 +0200 From: Philippe Mathieu-Daudé <philmd@linaro.org> Subject: [PATCH v2 12/16] target: Move ArchCPUClass definition to 'cpu.h' X-Mailer: git-send-email 2.41.0 The OBJECT_DECLARE_CPU_TYPE() macro forward-declares each ArchCPUClass type. These forward declarations are sufficient for code in hw/ to use the QOM definitions. No need to expose these structure definitions. Keep each local to their target/ by moving them to the corresponding "cpu.h" header. Suggested-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- target/alpha/cpu-qom.h | 16 --------------- target/alpha/cpu.h | 13 +++++++++++++ target/arm/cpu-qom.h | 27 ------------------------- target/arm/cpu.h | 25 ++++++++++++++++++++++++ target/avr/cpu-qom.h | 16 --------------- target/avr/cpu.h | 14 +++++++++++++ target/cris/cpu-qom.h | 19 ------------------ target/cris/cpu.h | 16 +++++++++++++++ target/hexagon/cpu-qom.h | 1 - target/hppa/cpu-qom.h | 16 --------------- target/hppa/cpu.h | 14 +++++++++++++ target/i386/cpu-qom.h | 39 ------------------------------------- target/i386/cpu.h | 35 +++++++++++++++++++++++++++++++++ target/loongarch/cpu-qom.h | 1 - target/m68k/cpu-qom.h | 16 --------------- target/m68k/cpu.h | 13 +++++++++++++ target/microblaze/cpu-qom.h | 16 --------------- target/microblaze/cpu.h | 13 +++++++++++++ target/mips/cpu-qom.h | 20 ------------------- target/mips/cpu.h | 17 ++++++++++++++++ target/nios2/cpu-qom.h | 1 - target/openrisc/cpu-qom.h | 1 - target/riscv/cpu-qom.h | 16 +-------------- target/riscv/cpu.h | 14 +++++++++++++ target/rx/cpu-qom.h | 15 -------------- target/rx/cpu.h | 14 +++++++++++++ target/s390x/cpu-qom.h | 37 +---------------------------------- target/s390x/cpu.h | 30 ++++++++++++++++++++++++++++ target/s390x/cpu_models.h | 8 ++++---- target/sh4/cpu-qom.h | 23 ---------------------- target/sh4/cpu.h | 20 +++++++++++++++++++ target/sparc/cpu-qom.h | 18 ----------------- target/sparc/cpu.h | 18 +++++++++++++++-- target/tricore/cpu-qom.h | 10 ---------- target/tricore/cpu.h | 6 ++++++ target/xtensa/cpu-qom.h | 21 -------------------- target/xtensa/cpu.h | 20 +++++++++++++++++-- 37 files changed, 284 insertions(+), 335 deletions(-)
diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h index dffc74c1ce..d4e216d000 100644 --- a/target/i386/cpu-qom.h +++ b/target/i386/cpu-qom.h @@ -21,8 +21,6 @@ #define QEMU_I386_CPU_QOM_H#include "hw/core/cpu.h"-#include "qemu/notify.h" -#include "qom/object.h"#ifdef TARGET_X86_64#define TYPE_X86_CPU "x86_64-cpu" @@ -35,41 +33,4 @@ OBJECT_DECLARE_CPU_TYPE(X86CPU, X86CPUClass, X86_CPU) #define X86_CPU_TYPE_SUFFIX "-" TYPE_X86_CPU #define X86_CPU_TYPE_NAME(name) (name X86_CPU_TYPE_SUFFIX)-typedef struct X86CPUModel X86CPUModel;- -/** - * X86CPUClass: - * @cpu_def: CPU model definition - * @host_cpuid_required: Whether CPU model requires cpuid from host. - * @ordering: Ordering on the "-cpu help" CPU model list. - * @migration_safe: See CpuDefinitionInfo::migration_safe - * @static_model: See CpuDefinitionInfo::static - * @parent_realize: The parent class' realize handler. - * @parent_phases: The parent class' reset phase handlers. - * - * An x86 CPU model or family. - */ -struct X86CPUClass { - CPUClass parent_class; - - /* CPU definition, automatically loaded by instance_init if not NULL. - * Should be eventually replaced by subclass-specific property defaults. - */ - X86CPUModel *model; - - bool host_cpuid_required; - int ordering; - bool migration_safe; - bool static_model; - - /* Optional description of CPU model. - * If unavailable, cpu_def->model_id is used */ - const char *model_description; - - DeviceRealize parent_realize; - DeviceUnrealize parent_unrealize; - ResettablePhases parent_phases; -}; - - #endif diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 2dea4df086..e21d293daa 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -2037,6 +2037,41 @@ struct ArchCPU { bool xen_vapic; };+typedef struct X86CPUModel X86CPUModel;Could we "typedef" this structure at its definition?
No, because X86CPUClass uses the declaration in its 'model' field in [*], so we have to forward-declare it first.
Then this explicit "typedef" can also be omitted, just like you did elsewhere.+ +/** + * X86CPUClass: + * @cpu_def: CPU model definition + * @host_cpuid_required: Whether CPU model requires cpuid from host. + * @ordering: Ordering on the "-cpu help" CPU model list. + * @migration_safe: See CpuDefinitionInfo::migration_safe + * @static_model: See CpuDefinitionInfo::static + * @parent_realize: The parent class' realize handler. + * @parent_phases: The parent class' reset phase handlers. + * + * An x86 CPU model or family. + */ +struct X86CPUClass { + CPUClass parent_class; + + /* CPU definition, automatically loaded by instance_init if not NULL. + * Should be eventually replaced by subclass-specific property defaults. + */ + X86CPUModel *model;
[*] ^^^^^^^^^^^
+ bool host_cpuid_required; + int ordering; + bool migration_safe; + bool static_model; + + /* Optional description of CPU model. + * If unavailable, cpu_def->model_id is used */ + const char *model_description; + + DeviceRealize parent_realize; + DeviceUnrealize parent_unrealize; + ResettablePhases parent_phases; +};
diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h index 76efb614a6..35ca5c4600 100644 --- a/target/riscv/cpu-qom.h +++ b/target/riscv/cpu-qom.h @@ -20,7 +20,6 @@ #define RISCV_CPU_QOM_H#include "hw/core/cpu.h"-#include "qom/object.h"#define TYPE_RISCV_CPU "riscv-cpu"#define TYPE_RISCV_DYNAMIC_CPU "riscv-dynamic-cpu" @@ -44,21 +43,8 @@ #define TYPE_RISCV_CPU_VEYRON_V1 RISCV_CPU_TYPE_NAME("veyron-v1") #define TYPE_RISCV_CPU_HOST RISCV_CPU_TYPE_NAME("host")-typedef struct CPUArchState CPURISCVState;+typedef struct CPUArchState CPURISCVState; // XXXIs "// XXX" redundant?
Good catch, I forgot about this comment. I now simply moved that to "cpu.h".
OBJECT_DECLARE_CPU_TYPE(RISCVCPU, RISCVCPUClass, RISCV_CPU) -/**- * RISCVCPUClass: - * @parent_realize: The parent class' realize handler. - * @parent_phases: The parent class' reset phase handlers. - * - * A RISCV CPU model. - */ -struct RISCVCPUClass { - CPUClass parent_class; - - DeviceRealize parent_realize; - ResettablePhases parent_phases; -}; #endif /* RISCV_CPU_QOM_H */ diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index d832696418..a7edf95213 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -414,6 +414,20 @@ struct ArchCPU { GHashTable *pmu_event_ctr_map; };+/**+ * RISCVCPUClass: + * @parent_realize: The parent class' realize handler. + * @parent_phases: The parent class' reset phase handlers. + * + * A RISCV CPU model. + */ +struct RISCVCPUClass { + CPUClass parent_class; + + DeviceRealize parent_realize; + ResettablePhases parent_phases; +}; +
diff --git a/target/s390x/cpu-qom.h b/target/s390x/cpu-qom.h index fcd70daddf..4037e31f79 100644 --- a/target/s390x/cpu-qom.h +++ b/target/s390x/cpu-qom.h @@ -21,7 +21,6 @@ #define QEMU_S390_CPU_QOM_H#include "hw/core/cpu.h"-#include "qom/object.h"#define TYPE_S390_CPU "s390x-cpu" @@ -30,40 +29,6 @@ OBJECT_DECLARE_CPU_TYPE(S390CPU, S390CPUClass, S390_CPU)#define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU #define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)-typedef struct S390CPUModel S390CPUModel;-typedef struct S390CPUDef S390CPUDef; - -typedef struct CPUArchState CPUS390XState; - -typedef enum cpu_reset_type { - S390_CPU_RESET_NORMAL, - S390_CPU_RESET_INITIAL, - S390_CPU_RESET_CLEAR, -} cpu_reset_type; - -/** - * S390CPUClass: - * @parent_realize: The parent class' realize handler. - * @parent_reset: The parent class' reset handler. - * @load_normal: Performs a load normal. - * @cpu_reset: Performs a CPU reset. - * @initial_cpu_reset: Performs an initial CPU reset. - * - * An S/390 CPU model. - */ -struct S390CPUClass { - CPUClass parent_class; - - const S390CPUDef *cpu_def; - bool kvm_required; - bool is_static; - bool is_migration_safe; - const char *desc; - - DeviceRealize parent_realize; - DeviceReset parent_reset; - void (*load_normal)(CPUState *cpu); - void (*reset)(CPUState *cpu, cpu_reset_type type); -}; +typedef struct CPUArchState CPUS390XState; // XXXsame here.
Oops, I forgot to send the preliminary series required to remove that comment, see: https://lore.kernel.org/qemu-devel/20231106114500.5269-1-philmd@linaro.org/
Just the above nits. Otherwise, Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Thanks for your careful review!
| [Prev in Thread] | Current Thread | [Next in Thread] |