kbuild: CRC versions for asm functions

scripts/Makefile.build +5 -0

+# .S file exports must have their C prototypes defined in asm/asm-prototypes.h
+# or a file that it includes, in order to get versioned symbols. We build a
+# dummy C file that includes asm-prototypes and the EXPORT_SYMBOL lines from
+# the .S file (with trailing ';'), and run genksyms on that, to extract vers.
+#

Dne 15.10.2016 v 14:43 Nicholas Piggin napsal(a):

Quoted 12 lines

scripts/Makefile.build +12 -0

+# .S file exports must have their C prototypes defined in asm/asm-prototypes.h
+# or a file that it includes, in order to get versioned symbols. We build a
+# dummy C file that includes asm-prototypes and the EXPORT_SYMBOL lines from
+# the .S file (with trailing ';'), and run genksyms on that, to extract vers.
+#
+# These mirror gensymtypes_c and co above, keep them in synch.
+cmd_gensymtypes_S =                                                         \
+    (echo "\#include <linux/kernel.h>" ;                                    \
+     echo "\#include <asm/asm-prototypes.h>" ;                              \
+     grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) |                              \
+    $(CPP) -D__GENKSYMS__ $(c_flags) -xc - |                                \
+    $(GENKSYMS) $(if $(1), -T $(2))                                         \

scripts/Makefile.build modkern_aflags := $(KBUILD_AFLAGS_KERNEL) $(AFLAGS_KERNEL) · line 334

Quoted 3 lines

scripts/Makefile.build +3 -0

+     $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))             \
+     $(if $(KBUILD_PRESERVE),-p)                                            \
+     -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))

I think it would be cleaner to add the #include to the .S files themselves and grep for both EXPORT_SYMBOL and #include here. The reason is that some files might need additional #includes to allow genksyms to properly expand some function prototypes.

Quoted 13 lines · CEST Michal Marek

Dne 15.10.2016 v 14:43 Nicholas Piggin napsal(a):

Quoted 12 lines

scripts/Makefile.build +12 -0

+# .S file exports must have their C prototypes defined in asm/asm-prototypes.h
+# or a file that it includes, in order to get versioned symbols. We build a
+# dummy C file that includes asm-prototypes and the EXPORT_SYMBOL lines from
+# the .S file (with trailing ';'), and run genksyms on that, to extract vers.
+#
+# These mirror gensymtypes_c and co above, keep them in synch.
+cmd_gensymtypes_S =                                                         \
+    (echo "\#include <linux/kernel.h>" ;                                    \
+     echo "\#include <asm/asm-prototypes.h>" ;                              \
+     grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) |                              \
+    $(CPP) -D__GENKSYMS__ $(c_flags) -xc - |                                \
+    $(GENKSYMS) $(if $(1), -T $(2))                                         \

Quoted 9 lines

scripts/Makefile.build modkern_aflags := $(KBUILD_AFLAGS_KERNEL) $(AFLAGS_KERNEL) · line 334

Quoted 3 lines

scripts/Makefile.build +3 -0

+     $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))             \
+     $(if $(KBUILD_PRESERVE),-p)                                            \
+     -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))  

I think it would be cleaner to add the #include to the .S files themselves and grep for both EXPORT_SYMBOL and #include here. The reason is that some files might need additional #includes to allow genksyms to properly expand some function prototypes.

This is something I tried earlier, and it wasn't pretty: Some of the assembler files rely on -D__ASSEMBLER__ to be set in order to read the right headers, but setting that macro means that all of the declarations get skipped.

I ended up testing for -D__GENKSYMS__ in each .S file, which was also rather ugly.

Yeah, I had the same idea as you and Michal too. It's conceptually nicer, but in practice it turned into a mess. If some architectures wanted to start protecting their .h files and including them into .S for the prototypes, we could start parsing those too. Should we do the quick and dirty way for 4.9?

Quoted 5 lines

Yeah, I had the same idea as you and Michal too. It's conceptually nicer, but in practice it turned into a mess. If some architectures wanted to start protecting their .h files and including them into .S for the prototypes, we could start parsing those too. Should we do the quick and dirty way for 4.9?

Let's stay with your approach for now.

Dne 20.10.2016 v 10:03 Arnd Bergmann napsal(a):

Quoted 8 lines · CEST Nicholas Piggin

Quoted 5 lines

Yeah, I had the same idea as you and Michal too. It's conceptually nicer, but in practice it turned into a mess. If some architectures wanted to start protecting their .h files and including them into .S for the prototypes, we could start parsing those too. Should we do the quick and dirty way for 4.9?

Let's stay with your approach for now.

Agreed.

     $(CPP) -D__GENKSYMS__ $(c_flags) -xc - |                                \
     $(GENKSYMS) $(if $(1), -T $(2))                                         \
      $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))             \

scripts/Makefile.build $(real-objs-m:.o=.s): modkern_aflags := $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE) · line 336

Quoted 13 lines · Nicholas Piggin

scripts/Makefile.build +7 -1

+# This is convoluted. The .S file must first be preprocessed to run guards and
+# expand names, then the resulting exports must be constructed into plain
+# EXPORT_SYMBOL(symbol); to build our dummy C file, and that gets preprocessed
+# to make the genksyms input.
 #
 # These mirror gensymtypes_c and co above, keep them in synch.
 cmd_gensymtypes_S =                                                         \
     (echo "\#include <linux/kernel.h>" ;                                    \
      echo "\#include <asm/asm-prototypes.h>" ;                              \
-     grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) |                              \
+    $(CPP) $(a_flags) $< |                                                  \
+     grep ^___EXPORT_SYMBOL |                                               \
+     sed 's/___EXPORT_SYMBOL \([a-zA-Z0-9_]*\),.*/EXPORT_SYMBOL(\1);/' ) |  \

Is this sed pass necessary? Just add -D__GENKSYMS__ also to the first cpp run and EXPORT_SYMBOL will stay intact.

Anyway, I'm going to merge your patch 2/2 now.

scripts/Makefile.build $(real-objs-m:.o=.s): modkern_aflags := $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE) · line 336

Quoted 13 lines · Nicholas Piggin

scripts/Makefile.build +7 -1

+# This is convoluted. The .S file must first be preprocessed to run guards and
+# expand names, then the resulting exports must be constructed into plain
+# EXPORT_SYMBOL(symbol); to build our dummy C file, and that gets preprocessed
+# to make the genksyms input.
 #
 # These mirror gensymtypes_c and co above, keep them in synch.
 cmd_gensymtypes_S =                                                         \
     (echo "\#include <linux/kernel.h>" ;                                    \
      echo "\#include <asm/asm-prototypes.h>" ;                              \
-     grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) |                              \
+    $(CPP) $(a_flags) $< |                                                  \
+     grep ^___EXPORT_SYMBOL |                                               \
+     sed 's/___EXPORT_SYMBOL \([a-zA-Z0-9_]*\),.*/EXPORT_SYMBOL(\1);/' ) |  \  

Is this sed pass necessary? Just add -D__GENKSYMS__ also to the first cpp run and EXPORT_SYMBOL will stay intact.

Anyway, I'm going to merge your patch 2/2 now.

Quoted 17 lines · On Tue, 1 Nov 2016 15:19:59 +0100 Michal Marek <[email protected]> wrote:

scripts/Makefile.build $(real-objs-m:.o=.s): modkern_aflags := $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE) · line 336

Quoted 13 lines · Nicholas Piggin

scripts/Makefile.build +7 -1

+# This is convoluted. The .S file must first be preprocessed to run guards and
+# expand names, then the resulting exports must be constructed into plain
+# EXPORT_SYMBOL(symbol); to build our dummy C file, and that gets preprocessed
+# to make the genksyms input.
 #
 # These mirror gensymtypes_c and co above, keep them in synch.
 cmd_gensymtypes_S =                                                         \
     (echo "\#include <linux/kernel.h>" ;                                    \
      echo "\#include <asm/asm-prototypes.h>" ;                              \
-     grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) |                              \
+    $(CPP) $(a_flags) $< |                                                  \
+     grep ^___EXPORT_SYMBOL |                                               \
+     sed 's/___EXPORT_SYMBOL \([a-zA-Z0-9_]*\),.*/EXPORT_SYMBOL(\1);/' ) |  \  

Is this sed pass necessary? Just add -D__GENKSYMS__ also to the first cpp run and EXPORT_SYMBOL will stay intact.

I can't see how it would work if asm-generic/export.h doesn't have any tests for GENKSYMS.

Quoted 17 lines · On Tue, 1 Nov 2016 15:19:59 +0100 Michal Marek <[email protected]> wrote:

scripts/Makefile.build $(real-objs-m:.o=.s): modkern_aflags := $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE) · line 336

Quoted 13 lines · Nicholas Piggin

scripts/Makefile.build +7 -1

+# This is convoluted. The .S file must first be preprocessed to run guards and
+# expand names, then the resulting exports must be constructed into plain
+# EXPORT_SYMBOL(symbol); to build our dummy C file, and that gets preprocessed
+# to make the genksyms input.
 #
 # These mirror gensymtypes_c and co above, keep them in synch.
 cmd_gensymtypes_S =                                                         \
     (echo "\#include <linux/kernel.h>" ;                                    \
      echo "\#include <asm/asm-prototypes.h>" ;                              \
-     grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) |                              \
+    $(CPP) $(a_flags) $< |                                                  \
+     grep ^___EXPORT_SYMBOL |                                               \
+     sed 's/___EXPORT_SYMBOL \([a-zA-Z0-9_]*\),.*/EXPORT_SYMBOL(\1);/' ) |  \  

Is this sed pass necessary? Just add -D__GENKSYMS__ also to the first cpp run and EXPORT_SYMBOL will stay intact.

I can't see how it would work if asm-generic/export.h doesn't have any tests for GENKSYMS.

Quoted 1 line

Anyway, I'm going to merge your patch 2/2 now.

Okay. We should come to some resolution of the preprocessing issue too. I'm logging off for tonight, but if you want to tweak or redo the incremental patch, feel free.