From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= Subject: Re: [Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses Date: Mon, 04 Feb 2013 13:52:32 +0100 Message-ID: <510FAF10.3050505@suse.de> References: <1359765428-27805-1-git-send-email-afaerber@suse.de> <1359765428-27805-4-git-send-email-afaerber@suse.de> <20130204120856.571533d5@nial.usersys.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: qemu-devel@nongnu.org, ehabkost@redhat.com, Gleb Natapov , Marcelo Tosatti , blauwirbel@gmail.com, anthony@codemonkey.ws, X86 To: Igor Mammedov Return-path: Received: from cantor2.suse.de ([195.135.220.15]:49566 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752478Ab3BDMwh (ORCPT ); Mon, 4 Feb 2013 07:52:37 -0500 In-Reply-To: <20130204120856.571533d5@nial.usersys.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Am 04.02.2013 12:08, schrieb Igor Mammedov: > On Sat, 2 Feb 2013 01:37:07 +0100 > Andreas F=E4rber wrote: >=20 >> Move x86_def_t definition to header and embed into X86CPUClass. >> Register types per built-in model definition. >> >> Move version initialization from x86_cpudef_setup() to class_init. >> >> Inline cpu_x86_register() into the X86CPU initfn. >> Since instance_init cannot reports errors, drop error handling. >> >> Replace cpu_x86_find_by_name() with x86_cpu_class_by_name(). >> Move KVM host vendor override from cpu_x86_find_by_name() to the ini= tfn. >> >> Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs. >> Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_= id(). >> >> Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu f= or >> comparison. >> >> Signed-off-by: Andreas F=E4rber >> --- >> target-i386/cpu-qom.h | 24 ++++ >> target-i386/cpu.c | 324 >> +++++++++++++++++-------------------------------- target-i386/cpu.h >> | 2 - target-i386/kvm.c | 93 ++++++++++++++ >> 4 Dateien ge=E4ndert, 228 Zeilen hinzugef=FCgt(+), 215 Zeilen entfe= rnt(-) >> >> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h >> index 48e6b54..80bf72d 100644 >> --- a/target-i386/cpu-qom.h >> +++ b/target-i386/cpu-qom.h >> @@ -30,6 +30,27 @@ >> #define TYPE_X86_CPU "i386-cpu" >> #endif >> =20 >> +#define TYPE_HOST_X86_CPU "host-" TYPE_X86_CPU >> + >> +typedef struct x86_def_t { >> + const char *name; >> + uint32_t level; >> + /* vendor is zero-terminated, 12 character ASCII string */ >> + char vendor[CPUID_VENDOR_SZ + 1]; >> + int family; >> + int model; >> + int stepping; >> + uint32_t features, ext_features, ext2_features, ext3_features; >> + uint32_t kvm_features, svm_features; >> + uint32_t xlevel; >> + char model_id[48]; >> + /* Store the results of Centaur's CPUID instructions */ >> + uint32_t ext4_features; >> + uint32_t xlevel2; >> + /* The feature bits on CPUID[EAX=3D7,ECX=3D0].EBX */ >> + uint32_t cpuid_7_0_ebx_features; >> +} x86_def_t; >> + >> #define X86_CPU_CLASS(klass) \ >> OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU) >> #define X86_CPU(obj) \ >> @@ -41,6 +62,7 @@ >> * X86CPUClass: >> * @parent_realize: The parent class' realize handler. >> * @parent_reset: The parent class' reset handler. >> + * @info: Model-specific data. >> * >> * An x86 CPU model or family. >> */ >> @@ -51,6 +73,8 @@ typedef struct X86CPUClass { >> =20 >> DeviceRealize parent_realize; >> void (*parent_reset)(CPUState *cpu); >> + >> + x86_def_t info; > Is it necessary to embed it here, could pointer to corresponding stat= ic array > be used here? > That way one could avoid extra memcpy in class_init(). That would require an allocation for -cpu host, which is undesired. x86_def_t is supposed to die, forgot to add a TODO comment like on ppc. Patches to do that are being being polished on qom-cpu-ppc. >> } X86CPUClass; >> =20 >> /** >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index ee2fd6b..6c95740 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -346,25 +346,6 @@ static void add_flagname_to_bitmaps(const char >> *flagname, } >> } >> =20 >> -typedef struct x86_def_t { >> - const char *name; >> - uint32_t level; >> - /* vendor is zero-terminated, 12 character ASCII string */ >> - char vendor[CPUID_VENDOR_SZ + 1]; >> - int family; >> - int model; >> - int stepping; >> - uint32_t features, ext_features, ext2_features, ext3_features; >> - uint32_t kvm_features, svm_features; >> - uint32_t xlevel; >> - char model_id[48]; >> - /* Store the results of Centaur's CPUID instructions */ >> - uint32_t ext4_features; >> - uint32_t xlevel2; >> - /* The feature bits on CPUID[EAX=3D7,ECX=3D0].EBX */ >> - uint32_t cpuid_7_0_ebx_features; >> -} x86_def_t; >> - >> #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) >> #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ >> CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_API= C) >> @@ -868,86 +849,6 @@ static x86_def_t builtin_x86_defs[] =3D { >> }, >> }; >> =20 >> -#ifdef CONFIG_KVM >> -static int cpu_x86_fill_model_id(char *str) >> -{ >> - uint32_t eax =3D 0, ebx =3D 0, ecx =3D 0, edx =3D 0; >> - int i; >> - >> - for (i =3D 0; i < 3; i++) { >> - host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx); >> - memcpy(str + i * 16 + 0, &eax, 4); >> - memcpy(str + i * 16 + 4, &ebx, 4); >> - memcpy(str + i * 16 + 8, &ecx, 4); >> - memcpy(str + i * 16 + 12, &edx, 4); >> - } >> - return 0; >> -} >> -#endif >> - >> -/* Fill a x86_def_t struct with information about the host CPU, and >> - * the CPU features supported by the host hardware + host kernel >> - * >> - * This function may be called only if KVM is enabled. >> - */ >> -static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) >> -{ >> -#ifdef CONFIG_KVM >> - KVMState *s =3D kvm_state; >> - uint32_t eax =3D 0, ebx =3D 0, ecx =3D 0, edx =3D 0; >> - >> - assert(kvm_enabled()); >> - >> - x86_cpu_def->name =3D "host"; >> - host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); >> - x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); >> - >> - host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); >> - x86_cpu_def->family =3D ((eax >> 8) & 0x0F) + ((eax >> 20) & 0x= =46F); >> - x86_cpu_def->model =3D ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >= > 12); >> - x86_cpu_def->stepping =3D eax & 0x0F; >> - >> - x86_cpu_def->level =3D kvm_arch_get_supported_cpuid(s, 0x0, 0, = R_EAX); >> - x86_cpu_def->features =3D kvm_arch_get_supported_cpuid(s, 0x1, = 0, R_EDX); >> - x86_cpu_def->ext_features =3D kvm_arch_get_supported_cpuid(s, 0= x1, 0, >> R_ECX); - >> - if (x86_cpu_def->level >=3D 7) { >> - x86_cpu_def->cpuid_7_0_ebx_features =3D >> - kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX); >> - } else { >> - x86_cpu_def->cpuid_7_0_ebx_features =3D 0; >> - } >> - >> - x86_cpu_def->xlevel =3D kvm_arch_get_supported_cpuid(s, 0x80000= 000, 0, >> R_EAX); >> - x86_cpu_def->ext2_features =3D >> - kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ED= X); >> - x86_cpu_def->ext3_features =3D >> - kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EC= X); >> - >> - cpu_x86_fill_model_id(x86_cpu_def->model_id); >> - >> - /* Call Centaur's CPUID instruction. */ >> - if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) { >> - host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx); >> - eax =3D kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EA= X); >> - if (eax >=3D 0xC0000001) { >> - /* Support VIA max extended level */ >> - x86_cpu_def->xlevel2 =3D eax; >> - host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx); >> - x86_cpu_def->ext4_features =3D >> - kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, = R_EDX); >> - } >> - } >> - >> - /* Other KVM-specific feature fields: */ >> - x86_cpu_def->svm_features =3D >> - kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX); >> - x86_cpu_def->kvm_features =3D >> - kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EA= X); >> - >> -#endif /* CONFIG_KVM */ >> -} >> - >> static int unavailable_host_feature(FeatureWordInfo *f, uint32_t ma= sk) >> { >> int i; >> @@ -975,31 +876,31 @@ static int unavailable_host_feature(FeatureWor= dInfo >> *f, uint32_t mask) static int kvm_check_features_against_host(X86CPU= *cpu) >> { >> CPUX86State *env =3D &cpu->env; >> - x86_def_t host_def; >> + ObjectClass *host_oc =3D object_class_by_name(TYPE_HOST_X86_CPU= ); >> + X86CPUClass *host_xcc =3D X86_CPU_CLASS(host_oc); >> uint32_t mask; >> int rv, i; >> struct model_features_t ft[] =3D { >> - {&env->cpuid_features, &host_def.features, >> + {&env->cpuid_features, &host_xcc->info.features, >> FEAT_1_EDX }, >> - {&env->cpuid_ext_features, &host_def.ext_features, >> + {&env->cpuid_ext_features, &host_xcc->info.ext_features, >> FEAT_1_ECX }, >> - {&env->cpuid_ext2_features, &host_def.ext2_features, >> + {&env->cpuid_ext2_features, &host_xcc->info.ext2_features, >> FEAT_8000_0001_EDX }, >> - {&env->cpuid_ext3_features, &host_def.ext3_features, >> + {&env->cpuid_ext3_features, &host_xcc->info.ext3_features, >> FEAT_8000_0001_ECX }, >> - {&env->cpuid_ext4_features, &host_def.ext4_features, >> + {&env->cpuid_ext4_features, &host_xcc->info.ext4_features, >> FEAT_C000_0001_EDX }, >> - {&env->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_feat= ures, >> + {&env->cpuid_7_0_ebx_features, >> &host_xcc->info.cpuid_7_0_ebx_features, FEAT_7_0_EBX }, >> - {&env->cpuid_svm_features, &host_def.svm_features, >> + {&env->cpuid_svm_features, &host_xcc->info.svm_features, >> FEAT_SVM }, >> - {&env->cpuid_kvm_features, &host_def.kvm_features, >> + {&env->cpuid_kvm_features, &host_xcc->info.kvm_features, >> FEAT_KVM }, >> }; >> =20 >> assert(kvm_enabled()); >> =20 >> - kvm_cpu_fill_host(&host_def); >> for (rv =3D 0, i =3D 0; i < ARRAY_SIZE(ft); ++i) { >> FeatureWord w =3D ft[i].feat_word; >> FeatureWordInfo *wi =3D &feature_word_info[w]; >> @@ -1261,40 +1162,30 @@ static void x86_cpuid_set_tsc_freq(Object *o= bj, >> Visitor *v, void *opaque, cpu->env.tsc_khz =3D value / 1000; >> } >> =20 >> -static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char = *name) >> +static ObjectClass *x86_cpu_class_by_name(const char *name) >> { >> - x86_def_t *def; >> - int i; >> + ObjectClass *oc; >> + char *typename; >> =20 >> if (name =3D=3D NULL) { >> - return -1; >> - } >> - if (kvm_enabled() && strcmp(name, "host") =3D=3D 0) { >> - kvm_cpu_fill_host(x86_cpu_def); >> - return 0; >> + return NULL; >> } >> =20 >> - for (i =3D 0; i < ARRAY_SIZE(builtin_x86_defs); i++) { >> - def =3D &builtin_x86_defs[i]; >> - if (strcmp(name, def->name) =3D=3D 0) { >> - memcpy(x86_cpu_def, def, sizeof(*def)); >> - /* sysenter isn't supported in compatibility mode on AM= D, >> - * syscall isn't supported in compatibility mode on Int= el. >> - * Normally we advertise the actual CPU vendor, but you= can >> - * override this using the 'vendor' property if you wan= t to use >> - * KVM's sysenter/syscall emulation in compatibility mo= de and >> - * when doing cross vendor migration >> - */ >> - if (kvm_enabled()) { >> - uint32_t ebx =3D 0, ecx =3D 0, edx =3D 0; >> - host_cpuid(0, 0, NULL, &ebx, &ecx, &edx); >> - x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, = edx, >> ecx); >> - } >> - return 0; >> + if (strcmp(name, "host") =3D=3D 0) { >> + if (kvm_enabled()) { >> + return object_class_by_name(TYPE_HOST_X86_CPU); >> } >> + return NULL; >> } > block is not necessary, the following block could yield the same resu= lt. No, it doesn't in the !kvm_enabled() case. I tripped over this when I left the kvm_enabled() check where it was before, falling back to the below lookup, leading to -cpu host being instantiated for TCG and originally not asserting (that's why I added the -cpu host initfn). Symptoms were x86_64 openSUSE .iso claiming no support for 486(?) processors. >> =20 >> - return -1; >> + typename =3D g_strdup_printf("%s-" TYPE_X86_CPU, name); >> + oc =3D object_class_by_name(typename); >> + g_free(typename); >> + if (oc !=3D NULL && (!object_class_dynamic_cast(oc, TYPE_X86_CP= U) || >> + object_class_is_abstract(oc))) { >> + oc =3D NULL; >> + } >> + return oc; >> } >> =20 >> /* Parse "+feature,-feature,feature=3Dfoo" CPU feature string >> @@ -1516,60 +1407,13 @@ static void filter_features_for_kvm(X86CPU *= cpu) >> } >> #endif >> =20 >> -static int cpu_x86_register(X86CPU *cpu, const char *name) >> -{ >> - CPUX86State *env =3D &cpu->env; >> - x86_def_t def1, *def =3D &def1; >> - Error *error =3D NULL; >> - >> - memset(def, 0, sizeof(*def)); >> - >> - if (cpu_x86_find_by_name(def, name) < 0) { >> - error_setg(&error, "Unable to find CPU definition: %s", nam= e); >> - goto out; >> - } >> - >> - if (kvm_enabled()) { >> - def->kvm_features |=3D kvm_default_features; >> - } >> - def->ext_features |=3D CPUID_EXT_HYPERVISOR; >> - >> - object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &er= ror); >> - object_property_set_int(OBJECT(cpu), def->level, "level", &erro= r); >> - object_property_set_int(OBJECT(cpu), def->family, "family", &er= ror); >> - object_property_set_int(OBJECT(cpu), def->model, "model", &erro= r); >> - object_property_set_int(OBJECT(cpu), def->stepping, "stepping", >> &error); >> - env->cpuid_features =3D def->features; >> - env->cpuid_ext_features =3D def->ext_features; >> - env->cpuid_ext2_features =3D def->ext2_features; >> - env->cpuid_ext3_features =3D def->ext3_features; >> - object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &er= ror); >> - env->cpuid_kvm_features =3D def->kvm_features; >> - env->cpuid_svm_features =3D def->svm_features; >> - env->cpuid_ext4_features =3D def->ext4_features; >> - env->cpuid_7_0_ebx_features =3D def->cpuid_7_0_ebx_features; >> - env->cpuid_xlevel2 =3D def->xlevel2; >> - >> - object_property_set_str(OBJECT(cpu), def->model_id, "model-id", >> &error); >> - if (error) { >> - goto out; >> - } >> - >> -out: >> - if (error) { >> - fprintf(stderr, "%s\n", error_get_pretty(error)); >> - error_free(error); >> - return -1; >> - } >> - return 0; >> -} >> - >> X86CPU *cpu_x86_init(const char *cpu_model) >> { >> X86CPU *cpu =3D NULL; >> CPUX86State *env; >> gchar **model_pieces; >> char *name, *features; >> + ObjectClass *oc; >> Error *error =3D NULL; >> =20 >> model_pieces =3D g_strsplit(cpu_model, ",", 2); >> @@ -1580,13 +1424,13 @@ X86CPU *cpu_x86_init(const char *cpu_model) >> name =3D model_pieces[0]; >> features =3D model_pieces[1]; >> =20 >> - cpu =3D X86_CPU(object_new(TYPE_X86_CPU)); >> - env =3D &cpu->env; >> - env->cpu_model_str =3D cpu_model; >> - >> - if (cpu_x86_register(cpu, name) < 0) { >> + oc =3D x86_cpu_class_by_name(name); >> + if (oc =3D=3D NULL) { > make sure error is reported when cpu is not found: > error_setg(&error, "Can't find CPU: %s", name); Why? The callers do that when NULL is returned. >> goto error; >> } >> + cpu =3D X86_CPU(object_new(object_class_get_name(oc))); >> + env =3D &cpu->env; >> + env->cpu_model_str =3D cpu_model; >> =20 >> cpu_x86_parse_featurestr(cpu, features, &error); >> if (error) { >> @@ -1621,30 +1465,6 @@ void cpu_clear_apic_feature(CPUX86State *env) >> =20 >> #endif /* !CONFIG_USER_ONLY */ >> =20 >> -/* Initialize list of CPU models, filling some non-static fields if >> necessary >> - */ >> -void x86_cpudef_setup(void) >> -{ >> - int i, j; >> - static const char *model_with_versions[] =3D { "qemu32", "qemu6= 4", >> "athlon" }; - >> - for (i =3D 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) { >> - x86_def_t *def =3D &builtin_x86_defs[i]; >> - >> - /* Look for specific "cpudef" models that */ >> - /* have the QEMU version in .model_id */ >> - for (j =3D 0; j < ARRAY_SIZE(model_with_versions); j++) { >> - if (strcmp(model_with_versions[j], def->name) =3D=3D 0)= { >> - pstrcpy(def->model_id, sizeof(def->model_id), >> - "QEMU Virtual CPU version "); >> - pstrcat(def->model_id, sizeof(def->model_id), >> - qemu_get_version()); >> - break; >> - } >> - } >> - } >> -} >> - >> static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx, >> uint32_t *ecx, uint32_t *edx) >> { >> @@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj) >> CPUState *cs =3D CPU(obj); >> X86CPU *cpu =3D X86_CPU(obj); >> CPUX86State *env =3D &cpu->env; >> + X86CPUClass *xcc =3D X86_CPU_GET_CLASS(obj); >> + const x86_def_t *def =3D &xcc->info; >> static int inited; >> =20 >> cpu_exec_init(env); >> @@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj) >> x86_cpuid_get_tsc_freq, >> x86_cpuid_set_tsc_freq, NULL, NULL, NULL); >> =20 >> + /* sysenter isn't supported in compatibility mode on AMD, >> + * syscall isn't supported in compatibility mode on Intel. >> + * Normally we advertise the actual CPU vendor, but you can >> + * override this using the 'vendor' property if you want to use >> + * KVM's sysenter/syscall emulation in compatibility mode and >> + * when doing cross vendor migration >> + */ >> + if (kvm_enabled()) { >> + host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_ven= dor2, >> + &env->cpuid_vendor3); > This is not per instance specific, this override would be better done= to > sub-classes in kvm_arch_init(). Hmm... I think the issue I addresses this way was that kvm.c didn't hav= e access to your static vendor_words2str() helper. Writing the words into the word fields is more direct than converting them to a string and writing them back into the words. > As bonus we would get actual 'vendor' when > doing class introspection provided it's done after kvm_init() is call= ed. Doing that would mean that we force class_init of every CPU class to ru= n every time we start with KVM enabled. Is there any practical downside t= o doing it at instance_init time? Properties are set afterwards, so it doesn't override anything. Where would we want to inspect an X86CPUClas= s to have an AMD model say Intel or vice versa? -cpu ? doesn't look at it today, nor does QMP. >> + } else { >> + object_property_set_str(OBJECT(cpu), def->vendor, "vendor",= NULL); >> + } >> + >> + object_property_set_int(OBJECT(cpu), def->level, "level", NULL)= ; >> + object_property_set_int(OBJECT(cpu), def->family, "family", NUL= L); >> + object_property_set_int(OBJECT(cpu), def->model, "model", NULL)= ; >> + object_property_set_int(OBJECT(cpu), def->stepping, "stepping",= NULL); >> + env->cpuid_features =3D def->features; >> + env->cpuid_ext_features =3D def->ext_features; >> + env->cpuid_ext_features |=3D CPUID_EXT_HYPERVISOR; >> + env->cpuid_ext2_features =3D def->ext2_features; >> + env->cpuid_ext3_features =3D def->ext3_features; >> + object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", NUL= L); >> + env->cpuid_kvm_features =3D def->kvm_features; >> + if (kvm_enabled()) { >> + env->cpuid_kvm_features |=3D kvm_default_features; >> + } >> + env->cpuid_svm_features =3D def->svm_features; >> + env->cpuid_ext4_features =3D def->ext4_features; >> + env->cpuid_7_0_ebx_features =3D def->cpuid_7_0_ebx_features; >> + env->cpuid_xlevel2 =3D def->xlevel2; >> + >> + object_property_set_str(OBJECT(cpu), def->model_id, "model-id",= NULL); > All this feature initialization in initfn() is not compatible with > static/global properties because they are set in device_initfn(). But= I can > remove properties/features from here as they are converted to static > properties and incrementally move their defaults initialization into > new x86_cpu_def_class_init(). Would it help to call the old registration function from here? Or what exactly would you need on top? >> + >> env->cpuid_apic_id =3D x86_cpu_apic_id_from_index(cs->cpu_index= ); >> =20 >> /* init various static tables used in TCG mode */ >> @@ -2239,6 +2096,27 @@ static void x86_cpu_initfn(Object *obj) >> } >> } >> =20 >> +static void x86_cpu_def_class_init(ObjectClass *oc, void *data) >> +{ >> + X86CPUClass *xcc =3D X86_CPU_CLASS(oc); >> + x86_def_t *def =3D data; >> + int i; >> + static const char *versioned_models[] =3D { "qemu32", "qemu64", >> "athlon" }; + >> + memcpy(&xcc->info, def, sizeof(x86_def_t)); > perhaps sizeof(xcc->info) would be better? OK. >> + >> + /* Look for specific models that have the QEMU version in .mode= l_id */ >> + for (i =3D 0; i < ARRAY_SIZE(versioned_models); i++) { >> + if (strcmp(versioned_models[i], def->name) =3D=3D 0) { >> + pstrcpy(xcc->info.model_id, sizeof(xcc->info.model_id), >> + "QEMU Virtual CPU version "); >> + pstrcat(xcc->info.model_id, sizeof(xcc->info.model_id), >> + qemu_get_version()); >> + break; >> + } >> + } >> +} >> + >> static void x86_cpu_common_class_init(ObjectClass *oc, void *data) >> { >> X86CPUClass *xcc =3D X86_CPU_CLASS(oc); >> @@ -2250,6 +2128,21 @@ static void x86_cpu_common_class_init(ObjectC= lass >> *oc, void *data)=20 >> xcc->parent_reset =3D cc->reset; >> cc->reset =3D x86_cpu_reset; >> + >> + cc->class_by_name =3D x86_cpu_class_by_name; >> +} >> + >> +static void x86_register_cpu_type(const x86_def_t *def) >> +{ >> + TypeInfo type_info =3D { >> + .parent =3D TYPE_X86_CPU, >> + .class_init =3D x86_cpu_def_class_init, >> + .class_data =3D (void *)def, >> + }; >> + >> + type_info.name =3D g_strdup_printf("%s-" TYPE_X86_CPU, def->nam= e); >> + type_register(&type_info); >> + g_free((void *)type_info.name); >> } >> =20 >> static const TypeInfo x86_cpu_type_info =3D { >> @@ -2257,14 +2150,19 @@ static const TypeInfo x86_cpu_type_info =3D = { >> .parent =3D TYPE_CPU, >> .instance_size =3D sizeof(X86CPU), >> .instance_init =3D x86_cpu_initfn, >> - .abstract =3D false, >> + .abstract =3D true, >> .class_size =3D sizeof(X86CPUClass), >> .class_init =3D x86_cpu_common_class_init, >> }; >> =20 >> static void x86_cpu_register_types(void) >> { >> + int i; >> + >> type_register_static(&x86_cpu_type_info); >> + for (i =3D 0; i < ARRAY_SIZE(builtin_x86_defs); i++) { >> + x86_register_cpu_type(&builtin_x86_defs[i]); >> + } >> } >> =20 >> type_init(x86_cpu_register_types) >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h >> index 7577e4f..2aef7b7 100644 >> --- a/target-i386/cpu.h >> +++ b/target-i386/cpu.h >> @@ -887,7 +887,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); >> =20 >> int cpu_get_pic_interrupt(CPUX86State *s); >> @@ -1079,7 +1078,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 >> =20 >> #define CPU_SAVE_VERSION 12 >> =20 >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c >> index 9ebf181..dbfa670 100644 >> --- a/target-i386/kvm.c >> +++ b/target-i386/kvm.c >> @@ -735,6 +735,80 @@ static int kvm_get_supported_msrs(KVMState *s) >> return ret; >> } >> =20 >> +static void kvm_host_cpu_initfn(Object *obj) >> +{ >> + assert(kvm_enabled()); >> +} >> + >> +static bool kvm_host_cpu_needs_class_init; >> + >> +static void kvm_host_cpu_class_init(ObjectClass *oc, void *data) >> +{ >> + X86CPUClass *xcc =3D X86_CPU_CLASS(oc); >> + x86_def_t *x86_cpu_def =3D &xcc->info; >> + KVMState *s =3D kvm_state; >> + uint32_t eax =3D 0, ebx =3D 0, ecx =3D 0, edx =3D 0; >> + int i; >> + >> + if (!kvm_enabled()) { >> + kvm_host_cpu_needs_class_init =3D true; >> + return; >> + } >> + >> + xcc->info.name =3D "host"; >> + >> + /* Vendor will be set in initfn if kvm_enabled() */ >> + >> + host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); >> + x86_cpu_def->family =3D ((eax >> 8) & 0x0F) + ((eax >> 20) & 0x= =46F); >> + x86_cpu_def->model =3D ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >= > 12); >> + x86_cpu_def->stepping =3D eax & 0x0F; >> + >> + x86_cpu_def->level =3D kvm_arch_get_supported_cpuid(s, 0x0, 0, = R_EAX); >> + x86_cpu_def->features =3D kvm_arch_get_supported_cpuid(s, 0x1, = 0, R_EDX); >> + x86_cpu_def->ext_features =3D kvm_arch_get_supported_cpuid(s, 0= x1, 0, >> R_ECX); + >> + if (x86_cpu_def->level >=3D 7) { >> + x86_cpu_def->cpuid_7_0_ebx_features =3D >> + kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX); >> + } else { >> + x86_cpu_def->cpuid_7_0_ebx_features =3D 0; >> + } >> + >> + x86_cpu_def->xlevel =3D kvm_arch_get_supported_cpuid(s, 0x80000= 000, 0, >> R_EAX); >> + x86_cpu_def->ext2_features =3D >> + kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ED= X); >> + x86_cpu_def->ext3_features =3D >> + kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EC= X); >> + >> + for (i =3D 0; i < 3; i++) { >> + host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx); >> + memcpy(xcc->info.model_id + i * 16 + 0, &eax, 4); >> + memcpy(xcc->info.model_id + i * 16 + 4, &ebx, 4); >> + memcpy(xcc->info.model_id + i * 16 + 8, &ecx, 4); >> + memcpy(xcc->info.model_id + i * 16 + 12, &edx, 4); >> + } >> + >> + /* Call Centaur's CPUID instruction. */ >> + if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) { >> + host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx); >> + eax =3D kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EA= X); >> + if (eax >=3D 0xC0000001) { >> + /* Support VIA max extended level */ >> + x86_cpu_def->xlevel2 =3D eax; >> + host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx); >> + x86_cpu_def->ext4_features =3D >> + kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, = R_EDX); >> + } >> + } >> + >> + /* Other KVM-specific feature fields: */ >> + x86_cpu_def->svm_features =3D >> + kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX); >> + x86_cpu_def->kvm_features =3D >> + kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EA= X); >> +} >> + >> int kvm_arch_init(KVMState *s) >> { >> QemuOptsList *list =3D qemu_find_opts("machine"); >> @@ -797,6 +871,11 @@ int kvm_arch_init(KVMState *s) >> } >> } >> } >> + >> + if (kvm_host_cpu_needs_class_init) { >> + kvm_host_cpu_class_init(object_class_by_name(TYPE_HOST_X86_= CPU), >> NULL); >> + } > kvm_host_cpu_needs_class_init is set to true in kvm_host_cpu_class_in= it() > so block is nop. Nope. It's a question of initialization ordering: > Why kvm_host_cpu_needs_class_init is needed anyway, why not just call > kvm_host_cpu_class_init() here? i) class_init may be called before kvm_enabled() evaluates to true, suc= h as for -cpu ?, so we need to handle that case gracefully. ii) The regular case is that the class_init is run in response to object_new() from pc.c, in which case kvm_enabled() evaluates to true and the needs_class_init remains false. Thus, both cases are used in practice. Regards, Andreas >> + >> return 0; >> } >> =20 >> @@ -2330,3 +2409,17 @@ int kvm_device_msix_deassign(KVMState *s, uin= t32_t >> dev_id) return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUES= T_MSIX | >> KVM_DEV_IRQ_HOST_MS= IX); >> } >> + >> +static const TypeInfo host_x86_cpu_type_info =3D { >> + .name =3D TYPE_HOST_X86_CPU, >> + .parent =3D TYPE_X86_CPU, >> + .instance_init =3D kvm_host_cpu_initfn, >> + .class_init =3D kvm_host_cpu_class_init, >> +}; >> + >> +static void kvm_register_types(void) >> +{ >> + type_register_static(&host_x86_cpu_type_info); >> +} >> + >> +type_init(kvm_register_types) --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg