Re: coretemp driver change on dual-die/package systems

From: Guenter Roeck <linux@roeck-us.net>
To: Zhang Rui <rui.zhang@intel.com>, jdelvare@suse.com
Cc: linux-hwmon@vger.kernel.org
Subject: Re: coretemp driver change on dual-die/package systems
Date: Mon, 11 Feb 2019 06:13:07 -0800	[thread overview]
Message-ID: <4ecc367e-5020-ed68-e7bc-839b2dc5bb12@roeck-us.net> (raw)
In-Reply-To: <1549878372.2099.36.camel@intel.com>

On 2/11/19 1:46 AM, Zhang Rui wrote:
> Hi, Jean and Guenter,
> 
> Per the Intel Software Developer's Manual, which you can found at www.i
> ntel.com/sdm, when CPUID.1F is present, it is preferred over CPUID.B.
> 
> New dual-die/package systems will see this difference:
>     CPUID.0B enumerates the CPUs on each die as if they were in
> different packages.
>     CPUID.1F enumerates the CPUs on each die within the same package.
> 
> This will manifest in the sysfs physical_package_id attribute. ie. In
> the example above, CPUID.B would cause lscpu to show 2 packages, and
> CPUID.1F will cause lscpu to show 1 package.
> 
> Also, with CPUID.B the concept of a package-scope MSR and a die-scope
> MSR are synonymous.  With CPUID.1F, it is possible for some MSRs to
> have die-scope, and other MSRs to have package-scope.
> 
> MSR_IA32_PACKAGE_THERM_STATUS is a die-scope MSR, thus we need to
> update the coretemp driver to become multi-die-aware when we support
> CPUID.1F.
> 
> Previously, we create one hwmon device for each package, now we need to
> create one hwmon device for each die.
> But there is one problem left. For each coretemp hwmon device, the
> "temp1_input" attribute represents the temperature got from
> MSR_IA32_PACKAGE_THERM_STATUS, and "temp1_label" is "Package id X",
> where X is the logical package id.
> Now we create one hwmon device for each die, thus temp1_label can not
> use logical package id any more, because there are two dies in the same
> package.
> To me, there are two choices,
> 1. changing "temp1_label" from "Package id X" to "Package id Y", where
> Y is just a unique number instead of logical package id, say, using
> ida.
> 2. changing "temp1_label" from "Package id X" to "Package id X Die id
> Y", where Y is the die id.
> 
> Question is I'm not sure how temp1_label is used and if this change
> will break any userspace, like lm-sensors?
> 

Please feel free to change the label as it makes sense. I would suggest option
2 to avoid confusion. The string it reports is not part of the ABI and
can be changed. It can be overwritten with sensors3.conf anyway, so nothing
should depend on it.

Guenter