From patchwork Mon Nov 12 21:48:40 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eduardo Habkost X-Patchwork-Id: 198479 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EE8352C008F for ; Tue, 13 Nov 2012 08:47:56 +1100 (EST) Received: from localhost ([::1]:35069 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TY1rH-0002PM-6t for incoming@patchwork.ozlabs.org; Mon, 12 Nov 2012 16:47:55 -0500 Received: from eggs.gnu.org ([208.118.235.92]:58925) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TY1r5-0002Ot-6J for qemu-devel@nongnu.org; Mon, 12 Nov 2012 16:47:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TY1r2-0007jY-30 for qemu-devel@nongnu.org; Mon, 12 Nov 2012 16:47:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52648) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TY1r1-0007jU-Mx for qemu-devel@nongnu.org; Mon, 12 Nov 2012 16:47:39 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qACLlbqI000851 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 12 Nov 2012 16:47:37 -0500 Received: from blackpad.lan.raisama.net (vpn1-5-14.gru2.redhat.com [10.97.5.14]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qACLlaAQ012992; Mon, 12 Nov 2012 16:47:36 -0500 Received: by blackpad.lan.raisama.net (Postfix, from userid 500) id 465E4203417; Mon, 12 Nov 2012 19:48:48 -0200 (BRST) From: Eduardo Habkost To: qemu-devel@nongnu.org Date: Mon, 12 Nov 2012 19:48:40 -0200 Message-Id: <1352756920-14117-1-git-send-email-ehabkost@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Igor Mammedov , Don Slutz , =?UTF-8?q?Andreas=20F=C3=A4rber?= , Anthony Liguori Subject: [Qemu-devel] [RFC] target-i386: register a class for each CPU model X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org This creates the following class hierarchy: - TYPE_X86_CPU ("-cpu") - TYPE_X86_DEFCPU "-cpu-predefined": abstract base class for the predefined CPU models - "-cpu-model-": a class for each predefined CPU model - TYPE_X86_HOST_CPU ("-cpu-model-host"): class for the "-cpu host" CPU model On TARGET_X86_64, "" is "x86_64", on TARGET_I386, "" is "i386". The trick here is to replace exactly what's implemented in the cpu_x86_find_cpudef() logic and nothing else. So, the only difference in relation to the previous code is that instead of looking at the CPU model table on cpu_x86_find_cpudef(), we just use the right CPU class, that will fill the X86CPUDefinition struct on a ->init_cpudef() method. The init_cpudef() method was added jut to not require us to kill the X86CPUDefinition array in the same patch. But the X86CPUDefinition struct may eventually go away, and all the default-value initialization for each CPU model can become just different defaults set on instance_init() or class_init(). Signed-off-by: Eduardo Habkost --- I am using those class names because I don't want the CPU model names to live in the same namespace as all device names. I want to avoid doing the same mistake that was done in the arm code, that registers a QOM class named "any". Some CPU model names, like "qemu64", don't make any sense unless you already know it is a CPU model name. As an alternative to the complex naming above, I was considering simply using "cpu-" as the class name. I don't know what others think. This patch depends on the "[PATCH 00/17] target-i386: CPU init cleanup for CPU classes/properties" series I have just sent. Changes v1 -> v2: - Rebase on top of CPU properties series - Use a static type (with a different class init function) for the "-cpu host" class Changes v2 -> v3: - Fix the "device not found" error to use the CPU model name, not the full -cpu string - kill cpudef_init() and cpudef_setup(), as we don't need a cpudef-specific initialization function, anymore - Instead of keeping a copy of a X86CPUDefinition struct inside X86CPUClass, keep only a pointer - Add a init_cpudef() class method, used to fill the X86CPUDefinition struct for the CPU - Use separate subclass for "-cpu host", that uses cpu_x86_fill_host() on init_cpudef() Changes v3 -> v4: - Rebase on top of qemu.git master, without the CPU properties series - Rename CPU classes to "-cpu-model" --- arch_init.c | 7 -- arch_init.h | 1 - bsd-user/main.c | 3 - linux-user/main.c | 3 - target-i386/cpu-qom.h | 22 +++++- target-i386/cpu.c | 208 +++++++++++++++++++++++++++++++++++++++++--------- target-i386/cpu.h | 2 - vl.c | 7 -- 8 files changed, 192 insertions(+), 61 deletions(-) diff --git a/arch_init.c b/arch_init.c index e6effe8..bea1b9c 100644 --- a/arch_init.c +++ b/arch_init.c @@ -1124,13 +1124,6 @@ void do_smbios_option(const char *optarg) #endif } -void cpudef_init(void) -{ -#if defined(cpudef_setup) - cpudef_setup(); /* parse cpu definitions in target config file */ -#endif -} - int audio_available(void) { #ifdef HAS_AUDIO diff --git a/arch_init.h b/arch_init.h index 5fc780c..84a7f9a 100644 --- a/arch_init.h +++ b/arch_init.h @@ -27,7 +27,6 @@ extern const uint32_t arch_type; void select_soundhw(const char *optarg); void do_acpitable_option(const char *optarg); void do_smbios_option(const char *optarg); -void cpudef_init(void); int audio_available(void); void audio_init(ISABus *isa_bus, PCIBus *pci_bus); int tcg_available(void); diff --git a/bsd-user/main.c b/bsd-user/main.c index 095ae8e..cf0a345 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -762,9 +762,6 @@ int main(int argc, char **argv) } cpu_model = NULL; -#if defined(cpudef_setup) - cpudef_setup(); /* parse cpu definitions in target config file (TBD) */ -#endif optind = 1; for(;;) { diff --git a/linux-user/main.c b/linux-user/main.c index 25e35cd..e881fcf 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3426,9 +3426,6 @@ int main(int argc, char **argv, char **envp) } cpu_model = NULL; -#if defined(cpudef_setup) - cpudef_setup(); /* parse cpu definitions in target config file (TBD) */ -#endif /* init debug */ cpu_set_log_filename(log_file); diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index 5901140..01ea9de 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -37,19 +37,37 @@ #define X86_CPU_GET_CLASS(obj) \ OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU) + +struct X86CPUDefinition; +typedef struct X86CPUDefinition X86CPUDefinition; + +struct X86CPUClass; +typedef struct X86CPUClass X86CPUClass; + /** * X86CPUClass: * @parent_reset: The parent class' reset handler. + * @init_cpudef: initialize X86CPUDefinition struct for CPU class * * An x86 CPU model or family. */ -typedef struct X86CPUClass { +struct X86CPUClass { /*< private >*/ CPUClass parent_class; /*< public >*/ void (*parent_reset)(CPUState *cpu); -} X86CPUClass; + + /* Init a X86CPUDefinition struct. + * This method should eventually go away when we make all info from + * X86CPUDefinition be settable using properties inside instance_init(). + * + * We can't just store a X86CPUDefinition pointer on the X86CPUClass + * struct because the "-cpu host" class needs KVM to be initialized, + * to probe host CPU/KVM capabilities. + */ + int (*init_cpudef)(X86CPUClass *xcc, X86CPUDefinition *def, Error **errp); +}; /** * X86CPU: diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 5f2ce7d..7b13987 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -271,7 +271,11 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, fprintf(stderr, "CPU feature %s not found\n", flagname); } -typedef struct X86CPUDefinition { +/* This struct should eventually go away and just become multiple series of + * properties and values to set by default on CPU objects, one for each CPU + * class. + */ +struct X86CPUDefinition { struct X86CPUDefinition *next; const char *name; uint32_t level; @@ -290,7 +294,7 @@ typedef struct X86CPUDefinition { uint32_t xlevel2; /* The feature bits on CPUID[EAX=7,ECX=0].EBX */ uint32_t cpuid_7_0_ebx_features; -} X86CPUDefinition; +}; #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ @@ -1146,33 +1150,6 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque, cpu->env.tsc_khz = value / 1000; } -/* Find a CPU model definition and put the result on a X86CPUDefinition struct - */ -static int cpu_x86_find_cpudef(const char *name, - X86CPUDefinition *result, - Error **errp) -{ - X86CPUDefinition *def; - - for (def = x86_defs; def; def = def->next) - if (name && !strcmp(name, def->name)) - break; - if (kvm_enabled() && name && strcmp(name, "host") == 0) { - kvm_cpu_fill_host(result); - } else if (!def) { - goto error; - } else { - memcpy(result, def, sizeof(*def)); - } - return 0; - -error: - if (!error_is_set(errp)) { - error_set(errp, QERR_INVALID_PARAMETER_COMBINATION); - } - return -1; -} - static int cpu_x86_parse_featurestr(X86CPUDefinition *x86_cpu_def, char *features, Error **errp) { @@ -1503,29 +1480,54 @@ static int cpudef_2_x86_cpu(X86CPU *cpu, X86CPUDefinition *def, Error **errp) return 0; } +/* Build class name for specific CPU model: + * + * The CPU class names are in the form -cpu-model-. + */ + +/* For statically-defined class names (e.g. TYPE_X86_CPU_HOST) */ +#define STATIC_CPU_CLASS_NAME(model) (TYPE_X86_CPU "-model-" model) + +/* For dynamically-defined class names (built from the cpudef array) */ +static char *build_cpu_class_name(const char *model) +{ + return g_strdup_printf("%s-model-%s", TYPE_X86_CPU, model); +} + X86CPU *cpu_x86_init(const char *cpu_string) { X86CPU *cpu = NULL; + ObjectClass *obj_class; + X86CPUClass *cpu_class; CPUX86State *env; Error *error = NULL; X86CPUDefinition def; - char *name, *features; + char *model_name, *features; gchar **model_pieces; + char *class_name = NULL; model_pieces = g_strsplit(cpu_string, ",", 2); if (!model_pieces[0]) { goto error; } - name = model_pieces[0]; + model_name = model_pieces[0]; features = model_pieces[1]; - cpu = X86_CPU(object_new(TYPE_X86_CPU)); + class_name = build_cpu_class_name(model_name); + obj_class = object_class_by_name(class_name); + if (!obj_class) { + error_set(&error, QERR_DEVICE_NOT_FOUND, model_name); + goto error; + } + + cpu = X86_CPU(object_new(class_name)); env = &cpu->env; env->cpu_model_str = cpu_string; memset(&def, 0, sizeof(def)); - if (cpu_x86_find_cpudef(name, &def, &error) < 0) { + cpu_class = X86_CPU_GET_CLASS(cpu); + if (cpu_class->init_cpudef(cpu_class, &def, &error) < 0) { goto error; } @@ -1543,9 +1545,11 @@ X86CPU *cpu_x86_init(const char *cpu_string) } g_strfreev(model_pieces); + g_free(class_name); return cpu; error: g_strfreev(model_pieces); + g_free(class_name); if (cpu) { object_delete(OBJECT(cpu)); } @@ -1567,7 +1571,7 @@ void cpu_clear_apic_feature(CPUX86State *env) /* Initialize list of CPU models, filling some non-static fields if necessary */ -void x86_cpudef_setup(void) +static void x86_cpudef_setup(void) { int i, j; static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" }; @@ -2123,15 +2127,147 @@ static const TypeInfo x86_cpu_type_info = { .name = TYPE_X86_CPU, .parent = TYPE_CPU, .instance_size = sizeof(X86CPU), - .instance_init = x86_cpu_initfn, - .abstract = false, + .abstract = true, .class_size = sizeof(X86CPUClass), .class_init = x86_cpu_common_class_init, }; +/* TYPE_X86_DEFCPU: abstract class for predefined CPU models */ + +#define TYPE_X86_DEFCPU (TYPE_X86_CPU "-predefined") + +#define X86_DEFCPU_CLASS(klass) \ + OBJECT_CLASS_CHECK(X86DEFCPUClass, (klass), TYPE_X86_DEFCPU) +#define X86_DEFCPU(obj) \ + OBJECT_CHECK(X86DEFCPU, (obj), TYPE_X86_DEFCPU) +#define X86_DEFCPU_GET_CLASS(obj) \ + OBJECT_GET_CLASS(X86DEFCPUClass, (obj), TYPE_X86_DEFCPU) + +/** + * X86DEFCPUClass: + * @cpudef: pointer to X86CPUDefinition for CPU model + * + * A predefined CPU model, pointing to a X86CPUDefinition struct. + */ +typedef struct X86DEFCPUClass { + /*< private >*/ + X86CPUClass parent_class; + + /*< public >*/ + X86CPUDefinition *cpudef; +} X86DEFCPUClass; + +/* The init_cpudef() method for children of TYPE_X86_DEFCPU */ +static int x86_predef_cpu_init_cpudef(X86CPUClass *xcc, X86CPUDefinition *def, + Error **errp) +{ + X86DEFCPUClass *defclass = X86_DEFCPU_CLASS(xcc); + memcpy(def, defclass->cpudef, sizeof(X86CPUDefinition)); + return 0; +} + +/* The class_init() method for children of TYPE_X86_DEFCPU + * + * Note that this is the class_init() method for _children_ of TYPE_X86_DEFCPU, + * not for TYPE_X86_DEFCPU itself, because we need the class-specific + * class_data pointer. + */ +static void x86_predef_cpu_class_init(ObjectClass *oc, void *data) +{ + X86CPUClass *xcc = X86_CPU_CLASS(oc); + X86DEFCPUClass *xdcc = X86_DEFCPU_CLASS(xcc); + X86CPUDefinition *def = data; + + xcc->init_cpudef = x86_predef_cpu_init_cpudef; + xdcc->cpudef = def; +} + +static TypeInfo x86_predef_cpu_type_info = { + .name = TYPE_X86_DEFCPU, + .parent = TYPE_X86_CPU, + .abstract = true, + .class_size = sizeof(X86DEFCPUClass), +}; + +/* Register a CPU class for a specific X86CPUDefinintion + * + * The class will be a child of TYPE_X86_DEFCPU. + */ +static void x86_cpu_register_class(const char *name, X86CPUDefinition *def) +{ + char *class_name = build_cpu_class_name(name); + TypeInfo type = { + .name = class_name, + .parent = TYPE_X86_DEFCPU, + .instance_size = sizeof(X86CPU), + .instance_init = x86_cpu_initfn, + .class_size = sizeof(X86DEFCPUClass), + .class_init = x86_predef_cpu_class_init, + .class_data = def, + }; + type_register(&type); + g_free(class_name); +} + + +/* TYPE_X86_HOST_CPU: + * + * X86CPU subclass for "-cpu host" + */ + +#define TYPE_X86_HOST_CPU STATIC_CPU_CLASS_NAME("host") + +/* The init_cpudef() method for TYPE_X86_HOST_CPU */ +static int x86_host_cpu_init_cpudef(X86CPUClass *xcc, X86CPUDefinition *def, + Error **errp) +{ + if (!kvm_enabled()) { + error_set(errp, ERROR_CLASS_GENERIC_ERROR, + "-cpu host is available only when using KVM"); + return -1; + } + memset(def, 0, sizeof(*def)); + kvm_cpu_fill_host(def); + return 0; +} + +/* The class_init() method for TYPE_X86_HOST_CPU */ +static void x86_cpu_host_class_init(ObjectClass *oc, void *data) +{ + X86CPUClass *xcc = X86_CPU_CLASS(oc); + + xcc->init_cpudef = x86_host_cpu_init_cpudef; +} + +static TypeInfo x86_cpu_host_type_info = { + .name = TYPE_X86_HOST_CPU, + .parent = TYPE_X86_CPU, + .instance_size = sizeof(X86CPU), + .instance_init = x86_cpu_initfn, + .class_size = sizeof(X86CPUClass), + .class_init = x86_cpu_host_class_init, +}; + static void x86_cpu_register_types(void) { + X86CPUDefinition *def; + + x86_cpudef_setup(); + + /* Abstract X86CPU class */ type_register_static(&x86_cpu_type_info); + + /* -cpu host class */ + type_register_static(&x86_cpu_host_type_info); + + /* Abstract class for predefined CPU models */ + type_register_static(&x86_predef_cpu_type_info); + + /* One class for each CPU model: */ + for (def = x86_defs; def; def = def->next) { + x86_cpu_register_class(def->name, def); + } + } type_init(x86_cpu_register_types) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 4d5510e..dba4f66 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -841,7 +841,6 @@ typedef struct CPUX86State { X86CPU *cpu_x86_init(const char *cpu_model); int cpu_x86_exec(CPUX86State *s); void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf); -void x86_cpudef_setup(void); int cpu_x86_support_mca_broadcast(CPUX86State *env); int cpu_get_pic_interrupt(CPUX86State *s); @@ -1022,7 +1021,6 @@ static inline CPUX86State *cpu_init(const char *cpu_model) #define cpu_gen_code cpu_x86_gen_code #define cpu_signal_handler cpu_x86_signal_handler #define cpu_list x86_cpu_list -#define cpudef_setup x86_cpudef_setup #define CPU_SAVE_VERSION 12 diff --git a/vl.c b/vl.c index 4f03a72..997d92a 100644 --- a/vl.c +++ b/vl.c @@ -3508,13 +3508,6 @@ int main(int argc, char **argv, char **envp) qemu_set_version(machine->hw_version); } - /* Init CPU def lists, based on config - * - Must be called after all the qemu_read_config_file() calls - * - Must be called before list_cpus() - * - Must be called before machine->init() - */ - cpudef_init(); - if (cpu_model && is_help_option(cpu_model)) { list_cpus(stdout, &fprintf, cpu_model); exit(0);