[PATCH 0/2] add thread id and online state to cpu topology info

[PATCH 0/2] add thread id and online state to cpu topology info

[Juergen Gross]

The topology information printed via "xl info -n" doesn't contain the
thread id of the cpu. Additionally especially with the "smt=0" boot
option it is interesting to know which cpu is online or offline.

This patch series adds the respective information to the cpu topology
information.

This patch series is depending on the series "fix smt=0 fallout".

Juergen Gross (2):
  add cpu_thread_id to struct cpuinfo_x86
  add thread and state info to XEN_SYSCTL_cputopoinfo

 tools/libxl/libxl.c             | 13 +++++++++++++
 tools/libxl/libxl_types.idl     |  8 ++++++++
 tools/xl/xl_info.c              |  8 +++++---
 xen/arch/x86/cpu/common.c       |  1 +
 xen/arch/x86/smpboot.c          | 10 ++++++++++
 xen/common/sysctl.c             |  4 ++++
 xen/include/asm-arm/processor.h |  1 +
 xen/include/asm-x86/processor.h |  2 ++
 xen/include/public/sysctl.h     |  7 ++++++-
 9 files changed, 50 insertions(+), 4 deletions(-)

-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Juergen Gross]

Add the thread-id to the cpu config data and an accessor macro
cpu_to_thread().

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/cpu/common.c       |  1 +
 xen/arch/x86/smpboot.c          | 10 ++++++++++
 xen/include/asm-arm/processor.h |  1 +
 xen/include/asm-x86/processor.h |  2 ++
 xen/include/public/sysctl.h     |  1 +
 5 files changed, 15 insertions(+)

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 057859ab14..f626a6a510 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
 	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
 	c->cpu_core_id = XEN_INVALID_CORE_ID;
 	c->compute_unit_id = INVALID_CUID;
+	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
 	memset(&c->x86_capability, 0, sizeof c->x86_capability);
 
 	generic_identify(c);
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 7e76cc3d68..2c6a40c543 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
     cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
     cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
 
+    c[cpu].cpu_thread_id = 0;
+
     if ( c[cpu].x86_num_siblings > 1 )
     {
         for_each_cpu ( i, &cpu_sibling_setup_map )
@@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
                        "CPU%u: unclear relationship with CPU%u\n",
                        cpu, i);
         }
+
+        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
+        {
+            if ( cpu == i )
+                break;
+            c[cpu].cpu_thread_id++;
+        }
+
     }
 
     if ( c[cpu].x86_max_cores == 1 )
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 222a02dd99..fbfbe1962c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -801,6 +801,7 @@ void show_registers(struct cpu_user_regs *regs);
 #define cpu_relax() barrier() /* Could yield? */
 
 /* All a bit UP for the moment */
+#define cpu_to_thread(_cpu) (0)
 #define cpu_to_core(_cpu)   (0)
 #define cpu_to_socket(_cpu) (0)
 
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index a166802344..850d8f50f0 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -134,6 +134,7 @@ struct cpuinfo_x86 {
     __u32 apicid;
     __u32 phys_proc_id;    /* package ID of each logical CPU */
     __u32 cpu_core_id;     /* core ID of each logical CPU*/
+    __u32 cpu_thread_id;   /* thread ID of each logical CPU */
     __u32 compute_unit_id; /* AMD compute unit ID of each logical CPU */
     unsigned short x86_clflush_size;
 } __cacheline_aligned;
@@ -174,6 +175,7 @@ extern void detect_extended_topology(struct cpuinfo_x86 *c);
 
 extern void detect_ht(struct cpuinfo_x86 *c);
 
+#define cpu_to_thread(_cpu) (cpu_data[_cpu].cpu_thread_id)
 #define cpu_to_core(_cpu)   (cpu_data[_cpu].cpu_core_id)
 #define cpu_to_socket(_cpu) (cpu_data[_cpu].phys_proc_id)
 
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 839c1b9f25..e439e00983 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -434,6 +434,7 @@ struct xen_sysctl_lockprof_op {
 };
 
 /* XEN_SYSCTL_cputopoinfo */
+#define XEN_INVALID_THREAD_ID   (~0U)
 #define XEN_INVALID_CORE_ID     (~0U)
 #define XEN_INVALID_SOCKET_ID   (~0U)
 #define XEN_INVALID_NODE_ID     (~0U)
-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/2] add thread and state info to XEN_SYSCTL_cputopoinfo

[Juergen Gross]

Today the topology information obtained via XEN_SYSCTL_cputopoinfo
doesn't contain the thread id. Add that.

As especially with the boot parameter "smt=0" offline cpus are more
common these days add a state indicator (online/offline) to the
returned information as well.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl.c         | 13 +++++++++++++
 tools/libxl/libxl_types.idl |  8 ++++++++
 tools/xl/xl_info.c          |  8 +++++---
 xen/common/sysctl.c         |  4 ++++
 xen/include/public/sysctl.h |  6 +++++-
 5 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b41ade9fda..fc38223a06 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -430,10 +430,23 @@ libxl_cputopology *libxl_get_cpu_topology(libxl_ctx *ctx, int *nb_cpu_out)
     for (i = 0; i < num_cpus; i++) {
 #define V(map, i, invalid) ( cputopo[i].map == invalid) ? \
    LIBXL_CPUTOPOLOGY_INVALID_ENTRY : cputopo[i].map
+        ret[i].thread = V(thread, i, XEN_INVALID_THREAD_ID);
         ret[i].core = V(core, i, XEN_INVALID_CORE_ID);
         ret[i].socket = V(socket, i, XEN_INVALID_SOCKET_ID);
         ret[i].node = V(node, i, XEN_INVALID_NODE_ID);
 #undef V
+        switch (cputopo[i].state) {
+        case XEN_TOPO_STATE_OFFLINE:
+            ret[i].state = LIBXL_CPUTOPOLOGY_STATE_TYPE_OFFLINE;
+            break;
+        case XEN_TOPO_STATE_ONLINE:
+            ret[i].state = LIBXL_CPUTOPOLOGY_STATE_TYPE_ONLINE;
+            break;
+        default:
+            LOGE(WARN, "Invalid cpu state");
+            ret[i].state = LIBXL_CPUTOPOLOGY_STATE_TYPE_UNKNOWN;
+            break;
+        }
     }
 
     *nb_cpu_out = num_cpus;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 4a385801ba..b1a8950ec7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -1076,10 +1076,18 @@ libxl_numainfo = Struct("numainfo", [
     ("dists", Array(uint32, "num_dists")),
     ], dir=DIR_OUT)
 
+libxl_cputopology_state_type = Enumeration("cputopology_state_type", [
+    (0, "unknown"),
+    (1, "offline"),
+    (2, "online")
+    ])
+
 libxl_cputopology = Struct("cputopology", [
+    ("thread", uint32),
     ("core", uint32),
     ("socket", uint32),
     ("node", uint32),
+    ("state", libxl_cputopology_state_type),
     ], dir=DIR_OUT)
 
 libxl_pcitopology = Struct("pcitopology", [
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 6c8be26119..34ef483e49 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -280,12 +280,14 @@ static void output_topologyinfo(void)
     }
 
     printf("cpu_topology           :\n");
-    printf("cpu:    core    socket     node\n");
+    printf("cpu:  thread     core   socket     node    state\n");
 
     for (i = 0; i < nr; i++) {
         if (cpuinfo[i].core != LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
-            printf("%3d:    %4d     %4d     %4d\n", i,
-                   cpuinfo[i].core, cpuinfo[i].socket, cpuinfo[i].node);
+            printf("%3d:    %4d     %4d     %4d     %4d    %s\n", i,
+                   cpuinfo[i].thread, cpuinfo[i].core, cpuinfo[i].socket,
+                   cpuinfo[i].node,
+                   libxl_cputopology_state_type_to_string(cpuinfo[i].state));
     }
 
     libxl_cputopology_list_free(cpuinfo, nr);
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index c0aa6bde4e..8b6263a067 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -360,6 +360,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
             {
                 if ( cpu_present(i) )
                 {
+                    cputopo.thread = cpu_to_thread(i);
                     cputopo.core = cpu_to_core(i);
                     cputopo.socket = cpu_to_socket(i);
                     cputopo.node = cpu_to_node(i);
@@ -368,10 +369,13 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
                 }
                 else
                 {
+                    cputopo.thread = XEN_INVALID_THREAD_ID;
                     cputopo.core = XEN_INVALID_CORE_ID;
                     cputopo.socket = XEN_INVALID_SOCKET_ID;
                     cputopo.node = XEN_INVALID_NODE_ID;
                 }
+                cputopo.state = cpu_online(i) ? XEN_TOPO_STATE_ONLINE
+                                              : XEN_TOPO_STATE_OFFLINE;
 
                 if ( copy_to_guest_offset(ti->cputopo, i, &cputopo, 1) )
                 {
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index e439e00983..ffb750a212 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
 #include "physdev.h"
 #include "tmem.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000011
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000012
 
 /*
  * Read console content from Xen buffer ring.
@@ -434,15 +434,19 @@ struct xen_sysctl_lockprof_op {
 };
 
 /* XEN_SYSCTL_cputopoinfo */
+#define XEN_TOPO_STATE_OFFLINE  0
+#define XEN_TOPO_STATE_ONLINE   1
 #define XEN_INVALID_THREAD_ID   (~0U)
 #define XEN_INVALID_CORE_ID     (~0U)
 #define XEN_INVALID_SOCKET_ID   (~0U)
 #define XEN_INVALID_NODE_ID     (~0U)
 
 struct xen_sysctl_cputopo {
+    uint32_t thread;
     uint32_t core;
     uint32_t socket;
     uint32_t node;
+    uint32_t state;
 };
 typedef struct xen_sysctl_cputopo xen_sysctl_cputopo_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cputopo_t);
-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Jan Beulich]

>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
> Add the thread-id to the cpu config data and an accessor macro
> cpu_to_thread().
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  xen/arch/x86/cpu/common.c       |  1 +
>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>  xen/include/asm-arm/processor.h |  1 +
>  xen/include/asm-x86/processor.h |  2 ++
>  xen/include/public/sysctl.h     |  1 +
>  5 files changed, 15 insertions(+)
> 
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index 057859ab14..f626a6a510 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>  	c->compute_unit_id = INVALID_CUID;
> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>  
>  	generic_identify(c);
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 7e76cc3d68..2c6a40c543 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>  
> +    c[cpu].cpu_thread_id = 0;
> +
>      if ( c[cpu].x86_num_siblings > 1 )
>      {
>          for_each_cpu ( i, &cpu_sibling_setup_map )
> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>                         "CPU%u: unclear relationship with CPU%u\n",
>                         cpu, i);
>          }
> +
> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
> +        {
> +            if ( cpu == i )
> +                break;
> +            c[cpu].cpu_thread_id++;

No. This wants doing alongside setting of the other similar fields,
i.e. in (at least) detect_extended_topology() and detect_ht(). It
is the hardware which tells us what the thread ID is.

I think you also want to reset the new field in cpu_smpboot_free().

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] add thread and state info to XEN_SYSCTL_cputopoinfo

[Jan Beulich]

>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
> @@ -434,15 +434,19 @@ struct xen_sysctl_lockprof_op {
>  };
>  
>  /* XEN_SYSCTL_cputopoinfo */
> +#define XEN_TOPO_STATE_OFFLINE  0
> +#define XEN_TOPO_STATE_ONLINE   1
>  #define XEN_INVALID_THREAD_ID   (~0U)
>  #define XEN_INVALID_CORE_ID     (~0U)
>  #define XEN_INVALID_SOCKET_ID   (~0U)
>  #define XEN_INVALID_NODE_ID     (~0U)
>  
>  struct xen_sysctl_cputopo {
> +    uint32_t thread;
>      uint32_t core;
>      uint32_t socket;
>      uint32_t node;
> +    uint32_t state;
>  };

Let's not burn an entire 32-bit field for this - a single bit suffices.
I'd suggest you call it flags, and use bit 0 for the online/offline
indicator. You then also don't need an "unknown" state in the
tools.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Juergen Gross]

