[PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Sohil Mehta]

From: Dave Hansen <dave.hansen@linux.intel.com>

Intel CPUs have been using Family 6 for a while. The Family-model checks
in the coretemp driver implicitly assume Family 6. With the upcoming
Family 18 and 19 models, some of these checks fall apart.

While reading the temperature target MSR, cpu_has_tjmax() performs model
checks only to determine if a device warning should be printed. Instead
of expanding the checks, get rid of the function and print the warning
once unconditionally if the MSR read fails. The checks aren't worth
preventing a single line warning to dmesg.

Update the rest of the x86_model checks with VFM ones to make them more
robust. This automatically covers the upcoming Family 18 and 19 as well
as any future extended families.

Add a code comment to reflect that none of the CPUs in Family 5 or
Family 15 set X86_FEATURE_DTHERM. The VFM checks do not impact these
CPUs since the driver does not load on them.

Missing-signoff: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
v3:
 - Use the new VFM checks instead of the raw x86_model comparisons.
 - Simplify the MSR read warning and get rid of cpu_has_tjmax()
 - Posting this patch separately since the new family cleanup series
   was merged without it.
 - Setting Dave as the author since he provided most of code.

v2: https://lore.kernel.org/lkml/20250211194407.2577252-8-sohil.mehta@intel.com/
---
 drivers/hwmon/coretemp.c | 76 +++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 40 deletions(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 1b9203b20d70..ad79db5a183e 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -122,29 +122,29 @@ static const struct tjmax tjmax_table[] = {
 };
 
 struct tjmax_model {
-	u8 model;
-	u8 mask;
+	u32 vfm;
+	u8 stepping_mask;
 	int tjmax;
 };
 
 #define ANY 0xff
 
 static const struct tjmax_model tjmax_model_table[] = {
-	{ 0x1c, 10, 100000 },	/* D4xx, K4xx, N4xx, D5xx, K5xx, N5xx */
-	{ 0x1c, ANY, 90000 },	/* Z5xx, N2xx, possibly others
-				 * Note: Also matches 230 and 330,
-				 * which are covered by tjmax_table
-				 */
-	{ 0x26, ANY, 90000 },	/* Atom Tunnel Creek (Exx), Lincroft (Z6xx)
-				 * Note: TjMax for E6xxT is 110C, but CPU type
-				 * is undetectable by software
-				 */
-	{ 0x27, ANY, 90000 },	/* Atom Medfield (Z2460) */
-	{ 0x35, ANY, 90000 },	/* Atom Clover Trail/Cloverview (Z27x0) */
-	{ 0x36, ANY, 100000 },	/* Atom Cedar Trail/Cedarview (N2xxx, D2xxx)
-				 * Also matches S12x0 (stepping 9), covered by
-				 * PCI table
-				 */
+	{ INTEL_ATOM_BONNELL,	      10,  100000 },	/* D4xx, K4xx, N4xx, D5xx, K5xx, N5xx */
+	{ INTEL_ATOM_BONNELL,	      ANY, 90000 },	/* Z5xx, N2xx, possibly others
+							 * Note: Also matches 230 and 330,
+							 * which are covered by tjmax_table
+							 */
+	{ INTEL_ATOM_BONNELL_MID,     ANY, 90000 },	/* Atom Tunnel Creek (Exx), Lincroft (Z6xx)
+							 * Note: TjMax for E6xxT is 110C, but CPU type
+							 * is undetectable by software
+							 */
+	{ INTEL_ATOM_SALTWELL_MID,    ANY, 90000 },	/* Atom Medfield (Z2460) */
+	{ INTEL_ATOM_SALTWELL_TABLET, ANY, 90000 },	/* Atom Clover Trail/Cloverview (Z27x0) */
+	{ INTEL_ATOM_SALTWELL,	      ANY, 100000 },	/* Atom Cedar Trail/Cedarview (N2xxx, D2xxx)
+							 * Also matches S12x0 (stepping 9), covered by
+							 * PCI table
+							 */
 };
 
 static bool is_pkg_temp_data(struct temp_data *tdata)
@@ -180,6 +180,11 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
 	}
 	pci_dev_put(host_bridge);
 
+	/*
+	 * This is literally looking for "CPU  XXX" in the model string.
+	 * Not checking it against the model as well. Just purely a
+	 * string search.
+	 */
 	for (i = 0; i < ARRAY_SIZE(tjmax_table); i++) {
 		if (strstr(c->x86_model_id, tjmax_table[i].id))
 			return tjmax_table[i].tjmax;
@@ -187,17 +192,18 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
 
 	for (i = 0; i < ARRAY_SIZE(tjmax_model_table); i++) {
 		const struct tjmax_model *tm = &tjmax_model_table[i];
-		if (c->x86_model == tm->model &&
-		    (tm->mask == ANY || c->x86_stepping == tm->mask))
+		if (c->x86_vfm == tm->vfm &&
+		    (tm->stepping_mask == ANY ||
+		     tm->stepping_mask == c->x86_stepping))
 			return tm->tjmax;
 	}
 
 	/* Early chips have no MSR for TjMax */
 
-	if (c->x86_model == 0xf && c->x86_stepping < 4)
+	if (c->x86_vfm == INTEL_CORE2_MEROM && c->x86_stepping < 4)
 		usemsr_ee = 0;
 
-	if (c->x86_model > 0xe && usemsr_ee) {
+	if (c->x86_vfm > INTEL_CORE_YONAH && usemsr_ee) {
 		u8 platform_id;
 
 		/*
@@ -211,7 +217,8 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
 				 "Unable to access MSR 0x17, assuming desktop"
 				 " CPU\n");
 			usemsr_ee = 0;
-		} else if (c->x86_model < 0x17 && !(eax & 0x10000000)) {
+		} else if (c->x86_vfm < INTEL_CORE2_PENRYN &&
+			   !(eax & 0x10000000)) {
 			/*
 			 * Trust bit 28 up to Penryn, I could not find any
 			 * documentation on that; if you happen to know
@@ -226,7 +233,7 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
 			 * Mobile Penryn CPU seems to be platform ID 7 or 5
 			 * (guesswork)
 			 */
-			if (c->x86_model == 0x17 &&
+			if (c->x86_vfm == INTEL_CORE2_PENRYN &&
 			    (platform_id == 5 || platform_id == 7)) {
 				/*
 				 * If MSR EE bit is set, set it to 90 degrees C,
@@ -258,18 +265,6 @@ static int adjust_tjmax(struct cpuinfo_x86 *c, u32 id, struct device *dev)
 	return tjmax;
 }
 
-static bool cpu_has_tjmax(struct cpuinfo_x86 *c)
-{
-	u8 model = c->x86_model;
-
-	return model > 0xe &&
-	       model != 0x1c &&
-	       model != 0x26 &&
-	       model != 0x27 &&
-	       model != 0x35 &&
-	       model != 0x36;
-}
-
 static int get_tjmax(struct temp_data *tdata, struct device *dev)
 {
 	struct cpuinfo_x86 *c = &cpu_data(tdata->cpu);
@@ -287,8 +282,7 @@ static int get_tjmax(struct temp_data *tdata, struct device *dev)
 	 */
 	err = rdmsr_safe_on_cpu(tdata->cpu, MSR_IA32_TEMPERATURE_TARGET, &eax, &edx);
 	if (err) {
-		if (cpu_has_tjmax(c))
-			dev_warn(dev, "Unable to read TjMax from CPU %u\n", tdata->cpu);
+		dev_warn_once(dev, "Unable to read TjMax from CPU %u\n", tdata->cpu);
 	} else {
 		val = (eax >> 16) & 0xff;
 		if (val)
@@ -460,7 +454,7 @@ static int chk_ucode_version(unsigned int cpu)
 	 * Readings might stop update when processor visited too deep sleep,
 	 * fixed for stepping D0 (6EC).
 	 */
-	if (c->x86_model == 0xe && c->x86_stepping < 0xc && c->microcode < 0x39) {
+	if (c->x86_vfm == INTEL_CORE_YONAH && c->x86_stepping < 0xc && c->microcode < 0x39) {
 		pr_err("Errata AE18 not fixed, update BIOS or microcode of the CPU!\n");
 		return -ENODEV;
 	}
@@ -580,7 +574,7 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
 	 * MSR_IA32_TEMPERATURE_TARGET register. Atoms don't have the register
 	 * at all.
 	 */
-	if (c->x86_model > 0xe && c->x86_model != 0x1c)
+	if (c->x86_vfm > INTEL_CORE_YONAH && c->x86_vfm != INTEL_ATOM_BONNELL)
 		if (get_ttarget(tdata, &pdev->dev) >= 0)
 			tdata->attr_size++;
 
@@ -793,7 +787,9 @@ static int __init coretemp_init(void)
 	/*
 	 * CPUID.06H.EAX[0] indicates whether the CPU has thermal
 	 * sensors. We check this bit only, all the early CPUs
-	 * without thermal sensors will be filtered out.
+	 * without thermal sensors will be filtered out. This
+	 * includes all the Family 5 and Family 15 (Pentium 4)
+	 * models, since they never set the CPUID bit.
 	 */
 	if (!x86_match_cpu(coretemp_ids))
 		return -ENODEV;
-- 
2.43.0

Re: [PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Dave Hansen]

On 8/28/25 13:17, Sohil Mehta wrote:
> 
> Add a code comment to reflect that none of the CPUs in Family 5 or
> Family 15 set X86_FEATURE_DTHERM. The VFM checks do not impact these
> CPUs since the driver does not load on them.
> 
> Missing-signoff: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>

Thanks for picking this back up from whatever dark hole I left it in! ;)

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>

I assume the hwmon folks will pick this up. If not, it's certainly
x86-ish enough for it to go through tip.

Oh, and do we want to cc:stable@ on this? Could this end up biting
anybody running an old kernel on the model 18/19 hardware?

Re: [PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Guenter Roeck]

On Thu, Aug 28, 2025 at 01:17:29PM -0700, Sohil Mehta wrote:
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> Intel CPUs have been using Family 6 for a while. The Family-model checks
> in the coretemp driver implicitly assume Family 6. With the upcoming
> Family 18 and 19 models, some of these checks fall apart.
> 
> While reading the temperature target MSR, cpu_has_tjmax() performs model
> checks only to determine if a device warning should be printed. Instead
> of expanding the checks, get rid of the function and print the warning
> once unconditionally if the MSR read fails. The checks aren't worth
> preventing a single line warning to dmesg.
> 
> Update the rest of the x86_model checks with VFM ones to make them more
> robust. This automatically covers the upcoming Family 18 and 19 as well
> as any future extended families.
> 
> Add a code comment to reflect that none of the CPUs in Family 5 or
> Family 15 set X86_FEATURE_DTHERM. The VFM checks do not impact these
> CPUs since the driver does not load on them.
> 
> Missing-signoff: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>

Checkpatch really doesn't like that:

ERROR: Missing Signed-off-by: line by nominal patch author 'Dave Hansen <dave.hansen@linux.intel.com>'

Never mind, applied anyway.

Guenter

Re: [PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Sohil Mehta]

On 8/28/2025 2:18 PM, Dave Hansen wrote:
> 
> Oh, and do we want to cc:stable@ on this? Could this end up biting
> anybody running an old kernel on the model 18/19 hardware?

I haven't evaluated the exact impact. But, maybe we should cc stable.
The patch applies and builds cleanly on 6.12. But beyond that, it may
need some massaging.

Re: [PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Sohil Mehta]

On 8/28/2025 2:42 PM, Guenter Roeck wrote:
>> Missing-signoff: Dave Hansen <dave.hansen@linux.intel.com>
>> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> 
> Checkpatch really doesn't like that:
> 
> ERROR: Missing Signed-off-by: line by nominal patch author 'Dave Hansen <dave.hansen@linux.intel.com>'
> 
> Never mind, applied anyway.
> 

Dave has provided his signoff on the patch now. Also, he suggested
including a Cc: stable.

Re: [PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Guenter Roeck]

On 8/28/25 14:18, Dave Hansen wrote:
> On 8/28/25 13:17, Sohil Mehta wrote:
>>
>> Add a code comment to reflect that none of the CPUs in Family 5 or
>> Family 15 set X86_FEATURE_DTHERM. The VFM checks do not impact these
>> CPUs since the driver does not load on them.
>>
>> Missing-signoff: Dave Hansen <dave.hansen@linux.intel.com>
>> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> 
> Thanks for picking this back up from whatever dark hole I left it in! ;)
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> 
> I assume the hwmon folks will pick this up. If not, it's certainly
> x86-ish enough for it to go through tip.
> 
Already done, but if you want to take it through some other branch
let me know and I'll drop it.

> Oh, and do we want to cc:stable@ on this? Could this end up biting
> anybody running an old kernel on the model 18/19 hardware?

It doesn't really bite, it will just not instantiate the driver.
I have no idea if those old kernels would run with the new hardware
in the first place. You tell me...

Personally I'd rather wait until someone complains.

Guenter

Re: [PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Dave Hansen]

On 8/28/25 14:45, Sohil Mehta wrote:
> On 8/28/2025 2:42 PM, Guenter Roeck wrote:
...
> Dave has provided his signoff on the patch now. Also, he suggested
> including a Cc: stable.

Let's just wait until Guenter sends it upstream. Once it hits Linus's
tree, you can ask for it to be in stable if we decide it's a good idea.

I asked if it was necessary because I'm not positive it's a good idea.

For instance, if the model numbers in play were all >100 and Intel has
zero plans to introduce a family 18/19, model >100, then it might not be
worth it. Or, if the only downside is a single warning on dmesg, it
might not be worth it.

But, if it's going to spew warnings constantly or set your brand new
machine ablaze, then maybe it's worth backporting.

So, let's actually look at what it would mean in practice to have it hit
stable@ or not. Just spent 10 minutes looking at it.

Re: [PATCH v3] hwmon: (coretemp) Replace x86_model checks with VFM ones

[Sohil Mehta]

On 8/28/2025 2:51 PM, Dave Hansen wrote:
> Let's just wait until Guenter sends it upstream. Once it hits Linus's
> tree, you can ask for it to be in stable if we decide it's a good idea.
> 

Sure, sounds good.

> I asked if it was necessary because I'm not positive it's a good idea.
> 

The model numbers in play are around 14, 15, 23. In some cases, the
model number is used to determine if a sysfs attribute should be
created. So, for lower model numbers, the TTARGET attr probably won't be
created.

	if (c->x86_model > 0xe && c->x86_model != 0x1c)
		if (get_ttarget(tdata, &pdev->dev) >= 0)
			tdata->attr_size++;

	/* Create sysfs interfaces */
	err = create_core_attrs(tdata, pdata->hwmon_dev);

In some other cases, the temperature adjustment would vary slightly.
But, nothing scary for now.

> So, let's actually look at what it would mean in practice to have it hit
> stable@ or not. Just spent 10 minutes looking at it.

Yeah, I am fine with deciding based on the severity of the practical impact.