[RFC PATCH for-4.22 v2 3/3] xenpm: Add get-intel-temp subcommand

[RFC PATCH for-4.22 v2 3/3] xenpm: Add get-intel-temp subcommand

[Teddy Astie]

get-intel-temp allows querying the per-core CPU temperature and
per-package one on Intel processors (as usual Dom0 drivers cannot
work due to misalignment between Dom0 vCPU and pCPUs).

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
v2: moved from a separate command to xenpm

 tools/misc/xenpm.c | 93 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 1 deletion(-)

diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index 682d092479..ef9abee48e 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -37,6 +37,7 @@

 static xc_interface *xc_handle;
 static unsigned int max_cpu_nr;
+static xc_physinfo_t physinfo;

 /* help message */
 void show_help(void)
@@ -93,6 +94,7 @@ void show_help(void)
             "                                           units default to \"us\" if unspecified.\n"
             "                                           truncates un-representable values.\n"
             "                                           0 lets the hardware decide.\n"
+            " get-intel-temp        [cpuid]       get Intel CPU temperature of <cpuid> or all\n"
             " start [seconds]                     start collect Cx/Px statistics,\n"
             "                                     output after CTRL-C or SIGINT or several seconds.\n"
             " enable-turbo-mode     [cpuid]       enable Turbo Mode for processors that support it.\n"
@@ -1354,6 +1356,95 @@ void enable_turbo_mode(int argc, char *argv[])
                 errno, strerror(errno));
 }

+#define MSR_DTS_THERM_STATUS         0x0000019c
+#define MSR_DTS_TEMPERATURE_TARGET   0x000001a2
+#define MSR_DTS_PACKAGE_THERM_STATUS 0x000001b1
+
+static 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_DTS_PACKAGE_THERM_STATUS : MSR_DTS_THERM_STATUS
+        },
+        (xc_resource_entry_t){ .idx = MSR_DTS_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] MSR_IA32_TEMPERATURE_TARGET is not supported, assume "
+                "tjmax=100°C, readings may be incorrect\n", cpu);
+        tjmax = 100;
+    }
+
+    *temp = tjmax - ((entries[0].val >> 16) & 0xff);
+    return 0;
+}
+
+
+void get_intel_temp(int argc, char *argv[])
+{
+    int temp, cpu = -1, socket;
+    bool has_data = false;
+
+    if (argc > 0)
+        parse_cpuid(argv[0], &cpu);
+
+    if (cpu != -1)
+    {
+        if ( !fetch_dts_temp(xc_handle, cpu, false, &temp) )
+            printf("CPU%d: %d°C\n", cpu, temp);
+        else
+            printf("No data\n");
+        return;
+    }
+
+    /* Per socket measurement */
+    for ( socket = 0, cpu = 0; cpu < max_cpu_nr;
+          socket++, cpu += physinfo.cores_per_socket * physinfo.threads_per_core )
+    {
+        if ( !fetch_dts_temp(xc_handle, cpu, true, &temp) )
+        {
+            has_data = true;
+            printf("Package%d: %d°C\n", socket, temp);
+        }
+    }
+
+    if ( has_data )
+        /* Avoid inserting a trailing line if we have nothing */
+        printf("\n");
+
+    for ( cpu = 0; cpu < max_cpu_nr; cpu += physinfo.threads_per_core )
+    {
+        if ( fetch_dts_temp(xc_handle, cpu, false, &temp) )
+            continue;
+
+        has_data = true;
+        printf("CPU%d: %d°C\n", cpu, temp);
+    }
+
+    if ( !has_data )
+        printf("No data\n");
+}
+
 void disable_turbo_mode(int argc, char *argv[])
 {
     int cpuid = -1;
@@ -1618,12 +1709,12 @@ struct {
     { "set-max-cstate", set_max_cstate_func},
     { "enable-turbo-mode", enable_turbo_mode },
     { "disable-turbo-mode", disable_turbo_mode },
+    { "get-intel-temp", get_intel_temp },
 };

 int main(int argc, char *argv[])
 {
     int i, ret = 0;
-    xc_physinfo_t physinfo;
     int nr_matches = 0;
     int matches_main_options[ARRAY_SIZE(main_options)];

--
2.51.2



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[RFC PATCH for-4.22 v2 0/3] Support for Intel temperature sensors (DTS)

[Teddy Astie]

The idea here is to expose the DTS sensors through XENPF_resource_op
and expose it for the user through xenpm.

v2:
 - moved userland part to xenpm
 - use cpu policy infrastructure instead of inline cpuid

Teddy Astie (3):
  x86/cpu-policy: Infrastructure for CPUID leaf 0x6
  x86/platform: Expose DTS sensors MSR
  xenpm: Add get-intel-temp subcommand

 tools/misc/xenpm.c                   | 93 +++++++++++++++++++++++++++-
 xen/arch/x86/include/asm/msr-index.h |  3 +
 xen/arch/x86/platform_hypercall.c    |  6 ++
 xen/include/xen/lib/x86/cpu-policy.h | 27 +++++++-
 4 files changed, 127 insertions(+), 2 deletions(-)

-- 
2.51.2



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[RFC PATCH for-4.22 v2 1/3] x86/cpu-policy: Infrastructure for CPUID leaf 0x6

[Teddy Astie]

From: Jan Beulich <jbeulich@suse.com>

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
v2: introduced

 xen/include/xen/lib/x86/cpu-policy.h | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index f94f23e159..c721c986cc 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -121,7 +121,32 @@ 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 */
+                    dts: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 */
-- 
2.51.2



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[RFC PATCH for-4.22 v2 2/3] x86/platform: Expose DTS sensors MSR

[Teddy Astie]

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>
---
v2:
 - move DTS MSR out of legacy list, review MSR naming
 - use CPU policy instead of inline CPUID

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

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index df52587c85..7c6af7bd4d 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -115,6 +115,9 @@
 #define  MCU_OPT_CTRL_GDS_MIT_DIS           (_AC(1, ULL) <<  4)
 #define  MCU_OPT_CTRL_GDS_MIT_LOCK          (_AC(1, ULL) <<  5)
 
+#define MSR_DTS_TEMPERATURE_TARGET          0x000001a2
+#define MSR_DTS_PACKAGE_THERM_STATUS        0x000001b1
+
 #define MSR_FRED_RSP_SL0                    0x000001cc
 #define MSR_FRED_RSP_SL1                    0x000001cd
 #define MSR_FRED_RSP_SL2                    0x000001ce
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 79bb99e0b6..d9872ddd3e 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -27,6 +27,7 @@
 #include <asm/current.h>
 #include <public/platform.h>
 #include <acpi/cpufreq/processor_perf.h>
+#include <asm/cpu-policy.h>
 #include <asm/edd.h>
 #include <asm/microcode.h>
 #include <asm/mtrr.h>
@@ -86,6 +87,11 @@ static bool msr_read_allowed(unsigned int msr)
 
     case MSR_MCU_OPT_CTRL:
         return cpu_has_srbds_ctrl;
+    
+    case MSR_IA32_THERM_STATUS:
+    case MSR_DTS_TEMPERATURE_TARGET:
+    case MSR_DTS_PACKAGE_THERM_STATUS:
+        return raw_cpu_policy.basic.pm.dts;
     }
 
     if ( ppin_msr && msr == ppin_msr )
-- 
2.51.2



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RFC PATCH for-4.22 v2 1/3] x86/cpu-policy: Infrastructure for CPUID leaf 0x6

[Jan Beulich]

On 29.10.2025 16:59, Teddy Astie wrote:
> From: Jan Beulich <jbeulich@suse.com>
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> v2: introduced

This being a change of mine, I'm not happy for the title to have changed, and
for the (little bit of) description to have been dropped.

> --- a/xen/include/xen/lib/x86/cpu-policy.h
> +++ b/xen/include/xen/lib/x86/cpu-policy.h
> @@ -121,7 +121,32 @@ 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 */
> +                    dts: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 */

