[RFC] allow multi-core guests: introduce cores= option to -cpu

[RFC] allow multi-core guests: introduce cores= option to -cpu

[Andre Przywara]

Hi,

currently SMP guests happen to see <n> vCPUs as <n> different sockets.
Some guests (Windows comes to mind) have license restrictions and refuse
to run on multi-socket machines.
So lets introduce a "cores=" parameter to the -cpu option to let the user
specify the number of _cores_ the guest should see.

This patch has not been tested with all corner cases, so I just want to
hear your comments whether
a) we need such an option  and
b) you like this particular approach.

Applying this qemu.git patch to qemu-kvm.git fixes Windows SMP boot on
some versions, I successfully tried up to -smp 16 -cpu host,cores=8 with
WindowsXP Pro.                                                                  

Regards,
Andre.

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

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 4a8608e..96fa471 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -657,6 +657,7 @@ typedef struct CPUX86State {
     uint32_t cpuid_ext3_features;
     uint32_t cpuid_apic_id;
     int cpuid_vendor_override;
+    int cpuid_cores;
 
     /* MTRRs */
     uint64_t mtrr_fixed[11];
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 82e1ff1..9c54fb9 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -103,6 +103,7 @@ typedef struct x86_def_t {
     uint32_t xlevel;
     char model_id[48];
     int vendor_override;
+    int cores;
 } x86_def_t;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -351,7 +352,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;
+    int family = -1, model = -1, stepping = -1, cores = 1;
 
     def = NULL;
     for (i = 0; i < ARRAY_SIZE(x86_defs); i++) {
@@ -406,6 +407,14 @@ 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, "cores")) {
+                char *err;
+                cores = strtol(val, &err, 10);
+                if (!*val || *err || cores < 1 || cores > 0xff) {
+                    fprintf(stderr, "bad numerical value %s\n", val);
+                    goto error;
+                }
+                x86_cpu_def->cores = cores;
             } else if (!strcmp(featurestr, "vendor")) {
                 if (strlen(val) != 12) {
                     fprintf(stderr, "vendor string must be 12 chars long\n");
@@ -473,6 +482,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
         env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
     }
     env->cpuid_vendor_override = def->vendor_override;
+    env->cpuid_cores = def->cores;
     env->cpuid_level = def->level;
     if (def->family > 0x0f)
         env->cpuid_version = 0xf00 | ((def->family - 0x0f) << 20);
@@ -1562,9 +1572,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 1:
         *eax = env->cpuid_version;
-        *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */
+        /* CLFLUSH size in quad words, Linux wants it. */
+        *ebx = (env->cpuid_apic_id << 24) | 8 << 8;
         *ecx = env->cpuid_ext_features;
         *edx = env->cpuid_features;
+        if (env->cpuid_cores > 1) {
+            *ebx |= env->cpuid_cores << 16;   /* LogicalProcessorCount */
+            *edx |= 1 << 28;    /* HTT bit */
+        }
         break;
     case 2:
         /* cache info: needed for Pentium Pro compatibility */
@@ -1642,6 +1657,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ecx = env->cpuid_ext3_features;
         *edx = env->cpuid_ext2_features;
 
+        if (env->cpuid_cores > 1) {
+            *ecx |= 1 << 1;    /* CmpLegacy bit */
+        }
+
         if (kvm_enabled()) {
             /* Nested SVM not yet supported in KVM */
             *ecx &= ~CPUID_EXT3_SVM;
@@ -1696,6 +1715,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx = 0;
         *ecx = 0;
         *edx = 0;
+        if (env->cpuid_cores > 1) {
+            *ecx |= env->cpuid_cores - 1;    /* NC: Number of CPU cores */
+        }
         break;
     case 0x8000000A:
         *eax = 0x00000001; /* SVM Revision */
-- 
1.6.1.3

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Samuel Thibault]

Andre Przywara, le Fri 03 Jul 2009 16:41:56 +0200, a écrit :
> -smp 16 -cpu host,cores=8

That means 8 cores with 2 threads each, thus 16 threads? Ok, that can be
later generalized into for instance

-smp 16 -cpu host,nodes=2,sockets=2,cores=2

to define 2 NUMA nodes of 2 sockets of 2 cores, each core thus having 16
threads.

Samuel

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Brian Jackson]

