[Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[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.

--- a/target-i386/helper2.c
+++ b/target-i386/helper2.c
@@ -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;
             }

Re: [Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Thiemo Seufer]

Dan Kenigsberg wrote:
> 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.
> 
> --- a/target-i386/helper2.c
> +++ b/target-i386/helper2.c
> @@ -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);

Endianness bug.


Thiemo

Re: [Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Dan Kenigsberg]

On Sun, Dec 09, 2007 at 03:02:44AM +0000, Thiemo Seufer wrote:
> Dan Kenigsberg wrote:
> > 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.
> > 
> > --- a/target-i386/helper2.c
> > +++ b/target-i386/helper2.c
> > @@ -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);
> 
> Endianness bug.
> 

Indeed. Please consider the following:

diff --git a/target-i386/helper2.c b/target-i386/helper2.c
index 67658e2..6109283 100644
--- a/target-i386/helper2.c
+++ b/target-i386/helper2.c
@@ -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;

Re: [Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Paul Brook]

> +                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

Re: [Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Dan Kenigsberg]

On Sun, Dec 09, 2007 at 11:36:34AM +0000, Paul Brook wrote:
> > +                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?


diff --git a/target-i386/helper2.c b/target-i386/helper2.c
index 67658e2..2ef4be3 100644
--- a/target-i386/helper2.c
+++ b/target-i386/helper2.c
@@ -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;

Re: [Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Andreas Schwab]

Dan Kenigsberg <danken@qumranet.com> writes:

> +                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.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
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."

Re: [Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Dan Kenigsberg]

On Sun, Dec 09, 2007 at 07:29:49PM +0100, Andreas Schwab wrote:
> Dan Kenigsberg <danken@qumranet.com> writes:
> 
> > +                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.


diff --git a/target-i386/helper2.c b/target-i386/helper2.c
index 67658e2..98c03cc 100644
--- a/target-i386/helper2.c
+++ b/target-i386/helper2.c
@@ -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;

Re: [Qemu-devel] [PATCH] Allow setting the vendor_id string with x86's -cpu option

[Dan Kenigsberg]

On Sun, Dec 09, 2007 at 08:58:34PM +0200, Dan Kenigsberg wrote:
> On Sun, Dec 09, 2007 at 07:29:49PM +0100, Andreas Schwab wrote:
> > Dan Kenigsberg <danken@qumranet.com> writes:
> > 
> > > +                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.
> 

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

Dan.

[Qemu-devel] [PATCH] Allow setting the vendor and model_id strings with x86's -cpu option

[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.


diff --git a/target-i386/helper2.c b/target-i386/helper2.c
index 6d40c64..b9b3093 100644
--- a/target-i386/helper2.c
+++ b/target-i386/helper2.c
@@ -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));
         }
     }