As I had said, this (really: the use of these bits in the host policy) actually
requires an adjustment to cpu-policy.c as well, which I'm carrying as a separate,
prereq change (re-produced below). May I suggest that your work go on top of mine
(which I'll post once we have branched 4.21 off)?

Jan

x86/cpu-policy: move invocation of recalculate_misc()

The function is about guest exposure of features / leaves. There's no need
for it to be applied on the host policy. In fact doing so gets in the way
of using the host policy in certain places.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -424,7 +424,6 @@ void __init calculate_host_policy(void)
     x86_cpu_featureset_to_policy(boot_cpu_data.x86_capability, p);
     recalculate_xstate(p);
     recalculate_tile(p);
-    recalculate_misc(p);
 
     /* When vPMU is disabled, drop it from the host policy. */
     if ( vpmu_mode == XENPMU_MODE_OFF )
@@ -705,6 +704,7 @@ static void __init calculate_pv_max_poli
     unsigned int i;
 
     *p = host_cpu_policy;
+    recalculate_misc(p);
 
     guest_common_max_leaves(p);
 
@@ -809,6 +809,7 @@ static void __init calculate_hvm_max_pol
     const uint32_t *mask;
 
     *p = host_cpu_policy;
+    recalculate_misc(p);
 
     guest_common_max_leaves(p);
 

Re: [RFC PATCH for-4.22 v2 1/3] x86/cpu-policy: Infrastructure for CPUID leaf 0x6

[Teddy Astie]

Le 30/10/2025 à 08:14, Jan Beulich a écrit :
> On 29.10.2025 16:59, Teddy Astie wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
>> ---
>> v2: introduced
>
> This being a change of mine, I'm not happy for the title to have changed, and
> for the (little bit of) description to have been dropped.
>
>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>> @@ -121,7 +121,32 @@ 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 */
>> +                    dts: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 */
>
> As I had said, this (really: the use of these bits in the host policy) actually
> requires an adjustment to cpu-policy.c as well, which I'm carrying as a separate,
> prereq change (re-produced below). May I suggest that your work go on top of mine
> (which I'll post once we have branched 4.21 off)?
>

I'm ok with it.

> Jan
>
> x86/cpu-policy: move invocation of recalculate_misc()
>
> The function is about guest exposure of features / leaves. There's no need
> for it to be applied on the host policy. In fact doing so gets in the way
> of using the host policy in certain places.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -424,7 +424,6 @@ void __init calculate_host_policy(void)
>       x86_cpu_featureset_to_policy(boot_cpu_data.x86_capability, p);
>       recalculate_xstate(p);
>       recalculate_tile(p);
> -    recalculate_misc(p);
>
>       /* When vPMU is disabled, drop it from the host policy. */
>       if ( vpmu_mode == XENPMU_MODE_OFF )
> @@ -705,6 +704,7 @@ static void __init calculate_pv_max_poli
>       unsigned int i;
>
>       *p = host_cpu_policy;
> +    recalculate_misc(p);
>
>       guest_common_max_leaves(p);
>
> @@ -809,6 +809,7 @@ static void __init calculate_hvm_max_pol
>       const uint32_t *mask;
>
>       *p = host_cpu_policy;
> +    recalculate_misc(p);
>
>       guest_common_max_leaves(p);
>
>
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RFC PATCH for-4.22 v2 2/3] x86/platform: Expose DTS sensors MSR

[Jan Beulich]

On 29.10.2025 16:59, Teddy Astie wrote:
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -115,6 +115,9 @@
>  #define  MCU_OPT_CTRL_GDS_MIT_DIS           (_AC(1, ULL) <<  4)
>  #define  MCU_OPT_CTRL_GDS_MIT_LOCK          (_AC(1, ULL) <<  5)
>  
> +#define MSR_DTS_TEMPERATURE_TARGET          0x000001a2
> +#define MSR_DTS_PACKAGE_THERM_STATUS        0x000001b1

Where are the DTS infixes coming from? The SDM doesn't have such. We try to
stay as close as reasonable to the SDM / PM names, with the exception that
typically we omit IA32 infixes.

I'd also like to note that unlike the two THERM_STATUS, MSR_TEMPERATURE_TARGET
(as per the absence if an IA32 infix in the SDM) isn't an architectural MSR,
and hence I'm not entirely convinced we can "blindly" expose it. (Interestingly
in Linux code there is an IA32 infix.)

> @@ -86,6 +87,11 @@ static bool msr_read_allowed(unsigned int msr)
>  
>      case MSR_MCU_OPT_CTRL:
>          return cpu_has_srbds_ctrl;
> +    
> +    case MSR_IA32_THERM_STATUS:
> +    case MSR_DTS_TEMPERATURE_TARGET:
> +    case MSR_DTS_PACKAGE_THERM_STATUS:
> +        return raw_cpu_policy.basic.pm.dts;

This, aiui (and according to related comments I got from Andrew on remotely
similar changes I was doing) wants to use host_policy. Hence why that other
prereq change is needed that I talked about (and that iirc I reproduced on
the other sub-thread).

Jan

Re: [RFC PATCH for-4.22 v2 3/3] xenpm: Add get-intel-temp subcommand

[Jan Beulich]

On 29.10.2025 16:59, Teddy Astie wrote:
> @@ -1354,6 +1356,95 @@ void enable_turbo_mode(int argc, char *argv[])
>                  errno, strerror(errno));
>  }
>  
> +#define MSR_DTS_THERM_STATUS         0x0000019c
> +#define MSR_DTS_TEMPERATURE_TARGET   0x000001a2
> +#define MSR_DTS_PACKAGE_THERM_STATUS 0x000001b1

DTS infix question again. Actually, can't we use the hypervisor's msr-index.h here?
We already use it from the emulator test harness.

> +static 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_DTS_PACKAGE_THERM_STATUS : MSR_DTS_THERM_STATUS
> +        },
> +        (xc_resource_entry_t){ .idx = MSR_DTS_TEMPERATURE_TARGET },
> +    };
> +    struct xc_resource_op ops = {
> +        .cpu = cpu,
> +        .entries = entries,
> +        .nr_entries = 2,
> +    };
> +    int tjmax;

Plain int? (Same for the last function parameter.)

> +    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] MSR_IA32_TEMPERATURE_TARGET is not supported, assume "
> +                "tjmax=100°C, readings may be incorrect\n", cpu);

As per remarks elsewhere, I don't see why there is an IA32 infix here.

> +        tjmax = 100;
> +    }
> +    
> +    *temp = tjmax - ((entries[0].val >> 16) & 0xff);
> +    return 0;
> +}
> +
> +
> +void get_intel_temp(int argc, char *argv[])
> +{
> +    int temp, cpu = -1, socket;

Plain int question again, for temp and socket.

> +    bool has_data = false;
> +
> +    if (argc > 0)

This and ...

> +        parse_cpuid(argv[0], &cpu);
> +
> +    if (cpu != -1)

... this if() don't fit the (hypervisor) style used elsewhere.

Jan

Re: [RFC PATCH for-4.22 v2 2/3] x86/platform: Expose DTS sensors MSR

[Teddy Astie]

Le 30/10/2025 à 14:54, Jan Beulich a écrit :
> On 29.10.2025 16:59, Teddy Astie wrote:
>> --- a/xen/arch/x86/include/asm/msr-index.h
>> +++ b/xen/arch/x86/include/asm/msr-index.h
>> @@ -115,6 +115,9 @@
>>   #define  MCU_OPT_CTRL_GDS_MIT_DIS           (_AC(1, ULL) <<  4)
>>   #define  MCU_OPT_CTRL_GDS_MIT_LOCK          (_AC(1, ULL) <<  5)
>>
>> +#define MSR_DTS_TEMPERATURE_TARGET          0x000001a2
>> +#define MSR_DTS_PACKAGE_THERM_STATUS        0x000001b1
>
> Where are the DTS infixes coming from? The SDM doesn't have such. We try to
> stay as close as reasonable to the SDM / PM names, with the exception that
> typically we omit IA32 infixes.
>

I got confused with the naming of MSRs (and their lack of IA32 prefix)
in the upper list of MSR. I guess it should be MSR_PACKAGE_THERM_STATUS
and MSR_TEMPERATURE_TARGET.

> I'd also like to note that unlike the two THERM_STATUS, MSR_TEMPERATURE_TARGET
> (as per the absence if an IA32 infix in the SDM) isn't an architectural MSR,
> and hence I'm not entirely convinced we can "blindly" expose it. (Interestingly
> in Linux code there is an IA32 infix.)
>

We only perform rdmsr_safe on this MSR, so I don't think there is much
problem with it as I don't expect Intel to reuse this MSR number for
something else (at worst, it is going to not be implemented and would
gracefully fail).

Some parts of this MSR slightly change, but the one (tjmax) that is
interesting in here is consistent across the architectures.

>> @@ -86,6 +87,11 @@ static bool msr_read_allowed(unsigned int msr)
>>
>>       case MSR_MCU_OPT_CTRL:
>>           return cpu_has_srbds_ctrl;
>> +
>> +    case MSR_IA32_THERM_STATUS:
>> +    case MSR_DTS_TEMPERATURE_TARGET:
>> +    case MSR_DTS_PACKAGE_THERM_STATUS:
>> +        return raw_cpu_policy.basic.pm.dts;
>
> This, aiui (and according to related comments I got from Andrew on remotely
> similar changes I was doing) wants to use host_policy. Hence why that other
> prereq change is needed that I talked about (and that iirc I reproduced on
> the other sub-thread).
>

yes

> Jan



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RFC PATCH for-4.22 v2 3/3] xenpm: Add get-intel-temp subcommand

[Teddy Astie]

Le 30/10/2025 à 15:05, Jan Beulich a écrit :
> On 29.10.2025 16:59, Teddy Astie wrote:
>> @@ -1354,6 +1356,95 @@ void enable_turbo_mode(int argc, char *argv[])
>>                   errno, strerror(errno));
>>   }
>>
>> +#define MSR_DTS_THERM_STATUS         0x0000019c
>> +#define MSR_DTS_TEMPERATURE_TARGET   0x000001a2
>> +#define MSR_DTS_PACKAGE_THERM_STATUS 0x000001b1
>
> DTS infix question again. Actually, can't we use the hypervisor's msr-index.h here?
> We already use it from the emulator test harness.
>

I wasn't sure whether tools could use msr-index.h or not. If we can, we
also likely want to make some of the existing tools to rely on it
instead of having them defining it in their files.

>> +static 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_DTS_PACKAGE_THERM_STATUS : MSR_DTS_THERM_STATUS
>> +        },
>> +        (xc_resource_entry_t){ .idx = MSR_DTS_TEMPERATURE_TARGET },
>> +    };
>> +    struct xc_resource_op ops = {
>> +        .cpu = cpu,
>> +        .entries = entries,
>> +        .nr_entries = 2,
>> +    };
>> +    int tjmax;
>
> Plain int? (Same for the last function parameter.)
>
>> +    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] MSR_IA32_TEMPERATURE_TARGET is not supported, assume "
>> +                "tjmax=100°C, readings may be incorrect\n", cpu);
>
> As per remarks elsewhere, I don't see why there is an IA32 infix here.
>
>> +        tjmax = 100;
>> +    }
>> +
>> +    *temp = tjmax - ((entries[0].val >> 16) & 0xff);
>> +    return 0;
>> +}
>> +
>> +
>> +void get_intel_temp(int argc, char *argv[])
>> +{
>> +    int temp, cpu = -1, socket;
>
> Plain int question again, for temp and socket.
>

socket should be unsigned. But temp (as being CPU temperature) can
actually be negative (even though it is going to be quite specific).
The use of int here is consistent with what Linux coretemp uses to store
temperatures.

>> +    bool has_data = false;
>> +
>> +    if (argc > 0)
>
> This and ...
>
>> +        parse_cpuid(argv[0], &cpu);
>> +
>> +    if (cpu != -1)
>
> ... this if() don't fit the (hypervisor) style used elsewhere.
>

ok

> Jan
>



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RFC PATCH for-4.22 v2 2/3] x86/platform: Expose DTS sensors MSR

[Jan Beulich]

On 03.11.2025 15:30, Teddy Astie wrote:
> Le 30/10/2025 à 14:54, Jan Beulich a écrit :
>> On 29.10.2025 16:59, Teddy Astie wrote:
>> I'd also like to note that unlike the two THERM_STATUS, MSR_TEMPERATURE_TARGET
>> (as per the absence if an IA32 infix in the SDM) isn't an architectural MSR,
>> and hence I'm not entirely convinced we can "blindly" expose it. (Interestingly
>> in Linux code there is an IA32 infix.)
> 
> We only perform rdmsr_safe on this MSR, so I don't think there is much 
> problem with it as I don't expect Intel to reuse this MSR number for 
> something else (at worst, it is going to not be implemented and would 
> gracefully fail).
> 
> Some parts of this MSR slightly change, but the one (tjmax) that is 
> interesting in here is consistent across the architectures.

Perhaps it's indeed okay here, but the aspect needs calling out / justifying in
the description.

Jan