Andre Przywara wrote:
> Hi,
> 
> currently SMP guests happen to see <n> vCPUs as <n> different sockets.
> Some guests (Windows comes to mind) have license restrictions and refuse
> to run on multi-socket machines.
> So lets introduce a "cores=" parameter to the -cpu option to let the user
> specify the number of _cores_ the guest should see.
> 
> This patch has not been tested with all corner cases, so I just want to
> hear your comments whether
> a) we need such an option  and
> b) you like this particular approach.
> 
> Applying this qemu.git patch to qemu-kvm.git fixes Windows SMP boot on
> some versions, I successfully tried up to -smp 16 -cpu host,cores=8 with
> WindowsXP Pro.                  


Personally, I'd like to see it as an extra arg to the -smp option. We've 
seen too many people use -cpu incorrectly in #kvm, so we've gotten into 
the habit of telling people not to touch that option unless they know 
exactly what they are doing. Plus it seems odd to have to use -cpu foo 
when you just want more cpus, not a specific cpu.

--Iggy


> 
> Regards,
> Andre.
> 
> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
> ---
>  target-i386/cpu.h    |    1 +
>  target-i386/helper.c |   26 ++++++++++++++++++++++++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 4a8608e..96fa471 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -657,6 +657,7 @@ typedef struct CPUX86State {
>      uint32_t cpuid_ext3_features;
>      uint32_t cpuid_apic_id;
>      int cpuid_vendor_override;
> +    int cpuid_cores;
>  
>      /* MTRRs */
>      uint64_t mtrr_fixed[11];
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 82e1ff1..9c54fb9 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -103,6 +103,7 @@ typedef struct x86_def_t {
>      uint32_t xlevel;
>      char model_id[48];
>      int vendor_override;
> +    int cores;
>  } x86_def_t;
>  
>  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> @@ -351,7 +352,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;
> +    int family = -1, model = -1, stepping = -1, cores = 1;
>  
>      def = NULL;
>      for (i = 0; i < ARRAY_SIZE(x86_defs); i++) {
> @@ -406,6 +407,14 @@ 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, "cores")) {
> +                char *err;
> +                cores = strtol(val, &err, 10);
> +                if (!*val || *err || cores < 1 || cores > 0xff) {
> +                    fprintf(stderr, "bad numerical value %s\n", val);
> +                    goto error;
> +                }
> +                x86_cpu_def->cores = cores;
>              } else if (!strcmp(featurestr, "vendor")) {
>                  if (strlen(val) != 12) {
>                      fprintf(stderr, "vendor string must be 12 chars long\n");
> @@ -473,6 +482,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
>          env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
>      }
>      env->cpuid_vendor_override = def->vendor_override;
> +    env->cpuid_cores = def->cores;
>      env->cpuid_level = def->level;
>      if (def->family > 0x0f)
>          env->cpuid_version = 0xf00 | ((def->family - 0x0f) << 20);
> @@ -1562,9 +1572,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          break;
>      case 1:
>          *eax = env->cpuid_version;
> -        *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */
> +        /* CLFLUSH size in quad words, Linux wants it. */
> +        *ebx = (env->cpuid_apic_id << 24) | 8 << 8;
>          *ecx = env->cpuid_ext_features;
>          *edx = env->cpuid_features;
> +        if (env->cpuid_cores > 1) {
> +            *ebx |= env->cpuid_cores << 16;   /* LogicalProcessorCount */
> +            *edx |= 1 << 28;    /* HTT bit */
> +        }
>          break;
>      case 2:
>          /* cache info: needed for Pentium Pro compatibility */
> @@ -1642,6 +1657,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *ecx = env->cpuid_ext3_features;
>          *edx = env->cpuid_ext2_features;
>  
> +        if (env->cpuid_cores > 1) {
> +            *ecx |= 1 << 1;    /* CmpLegacy bit */
> +        }
> +
>          if (kvm_enabled()) {
>              /* Nested SVM not yet supported in KVM */
>              *ecx &= ~CPUID_EXT3_SVM;
> @@ -1696,6 +1715,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *ebx = 0;
>          *ecx = 0;
>          *edx = 0;
> +        if (env->cpuid_cores > 1) {
> +            *ecx |= env->cpuid_cores - 1;    /* NC: Number of CPU cores */
> +        }
>          break;
>      case 0x8000000A:
>          *eax = 0x00000001; /* SVM Revision */

Re: [Qemu-devel] [RFC] allow multi-core guests: introduce cores= option to -cpu

[Paul Brook]

> currently SMP guests happen to see <n> vCPUs as <n> different sockets.
> Some guests (Windows comes to mind) have license restrictions and refuse
> to run on multi-socket machines.
> So lets introduce a "cores=" parameter to the -cpu option to let the user
> specify the number of _cores_ the guest should see.

Sounds like this should be part of the -numa option.

Paul

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Andre Przywara]

Brian Jackson wrote:
> Andre Przywara wrote:
>> currently SMP guests happen to see <n> vCPUs as <n> different sockets.
>> Some guests (Windows comes to mind) have license restrictions and refuse
>> to run on multi-socket machines.
>> So lets introduce a "cores=" parameter to the -cpu option to let the user
>> specify the number of _cores_ the guest should see.
 >> ...
>>
> Personally, I'd like to see it as an extra arg to the -smp option. We've 
> seen too many people use -cpu incorrectly in #kvm, so we've gotten into 
> the habit of telling people not to touch that option unless they know 
> exactly what they are doing. Plus it seems odd to have to use -cpu foo 
> when you just want more cpus, not a specific cpu.

Ok, I see your point. I simply used -cpu because of technical reasons 
(the core topology is reflected in CPUID, which -cpu cares about). But 
you are right, it does not belong here, -smp looks like a good candidate 
(IMO better than -numa). Or we use an abstract "-topology" for this, but 
this seems like overkill.
So what about: "-smp 4,cores=2,threads=2[,sockets=1]" to inject 4 vCPUs 
in one package (automatically determined if omitted) with two cores and 
two threads/core? All parameters except the number of vCPUs would be 
optional, which would make this new format backwards compatible.
Only we have to agree on the default topology: multi-socket like the 
current implementation or multi-core, which would mimic the most common 
SMP architecture today. It seems that the latter one causes less 
problems for guests.

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

Re: [Qemu-devel] [RFC] allow multi-core guests: introduce cores= option to -cpu

[Andre Przywara]

Samuel Thibault wrote:
> Andre Przywara, le Fri 03 Jul 2009 16:41:56 +0200, a écrit :
>> -smp 16 -cpu host,cores=8
> 
> That means 8 cores with 2 threads each, thus 16 threads?
No, that meant: 16 vCPUs total with 8 cores per physical packages. I 
don't have any notion for threads in the current code, although I agree 
that this should be introduced.
AFAIK Windows XP Home is limited to one socket, where XP Pro can use up 
to two sockets. Using -smp 4 on a XP Pro (64bit) caused an early BSOD 
stating "MULTIPROCESSOR_CONFIGURATION_NOT_SUPPORTED" (because it saw 
four sockets instead of four cores).
I think WindowsXP Home should even be worse (though I don't have an 
image here to test it), because it would not boot any SMP guest.

> Ok, that can be
> later generalized into for instance
> 
> -smp 16 -cpu host,nodes=2,sockets=2,cores=2
> 
> to define 2 NUMA nodes of 2 sockets of 2 cores, each core thus having 16
> threads.
Well, generalizing is a good idea, but as Brian wrote this shouldn't be 
done in the -cpu option. Putting the NUMA nodes into here seems quite 
logical, but would also requires to
a) describe the NUMA topology separated from this option    or
b) pull all the NUMA topology description into this one.
Maybe one could describe cores, threads, sockets and nodes in -smp and 
declare the memory topology only in -numa.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12