On 03/09/18 15:46, Jan Beulich wrote:
>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>> Add the thread-id to the cpu config data and an accessor macro
>> cpu_to_thread().
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  xen/arch/x86/cpu/common.c       |  1 +
>>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>>  xen/include/asm-arm/processor.h |  1 +
>>  xen/include/asm-x86/processor.h |  2 ++
>>  xen/include/public/sysctl.h     |  1 +
>>  5 files changed, 15 insertions(+)
>>
>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>> index 057859ab14..f626a6a510 100644
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>>  	c->compute_unit_id = INVALID_CUID;
>> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>>  
>>  	generic_identify(c);
>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>> index 7e76cc3d68..2c6a40c543 100644
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>>  
>> +    c[cpu].cpu_thread_id = 0;
>> +
>>      if ( c[cpu].x86_num_siblings > 1 )
>>      {
>>          for_each_cpu ( i, &cpu_sibling_setup_map )
>> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>                         "CPU%u: unclear relationship with CPU%u\n",
>>                         cpu, i);
>>          }
>> +
>> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
>> +        {
>> +            if ( cpu == i )
>> +                break;
>> +            c[cpu].cpu_thread_id++;
> 
> No. This wants doing alongside setting of the other similar fields,
> i.e. in (at least) detect_extended_topology() and detect_ht(). It
> is the hardware which tells us what the thread ID is.

I wasn't able to find the related information for AMD cpus. In case it
is in the specs it is hidden very well.

> I think you also want to reset the new field in cpu_smpboot_free().

Ah yes, indeed!


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] add thread and state info to XEN_SYSCTL_cputopoinfo

[Juergen Gross]

On 03/09/18 15:49, Jan Beulich wrote:
>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>> @@ -434,15 +434,19 @@ struct xen_sysctl_lockprof_op {
>>  };
>>  
>>  /* XEN_SYSCTL_cputopoinfo */
>> +#define XEN_TOPO_STATE_OFFLINE  0
>> +#define XEN_TOPO_STATE_ONLINE   1
>>  #define XEN_INVALID_THREAD_ID   (~0U)
>>  #define XEN_INVALID_CORE_ID     (~0U)
>>  #define XEN_INVALID_SOCKET_ID   (~0U)
>>  #define XEN_INVALID_NODE_ID     (~0U)
>>  
>>  struct xen_sysctl_cputopo {
>> +    uint32_t thread;
>>      uint32_t core;
>>      uint32_t socket;
>>      uint32_t node;
>> +    uint32_t state;
>>  };
> 
> Let's not burn an entire 32-bit field for this - a single bit suffices.
> I'd suggest you call it flags, and use bit 0 for the online/offline
> indicator. You then also don't need an "unknown" state in the
> tools.

Okay.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Jan Beulich]

>>> On 03.09.18 at 15:53, <jgross@suse.com> wrote:
> On 03/09/18 15:46, Jan Beulich wrote:
>>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>>> Add the thread-id to the cpu config data and an accessor macro
>>> cpu_to_thread().
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>>  xen/arch/x86/cpu/common.c       |  1 +
>>>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>>>  xen/include/asm-arm/processor.h |  1 +
>>>  xen/include/asm-x86/processor.h |  2 ++
>>>  xen/include/public/sysctl.h     |  1 +
>>>  5 files changed, 15 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>>> index 057859ab14..f626a6a510 100644
>>> --- a/xen/arch/x86/cpu/common.c
>>> +++ b/xen/arch/x86/cpu/common.c
>>> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>>>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>>>  	c->compute_unit_id = INVALID_CUID;
>>> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>>>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>>>  
>>>  	generic_identify(c);
>>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>>> index 7e76cc3d68..2c6a40c543 100644
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>>>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>>>  
>>> +    c[cpu].cpu_thread_id = 0;
>>> +
>>>      if ( c[cpu].x86_num_siblings > 1 )
>>>      {
>>>          for_each_cpu ( i, &cpu_sibling_setup_map )
>>> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>                         "CPU%u: unclear relationship with CPU%u\n",
>>>                         cpu, i);
>>>          }
>>> +
>>> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
>>> +        {
>>> +            if ( cpu == i )
>>> +                break;
>>> +            c[cpu].cpu_thread_id++;
>> 
>> No. This wants doing alongside setting of the other similar fields,
>> i.e. in (at least) detect_extended_topology() and detect_ht(). It
>> is the hardware which tells us what the thread ID is.
> 
> I wasn't able to find the related information for AMD cpus. In case it
> is in the specs it is hidden very well.

What was used for compute units in Fam16 and earlier is used for
hyperthreads in Fam17 (and going forward, from what iirc Brian
has said).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Juergen Gross]

On 03/09/18 16:47, Jan Beulich wrote:
>>>> On 03.09.18 at 15:53, <jgross@suse.com> wrote:
>> On 03/09/18 15:46, Jan Beulich wrote:
>>>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>>>> Add the thread-id to the cpu config data and an accessor macro
>>>> cpu_to_thread().
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>>  xen/arch/x86/cpu/common.c       |  1 +
>>>>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>>>>  xen/include/asm-arm/processor.h |  1 +
>>>>  xen/include/asm-x86/processor.h |  2 ++
>>>>  xen/include/public/sysctl.h     |  1 +
>>>>  5 files changed, 15 insertions(+)
>>>>
>>>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>>>> index 057859ab14..f626a6a510 100644
>>>> --- a/xen/arch/x86/cpu/common.c
>>>> +++ b/xen/arch/x86/cpu/common.c
>>>> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>>>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>>>>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>>>>  	c->compute_unit_id = INVALID_CUID;
>>>> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>>>>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>>>>  
>>>>  	generic_identify(c);
>>>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>>>> index 7e76cc3d68..2c6a40c543 100644
>>>> --- a/xen/arch/x86/smpboot.c
>>>> +++ b/xen/arch/x86/smpboot.c
>>>> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>>>>  
>>>> +    c[cpu].cpu_thread_id = 0;
>>>> +
>>>>      if ( c[cpu].x86_num_siblings > 1 )
>>>>      {
>>>>          for_each_cpu ( i, &cpu_sibling_setup_map )
>>>> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>                         "CPU%u: unclear relationship with CPU%u\n",
>>>>                         cpu, i);
>>>>          }
>>>> +
>>>> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
>>>> +        {
>>>> +            if ( cpu == i )
>>>> +                break;
>>>> +            c[cpu].cpu_thread_id++;
>>>
>>> No. This wants doing alongside setting of the other similar fields,
>>> i.e. in (at least) detect_extended_topology() and detect_ht(). It
>>> is the hardware which tells us what the thread ID is.
>>
>> I wasn't able to find the related information for AMD cpus. In case it
>> is in the specs it is hidden very well.
> 
> What was used for compute units in Fam16 and earlier is used for
> hyperthreads in Fam17 (and going forward, from what iirc Brian
> has said).

And what about hyperthreads in Fam16 and earlier? Or are "compute units"
just another name for the threads and I can rename that field?


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Jan Beulich]

>>> On 03.09.18 at 16:52, <jgross@suse.com> wrote:
> On 03/09/18 16:47, Jan Beulich wrote:
>>>>> On 03.09.18 at 15:53, <jgross@suse.com> wrote:
>>> On 03/09/18 15:46, Jan Beulich wrote:
>>>>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>>>>> Add the thread-id to the cpu config data and an accessor macro
>>>>> cpu_to_thread().
>>>>>
>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>> ---
>>>>>  xen/arch/x86/cpu/common.c       |  1 +
>>>>>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>>>>>  xen/include/asm-arm/processor.h |  1 +
>>>>>  xen/include/asm-x86/processor.h |  2 ++
>>>>>  xen/include/public/sysctl.h     |  1 +
>>>>>  5 files changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>>>>> index 057859ab14..f626a6a510 100644
>>>>> --- a/xen/arch/x86/cpu/common.c
>>>>> +++ b/xen/arch/x86/cpu/common.c
>>>>> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>>>>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>>>>>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>>>>>  	c->compute_unit_id = INVALID_CUID;
>>>>> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>>>>>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>>>>>  
>>>>>  	generic_identify(c);
>>>>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>>>>> index 7e76cc3d68..2c6a40c543 100644
>>>>> --- a/xen/arch/x86/smpboot.c
>>>>> +++ b/xen/arch/x86/smpboot.c
>>>>> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>>>>>  
>>>>> +    c[cpu].cpu_thread_id = 0;
>>>>> +
>>>>>      if ( c[cpu].x86_num_siblings > 1 )
>>>>>      {
>>>>>          for_each_cpu ( i, &cpu_sibling_setup_map )
>>>>> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>                         "CPU%u: unclear relationship with CPU%u\n",
>>>>>                         cpu, i);
>>>>>          }
>>>>> +
>>>>> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
>>>>> +        {
>>>>> +            if ( cpu == i )
>>>>> +                break;
>>>>> +            c[cpu].cpu_thread_id++;
>>>>
>>>> No. This wants doing alongside setting of the other similar fields,
>>>> i.e. in (at least) detect_extended_topology() and detect_ht(). It
>>>> is the hardware which tells us what the thread ID is.
>>>
>>> I wasn't able to find the related information for AMD cpus. In case it
>>> is in the specs it is hidden very well.
>> 
>> What was used for compute units in Fam16 and earlier is used for
>> hyperthreads in Fam17 (and going forward, from what iirc Brian
>> has said).
> 
> And what about hyperthreads in Fam16 and earlier? Or are "compute units"
> just another name for the threads and I can rename that field?

When there are compute units, there are no threads. But the difference
is more than just the names, so folding the fields is not a good idea imo.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Andrew Cooper]

On 03/09/18 15:52, Juergen Gross wrote:
> On 03/09/18 16:47, Jan Beulich wrote:
>>>>> On 03.09.18 at 15:53, <jgross@suse.com> wrote:
>>> On 03/09/18 15:46, Jan Beulich wrote:
>>>>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>>>>> Add the thread-id to the cpu config data and an accessor macro
>>>>> cpu_to_thread().
>>>>>
>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>> ---
>>>>>  xen/arch/x86/cpu/common.c       |  1 +
>>>>>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>>>>>  xen/include/asm-arm/processor.h |  1 +
>>>>>  xen/include/asm-x86/processor.h |  2 ++
>>>>>  xen/include/public/sysctl.h     |  1 +
>>>>>  5 files changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>>>>> index 057859ab14..f626a6a510 100644
>>>>> --- a/xen/arch/x86/cpu/common.c
>>>>> +++ b/xen/arch/x86/cpu/common.c
>>>>> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>>>>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>>>>>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>>>>>  	c->compute_unit_id = INVALID_CUID;
>>>>> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>>>>>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>>>>>  
>>>>>  	generic_identify(c);
>>>>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>>>>> index 7e76cc3d68..2c6a40c543 100644
>>>>> --- a/xen/arch/x86/smpboot.c
>>>>> +++ b/xen/arch/x86/smpboot.c
>>>>> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>>>>>  
>>>>> +    c[cpu].cpu_thread_id = 0;
>>>>> +
>>>>>      if ( c[cpu].x86_num_siblings > 1 )
>>>>>      {
>>>>>          for_each_cpu ( i, &cpu_sibling_setup_map )
>>>>> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>                         "CPU%u: unclear relationship with CPU%u\n",
>>>>>                         cpu, i);
>>>>>          }
>>>>> +
>>>>> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
>>>>> +        {
>>>>> +            if ( cpu == i )
>>>>> +                break;
>>>>> +            c[cpu].cpu_thread_id++;
>>>> No. This wants doing alongside setting of the other similar fields,
>>>> i.e. in (at least) detect_extended_topology() and detect_ht(). It
>>>> is the hardware which tells us what the thread ID is.
>>> I wasn't able to find the related information for AMD cpus. In case it
>>> is in the specs it is hidden very well.
>> What was used for compute units in Fam16 and earlier is used for
>> hyperthreads in Fam17 (and going forward, from what iirc Brian
>> has said).
> And what about hyperthreads in Fam16 and earlier? Or are "compute units"
> just another name for the threads and I can rename that field?

Fam 15/16 "Compute Units" are complicated.

The have a shared L1 I$, fetch and branch prediction, separate integer
pipelines, but a shared FPU pipeline.

Half of the pipeline looks very much like threads, and half looks like
separate cores.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Juergen Gross]

On 03/09/18 17:07, Jan Beulich wrote:
>>>> On 03.09.18 at 16:52, <jgross@suse.com> wrote:
>> On 03/09/18 16:47, Jan Beulich wrote:
>>>>>> On 03.09.18 at 15:53, <jgross@suse.com> wrote:
>>>> On 03/09/18 15:46, Jan Beulich wrote:
>>>>>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>>>>>> Add the thread-id to the cpu config data and an accessor macro
>>>>>> cpu_to_thread().
>>>>>>
>>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>>> ---
>>>>>>  xen/arch/x86/cpu/common.c       |  1 +
>>>>>>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>>>>>>  xen/include/asm-arm/processor.h |  1 +
>>>>>>  xen/include/asm-x86/processor.h |  2 ++
>>>>>>  xen/include/public/sysctl.h     |  1 +
>>>>>>  5 files changed, 15 insertions(+)
>>>>>>
>>>>>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>>>>>> index 057859ab14..f626a6a510 100644
>>>>>> --- a/xen/arch/x86/cpu/common.c
>>>>>> +++ b/xen/arch/x86/cpu/common.c
>>>>>> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>>>>>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>>>>>>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>>>>>>  	c->compute_unit_id = INVALID_CUID;
>>>>>> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>>>>>>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>>>>>>  
>>>>>>  	generic_identify(c);
>>>>>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>>>>>> index 7e76cc3d68..2c6a40c543 100644
>>>>>> --- a/xen/arch/x86/smpboot.c
>>>>>> +++ b/xen/arch/x86/smpboot.c
>>>>>> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>>>>>>  
>>>>>> +    c[cpu].cpu_thread_id = 0;
>>>>>> +
>>>>>>      if ( c[cpu].x86_num_siblings > 1 )
>>>>>>      {
>>>>>>          for_each_cpu ( i, &cpu_sibling_setup_map )
>>>>>> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>>                         "CPU%u: unclear relationship with CPU%u\n",
>>>>>>                         cpu, i);
>>>>>>          }
>>>>>> +
>>>>>> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
>>>>>> +        {
>>>>>> +            if ( cpu == i )
>>>>>> +                break;
>>>>>> +            c[cpu].cpu_thread_id++;
>>>>>
>>>>> No. This wants doing alongside setting of the other similar fields,
>>>>> i.e. in (at least) detect_extended_topology() and detect_ht(). It
>>>>> is the hardware which tells us what the thread ID is.
>>>>
>>>> I wasn't able to find the related information for AMD cpus. In case it
>>>> is in the specs it is hidden very well.
>>>
>>> What was used for compute units in Fam16 and earlier is used for
>>> hyperthreads in Fam17 (and going forward, from what iirc Brian
>>> has said).
>>
>> And what about hyperthreads in Fam16 and earlier? Or are "compute units"
>> just another name for the threads and I can rename that field?
> 
> When there are compute units, there are no threads. But the difference
> is more than just the names, so folding the fields is not a good idea imo.

So another flag and display the column title accordingly?

I'd put thread and compute unit in a union then.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] add cpu_thread_id to struct cpuinfo_x86

[Jan Beulich]

>>> On 03.09.18 at 17:11, <jgross@suse.com> wrote:
> On 03/09/18 17:07, Jan Beulich wrote:
>>>>> On 03.09.18 at 16:52, <jgross@suse.com> wrote:
>>> On 03/09/18 16:47, Jan Beulich wrote:
>>>>>>> On 03.09.18 at 15:53, <jgross@suse.com> wrote:
>>>>> On 03/09/18 15:46, Jan Beulich wrote:
>>>>>>>>> On 31.08.18 at 18:22, <jgross@suse.com> wrote:
>>>>>>> Add the thread-id to the cpu config data and an accessor macro
>>>>>>> cpu_to_thread().
>>>>>>>
>>>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>>>> ---
>>>>>>>  xen/arch/x86/cpu/common.c       |  1 +
>>>>>>>  xen/arch/x86/smpboot.c          | 10 ++++++++++
>>>>>>>  xen/include/asm-arm/processor.h |  1 +
>>>>>>>  xen/include/asm-x86/processor.h |  2 ++
>>>>>>>  xen/include/public/sysctl.h     |  1 +
>>>>>>>  5 files changed, 15 insertions(+)
>>>>>>>
>>>>>>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>>>>>>> index 057859ab14..f626a6a510 100644
>>>>>>> --- a/xen/arch/x86/cpu/common.c
>>>>>>> +++ b/xen/arch/x86/cpu/common.c
>>>>>>> @@ -434,6 +434,7 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>>>>>>  	c->phys_proc_id = XEN_INVALID_SOCKET_ID;
>>>>>>>  	c->cpu_core_id = XEN_INVALID_CORE_ID;
>>>>>>>  	c->compute_unit_id = INVALID_CUID;
>>>>>>> +	c->cpu_thread_id = XEN_INVALID_THREAD_ID;
>>>>>>>  	memset(&c->x86_capability, 0, sizeof c->x86_capability);
>>>>>>>  
>>>>>>>  	generic_identify(c);
>>>>>>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>>>>>>> index 7e76cc3d68..2c6a40c543 100644
>>>>>>> --- a/xen/arch/x86/smpboot.c
>>>>>>> +++ b/xen/arch/x86/smpboot.c
>>>>>>> @@ -247,6 +247,8 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>>>>>>>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>>>>>>>  
>>>>>>> +    c[cpu].cpu_thread_id = 0;
>>>>>>> +
>>>>>>>      if ( c[cpu].x86_num_siblings > 1 )
>>>>>>>      {
>>>>>>>          for_each_cpu ( i, &cpu_sibling_setup_map )
>>>>>>> @@ -270,6 +272,14 @@ static void set_cpu_sibling_map(unsigned int cpu)
>>>>>>>                         "CPU%u: unclear relationship with CPU%u\n",
>>>>>>>                         cpu, i);
>>>>>>>          }
>>>>>>> +
>>>>>>> +        for_each_cpu ( i, per_cpu(cpu_sibling_mask, cpu) )
>>>>>>> +        {
>>>>>>> +            if ( cpu == i )
>>>>>>> +                break;
>>>>>>> +            c[cpu].cpu_thread_id++;
>>>>>>
>>>>>> No. This wants doing alongside setting of the other similar fields,
>>>>>> i.e. in (at least) detect_extended_topology() and detect_ht(). It
>>>>>> is the hardware which tells us what the thread ID is.
>>>>>
>>>>> I wasn't able to find the related information for AMD cpus. In case it
>>>>> is in the specs it is hidden very well.
>>>>
>>>> What was used for compute units in Fam16 and earlier is used for
>>>> hyperthreads in Fam17 (and going forward, from what iirc Brian
>>>> has said).
>>>
>>> And what about hyperthreads in Fam16 and earlier? Or are "compute units"
>>> just another name for the threads and I can rename that field?
>> 
>> When there are compute units, there are no threads. But the difference
>> is more than just the names, so folding the fields is not a good idea imo.
> 
> So another flag and display the column title accordingly?
> 
> I'd put thread and compute unit in a union then.

Both fine with me.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel