From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57650) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvNTd-0002ZG-Kr for qemu-devel@nongnu.org; Wed, 16 Jan 2013 02:32:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TvNTa-0002SX-Ub for qemu-devel@nongnu.org; Wed, 16 Jan 2013 02:32:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:22316) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvNTa-0002SO-J3 for qemu-devel@nongnu.org; Wed, 16 Jan 2013 02:31:58 -0500 Date: Wed, 16 Jan 2013 08:31:50 +0100 From: Igor Mammedov Message-ID: <20130116083150.3a18ff03@thinkpad.mammed.net> In-Reply-To: <1358320025.23348.11.camel@liguang.fnst.cn.fujitsu.com> References: <20130114214400.GA4168@otherpad.lan.raisama.net> <1358251588-18376-1-git-send-email-imammedo@redhat.com> <1358320025.23348.11.camel@liguang.fnst.cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] target-i386: make x86_def_t.vendor[1, 2, 3] a string and use cpuid_vendor union in CPUX86State List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: li guang Cc: qemu-devel@nongnu.org, afaerber@suse.de, ehabkost@redhat.com On Wed, 16 Jan 2013 15:07:05 +0800 li guang wrote: > =E5=9C=A8 2013-01-15=E4=BA=8C=E7=9A=84 13:06 +0100=EF=BC=8CIgor Mammedov= =E5=86=99=E9=81=93=EF=BC=9A > > Tested on x86 host yet, I don't have bigendian host avilable right now. > >=20 > > 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. > >=20 > > 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. > >=20 > > 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 > >=20 > > Signed-off-by: Igor Mammedov > > --- > > v4: > > - use union for cpuid_vendor to simplify convertions string<=3D>regist= ers > > 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 > > --- > > target-i386/cpu.c | 180 ++++++++++++++-------------------------= -------- > > target-i386/cpu.h | 17 +++-- > > target-i386/translate.c | 4 +- > > 3 files changed, 67 insertions(+), 134 deletions(-) > >=20 > > 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 > > =20 > > +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > > + uint32_t vendor2, uint32_t vendor= 3) > > +{ > > + int i; > > + for (i =3D 0; i < 4; i++) { > > + dst[i] =3D vendor1 >> (8 * i); > > + dst[i + 4] =3D vendor2 >> (8 * i); > > + dst[i + 8] =3D vendor3 >> (8 * i); > > + } > > + dst[CPUID_VENDOR_SZ] =3D '\0'; > > +} > > + > > /* feature flags taken from "Intel Processor Identification and the CP= UID > > * Instruction" and AMD's "CPUID Specification". In cases of disagree= ment > > * 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[] =3D { > > { > > .name =3D "qemu64", > > .level =3D 4, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 6, > > .model =3D 2, > > .stepping =3D 3, > > @@ -425,9 +435,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "phenom", > > .level =3D 5, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 16, > > .model =3D 2, > > .stepping =3D 3, > > @@ -453,9 +461,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "core2duo", > > .level =3D 10, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 15, > > .stepping =3D 11, > > @@ -474,9 +480,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "kvm64", > > .level =3D 5, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 15, > > .model =3D 6, > > .stepping =3D 1, > > @@ -500,9 +504,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "qemu32", > > .level =3D 4, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 3, > > .stepping =3D 3, > > @@ -513,9 +515,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "kvm32", > > .level =3D 5, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 15, > > .model =3D 6, > > .stepping =3D 1, > > @@ -530,9 +530,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "coreduo", > > .level =3D 10, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 14, > > .stepping =3D 8, > > @@ -548,9 +546,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "486", > > .level =3D 1, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 4, > > .model =3D 0, > > .stepping =3D 0, > > @@ -560,9 +556,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "pentium", > > .level =3D 1, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 5, > > .model =3D 4, > > .stepping =3D 3, > > @@ -572,9 +566,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "pentium2", > > .level =3D 2, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 5, > > .stepping =3D 2, > > @@ -584,9 +576,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "pentium3", > > .level =3D 2, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 7, > > .stepping =3D 3, > > @@ -596,9 +586,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "athlon", > > .level =3D 2, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 6, > > .model =3D 2, > > .stepping =3D 3, > > @@ -612,9 +600,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > .name =3D "n270", > > /* original is on level 10 */ > > .level =3D 5, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 28, > > .stepping =3D 2, > > @@ -633,9 +619,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Conroe", > > .level =3D 2, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 2, > > .stepping =3D 3, > > @@ -653,9 +637,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Penryn", > > .level =3D 2, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 2, > > .stepping =3D 3, > > @@ -674,9 +656,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Nehalem", > > .level =3D 2, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 2, > > .stepping =3D 3, > > @@ -695,9 +675,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Westmere", > > .level =3D 11, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 44, > > .stepping =3D 1, > > @@ -717,9 +695,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "SandyBridge", > > .level =3D 0xd, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 42, > > .stepping =3D 1, > > @@ -742,9 +718,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Haswell", > > .level =3D 0xd, > > - .vendor1 =3D CPUID_VENDOR_INTEL_1, > > - .vendor2 =3D CPUID_VENDOR_INTEL_2, > > - .vendor3 =3D CPUID_VENDOR_INTEL_3, > > + .vendor =3D CPUID_VENDOR_INTEL, > > .family =3D 6, > > .model =3D 60, > > .stepping =3D 1, > > @@ -772,9 +746,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Opteron_G1", > > .level =3D 5, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 15, > > .model =3D 6, > > .stepping =3D 1, > > @@ -796,9 +768,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Opteron_G2", > > .level =3D 5, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 15, > > .model =3D 6, > > .stepping =3D 1, > > @@ -822,9 +792,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Opteron_G3", > > .level =3D 5, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 15, > > .model =3D 6, > > .stepping =3D 1, > > @@ -850,9 +818,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Opteron_G4", > > .level =3D 0xd, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 21, > > .model =3D 1, > > .stepping =3D 2, > > @@ -882,9 +848,7 @@ static x86_def_t builtin_x86_defs[] =3D { > > { > > .name =3D "Opteron_G5", > > .level =3D 0xd, > > - .vendor1 =3D CPUID_VENDOR_AMD_1, > > - .vendor2 =3D CPUID_VENDOR_AMD_2, > > - .vendor3 =3D CPUID_VENDOR_AMD_3, > > + .vendor =3D CPUID_VENDOR_AMD, > > .family =3D 21, > > .model =3D 2, > > .stepping =3D 0, > > @@ -945,9 +909,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_de= f) > > =20 > > x86_cpu_def->name =3D "host"; > > host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); > > - x86_cpu_def->vendor1 =3D ebx; > > - x86_cpu_def->vendor2 =3D edx; > > - x86_cpu_def->vendor3 =3D ecx; > > + x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); > > =20 > > host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); > > x86_cpu_def->family =3D ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF); > > @@ -975,9 +937,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_de= f) > > x86_cpu_def->vendor_override =3D 0; > > =20 > > /* Call Centaur's CPUID instruction. */ > > - if (x86_cpu_def->vendor1 =3D=3D CPUID_VENDOR_VIA_1 && > > - x86_cpu_def->vendor2 =3D=3D CPUID_VENDOR_VIA_2 && > > - x86_cpu_def->vendor3 =3D=3D CPUID_VENDOR_VIA_3) { > > + 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_EAX); > > if (eax >=3D 0xC0000001) { > > @@ -1213,15 +1173,9 @@ static char *x86_cpuid_get_vendor(Object *obj, E= rror **errp) > > X86CPU *cpu =3D X86_CPU(obj); > > CPUX86State *env =3D &cpu->env; > > char *value; > > - int i; > > =20 > > value =3D (char *)g_malloc(CPUID_VENDOR_SZ + 1); > > - for (i =3D 0; i < 4; i++) { > > - value[i ] =3D env->cpuid_vendor1 >> (8 * i); > > - value[i + 4] =3D env->cpuid_vendor2 >> (8 * i); > > - value[i + 8] =3D env->cpuid_vendor3 >> (8 * i); > > - } > > - value[CPUID_VENDOR_SZ] =3D '\0'; > > + pstrcpy(value, CPUID_VENDOR_SZ + 1, env->cpuid_vendor.str); > > return value; > > } > > =20 > > @@ -1230,7 +1184,6 @@ static void x86_cpuid_set_vendor(Object *obj, con= st char *value, > > { > > X86CPU *cpu =3D X86_CPU(obj); > > CPUX86State *env =3D &cpu->env; > > - int i; > > =20 > > if (strlen(value) !=3D CPUID_VENDOR_SZ) { > > error_set(errp, QERR_PROPERTY_VALUE_BAD, "", > > @@ -1238,14 +1191,7 @@ static void x86_cpuid_set_vendor(Object *obj, co= nst char *value, > > return; > > } > > =20 > > - env->cpuid_vendor1 =3D 0; > > - env->cpuid_vendor2 =3D 0; > > - env->cpuid_vendor3 =3D 0; > > - for (i =3D 0; i < 4; i++) { > > - env->cpuid_vendor1 |=3D ((uint8_t)value[i ]) << (8 * i); > > - env->cpuid_vendor2 |=3D ((uint8_t)value[i + 4]) << (8 * i); > > - env->cpuid_vendor3 |=3D ((uint8_t)value[i + 8]) << (8 * i); > > - } > > + pstrcpy(env->cpuid_vendor.str, sizeof(env->cpuid_vendor.str), valu= e); > > env->cpuid_vendor_override =3D 1; > > } > > =20 > > @@ -1341,7 +1287,6 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cp= u_def, const char *name) > > */ > > static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *feat= ures) > > { > > - unsigned int i; > > char *featurestr; /* Single 'key=3Dvalue" string being parsed */ > > /* Features to be added */ > > FeatureWordArray plus_features =3D { 0 }; > > @@ -1403,18 +1348,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x= 86_cpu_def, char *features) > > } > > x86_cpu_def->xlevel =3D numvalue; > > } else if (!strcmp(featurestr, "vendor")) { > > - if (strlen(val) !=3D 12) { > > - fprintf(stderr, "vendor string must be 12 chars lo= ng\n"); > > - goto error; > > - } > > - x86_cpu_def->vendor1 =3D 0; > > - x86_cpu_def->vendor2 =3D 0; > > - x86_cpu_def->vendor3 =3D 0; > > - for(i =3D 0; i < 4; i++) { > > - x86_cpu_def->vendor1 |=3D ((uint8_t)val[i ]) <<= (8 * i); > > - x86_cpu_def->vendor2 |=3D ((uint8_t)val[i + 4]) <<= (8 * i); > > - x86_cpu_def->vendor3 |=3D ((uint8_t)val[i + 8]) <<= (8 * i); > > - } > > + pstrcpy(x86_cpu_def->vendor, sizeof(x86_cpu_def->vendo= r), val); > > x86_cpu_def->vendor_override =3D 1; > > } else if (!strcmp(featurestr, "model_id")) { > > pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->mod= el_id), > > @@ -1609,10 +1543,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cp= u_model) > > error_setg(&error, "Invalid cpu_model string format: %s", cpu_= model); > > goto out; > > } > > - assert(def->vendor1); > > - env->cpuid_vendor1 =3D def->vendor1; > > - env->cpuid_vendor2 =3D def->vendor2; > > - env->cpuid_vendor3 =3D def->vendor3; > > + assert(def->vendor[0]); > > + object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error= ); > > env->cpuid_vendor_override =3D 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 =3D env->cpuid_vendor1; > > - *edx =3D env->cpuid_vendor2; > > - *ecx =3D 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); > > =20 > > /* 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 ind= ex, uint32_t count, > > break; > > case 0x80000000: > > *eax =3D env->cpuid_xlevel; > > - *ebx =3D env->cpuid_vendor1; > > - *edx =3D env->cpuid_vendor2; > > - *ecx =3D env->cpuid_vendor3; > > + *ebx =3D env->cpuid_vendor.regs.ebx; > > + *edx =3D env->cpuid_vendor.regs.edx; > > + *ecx =3D env->cpuid_vendor.regs.ecx; > > break; > > case 0x80000001: > > *eax =3D env->cpuid_version; > > @@ -1877,11 +1809,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t in= dex, 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 !=3D CPUID_VENDOR_INTEL_1 || > > - tedx !=3D CPUID_VENDOR_INTEL_2 || > > - tecx !=3D CPUID_VENDOR_INTEL_3) { > > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_INTEL, > > + sizeof(env->cpuid_vendor.str))) { > > *ecx |=3D 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 =3D=3D CPUID_VENDOR_AMD_1 && > > - env->cpuid_vendor2 =3D=3D CPUID_VENDOR_AMD_2 && > > - env->cpuid_vendor3 =3D=3D CPUID_VENDOR_AMD_3) { > > + if (!strncmp(env->cpuid_vendor.str, CPUID_VENDOR_AMD, > > + sizeof(env->cpuid_vendor.str))) { > > env->cpuid_ext2_features &=3D ~CPUID_EXT2_AMD_ALIASES; > > env->cpuid_ext2_features |=3D (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" > > =20 > > #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" >=20 > 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? >=20 > > =20 > > -#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" > > =20 > > #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 { > > =20 > > /* 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 !=3D CPUID_VENDOR_INTEL_1) > > + if (CODE64(s) && env->cpuid_vendor.regs.ebx !=3D 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 !=3D CPUID_VENDOR_INTEL_1) > > + if (CODE64(s) && env->cpuid_vendor.regs.ebx !=3D CPUID_VENDOR_= INTEL_1) > > goto illegal_op; > > if (!s->pe) { > > gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); >=20 > --=20 > regards! > li guang >=20 --=20 Regards, Igor