[PATCH v9 1/3] libxc: Report EINVAL in invalid xc_resource_op use

[PATCH v9 1/3] libxc: Report EINVAL in invalid xc_resource_op use

[Teddy Astie]

xc_report_op doesn't update errno when called with 0 operations
(even though it returns -1).

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
 tools/libs/ctrl/xc_resource.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libs/ctrl/xc_resource.c b/tools/libs/ctrl/xc_resource.c
index cb6a97202b..f65127f91c 100644
--- a/tools/libs/ctrl/xc_resource.c
+++ b/tools/libs/ctrl/xc_resource.c
@@ -137,6 +137,7 @@ int xc_resource_op(xc_interface *xch, uint32_t nr_ops, xc_resource_op_t *ops)
     if ( nr_ops > 1 )
         return xc_resource_op_multi(xch, nr_ops, ops);
 
+    errno = EINVAL;
     return -1;
 }
 
-- 
2.53.0



--
 | Vates

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v9 2/3] xenpm: Use EXIT_{SUCCESS,FAILURE} instead of errno as exit codes

[Teddy Astie]

errno is not unified accross platforms, which makes error codes actually
platform specific. C standard defines EXIT_SUCCESS and EXIT_FAILURE
(respectively 0 and 1) as standard errors codes, even though it only reports
whether it failed or not.

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

 tools/misc/xenpm.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index 682d092479..e4902d2e82 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -112,7 +112,7 @@ static void parse_cpuid(const char *arg, int *cpuid)
         if ( strcasecmp(arg, "all") )
         {
             fprintf(stderr, "Invalid CPU identifier: '%s'\n", arg);
-            exit(EINVAL);
+            exit(EXIT_FAILURE);
         }
         *cpuid = -1;
     }
@@ -124,7 +124,7 @@ static void parse_cpuid_and_int(int argc, char *argv[],
     if ( argc == 0 )
     {
          fprintf(stderr, "Missing %s\n", what);
-         exit(EINVAL);
+         exit(EXIT_FAILURE);
     }
 
     if ( argc > 1 )
@@ -133,7 +133,7 @@ static void parse_cpuid_and_int(int argc, char *argv[],
     if ( sscanf(argv[argc > 1], "%d", val) != 1 )
     {
         fprintf(stderr, "Invalid %s '%s'\n", what, argv[argc > 1]);
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
     }
 }
 
@@ -662,7 +662,7 @@ static void signal_int_handler(int signo)
 out:
     free(cputopo);
     xc_interface_close(xc_handle);
-    exit(0);
+    exit(EXIT_SUCCESS);
 }
 
 void start_gather_func(int argc, char *argv[])
@@ -1154,7 +1154,7 @@ void scaling_governor_func(int argc, char *argv[])
     else
     {
         fprintf(stderr, "Missing argument(s)\n");
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
     }
 
     if ( cpuid < 0 )
@@ -1215,7 +1215,7 @@ void cpu_topology_func(int argc, char *argv[])
 out:
     free(cputopo);
     if ( rc )
-        exit(rc);
+        exit(EXIT_FAILURE);
 }
 
 void set_sched_smt_func(int argc, char *argv[])
@@ -1224,7 +1224,7 @@ void set_sched_smt_func(int argc, char *argv[])
 
     if ( argc != 1 ) {
         fprintf(stderr, "Missing or invalid argument(s)\n");
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
     }
 
     if ( !strcasecmp(argv[0], "disable") )
@@ -1234,7 +1234,7 @@ void set_sched_smt_func(int argc, char *argv[])
     else
     {
         fprintf(stderr, "Invalid argument: %s\n", argv[0]);
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
     }
 
     if ( !xc_set_sched_opt_smt(xc_handle, value) )
@@ -1254,12 +1254,12 @@ void set_vcpu_migration_delay_func(int argc, char *argv[])
 
     if ( argc != 1 || (value = atoi(argv[0])) < 0 ) {
         fprintf(stderr, "Missing or invalid argument(s)\n");
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
     }
 
     if ( xc_sched_credit_params_get(xc_handle, 0, &sparam) < 0 ) {
         fprintf(stderr, "getting Credit scheduler parameters failed\n");
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
     }
     sparam.vcpu_migr_delay_us = value;
 