Re: [Qemu-devel] [RFC] allow multi-core guests: introduce cores= option to -cpu

[Andre Przywara]

Paul Brook wrote:
>> currently SMP guests happen to see <n> vCPUs as <n> different sockets.
>> Some guests (Windows comes to mind) have license restrictions and refuse
>> to run on multi-socket machines.
>> So lets introduce a "cores=" parameter to the -cpu option to let the user
>> specify the number of _cores_ the guest should see.
> 
> Sounds like this should be part of the -numa option.
Sound reasonable on the first glance, but would make it rather 
complicated in real life. I suppose multi-core is far more interesting 
to most of the people than multi-node, so I would opt for the easier: 
-smp 2,cores=2 to specify a dual core guest.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Samuel Thibault]

Andre Przywara, le Sat 04 Jul 2009 01:28:43 +0200, a écrit :
> Maybe one could describe cores, threads, sockets and nodes in -smp and 
> declare the memory topology only in -numa.

Mmm, I'd rather just describe both in a -topology option.

Samuel

Re: [Qemu-devel] Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Jamie Lokier]

Andre Przywara wrote:
> So what about: "-smp 4,cores=2,threads=2[,sockets=1]" to inject 4 vCPUs 
> in one package (automatically determined if omitted) with two cores and 
> two threads/core? All parameters except the number of vCPUs would be 
> optional,

