[PATCH v7 1/2] libxc: Report consistent errors in xc_resource_op

[PATCH v7 1/2] libxc: Report consistent errors in xc_resource_op

[Teddy Astie]

xc_report_op returns -1 in some error conditions.
Make sure it returns -ENOMEM in out of memory errors and -EINVAL
in invalid usages errors.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
v7: Introduced

 tools/libs/ctrl/xc_resource.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/libs/ctrl/xc_resource.c b/tools/libs/ctrl/xc_resource.c
index cb6a97202b..1ca71ee048 100644
--- a/tools/libs/ctrl/xc_resource.c
+++ b/tools/libs/ctrl/xc_resource.c
@@ -28,7 +28,7 @@ static int xc_resource_op_one(xc_interface *xch, xc_resource_op_t *op)
                                 XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
 
     if ( xc_hypercall_bounce_pre(xch, entries) )
-        return -1;
+        return -ENOMEM;
 
     platform_op.cmd = XENPF_resource_op;
     platform_op.u.resource_op.nr_entries = op->nr_entries;
@@ -54,19 +54,19 @@ static int xc_resource_op_multi(xc_interface *xch, uint32_t nr_ops, xc_resource_
     call_list = xc_hypercall_buffer_alloc(xch, call_list,
                                           sizeof(*call_list) * nr_ops);
     if ( !call_list )
-        return -1;
+        return -ENOMEM;
 
     platform_ops = xc_hypercall_buffer_array_create(xch, nr_ops);
     if ( !platform_ops )
     {
-        rc = -1;
+        rc = -ENOMEM;
         goto out;
     }
 
     entries_list = xc_hypercall_buffer_array_create(xch, nr_ops);
     if ( !entries_list )
     {
-        rc = -1;
+        rc = -ENOMEM;
         goto out;
     }
 
@@ -81,7 +81,7 @@ static int xc_resource_op_multi(xc_interface *xch, uint32_t nr_ops, xc_resource_
                         platform_op, sizeof(xen_platform_op_t));
         if ( !platform_op )
         {
-            rc = -1;
+            rc = -ENOMEM;
             goto out;
         }
 
@@ -90,7 +90,7 @@ static int xc_resource_op_multi(xc_interface *xch, uint32_t nr_ops, xc_resource_
                    entries, entries_size);
         if ( !entries)
         {
-            rc = -1;
+            rc = -ENOMEM;
             goto out;
         }
         memcpy(entries, op->entries, entries_size);
@@ -137,7 +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);
 
