diff mbox series

[v13,4/5] i386: Verify and enable topoext feature if supported

Message ID 1528498581-131037-5-git-send-email-babu.moger@amd.com
State New
Headers show
Series i386: Enable TOPOEXT to support hyperthreading on AMD CPU | expand

Commit Message

Moger, Babu June 8, 2018, 10:56 p.m. UTC 
If the CPU model supports topoext feature, enabled the
feature automatically if it can be supported.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 target/i386/cpu.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Eduardo Habkost June 11, 2018, 8:51 p.m. UTC | #1 
On Fri, Jun 08, 2018 at 06:56:20PM -0400, Babu Moger wrote:
> If the CPU model supports topoext feature, enabled the
> feature automatically if it can be supported.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  target/i386/cpu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 4dd9a82..88bc73d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4763,6 +4763,33 @@ static int x86_cpu_filter_features(X86CPU *cpu)
>  #define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
>                           (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
>                           (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
> +/*
> + * Check if we can support this topology
> + * Fail if number of cores are beyond the supported config
> + * or nr_threads is more than 2
> + */
> +static int topology_supports_topoext(int nr_cores, int nr_threads,
> +            Error **errp)
> +{
> +    if (nr_cores > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET)) {
> +        error_setg(errp, "TOPOEXT unsupported with %d cores per socket",
> +                   nr_cores);
> +        error_append_hint(errp, "TOPOEXT supports only up to %d cores per"
> +                          " socket\n",
> +                          (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
> +        return false;
> +    }
> +
> +    if (nr_threads > 2) {
> +        error_setg(errp, "TOPOEXT unsupported with %d threads per core",
> +                   nr_threads);
> +        error_append_hint(errp, "TOPOEXT supports only up to 2 threads"
> +                          " per core\n");
> +        return false;
> +    }
> +    return true;
> +}
> +
>  static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>  {
>      CPUState *cs = CPU(dev);
> @@ -4953,6 +4980,19 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>  
>      qemu_init_vcpu(cs);
>  
> +    if (cpu->auto_topoext &&
> +        !(env->user_features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT)) {
> +        if (cs->nr_cores <= (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET) &&
> +            (cs->nr_threads <= 2)) {

This duplicates the logic from topology_supports_topoext().  Why
not just call
  topology_supports_topoext(cs->nr_cores, cs->nr_threads, NULL)
here?

> +            env->features[FEAT_8000_0001_ECX] |= CPUID_EXT3_TOPOEXT;
> +        }
> +    }
> +
> +    if ((env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
> +        !topology_supports_topoext(cs->nr_cores, cs->nr_threads, errp)) {
> +        return;
> +    }
> +
>      /* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
>       * issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX
>       * based on inputs (sockets,cores,threads), it is still better to gives
> -- 
> 1.8.3.1
>
Moger, Babu June 11, 2018, 9:01 p.m. UTC | #2 
> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org]
> On Behalf Of Eduardo Habkost
> Sent: Monday, June 11, 2018 3:52 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com;
> rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com
> Subject: Re: [PATCH v13 4/5] i386: Verify and enable topoext feature if
> supported
> 
> On Fri, Jun 08, 2018 at 06:56:20PM -0400, Babu Moger wrote:
> > If the CPU model supports topoext feature, enabled the
> > feature automatically if it can be supported.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  target/i386/cpu.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 4dd9a82..88bc73d 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -4763,6 +4763,33 @@ static int x86_cpu_filter_features(X86CPU *cpu)
> >  #define IS_AMD_CPU(env) ((env)->cpuid_vendor1 ==
> CPUID_VENDOR_AMD_1 && \
> >                           (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
> >                           (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
> > +/*
> > + * Check if we can support this topology
> > + * Fail if number of cores are beyond the supported config
> > + * or nr_threads is more than 2
> > + */
> > +static int topology_supports_topoext(int nr_cores, int nr_threads,
> > +            Error **errp)
> > +{
> > +    if (nr_cores > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET)) {
> > +        error_setg(errp, "TOPOEXT unsupported with %d cores per socket",
> > +                   nr_cores);
> > +        error_append_hint(errp, "TOPOEXT supports only up to %d cores
> per"
> > +                          " socket\n",
> > +                          (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
> > +        return false;
> > +    }
> > +
> > +    if (nr_threads > 2) {
> > +        error_setg(errp, "TOPOEXT unsupported with %d threads per core",
> > +                   nr_threads);
> > +        error_append_hint(errp, "TOPOEXT supports only up to 2 threads"
> > +                          " per core\n");
> > +        return false;
> > +    }
> > +    return true;
> > +}
> > +
> >  static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> >  {
> >      CPUState *cs = CPU(dev);
> > @@ -4953,6 +4980,19 @@ static void x86_cpu_realizefn(DeviceState *dev,
> Error **errp)
> >
> >      qemu_init_vcpu(cs);
> >
> > +    if (cpu->auto_topoext &&
> > +        !(env->user_features[FEAT_8000_0001_ECX] &
> CPUID_EXT3_TOPOEXT)) {
> > +        if (cs->nr_cores <= (MAX_CORES_IN_NODE *
> MAX_NODES_PER_SOCKET) &&
> > +            (cs->nr_threads <= 2)) {
> 
> This duplicates the logic from topology_supports_topoext().  Why
> not just call
>   topology_supports_topoext(cs->nr_cores, cs->nr_threads, NULL)
> here?

Ok. Will do it.

> 
> > +            env->features[FEAT_8000_0001_ECX] |= CPUID_EXT3_TOPOEXT;
> > +        }
> > +    }
> > +
> > +    if ((env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
> > +        !topology_supports_topoext(cs->nr_cores, cs->nr_threads, errp)) {
> > +        return;
> > +    }
> > +
> >      /* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
> >       * issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX
> >       * based on inputs (sockets,cores,threads), it is still better to gives
> > --
> > 1.8.3.1
> >
> 
> --
> Eduardo
diff mbox series

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4dd9a82..88bc73d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4763,6 +4763,33 @@  static int x86_cpu_filter_features(X86CPU *cpu)
 #define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
                          (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
                          (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
+/*
+ * Check if we can support this topology
+ * Fail if number of cores are beyond the supported config
+ * or nr_threads is more than 2
+ */
+static int topology_supports_topoext(int nr_cores, int nr_threads,
+            Error **errp)
+{
+    if (nr_cores > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET)) {
+        error_setg(errp, "TOPOEXT unsupported with %d cores per socket",
+                   nr_cores);
+        error_append_hint(errp, "TOPOEXT supports only up to %d cores per"
+                          " socket\n",
+                          (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
+        return false;
+    }
+
+    if (nr_threads > 2) {
+        error_setg(errp, "TOPOEXT unsupported with %d threads per core",
+                   nr_threads);
+        error_append_hint(errp, "TOPOEXT supports only up to 2 threads"
+                          " per core\n");
+        return false;
+    }
+    return true;
+}
+
 static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -4953,6 +4980,19 @@  static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
 
     qemu_init_vcpu(cs);
 
+    if (cpu->auto_topoext &&
+        !(env->user_features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT)) {
+        if (cs->nr_cores <= (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET) &&
+            (cs->nr_threads <= 2)) {
+            env->features[FEAT_8000_0001_ECX] |= CPUID_EXT3_TOPOEXT;
+        }
+    }
+
+    if ((env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
+        !topology_supports_topoext(cs->nr_cores, cs->nr_threads, errp)) {
+        return;
+    }
+
     /* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
      * issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX
      * based on inputs (sockets,cores,threads), it is still better to gives