From: Igor Mammedov <imammedo@redhat.com>
To: li guang <lig.fnst@cn.fujitsu.com>
Cc: qemu-devel@nongnu.org, afaerber@suse.de, ehabkost@redhat.com
Subject: Re: [Qemu-devel] [PATCH] target-i386: make x86_def_t.vendor[1, 2, 3] a string and use cpuid_vendor union in CPUX86State
Date: Wed, 16 Jan 2013 08:31:50 +0100 [thread overview]
Message-ID: <20130116083150.3a18ff03@thinkpad.mammed.net> (raw)
In-Reply-To: <1358320025.23348.11.camel@liguang.fnst.cn.fujitsu.com>
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
>
--
Regards,
Igor
next prev parent reply other threads:[~2013-01-16 7:32 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-11 2:10 [Qemu-devel] [PATCH qom-cpu 00/17] x86 CPU cleanup, part 3 Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 01/17] target-i386: move setting defaults out of cpu_x86_parse_featurestr() Igor Mammedov
2013-01-11 2:20 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 02/17] target-i386: cpu_x86_register() consolidate freeing resources Igor Mammedov
2013-01-11 2:21 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 03/17] target-i386: move kvm_check_features_against_host() check to realize time Igor Mammedov
2013-01-11 2:25 ` Eduardo Habkost
2013-04-02 20:30 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 04/17] target-i386: add x86_cpu_vendor_words2str() Igor Mammedov
2013-01-11 2:28 ` Eduardo Habkost
2013-01-14 17:14 ` Andreas Färber
2013-01-14 18:24 ` Igor Mammedov
2013-01-14 18:33 ` [Qemu-devel] [PATCH 04/17 v3] " Igor Mammedov
2013-01-16 2:26 ` li guang
2013-01-11 2:46 ` [Qemu-devel] [PATCH 04/17 v2] " Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 05/17] target-i386: replace uint32_t vendor fields by vendor string in x86_def_t Igor Mammedov
2013-01-11 2:30 ` Eduardo Habkost
2013-01-14 19:32 ` Andreas Färber
2013-01-14 21:44 ` Eduardo Habkost
2013-01-15 12:06 ` [Qemu-devel] [PATCH] target-i386: make x86_def_t.vendor[1, 2, 3] a string and use cpuid_vendor union in CPUX86State Igor Mammedov
2013-01-15 17:51 ` Eduardo Habkost
2013-01-15 19:55 ` Igor Mammedov
2013-01-16 7:07 ` li guang
2013-01-16 7:31 ` Igor Mammedov [this message]
2013-01-14 21:52 ` [Qemu-devel] [PATCH 05/17] target-i386: replace uint32_t vendor fields by vendor string in x86_def_t Igor Mammedov
2013-01-15 10:53 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 06/17] target-i386: remove vendor_override field from CPUX86State Igor Mammedov
2013-01-11 2:42 ` Eduardo Habkost
2013-01-11 3:09 ` Igor Mammedov
2013-01-11 12:04 ` Eduardo Habkost
2013-01-11 12:06 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 07/17] target-i386: prepare cpu_x86_parse_featurestr() to return a set of key, value property pairs Igor Mammedov
2013-01-14 15:14 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 08/17] target-i386: set custom 'vendor' without intermediate x86_def_t Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 09/17] target-i386: print deprecated warning if xlevel < 0x80000000 Igor Mammedov
2013-01-14 15:16 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 10/17] target-i386: set custom 'xlevel' without intermediate x86_def_t Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 11/17] target-i386: set custom 'level' " Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 12/17] target-i386: set custom 'model-id' " Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 13/17] target-i386: set custom 'stepping' " Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 14/17] target-i386: set custom 'model' " Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 15/17] target-i386: set custom 'family' " Igor Mammedov
2013-01-11 2:10 ` [Qemu-devel] [PATCH 16/17] target-i386: set custom 'tsc-frequency' " Igor Mammedov
2013-01-14 15:20 ` Eduardo Habkost
2013-01-14 15:49 ` Igor Mammedov
2013-01-14 16:10 ` Eduardo Habkost
2013-01-11 2:10 ` [Qemu-devel] [PATCH 17/17] target-i386: remove setting tsc-frequency from x86_def_t Igor Mammedov
2013-01-14 15:21 ` Eduardo Habkost
2013-01-15 4:16 ` [Qemu-devel] [PATCH qom-cpu 00/17] x86 CPU cleanup, part 3 Andreas Färber
2013-01-15 9:03 ` Igor Mammedov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130116083150.3a18ff03@thinkpad.mammed.net \
--to=imammedo@redhat.com \
--cc=afaerber@suse.de \
--cc=ehabkost@redhat.com \
--cc=lig.fnst@cn.fujitsu.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.