[patch 07/10] watchdog: hpwdt: fix use of inline assembly

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: <linux-kernel@...>, <stable@...>
Cc: Justin Forbes <jmforbes@...>, Zwane Mwaikambo <zwane@...>, Theodore Ts'o <tytso@...>, Randy Dunlap <rdunlap@...>, Dave Jones <davej@...>, Chuck Wolber <chuckw@...>, Chris Wedgwood <reviews@...>, Michael Krufky <mkrufky@...>, Chuck Ebbert <cebbert@...>, Domenico Andreoli <cavokz@...>, Willy Tarreau <w@...>, Rodrigo Rubira Branco <rbranco@...>, <torvalds@...>, <akpm@...>, <alan@...>, Thomas Mingarelli <Thomas.Mingarelli@...>
Date: Monday, June 23, 2008 - 7:05 pm

2.6.25.9-stable review patch.  If anyone has any objections, please let
us know.

------------------ 
From: Linus Torvalds <torvalds@linux-foundation.org>

commit 1f6ef2342972dc7fd623f360f84006e2304eb935 upstream

The inline assembly in drivers/watchdog/hpwdt.c was incredibly broken,
and included all the function prologue and epilogue stuff, even though
it was itself then inside a C function where the compiler would add its
own prologue and epilogue on top of it all.

This then just _happened_ to work if you had exactly the right compiler
version and exactly the right compiler flags, so that gcc just happened
to not create any prologue at all (the gcc-generated epilogue wouldn't
matter, since it would never be reached).

But the more proper way to fix it is to simply not do this.  Move the
inline asm to the top level, with no surrounding function at all (the
better alternative would be to remove the prologue and make it actually
use proper description of the arguments to the inline asm, but that's a
bigger change than the one I'm willing to make right now).

Tested-by: S.Çağlar Onur <caglar@pardus.org.tr>
Acked-by: Thomas Mingarelli <Thomas.Mingarelli@hp.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/watchdog/hpwdt.c |  154 ++++++++++++++++++++++++-----------------------
 1 file changed, 79 insertions(+), 75 deletions(-)

--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -140,49 +140,52 @@ static struct pci_device_id hpwdt_device
 };
 MODULE_DEVICE_TABLE(pci, hpwdt_devices);
 
+extern asmlinkage void asminline_call(struct cmn_registers *pi86Regs, unsigned long *pRomEntry);
+
 #ifndef CONFIG_X86_64
 /* --32 Bit Bios------------------------------------------------------------ */
 
 #define HPWDT_ARCH	32
 
-static void asminline_call(struct cmn_registers *pi86Regs,
-			   unsigned long *pRomEntry)
-{
-	asm("pushl       %ebp               \n\t"
-	    "movl        %esp, %ebp         \n\t"
-	    "pusha                          \n\t"
-	    "pushf                          \n\t"
-	    "push        %es                \n\t"
-	    "push        %ds                \n\t"
-	    "pop         %es                \n\t"
-	    "movl        8(%ebp),%eax       \n\t"
-	    "movl        4(%eax),%ebx       \n\t"
-	    "movl        8(%eax),%ecx       \n\t"
-	    "movl        12(%eax),%edx      \n\t"
-	    "movl        16(%eax),%esi      \n\t"
-	    "movl        20(%eax),%edi      \n\t"
-	    "movl        (%eax),%eax        \n\t"
-	    "push        %cs                \n\t"
-	    "call        *12(%ebp)          \n\t"
-	    "pushf                          \n\t"
-	    "pushl       %eax               \n\t"
-	    "movl        8(%ebp),%eax       \n\t"
-	    "movl        %ebx,4(%eax)       \n\t"
-	    "movl        %ecx,8(%eax)       \n\t"
-	    "movl        %edx,12(%eax)      \n\t"
-	    "movl        %esi,16(%eax)      \n\t"
-	    "movl        %edi,20(%eax)      \n\t"
-	    "movw        %ds,24(%eax)       \n\t"
-	    "movw        %es,26(%eax)       \n\t"
-	    "popl        %ebx               \n\t"
-	    "movl        %ebx,(%eax)        \n\t"
-	    "popl        %ebx               \n\t"
-	    "movl        %ebx,28(%eax)      \n\t"
-	    "pop         %es                \n\t"
-	    "popf                           \n\t"
-	    "popa                           \n\t"
-	    "leave                          \n\t" "ret");
-}
+asm(".text                          \n\t"
+    ".align 4                       \n"
+    "asminline_call:                \n\t"
+    "pushl       %ebp               \n\t"
+    "movl        %esp, %ebp         \n\t"
+    "pusha                          \n\t"
+    "pushf                          \n\t"
+    "push        %es                \n\t"
+    "push        %ds                \n\t"
+    "pop         %es                \n\t"
+    "movl        8(%ebp),%eax       \n\t"
+    "movl        4(%eax),%ebx       \n\t"
+    "movl        8(%eax),%ecx       \n\t"
+    "movl        12(%eax),%edx      \n\t"
+    "movl        16(%eax),%esi      \n\t"
+    "movl        20(%eax),%edi      \n\t"
+    "movl        (%eax),%eax        \n\t"
+    "push        %cs                \n\t"
+    "call        *12(%ebp)          \n\t"
+    "pushf                          \n\t"
+    "pushl       %eax               \n\t"
+    "movl        8(%ebp),%eax       \n\t"
+    "movl        %ebx,4(%eax)       \n\t"
+    "movl        %ecx,8(%eax)       \n\t"
+    "movl        %edx,12(%eax)      \n\t"
+    "movl        %esi,16(%eax)      \n\t"
+    "movl        %edi,20(%eax)      \n\t"
+    "movw        %ds,24(%eax)       \n\t"
+    "movw        %es,26(%eax)       \n\t"
+    "popl        %ebx               \n\t"
+    "movl        %ebx,(%eax)        \n\t"
+    "popl        %ebx               \n\t"
+    "movl        %ebx,28(%eax)      \n\t"
+    "pop         %es                \n\t"
+    "popf                           \n\t"
+    "popa                           \n\t"
+    "leave                          \n\t"
+    "ret                            \n\t"
+    ".previous");
 
 /*
  *	cru_detect
@@ -333,43 +336,44 @@ static int __devinit detect_cru_service(
 
 #define HPWDT_ARCH	64
 
-static void asminline_call(struct cmn_registers *pi86Regs,
-			   unsigned long *pRomEntry)
-{
-	asm("pushq      %rbp            \n\t"
-	    "movq       %rsp, %rbp      \n\t"
-	    "pushq      %rax            \n\t"
-	    "pushq      %rbx            \n\t"
-	    "pushq      %rdx            \n\t"
-	    "pushq      %r12            \n\t"
-	    "pushq      %r9             \n\t"
-	    "movq       %rsi, %r12      \n\t"
-	    "movq       %rdi, %r9       \n\t"
-	    "movl       4(%r9),%ebx     \n\t"
-	    "movl       8(%r9),%ecx     \n\t"
-	    "movl       12(%r9),%edx    \n\t"
-	    "movl       16(%r9),%esi    \n\t"
-	    "movl       20(%r9),%edi    \n\t"
-	    "movl       (%r9),%eax      \n\t"
-	    "call       *%r12           \n\t"
-	    "pushfq                     \n\t"
-	    "popq        %r12           \n\t"
-	    "popfq                      \n\t"
-	    "movl       %eax, (%r9)     \n\t"
-	    "movl       %ebx, 4(%r9)    \n\t"
-	    "movl       %ecx, 8(%r9)    \n\t"
-	    "movl       %edx, 12(%r9)   \n\t"
-	    "movl       %esi, 16(%r9)   \n\t"
-	    "movl       %edi, 20(%r9)   \n\t"
-	    "movq       %r12, %rax      \n\t"
-	    "movl       %eax, 28(%r9)   \n\t"
-	    "popq       %r9             \n\t"
-	    "popq       %r12            \n\t"
-	    "popq       %rdx            \n\t"
-	    "popq       %rbx            \n\t"
-	    "popq       %rax            \n\t"
-	    "leave                      \n\t" "ret");
-}
+asm(".text                      \n\t"
+    ".align 4                   \n"
+    "asminline_call:            \n\t"
+    "pushq      %rbp            \n\t"
+    "movq       %rsp, %rbp      \n\t"
+    "pushq      %rax            \n\t"
+    "pushq      %rbx            \n\t"
+    "pushq      %rdx            \n\t"
+    "pushq      %r12            \n\t"
+    "pushq      %r9             \n\t"
+    "movq       %rsi, %r12      \n\t"
+    "movq       %rdi, %r9       \n\t"
+    "movl       4(%r9),%ebx     \n\t"
+    "movl       8(%r9),%ecx     \n\t"
+    "movl       12(%r9),%edx    \n\t"
+    "movl       16(%r9),%esi    \n\t"
+    "movl       20(%r9),%edi    \n\t"
+    "movl       (%r9),%eax      \n\t"
+    "call       *%r12           \n\t"
+    "pushfq                     \n\t"
+    "popq        %r12           \n\t"
+    "popfq                      \n\t"
+    "movl       %eax, (%r9)     \n\t"
+    "movl       %ebx, 4(%r9)    \n\t"
+    "movl       %ecx, 8(%r9)    \n\t"
+    "movl       %edx, 12(%r9)   \n\t"
+    "movl       %esi, 16(%r9)   \n\t"
+    "movl       %edi, 20(%r9)   \n\t"
+    "movq       %r12, %rax      \n\t"
+    "movl       %eax, 28(%r9)   \n\t"
+    "popq       %r9             \n\t"
+    "popq       %r12            \n\t"
+    "popq       %rdx            \n\t"
+    "popq       %rbx            \n\t"
+    "popq       %rax            \n\t"
+    "leave                      \n\t"
+    "ret                        \n\t"
+    ".previous");
 
 /*
  *	dmi_find_cru

-- 
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[patch 00/10] 2.6.28.9-rc2 review, Greg KH, (Mon Jun 23, 7:04 pm)
Re: [patch 00/10] 2.6.28.9-rc2 review, Greg KH, (Mon Jun 23, 7:22 pm)
[patch 07/10] watchdog: hpwdt: fix use of inline assembly, Greg KH, (Mon Jun 23, 7:05 pm)
[patch 10/10] Fix ZERO_PAGE breakage with vmware, Greg KH, (Mon Jun 23, 7:04 pm)
Re: [patch 10/10] Fix ZERO_PAGE breakage with vmware, Linus Torvalds, (Mon Jun 23, 7:28 pm)