[PATCH] Arch: x86_32, fix fault_msg nul termination

[PATCH] Arch: x86_32, fix fault_msg nul termination

[Jiri Slaby]

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

[PATCH] Mark early_printk as asmlinkage

[Jiri Slaby]

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

Re: [PATCH] Mark early_printk as asmlinkage

[H. Peter Anvin]

Jiri Slaby wrote:
> 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.

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

Re: [PATCH] Mark early_printk as asmlinkage

[Jiri Slaby]

On 03/12/2008 03:04 PM, H. Peter Anvin wrote:
> Jiri Slaby wrote:
>> 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.
> 
> 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.

Makes sense. Why is printk marked as asmlinkage?

Re: [PATCH] Mark early_printk as asmlinkage

[H. Peter Anvin]

Jiri Slaby wrote:
> On 03/12/2008 03:04 PM, H. Peter Anvin wrote:
>> Jiri Slaby wrote:
>>> 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.
>>
>> 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.
> 
> Makes sense. Why is printk marked as asmlinkage?

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

Re: [PATCH] Mark early_printk as asmlinkage

[Pavel Machek]

On Wed 2008-03-12 15:04:43, H. Peter Anvin wrote:
> Jiri Slaby wrote:
> >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.
> 
> 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.

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

Re: [PATCH] Mark early_printk as asmlinkage

[H. Peter Anvin]

Pavel Machek wrote:
> On Wed 2008-03-12 15:04:43, H. Peter Anvin wrote:
>> Jiri Slaby wrote:
>>> 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.
>> 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.
> 
> I'd call asmlinkage kind of documentation, then. Not everyone is as
> good with x86 abi as you are...
> 

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

Re: [PATCH] Arch: x86_32, fix fault_msg nul termination

[Ingo Molnar]

* Jiri Slaby <jirislaby@gmail.com> wrote:

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

thanks Jiri, applied.

>  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"		\

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

	Ingo

Re: [PATCH] Mark early_printk as asmlinkage

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

>> I'd call asmlinkage kind of documentation, then. Not everyone is as 
>> good with x86 abi as you are...
>
> 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.

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