Why is the number of vCPUs required at all?

   -smp cores=2,threads=2

The "4" is redundant.

-- Jamie

Re: [Qemu-devel] [RFC] allow multi-core guests: introduce cores= option to -cpu

[Paul Brook]

On Saturday 04 July 2009, Andre Przywara wrote:
> Paul Brook wrote:
> >> currently SMP guests happen to see <n> vCPUs as <n> different sockets.
> >> Some guests (Windows comes to mind) have license restrictions and refuse
> >> to run on multi-socket machines.
> >> So lets introduce a "cores=" parameter to the -cpu option to let the
> >> user specify the number of _cores_ the guest should see.
> >
> > Sounds like this should be part of the -numa option.
>
> Sound reasonable on the first glance, but would make it rather
> complicated in real life. I suppose multi-core is far more interesting
> to most of the people than multi-node, so I would opt for the easier:
> -smp 2,cores=2 to specify a dual core guest.

I disagree. I think it makes a sense of the topology of nodes, cores and 
threads to all be specified in the same place. All the options you don't 
specify should have sensible defaults.

Paul

Re: [Qemu-devel] Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Gleb Natapov]

On Sat, Jul 04, 2009 at 12:52:30AM +0200, Andre Przywara wrote:
> Brian Jackson wrote:
>> Andre Przywara wrote:
>>> currently SMP guests happen to see <n> vCPUs as <n> different sockets.
>>> Some guests (Windows comes to mind) have license restrictions and refuse
>>> to run on multi-socket machines.
>>> So lets introduce a "cores=" parameter to the -cpu option to let the user
>>> specify the number of _cores_ the guest should see.
> >> ...
>>>
>> Personally, I'd like to see it as an extra arg to the -smp option. 
>> We've seen too many people use -cpu incorrectly in #kvm, so we've 
>> gotten into the habit of telling people not to touch that option unless 
>> they know exactly what they are doing. Plus it seems odd to have to use 
>> -cpu foo when you just want more cpus, not a specific cpu.
>
> Ok, I see your point. I simply used -cpu because of technical reasons  
> (the core topology is reflected in CPUID, which -cpu cares about). But  
> you are right, it does not belong here, -smp looks like a good candidate  
> (IMO better than -numa). Or we use an abstract "-topology" for this, but  
> this seems like overkill.
> So what about: "-smp 4,cores=2,threads=2[,sockets=1]" to inject 4 vCPUs  
> in one package (automatically determined if omitted) with two cores and  
> two threads/core? All parameters except the number of vCPUs would be  
> optional, which would make this new format backwards compatible.
> Only we have to agree on the default topology: multi-socket like the  
> current implementation or multi-core, which would mimic the most common  
> SMP architecture today. It seems that the latter one causes less  
> problems for guests.
>

There is not point in [,sockets=1] option -smp socketnum[,cores=n][,threads=n]
is backwards compatible already if default for cores and threads is 1.
I have a similar patch locally that does that. What I think is missing
for KVM (and in the future for QEMU) is hotplug support. The current
hotplug code can't hotplug the socket, only one vcpu. What is missing is
the ability to specify number of available sockets vs number of
populated sockets and specify base apic id for each available socket.
Cpu hotplug will get base apic id as a parameter and will hotplug all
logical cpus simultaneously.

BTW I don't see that you change cpulevel to 4 in your patch. Guest
will not use cpu topology information without it.

diff --git a/sysemu.h b/sysemu.h
index 5582633..7187920 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -120,6 +120,8 @@ extern int usb_enabled;
 extern int virtio_balloon;
 extern const char *virtio_balloon_devaddr;
 extern int smp_cpus;
+extern int cpu_cores;
+extern int core_threads;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 87c04e5..585af4c 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -24,6 +24,8 @@
 #include <inttypes.h>
 #include <signal.h>
 
+#include "qemu-common.h"
+#include "sysemu.h"
 #include "cpu.h"
 #include "exec-all.h"
 #include "qemu-common.h"
