qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3]: Introduce multi-core and multi-thread support for guests
@ 2009-08-19 13:42 Andre Przywara
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 1/3] extend -smp parsing to include cores= and threads= options Andre Przywara
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Andre Przywara @ 2009-08-19 13:42 UTC (permalink / raw)
  To: anthony; +Cc: Andre Przywara, qemu-devel

Hi,

currently QEMU's -smp <n> option only injects multiple CPU sockets into
the guest. Some operating systems(TM) restrict the number of "real" CPU
sockets for licensing reasons, but allow multi-core CPUs.
This patch extends the -smp option to let the user specify a topology:
-smp <smp_value>[,cores=<nrcores>][,threads=<nrthreads>]
	[,sockets=<nrsockets>][,maxcpus=<max_hotplug_cpus>]
The <smp_value> is the legacy number of virtual CPUs QEMU emulates. If any
of the cores, threads or sockets option is given, this value can be omitted.
Missing values are calculated to fulfill:
smp_value = nrsockets * nrcores * nrthreads
Where in doubt it will favor multiple sockets over multiple cores over
multiple threads (to mimic the current behavior).
The entered values will be propagated to the CPUID emulation, where the
corresponding bits will be set (both for Intel and AMD processors).
These patches are againt QEMU HEAD, but should apply against qemu-kvm, too.

Please review, comment and apply!

Thanks,
Andre.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/3] extend -smp parsing to include cores= and threads= options
  2009-08-19 13:42 [Qemu-devel] [PATCH 0/3]: Introduce multi-core and multi-thread support for guests Andre Przywara
@ 2009-08-19 13:42 ` Andre Przywara
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding Andre Przywara
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 3/3] set CPUID bits to present cores and threads topology Andre Przywara
  2 siblings, 0 replies; 11+ messages in thread
From: Andre Przywara @ 2009-08-19 13:42 UTC (permalink / raw)
  To: anthony; +Cc: Andre Przywara, qemu-devel

For injecting multi-core and multi-threading CPU topology into guests
extend the -smp syntax to accommodate cores and threads specification.
Syntax: -smp smp_value[,cores=nr_cores][,threads=nr_threads]\
[,socket=nr_sockets][,maxcpus=max_cpus]
smp_value is the legacy value specifying the total number of vCPUs for
the guest. If you specify one of cores, threads or sockets this value
can be omitted. Missing values will be computed to fulfill:
smp_value = nr_cores * nr_threads * nr_sockets
where it will favour sockets over cores over threads (to mimic the
current behavior, which will only inject multiple sockets.)
So -smp 4,threads=2 will inject two sockets with 2 threads each,
-smp cores=4 is an abbreviation for -smp 4,cores=4,threads=1,sockets=1.
If max_cpus (the number of hotpluggable CPUs) is omitted, it will
be set to smp_value.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
 cpu-defs.h |    2 +
 vl.c       |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index 5b80b1b..b6c8b95 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -185,6 +185,8 @@ typedef struct CPUWatchpoint {
     int cpu_index; /* CPU index (informative) */                        \
     uint32_t host_tid; /* host thread ID */                             \
     int numa_node; /* NUMA node this cpu is belonging to  */            \
+    int nr_cores;  /* number of cores within this CPU package */        \
+    int nr_threads;/* number of threads within this CPU */              \
     int running; /* Nonzero if cpu is currently running(usermode).  */  \
     /* user data */                                                     \
     void *opaque;                                                       \
diff --git a/vl.c b/vl.c
index 8b2b289..0cce808 100644
--- a/vl.c
+++ b/vl.c
@@ -220,6 +220,8 @@ int usb_enabled = 0;
 int singlestep = 0;
 int smp_cpus = 1;
 int max_cpus = 0;
+int smp_cores = 1;
+int smp_threads = 1;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
@@ -2363,6 +2365,56 @@ static void numa_add(const char *optarg)
     return;
 }
 
+static void smp_parse(const char *optarg)
+{
+    int smp, sockets = 0, threads = 0, cores = 0;
+    char *endptr;
+    char option[128];
+
+    smp = strtoul(optarg, &endptr, 10);
+    if (endptr != optarg) {
+        if (*endptr == ',') {
+            endptr++;
+        }
+    }
+    if (get_param_value(option, 128, "sockets", endptr) != 0)
+        sockets = strtoull(option, NULL, 10);
+    if (get_param_value(option, 128, "cores", endptr) != 0)
+        cores = strtoull(option, NULL, 10);
+    if (get_param_value(option, 128, "threads", endptr) != 0)
+        threads = strtoull(option, NULL, 10);
+    if (get_param_value(option, 128, "maxcpus", endptr) != 0)
+        max_cpus = strtoull(option, NULL, 10);
+
+    /* compute missing values, prefer sockets over cores over threads */
+    if (smp == 0 || sockets == 0) {
+        sockets = sockets > 0 ? sockets : 1;
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        if (smp == 0) {
+            smp = cores * threads * sockets;
+        } else {
+            sockets = smp / (cores * threads);
+        }
+    } else {
+        if (cores == 0) {
+            threads = threads > 0 ? threads : 1;
+            cores = smp / (sockets * threads);
+        } else {
+            if (sockets == 0) {
+                sockets = smp / (cores * threads);
+            } else {
+                threads = smp / (cores * sockets);
+            }
+        }
+    }
+    smp_cpus = smp;
+    smp_cores = cores > 0 ? cores : 1;
+    smp_threads = threads > 0 ? threads : 1;
+    if (max_cpus == 0)
+        max_cpus = smp_cpus;
+}
+
 /***********************************************************/
 /* USB devices */
 
@@ -3572,6 +3624,8 @@ void qemu_init_vcpu(void *_env)
 
     if (kvm_enabled())
         kvm_init_vcpu(env);
+    env->nr_cores = smp_cores;
+    env->nr_threads = smp_threads;
     return;
 }
 
@@ -3899,6 +3953,8 @@ void qemu_init_vcpu(void *_env)
         kvm_start_vcpu(env);
     else
         tcg_init_vcpu(env);
+    env->nr_cores = smp_cores;
+    env->nr_threads = smp_threads;
 }
 
 void qemu_notify_event(void)
@@ -5403,18 +5459,11 @@ int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_smp:
-            {
-                char *p;
-                char option[128];
-                smp_cpus = strtol(optarg, &p, 10);
+                smp_parse(optarg);
                 if (smp_cpus < 1) {
                     fprintf(stderr, "Invalid number of CPUs\n");
                     exit(1);
                 }
-                if (*p++ != ',')
-                    break;
-                if (get_param_value(option, 128, "maxcpus", p))
-                    max_cpus = strtol(option, NULL, 0);
                 if (max_cpus < smp_cpus) {
                     fprintf(stderr, "maxcpus must be equal to or greater than "
                             "smp\n");
@@ -5425,7 +5474,6 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
-            }
 	    case QEMU_OPTION_vnc:
                 display_type = DT_VNC;
 		vnc_display = optarg;
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding
  2009-08-19 13:42 [Qemu-devel] [PATCH 0/3]: Introduce multi-core and multi-thread support for guests Andre Przywara
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 1/3] extend -smp parsing to include cores= and threads= options Andre Przywara
@ 2009-08-19 13:42 ` Andre Przywara
  2009-08-20 10:06   ` Avi Kivity
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 3/3] set CPUID bits to present cores and threads topology Andre Przywara
  2 siblings, 1 reply; 11+ messages in thread
From: Andre Przywara @ 2009-08-19 13:42 UTC (permalink / raw)
  To: anthony; +Cc: Andre Przywara, qemu-devel

Intel CPUs store the number of cores in CPUID leaf 4. So push
the maxleaf value to 4 to allow the guests access to this leaf.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
 target-i386/helper.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index dd89885..7de4c07 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -119,7 +119,7 @@ static x86_def_t x86_defs[] = {
 #ifdef TARGET_X86_64
     {
         .name = "qemu64",
-        .level = 2,
+        .level = 4,
         .vendor1 = CPUID_VENDOR_AMD_1,
         .vendor2 = CPUID_VENDOR_AMD_2,
         .vendor3 = CPUID_VENDOR_AMD_3,
@@ -190,7 +190,7 @@ static x86_def_t x86_defs[] = {
 #endif
     {
         .name = "qemu32",
-        .level = 2,
+        .level = 4,
         .family = 6,
         .model = 3,
         .stepping = 3,
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/3] set CPUID bits to present cores and threads topology
  2009-08-19 13:42 [Qemu-devel] [PATCH 0/3]: Introduce multi-core and multi-thread support for guests Andre Przywara
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 1/3] extend -smp parsing to include cores= and threads= options Andre Przywara
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding Andre Przywara
@ 2009-08-19 13:42 ` Andre Przywara
  2 siblings, 0 replies; 11+ messages in thread
