On 12.01.2026 17:47, Teddy Astie wrote:
> @@ -1354,6 +1358,115 @@ 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 -1:
> + /* xc_resource_op returns -1 in out of memory scenarios */
> + return -ENOMEM;
Assuming xc_resource_op() is well-behaved in this regard, why not return errno
here? Or yet better stick to -1, leaving it to the caller to consume errno? And
then ...
> + case 0:
> + /* This CPU isn't online or can't query this MSR */
> + return -ENODATA;
... set errno here and return -1? With this normalized here, ...
> + 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);
> + return -EINVAL;
> + }
> + return ret;
> + }
> +
> + *temp = tjmax - ((entries[0].val >> 16) & 0xff);
> + return 0;
> +}
> +
> +static void get_intel_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
> + printf("No data\n");
... you can then use perror() here (and perhaps elsewhere). Right now the
distinct non-zero return values of fetch_dts_temp() are of no interest to
any of the callers.
Jan
