xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH for-4.22 0/2] Support for Intel temperature sensors (DTS)
@ 2025-10-27 17:26 Teddy Astie
  2025-10-27 17:26 ` [RFC PATCH for-4.22 2/2] tools: Introduce xen-inteltemp tool Teddy Astie
  2025-10-27 17:26 ` [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR Teddy Astie
  0 siblings, 2 replies; 9+ messages in thread
From: Teddy Astie @ 2025-10-27 17:26 UTC (permalink / raw)
  To: xen-devel
  Cc: Teddy Astie, Jan Beulich, Andrew Cooper, Roger Pau Monné,
	Anthony PERARD

The idea here is to expose the DTS sensors through XENPF_resource_op
and introduce a new dedicated tool to expose it for the user.

Teddy Astie (2):
  x86/platform: Expose DTS sensors MSR
  tools: Introduce xen-inteltemp tool

 tools/misc/.gitignore                |  1 +
 tools/misc/Makefile                  |  4 ++
 tools/misc/xen-inteltemp.c           | 98 ++++++++++++++++++++++++++++
 xen/arch/x86/include/asm/msr-index.h |  2 +
 xen/arch/x86/platform_hypercall.c    |  6 ++
 5 files changed, 111 insertions(+)
 create mode 100644 tools/misc/xen-inteltemp.c

-- 
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH for-4.22 2/2] tools: Introduce xen-inteltemp tool
  2025-10-27 17:26 [RFC PATCH for-4.22 0/2] Support for Intel temperature sensors (DTS) Teddy Astie
@ 2025-10-27 17:26 ` Teddy Astie
  2025-10-28  9:24   ` Jan Beulich
  2025-10-27 17:26 ` [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR Teddy Astie
  1 sibling, 1 reply; 9+ messages in thread
From: Teddy Astie @ 2025-10-27 17:26 UTC (permalink / raw)
  To: xen-devel; +Cc: Teddy Astie, Anthony PERARD

Introduce a new tool to fetch Intel CPU temperatures through the
Intel DTS interface using XENPF_resource_op hypercall.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
 tools/misc/.gitignore      |  1 +
 tools/misc/Makefile        |  4 ++
 tools/misc/xen-inteltemp.c | 98 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 tools/misc/xen-inteltemp.c

diff --git a/tools/misc/.gitignore b/tools/misc/.gitignore
index 28af46280f..65048eb901 100644
--- a/tools/misc/.gitignore
+++ b/tools/misc/.gitignore
@@ -5,6 +5,7 @@ xen-diag
 xen-hptool
 xen-hvmcrash
 xen-hvmctx
+xen-inteltemp
 xen-livepatch
 xen-lowmemd
 xen-mceinj
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index c26e544e83..6498b47ec0 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -25,6 +25,7 @@ INSTALL_SBIN-$(CONFIG_X86)     += xen-memshare
 INSTALL_SBIN-$(CONFIG_X86)     += xen-mfndump
 INSTALL_SBIN-$(CONFIG_X86)     += xen-ucode
 INSTALL_SBIN-$(CONFIG_X86)     += xen-vmtrace
+INSTALL_SBIN-$(CONFIG_X86)     += xen-inteltemp
 INSTALL_SBIN                   += xencov
 INSTALL_SBIN                   += xenhypfs
 INSTALL_SBIN                   += xenlockprof
@@ -89,6 +90,9 @@ xen-memshare: xen-memshare.o
 xen-vmtrace: xen-vmtrace.o
 	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(LDLIBS_libxenforeignmemory) $(APPEND_LDFLAGS)

+xen-inteltemp: xen-inteltemp.o
+	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+
 xen-mceinj: xen-mceinj.o
 	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)

diff --git a/tools/misc/xen-inteltemp.c b/tools/misc/xen-inteltemp.c
new file mode 100644
index 0000000000..624c74ca7f
--- /dev/null
+++ b/tools/misc/xen-inteltemp.c
@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * xen-inteltemp.c
+ *
+ * Get the CPU temperature of Intel processors.
+ *
+ * Copyright 2025 Teddy Astie <teddy.astie@vates.tech>
+ */
+
+#include <stdio.h>
+#include <errno.h>
+#include <xenctrl.h>
+
+#define MSR_IA32_THERM_STATUS         0x0000019c
+#define MSR_IA32_TEMPERATURE_TARGET   0x000001a2
+#define MSR_IA32_PACKAGE_THERM_STATUS 0x000001b1
+
+int fetch_dts_temp(xc_interface *xch, uint32_t cpu, bool package, int *temp)
+{
+    xc_resource_entry_t entries[2] = {
+        (xc_resource_entry_t){
+            .idx = package ? MSR_IA32_PACKAGE_THERM_STATUS : MSR_IA32_THERM_STATUS
+        },
+        (xc_resource_entry_t){ .idx = MSR_IA32_TEMPERATURE_TARGET },
+    };
+    struct xc_resource_op ops = {
+        .cpu = cpu,
+        .entries = entries,
+        .nr_entries = 2,
+    };
+    int tjmax;
+
+    int ret = xc_resource_op(xch, 1, &ops);
+
+    if ( ret <= 0 )
+        /* This CPU isn't online or can't query this MSR */
+        return ret ?: -EOPNOTSUPP;
+
+    if ( ret == 2 )
+        tjmax = (entries[1].val >> 16) & 0xff;
+    else
+    {
+        /*
+         * The CPU doesn't support MSR_IA32_TEMPERATURE_TARGET, we assume it's 100 which
+         * is correct aside a few selected Atom CPUs. Check coretemp source code for more
+         * information.
+         */
+        fprintf(stderr, "CPU%d doesn't support MSR_IA32_TEMPERATURE_TARGET, assume "
+               "tjmax=100°C, readings may be incorrect\n", cpu);
+        tjmax = 100;
+    }
+
+    *temp = tjmax - ((entries[0].val >> 16) & 0xff);
+    return 0;
+}
+
+int main(void)
+{
+    int rc = 0, temp, cpu, socket;
+    bool has_data = false;
+    xc_interface *xch = xc_interface_open(0, 0, 0);
+    xc_physinfo_t info;
+
+    if ( (rc = xc_physinfo(xch, &info)) < 0 )
+    {
+        perror("Getting physinfo failed");
+        return rc;
+    }
+
+    /* Per socket measurement */
+    for ( socket = 0, cpu = 0;
+          cpu < (info.max_cpu_id + 1);
+          socket++, cpu += info.cores_per_socket * info.threads_per_core )
+    {
+        if ( !fetch_dts_temp(xch, cpu, true, &temp) )
+        {
+            has_data = true;
+            printf("Package%d: %d°C\n", socket, temp);
+        }
+    }
+
+    printf("\n");
+
+    for ( cpu = 0; cpu < (info.max_cpu_id + 1); cpu += info.threads_per_core )
+    {
+        if ( fetch_dts_temp(xch, cpu, false, &temp) )
+            continue;
+
+        has_data = true;
+        printf("CPU%d: %d°C\n", cpu, temp);
+    }
+
+    if ( !has_data )
+        printf("No data\n");
+
+    xc_interface_close(xch);
+    return 0;
+}
\ No newline at end of file
--
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech




^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR
  2025-10-27 17:26 [RFC PATCH for-4.22 0/2] Support for Intel temperature sensors (DTS) Teddy Astie
  2025-10-27 17:26 ` [RFC PATCH for-4.22 2/2] tools: Introduce xen-inteltemp tool Teddy Astie
@ 2025-10-27 17:26 ` Teddy Astie
  2025-10-27 19:38   ` Andrew Cooper
  2025-10-28  9:22   ` Jan Beulich
  1 sibling, 2 replies; 9+ messages in thread
From: Teddy Astie @ 2025-10-27 17:26 UTC (permalink / raw)
  To: xen-devel; +Cc: Teddy Astie, Jan Beulich, Andrew Cooper, Roger Pau Monné

Intel provide CPU sensors through "DTS" MSRs. As there MSR are core-specific
(or package-specific), we can't reliably fetch them from Dom0 directly.
Expose these MSR (if supported) through XENPF_resource_op so that it is
accessible through hypercall.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
I'm not a fan of doing a inline cpuid check here, but I don't have a
better approach in mind.

 xen/arch/x86/include/asm/msr-index.h | 2 ++
 xen/arch/x86/platform_hypercall.c    | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index df52587c85..98dda629e5 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -510,6 +510,8 @@
 #define MSR_IA32_THERM_INTERRUPT	0x0000019b
 #define MSR_IA32_THERM_STATUS		0x0000019c
 #define MSR_IA32_MISC_ENABLE		0x000001a0
+#define MSR_IA32_TEMPERATURE_TARGET   0x000001a2
+#define MSR_IA32_PACKAGE_THERM_STATUS 0x000001b1
 #define MSR_IA32_MISC_ENABLE_FAST_STRING  (1<<0)
 #define MSR_IA32_MISC_ENABLE_PERF_AVAIL   (1<<7)
 #define MSR_IA32_MISC_ENABLE_BTS_UNAVAIL  (1<<11)
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 79bb99e0b6..3190803cc2 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -86,6 +86,12 @@ static bool msr_read_allowed(unsigned int msr)
 
     case MSR_MCU_OPT_CTRL:
         return cpu_has_srbds_ctrl;
+    
+    case MSR_IA32_TEMPERATURE_TARGET:
+    case MSR_IA32_THERM_STATUS:
+    case MSR_IA32_PACKAGE_THERM_STATUS:
+        return boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+            (cpuid_eax(0x6) & 0x1); /* Digital temperature sensor */
     }
 
     if ( ppin_msr && msr == ppin_msr )
-- 
2.51.1



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR
  2025-10-27 17:26 ` [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR Teddy Astie
@ 2025-10-27 19:38   ` Andrew Cooper
  2025-10-28  9:20     ` Jan Beulich
  2025-10-28  9:22   ` Jan Beulich
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2025-10-27 19:38 UTC (permalink / raw)
  To: Teddy Astie, xen-devel; +Cc: Jan Beulich, Roger Pau Monné

On 27/10/2025 5:26 pm, Teddy Astie wrote:
> I'm not a fan of doing a inline cpuid check here, but I don't have a
> better approach in mind.

I'm not sure if there's enough information in leaf 6 to justify putting
it fully into the CPUID infrastructure.

But, if you do something like this:

diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index f94f23e159d2..d02fe4d22151 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -121,7 +121,13 @@ struct cpu_policy
             uint64_t :64, :64; /* Leaf 0x3 - PSN. */
             uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
             uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
-            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
+
+            /* Leaf 0x6 - Thermal and Perf. */
+            struct {
+                bool /* a */ dts:1;
+                uint32_t /* b */:32, /* c */:32, /* d */:32;
+            };
+
             uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
             uint64_t :64, :64; /* Leaf 0x8 - rsvd */
             uint64_t :64, :64; /* Leaf 0x9 - DCA */


then ...

> diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
> index 79bb99e0b6..3190803cc2 100644
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -86,6 +86,12 @@ static bool msr_read_allowed(unsigned int msr)
>  
>      case MSR_MCU_OPT_CTRL:
>          return cpu_has_srbds_ctrl;
> +    

You've added trailing whitespace here.

> +    case MSR_IA32_TEMPERATURE_TARGET:
> +    case MSR_IA32_THERM_STATUS:
> +    case MSR_IA32_PACKAGE_THERM_STATUS:
> +        return boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> +            (cpuid_eax(0x6) & 0x1); /* Digital temperature sensor */

... you ought to be able to use host_policy.basic.dts here.  In
principle the Intel check can be dropped too.

~Andrew


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR
  2025-10-27 19:38   ` Andrew Cooper
@ 2025-10-28  9:20     ` Jan Beulich
  2025-10-29 16:06       ` Andrew Cooper
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2025-10-28  9:20 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, xen-devel, Teddy Astie

On 27.10.2025 20:38, Andrew Cooper wrote:
> On 27/10/2025 5:26 pm, Teddy Astie wrote:
>> I'm not a fan of doing a inline cpuid check here, but I don't have a
>> better approach in mind.
> 
> I'm not sure if there's enough information in leaf 6 to justify putting
> it fully into the CPUID infrastructure.
> 
> But, if you do something like this:
> 
> diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
> index f94f23e159d2..d02fe4d22151 100644
> --- a/xen/include/xen/lib/x86/cpu-policy.h
> +++ b/xen/include/xen/lib/x86/cpu-policy.h
> @@ -121,7 +121,13 @@ struct cpu_policy
>              uint64_t :64, :64; /* Leaf 0x3 - PSN. */
>              uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
>              uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
> -            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
> +
> +            /* Leaf 0x6 - Thermal and Perf. */
> +            struct {
> +                bool /* a */ dts:1;
> +                uint32_t /* b */:32, /* c */:32, /* d */:32;
> +            };
> +
>              uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
>              uint64_t :64, :64; /* Leaf 0x8 - rsvd */
>              uint64_t :64, :64; /* Leaf 0x9 - DCA */

Just to mention, below a patch I have pending as part of a series to
e.g. replace the various CPUID6_* values we presently use. As you did
indicate when we talked about this, a prereq to then use respective
bits from host_policy is an adjustment to cpu-policy.c, which is also
part of that series. If we weren't in freeze right now, I would have
posted the series already.

Jan

x86/cpu-policy: define bits of leaf 6

... as far as we presently use them in the codebase.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Or should we make both parts proper featureset elements? At least
APERFMPERF could likely be made visible to guests (in principle).

--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -128,7 +128,31 @@ struct cpu_policy
             uint64_t :64, :64; /* Leaf 0x3 - PSN. */
             uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
             uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
-            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
+
+            /* Leaf 0x6 - Therm/Perf. */
+            struct {
+                uint32_t /* a */:1,
+                    turbo:1,
+                    arat:1,
+                    :4,
+                    hwp:1,
+                    hwp_notification:1,
+                    hwp_activity_window:1,
+                    hwp_epp:1,
+                    hwp_plr:1,
+                    :1,
+                    hdc:1,
+                    :2,
+                    hwp_peci:1,
+                    :2,
+                    hw_feedback:1,
+                    :12;
+                uint32_t /* b */:32;
+                uint32_t /* c */ aperfmperf:1,
+                    :31;
+                uint32_t /* d */:32;
+            } pm;
+
             uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
             uint64_t :64, :64; /* Leaf 0x8 - rsvd */
             uint64_t :64, :64; /* Leaf 0x9 - DCA */



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR
  2025-10-27 17:26 ` [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR Teddy Astie
  2025-10-27 19:38   ` Andrew Cooper
@ 2025-10-28  9:22   ` Jan Beulich
  1 sibling, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2025-10-28  9:22 UTC (permalink / raw)
  To: Teddy Astie; +Cc: Andrew Cooper, Roger Pau Monné, xen-devel

On 27.10.2025 18:26, Teddy Astie wrote:
> Intel provide CPU sensors through "DTS" MSRs. As there MSR are core-specific
> (or package-specific), we can't reliably fetch them from Dom0 directly.
> Expose these MSR (if supported) through XENPF_resource_op so that it is
> accessible through hypercall.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> I'm not a fan of doing a inline cpuid check here, but I don't have a
> better approach in mind.
> 
>  xen/arch/x86/include/asm/msr-index.h | 2 ++
>  xen/arch/x86/platform_hypercall.c    | 6 ++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
> index df52587c85..98dda629e5 100644
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -510,6 +510,8 @@
>  #define MSR_IA32_THERM_INTERRUPT	0x0000019b
>  #define MSR_IA32_THERM_STATUS		0x0000019c
>  #define MSR_IA32_MISC_ENABLE		0x000001a0
> +#define MSR_IA32_TEMPERATURE_TARGET   0x000001a2
> +#define MSR_IA32_PACKAGE_THERM_STATUS 0x000001b1
>  #define MSR_IA32_MISC_ENABLE_FAST_STRING  (1<<0)
>  #define MSR_IA32_MISC_ENABLE_PERF_AVAIL   (1<<7)
>  #define MSR_IA32_MISC_ENABLE_BTS_UNAVAIL  (1<<11)

Now new additions like this to this file please (and even less so ones
disagreeing in padding with adjacent lines). Please go find this comment
in the file:

/*
 * Legacy MSR constants in need of cleanup.  No new MSRs below this comment.
 */

Jan


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH for-4.22 2/2] tools: Introduce xen-inteltemp tool
  2025-10-27 17:26 ` [RFC PATCH for-4.22 2/2] tools: Introduce xen-inteltemp tool Teddy Astie
@ 2025-10-28  9:24   ` Jan Beulich
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2025-10-28  9:24 UTC (permalink / raw)
  To: Teddy Astie; +Cc: Anthony PERARD, xen-devel

On 27.10.2025 18:26, Teddy Astie wrote:
> Introduce a new tool to fetch Intel CPU temperatures through the
> Intel DTS interface using XENPF_resource_op hypercall.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
>  tools/misc/.gitignore      |  1 +
>  tools/misc/Makefile        |  4 ++
>  tools/misc/xen-inteltemp.c | 98 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 103 insertions(+)
>  create mode 100644 tools/misc/xen-inteltemp.c

Instead of introducing a new tool, might this not fit in xenpm?

> --- /dev/null
> +++ b/tools/misc/xen-inteltemp.c
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * xen-inteltemp.c
> + *
> + * Get the CPU temperature of Intel processors.
> + *
> + * Copyright 2025 Teddy Astie <teddy.astie@vates.tech>
> + */
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include <xenctrl.h>
> +
> +#define MSR_IA32_THERM_STATUS         0x0000019c
> +#define MSR_IA32_TEMPERATURE_TARGET   0x000001a2
> +#define MSR_IA32_PACKAGE_THERM_STATUS 0x000001b1
> +
> +int fetch_dts_temp(xc_interface *xch, uint32_t cpu, bool package, int *temp)
> +{
> +    xc_resource_entry_t entries[2] = {
> +        (xc_resource_entry_t){
> +            .idx = package ? MSR_IA32_PACKAGE_THERM_STATUS : MSR_IA32_THERM_STATUS
> +        },
> +        (xc_resource_entry_t){ .idx = MSR_IA32_TEMPERATURE_TARGET },
> +    };
> +    struct xc_resource_op ops = {
> +        .cpu = cpu,
> +        .entries = entries,
> +        .nr_entries = 2,
> +    };
> +    int tjmax;
> +
> +    int ret = xc_resource_op(xch, 1, &ops);
> +
> +    if ( ret <= 0 )
> +        /* This CPU isn't online or can't query this MSR */
> +        return ret ?: -EOPNOTSUPP;
> +
> +    if ( ret == 2 )
> +        tjmax = (entries[1].val >> 16) & 0xff;
> +    else
> +    {
> +        /*
> +         * The CPU doesn't support MSR_IA32_TEMPERATURE_TARGET, we assume it's 100 which
> +         * is correct aside a few selected Atom CPUs. Check coretemp source code for more
> +         * information.
> +         */
> +        fprintf(stderr, "CPU%d doesn't support MSR_IA32_TEMPERATURE_TARGET, assume "
> +               "tjmax=100°C, readings may be incorrect\n", cpu);
> +        tjmax = 100;
> +    }
> +    
> +    *temp = tjmax - ((entries[0].val >> 16) & 0xff);
> +    return 0;
> +}
> +
> +int main(void)
> +{
> +    int rc = 0, temp, cpu, socket;
> +    bool has_data = false;
> +    xc_interface *xch = xc_interface_open(0, 0, 0);
> +    xc_physinfo_t info;
> +
> +    if ( (rc = xc_physinfo(xch, &info)) < 0 )
> +    {
> +        perror("Getting physinfo failed");
> +        return rc;
> +    }
> +
> +    /* Per socket measurement */
> +    for ( socket = 0, cpu = 0;
> +          cpu < (info.max_cpu_id + 1);
> +          socket++, cpu += info.cores_per_socket * info.threads_per_core )
> +    {
> +        if ( !fetch_dts_temp(xch, cpu, true, &temp) )
> +        {
> +            has_data = true;
> +            printf("Package%d: %d°C\n", socket, temp);
> +        }
> +    }
> +
> +    printf("\n");
> +
> +    for ( cpu = 0; cpu < (info.max_cpu_id + 1); cpu += info.threads_per_core )
> +    {
> +        if ( fetch_dts_temp(xch, cpu, false, &temp) )
> +            continue;
> +
> +        has_data = true;
> +        printf("CPU%d: %d°C\n", cpu, temp);
> +    }
> +
> +    if ( !has_data )
> +        printf("No data\n");
> +
> +    xc_interface_close(xch);
> +    return 0;
> +}
> \ No newline at end of file

Please never introduce files without trailing newline.

Jan


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR
  2025-10-28  9:20     ` Jan Beulich
@ 2025-10-29 16:06       ` Andrew Cooper
  2025-10-30  7:07         ` Jan Beulich
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2025-10-29 16:06 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Roger Pau Monné, xen-devel, Teddy Astie

On 28/10/2025 9:20 am, Jan Beulich wrote:
> On 27.10.2025 20:38, Andrew Cooper wrote:
>> On 27/10/2025 5:26 pm, Teddy Astie wrote:
>>> I'm not a fan of doing a inline cpuid check here, but I don't have a
>>> better approach in mind.
>> I'm not sure if there's enough information in leaf 6 to justify putting
>> it fully into the CPUID infrastructure.
>>
>> But, if you do something like this:
>>
>> diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
>> index f94f23e159d2..d02fe4d22151 100644
>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>> @@ -121,7 +121,13 @@ struct cpu_policy
>>              uint64_t :64, :64; /* Leaf 0x3 - PSN. */
>>              uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
>>              uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
>> -            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
>> +
>> +            /* Leaf 0x6 - Thermal and Perf. */
>> +            struct {
>> +                bool /* a */ dts:1;
>> +                uint32_t /* b */:32, /* c */:32, /* d */:32;
>> +            };
>> +
>>              uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
>>              uint64_t :64, :64; /* Leaf 0x8 - rsvd */
>>              uint64_t :64, :64; /* Leaf 0x9 - DCA */
> Just to mention, below a patch I have pending as part of a series to
> e.g. replace the various CPUID6_* values we presently use. As you did
> indicate when we talked about this, a prereq to then use respective
> bits from host_policy is an adjustment to cpu-policy.c, which is also
> part of that series. If we weren't in freeze right now, I would have
> posted the series already.
>
> Jan
>
> x86/cpu-policy: define bits of leaf 6
>
> ... as far as we presently use them in the codebase.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Or should we make both parts proper featureset elements? At least
> APERFMPERF could likely be made visible to guests (in principle).
>
> --- a/xen/include/xen/lib/x86/cpu-policy.h
> +++ b/xen/include/xen/lib/x86/cpu-policy.h
> @@ -128,7 +128,31 @@ struct cpu_policy
>              uint64_t :64, :64; /* Leaf 0x3 - PSN. */
>              uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
>              uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
> -            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
> +
> +            /* Leaf 0x6 - Therm/Perf. */
> +            struct {
> +                uint32_t /* a */:1,
> +                    turbo:1,
> +                    arat:1,
> +                    :4,
> +                    hwp:1,
> +                    hwp_notification:1,
> +                    hwp_activity_window:1,
> +                    hwp_epp:1,
> +                    hwp_plr:1,
> +                    :1,
> +                    hdc:1,
> +                    :2,
> +                    hwp_peci:1,
> +                    :2,
> +                    hw_feedback:1,
> +                    :12;
> +                uint32_t /* b */:32;
> +                uint32_t /* c */ aperfmperf:1,
> +                    :31;
> +                uint32_t /* d */:32;
> +            } pm;

This works too, although we don't have 'pm' equivalents elsewhere in
this part of the union.

APERF/MPERF is a disaster of an interface.  It can't safely be read even
in root mode, because an NMI/SMI breaks the algorithm in a way that
isn't easy to spot and retry.  On AMD, it's marginally better because
GIF can be used to exclude NMIs and non-fatal MCEs while sampling the
register pair.

In a VM, it's simply unusable.  Any VMExit, and even a vCPU reschedule,
breaks reading the pair.

Until the CPU vendors produce a way of reading the two counters together
(i.e. atomically, which has been asked for, repeatedly), there's no
point considering it for use in a VM.

~Andrew


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR
  2025-10-29 16:06       ` Andrew Cooper
@ 2025-10-30  7:07         ` Jan Beulich
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2025-10-30  7:07 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, xen-devel, Teddy Astie

On 29.10.2025 17:06, Andrew Cooper wrote:
> On 28/10/2025 9:20 am, Jan Beulich wrote:
>> On 27.10.2025 20:38, Andrew Cooper wrote:
>>> On 27/10/2025 5:26 pm, Teddy Astie wrote:
>>>> I'm not a fan of doing a inline cpuid check here, but I don't have a
>>>> better approach in mind.
>>> I'm not sure if there's enough information in leaf 6 to justify putting
>>> it fully into the CPUID infrastructure.
>>>
>>> But, if you do something like this:
>>>
>>> diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
>>> index f94f23e159d2..d02fe4d22151 100644
>>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>>> @@ -121,7 +121,13 @@ struct cpu_policy
>>>              uint64_t :64, :64; /* Leaf 0x3 - PSN. */
>>>              uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
>>>              uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
>>> -            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
>>> +
>>> +            /* Leaf 0x6 - Thermal and Perf. */
>>> +            struct {
>>> +                bool /* a */ dts:1;
>>> +                uint32_t /* b */:32, /* c */:32, /* d */:32;
>>> +            };
>>> +
>>>              uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
>>>              uint64_t :64, :64; /* Leaf 0x8 - rsvd */
>>>              uint64_t :64, :64; /* Leaf 0x9 - DCA */
>> Just to mention, below a patch I have pending as part of a series to
>> e.g. replace the various CPUID6_* values we presently use. As you did
>> indicate when we talked about this, a prereq to then use respective
>> bits from host_policy is an adjustment to cpu-policy.c, which is also
>> part of that series. If we weren't in freeze right now, I would have
>> posted the series already.
>>
>> Jan
>>
>> x86/cpu-policy: define bits of leaf 6
>>
>> ... as far as we presently use them in the codebase.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Or should we make both parts proper featureset elements? At least
>> APERFMPERF could likely be made visible to guests (in principle).
>>
>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>> @@ -128,7 +128,31 @@ struct cpu_policy
>>              uint64_t :64, :64; /* Leaf 0x3 - PSN. */
>>              uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
>>              uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
>> -            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
>> +
>> +            /* Leaf 0x6 - Therm/Perf. */
>> +            struct {
>> +                uint32_t /* a */:1,
>> +                    turbo:1,
>> +                    arat:1,
>> +                    :4,
>> +                    hwp:1,
>> +                    hwp_notification:1,
>> +                    hwp_activity_window:1,
>> +                    hwp_epp:1,
>> +                    hwp_plr:1,
>> +                    :1,
>> +                    hdc:1,
>> +                    :2,
>> +                    hwp_peci:1,
>> +                    :2,
>> +                    hw_feedback:1,
>> +                    :12;
>> +                uint32_t /* b */:32;
>> +                uint32_t /* c */ aperfmperf:1,
>> +                    :31;
>> +                uint32_t /* d */:32;
>> +            } pm;
> 
> This works too, although we don't have 'pm' equivalents elsewhere in
> this part of the union.
> 
> APERF/MPERF is a disaster of an interface.  It can't safely be read even
> in root mode, because an NMI/SMI breaks the algorithm in a way that
> isn't easy to spot and retry.  On AMD, it's marginally better because
> GIF can be used to exclude NMIs and non-fatal MCEs while sampling the
> register pair.
> 
> In a VM, it's simply unusable.  Any VMExit, and even a vCPU reschedule,
> breaks reading the pair.
> 
> Until the CPU vendors produce a way of reading the two counters together
> (i.e. atomically, which has been asked for, repeatedly), there's no
> point considering it for use in a VM.

Right now none of this is meant for exposure. It's solely for us to use
the bits in the host policy.

Jan


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-10-30  7:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-27 17:26 [RFC PATCH for-4.22 0/2] Support for Intel temperature sensors (DTS) Teddy Astie
2025-10-27 17:26 ` [RFC PATCH for-4.22 2/2] tools: Introduce xen-inteltemp tool Teddy Astie
2025-10-28  9:24   ` Jan Beulich
2025-10-27 17:26 ` [RFC PATCH for-4.22 1/2] x86/platform: Expose DTS sensors MSR Teddy Astie
2025-10-27 19:38   ` Andrew Cooper
2025-10-28  9:20     ` Jan Beulich
2025-10-29 16:06       ` Andrew Cooper
2025-10-30  7:07         ` Jan Beulich
2025-10-28  9:22   ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).