| Message ID | 1358251588-18376-1-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 15, 2013 at 01:06:28PM +0100, Igor Mammedov wrote: > Tested on x86 host yet, I don't have bigendian host avilable right now. > > Vendor property setter takes string as vendor value but cpudefs > use uint32_t vendor[123] fields to define vendor value. It makes it > difficult to unify and use property setter for values from cpudefs. > > Simplify code by using vendor property setter, vendor[123] fields > are converted into vendor[13] array to keep its value. And vendor > property setter is used to access/set value on CPU. > > Extra: > - replace cpuid_vendor[1.2.3] words in CPUX86State with union > to simplify vendor property setter and pack/unpack procedures > - add x86_cpu_vendor_words2str() to make for() cycle reusable > - convert words in cpuid_vendor union to little-endian when > returning them to guest in cpuid instruction emulation, since > they are not packed manualy anymore What about all other cases where CPUID_VENDOR_*_[123] constants are used? > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v4: > - use union for cpuid_vendor to simplify convertions string<=>registers > v3: > - replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str() > due to renaming of the last in previous patch > - rebased with "target-i386: move out CPU features initialization > in separate func" patch dropped > v2: > - restore deleted host_cpuid() call in kvm_cpu_fill_host() > Spotted-By: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu.c | 180 ++++++++++++++--------------------------------- > target-i386/cpu.h | 17 +++-- > target-i386/translate.c | 4 +- > 3 files changed, 67 insertions(+), 134 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 333745b..882da50 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -45,6 +45,18 @@ > #include "hw/apic_internal.h" > #endif > > +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > + uint32_t vendor2, uint32_t vendor3) > +{ > + int i; > + for (i = 0; i < 4; i++) { > + dst[i] = vendor1 >> (8 * i); > + dst[i + 4] = vendor2 >> (8 * i); > + dst[i + 8] = vendor3 >> (8 * i); > + } > + dst[CPUID_VENDOR_SZ] = '\0'; > +} Why this function still exists, if we have the union? Anyway: even with the union, we can't avoid having explicit conversion code because of endianness. It doesn't matter which option we choose: - if we store vendor[123], we need to convert to string on the vendor property getter/setter; - if we store vendor string, we need to convert to register values on cpu_x86_cpuid(); - if we store it in an union we need endianness conversion code (inline or in a function) every time we read vendor[123]. That said, I don't see the point of the union. > + > /* feature flags taken from "Intel Processor Identification and the CPUID > * Instruction" and AMD's "CPUID Specification". In cases of disagreement > * between feature naming conventions, aliases may be added. > @@ -341,7 +353,7 @@ typedef struct x86_def_t { > struct x86_def_t *next; > const char *name; > uint32_t level; > - uint32_t vendor1, vendor2, vendor3; > + char vendor[CPUID_VENDOR_SZ + 1]; > int family; > int model; > int stepping; > @@ -406,9 +418,7 @@ static x86_def_t builtin_x86_defs[] = { [...] > @@ -882,9 +848,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Opteron_G5", > .level = 0xd, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 21, > .model = 2, > .stepping = 0, > @@ -945,9 +909,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > > x86_cpu_def->name = "host"; > host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); > - x86_cpu_def->vendor1 = ebx; > - x86_cpu_def->vendor2 = edx; > - x86_cpu_def->vendor3 = ecx; > + x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); > > host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); > x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF); > @@ -975,9 +937,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > x86_cpu_def->vendor_override = 0; > > /* Call Centaur's CPUID instruction. */ > - if (x86_cpu_def->vendor1 == CPUID_VENDOR_VIA_1 && > - x86_cpu_def->vendor2 == CPUID_VENDOR_VIA_2 && > - x86_cpu_def->vendor3 == CPUID_VENDOR_VIA_3) { > + if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) { > host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx); > eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > if (eax >= 0xC0000001) { > @@ -1213,15 +1173,9 @@ static char *x86_cpuid_get_vendor(Object *obj, Error **errp) > X86CPU *cpu = X86_CPU(obj); > CPUX86State *env = &cpu->env; > char *value; > - int i; > > value = (char *)g_malloc(CPUID_VENDOR_SZ + 1); > - for (i = 0; i < 4; i++) { > - value[i ] = env->cpuid_vendor1 >> (8 * i); > - value[i + 4] = env->cpuid_vendor2 >> (8 * i); > - value[i + 8] = env->cpuid_vendor3 >> (8 * i); > - } > - value[CPUID_VENDOR_SZ] = '\0'; > + pstrcpy(value, CPUID_VENDOR_SZ + 1, env->cpuid_vendor.str); > return value; > } > > @@ -1230,7 +1184,6 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > { > X86CPU *cpu = X86_CPU(obj); > CPUX86State *env = &cpu->env; > - int i; > > if (strlen(value) != CPUID_VENDOR_SZ) { > error_set(errp, QERR_PROPERTY_VALUE_BAD, "", > @@ -1238,14 +1191,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > return; > } > > - env->cpuid_vendor1 = 0; > - env->cpuid_vendor2 = 0; > - env->cpuid_vendor3 = 0; > - for (i = 0; i < 4; i++) { > - env->cpuid_vendor1 |= ((uint8_t)value[i ]) << (8 * i); > - env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i); > - env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i); > - } > + pstrcpy(env->cpuid_vendor.str, sizeof(env->cpuid_vendor.str), value); > env->cpuid_vendor_override = 1; > } > > @@ -1341,7 +1287,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name) > */ > static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > { > - unsigned int i; > char *featurestr; /* Single 'key=value" string being parsed */ > /* Features to be added */ > FeatureWordArray plus_features = { 0 }; > @@ -1403,18 +1348,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > } > x86_cpu_def->xlevel = numvalue; > } else if (!strcmp(featurestr, "vendor")) { > - if (strlen(val) != 12) { > - fprintf(stderr, "vendor string must be 12 chars long\n"); > - goto error; > - } > - x86_cpu_def->vendor1 = 0; > - x86_cpu_def->vendor2 = 0; > - x86_cpu_def->vendor3 = 0; > - for(i = 0; i < 4; i++) { > - x86_cpu_def->vendor1 |= ((uint8_t)val[i ]) << (8 * i); > - x86_cpu_def->vendor2 |= ((uint8_t)val[i + 4]) << (8 * i); > - x86_cpu_def->vendor3 |= ((uint8_t)val[i + 8]) << (8 * i); > - } > + 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), > @@ -1609,10 +1543,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > error_setg(&error, "Invalid cpu_model string format: %s", cpu_model); > goto out; > } > - assert(def->vendor1); > - env->cpuid_vendor1 = def->vendor1; > - env->cpuid_vendor2 = def->vendor2; > - env->cpuid_vendor3 = def->vendor3; > + assert(def->vendor[0]); > + object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error); > env->cpuid_vendor_override = def->vendor_override; > object_property_set_int(OBJECT(cpu), def->level, "level", &error); > object_property_set_int(OBJECT(cpu), def->family, "family", &error); > @@ -1682,9 +1614,9 @@ void x86_cpudef_setup(void) > static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx, > uint32_t *ecx, uint32_t *edx) > { > - *ebx = env->cpuid_vendor1; > - *edx = env->cpuid_vendor2; > - *ecx = env->cpuid_vendor3; > + cpu_to_le32wu(ebx, env->cpuid_vendor.regs.ebx); > + cpu_to_le32wu(edx, env->cpuid_vendor.regs.edx); > + cpu_to_le32wu(ecx, env->cpuid_vendor.regs.ecx); > > /* sysenter isn't supported on compatibility mode on AMD, syscall > * isn't supported in compatibility mode on Intel. > @@ -1862,9 +1794,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > break; > case 0x80000000: > *eax = env->cpuid_xlevel; > - *ebx = env->cpuid_vendor1; > - *edx = env->cpuid_vendor2; > - *ecx = env->cpuid_vendor3; > + *ebx = env->cpuid_vendor.regs.ebx; > + *edx = env->cpuid_vendor.regs.edx; > + *ecx = env->cpuid_vendor.regs.ecx; Don't we need endianness conversion here as well? > break; > case 0x80000001: > *eax = env->cpuid_version; > @@ -1877,11 +1809,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > * So dont set it here for Intel to make Linux guests happy. > */ > if (cs->nr_cores * cs->nr_threads > 1) { > - uint32_t tebx, tecx, tedx; > - get_cpuid_vendor(env, &tebx, &tecx, &tedx); > - if (tebx != CPUID_VENDOR_INTEL_1 || > - tedx != CPUID_VENDOR_INTEL_2 || > - tecx != CPUID_VENDOR_INTEL_3) { > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_INTEL, > + sizeof(env->cpuid_vendor.str))) { > *ecx |= 1 << 1; /* CmpLegacy bit */ > } > } > @@ -2152,9 +2081,8 @@ void x86_cpu_realize(Object *obj, Error **errp) > /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on > * CPUID[1].EDX. > */ > - if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && > - env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && > - env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) { > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_AMD, > + sizeof(env->cpuid_vendor.str))) { > env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES; > env->cpuid_ext2_features |= (env->cpuid_features > & CPUID_EXT2_AMD_ALIASES); > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index e4a7c50..09a3b18 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -531,14 +531,14 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */ > #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */ > #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */ > +#define CPUID_VENDOR_INTEL "GenuineIntel" > > #define CPUID_VENDOR_AMD_1 0x68747541 /* "Auth" */ > #define CPUID_VENDOR_AMD_2 0x69746e65 /* "enti" */ > #define CPUID_VENDOR_AMD_3 0x444d4163 /* "cAMD" */ > +#define CPUID_VENDOR_AMD "AuthenticAMD" > > -#define CPUID_VENDOR_VIA_1 0x746e6543 /* "Cent" */ > -#define CPUID_VENDOR_VIA_2 0x48727561 /* "aurH" */ > -#define CPUID_VENDOR_VIA_3 0x736c7561 /* "auls" */ > +#define CPUID_VENDOR_VIA "CentaurHauls" > > #define CPUID_MWAIT_IBE (1 << 1) /* Interrupts can exit capability */ > #define CPUID_MWAIT_EMX (1 << 0) /* enumeration supported */ > @@ -818,9 +818,14 @@ typedef struct CPUX86State { > > /* processor features (e.g. for CPUID insn) */ > uint32_t cpuid_level; > - uint32_t cpuid_vendor1; > - uint32_t cpuid_vendor2; > - uint32_t cpuid_vendor3; > + union { > + struct __attribute__((packed)) { > + uint32_t ebx; > + uint32_t edx; > + uint32_t ecx; > + } regs; > + char str[CPUID_VENDOR_SZ + 1]; > + } cpuid_vendor; > uint32_t cpuid_version; > uint32_t cpuid_features; > uint32_t cpuid_ext_features; > diff --git a/target-i386/translate.c b/target-i386/translate.c > index 32d21f5..985080b 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -7028,7 +7028,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > break; > case 0x134: /* sysenter */ > /* For Intel SYSENTER is valid on 64-bit */ > - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) > + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) This needs endianness conversion as well. > goto illegal_op; > if (!s->pe) { > gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); > @@ -7041,7 +7041,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > break; > case 0x135: /* sysexit */ > /* For Intel SYSEXIT is valid on 64-bit */ > - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) > + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) Ditto. > goto illegal_op; > if (!s->pe) { > gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); > -- > 1.7.1 >
On Tue, 15 Jan 2013 15:51:18 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Jan 15, 2013 at 01:06:28PM +0100, Igor Mammedov wrote: [..] > > - replace cpuid_vendor[1.2.3] words in CPUX86State with union > > to simplify vendor property setter and pack/unpack procedures > > - add x86_cpu_vendor_words2str() to make for() cycle reusable > > - convert words in cpuid_vendor union to little-endian when > > returning them to guest in cpuid instruction emulation, since > > they are not packed manualy anymore > > What about all other cases where CPUID_VENDOR_*_[123] constants are > used? NACK patch, it is probably horribly broken on big-endian acrh. > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v4: > > - use union for cpuid_vendor to simplify convertions string<=>registers > > v3: > > - replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str() > > due to renaming of the last in previous patch > > - rebased with "target-i386: move out CPU features initialization > > in separate func" patch dropped > > v2: > > - restore deleted host_cpuid() call in kvm_cpu_fill_host() > > Spotted-By: Eduardo Habkost <ehabkost@redhat.com> > > --- > > target-i386/cpu.c | 180 ++++++++++++++--------------------------------- > > target-i386/cpu.h | 17 +++-- > > target-i386/translate.c | 4 +- > > 3 files changed, 67 insertions(+), 134 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 333745b..882da50 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -45,6 +45,18 @@ > > #include "hw/apic_internal.h" > > #endif > > > > +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > > + uint32_t vendor2, uint32_t vendor3) > > +{ > > + int i; > > + for (i = 0; i < 4; i++) { > > + dst[i] = vendor1 >> (8 * i); > > + dst[i + 4] = vendor2 >> (8 * i); > > + dst[i + 8] = vendor3 >> (8 * i); > > + } > > + dst[CPUID_VENDOR_SZ] = '\0'; > > +} > > Why this function still exists, if we have the union? to copy words from host_cpuid(0, 0, NULL, &ebx, &ecx, &edx); to x86_cpu_def->vendor when vendor_override is removed. but never mind. > > Anyway: even with the union, we can't avoid having explicit conversion > code because of endianness. It doesn't matter which option we choose: > > - if we store vendor[123], we need to convert to string on the vendor > property getter/setter; > - if we store vendor string, we need to convert to register values on > cpu_x86_cpuid(); > - if we store it in an union we need endianness conversion code (inline > or in a function) every time we read vendor[123]. > > That said, I don't see the point of the union. Agreed, union patch gets more intrusive as we would need to care about endiannes in every place words are consumed. Original patch looks much less intrusive and touches only words=>string conversion. +1 in favor of original patch. > [...] > > -- > Eduardo
在 2013-01-15二的 13:06 +0100，Igor Mammedov写道： > Tested on x86 host yet, I don't have bigendian host avilable right now. > > Vendor property setter takes string as vendor value but cpudefs > use uint32_t vendor[123] fields to define vendor value. It makes it > difficult to unify and use property setter for values from cpudefs. > > Simplify code by using vendor property setter, vendor[123] fields > are converted into vendor[13] array to keep its value. And vendor > property setter is used to access/set value on CPU. > > Extra: > - replace cpuid_vendor[1.2.3] words in CPUX86State with union > to simplify vendor property setter and pack/unpack procedures > - add x86_cpu_vendor_words2str() to make for() cycle reusable > - convert words in cpuid_vendor union to little-endian when > returning them to guest in cpuid instruction emulation, since > they are not packed manualy anymore > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v4: > - use union for cpuid_vendor to simplify convertions string<=>registers > v3: > - replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str() > due to renaming of the last in previous patch > - rebased with "target-i386: move out CPU features initialization > in separate func" patch dropped > v2: > - restore deleted host_cpuid() call in kvm_cpu_fill_host() > Spotted-By: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu.c | 180 ++++++++++++++--------------------------------- > target-i386/cpu.h | 17 +++-- > target-i386/translate.c | 4 +- > 3 files changed, 67 insertions(+), 134 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 333745b..882da50 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -45,6 +45,18 @@ > #include "hw/apic_internal.h" > #endif > > +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > + uint32_t vendor2, uint32_t vendor3) > +{ > + int i; > + for (i = 0; i < 4; i++) { > + dst[i] = vendor1 >> (8 * i); > + dst[i + 4] = vendor2 >> (8 * i); > + dst[i + 8] = vendor3 >> (8 * i); > + } > + dst[CPUID_VENDOR_SZ] = '\0'; > +} > + > /* feature flags taken from "Intel Processor Identification and the CPUID > * Instruction" and AMD's "CPUID Specification". In cases of disagreement > * between feature naming conventions, aliases may be added. > @@ -341,7 +353,7 @@ typedef struct x86_def_t { > struct x86_def_t *next; > const char *name; > uint32_t level; > - uint32_t vendor1, vendor2, vendor3; > + char vendor[CPUID_VENDOR_SZ + 1]; > int family; > int model; > int stepping; > @@ -406,9 +418,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "qemu64", > .level = 4, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 6, > .model = 2, > .stepping = 3, > @@ -425,9 +435,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "phenom", > .level = 5, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 16, > .model = 2, > .stepping = 3, > @@ -453,9 +461,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "core2duo", > .level = 10, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 15, > .stepping = 11, > @@ -474,9 +480,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "kvm64", > .level = 5, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 15, > .model = 6, > .stepping = 1, > @@ -500,9 +504,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "qemu32", > .level = 4, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 3, > .stepping = 3, > @@ -513,9 +515,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "kvm32", > .level = 5, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 15, > .model = 6, > .stepping = 1, > @@ -530,9 +530,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "coreduo", > .level = 10, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 14, > .stepping = 8, > @@ -548,9 +546,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "486", > .level = 1, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 4, > .model = 0, > .stepping = 0, > @@ -560,9 +556,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "pentium", > .level = 1, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 5, > .model = 4, > .stepping = 3, > @@ -572,9 +566,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "pentium2", > .level = 2, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 5, > .stepping = 2, > @@ -584,9 +576,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "pentium3", > .level = 2, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 7, > .stepping = 3, > @@ -596,9 +586,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "athlon", > .level = 2, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 6, > .model = 2, > .stepping = 3, > @@ -612,9 +600,7 @@ static x86_def_t builtin_x86_defs[] = { > .name = "n270", > /* original is on level 10 */ > .level = 5, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 28, > .stepping = 2, > @@ -633,9 +619,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Conroe", > .level = 2, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 2, > .stepping = 3, > @@ -653,9 +637,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Penryn", > .level = 2, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 2, > .stepping = 3, > @@ -674,9 +656,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Nehalem", > .level = 2, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 2, > .stepping = 3, > @@ -695,9 +675,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Westmere", > .level = 11, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 44, > .stepping = 1, > @@ -717,9 +695,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "SandyBridge", > .level = 0xd, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 42, > .stepping = 1, > @@ -742,9 +718,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Haswell", > .level = 0xd, > - .vendor1 = CPUID_VENDOR_INTEL_1, > - .vendor2 = CPUID_VENDOR_INTEL_2, > - .vendor3 = CPUID_VENDOR_INTEL_3, > + .vendor = CPUID_VENDOR_INTEL, > .family = 6, > .model = 60, > .stepping = 1, > @@ -772,9 +746,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Opteron_G1", > .level = 5, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 15, > .model = 6, > .stepping = 1, > @@ -796,9 +768,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Opteron_G2", > .level = 5, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 15, > .model = 6, > .stepping = 1, > @@ -822,9 +792,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Opteron_G3", > .level = 5, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 15, > .model = 6, > .stepping = 1, > @@ -850,9 +818,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Opteron_G4", > .level = 0xd, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 21, > .model = 1, > .stepping = 2, > @@ -882,9 +848,7 @@ static x86_def_t builtin_x86_defs[] = { > { > .name = "Opteron_G5", > .level = 0xd, > - .vendor1 = CPUID_VENDOR_AMD_1, > - .vendor2 = CPUID_VENDOR_AMD_2, > - .vendor3 = CPUID_VENDOR_AMD_3, > + .vendor = CPUID_VENDOR_AMD, > .family = 21, > .model = 2, > .stepping = 0, > @@ -945,9 +909,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > > x86_cpu_def->name = "host"; > host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); > - x86_cpu_def->vendor1 = ebx; > - x86_cpu_def->vendor2 = edx; > - x86_cpu_def->vendor3 = ecx; > + x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); > > host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); > x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF); > @@ -975,9 +937,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > x86_cpu_def->vendor_override = 0; > > /* Call Centaur's CPUID instruction. */ > - if (x86_cpu_def->vendor1 == CPUID_VENDOR_VIA_1 && > - x86_cpu_def->vendor2 == CPUID_VENDOR_VIA_2 && > - x86_cpu_def->vendor3 == CPUID_VENDOR_VIA_3) { > + if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) { > host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx); > eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > if (eax >= 0xC0000001) { > @@ -1213,15 +1173,9 @@ static char *x86_cpuid_get_vendor(Object *obj, Error **errp) > X86CPU *cpu = X86_CPU(obj); > CPUX86State *env = &cpu->env; > char *value; > - int i; > > value = (char *)g_malloc(CPUID_VENDOR_SZ + 1); > - for (i = 0; i < 4; i++) { > - value[i ] = env->cpuid_vendor1 >> (8 * i); > - value[i + 4] = env->cpuid_vendor2 >> (8 * i); > - value[i + 8] = env->cpuid_vendor3 >> (8 * i); > - } > - value[CPUID_VENDOR_SZ] = '\0'; > + pstrcpy(value, CPUID_VENDOR_SZ + 1, env->cpuid_vendor.str); > return value; > } > > @@ -1230,7 +1184,6 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > { > X86CPU *cpu = X86_CPU(obj); > CPUX86State *env = &cpu->env; > - int i; > > if (strlen(value) != CPUID_VENDOR_SZ) { > error_set(errp, QERR_PROPERTY_VALUE_BAD, "", > @@ -1238,14 +1191,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > return; > } > > - env->cpuid_vendor1 = 0; > - env->cpuid_vendor2 = 0; > - env->cpuid_vendor3 = 0; > - for (i = 0; i < 4; i++) { > - env->cpuid_vendor1 |= ((uint8_t)value[i ]) << (8 * i); > - env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i); > - env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i); > - } > + pstrcpy(env->cpuid_vendor.str, sizeof(env->cpuid_vendor.str), value); > env->cpuid_vendor_override = 1; > } > > @@ -1341,7 +1287,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name) > */ > static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > { > - unsigned int i; > char *featurestr; /* Single 'key=value" string being parsed */ > /* Features to be added */ > FeatureWordArray plus_features = { 0 }; > @@ -1403,18 +1348,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > } > x86_cpu_def->xlevel = numvalue; > } else if (!strcmp(featurestr, "vendor")) { > - if (strlen(val) != 12) { > - fprintf(stderr, "vendor string must be 12 chars long\n"); > - goto error; > - } > - x86_cpu_def->vendor1 = 0; > - x86_cpu_def->vendor2 = 0; > - x86_cpu_def->vendor3 = 0; > - for(i = 0; i < 4; i++) { > - x86_cpu_def->vendor1 |= ((uint8_t)val[i ]) << (8 * i); > - x86_cpu_def->vendor2 |= ((uint8_t)val[i + 4]) << (8 * i); > - x86_cpu_def->vendor3 |= ((uint8_t)val[i + 8]) << (8 * i); > - } > + 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), > @@ -1609,10 +1543,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > error_setg(&error, "Invalid cpu_model string format: %s", cpu_model); > goto out; > } > - assert(def->vendor1); > - env->cpuid_vendor1 = def->vendor1; > - env->cpuid_vendor2 = def->vendor2; > - env->cpuid_vendor3 = def->vendor3; > + assert(def->vendor[0]); > + object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error); > env->cpuid_vendor_override = def->vendor_override; > object_property_set_int(OBJECT(cpu), def->level, "level", &error); > object_property_set_int(OBJECT(cpu), def->family, "family", &error); > @@ -1682,9 +1614,9 @@ void x86_cpudef_setup(void) > static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx, > uint32_t *ecx, uint32_t *edx) > { > - *ebx = env->cpuid_vendor1; > - *edx = env->cpuid_vendor2; > - *ecx = env->cpuid_vendor3; > + cpu_to_le32wu(ebx, env->cpuid_vendor.regs.ebx); > + cpu_to_le32wu(edx, env->cpuid_vendor.regs.edx); > + cpu_to_le32wu(ecx, env->cpuid_vendor.regs.ecx); > > /* sysenter isn't supported on compatibility mode on AMD, syscall > * isn't supported in compatibility mode on Intel. > @@ -1862,9 +1794,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > break; > case 0x80000000: > *eax = env->cpuid_xlevel; > - *ebx = env->cpuid_vendor1; > - *edx = env->cpuid_vendor2; > - *ecx = env->cpuid_vendor3; > + *ebx = env->cpuid_vendor.regs.ebx; > + *edx = env->cpuid_vendor.regs.edx; > + *ecx = env->cpuid_vendor.regs.ecx; > break; > case 0x80000001: > *eax = env->cpuid_version; > @@ -1877,11 +1809,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > * So dont set it here for Intel to make Linux guests happy. > */ > if (cs->nr_cores * cs->nr_threads > 1) { > - uint32_t tebx, tecx, tedx; > - get_cpuid_vendor(env, &tebx, &tecx, &tedx); > - if (tebx != CPUID_VENDOR_INTEL_1 || > - tedx != CPUID_VENDOR_INTEL_2 || > - tecx != CPUID_VENDOR_INTEL_3) { > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_INTEL, > + sizeof(env->cpuid_vendor.str))) { > *ecx |= 1 << 1; /* CmpLegacy bit */ > } > } > @@ -2152,9 +2081,8 @@ void x86_cpu_realize(Object *obj, Error **errp) > /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on > * CPUID[1].EDX. > */ > - if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && > - env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && > - env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) { > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_AMD, > + sizeof(env->cpuid_vendor.str))) { > env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES; > env->cpuid_ext2_features |= (env->cpuid_features > & CPUID_EXT2_AMD_ALIASES); > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index e4a7c50..09a3b18 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -531,14 +531,14 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */ > #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */ > #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */ > +#define CPUID_VENDOR_INTEL "GenuineIntel" > > #define CPUID_VENDOR_AMD_1 0x68747541 /* "Auth" */ > #define CPUID_VENDOR_AMD_2 0x69746e65 /* "enti" */ > #define CPUID_VENDOR_AMD_3 0x444d4163 /* "cAMD" */ > +#define CPUID_VENDOR_AMD "AuthenticAMD" why not also remove these 2 VENDOR_{INTEL,AMD}_{1,2,3} ? > > -#define CPUID_VENDOR_VIA_1 0x746e6543 /* "Cent" */ > -#define CPUID_VENDOR_VIA_2 0x48727561 /* "aurH" */ > -#define CPUID_VENDOR_VIA_3 0x736c7561 /* "auls" */ > +#define CPUID_VENDOR_VIA "CentaurHauls" > > #define CPUID_MWAIT_IBE (1 << 1) /* Interrupts can exit capability */ > #define CPUID_MWAIT_EMX (1 << 0) /* enumeration supported */ > @@ -818,9 +818,14 @@ typedef struct CPUX86State { > > /* processor features (e.g. for CPUID insn) */ > uint32_t cpuid_level; > - uint32_t cpuid_vendor1; > - uint32_t cpuid_vendor2; > - uint32_t cpuid_vendor3; > + union { > + struct __attribute__((packed)) { > + uint32_t ebx; > + uint32_t edx; > + uint32_t ecx; > + } regs; > + char str[CPUID_VENDOR_SZ + 1]; > + } cpuid_vendor; > uint32_t cpuid_version; > uint32_t cpuid_features; > uint32_t cpuid_ext_features; > diff --git a/target-i386/translate.c b/target-i386/translate.c > index 32d21f5..985080b 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -7028,7 +7028,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > break; > case 0x134: /* sysenter */ > /* For Intel SYSENTER is valid on 64-bit */ > - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) > + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) > goto illegal_op; > if (!s->pe) { > gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); > @@ -7041,7 +7041,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > break; > case 0x135: /* sysexit */ > /* For Intel SYSEXIT is valid on 64-bit */ > - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) > + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) > goto illegal_op; > if (!s->pe) { > gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
On Wed, 16 Jan 2013 15:07:05 +0800 li guang <lig.fnst@cn.fujitsu.com> wrote: > 在 2013-01-15二的 13:06 +0100，Igor Mammedov写道： > > Tested on x86 host yet, I don't have bigendian host avilable right now. > > > > Vendor property setter takes string as vendor value but cpudefs > > use uint32_t vendor[123] fields to define vendor value. It makes it > > difficult to unify and use property setter for values from cpudefs. > > > > Simplify code by using vendor property setter, vendor[123] fields > > are converted into vendor[13] array to keep its value. And vendor > > property setter is used to access/set value on CPU. > > > > Extra: > > - replace cpuid_vendor[1.2.3] words in CPUX86State with union > > to simplify vendor property setter and pack/unpack procedures > > - add x86_cpu_vendor_words2str() to make for() cycle reusable > > - convert words in cpuid_vendor union to little-endian when > > returning them to guest in cpuid instruction emulation, since > > they are not packed manualy anymore > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v4: > > - use union for cpuid_vendor to simplify convertions string<=>registers > > v3: > > - replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str() > > due to renaming of the last in previous patch > > - rebased with "target-i386: move out CPU features initialization > > in separate func" patch dropped > > v2: > > - restore deleted host_cpuid() call in kvm_cpu_fill_host() > > Spotted-By: Eduardo Habkost <ehabkost@redhat.com> > > --- > > target-i386/cpu.c | 180 ++++++++++++++--------------------------------- > > target-i386/cpu.h | 17 +++-- > > target-i386/translate.c | 4 +- > > 3 files changed, 67 insertions(+), 134 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 333745b..882da50 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -45,6 +45,18 @@ > > #include "hw/apic_internal.h" > > #endif > > > > +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > > + uint32_t vendor2, uint32_t vendor3) > > +{ > > + int i; > > + for (i = 0; i < 4; i++) { > > + dst[i] = vendor1 >> (8 * i); > > + dst[i + 4] = vendor2 >> (8 * i); > > + dst[i + 8] = vendor3 >> (8 * i); > > + } > > + dst[CPUID_VENDOR_SZ] = '\0'; > > +} > > + > > /* feature flags taken from "Intel Processor Identification and the CPUID > > * Instruction" and AMD's "CPUID Specification". In cases of disagreement > > * between feature naming conventions, aliases may be added. > > @@ -341,7 +353,7 @@ typedef struct x86_def_t { > > struct x86_def_t *next; > > const char *name; > > uint32_t level; > > - uint32_t vendor1, vendor2, vendor3; > > + char vendor[CPUID_VENDOR_SZ + 1]; > > int family; > > int model; > > int stepping; > > @@ -406,9 +418,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "qemu64", > > .level = 4, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 6, > > .model = 2, > > .stepping = 3, > > @@ -425,9 +435,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "phenom", > > .level = 5, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 16, > > .model = 2, > > .stepping = 3, > > @@ -453,9 +461,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "core2duo", > > .level = 10, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 15, > > .stepping = 11, > > @@ -474,9 +480,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "kvm64", > > .level = 5, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 15, > > .model = 6, > > .stepping = 1, > > @@ -500,9 +504,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "qemu32", > > .level = 4, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 3, > > .stepping = 3, > > @@ -513,9 +515,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "kvm32", > > .level = 5, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 15, > > .model = 6, > > .stepping = 1, > > @@ -530,9 +530,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "coreduo", > > .level = 10, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 14, > > .stepping = 8, > > @@ -548,9 +546,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "486", > > .level = 1, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 4, > > .model = 0, > > .stepping = 0, > > @@ -560,9 +556,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "pentium", > > .level = 1, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 5, > > .model = 4, > > .stepping = 3, > > @@ -572,9 +566,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "pentium2", > > .level = 2, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 5, > > .stepping = 2, > > @@ -584,9 +576,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "pentium3", > > .level = 2, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 7, > > .stepping = 3, > > @@ -596,9 +586,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "athlon", > > .level = 2, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 6, > > .model = 2, > > .stepping = 3, > > @@ -612,9 +600,7 @@ static x86_def_t builtin_x86_defs[] = { > > .name = "n270", > > /* original is on level 10 */ > > .level = 5, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 28, > > .stepping = 2, > > @@ -633,9 +619,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Conroe", > > .level = 2, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 2, > > .stepping = 3, > > @@ -653,9 +637,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Penryn", > > .level = 2, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 2, > > .stepping = 3, > > @@ -674,9 +656,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Nehalem", > > .level = 2, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 2, > > .stepping = 3, > > @@ -695,9 +675,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Westmere", > > .level = 11, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 44, > > .stepping = 1, > > @@ -717,9 +695,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "SandyBridge", > > .level = 0xd, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 42, > > .stepping = 1, > > @@ -742,9 +718,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Haswell", > > .level = 0xd, > > - .vendor1 = CPUID_VENDOR_INTEL_1, > > - .vendor2 = CPUID_VENDOR_INTEL_2, > > - .vendor3 = CPUID_VENDOR_INTEL_3, > > + .vendor = CPUID_VENDOR_INTEL, > > .family = 6, > > .model = 60, > > .stepping = 1, > > @@ -772,9 +746,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Opteron_G1", > > .level = 5, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 15, > > .model = 6, > > .stepping = 1, > > @@ -796,9 +768,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Opteron_G2", > > .level = 5, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 15, > > .model = 6, > > .stepping = 1, > > @@ -822,9 +792,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Opteron_G3", > > .level = 5, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 15, > > .model = 6, > > .stepping = 1, > > @@ -850,9 +818,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Opteron_G4", > > .level = 0xd, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 21, > > .model = 1, > > .stepping = 2, > > @@ -882,9 +848,7 @@ static x86_def_t builtin_x86_defs[] = { > > { > > .name = "Opteron_G5", > > .level = 0xd, > > - .vendor1 = CPUID_VENDOR_AMD_1, > > - .vendor2 = CPUID_VENDOR_AMD_2, > > - .vendor3 = CPUID_VENDOR_AMD_3, > > + .vendor = CPUID_VENDOR_AMD, > > .family = 21, > > .model = 2, > > .stepping = 0, > > @@ -945,9 +909,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > > > > x86_cpu_def->name = "host"; > > host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); > > - x86_cpu_def->vendor1 = ebx; > > - x86_cpu_def->vendor2 = edx; > > - x86_cpu_def->vendor3 = ecx; > > + x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); > > > > host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); > > x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF); > > @@ -975,9 +937,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > > x86_cpu_def->vendor_override = 0; > > > > /* Call Centaur's CPUID instruction. */ > > - if (x86_cpu_def->vendor1 == CPUID_VENDOR_VIA_1 && > > - x86_cpu_def->vendor2 == CPUID_VENDOR_VIA_2 && > > - x86_cpu_def->vendor3 == CPUID_VENDOR_VIA_3) { > > + if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) { > > host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx); > > eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); > > if (eax >= 0xC0000001) { > > @@ -1213,15 +1173,9 @@ static char *x86_cpuid_get_vendor(Object *obj, Error **errp) > > X86CPU *cpu = X86_CPU(obj); > > CPUX86State *env = &cpu->env; > > char *value; > > - int i; > > > > value = (char *)g_malloc(CPUID_VENDOR_SZ + 1); > > - for (i = 0; i < 4; i++) { > > - value[i ] = env->cpuid_vendor1 >> (8 * i); > > - value[i + 4] = env->cpuid_vendor2 >> (8 * i); > > - value[i + 8] = env->cpuid_vendor3 >> (8 * i); > > - } > > - value[CPUID_VENDOR_SZ] = '\0'; > > + pstrcpy(value, CPUID_VENDOR_SZ + 1, env->cpuid_vendor.str); > > return value; > > } > > > > @@ -1230,7 +1184,6 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > > { > > X86CPU *cpu = X86_CPU(obj); > > CPUX86State *env = &cpu->env; > > - int i; > > > > if (strlen(value) != CPUID_VENDOR_SZ) { > > error_set(errp, QERR_PROPERTY_VALUE_BAD, "", > > @@ -1238,14 +1191,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > > return; > > } > > > > - env->cpuid_vendor1 = 0; > > - env->cpuid_vendor2 = 0; > > - env->cpuid_vendor3 = 0; > > - for (i = 0; i < 4; i++) { > > - env->cpuid_vendor1 |= ((uint8_t)value[i ]) << (8 * i); > > - env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i); > > - env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i); > > - } > > + pstrcpy(env->cpuid_vendor.str, sizeof(env->cpuid_vendor.str), value); > > env->cpuid_vendor_override = 1; > > } > > > > @@ -1341,7 +1287,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name) > > */ > > static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > > { > > - unsigned int i; > > char *featurestr; /* Single 'key=value" string being parsed */ > > /* Features to be added */ > > FeatureWordArray plus_features = { 0 }; > > @@ -1403,18 +1348,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) > > } > > x86_cpu_def->xlevel = numvalue; > > } else if (!strcmp(featurestr, "vendor")) { > > - if (strlen(val) != 12) { > > - fprintf(stderr, "vendor string must be 12 chars long\n"); > > - goto error; > > - } > > - x86_cpu_def->vendor1 = 0; > > - x86_cpu_def->vendor2 = 0; > > - x86_cpu_def->vendor3 = 0; > > - for(i = 0; i < 4; i++) { > > - x86_cpu_def->vendor1 |= ((uint8_t)val[i ]) << (8 * i); > > - x86_cpu_def->vendor2 |= ((uint8_t)val[i + 4]) << (8 * i); > > - x86_cpu_def->vendor3 |= ((uint8_t)val[i + 8]) << (8 * i); > > - } > > + 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), > > @@ -1609,10 +1543,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > > error_setg(&error, "Invalid cpu_model string format: %s", cpu_model); > > goto out; > > } > > - assert(def->vendor1); > > - env->cpuid_vendor1 = def->vendor1; > > - env->cpuid_vendor2 = def->vendor2; > > - env->cpuid_vendor3 = def->vendor3; > > + assert(def->vendor[0]); > > + object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error); > > env->cpuid_vendor_override = def->vendor_override; > > object_property_set_int(OBJECT(cpu), def->level, "level", &error); > > object_property_set_int(OBJECT(cpu), def->family, "family", &error); > > @@ -1682,9 +1614,9 @@ void x86_cpudef_setup(void) > > static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx, > > uint32_t *ecx, uint32_t *edx) > > { > > - *ebx = env->cpuid_vendor1; > > - *edx = env->cpuid_vendor2; > > - *ecx = env->cpuid_vendor3; > > + cpu_to_le32wu(ebx, env->cpuid_vendor.regs.ebx); > > + cpu_to_le32wu(edx, env->cpuid_vendor.regs.edx); > > + cpu_to_le32wu(ecx, env->cpuid_vendor.regs.ecx); > > > > /* sysenter isn't supported on compatibility mode on AMD, syscall > > * isn't supported in compatibility mode on Intel. > > @@ -1862,9 +1794,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > > break; > > case 0x80000000: > > *eax = env->cpuid_xlevel; > > - *ebx = env->cpuid_vendor1; > > - *edx = env->cpuid_vendor2; > > - *ecx = env->cpuid_vendor3; > > + *ebx = env->cpuid_vendor.regs.ebx; > > + *edx = env->cpuid_vendor.regs.edx; > > + *ecx = env->cpuid_vendor.regs.ecx; > > break; > > case 0x80000001: > > *eax = env->cpuid_version; > > @@ -1877,11 +1809,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > > * So dont set it here for Intel to make Linux guests happy. > > */ > > if (cs->nr_cores * cs->nr_threads > 1) { > > - uint32_t tebx, tecx, tedx; > > - get_cpuid_vendor(env, &tebx, &tecx, &tedx); > > - if (tebx != CPUID_VENDOR_INTEL_1 || > > - tedx != CPUID_VENDOR_INTEL_2 || > > - tecx != CPUID_VENDOR_INTEL_3) { > > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_INTEL, > > + sizeof(env->cpuid_vendor.str))) { > > *ecx |= 1 << 1; /* CmpLegacy bit */ > > } > > } > > @@ -2152,9 +2081,8 @@ void x86_cpu_realize(Object *obj, Error **errp) > > /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on > > * CPUID[1].EDX. > > */ > > - if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && > > - env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && > > - env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) { > > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_AMD, > > + sizeof(env->cpuid_vendor.str))) { > > env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES; > > env->cpuid_ext2_features |= (env->cpuid_features > > & CPUID_EXT2_AMD_ALIASES); > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > > index e4a7c50..09a3b18 100644 > > --- a/target-i386/cpu.h > > +++ b/target-i386/cpu.h > > @@ -531,14 +531,14 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; > > #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */ > > #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */ > > #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */ > > +#define CPUID_VENDOR_INTEL "GenuineIntel" > > > > #define CPUID_VENDOR_AMD_1 0x68747541 /* "Auth" */ > > #define CPUID_VENDOR_AMD_2 0x69746e65 /* "enti" */ > > #define CPUID_VENDOR_AMD_3 0x444d4163 /* "cAMD" */ > > +#define CPUID_VENDOR_AMD "AuthenticAMD" > > why not also remove these 2 VENDOR_{INTEL,AMD}_{1,2,3} ? Indeed with this patch VENDOR_AMD_{1,2,3} unused, and could be deleted, but CPUID_VENDOR_INTEL_{1,2,3} is used elsewhere. This said, this patch looks more intrusive, could you consider to review original patch: "[PATCH 05/17] target-i386: replace uint32_t vendor fields by vendor string in x86_def_t" instead in this series, pls? > > > > > -#define CPUID_VENDOR_VIA_1 0x746e6543 /* "Cent" */ > > -#define CPUID_VENDOR_VIA_2 0x48727561 /* "aurH" */ > > -#define CPUID_VENDOR_VIA_3 0x736c7561 /* "auls" */ > > +#define CPUID_VENDOR_VIA "CentaurHauls" > > > > #define CPUID_MWAIT_IBE (1 << 1) /* Interrupts can exit capability */ > > #define CPUID_MWAIT_EMX (1 << 0) /* enumeration supported */ > > @@ -818,9 +818,14 @@ typedef struct CPUX86State { > > > > /* processor features (e.g. for CPUID insn) */ > > uint32_t cpuid_level; > > - uint32_t cpuid_vendor1; > > - uint32_t cpuid_vendor2; > > - uint32_t cpuid_vendor3; > > + union { > > + struct __attribute__((packed)) { > > + uint32_t ebx; > > + uint32_t edx; > > + uint32_t ecx; > > + } regs; > > + char str[CPUID_VENDOR_SZ + 1]; > > + } cpuid_vendor; > > uint32_t cpuid_version; > > uint32_t cpuid_features; > > uint32_t cpuid_ext_features; > > diff --git a/target-i386/translate.c b/target-i386/translate.c > > index 32d21f5..985080b 100644 > > --- a/target-i386/translate.c > > +++ b/target-i386/translate.c > > @@ -7028,7 +7028,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > > break; > > case 0x134: /* sysenter */ > > /* For Intel SYSENTER is valid on 64-bit */ > > - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) > > + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) > > goto illegal_op; > > if (!s->pe) { > > gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); > > @@ -7041,7 +7041,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > > break; > > case 0x135: /* sysexit */ > > /* For Intel SYSEXIT is valid on 64-bit */ > > - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) > > + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) > > goto illegal_op; > > if (!s->pe) { > > gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); > > -- > regards! > li guang >
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 333745b..882da50 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -45,6 +45,18 @@ #include "hw/apic_internal.h" #endif +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, + uint32_t vendor2, uint32_t vendor3) +{ + int i; + for (i = 0; i < 4; i++) { + dst[i] = vendor1 >> (8 * i); + dst[i + 4] = vendor2 >> (8 * i); + dst[i + 8] = vendor3 >> (8 * i); + } + dst[CPUID_VENDOR_SZ] = '\0'; +} + /* feature flags taken from "Intel Processor Identification and the CPUID * Instruction" and AMD's "CPUID Specification". In cases of disagreement * between feature naming conventions, aliases may be added. @@ -341,7 +353,7 @@ typedef struct x86_def_t { struct x86_def_t *next; const char *name; uint32_t level; - uint32_t vendor1, vendor2, vendor3; + char vendor[CPUID_VENDOR_SZ + 1]; int family; int model; int stepping; @@ -406,9 +418,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "qemu64", .level = 4, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 6, .model = 2, .stepping = 3, @@ -425,9 +435,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "phenom", .level = 5, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 16, .model = 2, .stepping = 3, @@ -453,9 +461,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "core2duo", .level = 10, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 15, .stepping = 11, @@ -474,9 +480,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "kvm64", .level = 5, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 15, .model = 6, .stepping = 1, @@ -500,9 +504,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "qemu32", .level = 4, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 3, .stepping = 3, @@ -513,9 +515,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "kvm32", .level = 5, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 15, .model = 6, .stepping = 1, @@ -530,9 +530,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "coreduo", .level = 10, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 14, .stepping = 8, @@ -548,9 +546,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "486", .level = 1, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 4, .model = 0, .stepping = 0, @@ -560,9 +556,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "pentium", .level = 1, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 5, .model = 4, .stepping = 3, @@ -572,9 +566,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "pentium2", .level = 2, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 5, .stepping = 2, @@ -584,9 +576,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "pentium3", .level = 2, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 7, .stepping = 3, @@ -596,9 +586,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "athlon", .level = 2, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 6, .model = 2, .stepping = 3, @@ -612,9 +600,7 @@ static x86_def_t builtin_x86_defs[] = { .name = "n270", /* original is on level 10 */ .level = 5, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 28, .stepping = 2, @@ -633,9 +619,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Conroe", .level = 2, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 2, .stepping = 3, @@ -653,9 +637,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Penryn", .level = 2, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 2, .stepping = 3, @@ -674,9 +656,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Nehalem", .level = 2, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 2, .stepping = 3, @@ -695,9 +675,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Westmere", .level = 11, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 44, .stepping = 1, @@ -717,9 +695,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "SandyBridge", .level = 0xd, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 42, .stepping = 1, @@ -742,9 +718,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Haswell", .level = 0xd, - .vendor1 = CPUID_VENDOR_INTEL_1, - .vendor2 = CPUID_VENDOR_INTEL_2, - .vendor3 = CPUID_VENDOR_INTEL_3, + .vendor = CPUID_VENDOR_INTEL, .family = 6, .model = 60, .stepping = 1, @@ -772,9 +746,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Opteron_G1", .level = 5, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 15, .model = 6, .stepping = 1, @@ -796,9 +768,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Opteron_G2", .level = 5, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 15, .model = 6, .stepping = 1, @@ -822,9 +792,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Opteron_G3", .level = 5, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 15, .model = 6, .stepping = 1, @@ -850,9 +818,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Opteron_G4", .level = 0xd, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 21, .model = 1, .stepping = 2, @@ -882,9 +848,7 @@ static x86_def_t builtin_x86_defs[] = { { .name = "Opteron_G5", .level = 0xd, - .vendor1 = CPUID_VENDOR_AMD_1, - .vendor2 = CPUID_VENDOR_AMD_2, - .vendor3 = CPUID_VENDOR_AMD_3, + .vendor = CPUID_VENDOR_AMD, .family = 21, .model = 2, .stepping = 0, @@ -945,9 +909,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) x86_cpu_def->name = "host"; host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); - x86_cpu_def->vendor1 = ebx; - x86_cpu_def->vendor2 = edx; - x86_cpu_def->vendor3 = ecx; + x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF); @@ -975,9 +937,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) x86_cpu_def->vendor_override = 0; /* Call Centaur's CPUID instruction. */ - if (x86_cpu_def->vendor1 == CPUID_VENDOR_VIA_1 && - x86_cpu_def->vendor2 == CPUID_VENDOR_VIA_2 && - x86_cpu_def->vendor3 == CPUID_VENDOR_VIA_3) { + if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) { host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx); eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX); if (eax >= 0xC0000001) { @@ -1213,15 +1173,9 @@ static char *x86_cpuid_get_vendor(Object *obj, Error **errp) X86CPU *cpu = X86_CPU(obj); CPUX86State *env = &cpu->env; char *value; - int i; value = (char *)g_malloc(CPUID_VENDOR_SZ + 1); - for (i = 0; i < 4; i++) { - value[i ] = env->cpuid_vendor1 >> (8 * i); - value[i + 4] = env->cpuid_vendor2 >> (8 * i); - value[i + 8] = env->cpuid_vendor3 >> (8 * i); - } - value[CPUID_VENDOR_SZ] = '\0'; + pstrcpy(value, CPUID_VENDOR_SZ + 1, env->cpuid_vendor.str); return value; } @@ -1230,7 +1184,6 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, { X86CPU *cpu = X86_CPU(obj); CPUX86State *env = &cpu->env; - int i; if (strlen(value) != CPUID_VENDOR_SZ) { error_set(errp, QERR_PROPERTY_VALUE_BAD, "", @@ -1238,14 +1191,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, return; } - env->cpuid_vendor1 = 0; - env->cpuid_vendor2 = 0; - env->cpuid_vendor3 = 0; - for (i = 0; i < 4; i++) { - env->cpuid_vendor1 |= ((uint8_t)value[i ]) << (8 * i); - env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i); - env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i); - } + pstrcpy(env->cpuid_vendor.str, sizeof(env->cpuid_vendor.str), value); env->cpuid_vendor_override = 1; } @@ -1341,7 +1287,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name) */ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) { - unsigned int i; char *featurestr; /* Single 'key=value" string being parsed */ /* Features to be added */ FeatureWordArray plus_features = { 0 }; @@ -1403,18 +1348,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) } x86_cpu_def->xlevel = numvalue; } else if (!strcmp(featurestr, "vendor")) { - if (strlen(val) != 12) { - fprintf(stderr, "vendor string must be 12 chars long\n"); - goto error; - } - x86_cpu_def->vendor1 = 0; - x86_cpu_def->vendor2 = 0; - x86_cpu_def->vendor3 = 0; - for(i = 0; i < 4; i++) { - x86_cpu_def->vendor1 |= ((uint8_t)val[i ]) << (8 * i); - x86_cpu_def->vendor2 |= ((uint8_t)val[i + 4]) << (8 * i); - x86_cpu_def->vendor3 |= ((uint8_t)val[i + 8]) << (8 * i); - } + 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), @@ -1609,10 +1543,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model) error_setg(&error, "Invalid cpu_model string format: %s", cpu_model); goto out; } - assert(def->vendor1); - env->cpuid_vendor1 = def->vendor1; - env->cpuid_vendor2 = def->vendor2; - env->cpuid_vendor3 = def->vendor3; + assert(def->vendor[0]); + object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error); env->cpuid_vendor_override = def->vendor_override; object_property_set_int(OBJECT(cpu), def->level, "level", &error); object_property_set_int(OBJECT(cpu), def->family, "family", &error); @@ -1682,9 +1614,9 @@ void x86_cpudef_setup(void) static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx, uint32_t *ecx, uint32_t *edx) { - *ebx = env->cpuid_vendor1; - *edx = env->cpuid_vendor2; - *ecx = env->cpuid_vendor3; + cpu_to_le32wu(ebx, env->cpuid_vendor.regs.ebx); + cpu_to_le32wu(edx, env->cpuid_vendor.regs.edx); + cpu_to_le32wu(ecx, env->cpuid_vendor.regs.ecx); /* sysenter isn't supported on compatibility mode on AMD, syscall * isn't supported in compatibility mode on Intel. @@ -1862,9 +1794,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 0x80000000: *eax = env->cpuid_xlevel; - *ebx = env->cpuid_vendor1; - *edx = env->cpuid_vendor2; - *ecx = env->cpuid_vendor3; + *ebx = env->cpuid_vendor.regs.ebx; + *edx = env->cpuid_vendor.regs.edx; + *ecx = env->cpuid_vendor.regs.ecx; break; case 0x80000001: *eax = env->cpuid_version; @@ -1877,11 +1809,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, * So dont set it here for Intel to make Linux guests happy. */ if (cs->nr_cores * cs->nr_threads > 1) { - uint32_t tebx, tecx, tedx; - get_cpuid_vendor(env, &tebx, &tecx, &tedx); - if (tebx != CPUID_VENDOR_INTEL_1 || - tedx != CPUID_VENDOR_INTEL_2 || - tecx != CPUID_VENDOR_INTEL_3) { + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_INTEL, + sizeof(env->cpuid_vendor.str))) { *ecx |= 1 << 1; /* CmpLegacy bit */ } } @@ -2152,9 +2081,8 @@ void x86_cpu_realize(Object *obj, Error **errp) /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on * CPUID[1].EDX. */ - if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && - env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && - env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) { + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_AMD, + sizeof(env->cpuid_vendor.str))) { env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES; env->cpuid_ext2_features |= (env->cpuid_features & CPUID_EXT2_AMD_ALIASES); diff --git a/target-i386/cpu.h b/target-i386/cpu.h index e4a7c50..09a3b18 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -531,14 +531,14 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */ #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */ #define CPUID_VENDOR_INTEL_3 0x6c65746e /* "ntel" */ +#define CPUID_VENDOR_INTEL "GenuineIntel" #define CPUID_VENDOR_AMD_1 0x68747541 /* "Auth" */ #define CPUID_VENDOR_AMD_2 0x69746e65 /* "enti" */ #define CPUID_VENDOR_AMD_3 0x444d4163 /* "cAMD" */ +#define CPUID_VENDOR_AMD "AuthenticAMD" -#define CPUID_VENDOR_VIA_1 0x746e6543 /* "Cent" */ -#define CPUID_VENDOR_VIA_2 0x48727561 /* "aurH" */ -#define CPUID_VENDOR_VIA_3 0x736c7561 /* "auls" */ +#define CPUID_VENDOR_VIA "CentaurHauls" #define CPUID_MWAIT_IBE (1 << 1) /* Interrupts can exit capability */ #define CPUID_MWAIT_EMX (1 << 0) /* enumeration supported */ @@ -818,9 +818,14 @@ typedef struct CPUX86State { /* processor features (e.g. for CPUID insn) */ uint32_t cpuid_level; - uint32_t cpuid_vendor1; - uint32_t cpuid_vendor2; - uint32_t cpuid_vendor3; + union { + struct __attribute__((packed)) { + uint32_t ebx; + uint32_t edx; + uint32_t ecx; + } regs; + char str[CPUID_VENDOR_SZ + 1]; + } cpuid_vendor; uint32_t cpuid_version; uint32_t cpuid_features; uint32_t cpuid_ext_features; diff --git a/target-i386/translate.c b/target-i386/translate.c index 32d21f5..985080b 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -7028,7 +7028,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, break; case 0x134: /* sysenter */ /* For Intel SYSENTER is valid on 64-bit */ - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) goto illegal_op; if (!s->pe) { gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); @@ -7041,7 +7041,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, break; case 0x135: /* sysexit */ /* For Intel SYSEXIT is valid on 64-bit */ - if (CODE64(s) && env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1) + if (CODE64(s) && env->cpuid_vendor.regs.ebx != CPUID_VENDOR_INTEL_1) goto illegal_op; if (!s->pe) { gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
Tested on x86 host yet, I don't have bigendian host avilable right now. Vendor property setter takes string as vendor value but cpudefs use uint32_t vendor[123] fields to define vendor value. It makes it difficult to unify and use property setter for values from cpudefs. Simplify code by using vendor property setter, vendor[123] fields are converted into vendor[13] array to keep its value. And vendor property setter is used to access/set value on CPU. Extra: - replace cpuid_vendor[1.2.3] words in CPUX86State with union to simplify vendor property setter and pack/unpack procedures - add x86_cpu_vendor_words2str() to make for() cycle reusable - convert words in cpuid_vendor union to little-endian when returning them to guest in cpuid instruction emulation, since they are not packed manualy anymore Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v4: - use union for cpuid_vendor to simplify convertions string<=>registers v3: - replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str() due to renaming of the last in previous patch - rebased with "target-i386: move out CPU features initialization in separate func" patch dropped v2: - restore deleted host_cpuid() call in kvm_cpu_fill_host() Spotted-By: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 180 ++++++++++++++--------------------------------- target-i386/cpu.h | 17 +++-- target-i386/translate.c | 4 +- 3 files changed, 67 insertions(+), 134 deletions(-)