-    return -1;
+    return -EINVAL;
 }
 
 /*
-- 
2.53.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v7 2/2] xenpm: Add get-dts-temp subcommand

[Teddy Astie]

get-dts-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

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

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 18f3d10f20..d7fac4a8d0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
    - Support for Bus Lock Threshold on AMD Zen5 and later CPUs, used by Xen to
      mitigate (by rate-limiting) the system wide impact of an HVM guest
      misusing atomic instructions.
+   - Introduce get-intel-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 682d092479..8d6a91d680 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-dts-temp          [cpuid]       get CPU temperature through DTS for <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 +1358,130 @@ 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 -errno;
+
+    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;
+        }
+        else
+            errno = -ret;
+        return -errno;
+    }
+
+    *temp = tjmax - ((entries[0].val >> 16) & 0xff);
+    return 0;
+}
+
+static void get_dts_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(ENODATA);
+        }
+        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(ENODATA);
+    }
+}
+
 void disable_turbo_mode(int argc, char *argv[])
 {
     int cpuid = -1;
@@ -1618,12 +1746,12 @@ struct {
     { "set-max-cstate", set_max_cstate_func},
     { "enable-turbo-mode", enable_turbo_mode },
     { "disable-turbo-mode", disable_turbo_mode },
+    { "get-dts-temp", get_dts_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 v7 2/2] xenpm: Add get-dts-temp subcommand

[Jan Beulich]

On 23.02.2026 17:06, Teddy Astie wrote:
> get-dts-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

I'm sorry, but would you clarify in how far "DTS" is less Intel-ish than
"Intel"? Something generic, re-usable (for other vendors) may be "core".

> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
>     - Support for Bus Lock Threshold on AMD Zen5 and later CPUs, used by Xen to
>       mitigate (by rate-limiting) the system wide impact of an HVM guest
>       misusing atomic instructions.
> +   - Introduce get-intel-temp to xenpm to query CPU temperatures on Intel
> +     platforms.

Nit: This still says "intel". Also the sentence looks incomplete; perhaps
missing something like "command line option" or "sub-command".

> @@ -1354,6 +1358,130 @@ 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 -errno;

I fear I still don't follow any of this errno handling. Why would a function
return -errno after setting it? Yet, here you want to "synthesize" an error,
but that wants doing to match library functions' behavior.

Jan

Re: [PATCH v7 1/2] libxc: Report consistent errors in xc_resource_op

[Jan Beulich]

On 23.02.2026 17:06, Teddy Astie wrote:
> xc_report_op returns -1 in some error conditions.
> Make sure it returns -ENOMEM in out of memory errors and -EINVAL
> in invalid usages errors.

Isn't this a move in the wrong direction? -1 as a return value is quite okay.
errno wants setting to indicate the cause of the error (if called functions
don't already set it properly).

Also nit: s/xc_report_op/xc_resource_op/ .

Jan

Re: [PATCH v7 1/2] libxc: Report consistent errors in xc_resource_op

[Teddy Astie]

Le 23/02/2026 à 17:15, Jan Beulich a écrit :
> On 23.02.2026 17:06, Teddy Astie wrote:
>> xc_report_op returns -1 in some error conditions.
>> Make sure it returns -ENOMEM in out of memory errors and -EINVAL
>> in invalid usages errors.
>
> Isn't this a move in the wrong direction? -1 as a return value is quite okay.
> errno wants setting to indicate the cause of the error (if called functions
> don't already set it properly).
>

To me, passing error through errno here feels more like a workaround
rather than a proper error handling. It doesn't feel consistent in libxc
overall (some functions returns a negative value corresponding to a
error number while some others -1; in some cases we update errno).

What are the error handling rules for xenctrl ?

> Also nit: s/xc_report_op/xc_resource_op/ .
>

ack

> Jan
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v7 1/2] libxc: Report consistent errors in xc_resource_op

[Jan Beulich]

On 24.02.2026 10:28, Teddy Astie wrote:
> Le 23/02/2026 à 17:15, Jan Beulich a écrit :
>> On 23.02.2026 17:06, Teddy Astie wrote:
>>> xc_report_op returns -1 in some error conditions.
>>> Make sure it returns -ENOMEM in out of memory errors and -EINVAL
>>> in invalid usages errors.
>>
>> Isn't this a move in the wrong direction? -1 as a return value is quite okay.
>> errno wants setting to indicate the cause of the error (if called functions
>> don't already set it properly).
> 
> To me, passing error through errno here feels more like a workaround 
> rather than a proper error handling. It doesn't feel consistent in libxc 
> overall (some functions returns a negative value corresponding to a 
> error number while some others -1; in some cases we update errno).
> 
> What are the error handling rules for xenctrl ?

Question goes mainly to Anthony. My take is that library functions should
behave properly, and where that isn't the case it wants adjusting.

Jan

Re: [PATCH v7 1/2] libxc: Report consistent errors in xc_resource_op

[Anthony PERARD]

On Tue, Feb 24, 2026 at 09:28:57AM +0000, Teddy Astie wrote:
> Le 23/02/2026 à 17:15, Jan Beulich a écrit :
> > On 23.02.2026 17:06, Teddy Astie wrote:
> >> xc_report_op returns -1 in some error conditions.
> >> Make sure it returns -ENOMEM in out of memory errors and -EINVAL
> >> in invalid usages errors.
> >
> > Isn't this a move in the wrong direction? -1 as a return value is quite okay.
> > errno wants setting to indicate the cause of the error (if called functions
> > don't already set it properly).
> >
>
> To me, passing error through errno here feels more like a workaround
> rather than a proper error handling. It doesn't feel consistent in libxc
> overall (some functions returns a negative value corresponding to a
> error number while some others -1; in some cases we update errno).
>
> What are the error handling rules for xenctrl ?

They are written down. See
    https://elixir.bootlin.com/xen/v4.21.0/source/tools/include/xenctrl.h#L76

    Unless otherwise specified, each function here returns zero or a
    non-null pointer on success; or in case of failure, sets errno and
    returns -1 or a null pointer.

    Unless otherwise specified, errors result in a call to the error
    handler function, which by default prints a message to the
    FILE* passed as the caller_data, which by default is stderr.
    (This is described below as "logging errors".)

    The error handler can safely trash errno, as libxc saves it across
    the callback.

`errno` isn't a workaround, it is the way many libc functions and other
passes generic error code. It may feel awkward, but that C.

If some functions don't update `errno`, it might be because the syscall
already set `errno` and there's no need to change it.

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech