[RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[Cyrill Gorcunov]

Hi,

here is a small series of cleanups for assembler
files. Not sure if introducing new GLOBAL macro
was a good idea but code got shorter form of
writting.

Please review. Any comments are highly appreciated.

Cyrill

[RFC 1/6] x86: asm linkage - introduce GLOBAL macro

[Cyrill Gorcunov]

[-- Attachment #1: x86-linkage-global --]
[-- Type: text/plain, Size: 819 bytes --]

If the code is time critical and this entry is called
from other places we use ENTRY to have it globally defined
and especially aligned.

Contrary we have some snippets which are size
critical. So we use plane ".globl name; name:"
directive. Introduce GLOBAL macro for this.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/include/asm/linkage.h |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-2.6.git/arch/x86/include/asm/linkage.h
===================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/linkage.h
+++ linux-2.6.git/arch/x86/include/asm/linkage.h
@@ -52,6 +52,10 @@
 
 #endif
 
+#define GLOBAL(name)	\
+	.globl name;	\
+	name:
+
 #ifdef CONFIG_X86_ALIGNMENT_16
 #define __ALIGN .align 16,0x90
 #define __ALIGN_STR ".align 16,0x90"

[RFC 2/6] x86: linkage - get rid of _X86 macros

[Cyrill Gorcunov]

[-- Attachment #1: x86-linkage-slim-it --]
[-- Type: text/plain, Size: 2084 bytes --]

Impact: cleanup

There was an attempt to bring build-time checking for
missed ENTRY_X86/END_X86 and KPROBE... pairs. Using
them will add messy in code. Get just rid of them.
This commit could be easily restored if the need appear
in future.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/include/asm/linkage.h |   60 -----------------------------------------
 1 file changed, 60 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/linkage.h
===================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/linkage.h
+++ linux-2.6.git/arch/x86/include/asm/linkage.h
@@ -61,65 +61,5 @@
 #define __ALIGN_STR ".align 16,0x90"
 #endif
 
-/*
- * to check ENTRY_X86/END_X86 and
- * KPROBE_ENTRY_X86/KPROBE_END_X86
- * unbalanced-missed-mixed appearance
- */
-#define __set_entry_x86		.set ENTRY_X86_IN, 0
-#define __unset_entry_x86	.set ENTRY_X86_IN, 1
-#define __set_kprobe_x86	.set KPROBE_X86_IN, 0
-#define __unset_kprobe_x86	.set KPROBE_X86_IN, 1
-
-#define __macro_err_x86 .error "ENTRY_X86/KPROBE_X86 unbalanced,missed,mixed"
-
-#define __check_entry_x86	\
-	.ifdef ENTRY_X86_IN;	\
-	.ifeq ENTRY_X86_IN;	\
-	__macro_err_x86;	\
-	.abort;			\
-	.endif;			\
-	.endif
-
-#define __check_kprobe_x86	\
-	.ifdef KPROBE_X86_IN;	\
-	.ifeq KPROBE_X86_IN;	\
-	__macro_err_x86;	\
-	.abort;			\
-	.endif;			\
-	.endif
-
-#define __check_entry_kprobe_x86	\
-	__check_entry_x86;		\
-	__check_kprobe_x86
-
-#define ENTRY_KPROBE_FINAL_X86 __check_entry_kprobe_x86
-
-#define ENTRY_X86(name)			\
-	__check_entry_kprobe_x86;	\
-	__set_entry_x86;		\
-	.globl name;			\
-	__ALIGN;			\
-	name:
-
-#define END_X86(name)			\
-	__unset_entry_x86;		\
-	__check_entry_kprobe_x86;	\
-	.size name, .-name
-
-#define KPROBE_ENTRY_X86(name)		\
-	__check_entry_kprobe_x86;	\
-	__set_kprobe_x86;		\
-	.pushsection .kprobes.text, "ax"; \
-	.globl name;			\
-	__ALIGN;			\
-	name:
-
-#define KPROBE_END_X86(name)		\
-	__unset_kprobe_x86;		\
-	__check_entry_kprobe_x86;	\
-	.size name, .-name;		\
-	.popsection
-
 #endif /* _ASM_X86_LINKAGE_H */
 

[RFC 3/6] x86: copy.S - use GLOBAL,ENDPROC macros

[Cyrill Gorcunov]

[-- Attachment #1: x86-boot-copy-add-global --]
[-- Type: text/plain, Size: 1938 bytes --]

Impact: cleanup

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/boot/copy.S |   40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

Index: linux-2.6.git/arch/x86/boot/copy.S
===================================================================
--- linux-2.6.git.orig/arch/x86/boot/copy.S
+++ linux-2.6.git/arch/x86/boot/copy.S
@@ -8,6 +8,8 @@
  *
  * ----------------------------------------------------------------------- */
 
+#include <linux/linkage.h>
+
 /*
  * Memory copy routines
  */
@@ -15,9 +17,7 @@
 	.code16gcc
 	.text
 
-	.globl	memcpy
-	.type	memcpy, @function
-memcpy:
+GLOBAL(memcpy)
 	pushw	%si
 	pushw	%di
 	movw	%ax, %di
@@ -31,11 +31,9 @@ memcpy:
 	popw	%di
 	popw	%si
 	ret
-	.size	memcpy, .-memcpy
+ENDPROC(memcpy)
 
-	.globl	memset
-	.type	memset, @function
-memset:
+GLOBAL(memset)
 	pushw	%di
 	movw	%ax, %di
 	movzbl	%dl, %eax
@@ -48,52 +46,42 @@ memset:
 	rep; stosb
 	popw	%di
 	ret
-	.size	memset, .-memset
+ENDPROC(memset)
 
-	.globl	copy_from_fs
-	.type	copy_from_fs, @function
-copy_from_fs:
+GLOBAL(copy_from_fs)
 	pushw	%ds
 	pushw	%fs
 	popw	%ds
 	call	memcpy
 	popw	%ds
 	ret
-	.size	copy_from_fs, .-copy_from_fs
+ENDPROC(copy_from_fs)
 
-	.globl	copy_to_fs
-	.type	copy_to_fs, @function
-copy_to_fs:
+GLOBAL(copy_to_fs)
 	pushw	%es
 	pushw	%fs
 	popw	%es
 	call	memcpy
 	popw	%es
 	ret
-	.size	copy_to_fs, .-copy_to_fs
+ENDPROC(copy_to_fs)
 
 #if 0 /* Not currently used, but can be enabled as needed */
-
-	.globl	copy_from_gs
-	.type	copy_from_gs, @function
-copy_from_gs:
+GLOBAL(copy_from_gs)
 	pushw	%ds
 	pushw	%gs
 	popw	%ds
 	call	memcpy
 	popw	%ds
 	ret
-	.size	copy_from_gs, .-copy_from_gs
-	.globl	copy_to_gs
+ENDPROC(copy_from_gs)
 
-	.type	copy_to_gs, @function
-copy_to_gs:
+GLOBAL(copy_to_gs)
 	pushw	%es
 	pushw	%gs
 	popw	%es
 	call	memcpy
 	popw	%es
 	ret
-	.size	copy_to_gs, .-copy_to_gs
-
+ENDPROC(copy_to_gs)
 #endif

[RFC 4/6] x86: pmjump - use GLOBAL,ENDPROC macros

[Cyrill Gorcunov]

[-- Attachment #1: x86-boot-pmjump-linkage --]
[-- Type: text/plain, Size: 1375 bytes --]

Impact: cleanup

We are in setup stage so we use GLOBAL
instead of ENTRY and do not increase code
size.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/boot/pmjump.S |   16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

Index: linux-2.6.git/arch/x86/boot/pmjump.S
===================================================================
--- linux-2.6.git.orig/arch/x86/boot/pmjump.S
+++ linux-2.6.git/arch/x86/boot/pmjump.S
@@ -15,18 +15,15 @@
 #include <asm/boot.h>
 #include <asm/processor-flags.h>
 #include <asm/segment.h>
+#include <linux/linkage.h>
 
 	.text
-
-	.globl	protected_mode_jump
-	.type	protected_mode_jump, @function
-
 	.code16
 
 /*
  * void protected_mode_jump(u32 entrypoint, u32 bootparams);
  */
-protected_mode_jump:
+GLOBAL(protected_mode_jump)
 	movl	%edx, %esi		# Pointer to boot_params table
 
 	xorl	%ebx, %ebx
@@ -47,12 +44,10 @@ protected_mode_jump:
 	.byte	0x66, 0xea		# ljmpl opcode
 2:	.long	in_pm32			# offset
 	.word	__BOOT_CS		# segment
-
-	.size	protected_mode_jump, .-protected_mode_jump
+ENDPROC(protected_mode_jump)
 
 	.code32
-	.type	in_pm32, @function
-in_pm32:
+GLOBAL(in_pm32)
 	# Set up data segments for flat 32-bit mode
 	movl	%ecx, %ds
 	movl	%ecx, %es
@@ -78,5 +73,4 @@ in_pm32:
 	lldt	%cx
 
 	jmpl	*%eax			# Jump to the 32-bit entrypoint
-
-	.size	in_pm32, .-in_pm32
+ENDPROC(in_pm32)

[RFC 5/6] x86: compressed head_64 - use ENTRY,ENDPROC macros

[Cyrill Gorcunov]

[-- Attachment #1: x86-compressed-head-64 --]
[-- Type: text/plain, Size: 1020 bytes --]

Impact: clenaup

Linker script will put startup_32 at predefined
address so using ENTRY will not bloat the code
size.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/boot/compressed/head_64.S |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6.git/arch/x86/boot/compressed/head_64.S
===================================================================
--- linux-2.6.git.orig/arch/x86/boot/compressed/head_64.S
+++ linux-2.6.git/arch/x86/boot/compressed/head_64.S
@@ -35,9 +35,7 @@
 
 .section ".text.head"
 	.code32
-	.globl startup_32
-
-startup_32:
+ENTRY(startup_32)
 	cld
 	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
 	 * us to not reload segments */
@@ -176,6 +174,7 @@ startup_32:
 
 	/* Jump from 32bit compatibility mode into 64bit mode. */
 	lret
+ENDPROC(startup_32)
 
 no_longmode:
 	/* This isn't an x86-64 CPU so hang */
@@ -295,7 +294,6 @@ relocated:
 	call	decompress_kernel
 	popq	%rsi
 
-
 /*
  * Jump to the decompressed kernel.
  */

[RFC 6/6] x86: compressed head_32 - use ENTRY,ENDPROC macros

[Cyrill Gorcunov]

[-- Attachment #1: x86-compressed-head-32 --]
[-- Type: text/plain, Size: 896 bytes --]

Impact: clenaup

Linker script will put startup_32 at predefined
address so using startup_32 will not bloat the
code size.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/boot/compressed/head_32.S |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6.git/arch/x86/boot/compressed/head_32.S
===================================================================
--- linux-2.6.git.orig/arch/x86/boot/compressed/head_32.S
+++ linux-2.6.git/arch/x86/boot/compressed/head_32.S
@@ -30,9 +30,7 @@
 #include <asm/asm-offsets.h>
 
 .section ".text.head","ax",@progbits
-	.globl startup_32
-
-startup_32:
+ENTRY(startup_32)
 	cld
 	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
 	 * us to not reload segments */
@@ -113,6 +111,8 @@ startup_32:
  */
 	leal relocated(%ebx), %eax
 	jmp *%eax
+ENDPROC(startup_32)
+
 .section ".text"
 relocated:
 

Re: [RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[Alexander van Heukelum]

On Sat, 14 Feb 2009 00:50:17 +0300, "Cyrill Gorcunov"
<gorcunov@gmail.com> said:
> Hi,
> 
> here is a small series of cleanups for assembler
> files. Not sure if introducing new GLOBAL macro
> was a good idea but code got shorter form of
> writting.
> 
> Please review. Any comments are highly appreciated.

Hi Cyrill,

I like this direction. If I understand correctly:

ENTRY/END or GLOBAL/END for data.
ENTRY/ENDPROC or GLOBAL/ENDPROC for functions.

The patches look sound. You're welcome to add:

Acked-by: Alexander van Heukelum <heukelum@fastmail.fm>

Greetings,
    Alexander

> Cyrill
-- 
  Alexander van Heukelum
  heukelum@fastmail.fm

-- 
http://www.fastmail.fm - Access your email from home and the web

Re: [RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[H. Peter Anvin]

Alexander van Heukelum wrote:
> 
> Hi Cyrill,
> 
> I like this direction. If I understand correctly:
> 
> ENTRY/END or GLOBAL/END for data.
> ENTRY/ENDPROC or GLOBAL/ENDPROC for functions.
> 

Fine for functions, but it's really not okay to use the same macros for 
data.  Furthermore, we need to consider special entry points that don't 
behave like normal functions -- like system call or interrupt entry.

Why?  Because if we're compiling with frame pointers, we would like the 
wrapper macros for functions to handle setting up and tearing down the 
frame pointer, at least in the common case.

	-hpa

Re: [RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[Cyrill Gorcunov]

[H. Peter Anvin - Fri, Feb 13, 2009 at 04:34:00PM -0800]
> Alexander van Heukelum wrote:
>>
>> Hi Cyrill,
>>
>> I like this direction. If I understand correctly:
>>
>> ENTRY/END or GLOBAL/END for data.
>> ENTRY/ENDPROC or GLOBAL/ENDPROC for functions.
>>
>
> Fine for functions, but it's really not okay to use the same macros for  
> data.  Furthermore, we need to consider special entry points that don't  
> behave like normal functions -- like system call or interrupt entry.
>
> Why?  Because if we're compiling with frame pointers, we would like the  
> wrapper macros for functions to handle setting up and tearing down the  
> frame pointer, at least in the common case.
>
> 	-hpa
>

Peter, Alexander -- thanks for review!

>> ENTRY/END or GLOBAL/END for data.
>> ENTRY/ENDPROC or GLOBAL/ENDPROC for functions.

Well, there are nuances I believe. Plain .size without .cfi_...
for functions would not help in frame unwinding I think (in *.S
under debugger).

I wish we have strict rules for functions and data but it's not
that simple :)

For example, trampoline_64.S:
...
	.org 0x1000
trampoline_stack_end:
ENTRY(trampoline_level4_pgt)
...

we could use plain .globl (or new GLOBAL) here since we
have .org and ALIGN in ENTRY just not needed.

Same file:
...
ENTRY(trampoline_end)
...

I think we could have plain .globl here as well
(and we do that for 32bit version). Moreover TRAMPOLINE_SIZE
is page rounded anyway.

But in general I think you're right -- which means:
start with END for data and ENDPROC for functions and
then checkout the details as Peter mentioned. At least
I do ratiocinate like this. But I could be wrong :)

	- Cyrill -

Re: [RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[Alexander van Heukelum]

On Fri, 13 Feb 2009 16:34:00 -0800, "H. Peter Anvin" <hpa@zytor.com>
said:
> Alexander van Heukelum wrote:
> > 
> > Hi Cyrill,
> > 
> > I like this direction. If I understand correctly:
> > 
> > ENTRY/END or GLOBAL/END for data.
> > ENTRY/ENDPROC or GLOBAL/ENDPROC for functions.
> > 
> 
> Fine for functions, but it's really not okay to use the same macros for 
> data.  Furthermore, we need to consider special entry points that don't 
> behave like normal functions -- like system call or interrupt entry.
> 
> Why?  Because if we're compiling with frame pointers, we would like the 
> wrapper macros for functions to handle setting up and tearing down the 
> frame pointer, at least in the common case.

Hi Peter,

I see. But that would be new behaviour. I would propose to use
completely separate macro's to handle frame-setup code generation,
and keep ENTRY/GLOBAL/END/ENDPROC only for setting metadata and
alignment. I think it's worth it to spell out code-generating
macro's explicitly: there are not that many asm functions, and
quite a few of them would need special handling. I think noone
wants to see an ENDPROC_NOFRAMETEARDOWN ;).

The common-case example would look like this.

GLOBAL(c_callable_function)
        ENTER
        [asm-code]
        LEAVE
        ret
ENDPROC(c_callable_function)

Greetings,
    Alexander

> 	-hpa
-- 
  Alexander van Heukelum
  heukelum@fastmail.fm

-- 
http://www.fastmail.fm - I mean, what is it about a decent email service?

Re: [RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[H. Peter Anvin]

Alexander van Heukelum wrote:
> 
> Hi Peter,
> 
> I see. But that would be new behaviour. I would propose to use
> completely separate macro's to handle frame-setup code generation,
> and keep ENTRY/GLOBAL/END/ENDPROC only for setting metadata and
> alignment. I think it's worth it to spell out code-generating
> macro's explicitly: there are not that many asm functions, and
> quite a few of them would need special handling. I think noone
> wants to see an ENDPROC_NOFRAMETEARDOWN ;).
> 
> The common-case example would look like this.
> 
> GLOBAL(c_callable_function)
>         ENTER
>         [asm-code]
>         LEAVE
>         ret
> ENDPROC(c_callable_function)
> 

I guess I'm a bit concerned about people omitting them, but it's equally
concerning that people use the wrong macros, so yes, it's probably the
better thing in the long run.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [RFC 0/6] x86 tip asm ENTRY,ENDPROC cleanup

[Ingo Molnar]

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

> Alexander van Heukelum wrote:
> > 
> > Hi Peter,
> > 
> > I see. But that would be new behaviour. I would propose to use
> > completely separate macro's to handle frame-setup code generation,
> > and keep ENTRY/GLOBAL/END/ENDPROC only for setting metadata and
> > alignment. I think it's worth it to spell out code-generating
> > macro's explicitly: there are not that many asm functions, and
> > quite a few of them would need special handling. I think noone
> > wants to see an ENDPROC_NOFRAMETEARDOWN ;).
> > 
> > The common-case example would look like this.
> > 
> > GLOBAL(c_callable_function)
> >         ENTER
> >         [asm-code]
> >         LEAVE
> >         ret
> > ENDPROC(c_callable_function)
> > 
> 
> I guess I'm a bit concerned about people omitting them, but it's equally
> concerning that people use the wrong macros, so yes, it's probably the
> better thing in the long run.

ok - i've applied Cyrill's patches to tip/x86/asm.

	Ingo