The first patch introduces the PROC macro in the generic
header file include/linux/linkage.h to annotate functions
in assembly files. This is a first step to fully annotate
functions (procedures) in .S-files. The PROC macro
complements the already existing and being used ENDPROC
macro. The generic implementation of PROC is exactly the
same as ENTRY.
The goal is to annotate functions, at least those called
from C code, with PROC at the beginning and ENDPROC at the
end. This is for the benefit of debugging and tracing.
The second patch introduces a framework to check for nesting
problems and missing annotations by overriding ENTRY/END
and PROC/ENDPROC in x86-specific code. It should not be
applied at this time, because it will just cause the build
to fail due to existing annotation problems. I intend to
fix the annotations for the x86 assembly files in the
comming months and resubmit this second patch if this
work is comming to an end.
The first patch touches generic code, but I think it is
trivial enough that it can be introduced via the x86 tree.
Greetings,
Alexander
Introduce the PROC macro in the generic header file
include/linux/linkage.h to annotate functions in assembly
files. This is a first step to fully annotate functions
(procedures) in .S-files. The PROC macro complements the
already existing and being used ENDPROC macro. The generic
implementation of PROC is exactly the same as ENTRY.
The goal is to annotate functions, at least those called
from C code, with PROC at the beginning and ENDPROC at the
end. This is for the benefit of debugging and tracing. It
will also allow to introduce a framework to check for
nesting problems and missing annotations in a later stage
by overriding ENTRY/END and PROC/ENDPROC in architecture-
specific code, after the annotation errors have been fixed.
Signed-off-by: Alexander van Heukelum <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: Andrew Morton <[email protected]>
---
include/linux/linkage.h | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index fee9e59..f4bb5a7 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -69,10 +69,18 @@
.size name, .-name
#endif
-/* If symbol 'name' is treated as a subroutine (gets called, and returns)
- * then please use ENDPROC to mark 'name' as STT_FUNC for the benefit of
- * static analysis tools such as stack depth analyzer.
+/*
+ * If symbol 'name' is treated as a subroutine (gets called, and returns)
+ * then please use PROC/ENDPROC to mark 'name' as STT_FUNC for the benefit
+ * of static analysis tools such as stack depth analyzer.
*/
+#ifndef PROC
+#define PROC(name) \
+ .globl name; \
+ ALIGN; \
+ name:
+#endif
+
#ifndef ENDPROC
#define ENDPROC(name) \
.type name, @function; \
--
1.5.4.3
[ DO NOT APPLY (yet...) At this point this patch will
just cause the build to abort due to annotation errors
found. ]
Introduce a checking framework to check correct pairing
of ENTRY/END and PROC/ENDPROC. It also checks that the
annotations are not nested. I have used the ideas and
most of the implementation from Cyrill Gorcunov who
introduced the framework to check for mismatching
KPROBE_ENTRY annotations, which was however soon made
obsolete by the removal of KPROBE_ENTRY/KPROBE_END.
Checks performed:
o END must terminate an ENTRY annotation
o ENDPROC must terminate a PROC annotation
o ENTRY or PROC cannot be nested inside
another ENTRY or PROC section.
Finally the macro ENTRY_PROC_FINAL is introduced to
enable checking correct closing of PROC and ENTRY
sections at the end of assembly files.
Signed-off-by: Alexander van Heukelum <[email protected]>
Cc: Cyrill Gorcunov <[email protected]>
---
arch/x86/include/asm/linkage.h | 96 ++++++++++++++++++++++++----------------
1 files changed, 58 insertions(+), 38 deletions(-)
diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
index 5d98d0b..299cdf2 100644
--- a/arch/x86/include/asm/linkage.h
+++ b/arch/x86/include/asm/linkage.h
@@ -58,64 +58,84 @@
#endif
/*
- * to check ENTRY_X86/END_X86 and
- * KPROBE_ENTRY_X86/KPROBE_END_X86
+ * to check ENTRY/END and PROC/ENDPROC
* 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; \
+#ifdef __ASSEMBLER__
+#define __set_entry .set ENTRY_IN, 0
+#define __unset_entry .set ENTRY_IN, 1
+#define __set_proc .set PROC_IN, 0
+#define __unset_proc .set PROC_IN, 1
+
+#define __macro_err \
+ .error "ENTRY/PROC unbalanced,missed,mixed"; \
+ .abort
+
+#define __check_entry \
+ .ifdef ENTRY_IN; \
+ .ifeq ENTRY_IN; \
+ __macro_err; \
.endif; \
.endif
-#define __check_kprobe_x86 \
- .ifdef KPROBE_X86_IN; \
- .ifeq KPROBE_X86_IN; \
- __macro_err_x86; \
- .abort; \
+#define __check_in_entry \
+ .ifndef ENTRY_IN; \
+ __macro_err; \
+ .else; \
+ .ifeq !ENTRY_IN; \
+ __macro_err; \
.endif; \
.endif
-#define __check_entry_kprobe_x86 \
- __check_entry_x86; \
- __check_kprobe_x86
+#define __check_proc \
+ .ifdef PROC_IN; \
+ .ifeq PROC_IN; \
+ __macro_err; \
+ .endif; \
+ .endif
-#define ENTRY_KPROBE_FINAL_X86 __check_entry_kprobe_x86
+#define __check_in_proc \
+ .ifndef PROC_IN; \
+ __macro_err; \
+ .else; \
+ .ifeq !PROC_IN; \
+ __macro_err; \
+ .endif; \
+ .endif
-#define ENTRY_X86(name) \
- __check_entry_kprobe_x86; \
- __set_entry_x86; \
+#define __check_entry_proc \
+ __check_entry; \
+ __check_proc
+
+#define ENTRY_PROC_FINAL __check_entry_proc
+
+#define ENTRY(name) \
+ __check_entry_proc; \
+ __set_entry; \
.globl name; \
__ALIGN; \
name:
-#define END_X86(name) \
- __unset_entry_x86; \
- __check_entry_kprobe_x86; \
+#define END(name) \
+ __check_in_entry; \
+ __unset_entry; \
+ __check_entry_proc; \
.size name, .-name
-#define KPROBE_ENTRY_X86(name) \
- __check_entry_kprobe_x86; \
- __set_kprobe_x86; \
- .pushsection .kprobes.text, "ax"; \
+#define PROC(name) \
+ __check_entry_proc; \
+ __set_proc; \
.globl name; \
__ALIGN; \
name:
-#define KPROBE_END_X86(name) \
- __unset_kprobe_x86; \
- __check_entry_kprobe_x86; \
- .size name, .-name; \
- .popsection
+#define ENDPROC(name) \
+ __check_in_proc; \
+ __unset_proc; \
+ __check_entry_proc; \
+ .type name, @function; \
+ .size name, .-name
+#endif /* __ASSEMBLER__ */
#endif /* _ASM_X86_LINKAGE_H */
--
1.5.4.3
Alexander van Heukelum <[email protected]> wrote:
> The goal is to annotate functions, at least those called
> from C code, with PROC at the beginning and ENDPROC at the
> end. This is for the benefit of debugging and tracing.
What about asm functions that have multiple entry points?
Take arch/mn10300/mm/cache-flush-mn10300.S for example. Several functions in
there share bodies by virtue of falling through one to another.
David
On Wed, Dec 17, 2008 at 10:53:53AM +0000, David Howells wrote:
> Alexander van Heukelum <[email protected]> wrote:
>
> > The goal is to annotate functions, at least those called
> > from C code, with PROC at the beginning and ENDPROC at the
> > end. This is for the benefit of debugging and tracing.
>
> What about asm functions that have multiple entry points?
>
> Take arch/mn10300/mm/cache-flush-mn10300.S for example. Several functions in
> there share bodies by virtue of falling through one to another.
>
> David
Yeah, assembly files contain some interesting nesting. In this
particular case I think the solution is simple... Just use PROC
and ENDPROC around the complete functions, and leave the explicit
.global's for the additional entry points.
Have a look at arch/x86/crypto/aes-x86_64-asm_64.S too if you wish ;).
Greetings,
Alexander
arch/mn10300/mm/cache-flush-mn10300.S | 25 +++++++++++++------------
1 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/arch/mn10300/mm/cache-flush-mn10300.S b/arch/mn10300/mm/cache-flush-mn10300.S
index c8ed1cb..2c0f26d 100644
--- a/arch/mn10300/mm/cache-flush-mn10300.S
+++ b/arch/mn10300/mm/cache-flush-mn10300.S
@@ -16,12 +16,9 @@
#include <asm/cache.h>
.am33_2
- .globl mn10300_dcache_flush
- .globl mn10300_dcache_flush_page
+ /* nested entry points */
.globl mn10300_dcache_flush_range
.globl mn10300_dcache_flush_range2
- .globl mn10300_dcache_flush_inv
- .globl mn10300_dcache_flush_inv_page
.globl mn10300_dcache_flush_inv_range
.globl mn10300_dcache_flush_inv_range2
@@ -31,8 +28,8 @@
# Flush the entire data cache back to RAM
#
###############################################################################
- ALIGN
-mn10300_dcache_flush:
+
+PROC(mn10300_dcache_flush)
movhu (CHCTR),d0
btst CHCTR_DCEN,d0
beq mn10300_dcache_flush_end
@@ -59,6 +56,7 @@ mn10300_dcache_flush_skip:
mn10300_dcache_flush_end:
ret [],0
+ENDPROC(mn10300_dcache_flush)
###############################################################################
#
@@ -68,8 +66,8 @@ mn10300_dcache_flush_end:
# Flush a range of addresses on a page in the dcache
#
###############################################################################
- ALIGN
-mn10300_dcache_flush_page:
+
+PROC(mn10300_dcache_flush_page)
mov PAGE_SIZE,d1
mn10300_dcache_flush_range2:
add d0,d1
@@ -113,6 +111,7 @@ mn10300_dcache_flush_range_loop:
mn10300_dcache_flush_range_end:
ret [d2,d3],8
+ENDPROC(mn10300_dcache_flush_page)
###############################################################################
#
@@ -120,8 +119,8 @@ mn10300_dcache_flush_range_end:
# Flush the entire data cache and invalidate all entries
#
###############################################################################
- ALIGN
-mn10300_dcache_flush_inv:
+
+PROC(mn10300_dcache_flush_inv)
movhu (CHCTR),d0
btst CHCTR_DCEN,d0
beq mn10300_dcache_flush_inv_end
@@ -139,6 +138,7 @@ mn10300_dcache_flush_inv_loop:
mn10300_dcache_flush_inv_end:
ret [],0
+ENDPROC(mn10300_dcache_flush_inv)
###############################################################################
#
@@ -148,8 +148,8 @@ mn10300_dcache_flush_inv_end:
# Flush and invalidate a range of addresses on a page in the dcache
#
###############################################################################
- ALIGN
-mn10300_dcache_flush_inv_page:
+
+PROC(mn10300_dcache_flush_inv_page)
mov PAGE_SIZE,d1
mn10300_dcache_flush_inv_range2:
add d0,d1
@@ -190,3 +190,4 @@ mn10300_dcache_flush_inv_range_loop:
mn10300_dcache_flush_inv_range_end:
ret [d2,d3],8
+ENDPROC(mn10300_dcache_flush_inv_page)
On Wed, Dec 17, 2008 at 12:17 PM, Alexander van Heukelum
<[email protected]> wrote:
> [ DO NOT APPLY (yet...) At this point this patch will
> just cause the build to abort due to annotation errors
> found. ]
>
> Introduce a checking framework to check correct pairing
> of ENTRY/END and PROC/ENDPROC. It also checks that the
> annotations are not nested. I have used the ideas and
> most of the implementation from Cyrill Gorcunov who
> introduced the framework to check for mismatching
> KPROBE_ENTRY annotations, which was however soon made
> obsolete by the removal of KPROBE_ENTRY/KPROBE_END.
>
> Checks performed:
> o END must terminate an ENTRY annotation
> o ENDPROC must terminate a PROC annotation
> o ENTRY or PROC cannot be nested inside
> another ENTRY or PROC section.
>
> Finally the macro ENTRY_PROC_FINAL is introduced to
> enable checking correct closing of PROC and ENTRY
> sections at the end of assembly files.
>
> Signed-off-by: Alexander van Heukelum <[email protected]>
> Cc: Cyrill Gorcunov <[email protected]>
...
Thanks Alexander!
You know I think you meant __ASSEMBLY__ while
were typing __ASSEMBLER__. Don't you? :)
On Wed, Dec 17, 2008 at 02:51:53PM +0300, Cyrill Gorcunov wrote:
> On Wed, Dec 17, 2008 at 12:17 PM, Alexander van Heukelum
> <[email protected]> wrote:
> > [ DO NOT APPLY (yet...) At this point this patch will
> > just cause the build to abort due to annotation errors
> > found. ]
> >
> > Introduce a checking framework to check correct pairing
> > of ENTRY/END and PROC/ENDPROC. It also checks that the
> > annotations are not nested. I have used the ideas and
> > most of the implementation from Cyrill Gorcunov who
> > introduced the framework to check for mismatching
> > KPROBE_ENTRY annotations, which was however soon made
> > obsolete by the removal of KPROBE_ENTRY/KPROBE_END.
> >
> > Checks performed:
> > o END must terminate an ENTRY annotation
> > o ENDPROC must terminate a PROC annotation
> > o ENTRY or PROC cannot be nested inside
> > another ENTRY or PROC section.
> >
> > Finally the macro ENTRY_PROC_FINAL is introduced to
> > enable checking correct closing of PROC and ENTRY
> > sections at the end of assembly files.
> >
> > Signed-off-by: Alexander van Heukelum <[email protected]>
> > Cc: Cyrill Gorcunov <[email protected]>
> ...
>
> Thanks Alexander!
>
> You know I think you meant __ASSEMBLY__ while
> were typing __ASSEMBLER__. Don't you? :)
If it matters there is some cleanup to do in the kernel tree ;).
Greetings,
Alexander
[Alexander van Heukelum - Wed, Dec 17, 2008 at 01:04:50PM +0100]
...
| > Thanks Alexander!
| >
| > You know I think you meant __ASSEMBLY__ while
| > were typing __ASSEMBLER__. Don't you? :)
|
| If it matters there is some cleanup to do in the kernel tree ;).
|
| Greetings,
| Alexander
|
I somehow missed the line the pointed to not apply this patch
that is why I was amaized since I dont remember where we use
__ASSEMBLER__. Got it :)
- Cyrill -
On Wed, Dec 17, 2008 at 10:17:54AM +0100, Alexander van Heukelum wrote:
> Introduce the PROC macro in the generic header file
> include/linux/linkage.h to annotate functions in assembly
> files. This is a first step to fully annotate functions
> (procedures) in .S-files. The PROC macro complements the
> already existing and being used ENDPROC macro. The generic
> implementation of PROC is exactly the same as ENTRY.
>
> The goal is to annotate functions, at least those called
> from C code, with PROC at the beginning and ENDPROC at the
> end. This is for the benefit of debugging and tracing. It
> will also allow to introduce a framework to check for
> nesting problems and missing annotations in a later stage
> by overriding ENTRY/END and PROC/ENDPROC in architecture-
> specific code, after the annotation errors have been fixed.
>
> Signed-off-by: Alexander van Heukelum <[email protected]>
> Cc: Sam Ravnborg <[email protected]>
> Cc: Andrew Morton <[email protected]>
I understand where you are coming from with these.
But what I see now is:
ENTRY/END
PROC/ENDPROC
KPROBE_ENTRY/KPROBE_END
And it is not obvious for me reading the comment when I should
expect which one to be used.
Could we try to keep it down to two variants?
And then document when to use which one.
Sam
[Sam Ravnborg - Wed, Dec 17, 2008 at 06:26:40PM +0100]
| On Wed, Dec 17, 2008 at 10:17:54AM +0100, Alexander van Heukelum wrote:
| > Introduce the PROC macro in the generic header file
| > include/linux/linkage.h to annotate functions in assembly
| > files. This is a first step to fully annotate functions
| > (procedures) in .S-files. The PROC macro complements the
| > already existing and being used ENDPROC macro. The generic
| > implementation of PROC is exactly the same as ENTRY.
| >
| > The goal is to annotate functions, at least those called
| > from C code, with PROC at the beginning and ENDPROC at the
| > end. This is for the benefit of debugging and tracing. It
| > will also allow to introduce a framework to check for
| > nesting problems and missing annotations in a later stage
| > by overriding ENTRY/END and PROC/ENDPROC in architecture-
| > specific code, after the annotation errors have been fixed.
| >
| > Signed-off-by: Alexander van Heukelum <[email protected]>
| > Cc: Sam Ravnborg <[email protected]>
| > Cc: Andrew Morton <[email protected]>
|
| I understand where you are coming from with these.
| But what I see now is:
|
| ENTRY/END
| PROC/ENDPROC
| KPROBE_ENTRY/KPROBE_END
|
| And it is not obvious for me reading the comment when I should
| expect which one to be used.
|
| Could we try to keep it down to two variants?
| And then document when to use which one.
|
| Sam
|
Sam, I think eventually we should get something like this:
- KPROBE will be eliminated and explicit section descriptions
are to be used
- ENTRY could be used / or renamed for something more descriptive
and being used aligned jmp targets or in case of procs with
shared body
- PROC/ENDPROC are to replace old ENTRY/END for procs being called
mostly from C code
Did I miss something? Does it sound like a good/bad plan?
- Cyrill -
On Wed, Dec 17, 2008 at 08:38:24PM +0300, Cyrill Gorcunov wrote:
> [Sam Ravnborg - Wed, Dec 17, 2008 at 06:26:40PM +0100]
> | On Wed, Dec 17, 2008 at 10:17:54AM +0100, Alexander van Heukelum wrote:
> | > Introduce the PROC macro in the generic header file
> | > include/linux/linkage.h to annotate functions in assembly
> | > files. This is a first step to fully annotate functions
> | > (procedures) in .S-files. The PROC macro complements the
> | > already existing and being used ENDPROC macro. The generic
> | > implementation of PROC is exactly the same as ENTRY.
> | >
> | > The goal is to annotate functions, at least those called
> | > from C code, with PROC at the beginning and ENDPROC at the
> | > end. This is for the benefit of debugging and tracing. It
> | > will also allow to introduce a framework to check for
> | > nesting problems and missing annotations in a later stage
> | > by overriding ENTRY/END and PROC/ENDPROC in architecture-
> | > specific code, after the annotation errors have been fixed.
> | >
> | > Signed-off-by: Alexander van Heukelum <[email protected]>
> | > Cc: Sam Ravnborg <[email protected]>
> | > Cc: Andrew Morton <[email protected]>
> |
> | I understand where you are coming from with these.
> | But what I see now is:
> |
> | ENTRY/END
> | PROC/ENDPROC
> | KPROBE_ENTRY/KPROBE_END
> |
> | And it is not obvious for me reading the comment when I should
> | expect which one to be used.
> |
> | Could we try to keep it down to two variants?
> | And then document when to use which one.
> |
> | Sam
> |
>
> Sam, I think eventually we should get something like this:
>
> - KPROBE will be eliminated and explicit section descriptions
> are to be used
> - ENTRY could be used / or renamed for something more descriptive
> and being used aligned jmp targets or in case of procs with
> shared body
> - PROC/ENDPROC are to replace old ENTRY/END for procs being called
> mostly from C code
So what prevents us from extending ENTRY/END instead of introducing
another set?
Let us try to extend what we have and not introduce something new.
Sam
[Sam Ravnborg - Wed, Dec 17, 2008 at 07:00:23PM +0100]
...
| > Sam, I think eventually we should get something like this:
| >
| > - KPROBE will be eliminated and explicit section descriptions
| > are to be used
| > - ENTRY could be used / or renamed for something more descriptive
| > and being used aligned jmp targets or in case of procs with
| > shared body
| > - PROC/ENDPROC are to replace old ENTRY/END for procs being called
| > mostly from C code
|
| So what prevents us from extending ENTRY/END instead of introducing
| another set?
| Let us try to extend what we have and not introduce something new.
|
| Sam
|
It could disable us to make such a conversion step-by-step I think.
Of course it would be better to just extend ENTRY/END (since already
there) and we could even restrict it to X86 only at the beginning
but even then we have to check all ENTRY/END that they are used properly
(ie like a procedure markers having @function attribute). Not sure
what would be better. And btw ENDPROC is more descriptive then plain END :)
- Cyrill -
On Wed, 17 Dec 2008 18:26:40 +0100, "Sam Ravnborg" <[email protected]>
said:
> On Wed, Dec 17, 2008 at 10:17:54AM +0100, Alexander van Heukelum wrote:
> > Introduce the PROC macro in the generic header file
> > include/linux/linkage.h to annotate functions in assembly
> > files. This is a first step to fully annotate functions
> > (procedures) in .S-files. The PROC macro complements the
> > already existing and being used ENDPROC macro. The generic
> > implementation of PROC is exactly the same as ENTRY.
> >
> > The goal is to annotate functions, at least those called
> > from C code, with PROC at the beginning and ENDPROC at the
> > end. This is for the benefit of debugging and tracing. It
> > will also allow to introduce a framework to check for
> > nesting problems and missing annotations in a later stage
> > by overriding ENTRY/END and PROC/ENDPROC in architecture-
> > specific code, after the annotation errors have been fixed.
> >
> > Signed-off-by: Alexander van Heukelum <[email protected]>
> > Cc: Sam Ravnborg <[email protected]>
> > Cc: Andrew Morton <[email protected]>
>
> I understand where you are coming from with these.
> But what I see now is:
>
> ENTRY/END
> PROC/ENDPROC
> KPROBE_ENTRY/KPROBE_END
>
> And it is not obvious for me reading the comment when I should
> expect which one to be used.
>
> Could we try to keep it down to two variants?
> And then document when to use which one.
Hi Sam,
Patches to remove KPROBE_ENTRY and KPROBE_END are currently
in the x86-tree.
PROC/ENDPROC should be used around code.
ENTRY/END should be used around objects which are never
executed, like data arrays.
That should be clear enough, I think. The only catch is how
to decide where to use the annotations at all. I have no
complete answer to that. However, in general it would be
good to annotate any object for which an address can be
found in registers, on the stack or in variables. Doubly
so if addresses are within the object but not exactly at
the start of it. This includes assembly functions called
from C (instruction pointer) and arrays.
The macro's are used to align code and data, to identify the
object as code or data and to add object size information to
the debugging info.
Greetings,
Alexander
> Sam
--
Alexander van Heukelum
[email protected]
--
http://www.fastmail.fm - Access your email from home and the web
On Wed, 17 Dec 2008 19:00:23 +0100, "Sam Ravnborg" <[email protected]>
said:
> On Wed, Dec 17, 2008 at 08:38:24PM +0300, Cyrill Gorcunov wrote:
> > [Sam Ravnborg - Wed, Dec 17, 2008 at 06:26:40PM +0100]
> > | On Wed, Dec 17, 2008 at 10:17:54AM +0100, Alexander van Heukelum wrote:
> > | > Introduce the PROC macro in the generic header file
> > | > include/linux/linkage.h to annotate functions in assembly
> > | > files. This is a first step to fully annotate functions
> > | > (procedures) in .S-files. The PROC macro complements the
> > | > already existing and being used ENDPROC macro. The generic
> > | > implementation of PROC is exactly the same as ENTRY.
> > | >
> > | > The goal is to annotate functions, at least those called
> > | > from C code, with PROC at the beginning and ENDPROC at the
> > | > end. This is for the benefit of debugging and tracing. It
> > | > will also allow to introduce a framework to check for
> > | > nesting problems and missing annotations in a later stage
> > | > by overriding ENTRY/END and PROC/ENDPROC in architecture-
> > | > specific code, after the annotation errors have been fixed.
> > | >
> > | > Signed-off-by: Alexander van Heukelum <[email protected]>
> > | > Cc: Sam Ravnborg <[email protected]>
> > | > Cc: Andrew Morton <[email protected]>
> > |
> > | I understand where you are coming from with these.
> > | But what I see now is:
> > |
> > | ENTRY/END
> > | PROC/ENDPROC
> > | KPROBE_ENTRY/KPROBE_END
> > |
> > | And it is not obvious for me reading the comment when I should
> > | expect which one to be used.
> > |
> > | Could we try to keep it down to two variants?
> > | And then document when to use which one.
> > |
> > | Sam
> > |
> >
> > Sam, I think eventually we should get something like this:
> >
> > - KPROBE will be eliminated and explicit section descriptions
> > are to be used
> > - ENTRY could be used / or renamed for something more descriptive
> > and being used aligned jmp targets or in case of procs with
> > shared body
I don't think ENTRY should be used for nested procedures. If the
author wants to do something like that, he better knew something
about the assembler anyhow.
> > - PROC/ENDPROC are to replace old ENTRY/END for procs being called
> > mostly from C code
Currently there is many different patterns. Some functions use ENTRY
without END, some use ENTRY/ENDPROC, some use ENDPROC without annotation
at the start...
> So what prevents us from extending ENTRY/END instead of introducing
> another set?
ENTRY/END alone is not enough if one wants to be able to distinguish
between code (functions) and non-executed data.
> Let us try to extend what we have and not introduce something new.
Agreed. I vote to complement the existing ENDPROC annotation with
the proposed PROC annotation. Let's call that an extension, not
something new ;). As it stands it is not impossible to go with
ENTRY/ENDPROC for code and ENTRY/END for data. However, ENTRY
implies alignment and the prefered alignment for code and data
might differ.
Greetings,
Alexander
> Sam
--
Alexander van Heukelum
[email protected]
--
http://www.fastmail.fm - Access your email from home and the web
On Thu, Dec 18, 2008 at 10:51:58AM +0100, Alexander van Heukelum wrote:
> Agreed. I vote to complement the existing ENDPROC annotation with
> the proposed PROC annotation. Let's call that an extension, not
> something new ;). As it stands it is not impossible to go with
> ENTRY/ENDPROC for code and ENTRY/END for data. However, ENTRY
> implies alignment and the prefered alignment for code and data
> might differ.
Have you looked at the number of ENTRY uses for code vs for data?
If all you're after is separating the two uses, then it might be a
smaller patch to change the ENTRY use for data rather than changing
all the ENTRY uses for code.
There are 589 uses of ENTRY in arch/arm/*/*.S. Of those about 50
aren't called code.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
>>> "Alexander van Heukelum" <[email protected]> 18.12.08 10:51 >>>
>Agreed. I vote to complement the existing ENDPROC annotation with
>the proposed PROC annotation. Let's call that an extension, not
>something new ;). As it stands it is not impossible to go with
>ENTRY/ENDPROC for code and ENTRY/END for data. However, ENTRY
Not really: At least on ia64 these cannot be mixed (and there as well as
any other architectures that may have such requirements) replacing
ENTRY() with PROC() and END() with ENDPROC() will likely be necessary.
>implies alignment and the prefered alignment for code and data
>might differ.
Jan
On Thu, 18 Dec 2008 10:07:47 +0000, "Russell King"
<[email protected]> said:
> On Thu, Dec 18, 2008 at 10:51:58AM +0100, Alexander van Heukelum wrote:
> > Agreed. I vote to complement the existing ENDPROC annotation with
> > the proposed PROC annotation. Let's call that an extension, not
> > something new ;). As it stands it is not impossible to go with
> > ENTRY/ENDPROC for code and ENTRY/END for data. However, ENTRY
> > implies alignment and the prefered alignment for code and data
> > might differ.
>
> Have you looked at the number of ENTRY uses for code vs for data?
> If all you're after is separating the two uses, then it might be a
> smaller patch to change the ENTRY use for data rather than changing
> all the ENTRY uses for code.
>
> There are 589 uses of ENTRY in arch/arm/*/*.S. Of those about 50
> aren't called code.
Hi,
Things are similar for x86, but I didn't consider it a problem.
The alternative I see is to is to introduce DATAENTRY and DATAEND
for use with data objects in generic code, equal to ENTRY/END. Then
deprecate the use of ENDPROC, so we can try to get rid of it in the
long run.
Minor nit is that all archs need to override ENTRY and/or END to
include an assembly directive that indicates that the symbol is
a function. Changing this in generic code is not possible as long
as there are ARCHs which have not been converted.
ENDPROC might stick for a very long time, though.
Greetings,
Alexander
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of:
--
Alexander van Heukelum
[email protected]
--
http://www.fastmail.fm - A fast, anti-spam email service.
On Wed, Dec 17, 2008 at 12:12:14PM +0100, Alexander van Heukelum wrote:
> Yeah, assembly files contain some interesting nesting. In this
> particular case I think the solution is simple... Just use PROC
> and ENDPROC around the complete functions, and leave the explicit
> .global's for the additional entry points.
I'm sorry, that doesn't work in all cases.
On ARM with later toolchains, there's additional metadata associated with
every symbol, and it's beginning to matter getting this right. That
metadata includes whether it's a function, and more importantly whether
the code pointed to by the symbol is Thumb or ARM.
This leads to:
ENTRY(__ashldi3)
ENTRY(__aeabi_llsl)
...
ENDPROC(__ashldi3)
ENDPROC(__aeabi_llsl)
and we want both of those symbols to have exactly the same attributes.
Merely adding a .globl for the second name doesn't do that. It needs
.globl, .size, and .type.
So what you're actually talking about using your approach is enforcing
the pairing of the existing ENTRY/ENDPROC and open coding everything
else.
Forgive me if I think this is a backward step. It certainly seems to
add some insane restrictions.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Thu, Dec 18, 2008 at 12:51 PM, Alexander van Heukelum
<[email protected]> wrote:
[...]
>> >
>> > Sam, I think eventually we should get something like this:
>> >
>> > - KPROBE will be eliminated and explicit section descriptions
>> > are to be used
>> > - ENTRY could be used / or renamed for something more descriptive
>> > and being used aligned jmp targets or in case of procs with
>> > shared body
>
> I don't think ENTRY should be used for nested procedures. If the
> author wants to do something like that, he better knew something
> about the assembler anyhow.
Author anyway have to knew something. We can't bring some kind
of lexical machine that eliminate this needing :)
>
>> > - PROC/ENDPROC are to replace old ENTRY/END for procs being called
>> > mostly from C code
>
> Currently there is many different patterns. Some functions use ENTRY
> without END, some use ENTRY/ENDPROC, some use ENDPROC without annotation
> at the start...
Alexander, I was just trying to say Sam about what we're planning to get
at the end of all this procedure. I mean I know there are some issues to
be fixed first.
Fix me if I'm wrong.
>
>> So what prevents us from extending ENTRY/END instead of introducing
>> another set?
>
> ENTRY/END alone is not enough if one wants to be able to distinguish
> between code (functions) and non-executed data.
>
>> Let us try to extend what we have and not introduce something new.
>
> Agreed. I vote to complement the existing ENDPROC annotation with
> the proposed PROC annotation. Let's call that an extension, not
> something new ;). As it stands it is not impossible to go with
> ENTRY/ENDPROC for code and ENTRY/END for data. However, ENTRY
> implies alignment and the prefered alignment for code and data
> might differ.
If ENTRY will be used for data objects it shouldn't contain any kind of
alignment since in general we could have arrays of bytes, words and so on.
>
> Greetings,
> Alexander
>
>> Sam
> --
> Alexander van Heukelum
> [email protected]
On Thu, 18 Dec 2008 11:44:27 +0000, "Russell King"
<[email protected]> said:
> On Wed, Dec 17, 2008 at 12:12:14PM +0100, Alexander van Heukelum wrote:
> > Yeah, assembly files contain some interesting nesting. In this
> > particular case I think the solution is simple... Just use PROC
> > and ENDPROC around the complete functions, and leave the explicit
> > .global's for the additional entry points.
>
> I'm sorry, that doesn't work in all cases.
>
> On ARM with later toolchains, there's additional metadata associated with
> every symbol, and it's beginning to matter getting this right. That
> metadata includes whether it's a function, and more importantly whether
> the code pointed to by the symbol is Thumb or ARM.
>
> This leads to:
>
> ENTRY(__ashldi3)
> ENTRY(__aeabi_llsl)
>
> ...
>
> ENDPROC(__ashldi3)
> ENDPROC(__aeabi_llsl)
>
> and we want both of those symbols to have exactly the same attributes.
>
> Merely adding a .globl for the second name doesn't do that. It needs
> .globl, .size, and .type.
>
> So what you're actually talking about using your approach is enforcing
> the pairing of the existing ENTRY/ENDPROC and open coding everything
> else.
Note that enforcing the pairing will be enabled by ARCH code. Is the
construct you show here (two symbols covering identical code) the only
problem you forsee? I don't want to introduce too many macro's to
handle special cases, but this one should be solved.
> Forgive me if I think this is a backward step. It certainly seems to
> add some insane restrictions.
Some restrictions are introduced, indeed. And I agree that evading the
checking framework by doing things manually should be avoided.
Greetings,
Alexander
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of:
--
Alexander van Heukelum
[email protected]
--
http://www.fastmail.fm - The way an email service should be
On Thu, 18 Dec 2008 15:03:25 +0300, "Cyrill Gorcunov"
<[email protected]> said:
> On Thu, Dec 18, 2008 at 12:51 PM, Alexander van Heukelum
> <[email protected]> wrote:
> [...]
> >> >
> >> > Sam, I think eventually we should get something like this:
> >> >
> >> > - KPROBE will be eliminated and explicit section descriptions
> >> > are to be used
> >> > - ENTRY could be used / or renamed for something more descriptive
> >> > and being used aligned jmp targets or in case of procs with
> >> > shared body
> >
> > I don't think ENTRY should be used for nested procedures. If the
> > author wants to do something like that, he better knew something
> > about the assembler anyhow.
>
> Author anyway have to knew something. We can't bring some kind
> of lexical machine that eliminate this needing :)
>
> >
> >> > - PROC/ENDPROC are to replace old ENTRY/END for procs being called
> >> > mostly from C code
> >
> > Currently there is many different patterns. Some functions use ENTRY
> > without END, some use ENTRY/ENDPROC, some use ENDPROC without annotation
> > at the start...
>
> Alexander, I was just trying to say Sam about what we're planning to get
> at the end of all this procedure. I mean I know there are some issues to
> be fixed first.
I understood, but I wanted to avoid the meme that this procedure is
just ebout renaming ENTRY->PROC and END->ENDPROC ;).
> Fix me if I'm wrong.
>
> >
> >> So what prevents us from extending ENTRY/END instead of introducing
> >> another set?
> >
> > ENTRY/END alone is not enough if one wants to be able to distinguish
> > between code (functions) and non-executed data.
> >
> >> Let us try to extend what we have and not introduce something new.
> >
> > Agreed. I vote to complement the existing ENDPROC annotation with
> > the proposed PROC annotation. Let's call that an extension, not
> > something new ;). As it stands it is not impossible to go with
> > ENTRY/ENDPROC for code and ENTRY/END for data. However, ENTRY
> > implies alignment and the prefered alignment for code and data
> > might differ.
>
> If ENTRY will be used for data objects it shouldn't contain any kind of
> alignment since in general we could have arrays of bytes, words and so
> on.
I would suggest using sizeof(long) alignment for data.
Greetings,
Alexander
--
Alexander van Heukelum
[email protected]
--
http://www.fastmail.fm - mmm... Fastmail...
* Sam Ravnborg <[email protected]> wrote:
> On Wed, Dec 17, 2008 at 10:17:54AM +0100, Alexander van Heukelum wrote:
> > Introduce the PROC macro in the generic header file
> > include/linux/linkage.h to annotate functions in assembly
> > files. This is a first step to fully annotate functions
> > (procedures) in .S-files. The PROC macro complements the
> > already existing and being used ENDPROC macro. The generic
> > implementation of PROC is exactly the same as ENTRY.
> >
> > The goal is to annotate functions, at least those called
> > from C code, with PROC at the beginning and ENDPROC at the
> > end. This is for the benefit of debugging and tracing. It
> > will also allow to introduce a framework to check for
> > nesting problems and missing annotations in a later stage
> > by overriding ENTRY/END and PROC/ENDPROC in architecture-
> > specific code, after the annotation errors have been fixed.
> >
> > Signed-off-by: Alexander van Heukelum <[email protected]>
> > Cc: Sam Ravnborg <[email protected]>
> > Cc: Andrew Morton <[email protected]>
>
> I understand where you are coming from with these.
> But what I see now is:
>
> ENTRY/END
> PROC/ENDPROC
> KPROBE_ENTRY/KPROBE_END
btw., KPROBE_ENTRY is an ugly euphemism. It should be renamed to
NOKPROBE_ENTRY/NOKPROBE_END.
Ingo
On Thu, Dec 18, 2008 at 01:35:02PM +0100, Alexander van Heukelum wrote:
> Note that enforcing the pairing will be enabled by ARCH code. Is the
> construct you show here (two symbols covering identical code) the only
> problem you forsee? I don't want to introduce too many macro's to
> handle special cases, but this one should be solved.
Well, there are some cases where we don't have ENTRY but do have ENDPROC:
__pabt_usr:
usr_entry
...
ENTRY(ret_from_exception)
...
ENDPROC(__pabt_usr)
ENDPROC(ret_from_exception)
.macro vector_stub, name, mode, correction=0
.align 5
vector_\name:
...
ENDPROC(vector_\name)
.endm
Not sure if we have any other oddities.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
[Alexander van Heukelum - Thu, Dec 18, 2008 at 01:40:55PM +0100]
|
| On Thu, 18 Dec 2008 15:03:25 +0300, "Cyrill Gorcunov"
| <[email protected]> said:
| > On Thu, Dec 18, 2008 at 12:51 PM, Alexander van Heukelum
| > <[email protected]> wrote:
| > [...]
| > >> >
| > >> > Sam, I think eventually we should get something like this:
| > >> >
| > >> > - KPROBE will be eliminated and explicit section descriptions
| > >> > are to be used
| > >> > - ENTRY could be used / or renamed for something more descriptive
| > >> > and being used aligned jmp targets or in case of procs with
| > >> > shared body
| > >
| > > I don't think ENTRY should be used for nested procedures. If the
| > > author wants to do something like that, he better knew something
| > > about the assembler anyhow.
| >
| > Author anyway have to knew something. We can't bring some kind
| > of lexical machine that eliminate this needing :)
| >
| > >
| > >> > - PROC/ENDPROC are to replace old ENTRY/END for procs being called
| > >> > mostly from C code
| > >
| > > Currently there is many different patterns. Some functions use ENTRY
| > > without END, some use ENTRY/ENDPROC, some use ENDPROC without annotation
| > > at the start...
| >
| > Alexander, I was just trying to say Sam about what we're planning to get
| > at the end of all this procedure. I mean I know there are some issues to
| > be fixed first.
|
| I understood, but I wanted to avoid the meme that this procedure is
| just ebout renaming ENTRY->PROC and END->ENDPROC ;).
I wish it would be just renaming :-)
|
| > Fix me if I'm wrong.
| >
| > >
| > >> So what prevents us from extending ENTRY/END instead of introducing
| > >> another set?
| > >
| > > ENTRY/END alone is not enough if one wants to be able to distinguish
| > > between code (functions) and non-executed data.
| > >
| > >> Let us try to extend what we have and not introduce something new.
| > >
| > > Agreed. I vote to complement the existing ENDPROC annotation with
| > > the proposed PROC annotation. Let's call that an extension, not
| > > something new ;). As it stands it is not impossible to go with
| > > ENTRY/ENDPROC for code and ENTRY/END for data. However, ENTRY
| > > implies alignment and the prefered alignment for code and data
| > > might differ.
| >
| > If ENTRY will be used for data objects it shouldn't contain any kind of
| > alignment since in general we could have arrays of bytes, words and so
| > on.
|
| I would suggest using sizeof(long) alignment for data.
Maybe we could use more flexible scheme? Lets imagine we could
need to use not sizeof(long) on x86-64 for example... say in
boot/compressed/head_64.S... say for boot_heap. Should we use
DATAENTRY here? Or it's planned to use DATAENTRY for global defs
only? Alexander, don't get me wrong I'm just starting to confuse
with ENRTY/PROC/DATAENTRY :)
Letme try to classify them a bit (like they would be used in future).
If we have them classified it would be easier to distinguish their
usage and how they should be implemented
- PROC/ENDPROC for "C" callers
- they are: aligned, .global, .size and .type and @function
- DATA/ENDDATA for data objects
- they are: aligned as sizeof(long), .global, .size
right?
|
| Greetings,
| Alexander
| --
| Alexander van Heukelum
| [email protected]
- Cyrill -