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

[Brian Jackson <iggy@theiggy.com>]

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 */