Re: [PATCH] Mark early_printk as asmlinkage

Previous thread: [patch 6/6] Guest page hinting: s390 support. by Martin Schwidefsky on Wednesday, March 12, 2008 - 6:21 am. (19 messages)

Next thread: [PATCH] Save some bytes in mm_struct by filling holes on 64bit by Andi Kleen on Wednesday, March 12, 2008 - 7:28 am. (4 messages)
From: Jiri Slaby
Date: Wednesday, March 12, 2008 - 6:53 am

The fault_msg text is not explictly nul terminated now in startup
assembly. Do so by converting .ascii to .asciz.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/head_32.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 9e7e015..826988a 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -657,7 +657,7 @@ int_msg:
 	.asciz "Unknown interrupt or fault at EIP %p %p %p\n"
 
 fault_msg:
-	.ascii								\
+	.asciz								\
 /* fault info: */	"BUG: Int %d: CR2 %p\n"				\
 /* pusha regs: */	"     EDI %p  ESI %p  EBP %p  ESP %p\n"		\
 			"     EBX %p  EDX %p  ECX %p  EAX %p\n"		\
-- 
1.5.4.1

--

From: Jiri Slaby
Date: Wednesday, March 12, 2008 - 6:53 am

It's not explicitly marked as asmlinkage, but invoked from x86_32
startup code with parameters on stack.

No other architectures define early_printk and none of them are affected
by this change, since defines asmlinkage as empty token.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
 arch/x86/kernel/early_printk.c |    2 +-
 include/linux/kernel.h         |    2 +-
 kernel/printk.c                |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 643fd86..ff9e735 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -196,7 +196,7 @@ static struct console simnow_console = {
 static struct console *early_console = &early_vga_console;
 static int early_console_initialized;
 
-void early_printk(const char *fmt, ...)
+asmlinkage void early_printk(const char *fmt, ...)
 {
 	char buf[512];
 	int n;
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 7e68c07..c7d61a1 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -210,7 +210,7 @@ static inline bool printk_timed_ratelimit(unsigned long *caller_jiffies, \
 		{ return false; }
 #endif
 
-extern void __attribute__((format(printf, 1, 2)))
+extern void asmlinkage __attribute__((format(printf, 1, 2)))
 	early_printk(const char *fmt, ...);
 
 unsigned long int_sqrt(unsigned long);
diff --git a/kernel/printk.c b/kernel/printk.c
index 3fb257c..951543d 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -38,7 +38,7 @@
 /*
  * Architectures can override it:
  */
-void __attribute__((weak)) early_printk(const char *fmt, ...)
+void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
 {
 }
 
-- 
1.5.4.1

--

From: H. Peter Anvin
Date: Wednesday, March 12, 2008 - 7:04 am

NAK.

The regparm ABI for x86-32 uses parameters on the stack when the 
function is varadic (as it is here), so this is unnecessary.

	-hpa
--

From: Jiri Slaby
Date: Wednesday, March 12, 2008 - 7:07 am

Makes sense. Why is printk marked as asmlinkage?
--

From: H. Peter Anvin
Date: Wednesday, March 12, 2008 - 7:21 am

Hm.  Don't know if it's historical, stylistic (anything called from 
assembly should have "asmlinkage"), or just based on a misunderstanding 
of the regparm ABI.

	-hpa
--

From: Pavel Machek
Date: Friday, March 14, 2008 - 11:04 am

I'd call asmlinkage kind of documentation, then. Not everyone is as
good with x86 abi as you are...

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--

From: H. Peter Anvin
Date: Friday, March 14, 2008 - 11:39 am

Since it's already only used on x86-32 and we no longer support 
non-regparm x86, I'd like to at least get to the point where x86-32 
doesn't have any function.  We can retain it for documentation's sake, 
but even then it's iffy... is "this is callable from assembly" really 
something arch-invariant.

	-hpa
--

From: Ingo Molnar
Date: Friday, March 21, 2008 - 5:25 am

the kernel still works if we disable regparm, and it makes sense to just 
have a good list of all functions that are called from assembly. But 
it's not just about non-regparm or documentation, we have regular 
trouble with over-eager gcc optimizations that assume that all code is 
generated by gcc ... Furthermore, code flow is easier to understand if 
we know what is called from assembly and what not. So documenting all 
these places makes sense to me and we've applied similar patches in the 
past.

	Ingo
--

From: Ingo Molnar
Date: Friday, March 21, 2008 - 4:50 am

i guess in practice we were saved by a NIL following this string, but 
there's no guarantee indeed.

	Ingo
--

Previous thread: [patch 6/6] Guest page hinting: s390 support. by Martin Schwidefsky on Wednesday, March 12, 2008 - 6:21 am. (19 messages)

Next thread: [PATCH] Save some bytes in mm_struct by filling holes on 64bit by Andi Kleen on Wednesday, March 12, 2008 - 7:28 am. (4 messages)