git-current: latest coretemp changes break s2ram

git-current: latest coretemp changes break s2ram

[Soeren Sonnenburg]

this commit makes coretemp fail on my macbook pro.

1) rmmod oopses (see below)
2) it breaks s2ram

Soeren

commit 67f363b1f6a31cf5027a97372f64bcced4f05ba6
Author: Rudolf Marek <r.marek@assembler.cz>
Date:   Sun May 27 22:17:43 2007 +0200

    hwmon/coretemp: Add more safety checks

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

    Signed-off-by: Rudolf Marek <r.marek@assembler.cz>
    Signed-off-by: Jean Delvare <khali@linux-fr.org>

[...]
PM: Adding info for platform:coretemp.0
coretemp coretemp.0: Errata AE18 not fixed, update BIOS or microcode of the CPU!
PM: Adding info for platform:coretemp.1
coretemp coretemp.1: Errata AE18 not fixed, update BIOS or microcode of the CPU!
[...]
BUG: unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
f887c09f
*pde = 00000000
Oops: 0000 [#1]
PREEMPT SMP 
Modules linked in: ohci1394 ieee1394 hfsplus binfmt_misc fuse eeprom coretemp applesmc hwmon snd_hda_intel snd_pcm_oss snd_mixer_oss snd_pcm firewire_ohci firewire_core snd_timer i2c_i801 appletouch sky2 crc_itu_t snd soundcore snd_page_alloc intel_agp agpgart evdev
CPU:    0
EIP:    0060:[<f887c09f>]    Not tainted VLI
EFLAGS: 00010286   (2.6.22-rc4-sonne #20)
EIP is at coretemp_remove+0x1f/0x60 [coretemp]
eax: f78c2800   ebx: f78c2890   ecx: 00000000   edx: f887d23c
esi: f78c2808   edi: 00000000   ebp: c0496760   esp: f7c07ef4
ds: 007b   es: 007b   fs: 00d8  gs: 0033  ss: 0068
Process rmmod (pid: 2646, ti=f7c06000 task=dfc07440 task.ti=f7c06000)
Stack: f78c2808 f887d23c f78c2890 c02b714c c02b580b f78c2808 f78c2808 c02b5c5e 
       f78c2808 c02b50c0 00000000 c02b3508 f78c2800 f78c2800 f7a298f0 f7c06000 
       c02b73f0 f78c2800 f7a298b0 c02b7828 f7a298f0 f887c69a 00000000 f887d380 
Call Trace:
 [<c02b714c>] platform_drv_remove+0xc/0x10
 [<c02b580b>] __device_release_driver+0x6b/0xa0
 [<c02b5c5e>] device_release_driver+0x1e/0x40
 [<c02b50c0>] bus_remove_device+0x50/0x80
 [<c02b3508>] device_del+0x138/0x230
 [<c02b73f0>] platform_device_del+0x10/0x70
 [<c02b7828>] platform_device_unregister+0x8/0x10
 [<f887c69a>] coretemp_exit+0x3a/0x7d [coretemp]
 [<c0150528>] sys_delete_module+0x148/0x1b0
 [<c011f653>] do_page_fault+0x333/0x620
 [<c0169ab7>] do_munmap+0x197/0x1f0
 [<c01041a2>] sysenter_past_esp+0x5f/0x85
 =======================
Code: 87 f8 5e 5f 5d e9 72 6a b4 c7 66 90 83 ec 0c 89 1c 24 89 c3 89 74 24 04 8d 70 08 81 c3 90 00 00 00 89 7c 24 08 8b be 20 01 00 00 <8b> 07 e8 5a 7f fc ff 89 d8 ba e0 c6 87 f8 e8 3e 20 94 c7 31 c0 
EIP: [<f887c09f>] coretemp_remove+0x1f/0x60 [coretemp] SS:ESP 0068:f7c07ef4

-- 
Sometimes, there's a moment as you're waking, when you become aware of
the real world around you, but you're still dreaming.

[PATCH] hwmon/coretemp: Fix a broken error path

[Jean Delvare]

Hi Soeren,

On Sat, 16 Jun 2007 22:43:05 +0200, Soeren Sonnenburg wrote:
> this commit makes coretemp fail on my macbook pro.
> 
> 1) rmmod oopses (see below)
> 2) it breaks s2ram
> 
> Soeren
> 
> commit 67f363b1f6a31cf5027a97372f64bcced4f05ba6
> Author: Rudolf Marek <r.marek@assembler.cz>
> Date:   Sun May 27 22:17:43 2007 +0200
> 
>     hwmon/coretemp: Add more safety checks
> 
>     Add detection of AE18 Errata of Core processor and warns
>     users that the absolute readings might be wrong for Core2 processor.
> 
>     Signed-off-by: Rudolf Marek <r.marek@assembler.cz>Subject: hwmon/coretemp: Fix a broken error path
>     Signed-off-by: Jean Delvare <khali@linux-fr.org>


Thanks for reporting. Indeed this patch is broken, sorry for
overlooking it. I tested it but my hardware is such that the faulty
error path was never taken. Please test the following patch (on top of
git-current) and confirm it fixes your problem:

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Rudolf Marek <r.marek@assembler.cz>
---
 drivers/hwmon/coretemp.c |    1 +
 1 file changed, 1 insertion(+)

--- linux-2.6.22-rc4.orig/drivers/hwmon/coretemp.c	2007-06-05 10:25:54.000000000 +0200
+++ linux-2.6.22-rc4/drivers/hwmon/coretemp.c	2007-06-16 23:14:06.000000000 +0200
@@ -185,6 +185,7 @@ static int __devinit coretemp_probe(stru
 		/* check for microcode update */
 		rdmsr_on_cpu(data->id, MSR_IA32_UCODE_REV, &eax, &edx);
 		if (edx < 0x39) {
+			err = -ENODEV;
 			dev_err(&pdev->dev,
 				"Errata AE18 not fixed, update BIOS or "
 				"microcode of the CPU!\n");


Thanks,
-- 
Jean Delvare

Re: [PATCH] hwmon/coretemp: Fix a broken error path

[Soeren Sonnenburg]

On Sat, 2007-06-16 at 23:17 +0200, Jean Delvare wrote:
> Hi Soeren,

Hi Jean,

[...]

> Thanks for reporting. Indeed this patch is broken, sorry for
> overlooking it. I tested it but my hardware is such that the faulty
> error path was never taken. Please test the following patch (on top of
> git-current) and confirm it fixes your problem:
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Rudolf Marek <r.marek@assembler.cz>
> ---
>  drivers/hwmon/coretemp.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- linux-2.6.22-rc4.orig/drivers/hwmon/coretemp.c	2007-06-05 10:25:54.000000000 +0200
> +++ linux-2.6.22-rc4/drivers/hwmon/coretemp.c	2007-06-16 23:14:06.000000000 +0200
> @@ -185,6 +185,7 @@ static int __devinit coretemp_probe(stru
>  		/* check for microcode update */
>  		rdmsr_on_cpu(data->id, MSR_IA32_UCODE_REV, &eax, &edx);
>  		if (edx < 0x39) {
> +			err = -ENODEV;
>  			dev_err(&pdev->dev,
>  				"Errata AE18 not fixed, update BIOS or "
>  				"microcode of the CPU!\n");

this patch indeed fixes the problem. Thanks!

Unfortunately the coretemp sensors are simply never there when that
patch is applied... which I guess was the intention ... As apple
probably won't update the bios does anyone know how to get a newer
microcode for a core duo cpu  (not core 2 duo!) ?

Soeren
-- 
Sometimes, there's a moment as you're waking, when you become aware of
the real world around you, but you're still dreaming.

Re: [PATCH] hwmon/coretemp: Fix a broken error path

[Rudolf Marek]

Hello all,

Sorry for the delay, I was hiking in the mountains.

> this patch indeed fixes the problem. Thanks!

Jean, thanks for the quick fix! Well obviously my fault :/ Sorry.

> Unfortunately the coretemp sensors are simply never there when that
> patch is applied... which I guess was the intention ... As apple
> probably won't update the bios does anyone know how to get a newer
> microcode for a core duo cpu  (not core 2 duo!) ?

Well it seems that it is not included in http://www.urbanmyth.org/microcode/
So I would recommend getting it from some BIOS from other vendor. You will need 
to decompress the BIOS and get the microcode from there. If you will find some 
BIOS for Core duo ntbk I might help with that. Just send me a URL for download 
and I will try to dig it out (with some luck there will be some update ;)

Rudolf

Re: [PATCH] hwmon/coretemp: Fix a broken error path - microcode update fix

[Rudolf Marek]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

Hello Soeren,

Sorry for the delay.

I'm ccing all lists maybe some other people are interested. There is known 
errata AE18 which prevents coretemp from working correctly on some mobile Core 
processors (family 6 model e). My driver refuses to load and now thanks to 
soeren will not crash ;) However what to do when no microcode update (no new 
BIOS) is available?

Soeren pointed at some T60, T60p BIOS update and luckily, there is a easy way 
how to extract the microcode update and even convert it into the .txt format as 
microcode update utility (http://www.urbanmyth.org/microcode/) expects.
Attached scripts generates the mcode.txt file which may be used by the update 
utility. Please can you give a try?

It seems that there is microcode update for CPUID 06E8 version 0x39 just as my 
driver is checking. So if your CPUID is 06e8 too you should get the coretemp 
driver working.

If so I will post a patch and document the script in documentation directory (or 
at least some general instructions how to do that)

Please tell me your stepping:
cat /proc/cpuinfo  | grep stepping

Thanks,

Rudolf



[-- Attachment #2: gen_mcode.txt --]
[-- Type: text/plain, Size: 313 bytes --]

#!/bin/bash

#get the BIOS http://www-307.ibm.com/pc/support/site.wss/MIGR-63024.html
wget ftp://ftp.software.ibm.com/pc/pccbbs/mobiles/79uj20us.exe
cabextract -F "*PAT" 79uj20us.exe
 ls -1 *PAT | while read line; do echo "/* Microcode from $line */" ;  hexdump -e  ' 4/4 "0x%08X, " "\n"'  $line ;done >mcode.txt

Re: [PATCH] hwmon/coretemp: Fix a broken error path - microcode update fix

[Soeren Sonnenburg]

On Thu, 2007-06-21 at 22:57 +0200, Rudolf Marek wrote:
> Hello Soeren,
> 
> Sorry for the delay.
> 
> I'm ccing all lists maybe some other people are interested. There is known 
> errata AE18 which prevents coretemp from working correctly on some mobile Core 
> processors (family 6 model e). My driver refuses to load and now thanks to 
> soeren will not crash ;) However what to do when no microcode update (no new 
> BIOS) is available?
> 
> Soeren pointed at some T60, T60p BIOS update and luckily, there is a easy way 
> how to extract the microcode update and even convert it into the .txt format as 
> microcode update utility (http://www.urbanmyth.org/microcode/) expects.
> Attached scripts generates the mcode.txt file which may be used by the update 
> utility. Please can you give a try?

great! it works:

sensors excerpt :

coretemp-isa-0000
Adapter: ISA adapter
temp1:       +62°C  (high =  +100°C)                     

coretemp-isa-0001
Adapter: ISA adapter
temp1:       +64°C  (high =  +100°C)

> It seems that there is microcode update for CPUID 06E8 version 0x39 just as my 
> driver is checking. So if your CPUID is 06e8 too you should get the coretemp 
> driver working.

how do I find that out ? I mean sensors seem to work but how do I know
which CPUID+version I have ?

> If so I will post a patch and document the script in documentation directory (or 
> at least some general instructions how to do that)
> 
> Please tell me your stepping:
> cat /proc/cpuinfo  | grep stepping

$ cat /proc/cpuinfo  | grep stepping
stepping        : 8
stepping        : 8

Thank you *very* much!
Soeren
-- 
Sometimes, there's a moment as you're waking, when you become aware of
the real world around you, but you're still dreaming.

Re: [PATCH] hwmon/coretemp: Fix a broken error path - microcode update fix

[Soeren Sonnenburg]

On Thu, 2007-06-21 at 22:57 +0200, Rudolf Marek wrote:
> Hello Soeren,
[...]
> Soeren pointed at some T60, T60p BIOS update and luckily, there is a
> easy way 
> how to extract the microcode update and even convert it into the .txt
> format as 
> microcode update utility (http://www.urbanmyth.org/microcode/)
> expects.
> Attached scripts generates the mcode.txt file which may be used by the
> update 
> utility. Please can you give a try?

Hi Rudolf,

just one more update:

When I put my machine into s2ram and make it resume, one of the coretemp
sensors gets lost. Ahh and I am already rmmod coretemp / loading
microcode after resume / insmod coretemp...

Any idea's on that ?

Soeren
-- 
Sometimes, there's a moment as you're waking, when you become aware of
the real world around you, but you're still dreaming.

Re: [PATCH] hwmon/coretemp: Fix a broken error path - microcode update fix

[Rudolf Marek]

> Hi Rudolf,
> 
> just one more update:
> 
> When I put my machine into s2ram and make it resume, one of the coretemp
> sensors gets lost. Ahh and I am already rmmod coretemp / loading
> microcode after resume / insmod coretemp...

Hello, If I understand correctly you unload the driver before suspend. Resume, 
update microcode, load the driver correct?

Please can you check dmesg if for example one core complains about bad microcode 
version?

Thanks,
Rudolf

Re: [PATCH] hwmon/coretemp: Fix a broken error path - microcode update fix

[Soeren Sonnenburg]

On Mon, 2007-06-25 at 20:20 +0200, Rudolf Marek wrote:
> > Hi Rudolf,
> > 
> > just one more update:
> > 
> > When I put my machine into s2ram and make it resume, one of the coretemp
> > sensors gets lost. Ahh and I am already rmmod coretemp / loading
> > microcode after resume / insmod coretemp...
> 
> Hello, If I understand correctly you unload the driver before suspend. Resume, 
> update microcode, load the driver correct?

no I did not unload the driver but rmmod it after resume before I load
the microcode.

> Please can you check dmesg if for example one core complains about bad microcode 
> version?

indeed core1 still complains. and I just now loaded the new microcode
( rmmod of coretemp before and insmod after) and voila coretemp displays
both cpu's again. So I guess the microcode reloading is done to soon
after resume...

Soeren
-- 
Sometimes, there's a moment as you're waking, when you become aware of
the real world around you, but you're still dreaming.

Re: [PATCH] hwmon/coretemp: Fix a broken error path

[Jean Delvare]

Hi Soeren,

On Sun, 17 Jun 2007 11:16:43 +0200, Soeren Sonnenburg wrote:
> On Sat, 2007-06-16 at 23:17 +0200, Jean Delvare wrote:
> > Thanks for reporting. Indeed this patch is broken, sorry for
> > overlooking it. I tested it but my hardware is such that the faulty
> > error path was never taken. Please test the following patch (on top of
> > git-current) and confirm it fixes your problem:
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Rudolf Marek <r.marek@assembler.cz>
> > ---
> >  drivers/hwmon/coretemp.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > --- linux-2.6.22-rc4.orig/drivers/hwmon/coretemp.c	2007-06-05 10:25:54.000000000 +0200
> > +++ linux-2.6.22-rc4/drivers/hwmon/coretemp.c	2007-06-16 23:14:06.000000000 +0200
> > @@ -185,6 +185,7 @@ static int __devinit coretemp_probe(stru
> >  		/* check for microcode update */
> >  		rdmsr_on_cpu(data->id, MSR_IA32_UCODE_REV, &eax, &edx);
> >  		if (edx < 0x39) {
> > +			err = -ENODEV;
> >  			dev_err(&pdev->dev,
> >  				"Errata AE18 not fixed, update BIOS or "
> >  				"microcode of the CPU!\n");
> 
> this patch indeed fixes the problem. Thanks!

OK, great. Andrew, can you please pick this patch:
http://lkml.org/lkml/2007/6/16/177
and push it to Linus in your next batch?

> Soeren
> -- 
> Sometimes, there's a moment as you're waking, when you become aware of
> the real world around you, but you're still dreaming.

I too listen to Astral Projection at times :)

Thanks,
-- 
Jean Delvare