[PATCH v2 0/3] drivers: thermal/hwmon: intel: Use model-specific bitmasks for temperature registers

[PATCH v2 0/3] drivers: thermal/hwmon: intel: Use model-specific bitmasks for temperature registers

[Ricardo Neri]

Hi,

Here is v2 of the patchset to use model-specific bitmasks to read TCC
offset and the digital temperature readout of IA32_[PACKAGE]_THERM_STATUS.

You can read the cover letter of v1 here [1] for details.

Changes since v1:
 * Dropped dependency on CONFIG_INTEL_TCC for the coretemp driver.
   Instead, extend the bitmask to 0xff to accommodate recent processors.
   (Bit 23 of IA32_[PACKAGE]_THERM_STATUS is likely to be zero in less
   recent processors.)
 * Renamed TCC_FAM6_MODEL_TEMP_MASKS as TCC_MODEL_TEMP_MASKS.
 * Renamed get_tcc_offset_mask() as intel_tcc_get_offset_mask().
 * Do not export intel_tcc_get_temp_mask() as it is no longer used
   outside intel_tcc.c
 * Dropped stub functions for digital temperature readout and TCC
   offset. They are not needed as users select CONFIG_INTEL_TCC.

I have tested these patches on Alder Lake, Meteor Lake, and Grand Ridge
systems.

These patches apply cleanly on top of the `testing` branches of the
linux-pm and hwmon repositories.

Thanks and BR,
Ricardo

[1]. https://lore.kernel.org/linux-pm/20240406010416.4821-1-ricardo.neri-calderon@linux.intel.com/

Ricardo Neri (3):
  thermal: intel: intel_tcc: Add model checks for temperature registers
  thermal: intel: intel_tcc_cooling: Use a model-specific bitmask for
    TCC offset
  hwmon: (coretemp) Extend the bitmask to read temperature to 0xff

 drivers/hwmon/coretemp.c                  |   2 +-
 drivers/thermal/intel/intel_tcc.c         | 177 +++++++++++++++++++++-
 drivers/thermal/intel/intel_tcc_cooling.c |   2 +-
 include/linux/intel_tcc.h                 |   1 +
 4 files changed, 175 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] thermal: intel: intel_tcc: Add model checks for temperature registers

[Ricardo Neri]

The register MSR_TEMPERATURE_TARGET is not architectural. Its fields may be
defined differently for each processor model. TCC_OFFSET is an example of
such case.

Despite being specified as architectural, the registers IA32_[PACKAGE]_
THERM_STATUS have become model-specific: in recent processors, the
digital temperature readout uses bits [23:16] whereas the Intel Software
Developer's manual specifies bits [22:16].

Create an array of processor models and their bitmasks for TCC_OFFSET and
the digital temperature readout fields. Do not include recent processors.
Instead, use the bitmasks of these recent processors as default.

Use these model-specific bitmasks when reading TCC_OFFSET or the
temperature sensors.

Initialize a model-specific data structure during subsys_initcall() to
have it ready when thermal drivers are loaded.

Expose the new interface intel_tcc_get_offset_mask(). The
intel_tcc_cooling driver will use it.

Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Lukasz Luba <lukasz.luba@arm.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org # v6.7+
---
Changes since v1:
 * Renamed TCC_FAM6_MODEL_TEMP_MASKS as TCC_MODEL_TEMP_MASKS. (Rui)
 * Renamed get_tcc_offset_mask() as intel_tcc_get_offset_mask(). (Rui)
 * Do not export intel_tcc_get_temp_mask() as it is no longer used
   outside intel_tcc.c
 * Dropped stub functions for digital temperature readout and TCC
   offset. They are not needed as users select CONFIG_INTEL_TCC.
---
 drivers/thermal/intel/intel_tcc.c | 177 +++++++++++++++++++++++++++++-
 include/linux/intel_tcc.h         |   1 +
 2 files changed, 173 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/intel/intel_tcc.c b/drivers/thermal/intel/intel_tcc.c
index 5e8b7f34b395..9943c43c06df 100644
--- a/drivers/thermal/intel/intel_tcc.c
+++ b/drivers/thermal/intel/intel_tcc.c
@@ -6,8 +6,170 @@
 
 #include <linux/errno.h>
 #include <linux/intel_tcc.h>
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
 #include <asm/msr.h>
 
+/**
+ * struct temp_masks - Bitmasks for temperature readings
+ * @tcc_offset:			TCC offset in MSR_TEMPERATURE_TARGET
+ * @digital_readout:		Digital readout in MSR_IA32_THERM_STATUS
+ * @pkg_digital_readout:	Digital readout in MSR_IA32_PACKAGE_THERM_STATUS
+ *
+ * Bitmasks to extract the fields of the MSR_TEMPERATURE and IA32_[PACKAGE]_
+ * THERM_STATUS registers for different processor models.
+ *
+ * The bitmask of TjMax is not included in this structure. It is always 0xff.
+ */
+struct temp_masks {
+	u32 tcc_offset;
+	u32 digital_readout;
+	u32 pkg_digital_readout;
+};
+
+#define TCC_MODEL_TEMP_MASKS(model, _tcc_offset, _digital_readout,	\
+			     _pkg_digital_readout)			\
+	static const struct temp_masks temp_##model __initconst = {	\
+		.tcc_offset = _tcc_offset,				\
+		.digital_readout = _digital_readout,			\
+		.pkg_digital_readout = _pkg_digital_readout		\
+	}
+
+TCC_MODEL_TEMP_MASKS(nehalem, 0, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(haswell_x, 0xf, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(broadwell, 0x3f, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(goldmont, 0x7f, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(tigerlake, 0x3f, 0xff, 0xff);
+TCC_MODEL_TEMP_MASKS(sapphirerapids, 0x3f, 0x7f, 0xff);
+
+/* Use these masks for processors not included in @tcc_cpu_ids. */
+static struct temp_masks intel_tcc_temp_masks __ro_after_init = {
+	.tcc_offset = 0x7f,
+	.digital_readout = 0xff,
+	.pkg_digital_readout = 0xff,
+};
+
+static const struct x86_cpu_id intel_tcc_cpu_ids[] __initconst = {
+	X86_MATCH_INTEL_FAM6_MODEL(CORE_YONAH,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(CORE2_MEROM,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(CORE2_MEROM_L,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(CORE2_PENRYN,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(CORE2_DUNNINGTON,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(NEHALEM,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_G,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_EP,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_EX,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(WESTMERE,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(WESTMERE_EP,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(WESTMERE_EX,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE_X,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X,		&temp_haswell_x),
+	X86_MATCH_INTEL_FAM6_MODEL(HASWELL,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X,		&temp_haswell_x),
+	X86_MATCH_INTEL_FAM6_MODEL(HASWELL_L,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(HASWELL_G,		&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(BROADWELL,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_G,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X,		&temp_haswell_x),
+	X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_D,		&temp_haswell_x),
+	X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X,		&temp_haswell_x),
+	X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE_L,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE_L,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(CANNONLAKE_L,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_L,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_NNPI,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ROCKETLAKE,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE_L,		&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE,		&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X,	&temp_sapphirerapids),
+	X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X,	&temp_sapphirerapids),
+	X86_MATCH_INTEL_FAM6_MODEL(LAKEFIELD,		&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE,		&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L,		&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE,		&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P,	&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S,	&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_BONNELL,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_BONNELL_MID,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL_MID,	&temp_nehalem),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_D,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT_MID,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT_NP,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT,	&temp_goldmont),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT_D,	&temp_goldmont),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT_PLUS,	&temp_goldmont),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_L,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(ATOM_GRACEMONT,	&temp_tigerlake),
+	X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL,	&temp_broadwell),
+	X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM,	&temp_broadwell),
+	{}
+};
+
+static int __init intel_tcc_init(void)
+{
+	const struct x86_cpu_id *id;
+
+	id = x86_match_cpu(intel_tcc_cpu_ids);
+	if (id)
+		memcpy(&intel_tcc_temp_masks, (const void *)id->driver_data,
+		       sizeof(intel_tcc_temp_masks));
+
+	return 0;
+}
+/*
+ * Use subsys_initcall to ensure temperature bitmasks are initialized before
+ * the drivers that use this library.
+ */
+subsys_initcall(intel_tcc_init);
+
+/**
+ * intel_tcc_get_offset_mask() - Returns the bitmask to read TCC offset
+ *
+ * Get the model-specific bitmask to extract TCC_OFFSET from the MSR
+ * TEMPERATURE_TARGET register. If the mask is 0, it means the processor does
+ * not support TCC offset.
+ *
+ * Return: The model-specific bitmask for TCC offset.
+ */
+u32 intel_tcc_get_offset_mask(void)
+{
+	return intel_tcc_temp_masks.tcc_offset;
+}
+EXPORT_SYMBOL_NS(intel_tcc_get_offset_mask, INTEL_TCC);
+
+/**
+ * get_temp_mask() - Returns the model-specific bitmask for temperature
+ *
+ * @pkg: true: Package Thermal Sensor. false: Core Thermal Sensor.
+ *
+ * Get the model-specific bitmask to extract the temperature reading from the
+ * MSR_IA32_[PACKAGE]_THERM_STATUS register.
+ *
+ * Callers must check if the thermal status registers are supported.
+ *
+ * Return: The model-specific bitmask for temperature reading
+ */
+static u32 get_temp_mask(bool pkg)
+{
+	return pkg ? intel_tcc_temp_masks.pkg_digital_readout :
+	       intel_tcc_temp_masks.digital_readout;
+}
+
 /**
  * intel_tcc_get_tjmax() - returns the default TCC activation Temperature
  * @cpu: cpu that the MSR should be run on, nagative value means any cpu.
@@ -56,7 +218,7 @@ int intel_tcc_get_offset(int cpu)
 	if (err)
 		return err;
 
-	return (low >> 24) & 0x3f;
+	return (low >> 24) & intel_tcc_temp_masks.tcc_offset;
 }
 EXPORT_SYMBOL_NS_GPL(intel_tcc_get_offset, INTEL_TCC);
 
@@ -76,7 +238,10 @@ int intel_tcc_set_offset(int cpu, int offset)
 	u32 low, high;
 	int err;
 
-	if (offset < 0 || offset > 0x3f)
+	if (!intel_tcc_temp_masks.tcc_offset)
+		return -ENODEV;
+
+	if (offset < 0 || offset > intel_tcc_temp_masks.tcc_offset)
 		return -EINVAL;
 
 	if (cpu < 0)
@@ -90,7 +255,7 @@ int intel_tcc_set_offset(int cpu, int offset)
 	if (low & BIT(31))
 		return -EPERM;
 
-	low &= ~(0x3f << 24);
+	low &= ~(intel_tcc_temp_masks.tcc_offset << 24);
 	low |= offset << 24;
 
 	if (cpu < 0)
@@ -113,8 +278,8 @@ EXPORT_SYMBOL_NS_GPL(intel_tcc_set_offset, INTEL_TCC);
  */
 int intel_tcc_get_temp(int cpu, int *temp, bool pkg)
 {
-	u32 low, high;
 	u32 msr = pkg ? MSR_IA32_PACKAGE_THERM_STATUS : MSR_IA32_THERM_STATUS;
+	u32 low, high, mask;
 	int tjmax, err;
 
 	tjmax = intel_tcc_get_tjmax(cpu);
@@ -132,7 +297,9 @@ int intel_tcc_get_temp(int cpu, int *temp, bool pkg)
 	if (!(low & BIT(31)))
 		return -ENODATA;
 
-	*temp = tjmax - ((low >> 16) & 0x7f);
+	mask = get_temp_mask(pkg);
+
+	*temp = tjmax - ((low >> 16) & mask);
 
 	return 0;
 }
diff --git a/include/linux/intel_tcc.h b/include/linux/intel_tcc.h
index 8ff8eabb4a98..fa788817acfc 100644
--- a/include/linux/intel_tcc.h
+++ b/include/linux/intel_tcc.h
@@ -14,5 +14,6 @@ int intel_tcc_get_tjmax(int cpu);
 int intel_tcc_get_offset(int cpu);
 int intel_tcc_set_offset(int cpu, int offset);
 int intel_tcc_get_temp(int cpu, int *temp, bool pkg);
+u32 intel_tcc_get_offset_mask(void);
 
 #endif /* __INTEL_TCC_H__ */
-- 
2.34.1

Re: [PATCH v2 1/3] thermal: intel: intel_tcc: Add model checks for temperature registers

[Zhang, Rui]

On Thu, 2024-04-25 at 10:13 -0700, Ricardo Neri wrote:
> The register MSR_TEMPERATURE_TARGET is not architectural. Its fields
> may be
> defined differently for each processor model. TCC_OFFSET is an
> example of
> such case.
> 
> Despite being specified as architectural, the registers
> IA32_[PACKAGE]_
> THERM_STATUS have become model-specific: in recent processors, the
> digital temperature readout uses bits [23:16] whereas the Intel
> Software
> Developer's manual specifies bits [22:16].
> 
> Create an array of processor models and their bitmasks for TCC_OFFSET
> and
> the digital temperature readout fields. Do not include recent
> processors.
> Instead, use the bitmasks of these recent processors as default.
> 
> Use these model-specific bitmasks when reading TCC_OFFSET or the
> temperature sensors.
> 
> Initialize a model-specific data structure during subsys_initcall()
> to
> have it ready when thermal drivers are loaded.
> 
> Expose the new interface intel_tcc_get_offset_mask(). The
> intel_tcc_cooling driver will use it.
> 
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Reviewed-by: Zhang Rui <rui.zhang@intel.com>

-rui
> ---
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Lukasz Luba <lukasz.luba@arm.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: linux-hwmon@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: stable@vger.kernel.org # v6.7+
> ---
> Changes since v1:
>  * Renamed TCC_FAM6_MODEL_TEMP_MASKS as TCC_MODEL_TEMP_MASKS. (Rui)
>  * Renamed get_tcc_offset_mask() as intel_tcc_get_offset_mask().
> (Rui)
>  * Do not export intel_tcc_get_temp_mask() as it is no longer used
>    outside intel_tcc.c
>  * Dropped stub functions for digital temperature readout and TCC
>    offset. They are not needed as users select CONFIG_INTEL_TCC.
> ---
>  drivers/thermal/intel/intel_tcc.c | 177
> +++++++++++++++++++++++++++++-
>  include/linux/intel_tcc.h         |   1 +
>  2 files changed, 173 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/intel/intel_tcc.c
> b/drivers/thermal/intel/intel_tcc.c
> index 5e8b7f34b395..9943c43c06df 100644
> --- a/drivers/thermal/intel/intel_tcc.c
> +++ b/drivers/thermal/intel/intel_tcc.c
> @@ -6,8 +6,170 @@
>  
>  #include <linux/errno.h>
>  #include <linux/intel_tcc.h>
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
>  #include <asm/msr.h>
>  
> +/**
> + * struct temp_masks - Bitmasks for temperature readings
> + * @tcc_offset:                        TCC offset in
> MSR_TEMPERATURE_TARGET
> + * @digital_readout:           Digital readout in
> MSR_IA32_THERM_STATUS
> + * @pkg_digital_readout:       Digital readout in
> MSR_IA32_PACKAGE_THERM_STATUS
> + *
> + * Bitmasks to extract the fields of the MSR_TEMPERATURE and
> IA32_[PACKAGE]_
> + * THERM_STATUS registers for different processor models.
> + *
> + * The bitmask of TjMax is not included in this structure. It is
> always 0xff.
> + */
> +struct temp_masks {
> +       u32 tcc_offset;
> +       u32 digital_readout;
> +       u32 pkg_digital_readout;
> +};
> +
> +#define TCC_MODEL_TEMP_MASKS(model, _tcc_offset,
> _digital_readout,     \
> +                           
> _pkg_digital_readout)                      \
> +       static const struct temp_masks temp_##model __initconst =
> {     \
> +               .tcc_offset =
> _tcc_offset,                              \
> +               .digital_readout =
> _digital_readout,                    \
> +               .pkg_digital_readout =
> _pkg_digital_readout             \
> +       }
> +
> +TCC_MODEL_TEMP_MASKS(nehalem, 0, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(haswell_x, 0xf, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(broadwell, 0x3f, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(goldmont, 0x7f, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(tigerlake, 0x3f, 0xff, 0xff);
> +TCC_MODEL_TEMP_MASKS(sapphirerapids, 0x3f, 0x7f, 0xff);
> +
> +/* Use these masks for processors not included in @tcc_cpu_ids. */
> +static struct temp_masks intel_tcc_temp_masks __ro_after_init = {
> +       .tcc_offset = 0x7f,
> +       .digital_readout = 0xff,
> +       .pkg_digital_readout = 0xff,
> +};
> +
> +static const struct x86_cpu_id intel_tcc_cpu_ids[] __initconst = {
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE_YONAH,          &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_MEROM,         &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_MEROM_L,       &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_PENRYN,        &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_DUNNINGTON,    &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM,             &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_G,           &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_EP,          &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_EX,          &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(WESTMERE,            &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(WESTMERE_EP,         &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(WESTMERE_EX,         &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE,         &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE_X,       &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE,           &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X,         &temp_haswell
> _x),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL,             &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X,           &temp_haswell
> _x),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL_L,           &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL_G,           &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL,           &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_G,         &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X,         &temp_haswell
> _x),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_D,         &temp_haswell
> _x),
> +       X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L,           &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE,             &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X,           &temp_haswell
> _x),
> +       X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE_L,          &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE,            &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE,           &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE_L,         &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(CANNONLAKE_L,        &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,           &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,           &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE,             &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_L,           &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_NNPI,        &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ROCKETLAKE,          &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE_L,         &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE,           &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X,    &temp_sapphir
> erapids),
> +       X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X,     &temp_sapphir
> erapids),
> +       X86_MATCH_INTEL_FAM6_MODEL(LAKEFIELD,           &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE,           &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L,         &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE,          &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P,        &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S,        &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_BONNELL,        &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_BONNELL_MID,    &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL,       &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL_MID,   &temp_nehalem
> ),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT,     &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_D,   &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID, &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT,        &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT_MID,    &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT_NP,     &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT,       &temp_goldmon
> t),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT_D,     &temp_goldmon
> t),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT_PLUS,  &temp_goldmon
> t),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,      &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT,        &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_L,      &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GRACEMONT,      &temp_tigerla
> ke),
> +       X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL,        &temp_broadwe
> ll),
> +       X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM,        &temp_broadwe
> ll),
> +       {}
> +};
> +
> +static int __init intel_tcc_init(void)
> +{
> +       const struct x86_cpu_id *id;
> +
> +       id = x86_match_cpu(intel_tcc_cpu_ids);
> +       if (id)
> +               memcpy(&intel_tcc_temp_masks, (const void *)id-
> >driver_data,
> +                      sizeof(intel_tcc_temp_masks));
> +
> +       return 0;
> +}
> +/*
> + * Use subsys_initcall to ensure temperature bitmasks are
> initialized before
> + * the drivers that use this library.
> + */
> +subsys_initcall(intel_tcc_init);
> +
> +/**
> + * intel_tcc_get_offset_mask() - Returns the bitmask to read TCC
> offset
> + *
> + * Get the model-specific bitmask to extract TCC_OFFSET from the MSR
> + * TEMPERATURE_TARGET register. If the mask is 0, it means the
> processor does
> + * not support TCC offset.
> + *
> + * Return: The model-specific bitmask for TCC offset.
> + */
> +u32 intel_tcc_get_offset_mask(void)
> +{
> +       return intel_tcc_temp_masks.tcc_offset;
> +}
> +EXPORT_SYMBOL_NS(intel_tcc_get_offset_mask, INTEL_TCC);
> +
> +/**
> + * get_temp_mask() - Returns the model-specific bitmask for
> temperature
> + *
> + * @pkg: true: Package Thermal Sensor. false: Core Thermal Sensor.
> + *
> + * Get the model-specific bitmask to extract the temperature reading
> from the
> + * MSR_IA32_[PACKAGE]_THERM_STATUS register.
> + *
> + * Callers must check if the thermal status registers are supported.
> + *
> + * Return: The model-specific bitmask for temperature reading
> + */
> +static u32 get_temp_mask(bool pkg)
> +{
> +       return pkg ? intel_tcc_temp_masks.pkg_digital_readout :
> +              intel_tcc_temp_masks.digital_readout;
> +}
> +
>  /**
>   * intel_tcc_get_tjmax() - returns the default TCC activation
> Temperature
>   * @cpu: cpu that the MSR should be run on, nagative value means any
> cpu.
> @@ -56,7 +218,7 @@ int intel_tcc_get_offset(int cpu)
>         if (err)
>                 return err;
>  
> -       return (low >> 24) & 0x3f;
> +       return (low >> 24) & intel_tcc_temp_masks.tcc_offset;
>  }
>  EXPORT_SYMBOL_NS_GPL(intel_tcc_get_offset, INTEL_TCC);
>  
> @@ -76,7 +238,10 @@ int intel_tcc_set_offset(int cpu, int offset)
>         u32 low, high;
>         int err;
>  
> -       if (offset < 0 || offset > 0x3f)
> +       if (!intel_tcc_temp_masks.tcc_offset)
> +               return -ENODEV;
> +
> +       if (offset < 0 || offset > intel_tcc_temp_masks.tcc_offset)
>                 return -EINVAL;
>  
>         if (cpu < 0)
> @@ -90,7 +255,7 @@ int intel_tcc_set_offset(int cpu, int offset)
>         if (low & BIT(31))
>                 return -EPERM;
>  
> -       low &= ~(0x3f << 24);
> +       low &= ~(intel_tcc_temp_masks.tcc_offset << 24);
>         low |= offset << 24;
>  
>         if (cpu < 0)
> @@ -113,8 +278,8 @@ EXPORT_SYMBOL_NS_GPL(intel_tcc_set_offset,
> INTEL_TCC);
>   */
>  int intel_tcc_get_temp(int cpu, int *temp, bool pkg)
>  {
> -       u32 low, high;
>         u32 msr = pkg ? MSR_IA32_PACKAGE_THERM_STATUS :
> MSR_IA32_THERM_STATUS;
> +       u32 low, high, mask;
>         int tjmax, err;
>  
>         tjmax = intel_tcc_get_tjmax(cpu);
> @@ -132,7 +297,9 @@ int intel_tcc_get_temp(int cpu, int *temp, bool
> pkg)
>         if (!(low & BIT(31)))
>                 return -ENODATA;
>  
> -       *temp = tjmax - ((low >> 16) & 0x7f);
> +       mask = get_temp_mask(pkg);
> +
> +       *temp = tjmax - ((low >> 16) & mask);
>  
>         return 0;
>  }
> diff --git a/include/linux/intel_tcc.h b/include/linux/intel_tcc.h
> index 8ff8eabb4a98..fa788817acfc 100644
> --- a/include/linux/intel_tcc.h
> +++ b/include/linux/intel_tcc.h
> @@ -14,5 +14,6 @@ int intel_tcc_get_tjmax(int cpu);
>  int intel_tcc_get_offset(int cpu);
>  int intel_tcc_set_offset(int cpu, int offset);
>  int intel_tcc_get_temp(int cpu, int *temp, bool pkg);
> +u32 intel_tcc_get_offset_mask(void);
>  
>  #endif /* __INTEL_TCC_H__ */

Re: [PATCH v2 1/3] thermal: intel: intel_tcc: Add model checks for temperature registers

[Rafael J. Wysocki]

On Thu, Apr 25, 2024 at 7:06 PM Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
>
> The register MSR_TEMPERATURE_TARGET is not architectural. Its fields may be
> defined differently for each processor model. TCC_OFFSET is an example of
> such case.
>
> Despite being specified as architectural, the registers IA32_[PACKAGE]_
> THERM_STATUS have become model-specific: in recent processors, the
> digital temperature readout uses bits [23:16] whereas the Intel Software
> Developer's manual specifies bits [22:16].
>
> Create an array of processor models and their bitmasks for TCC_OFFSET and
> the digital temperature readout fields. Do not include recent processors.
> Instead, use the bitmasks of these recent processors as default.
>
> Use these model-specific bitmasks when reading TCC_OFFSET or the
> temperature sensors.
>
> Initialize a model-specific data structure during subsys_initcall() to
> have it ready when thermal drivers are loaded.
>
> Expose the new interface intel_tcc_get_offset_mask(). The
> intel_tcc_cooling driver will use it.
>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Lukasz Luba <lukasz.luba@arm.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: linux-hwmon@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: stable@vger.kernel.org # v6.7+
> ---
> Changes since v1:
>  * Renamed TCC_FAM6_MODEL_TEMP_MASKS as TCC_MODEL_TEMP_MASKS. (Rui)
>  * Renamed get_tcc_offset_mask() as intel_tcc_get_offset_mask(). (Rui)
>  * Do not export intel_tcc_get_temp_mask() as it is no longer used
>    outside intel_tcc.c
>  * Dropped stub functions for digital temperature readout and TCC
>    offset. They are not needed as users select CONFIG_INTEL_TCC.
> ---
>  drivers/thermal/intel/intel_tcc.c | 177 +++++++++++++++++++++++++++++-
>  include/linux/intel_tcc.h         |   1 +
>  2 files changed, 173 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thermal/intel/intel_tcc.c b/drivers/thermal/intel/intel_tcc.c
> index 5e8b7f34b395..9943c43c06df 100644
> --- a/drivers/thermal/intel/intel_tcc.c
> +++ b/drivers/thermal/intel/intel_tcc.c
> @@ -6,8 +6,170 @@
>
>  #include <linux/errno.h>
>  #include <linux/intel_tcc.h>
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
>  #include <asm/msr.h>
>
> +/**
> + * struct temp_masks - Bitmasks for temperature readings
> + * @tcc_offset:                        TCC offset in MSR_TEMPERATURE_TARGET
> + * @digital_readout:           Digital readout in MSR_IA32_THERM_STATUS
> + * @pkg_digital_readout:       Digital readout in MSR_IA32_PACKAGE_THERM_STATUS
> + *
> + * Bitmasks to extract the fields of the MSR_TEMPERATURE and IA32_[PACKAGE]_
> + * THERM_STATUS registers for different processor models.
> + *
> + * The bitmask of TjMax is not included in this structure. It is always 0xff.
> + */
> +struct temp_masks {
> +       u32 tcc_offset;
> +       u32 digital_readout;
> +       u32 pkg_digital_readout;
> +};
> +
> +#define TCC_MODEL_TEMP_MASKS(model, _tcc_offset, _digital_readout,     \
> +                            _pkg_digital_readout)                      \
> +       static const struct temp_masks temp_##model __initconst = {     \
> +               .tcc_offset = _tcc_offset,                              \
> +               .digital_readout = _digital_readout,                    \
> +               .pkg_digital_readout = _pkg_digital_readout             \
> +       }
> +
> +TCC_MODEL_TEMP_MASKS(nehalem, 0, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(haswell_x, 0xf, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(broadwell, 0x3f, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(goldmont, 0x7f, 0x7f, 0x7f);
> +TCC_MODEL_TEMP_MASKS(tigerlake, 0x3f, 0xff, 0xff);
> +TCC_MODEL_TEMP_MASKS(sapphirerapids, 0x3f, 0x7f, 0xff);
> +
> +/* Use these masks for processors not included in @tcc_cpu_ids. */
> +static struct temp_masks intel_tcc_temp_masks __ro_after_init = {
> +       .tcc_offset = 0x7f,
> +       .digital_readout = 0xff,
> +       .pkg_digital_readout = 0xff,
> +};
> +
> +static const struct x86_cpu_id intel_tcc_cpu_ids[] __initconst = {
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE_YONAH,          &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_MEROM,         &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_MEROM_L,       &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_PENRYN,        &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(CORE2_DUNNINGTON,    &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM,             &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_G,           &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_EP,          &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(NEHALEM_EX,          &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(WESTMERE,            &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(WESTMERE_EP,         &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(WESTMERE_EX,         &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE,         &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(SANDYBRIDGE_X,       &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE,           &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X,         &temp_haswell_x),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL,             &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X,           &temp_haswell_x),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL_L,           &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(HASWELL_G,           &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL,           &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_G,         &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X,         &temp_haswell_x),
> +       X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_D,         &temp_haswell_x),
> +       X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L,           &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE,             &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X,           &temp_haswell_x),
> +       X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE_L,          &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE,            &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE,           &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(COMETLAKE_L,         &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(CANNONLAKE_L,        &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,           &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,           &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE,             &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_L,           &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_NNPI,        &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ROCKETLAKE,          &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE_L,         &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(TIGERLAKE,           &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X,    &temp_sapphirerapids),
> +       X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X,     &temp_sapphirerapids),
> +       X86_MATCH_INTEL_FAM6_MODEL(LAKEFIELD,           &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE,           &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L,         &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE,          &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P,        &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S,        &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_BONNELL,        &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_BONNELL_MID,    &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL,       &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SALTWELL_MID,   &temp_nehalem),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT,     &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_D,   &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID, &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT,        &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT_MID,    &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT_NP,     &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT,       &temp_goldmont),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT_D,     &temp_goldmont),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT_PLUS,  &temp_goldmont),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_D,      &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT,        &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_TREMONT_L,      &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(ATOM_GRACEMONT,      &temp_tigerlake),
> +       X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL,        &temp_broadwell),
> +       X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM,        &temp_broadwell),
> +       {}
> +};
> +
> +static int __init intel_tcc_init(void)
> +{
> +       const struct x86_cpu_id *id;
> +
> +       id = x86_match_cpu(intel_tcc_cpu_ids);
> +       if (id)
> +               memcpy(&intel_tcc_temp_masks, (const void *)id->driver_data,
> +                      sizeof(intel_tcc_temp_masks));
> +
> +       return 0;
> +}
> +/*
> + * Use subsys_initcall to ensure temperature bitmasks are initialized before
> + * the drivers that use this library.
> + */
> +subsys_initcall(intel_tcc_init);
> +
> +/**
> + * intel_tcc_get_offset_mask() - Returns the bitmask to read TCC offset
> + *
> + * Get the model-specific bitmask to extract TCC_OFFSET from the MSR
> + * TEMPERATURE_TARGET register. If the mask is 0, it means the processor does
> + * not support TCC offset.
> + *
> + * Return: The model-specific bitmask for TCC offset.
> + */
> +u32 intel_tcc_get_offset_mask(void)
> +{
> +       return intel_tcc_temp_masks.tcc_offset;
> +}
> +EXPORT_SYMBOL_NS(intel_tcc_get_offset_mask, INTEL_TCC);
> +
> +/**
> + * get_temp_mask() - Returns the model-specific bitmask for temperature
> + *
> + * @pkg: true: Package Thermal Sensor. false: Core Thermal Sensor.
> + *
> + * Get the model-specific bitmask to extract the temperature reading from the
> + * MSR_IA32_[PACKAGE]_THERM_STATUS register.
> + *
> + * Callers must check if the thermal status registers are supported.
> + *
> + * Return: The model-specific bitmask for temperature reading
> + */
> +static u32 get_temp_mask(bool pkg)
> +{
> +       return pkg ? intel_tcc_temp_masks.pkg_digital_readout :
> +              intel_tcc_temp_masks.digital_readout;
> +}
> +
>  /**
>   * intel_tcc_get_tjmax() - returns the default TCC activation Temperature
>   * @cpu: cpu that the MSR should be run on, nagative value means any cpu.
> @@ -56,7 +218,7 @@ int intel_tcc_get_offset(int cpu)
>         if (err)
>                 return err;
>
> -       return (low >> 24) & 0x3f;
> +       return (low >> 24) & intel_tcc_temp_masks.tcc_offset;
>  }
>  EXPORT_SYMBOL_NS_GPL(intel_tcc_get_offset, INTEL_TCC);
>
> @@ -76,7 +238,10 @@ int intel_tcc_set_offset(int cpu, int offset)
>         u32 low, high;
>         int err;
>
> -       if (offset < 0 || offset > 0x3f)
> +       if (!intel_tcc_temp_masks.tcc_offset)
> +               return -ENODEV;
> +
> +       if (offset < 0 || offset > intel_tcc_temp_masks.tcc_offset)
>                 return -EINVAL;
>
>         if (cpu < 0)
> @@ -90,7 +255,7 @@ int intel_tcc_set_offset(int cpu, int offset)
>         if (low & BIT(31))
>                 return -EPERM;
>
> -       low &= ~(0x3f << 24);
> +       low &= ~(intel_tcc_temp_masks.tcc_offset << 24);
>         low |= offset << 24;
>
>         if (cpu < 0)
> @@ -113,8 +278,8 @@ EXPORT_SYMBOL_NS_GPL(intel_tcc_set_offset, INTEL_TCC);
>   */
>  int intel_tcc_get_temp(int cpu, int *temp, bool pkg)
>  {
> -       u32 low, high;
>         u32 msr = pkg ? MSR_IA32_PACKAGE_THERM_STATUS : MSR_IA32_THERM_STATUS;
> +       u32 low, high, mask;
>         int tjmax, err;
>
>         tjmax = intel_tcc_get_tjmax(cpu);
> @@ -132,7 +297,9 @@ int intel_tcc_get_temp(int cpu, int *temp, bool pkg)
>         if (!(low & BIT(31)))
>                 return -ENODATA;
>
> -       *temp = tjmax - ((low >> 16) & 0x7f);
> +       mask = get_temp_mask(pkg);
> +
> +       *temp = tjmax - ((low >> 16) & mask);
>
>         return 0;
>  }
> diff --git a/include/linux/intel_tcc.h b/include/linux/intel_tcc.h
> index 8ff8eabb4a98..fa788817acfc 100644
> --- a/include/linux/intel_tcc.h
> +++ b/include/linux/intel_tcc.h
> @@ -14,5 +14,6 @@ int intel_tcc_get_tjmax(int cpu);
>  int intel_tcc_get_offset(int cpu);
>  int intel_tcc_set_offset(int cpu, int offset);
>  int intel_tcc_get_temp(int cpu, int *temp, bool pkg);
> +u32 intel_tcc_get_offset_mask(void);
>
>  #endif /* __INTEL_TCC_H__ */
> --

This clashes with the Tony Luck's rework of Intel CPU model defines
(see https://lore.kernel.org/lkml/20240430164913.73473-1-tony.luck@intel.com/),
so I'd rather defer it until commit 2eda374e883a ("x86/mm: Switch to
new Intel CPU model defines") reaches the mainline and make it use the
new CPU model defines from the start.

Re: [PATCH v2 1/3] thermal: intel: intel_tcc: Add model checks for temperature registers

[Ricardo Neri]

On Tue, Apr 30, 2024 at 09:07:39PM +0200, Rafael J. Wysocki wrote:
> 
> This clashes with the Tony Luck's rework of Intel CPU model defines
> (see https://lore.kernel.org/lkml/20240430164913.73473-1-tony.luck@intel.com/),
> so I'd rather defer it until commit 2eda374e883a ("x86/mm: Switch to
> new Intel CPU model defines") reaches the mainline and make it use the
> new CPU model defines from the start.

Sounds good to me. I can repost the remaining patches of this series once
Tony's rework is merged into mainline.

[PATCH v2 2/3] thermal: intel: intel_tcc_cooling: Use a model-specific bitmask for TCC offset

[Ricardo Neri]

The TCC offset field in the register MSR_TEMPERATURE_TARGET is not
architectural. The TCC library provides a model-specific bitmask. Use it to
determine the maximum TCC offset.

Suggested-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Lukasz Luba <lukasz.luba@arm.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org # v6.7+
---
Changes since v1:
 * Used renamed function intel_tcc_get_offset_mask().
---
 drivers/thermal/intel/intel_tcc_cooling.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/intel/intel_tcc_cooling.c b/drivers/thermal/intel/intel_tcc_cooling.c
index 6c392147e6d1..5bfc2b515c78 100644
--- a/drivers/thermal/intel/intel_tcc_cooling.c
+++ b/drivers/thermal/intel/intel_tcc_cooling.c
@@ -20,7 +20,7 @@ static struct thermal_cooling_device *tcc_cdev;
 static int tcc_get_max_state(struct thermal_cooling_device *cdev, unsigned long
 			     *state)
 {
-	*state = 0x3f;
+	*state = intel_tcc_get_offset_mask();
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH v2 2/3] thermal: intel: intel_tcc_cooling: Use a model-specific bitmask for TCC offset

[Zhang, Rui]

On Thu, 2024-04-25 at 10:13 -0700, Ricardo Neri wrote:
> The TCC offset field in the register MSR_TEMPERATURE_TARGET is not
> architectural. The TCC library provides a model-specific bitmask. Use
> it to
> determine the maximum TCC offset.
> 
> Suggested-by: Zhang Rui <rui.zhang@intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Reviewed-by: Zhang Rui <rui.zhang@intel.com>

-rui

> ---
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Lukasz Luba <lukasz.luba@arm.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: linux-hwmon@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: stable@vger.kernel.org # v6.7+
> ---
> Changes since v1:
>  * Used renamed function intel_tcc_get_offset_mask().
> ---
>  drivers/thermal/intel/intel_tcc_cooling.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/intel/intel_tcc_cooling.c
> b/drivers/thermal/intel/intel_tcc_cooling.c
> index 6c392147e6d1..5bfc2b515c78 100644
> --- a/drivers/thermal/intel/intel_tcc_cooling.c
> +++ b/drivers/thermal/intel/intel_tcc_cooling.c
> @@ -20,7 +20,7 @@ static struct thermal_cooling_device *tcc_cdev;
>  static int tcc_get_max_state(struct thermal_cooling_device *cdev,
> unsigned long
>                              *state)
>  {
> -       *state = 0x3f;
> +       *state = intel_tcc_get_offset_mask();
>         return 0;
>  }
>  

[PATCH v2 3/3] hwmon: (coretemp) Extend the bitmask to read temperature to 0xff

[Ricardo Neri]

The Intel Software Development manual defines the temperature digital
readout as the bits [22:16] of the IA32_[PACKAGE]_THERM_STATUS registers.
Bit 23 is specified as reserved.

In recent processors, however, the temperature digital readout uses bits
[23:16]. In those processors, using the bitmask 0x7f would lead to
incorrect readings if the temperature deviates from TjMax by more than
127 degrees Celsius.

Although not guaranteed, bit 23 is likely to be 0 in processors from a few
generations ago. The temperature reading would still be correct in those
processors when using a 0xff bitmask.

Model-specific provisions can be made for older processors in which bit 23
is not 0 should the need arise.

Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Lukasz Luba <lukasz.luba@arm.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org # v6.7+
---
Changes since v1:
 * Corrected wrong sentence in commit message. (Rui)
 * Removed dependency on INTEL_TCC. (Guenter)
---
 drivers/hwmon/coretemp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 616bd1a5b864..1b9203b20d70 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -411,7 +411,7 @@ static ssize_t show_temp(struct device *dev,
 		 * Return it instead of reporting an error which doesn't
 		 * really help at all.
 		 */
-		tdata->temp = tjmax - ((eax >> 16) & 0x7f) * 1000;
+		tdata->temp = tjmax - ((eax >> 16) & 0xff) * 1000;
 		tdata->last_updated = jiffies;
 	}
 
-- 
2.34.1

Re: [PATCH v2 3/3] hwmon: (coretemp) Extend the bitmask to read temperature to 0xff

[Guenter Roeck]

On Thu, Apr 25, 2024 at 10:13:11AM -0700, Ricardo Neri wrote:
> The Intel Software Development manual defines the temperature digital
> readout as the bits [22:16] of the IA32_[PACKAGE]_THERM_STATUS registers.
> Bit 23 is specified as reserved.
> 
> In recent processors, however, the temperature digital readout uses bits
> [23:16]. In those processors, using the bitmask 0x7f would lead to
> incorrect readings if the temperature deviates from TjMax by more than
> 127 degrees Celsius.
> 
> Although not guaranteed, bit 23 is likely to be 0 in processors from a few
> generations ago. The temperature reading would still be correct in those
> processors when using a 0xff bitmask.
> 
> Model-specific provisions can be made for older processors in which bit 23
> is not 0 should the need arise.
> 
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Applied.

Thanks,
Guenter