Skip to contentSkip to conversation

Allow setting the vendor_id string with x86's -cpu option

PATCH In review Dan Kenigsberg 4 participants 9 messages Nov 25, 2007 – Dec 21, 2007

lore
Share & tools
lore

Dan Kenigsberg

author
#0 lore raw

PATCH 1/1 Allow setting the vendor_id string with x86's -cpu option

Having AuthenticAMD hard-coded is nice, but allowing the user to impersonate whatever CPU she wants is even nicer.

Also, an English typo (due to me) is corrected.

Dan.

target-i386/helper2.c +10 -1
@@ -254,8 +254,17 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                     goto error;
                 }
                 x86_cpu_def->stepping = stepping;
+            }  else if (!strcmp(featurestr, "vendor")) {
+                if (strlen(val) != 12) {
+                    fprintf(stderr, "vendor string must be 12 chars long\n");
+                    x86_cpu_def = 0;
+                    goto error;
+                }
+                x86_cpu_def->vendor1 = *(uint32_t *)val;
+                x86_cpu_def->vendor2 = *(uint32_t *)(val + 4);
+                x86_cpu_def->vendor3 = *(uint32_t *)(val + 8);
             } else {
-                fprintf(stderr, "unregnized feature %s\n", featurestr);
+                fprintf(stderr, "unrecognized feature %s\n", featurestr);
                 x86_cpu_def = 0;
                 goto error;
             }
#1 lore raw
Quoted 12 lines · Dan Kenigsberg

Having AuthenticAMD hard-coded is nice, but allowing the user to impersonate whatever CPU she wants is even nicer.

Also, an English typo (due to me) is corrected.

Dan.

target-i386/helper2.c +0 -0
@@ -254,8 +254,17 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                     goto error;
                 }

target-i386/helper2.c static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) · line 265

Quoted 10 lines
target-i386/helper2.c +9 -0
                 x86_cpu_def->stepping = stepping;
+            }  else if (!strcmp(featurestr, "vendor")) {
+                if (strlen(val) != 12) {
+                    fprintf(stderr, "vendor string must be 12 chars long\n");
+                    x86_cpu_def = 0;
+                    goto error;
+                }
+                x86_cpu_def->vendor1 = *(uint32_t *)val;
+                x86_cpu_def->vendor2 = *(uint32_t *)(val + 4);
+                x86_cpu_def->vendor3 = *(uint32_t *)(val + 8);

Endianness bug.

Thiemo

author
#2 lore raw
Quoted 17 lines · Thiemo Seufer
Quoted 16 lines · Dan Kenigsberg

Having AuthenticAMD hard-coded is nice, but allowing the user to impersonate whatever CPU she wants is even nicer.

Also, an English typo (due to me) is corrected.

Dan.

target-i386/helper2.c +3 -0
@@ -254,8 +254,17 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                     goto error;
                 }
                 x86_cpu_def->stepping = stepping;
+            }  else if (!strcmp(featurestr, "vendor")) {
+                if (strlen(val) != 12) {
+                    fprintf(stderr, "vendor string must be 12 chars long\n");
Quoted 9 lines

target-i386/helper2.c static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) · line 265

Quoted 6 lines
target-i386/helper2.c +6 -0
+                    x86_cpu_def = 0;
+                    goto error;
+                }
+                x86_cpu_def->vendor1 = *(uint32_t *)val;
+                x86_cpu_def->vendor2 = *(uint32_t *)(val + 4);
+                x86_cpu_def->vendor3 = *(uint32_t *)(val + 8);

Endianness bug.

Indeed. Please consider the following:

target-i386/helper2.c +9 -0
index 67658e2..6109283 100644
@@ -254,6 +254,15 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                     goto error;
                 }
                 x86_cpu_def->stepping = stepping;
+            }  else if (!strcmp(featurestr, "vendor")) {
+                if (strlen(val) != 12) {
+                    fprintf(stderr, "vendor string must be 12 chars long\n");
+                    x86_cpu_def = 0;
+                    goto error;
+                }
+                x86_cpu_def->vendor1 = cpu_to_le32(*(uint32_t *)val);
+                x86_cpu_def->vendor2 = cpu_to_le32(*(uint32_t *)(val + 4));
+                x86_cpu_def->vendor3 = cpu_to_le32(*(uint32_t *)(val + 8));
             } else {
                 fprintf(stderr, "unrecognized feature %s\n", featurestr);
                 x86_cpu_def = 0;
#3 lore raw
Quoted 3 lines
(quoted diff) +2 -0
+                x86_cpu_def->vendor1 = cpu_to_le32(*(uint32_t *)val);
+                x86_cpu_def->vendor2 = cpu_to_le32(*(uint32_t *)(val +

4)); + x86_cpu_def->vendor3 = cpu_to_le32(*(uint32_t *)(val

Still not good enough. val might not be aligned.

Paul

author
#4 lore raw
Quoted 6 lines · Paul Brook
Quoted 3 lines
(quoted diff) +2 -0
+                x86_cpu_def->vendor1 = cpu_to_le32(*(uint32_t *)val);
+                x86_cpu_def->vendor2 = cpu_to_le32(*(uint32_t *)(val +

4)); + x86_cpu_def->vendor3 = cpu_to_le32(*(uint32_t *)(val

Still not good enough. val might not be aligned.

So, is it that my only hope is going back to basics?

target-i386/helper2.c +12 -0
index 67658e2..2ef4be3 100644
@@ -254,6 +254,18 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                     goto error;
                 }
                 x86_cpu_def->stepping = stepping;
+            }  else if (!strcmp(featurestr, "vendor")) {
+                if (strlen(val) != 12) {
+                    fprintf(stderr, "vendor string must be 12 chars long\n");
+                    x86_cpu_def = 0;
+                    goto error;
+                }
+                x86_cpu_def->vendor1 = val[0] + (val[1] << 8)
+                                     + (val[2] << 16) + (val[3] << 24);
+                x86_cpu_def->vendor2 = val[4] + (val[5] << 8)
+                                     + (val[6] << 16) + (val[7] << 24);
+                x86_cpu_def->vendor3 = val[8] + (val[9] << 8)
+                                     + (val[10] << 16) + (val[11] << 24);
             } else {
                 fprintf(stderr, "unrecognized feature %s\n", featurestr);
                 x86_cpu_def = 0;
#5 lore raw

target-i386/helper2.c static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) · line 268

Quoted 6 lines · Dan Kenigsberg
target-i386/helper2.c +6 -0
+                x86_cpu_def->vendor1 = val[0] + (val[1] << 8)
+                                     + (val[2] << 16) + (val[3] << 24);
+                x86_cpu_def->vendor2 = val[4] + (val[5] << 8)
+                                     + (val[6] << 16) + (val[7] << 24);
+                x86_cpu_def->vendor3 = val[8] + (val[9] << 8)
+                                     + (val[10] << 16) + (val[11] << 24);

That will do the wrong thing with the sign bit.

Andreas.

Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
author
#6 lore raw
Quoted 11 lines · Andreas Schwab

Review

Quoted 6 lines · Dan Kenigsberg
(quoted diff) +6 -0
+                x86_cpu_def->vendor1 = val[0] + (val[1] << 8)
+                                     + (val[2] << 16) + (val[3] << 24);
+                x86_cpu_def->vendor2 = val[4] + (val[5] << 8)
+                                     + (val[6] << 16) + (val[7] << 24);
+                x86_cpu_def->vendor3 = val[8] + (val[9] << 8)
+                                     + (val[10] << 16) + (val[11] << 24);

That will do the wrong thing with the sign bit.

Please tell me when I'm done making a fool of myself.

target-i386/helper2.c +12 -0
index 67658e2..98c03cc 100644
@@ -254,6 +254,18 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                     goto error;
                 }
                 x86_cpu_def->stepping = stepping;
+            }  else if (!strcmp(featurestr, "vendor")) {
+                if (strlen(val) != 12) {
+                    fprintf(stderr, "vendor string must be 12 chars long\n");
+                    x86_cpu_def = 0;
+                    goto error;
+                }
+                x86_cpu_def->vendor1 = val[0] ^ (val[1] << 8)
+                                     ^ (val[2] << 16) ^ (val[3] << 24);
+                x86_cpu_def->vendor2 = val[4] ^ (val[5] << 8)
+                                     ^ (val[6] << 16) ^ (val[7] << 24);
+                x86_cpu_def->vendor3 = val[8] ^ (val[9] << 8)
+                                     ^ (val[10] << 16) ^ (val[11] << 24);
             } else {
                 fprintf(stderr, "unrecognized feature %s\n", featurestr);
                 x86_cpu_def = 0;
author
#7 lore raw
Quoted 10 lines · Dan Kenigsberg
Quoted 9 lines · Andreas Schwab
Quoted 6 lines · Dan Kenigsberg
(quoted diff) +6 -0
+                x86_cpu_def->vendor1 = val[0] + (val[1] << 8)
+                                     + (val[2] << 16) + (val[3] << 24);
+                x86_cpu_def->vendor2 = val[4] + (val[5] << 8)
+                                     + (val[6] << 16) + (val[7] << 24);
+                x86_cpu_def->vendor3 = val[8] + (val[9] << 8)
+                                     + (val[10] << 16) + (val[11] << 24);
Quoted 5 lines
Quoted 2 lines

That will do the wrong thing with the sign bit.

Please tell me when I'm done making a fool of myself.

Apparently, I was not done. So I'll just shut up now. Sorry for the noise.

Dan.

author
#8 lore raw

PATCH 1/1 Allow setting the vendor and model_id strings with x86's -cpu option

In reply to Dan Kenigsberg

Having AuthenticAMD hard-coded is nice, but allowing the user to impersonate whatever CPU she wants is even nicer.

Hopefully it would now work on big endian host and with non-ASCII characters.

Dan.

target-i386/helper2.c +23 -7
index 6d40c64..b9b3093 100644
@@ -120,6 +120,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
 typedef struct x86_def_t {
     const char *name;
     uint32_t vendor1, vendor2, vendor3;
+    char model_id[48];
     int family;
     int model;
     int stepping;

@@ -255,7 +256,21 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
                     goto error;
                 }
                 x86_cpu_def->stepping = stepping;
-            } else {
+            } else if (!strcmp(featurestr, "vendor")) {
+                if (strlen(val) != 12) {
+                    fprintf(stderr, "vendor string must be 12 chars long\n");
+                    x86_cpu_def = 0;
+                    goto error;
+                }
+                x86_cpu_def->vendor1 = x86_cpu_def->vendor2 = x86_cpu_def->vendor3 = 0;
+                for(i = 0; i < 4; i++) {
+                    x86_cpu_def->vendor1 |= ((unsigned char)val[i    ]) << (8 * i);
+                    x86_cpu_def->vendor2 |= ((unsigned char)val[i + 4]) << (8 * i);
+                    x86_cpu_def->vendor3 |= ((unsigned char)val[i + 8]) << (8 * i);
+                }
+            } else if (!strcmp(featurestr, "model_id"))
+                strncpy(x86_cpu_def->model_id, val, 48);
+            else {
                 fprintf(stderr, "unrecognized feature %s\n", featurestr);
                 x86_cpu_def = 0;
                 goto error;

@@ -316,13 +331,14 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     env->cpuid_ext3_features = def->ext3_features;
     {
         const char *model_id = "QEMU Virtual CPU version " QEMU_VERSION;
-        int c, len, i;
-        len = strlen(model_id);
+        int c = -1, i;
+	
+        if (def->model_id[0] != '\0')
+            model_id = def->model_id;
+
         for(i = 0; i < 48; i++) {
-            if (i >= len)
-                c = '\0';
-            else
-                c = model_id[i];
+            if (c != '\0')
+                c = (unsigned char)model_id[i];
             env->cpuid_model[i >> 2] |= c << (8 * (i & 3));
         }
     }