On 09.02.2026 11:31, Teddy Astie wrote:
> @@ -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-intel-temp        [cpuid]       get Intel CPU temperature 
> of <cpuid> or all\n"

Sorry, thinking about it only now: Do we really want to build in the vendor
name to a command? "get-temp" would allow for adding an AMD implementation
later on?

> @@ -1354,6 +1358,131 @@ 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 */
> +        errno = -ENOMEM;

And xc_resource_op() doesn't itself set / inherit a properly set errno?
We don't want to override what the C library may have set.

> +        return -1;
> +
> +    case 0:
> +        /* This CPU isn't online or can't query this MSR */
> +        errno = -ENODATA;
> +        return -1;

Here we "synthesize" an error, so errno indeed needs setting. However,
doesn't errno want setting to positive E... values?

> +    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;

Why would this be? How do you know "ret" holds a value suitable for putting
in errno?

Jan

Reply via email to