diff mbox

[v3] target-i386: implement CPUID[0xB] (Extended Topology Enumeration)

Message ID 1463073866-28802-1-git-send-email-rkrcmar@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Radim Krčmář May 12, 2016, 5:24 p.m. UTC
I looked at a dozen Intel CPU that have this CPUID and all of them
always had Core offset as 1 (a wasted bit when hyperthreading is
disabled) and Package offset at least 4 (wasted bits at <= 4 cores).

QEMU uses more compact IDs and it doesn't make much sense to change it
now.  I keep the SMT and Core sub-leaves even if there is just one
thread/core;  it makes the code simpler and there should be no harm.

Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com>
---
 This patch depends on Igor's "pc: add 2.7 machine".

 v2:
 * assert *eax instead of silently masking [Eduardo]
 * backward compatibility through CPU property [Eduardo]

 v3: use count instead of *ecx to access leaves, hardware does

 include/hw/i386/pc.h  |  7 ++++++-
 target-i386/cpu-qom.h |  3 +++
 target-i386/cpu.c     | 32 ++++++++++++++++++++++++++++++++
 target-i386/cpu.h     |  5 +++++
 4 files changed, 46 insertions(+), 1 deletion(-)

Comments

Igor Mammedov May 13, 2016, 9:32 a.m. UTC | #1
On Thu, 12 May 2016 19:24:26 +0200
Radim Kr?má? <rkrcmar@redhat.com> wrote:

> I looked at a dozen Intel CPU that have this CPUID and all of them
> always had Core offset as 1 (a wasted bit when hyperthreading is
> disabled) and Package offset at least 4 (wasted bits at <= 4 cores).
> 
> QEMU uses more compact IDs and it doesn't make much sense to change it
> now.  I keep the SMT and Core sub-leaves even if there is just one
> thread/core;  it makes the code simpler and there should be no harm.
> 
> Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  This patch depends on Igor's "pc: add 2.7 machine".
> 
>  v2:
>  * assert *eax instead of silently masking [Eduardo]
>  * backward compatibility through CPU property [Eduardo]
> 
>  v3: use count instead of *ecx to access leaves, hardware does
> 
>  include/hw/i386/pc.h  |  7 ++++++-
>  target-i386/cpu-qom.h |  3 +++
>  target-i386/cpu.c     | 32 ++++++++++++++++++++++++++++++++
>  target-i386/cpu.h     |  5 +++++
>  4 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index a91e8e734f07..e294fa945d30 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -361,7 +361,12 @@ int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
>  #define PC_COMPAT_2_6 \
> -    HW_COMPAT_2_6
> +    HW_COMPAT_2_6 \
> +    {\
> +        .driver   = TYPE_X86_CPU,\
> +        .property = "cpuid-0xb",\
> +        .value    = "off",\
> +    },
>  
>  #define PC_COMPAT_2_5 \
>      PC_COMPAT_2_6 \
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index cb750176c0c0..b84963d42463 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -115,6 +115,9 @@ typedef struct X86CPU {
>       */
>      bool enable_pmu;
>  
> +    /* Compatibility bits for old machine types. */
> +    bool enable_cpuid_0xb;
> +
>      /* in order to simplify APIC support, we leave this pointer to the
>         user */
>      struct DeviceState *apic_state;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index d0b5b691563c..a17ef191c270 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -35,6 +35,7 @@
>  #include "sysemu/arch_init.h"
>  
>  #include "hw/hw.h"
> +#include "hw/i386/topology.h"
>  #if defined(CONFIG_KVM)
>  #include <linux/kvm_para.h>
>  #endif
> @@ -2460,6 +2461,36 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              *edx = 0;
>          }
>          break;
> +    case 0xB:
> +        /* Extended Topology Enumeration Leaf */
> +        if (!cpu->enable_cpuid_0xb) {
> +                *eax = *ebx = *ecx = *edx = 0;
> +                break;
> +        }
> +
> +        *ecx = count & 0xff;
> +        *edx = cpu->apic_id;
> +
> +        switch (count) {
> +        case 0:
> +            *eax = apicid_core_offset(smp_cores, smp_threads);
> +            *ebx = smp_threads;
> +            *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
> +            break;
> +        case 1:
> +            *eax = apicid_pkg_offset(smp_cores, smp_threads);
> +            *ebx = smp_cores * smp_threads;
> +            *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
> +            break;
> +        default:
> +            *eax = 0;
> +            *ebx = 0;
> +            *ecx |= CPUID_TOPOLOGY_LEVEL_INVALID;
> +        }
> +
> +        assert(!(*eax & ~0x1f));
> +        *ebx &= 0xffff; /* The count doesn't need to be reliable. */
> +        break;
>      case 0xD: {
>          KVMState *s = cs->kvm_state;
>          uint64_t ena_mask;
> @@ -3221,6 +3252,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
>      DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0),
>      DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
> +    DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 732eb6d7ec79..b460c2debc1c 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -635,6 +635,11 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>  #define CPUID_MWAIT_IBE     (1U << 1) /* Interrupts can exit capability */
>  #define CPUID_MWAIT_EMX     (1U << 0) /* enumeration supported */
>  
> +/* CPUID[0xB].ECX level types */
> +#define CPUID_TOPOLOGY_LEVEL_INVALID  (0U << 8)
> +#define CPUID_TOPOLOGY_LEVEL_SMT      (1U << 8)
> +#define CPUID_TOPOLOGY_LEVEL_CORE     (2U << 8)
> +
>  #ifndef HYPERV_SPINLOCK_NEVER_RETRY
>  #define HYPERV_SPINLOCK_NEVER_RETRY             0xFFFFFFFF
>  #endif
Eduardo Habkost May 27, 2016, 7:12 p.m. UTC | #2
On Thu, May 12, 2016 at 07:24:26PM +0200, Radim Krčmář wrote:
> I looked at a dozen Intel CPU that have this CPUID and all of them
> always had Core offset as 1 (a wasted bit when hyperthreading is
> disabled) and Package offset at least 4 (wasted bits at <= 4 cores).
> 
> QEMU uses more compact IDs and it doesn't make much sense to change it
> now.  I keep the SMT and Core sub-leaves even if there is just one
> thread/core;  it makes the code simpler and there should be no harm.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Applied to x86-next.
diff mbox

Patch

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a91e8e734f07..e294fa945d30 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -361,7 +361,12 @@  int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
 #define PC_COMPAT_2_6 \
-    HW_COMPAT_2_6
+    HW_COMPAT_2_6 \
+    {\
+        .driver   = TYPE_X86_CPU,\
+        .property = "cpuid-0xb",\
+        .value    = "off",\
+    },
 
 #define PC_COMPAT_2_5 \
     PC_COMPAT_2_6 \
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index cb750176c0c0..b84963d42463 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -115,6 +115,9 @@  typedef struct X86CPU {
      */
     bool enable_pmu;
 
+    /* Compatibility bits for old machine types. */
+    bool enable_cpuid_0xb;
+
     /* in order to simplify APIC support, we leave this pointer to the
        user */
     struct DeviceState *apic_state;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d0b5b691563c..a17ef191c270 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -35,6 +35,7 @@ 
 #include "sysemu/arch_init.h"
 
 #include "hw/hw.h"
+#include "hw/i386/topology.h"
 #if defined(CONFIG_KVM)
 #include <linux/kvm_para.h>
 #endif
@@ -2460,6 +2461,36 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *edx = 0;
         }
         break;
+    case 0xB:
+        /* Extended Topology Enumeration Leaf */
+        if (!cpu->enable_cpuid_0xb) {
+                *eax = *ebx = *ecx = *edx = 0;
+                break;
+        }
+
+        *ecx = count & 0xff;
+        *edx = cpu->apic_id;
+
+        switch (count) {
+        case 0:
+            *eax = apicid_core_offset(smp_cores, smp_threads);
+            *ebx = smp_threads;
+            *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
+            break;
+        case 1:
+            *eax = apicid_pkg_offset(smp_cores, smp_threads);
+            *ebx = smp_cores * smp_threads;
+            *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
+            break;
+        default:
+            *eax = 0;
+            *ebx = 0;
+            *ecx |= CPUID_TOPOLOGY_LEVEL_INVALID;
+        }
+
+        assert(!(*eax & ~0x1f));
+        *ebx &= 0xffff; /* The count doesn't need to be reliable. */
+        break;
     case 0xD: {
         KVMState *s = cs->kvm_state;
         uint64_t ena_mask;
@@ -3221,6 +3252,7 @@  static Property x86_cpu_properties[] = {
     DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
     DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0),
     DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
+    DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 732eb6d7ec79..b460c2debc1c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -635,6 +635,11 @@  typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_MWAIT_IBE     (1U << 1) /* Interrupts can exit capability */
 #define CPUID_MWAIT_EMX     (1U << 0) /* enumeration supported */
 
+/* CPUID[0xB].ECX level types */
+#define CPUID_TOPOLOGY_LEVEL_INVALID  (0U << 8)
+#define CPUID_TOPOLOGY_LEVEL_SMT      (1U << 8)
+#define CPUID_TOPOLOGY_LEVEL_CORE     (2U << 8)
+
 #ifndef HYPERV_SPINLOCK_NEVER_RETRY
 #define HYPERV_SPINLOCK_NEVER_RETRY             0xFFFFFFFF
 #endif