| Message ID | 1246632116-31366-1-git-send-email-andre.przywara@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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.
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, 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 */
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(-)