From: Andre Przywara @ 2009-08-19 13:42 UTC (permalink / raw)
  To: anthony; +Cc: Andre Przywara, qemu-devel

Controlled by the enhanced -smp option set the CPUID bits to present the
guest the desired topology. This is vendor specific, but (with the exception
of the CMP_LEGACY bit) not conflicting, so we set all bits everytime.
There is no real multithreading support for AMD CPUs, so report cores
instead.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
 target-i386/helper.c |   28 +++++++++++++++++++++++++---
 1 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 7de4c07..d4dfcbd 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1631,6 +1631,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */
         *ecx = env->cpuid_ext_features;
         *edx = env->cpuid_features;
+        if (env->nr_cores * env->nr_threads > 1) {
+            *ebx |= (env->nr_cores * env->nr_threads) << 16;
+            *edx |= 1 << 28;    /* HTT bit */
+        }
         break;
     case 2:
         /* cache info: needed for Pentium Pro compatibility */
@@ -1641,21 +1645,29 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 4:
         /* cache info: needed for Core compatibility */
+        if (env->nr_cores > 1) {
+        	*eax = (env->nr_cores - 1) << 26;
+        } else {
+        	*eax = 0;
+        }
         switch (count) {
             case 0: /* L1 dcache info */
-                *eax = 0x0000121;
+                *eax |= 0x0000121;
                 *ebx = 0x1c0003f;
                 *ecx = 0x000003f;
                 *edx = 0x0000001;
                 break;
             case 1: /* L1 icache info */
-                *eax = 0x0000122;
+                *eax |= 0x0000122;
                 *ebx = 0x1c0003f;
                 *ecx = 0x000003f;
                 *edx = 0x0000001;
                 break;
             case 2: /* L2 cache info */
-                *eax = 0x0000143;
+                *eax |= 0x0000143;
+                if (env->nr_threads > 1) {
+                    *eax |= (env->nr_threads - 1) << 14;
+                }
                 *ebx = 0x3c0003f;
                 *ecx = 0x0000fff;
                 *edx = 0x0000001;
@@ -1708,6 +1720,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ecx = env->cpuid_ext3_features;
         *edx = env->cpuid_ext2_features;
 
+        if (env->nr_cores * env->nr_threads > 1 &&
+            env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
+            env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
+            env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
+            *ecx |= 1 << 1;    /* CmpLegacy bit */
+        }
+
         if (kvm_enabled()) {
             /* Nested SVM not yet supported in KVM */
             *ecx &= ~CPUID_EXT3_SVM;
@@ -1762,6 +1781,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx = 0;
         *ecx = 0;
         *edx = 0;
+        if (env->nr_cores * env->nr_threads > 1) {
+            *ecx |= (env->nr_cores * env->nr_threads) - 1;
+        }
         break;
     case 0x8000000A:
         *eax = 0x00000001; /* SVM Revision */
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding
  2009-08-19 13:42 ` [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding Andre Przywara
@ 2009-08-20 10:06   ` Avi Kivity
  2009-08-20 10:36     ` Andre Przywara
  0 siblings, 1 reply; 11+ messages in thread
From: Avi Kivity @ 2009-08-20 10:06 UTC (permalink / raw)
  To: Andre Przywara; +Cc: qemu-devel

On 08/19/2009 04:42 PM, Andre Przywara wrote:
> Intel CPUs store the number of cores in CPUID leaf 4. So push
> the maxleaf value to 4 to allow the guests access to this leaf.
>    

There's a slight compatibility risk here.  If a guest has broken 
handling for cpuid level 4, then upgrading qemu would cause it to behave 
differently.

I don't think that's an issue for this patch, just highlighting the need 
for a systematic treatment of backward compatibility.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding
  2009-08-20 10:06   ` Avi Kivity
@ 2009-08-20 10:36     ` Andre Przywara
  2009-08-20 11:06       ` Avi Kivity
  2009-08-20 19:30       ` Jamie Lokier
  0 siblings, 2 replies; 11+ messages in thread
From: Andre Przywara @ 2009-08-20 10:36 UTC (permalink / raw)
  To: Avi Kivity; +Cc: qemu-devel

Avi Kivity wrote:
> On 08/19/2009 04:42 PM, Andre Przywara wrote:
>> Intel CPUs store the number of cores in CPUID leaf 4. So push
>> the maxleaf value to 4 to allow the guests access to this leaf.
>>    
> 
> There's a slight compatibility risk here.  If a guest has broken 
> handling for cpuid level 4, then upgrading qemu would cause it to behave 
> differently.
> 
> I don't think that's an issue for this patch, just highlighting the need 
> for a systematic treatment of backward compatibility.

If you have real headaches about it, I have two options:

What about allowing level to be specified at -cpu command line? This 
would allow users to say -cpu qemu64,level=2 if they experience 
problems. The default would stay at level 4.

The other option would be to push the level only to four if we use more 
than one thread or core.

In my research it turned out that Intel pushed the level beyond 4 with 
Pentium4 Prescott (probably with the introduction of real dual core 
chips to differentiate threads and cores), so this is quite some time 
ago. So I doubt that there are serious issues out there. The only 
problem I can think of is that the advertised cache topology is somehow 
bogus and could confuse OSes.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding
  2009-08-20 10:36     ` Andre Przywara
@ 2009-08-20 11:06       ` Avi Kivity
  2009-08-20 19:03         ` [Qemu-devel] [PATCH] allow overriding of CPUID level on command line Andre Przywara
  2009-08-25 12:21         ` [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding Andre Przywara
  2009-08-20 19:30       ` Jamie Lokier
  1 sibling, 2 replies; 11+ messages in thread
From: Avi Kivity @ 2009-08-20 11:06 UTC (permalink / raw)
  To: Andre Przywara; +Cc: qemu-devel

On 08/20/2009 01:36 PM, Andre Przywara wrote:
> Avi Kivity wrote:
>> On 08/19/2009 04:42 PM, Andre Przywara wrote:
>>> Intel CPUs store the number of cores in CPUID leaf 4. So push
>>> the maxleaf value to 4 to allow the guests access to this leaf.
>>
>> There's a slight compatibility risk here.  If a guest has broken 
>> handling for cpuid level 4, then upgrading qemu would cause it to 
>> behave differently.
>>
>> I don't think that's an issue for this patch, just highlighting the 
>> need for a systematic treatment of backward compatibility.
>
> If you have real headaches about it, I have two options:

It's really an imaginary headache.

>
> What about allowing level to be specified at -cpu command line? This 
> would allow users to say -cpu qemu64,level=2 if they experience 
> problems. The default would stay at level 4.

I think this is the best option.

> The other option would be to push the level only to four if we use 
> more than one thread or core.
>
> In my research it turned out that Intel pushed the level beyond 4 with 
> Pentium4 Prescott (probably with the introduction of real dual core 
> chips to differentiate threads and cores), so this is quite some time 
> ago. So I doubt that there are serious issues out there. 

I only pointed this out as an example of a new feature that has an 
effect even if it is not used, something we should beware of.

> The only problem I can think of is that the advertised cache topology 
> is somehow bogus and could confuse OSes.

So long as it's smaller than contemporary caches we should be fine.

btw, does -cpu host use the host cpu cache information?

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH] allow overriding of CPUID level on command line
  2009-08-20 11:06       ` Avi Kivity
@ 2009-08-20 19:03         ` Andre Przywara
  2009-08-25 12:21         ` [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding Andre Przywara
  1 sibling, 0 replies; 11+ messages in thread
From: Andre Przywara @ 2009-08-20 19:03 UTC (permalink / raw)
  To: avi, anthony; +Cc: qemu-devel

The CPUID level determines how many CPUID leafs are exposed to the guest.
Some features (like multi-core) cannot be propagated without the proper
level, but guests maybe confused by bogus entries in some leafs.
So add level= and xlevel= to the list of -cpu options to allow the user to
override the default settings. While at it, merge unnecessary local
variables into one and allow hexadecimal arguments.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
 target-i386/helper.c |   39 +++++++++++++++++++++++++++++----------
 1 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index d4dfcbd..21b268a 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -349,7 +349,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     char *featurestr, *name = strtok(s, ",");
     uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0;
     uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0;
-    int family = -1, model = -1, stepping = -1;
+    uint32_t numvalue;
 
     def = NULL;
     for (i = 0; i < ARRAY_SIZE(x86_defs); i++) {
@@ -381,28 +381,47 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
             *val = 0; val++;
             if (!strcmp(featurestr, "family")) {
                 char *err;
-                family = strtol(val, &err, 10);
-                if (!*val || *err || family < 0) {
+                numvalue = strtoul(val, &err, 0);
+                if (!*val || *err) {
                     fprintf(stderr, "bad numerical value %s\n", val);
                     goto error;
                 }
-                x86_cpu_def->family = family;
+                x86_cpu_def->family = numvalue;
             } else if (!strcmp(featurestr, "model")) {
                 char *err;
-                model = strtol(val, &err, 10);
-                if (!*val || *err || model < 0 || model > 0xff) {
+                numvalue = strtoul(val, &err, 0);
+                if (!*val || *err || numvalue > 0xff) {
                     fprintf(stderr, "bad numerical value %s\n", val);
                     goto error;
                 }
-                x86_cpu_def->model = model;
+                x86_cpu_def->model = numvalue;
             } else if (!strcmp(featurestr, "stepping")) {
                 char *err;
-                stepping = strtol(val, &err, 10);
-                if (!*val || *err || stepping < 0 || stepping > 0xf) {
+                numvalue = strtoul(val, &err, 0);
+                if (!*val || *err || numvalue > 0xf) {
                     fprintf(stderr, "bad numerical value %s\n", val);
                     goto error;
                 }
-                x86_cpu_def->stepping = stepping;
+                x86_cpu_def->stepping = numvalue ;
+            } else if (!strcmp(featurestr, "level")) {
+                char *err;
+                numvalue = strtoul(val, &err, 0);
+                if (!*val || *err) {
+                    fprintf(stderr, "bad numerical value %s\n", val);
+                    goto error;
+                }
+                x86_cpu_def->level = numvalue;
+            } else if (!strcmp(featurestr, "xlevel")) {
+                char *err;
+                numvalue = strtoul(val, &err, 0);
+                if (!*val || *err) {
+                    fprintf(stderr, "bad numerical value %s\n", val);
+                    goto error;
+                }
+                if (numvalue < 0x80000000) {
+                	numvalue += 0x80000000;
+                }
+                x86_cpu_def->xlevel = numvalue;
             } else if (!strcmp(featurestr, "vendor")) {
                 if (strlen(val) != 12) {
                     fprintf(stderr, "vendor string must be 12 chars long\n");
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding
  2009-08-20 10:36     ` Andre Przywara
  2009-08-20 11:06       ` Avi Kivity
@ 2009-08-20 19:30       ` Jamie Lokier
  2009-08-20 21:35         ` Andre Przywara
  1 sibling, 1 reply; 11+ messages in thread
From: Jamie Lokier @ 2009-08-20 19:30 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Avi Kivity, qemu-devel

Andre Przywara wrote:
> The other option would be to push the level only to four if we use more 
> than one thread or core.
> 
> In my research it turned out that Intel pushed the level beyond 4
> with Pentium4 Prescott (probably with the introduction of real dual
> core chips to differentiate threads and cores), so this is quite
> some time ago.

Pentium 4 wasn't that long ago, from a point of view of running legacy
guests.

I see quite a lot of code in Linux (to pick an example - presumably
also other OSes) which checks for things by looking at model and
family number, so I wonder if it's wise to depart a long way from
CPUIDs which resemble real hardware which has existed.

We've already seen a problem along these lines, which was the x86
CPUID_SEP feature not getting used properly, but that was quite well
hidden, only affecting one application (Skype) on Windows.

Could it make more sense to, in effect, upgrade the default CPUID
to Pentium 4 (instead of Pentium Pro) when cores > 1?

-- Jamie

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding
  2009-08-20 19:30       ` Jamie Lokier
@ 2009-08-20 21:35         ` Andre Przywara
  0 siblings, 0 replies; 11+ messages in thread
From: Andre Przywara @ 2009-08-20 21:35 UTC (permalink / raw)
  To: Jamie Lokier; +Cc: Avi Kivity, qemu-devel

Jamie Lokier wrote:
> Andre Przywara wrote:
>> The other option would be to push the level only to four if we use more 
>> than one thread or core.
>>
>> In my research it turned out that Intel pushed the level beyond 4
>> with Pentium4 Prescott (probably with the introduction of real dual
>> core chips to differentiate threads and cores), so this is quite
>> some time ago.
> 
> Pentium 4 wasn't that long ago, from a point of view of running legacy
> guests.
Well, but long enough to get those software issues ruled out. If any 
serious OS would have had problems, somebody would have noticed and 
fixed it.

> I see quite a lot of code in Linux (to pick an example - presumably
> also other OSes) which checks for things by looking at model and
> family number, so I wonder if it's wise to depart a long way from
> CPUIDs which resemble real hardware which has existed.
Yeah, but most of the stuff Linux checks are bugs and workarounds, so I 
am not 100% sure whether it's an advantage. We should be careful with 
adjusting the working default QEMU/KVM has right now.

> We've already seen a problem along these lines, which was the x86
> CPUID_SEP feature not getting used properly, but that was quite well
> hidden, only affecting one application (Skype) on Windows.
> 
> Could it make more sense to, in effect, upgrade the default CPUID
> to Pentium 4 (instead of Pentium Pro) when cores > 1?
OK, OK, I will finally send this patch out (following in another mail).
This is a kvm64 CPU (which should become the new KVM's default, if it 
proves to work well). It looks like a P4 Prescott, that seems to be a 
good base, since it has a decent base line of features and is among the 
first CPUs which are supported by KVM.
If someone has such a real beast (best would be a Pentium 4 662 or other 
Prescotts with x86_64, HT and VT-x), I would love to see a complete 
CPUID dump (including the 0000_0004 subleafs, which x86info omits).

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Jochen Polster; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding
  2009-08-20 11:06       ` Avi Kivity
  2009-08-20 19:03         ` [Qemu-devel] [PATCH] allow overriding of CPUID level on command line Andre Przywara
@ 2009-08-25 12:21         ` Andre Przywara
  1 sibling, 0 replies; 11+ messages in thread
From: Andre Przywara @ 2009-08-25 12:21 UTC (permalink / raw)
  To: Avi Kivity; +Cc: qemu-devel


  Avi Kivity wrote:
 > Andre Przywara wrote:
>> The only problem I can think of is that the advertised cache topology 
>> is somehow bogus and could confuse OSes.
> 
> So long as it's smaller than contemporary caches we should be fine.
Oh, I see now that Alex' Core2Duo (on which QEMU's leaf 4 values are 
based on) has 4 MB of L2-Cache. Although that is not gigantic for Intel 
CPUs, it's certainly too high for many host CPUs. I will lower this 
value to say 1MB or 512K to better match the majority of host. I will 
send a patch together with some more CPUID reworks I am working on.
> 
> btw, does -cpu host use the host cpu cache information?
Good point. It does not, it only deals with leaves 0, 1, 8000_0000 and 
8000_0001. I have created a white list of further CPUID leaves which are 
safe to propagate to the guest and will also send this in a while.

Thanks for these hints,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2009-08-25 12:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-19 13:42 [Qemu-devel] [PATCH 0/3]: Introduce multi-core and multi-thread support for guests Andre Przywara
2009-08-19 13:42 ` [Qemu-devel] [PATCH 1/3] extend -smp parsing to include cores= and threads= options Andre Przywara
2009-08-19 13:42 ` [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding Andre Przywara
2009-08-20 10:06   ` Avi Kivity
2009-08-20 10:36     ` Andre Przywara
2009-08-20 11:06       ` Avi Kivity
2009-08-20 19:03         ` [Qemu-devel] [PATCH] allow overriding of CPUID level on command line Andre Przywara
2009-08-25 12:21         ` [Qemu-devel] [PATCH 2/3] push CPUID level to 4 to allow Intel multicore decoding Andre Przywara
2009-08-20 19:30       ` Jamie Lokier
2009-08-20 21:35         ` Andre Przywara
2009-08-19 13:42 ` [Qemu-devel] [PATCH 3/3] set CPUID bits to present cores and threads topology Andre Przywara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).