@@ -1304,7 +1304,7 @@ void set_max_cstate_func(int argc, char *argv[])
            : (subval = XEN_SYSCTL_CX_UNLIMITED, strcmp(argv[1], "unlimited")))) )
     {
         fprintf(stderr, "Missing, excess, or invalid argument(s)\n");
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
     }
 
     snprintf(buf, ARRAY_SIZE(buf), "C%d", value);
@@ -1575,7 +1575,7 @@ static void cppc_set_func(int argc, char *argv[])
     uint32_t set_params;
 
     if ( parse_cppc_opts(&set_cppc, &cpuid, argc, argv) )
-        exit(EINVAL);
+        exit(EXIT_FAILURE);
 
     if ( cpuid != -1 )
     {
@@ -1637,7 +1637,7 @@ int main(int argc, char *argv[])
     if ( !xc_handle )
     {
         fprintf(stderr, "failed to get the handler\n");
-        return EIO;
+        return EXIT_FAILURE;
     }
 
     ret = xc_physinfo(xc_handle, &physinfo);
@@ -1647,7 +1647,7 @@ int main(int argc, char *argv[])
         fprintf(stderr, "failed to get processor information (%d - %s)\n",
                 ret, strerror(ret));
         xc_interface_close(xc_handle);
-        return ret;
+        return EXIT_FAILURE;
     }
     max_cpu_nr = physinfo.max_cpu_id + 1;
 
@@ -1662,7 +1662,8 @@ int main(int argc, char *argv[])
         for ( i = 0; i < nr_matches; i++ )
             fprintf(stderr, " %s", main_options[matches_main_options[i]].name);
         fprintf(stderr, "\n");
-        ret = EINVAL;
+        xc_interface_close(xc_handle);
+        return EXIT_FAILURE;
     }
     else if ( nr_matches == 1 )
         /* dispatch to the corresponding function handler */
@@ -1670,10 +1671,11 @@ int main(int argc, char *argv[])
     else
     {
         show_help();
-        ret = EINVAL;
+        xc_interface_close(xc_handle);
+        return EXIT_FAILURE;
     }
 
     xc_interface_close(xc_handle);
-    return ret;
+    return EXIT_SUCCESS;
 }
 
-- 
2.53.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v9 3/3] xenpm: Add get-core-temp subcommand

[Teddy Astie]

get-core-temp allows querying the per-core CPU temperature and
per-package one on processors that supports Digital Temperature Sensors
(most 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>
---
CC: Jan Beulich <jbeulich@suse.com>

v4: https://lore.kernel.org/xen-devel/cover.1766158766.git.teddy.astie@vates.tech/
v5: Removed trailing whitespace.
v6: Report errors through errno and use strerror() to display them
v7:
 - Rename get-intel-temp with get-dts-temp
 - handle properly errno
 - make process return a error code if no data
v8:
 - update Changelog
 - improve error handling
 - rename core-dts-temp with get-core-temp
v9:
 - exit with EXIT_{SUCCESS,FAILURE} instead of errno

 CHANGELOG.md       |   2 +
 tools/misc/xenpm.c | 128 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c191e504ab..69cce1c874 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
      mitigate (by rate-limiting) the system wide impact of an HVM guest
      misusing atomic instructions.
    - Support for CPIO microcode in discrete multiboot modules.
+   - Introduce get-core-temp to xenpm to query CPU temperatures on Intel
+     platforms.

 ### Removed
  - On x86:
diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index e4902d2e82..37f484b362 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -32,11 +32,14 @@

 #include <xen-tools/common-macros.h>

+#include <xen/asm/msr-index.h>
+
 #define MAX_PKG_RESIDENCIES 12
 #define MAX_CORE_RESIDENCIES 8

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

 /* help message */
 void show_help(void)
@@ -93,6 +96,7 @@ void show_help(void)
             "                                           units default to \"us\" if unspecified.\n"
             "                                           truncates un-representable values.\n"
             "                                           0 lets the hardware decide.\n"
+            " get-core-temp          [cpuid]      get CPU temperature for <cpuid> or all (Intel only)\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 +1358,128 @@ void enable_turbo_mode(int argc, char *argv[])
                 errno, strerror(errno));
 }

+static int fetch_dts_temp(xc_interface *xch, uint32_t cpu, bool package, int *temp)
+{
+    xc_resource_entry_t entries[] = {
+        { .idx = package ? MSR_PACKAGE_THERM_STATUS : MSR_IA32_THERM_STATUS },
+        { .idx = MSR_TEMPERATURE_TARGET },
+    };
+    struct xc_resource_op ops = {
+        .cpu = cpu,
+        .entries = entries,
+        .nr_entries = ARRAY_SIZE(entries),
+    };
+    int tjmax;
+
+    int ret = xc_resource_op(xch, 1, &ops);
+
+    switch ( ret )
+    {
+    case 0:
+        /* This CPU isn't online or can't query this MSR */
+        errno = ENODATA;
+        return -1;
+
+    case 1:
+    {
+        /*
+         * The CPU doesn't support MSR_TEMPERATURE_TARGET, we assume it's 100
+         * which is correct aside a few selected Atom CPUs. Check Linux
+         * kernel's coretemp.c for more information.
+         */
+        static bool has_reported_once = false;
+
+        if ( !has_reported_once )
+        {
+            fprintf(stderr, "MSR_TEMPERATURE_TARGET is not supported, assume "
+                            "tjmax = 100, readings may be incorrect.\n");
+            has_reported_once = true;
+        }
+
+        tjmax = 100;
+        break;
+    }
+
+    case 2:
+        tjmax = (entries[1].val >> 16) & 0xff;
+        break;
+
+    default:
+        if ( ret > 0 )
+        {
+            fprintf(stderr, "Got unexpected xc_resource_op return value: %d", ret);
+            errno = EINVAL;
+        }
+        return -1;
+    }
+
+    *temp = tjmax - ((entries[0].val >> 16) & 0xff);
+    return 0;
+}
+
+static void get_core_temp(int argc, char *argv[])
+{
+    int temp = -1, cpu = -1;
+    unsigned int 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
+        {
+            fprintf(stderr, "Unable to fetch temperature (%d - %s)\n",
+                    errno, strerror(errno));
+            printf("No data\n");
+            exit(EXIT_FAILURE);
+        }
+        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) )
+        {
+            fprintf(stderr,
+                    "[Package%u] Unable to fetch temperature (%d - %s)\n",
+                    cpu, errno, strerror(errno));
+            continue;
+        }
+
+        has_data = true;
+        printf("Package%u: %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) )
+        {
+            fprintf(stderr, "[CPU%d] Unable to fetch temperature (%d - %s)\n",
+                    cpu, errno, strerror(errno));
+            continue;
+        }
+
+        has_data = true;
+        printf("CPU%d: %d°C\n", cpu, temp);
+    }
+
+    if ( !has_data )
+    {
+        printf("No data\n");
+        exit(EXIT_FAILURE);
+    }
+}
+
 void disable_turbo_mode(int argc, char *argv[])
 {
     int cpuid = -1;
@@ -1618,12 +1744,12 @@ struct {
     { "set-max-cstate", set_max_cstate_func},
     { "enable-turbo-mode", enable_turbo_mode },
     { "disable-turbo-mode", disable_turbo_mode },
+    { "get-core-temp", get_core_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.53.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v9 1/3] libxc: Report EINVAL in invalid xc_resource_op use

[Anthony PERARD]

On Mon, Mar 16, 2026 at 02:34:05PM +0000, Teddy Astie wrote:
> xc_report_op doesn't update errno when called with 0 operations
> (even though it returns -1).
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v9 2/3] xenpm: Use EXIT_{SUCCESS,FAILURE} instead of errno as exit codes

[Anthony PERARD]

On Mon, Mar 16, 2026 at 02:34:07PM +0000, Teddy Astie wrote:
> errno is not unified accross platforms, which makes error codes actually
> platform specific. C standard defines EXIT_SUCCESS and EXIT_FAILURE
> (respectively 0 and 1) as standard errors codes, even though it only reports
> whether it failed or not.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v9 3/3] xenpm: Add get-core-temp subcommand

[Anthony PERARD]

On Mon, Mar 16, 2026 at 02:34:09PM +0000, Teddy Astie wrote:
> diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
> index e4902d2e82..37f484b362 100644
> --- a/tools/misc/xenpm.c
> +++ b/tools/misc/xenpm.c
> +static void get_core_temp(int argc, char *argv[])
> +{
> +    int temp = -1, cpu = -1;
> +    unsigned int 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
> +        {
> +            fprintf(stderr, "Unable to fetch temperature (%d - %s)\n",
> +                    errno, strerror(errno));
> +            printf("No data\n");

What is this "no data" for? There's already two clues which says that
there's nothing to print, the error message on stderr, and the non-zero
exit value.

> +            exit(EXIT_FAILURE);
> +        }
> +        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) )

Here, you deal with the error return by fetch_dts_temp() first, then the
success, but in the previous block (cpu=all) you do the opposite. Also,
here the success isn't even in the else part, but after. Could you
choose one style to be consistent?

I think I prefer to deal with the error first, so like here.

> +        {
> +            fprintf(stderr,
> +                    "[Package%u] Unable to fetch temperature (%d - %s)\n",
> +                    cpu, errno, strerror(errno));
> +            continue;

If we got an error one one package, aren't we likely to got more error?
Is it worth to keep trying on the next package?

> +        }
> +
> +        has_data = true;
> +        printf("Package%u: %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) )
> +        {
> +            fprintf(stderr, "[CPU%d] Unable to fetch temperature (%d - %s)\n",
> +                    cpu, errno, strerror(errno));
> +            continue;
> +        }
> +
> +        has_data = true;
> +        printf("CPU%d: %d°C\n", cpu, temp);
> +    }
> +
> +    if ( !has_data )
> +    {
> +        printf("No data\n");

Another "no data".

> +        exit(EXIT_FAILURE);
> +    }
> +}
> +

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v9 3/3] xenpm: Add get-core-temp subcommand

[Teddy Astie]

Le 30/03/2026 à 15:11, Anthony PERARD a écrit :
> On Mon, Mar 16, 2026 at 02:34:09PM +0000, Teddy Astie wrote:
>> diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
>> index e4902d2e82..37f484b362 100644
>> --- a/tools/misc/xenpm.c
>> +++ b/tools/misc/xenpm.c
>> +static void get_core_temp(int argc, char *argv[])
>> +{
>> +    int temp = -1, cpu = -1;
>> +    unsigned int 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
>> +        {
>> +            fprintf(stderr, "Unable to fetch temperature (%d - %s)\n",
>> +                    errno, strerror(errno));
>> +            printf("No data\n");
>
> What is this "no data" for? There's already two clues which says that
> there's nothing to print, the error message on stderr, and the non-zero
> exit value.
>

The "no data" could be removed now that the information is transmitted
differently.

>> +            exit(EXIT_FAILURE);
>> +        }
>> +        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) )
>
> Here, you deal with the error return by fetch_dts_temp() first, then the
> success, but in the previous block (cpu=all) you do the opposite. Also,
> here the success isn't even in the else part, but after. Could you
> choose one style to be consistent?
>
> I think I prefer to deal with the error first, so like here.
>

Looks good to me.

>> +        {
>> +            fprintf(stderr,
>> +                    "[Package%u] Unable to fetch temperature (%d - %s)\n",
>> +                    cpu, errno, strerror(errno));
>> +            continue;
>
> If we got an error one one package, aren't we likely to got more error?
> Is it worth to keep trying on the next package?
>
>> +        }
>> +
>> +        has_data = true;
>> +        printf("Package%u: %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) )
>> +        {
>> +            fprintf(stderr, "[CPU%d] Unable to fetch temperature (%d - %s)\n",
>> +                    cpu, errno, strerror(errno));
>> +            continue;
>> +        }
>> +
>> +        has_data = true;
>> +        printf("CPU%d: %d°C\n", cpu, temp);
>> +    }
>> +
>> +    if ( !has_data )
>> +    {
>> +        printf("No data\n");
>
> Another "no data".
>

It could be removed as well.

---

I wonder if we can remove the "has_data" boolean by making any (aside
package temperature if it's not supported) failure cause a
exit(EXIT_FAILURE) instead ?

We lose the empty line between Package and CPU temperature though, as we
need to add it only if package temperature exists.

>> +        exit(EXIT_FAILURE);
>> +    }
>> +}
>> +
>
> Thanks,
>
>
> --
> Anthony Perard | Vates XCP-ng Developer
>
> XCP-ng & Xen Orchestra - Vates solutions
>
> web: https://vates.tech
>
>
>



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech