Skip to contentSkip to conversation

coretemp add more safety checks

PATCH In review Rudolf Marek 2 participants 4 messages Apr 29, 2007 – May 13, 2007

Subsystems (2)
Hardware monitoring Coretemp hardware monitoring driver
lore
Share & tools
lore

Rudolf Marek

author
#0 lore raw

PATCH 1/1 coretemp add more safety checks

Hello all,

I hope I will have more time from now, here my patch. It adds check for Errata AE18 of core processors, which causes values not to update after the CPU deep sleeps. Intel claims that the TjMax detection might not work for the Core2, warn the users about this.

I would like to ask anyone using coretemp on notebook to test this patch!

In case it is ok (haha) here is the kernel stuff:

Signed-off-by Rudolf Marek <[email protected]>

This patch adds detection of AE18 Errata of Core processor and warns users that the absolute readings might be wrong for Core2 users.

Rudolf

drivers/hwmon/coretemp.c +26 -3
Index: linux-2.6.21-rc3/drivers/hwmon/coretemp.c
===================================================================
@@ -176,6 +176,22 @@
 		goto exit_free;
 	}
 
+	/* Check if we have problem with errata AE18 of Core processors:
+	   Readings might stop update when precessor visited too deep sleep,
+	   fixed for stepping D0 (6EC).
+	*/
+
+	if ((c->x86_model == 0xe) && (c->x86_mask < 0xc)) {
+		/* check for microcode update */
+		rdmsr_on_cpu(data->id, MSR_IA32_UCODE_REV, &eax, &edx);
+		if (edx < 0x39) {
+			dev_err(&pdev->dev,
+				"Errata AE18 not fixed, update BIOS or "
+				"microcode of the CPU!\n");
+			goto exit_free;
+		}
+	}
+
 	/* Some processors have Tjmax 85 following magic should detect it
 	   Intel won't disclose the information without signed NDA, but
 	   individuals cannot sign it. Catch(ed) 22.

@@ -193,6 +209,16 @@
 		}
 	}
 
+	/* Intel says that above should not work for Core2 processors,
+	   but it seems to work. There is no other way how get the absolute
+	   readings. Warn the user about this.
+	*/
+
+	if (((c->x86_model == 0xf) {
+		dev_warn(&pdev->dev, "Using undocumented features, absolute "
+			 "temperature might be wrong!\n");
+	}
+
 	platform_set_drvdata(pdev, data);
 
 	if ((err = sysfs_create_group(&pdev->dev.kobj, &coretemp_group)))

@@ -328,9 +354,6 @@
 	int i, err = -ENODEV;
 	struct pdev_entry *p, *n;
 
-	printk(KERN_NOTICE DRVNAME ": This driver uses undocumented features"
-		" of Core CPU. Temperature might be wrong!\n");
-
 	/* quick check if we run Intel */
 	if (cpu_data[0].x86_vendor != X86_VENDOR_INTEL)
 		goto exit;

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#1 lore raw

Hi Rudolf,

Quoted 5 lines · Rudolf Marek

I hope I will have more time from now, here my patch. It adds check for Errata AE18 of core processors, which causes values not to update after the CPU deep sleeps. Intel claims that the TjMax detection might not work for the Core2, warn the users about this.

Quoted 8 lines

I would like to ask anyone using coretemp on notebook to test this patch!

In case it is ok (haha) here is the kernel stuff:

Signed-off-by Rudolf Marek <[email protected]>

This patch adds detection of AE18 Errata of Core processor and warns users that the absolute readings might be wrong for Core2 users.

Your patch:
* Doesn't apply on top of the version of the coretemp driver I have on my hwmon stack [1].
* Once manually fixed to apply, fails to compile.

[1] http://khali.linux-fr.org/devel/linux-2.6/jdelvare-hwmon/hwmon-coretemp-new-driver.patch

Can you please provide a patch which at least applies and compiles, so that users have a chance to test it? Other than that, it looks good. Please resubmit. Thanks.

Signature · 6 lines
Jean Delvare

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
author
#2 lore raw
In reply to Rudolf Marek

Hello

here comes updated patch, I added a check if certain Core2 are desktop or mobile processors (using yet another not officially documented register - but it should be ok ). They have same CPUID as desktop but are in fact mobile ;)

Everyone, please try the patch with core2 and core notebooks/desktops, I do not have any chance to test it.

Signed-off-by Rudolf Marek <[email protected]>

This patch adds detection of AE18 Errata of Core processor and warns users that the absolute readings might be wrong for Core2 users.

Thanks,

Rudolf

drivers/hwmon/coretemp.c +29 -3
Index: linux-2.6.21/drivers/hwmon/coretemp.c
===================================================================
@@ -176,6 +176,22 @@
 		goto exit_free;
 	}
 
+	/* Check if we have problem with errata AE18 of Core processors:
+	   Readings might stop update when processor visited too deep sleep,
+	   fixed for stepping D0 (6EC).
+	*/
+
+	if ((c->x86_model == 0xe) && (c->x86_mask < 0xc)) {
+		/* check for microcode update */
+		rdmsr_on_cpu(data->id, MSR_IA32_UCODE_REV, &eax, &edx);
+		if (edx < 0x39) {
+			dev_err(&pdev->dev,
+				"Errata AE18 not fixed, update BIOS or "
+				"microcode of the CPU!\n");
+			goto exit_free;
+		}
+	}
+
 	/* Some processors have Tjmax 85 following magic should detect it
 	   Intel won't disclose the information without signed NDA, but
 	   individuals cannot sign it. Catch(ed) 22.

@@ -193,6 +209,19 @@
 		}
 	}
 
+	/* Intel says that above should not work for desktop Core2 processors,
+	   but it seems to work. There is no other way how get the absolute
+	   readings. Warn the user about this. First check if are desktop,
+	   bit 50 of MSR_IA32_PLATFORM_ID should be 0.
+	*/
+
+	rdmsr_safe_on_cpu(data->id, MSR_IA32_PLATFORM_ID, &eax, &edx);
+
+	if ((c->x86_model == 0xf) && (!(edx & 0x00040000))) {
+		dev_warn(&pdev->dev, "Using undocumented features, absolute "
+			 "temperature might be wrong!\n");
+	}
+
 	platform_set_drvdata(pdev, data);
 
 	if ((err = sysfs_create_group(&pdev->dev.kobj, &coretemp_group)))

@@ -328,9 +357,6 @@
 	int i, err = -ENODEV;
 	struct pdev_entry *p, *n;
 
-	printk(KERN_NOTICE DRVNAME ": This driver uses undocumented features "
-		"of Core CPU. Temperature might be wrong!\n");
-
 	/* quick check if we run Intel */
 	if (cpu_data[0].x86_vendor != X86_VENDOR_INTEL)
 		goto exit;

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#3 lore raw
In reply to Rudolf Marek

Hi Rudolf,

Quoted 6 lines · Rudolf Marek

Hello

here comes updated patch, I added a check if certain Core2 are desktop or mobile processors (using yet another not officially documented register - but it should be ok ). They have same CPUID as desktop but are in fact mobile ;)

Quoted 7 lines

Everyone, please try the patch with core2 and core notebooks/desktops, I do not have any chance to test it.

Signed-off-by Rudolf Marek <[email protected]>

This patch adds detection of AE18 Errata of Core processor and warns users that the absolute readings might be wrong for Core2 users.

Tested, it works fine for me (Core 6/14/8). The driver is no longer printing any warning when loaded, and otherwise behaves the same as before.

Patch applied, thanks.

Signature · 6 lines
Jean Delvare

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors