From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Jackson Subject: Re: [RFC] allow multi-core guests: introduce cores= option to -cpu Date: Fri, 03 Jul 2009 10:16:10 -0500 Message-ID: <4A4E20BA.2040100@theiggy.com> References: <1246632116-31366-1-git-send-email-andre.przywara@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: avi@redhat.com, anthony@codemonkey.ws, qemu-devel@nongnu.org, kvm@vger.kernel.org To: Andre Przywara Return-path: Received: from theiggy.com ([66.220.1.110]:34438 "EHLO mail.theiggy.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755752AbZGCPPL (ORCPT ); Fri, 3 Jul 2009 11:15:11 -0400 In-Reply-To: <1246632116-31366-1-git-send-email-andre.przywara@amd.com> Sender: kvm-owner@vger.kernel.org List-ID: Andre Przywara wrote: > Hi, > > currently SMP guests happen to see vCPUs as 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 > --- > 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 */