@@ -122,7 +124,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,
@@ -194,7 +196,7 @@ static x86_def_t x86_defs[] = {
 #endif
     {
         .name = "qemu32",
-        .level = 2,
+        .level = 4,
         .family = 6,
         .model = 3,
         .stepping = 3,
@@ -260,7 +262,7 @@ static x86_def_t x86_defs[] = {
     },
     {
         .name = "athlon",
-        .level = 2,
+        .level = 4,
         .vendor1 = CPUID_VENDOR_AMD_1,
         .vendor2 = CPUID_VENDOR_AMD_2,
         .vendor3 = CPUID_VENDOR_AMD_3,
@@ -483,6 +485,8 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     env->cpuid_version |= ((def->model & 0xf) << 4) | ((def->model >> 4) << 16);
     env->cpuid_version |= def->stepping;
     env->cpuid_features = def->features;
+    if (cpu_cores * core_threads > 1)
+        env->cpuid_features |= CPUID_HT;
     env->pat = 0x0007040600070406ULL;
     env->cpuid_ext_features = def->ext_features;
     env->cpuid_ext2_features = def->ext2_features;
@@ -1564,7 +1568,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 1:
         *eax = env->cpuid_version;
-        *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */
+        *ebx = (env->cpuid_apic_id << 24) | (cpu_cores*core_threads) << 16
+            | 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */
         *ecx = env->cpuid_ext_features;
         *edx = env->cpuid_features;
         break;
@@ -1579,7 +1584,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         /* cache info: needed for Core compatibility */
         switch (count) {
             case 0: /* L1 dcache info */
-                *eax = 0x0000121;
+                *eax = 0x0000121 | ((cpu_cores - 1) << 26);
                 *ebx = 0x1c0003f;
                 *ecx = 0x000003f;
                 *edx = 0x0000001;
diff --git a/vl.c b/vl.c
index 5d86e69..9098603 100644
--- a/vl.c
+++ b/vl.c
@@ -244,6 +244,8 @@ int singlestep = 0;
 const char *assigned_devices[MAX_DEV_ASSIGN_CMDLINE];
 int assigned_devices_index;
 int smp_cpus = 1;
+int cpu_cores = 1;
+int core_threads = 1;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
@@ -5696,12 +5698,23 @@ int main(int argc, char **argv, char **envp)
                 usb_devices_index++;
                 break;
             case QEMU_OPTION_smp:
-                smp_cpus = atoi(optarg);
+            {
+                char *p;
+                char option[128];
+                smp_cpus = strtol(optarg, &p, 10);
                 if (smp_cpus < 1) {
-                    fprintf(stderr, "Invalid number of CPUs\n");
+                    fprintf(stderr, "Invalid number of slots\n");
                     exit(1);
                 }
+                if (*p++ != ',')
+                    break;
+                if (get_param_value(option, 128, "cores", p))
+                    cpu_cores = strtol(option, NULL, 0);
+                if (get_param_value(option, 128, "threads", p))
+                    core_threads = strtol(option, NULL, 0);
+                smp_cpus *= (cpu_cores * core_threads);
                 break;
+            }
 	    case QEMU_OPTION_vnc:
                 display_type = DT_VNC;
 		vnc_display = optarg;
--
			Gleb.

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Avi Kivity]

On 07/03/2009 05:41 PM, Andre Przywara wrote:
> Hi,
>
> currently SMP guests happen to see<n>  vCPUs as<n>  different sockets.
> Some guests (Windows comes to mind) have license restrictions and refuse
> to run on multi-socket machines.
> So lets introduce a "cores=" parameter to the -cpu option to let the user
> specify the number of _cores_ the guest should see.
>
> This patch has not been tested with all corner cases, so I just want to
> hear your comments whether
> a) we need such an option  and
> b) you like this particular approach.
>
> Applying this qemu.git patch to qemu-kvm.git fixes Windows SMP boot on
> some versions, I successfully tried up to -smp 16 -cpu host,cores=8 with
> WindowsXP Pro.
>
>    

I thought of using -smp [processors=]2,cores=4,threads=2 (for a total of 
16 threads), but I think it makes more sense with -cpu.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Alexander Graf]

On 04.07.2009, at 17:25, Avi Kivity wrote:

> On 07/03/2009 05:41 PM, Andre Przywara wrote:
>> Hi,
>>
>> currently SMP guests happen to see<n>  vCPUs as<n>  different  
>> sockets.
>> Some guests (Windows comes to mind) have license restrictions and  
>> refuse
>> to run on multi-socket machines.
>> So lets introduce a "cores=" parameter to the -cpu option to let  
>> the user
>> specify the number of _cores_ the guest should see.
>>
>> This patch has not been tested with all corner cases, so I just  
>> want to
>> hear your comments whether
>> a) we need such an option  and
>> b) you like this particular approach.
>>
>> Applying this qemu.git patch to qemu-kvm.git fixes Windows SMP boot  
>> on
>> some versions, I successfully tried up to -smp 16 -cpu host,cores=8  
>> with
>> WindowsXP Pro.
>>
>>
>
> I thought of using -smp [processors=]2,cores=4,threads=2 (for a  
> total of 16 threads), but I think it makes more sense with -cpu.

I actually think putting this in -smp makes more sense. -cpu really  
shouldn't need to be touched by normal users and as long as you can  
either -cpu host or -cpu safe that should be enough.

But then again maybe we should replace -smp with something more useful  
like -numa where you'd then specify #CPUs, #cores, mem-cpu connection,  
etc.

Alex

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Avi Kivity]

On 07/05/2009 04:23 PM, Alexander Graf wrote:
>> I thought of using -smp [processors=]2,cores=4,threads=2 (for a total 
>> of 16 threads), but I think it makes more sense with -cpu.
>
>
> I actually think putting this in -smp makes more sense. -cpu really 
> shouldn't need to be touched by normal users and as long as you can 
> either -cpu host or -cpu safe that should be enough.

Maybe.  But in that case -cpu core2duo should imply cores=2 and -smp 2 
-cpu core2duo will bring up 4 cores spread across two sockets.

> But then again maybe we should replace -smp with something more useful 
> like -numa where you'd then specify #CPUs, #cores, mem-cpu connection, 
> etc.

I'd prefer -numa to specify the memory topology (and connections of 
sockets to memory nodes), and -smp or -cpu to specify the intra-socket 
topology.

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

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Gleb Natapov]

On Sun, Jul 05, 2009 at 05:53:17PM +0300, Avi Kivity wrote:
> On 07/05/2009 04:23 PM, Alexander Graf wrote:
>>> I thought of using -smp [processors=]2,cores=4,threads=2 (for a total 
>>> of 16 threads), but I think it makes more sense with -cpu.
>>
>>
>> I actually think putting this in -smp makes more sense. -cpu really  
>> shouldn't need to be touched by normal users and as long as you can  
>> either -cpu host or -cpu safe that should be enough.
>
> Maybe.  But in that case -cpu core2duo should imply cores=2 and -smp 2  
> -cpu core2duo will bring up 4 cores spread across two sockets.
>
core2duo does not imply 2 cores. OSes use cpuid to discover this
information.

>> But then again maybe we should replace -smp with something more useful  
>> like -numa where you'd then specify #CPUs, #cores, mem-cpu connection,  
>> etc.
>
> I'd prefer -numa to specify the memory topology (and connections of  
> sockets to memory nodes), and -smp or -cpu to specify the intra-socket  
> topology.
>
I agree. -numa is a different story. It is possible to have zillion
socket setup without numa at all.

--
			Gleb.

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Avi Kivity]

On 07/05/2009 06:04 PM, Gleb Natapov wrote:
> core2duo does not imply 2 cores. OSes use cpuid to discover this
> information.
>    

It does.  "Core 2" is the name of the core.  "Core 2 Duo" is a 2-core 
package containing (a pair) this core.  "Core 2 Solo" is a single-core 
package containing the core.

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

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Alexander Graf]

On 05.07.2009, at 17:11, Avi Kivity wrote:

> On 07/05/2009 06:04 PM, Gleb Natapov wrote:
>> core2duo does not imply 2 cores. OSes use cpuid to discover this
>> information.
>>
>
> It does.  "Core 2" is the name of the core.  "Core 2 Duo" is a 2- 
> core package containing (a pair) this core.  "Core 2 Solo" is a  
> single-core package containing the core.

Well, then let's better make it -cpu core2, no?
The only problem I see is how to generate the description string.  
Model number and the likes should be identical to a Core2Solo, right?

Alex

Re: [RFC] allow multi-core guests: introduce cores= option to -cpu

[Avi Kivity]

On 07/05/2009 06:11 PM, Alexander Graf wrote:
>
> On 05.07.2009, at 17:11, Avi Kivity wrote:
>
>> On 07/05/2009 06:04 PM, Gleb Natapov wrote:
>>> core2duo does not imply 2 cores. OSes use cpuid to discover this
>>> information.
>>>
>>
>> It does.  "Core 2" is the name of the core.  "Core 2 Duo" is a 2-core 
>> package containing (a pair) this core.  "Core 2 Solo" is a 
>> single-core package containing the core.
>
> Well, then let's better make it -cpu core2, no?

Maybe have all three.  But I agree -cpu core2 is better than -cpu core2duo.

> The only problem I see is how to generate the description string. 
> Model number and the likes should be identical to a Core2Solo, right?

I think so.

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