On Thu, Dec 27, 2012 at 03:59:27PM +0100, Igor Mammedov wrote: > commit 8935499831312 makes cpuid return to guest host's vendor value > instead of built-in one by default if kvm_enabled() == true and allows > to override this behavior if 'vendor' is specified on -cpu command line. > > But every time guest calls cpuid to get 'vendor' value, host's value is > read again and again in default case. > > It complicates semantic of vendor property and makes it harder to use. > > Instead of reading 'vendor' value from host every time cpuid[vendor] is > called, override 'vendor' value only once in cpu_x86_find_by_name(), when > built-in CPU model is found and if(kvm_enabled() == true). > > It provides the same default semantic > if (kvm_enabled() == true) vendor = host's vendor > else vendor = built-in vendor > > and then later: > if (custom vendor) vendor = custom vendor > > 'vendor' value is overridden when user provides it on -cpu command line, > and there isn't need in vendor_override field anymore, remove it. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> > --- > target-i386/cpu.c | 27 ++++++++++++--------------- > target-i386/cpu.h | 1 - > 2 files changed, 12 insertions(+), 16 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 6cce311..c223599 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -284,7 +284,6 @@ typedef struct x86_def_t { > uint32_t kvm_features, svm_features; > uint32_t xlevel; > char model_id[48]; > - int vendor_override; > /* Store the results of Centaur's CPUID instructions */ > uint32_t ext4_features; > uint32_t xlevel2; > @@ -865,7 +864,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX); > > cpu_x86_fill_model_id(x86_cpu_def->model_id); > - x86_cpu_def->vendor_override = 0; > > /* Call Centaur's CPUID instruction. */ > if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) { > @@ -1117,7 +1115,6 @@ static void x86_cpuid_set_vendor(Object *obj, const > char *value, > env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i); > env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i); > } > - env->cpuid_vendor_override = 1; > } > > static char *x86_cpuid_get_model_id(Object *obj, Error **errp) > @@ -1194,7 +1191,6 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t > *def, Error **errp) > > assert(def->vendor[0]); > object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp); > - env->cpuid_vendor_override = def->vendor_override; > object_property_set_int(OBJECT(cpu), def->level, "level", errp); > object_property_set_int(OBJECT(cpu), def->family, "family", errp); > object_property_set_int(OBJECT(cpu), def->model, "model", errp); > @@ -1231,6 +1227,18 @@ static int cpu_x86_find_by_name(x86_def_t > *x86_cpu_def, const char *name) > return -1; > } else { > memcpy(x86_cpu_def, def, sizeof(*def)); > + /* sysenter isn't supported on 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()) { > + uint32_t ebx = 0, ecx = 0, edx = 0; > + host_cpuid(0, 0, NULL, &ebx, &ecx, &edx); > + x86cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); > + } > } > > return 0; > @@ -1314,7 +1322,6 @@ static int cpu_x86_parse_featurestr(x86_def_t > *x86_cpu_def, char *features) > x86_cpu_def->xlevel = numvalue; > } else if (!strcmp(featurestr, "vendor")) { > pstrcpy(x86_cpu_def->vendor, sizeof(x86_cpu_def->vendor), > val); > - x86_cpu_def->vendor_override = 1; > } else if (!strcmp(featurestr, "model_id")) { > pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id), > val); > @@ -1563,16 +1570,6 @@ static void get_cpuid_vendor(CPUX86State *env, > uint32_t *ebx, > *ebx = env->cpuid_vendor1; > *edx = env->cpuid_vendor2; > *ecx = env->cpuid_vendor3; > - > - /* sysenter isn't supported on 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 if you want to use KVM's sysenter/syscall emulation > - * in compatibility mode and when doing cross vendor migration > - */ > - if (kvm_enabled() && ! env->cpuid_vendor_override) { > - host_cpuid(0, 0, NULL, ebx, ecx, edx); > - } > } > > void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index fbbe730..a15a09e 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -812,7 +812,6 @@ typedef struct CPUX86State { > uint32_t cpuid_ext2_features; > uint32_t cpuid_ext3_features; > uint32_t cpuid_apic_id; > - int cpuid_vendor_override; > /* Store the results of Centaur's CPUID instructions */ > uint32_t cpuid_xlevel2; > uint32_t cpuid_ext4_features; > -- > 1.7.1 > -- Eduardo