Bug 5553

bugme-daemon@bugzilla.kernel.org wrote:
>
> http://bugzilla.kernel.org/show_bug.cgi?id=5553
> 
>             Summary: speedstep-smi fails to load if CONFIG_CPU_FREQ_DEBUG is
>                      not set

Perhaps you could try adding this, it might tell us a bit more about why
it's failing?

--- devel/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c~a	2005-11-11 01:42:00.000000000 -0800
+++ devel-akpm/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c	2005-11-11 01:43:06.000000000 -0800
@@ -373,6 +373,9 @@ static int __init speedstep_init(void)
 		ist_info.signature, ist_info.command, ist_info.event, ist_info.perf_level);
 
 
+	printk("sig=%lx, smi_port=%d, smi_cmd=%d\n",
+		ist_info.signature, smi_port, smi_cmd);
+
 	/* Error if no IST-SMI BIOS or no PARM 
 		 sig= 'ISGE' aka 'Intel Speedstep Gate E' */
 	if ((ist_info.signature !=  0x47534943) && ( 
_

Also. Try modprobe'ing multiple times. There is something strange about
speedstep-smi that it works sometimes and doesn;t work at others...

Look at this bug #5082 as well

With the patch provided by Andrew Morton:

CONFIG_CPU_FREQ_DEBUG=n

# modprobe speedstep-smi
FATAL: Error inserting speedstep_smi (/.../speedstep-smi.ko): No such device
kernel: sig=47534943, smi_port=0, smi_cmd=0

# modprobe speedstep-smi smi_port=0xb2 smi_cmd=0x82
FATAL: Error inserting speedstep_smi (/.../speedstep-smi.ko): No such device
kernel: sig=47534943, smi_port=178, smi_cmd=130

As per venkatesh.pallipadi at intel comments I've tried:

# for i in `seq 1 100`; do modprobe speedstep-smi ;done

But still no go.

---

CONFIG_CPU_FREQ_DEBUG=y

# modprobe speedstep-smi
kernel: sig=47534943, smi_port=0, smi_cmd=0

# rmmod speedstep-smi

# modprobe speedstep-smi smi_port=0xb2 smi_cmd=0x82
kernel: sig=47534943, smi_port=178, smi_cmd=130

With debug enabled it just works.

Created attachment 6973 [details]
wait a bit before trying transitions

This is one of the strangest bugs I've ever seen... As there is no code change,
just a few lines of printk() added -- which might slow down things a bit, which
is what might be needed in this case... does this patch help, by chance?

Unfortunately the patch in attachment id 6973 did not solve the problem. I've 
tryed to nail down the time critical place by changing dprintk()s to printk() 
one by one. speedstep-smi is working for me with the changes in the attached 
patch. 

Created attachment 7093 [details]
change dprintk to printk in speedstep_smi_ownership()

It's strange!

Theodor wrote;
>cpufreq used to work with 2.6.8 and 2.6.11 kernels, but doesn't work >with
2.6.12 and 2.6.14 for me. 

and inserting printk before calling SMI BIOS solves a problem.  

But in 2.6.8, speedstep-smi driver DON'T have printk() there.
Do you use same compiler version as compiling 2.6.8 or 2.6.11? 
If the problem is this timing issue, 2.6.8 should have same problem, too.  





 

Uhmm, it seems some cache coherency problem.
Touching 'command' variable in printk() flashes cache including 'magic_data',
doesn't it?

How about moving definition of magic_data[] to global?
The valiable will be initialized at load time. 
--- speedstep-smi.c.orig        2005-09-17 10:02:12.000000000 +0900
+++ speedstep-smi.c     2006-02-05 23:52:16.000000000 +0900
@@ -61,11 +61,13 @@
 /**
  * speedstep_smi_ownership
  */
+
+static unsigned char magic_data[] = "Copyright (c) 1999 Intel Corporation";
+
 static int speedstep_smi_ownership (void)
 {
        u32 command, result, magic;
        u32 function = GET_SPEEDSTEP_OWNER;
-       unsigned char magic_data[] = "Copyright (c) 1999 Intel Corporation";

        command = (smi_sig & 0xffffff00) | (smi_cmd & 0xff);
        magic = virt_to_phys(magic_data);

Hallo, I also have this problem on linux 2.6.16, and the patch in
http://bugzilla.kernel.org/show_bug.cgi?id=5553#c8 does not help.

I found that any printk() statement before the asm code seems to fix the symptom
(hence the driver can be loaded without problems).

Just for istance, the following patch make the module loadable without touching
explicitly the 'command' variable or anything else.

--- speedstep-smi.c.orig	2006-03-23 19:22:05.000000000 +0100
+++ speedstep-smi.c	2006-03-23 20:05:28.000000000 +0100
@@ -70,6 +70,7 @@ static int speedstep_smi_ownership (void
 	command = (smi_sig & 0xffffff00) | (smi_cmd & 0xff);
 	magic = virt_to_phys(magic_data);
 
+	printk("Loading speedstep-smi.\n");
 	dprintk("trying to obtain ownership with command %x at port %x\n", command,
smi_port);
 
 	__asm__ __volatile__(


Maybe a workaround like this can be used in a future kernel version to let users
load the module, while thinking to a real fix (it sounds a bit 'redmondish', I
know :) )

Thanks,
   Antonio Ospite.

Linus Torvalds <torvalds@osdl.org> wrote:
>
> 
> 
> On Thu, 23 Mar 2006, Andrew Morton wrote:
> > 
> > hm.  "any printk before the asm seems to fix the symptom".  That makes me
> > think that the asm might have incorrect constraints or clobbers.
> > 
> > Linus, you're the man.  Does this look OK?
> 
> The thing looks fine, so I suspect it is just that the printk() changes 
> timings.
> 
> On the other hand, it does seem to depend on the _memory_ contents of that 
> "magic_data[]", so maybe adding a
> 
> 	: "memory"
> 
> at the end of the asm would actually help some?
> 
> 		Linus

Can someone please test this?


--- 25/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c~a	Thu Mar 23 16:18:19 2006
+++ 25-akpm/arch/i386/kernel/cpu/cpufreq/speedstep-smi.c	Thu Mar 23 16:19:07 2006
@@ -75,7 +75,9 @@ static int speedstep_smi_ownership (void
 	__asm__ __volatile__(
 		"out %%al, (%%dx)\n"
 		: "=D" (result)
-		: "a" (command), "b" (function), "c" (0), "d" (smi_port), "D" (0), "S" (magic)
+		: "a" (command), "b" (function), "c" (0), "d" (smi_port),
+			"D" (0), "S" (magic)
+		: "memory"
 	);
 
 	dprintk("result is %x\n", result);
_

Great!
The latest patch from Andrew worked for me. 
Hope to see this change in the next version.

Many thanks to Linus... again.

Regards,
   Antonio

Thanks for confirming.  I'll send this in for 2.6.16.1 as well.

closed by:


commit f081a529f808ed450c22553de7b3275e0ffde9a0
Author: Andrew Morton <akpm@osdl.org>
Date:   Sat Mar 25 01:51:51 2006 -0800

    [PATCH] cpufreq: speedstep-smi asm fix
    
    Fix bug identified by Linus Torvalds <torvalds@osdl.org>: the `out'
    instruction depends upon the state of memory_data[], so we need to tell gcc
    that before executing it. (The opcode, not gcc).
    
    Fixes http://bugzilla.kernel.org/show_bug.cgi?id=5553
    
    Thanks to Antonio Ospite <ospite@studenti.unina.it> for testing.
    
    Cc: Dave Jones <davej@codemonkey.org.uk>
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>