[PATCH] CONFIG_RELOCATABLE modpost fix

[PATCH] CONFIG_RELOCATABLE modpost fix

[Magnus Damm]

CONFIG_RELOCATABLE modpost fix

Run modpost on vmlinux regardless of CONFIG_MODULES. The modpost code is also
extended to ignore references from ".pci_fixup" to ".init.text".

Signed-off-by: Magnus Damm <magnus@valinux.co.jp>
---

 This patch applies on top of Eric W. Biedermans CONFIG_RELOCATABLE tree

 Makefile                 |    1 +
 scripts/Makefile         |    4 ++--
 scripts/Makefile.modpost |    6 +++++-
 scripts/mod/modpost.c    |   14 +++++++++++---

--- 0001/Makefile
+++ work/Makefile	2006-08-07 13:11:44.000000000 +0900
@@ -723,6 +723,7 @@ endif # ifdef CONFIG_KALLSYMS
 # vmlinux image - including updated kernel symbols
 vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) $(kallsyms.o) FORCE
 	$(call if_changed_rule,vmlinux__)
+	$(Q)$(MAKE) -rR -f $(srctree)/scripts/Makefile.modpost __modpost_kernel
 	$(Q)rm -f .old_version
 
 # The actual objects are generated when descending, 
--- 0001/scripts/Makefile
+++ work/scripts/Makefile	2006-08-07 13:09:47.000000000 +0900
@@ -16,7 +16,7 @@ hostprogs-$(CONFIG_IKCONFIG)     += bin2
 always		:= $(hostprogs-y)
 
 subdir-$(CONFIG_MODVERSIONS) += genksyms
-subdir-$(CONFIG_MODULES)     += mod
+subdir-y                     += mod
 
 # Let clean descend into subdirs
-subdir-	+= basic kconfig package
+subdir-	+= basic kconfig package mod
--- 0001/scripts/Makefile.modpost
+++ work/scripts/Makefile.modpost	2006-08-07 13:10:44.000000000 +0900
@@ -61,7 +61,11 @@ quiet_cmd_modpost = MODPOST
 	$(filter-out FORCE,$^)
 
 PHONY += __modpost
-__modpost: $(wildcard vmlinux) $(modules:.ko=.o) FORCE
+__modpost: $(modules:.ko=.o) FORCE
+	$(call cmd,modpost)
+
+PHONY += __modpost_kernel
+__modpost_kernel: $(wildcard vmlinux) FORCE
 	$(call cmd,modpost)
 
 # Declare generated files as targets for modpost
--- 0001/scripts/mod/modpost.c
+++ work/scripts/mod/modpost.c	2006-08-07 16:45:30.000000000 +0900
@@ -581,8 +581,8 @@ static int strrcmp(const char *s, const 
  *   fromsec = .data
  *   atsym = *driver, *_template, *_sht, *_ops, *_probe, *probe_one
  **/
-static int secref_whitelist(const char *tosec, const char *fromsec,
-			    const char *atsym)
+static int secref_whitelist(const char *modname, const char *tosec, 
+			    const char *fromsec, const char *atsym)
 {
 	int f1 = 1, f2 = 1;
 	const char **s;
@@ -596,6 +596,13 @@ static int secref_whitelist(const char *
 		NULL
 	};
 
+	/* Ignore all references from .pci_fixup section if vmlinux */
+	if (strcmp(modname, "vmlinux") == 0) {
+		if ((strcmp(fromsec, ".pci_fixup") == 0) && 
+		    (strcmp(tosec, ".init.text") == 0))
+		return 1;
+	}
+
 	/* Check for pattern 1 */
 	if (strcmp(tosec, ".init.data") != 0)
 		f1 = 0;
@@ -726,7 +733,8 @@ static void warn_sec_mismatch(const char
 
 	/* check whitelist - we may ignore it */
 	if (before &&
-	    secref_whitelist(secname, fromsec, elf->strtab + before->st_name))
+	    secref_whitelist(modname, secname, fromsec, 
+			     elf->strtab + before->st_name))
 		return;
 
 	if (before && after) {

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Sam Ravnborg]

On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> CONFIG_RELOCATABLE modpost fix
> 
> Run modpost on vmlinux regardless of CONFIG_MODULES. The modpost code is also
> extended to ignore references from ".pci_fixup" to ".init.text".
I've splitted the patch in two parts.
First is this one (I slightly modified your version and removed trailing
whitespaces).

	Sam

commit 1f43a633dc485f90fddf667270179058a07b9d77
Author: Magnus Damm <magnus@valinux.co.jp>
Date:   Tue Aug 8 17:32:11 2006 +0900

    kbuild: ignore references from ".pci_fixup" to ".init.text"
    
    The modpost code is extended to ignore references
    from ".pci_fixup" to ".init.text".
    
    Signed-off-by: Magnus Damm <magnus@valinux.co.jp>

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index dfde0e8..5028d46 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -581,8 +581,8 @@ static int strrcmp(const char *s, const 
  *   fromsec = .data
  *   atsym = *driver, *_template, *_sht, *_ops, *_probe, *probe_one
  **/
-static int secref_whitelist(const char *tosec, const char *fromsec,
-			    const char *atsym)
+static int secref_whitelist(const char *modname, const char *tosec,
+			    const char *fromsec, const char *atsym)
 {
 	int f1 = 1, f2 = 1;
 	const char **s;
@@ -618,8 +618,15 @@ static int secref_whitelist(const char *
 	for (s = pat2sym; *s; s++)
 		if (strrcmp(atsym, *s) == 0)
 			f1 = 1;
+	if (f1 && f2)
+		return 1;
 
-	return f1 && f2;
+	/* Whitelist all references from .pci_fixup section if vmlinux */
+	if (is_vmlinux(modname)) {
+		if ((strcmp(fromsec, ".pci_fixup") == 0) &&
+		    (strcmp(tosec, ".init.text") == 0))
+		return 1;
+	}
 }
 
 /**
@@ -726,7 +733,8 @@ static void warn_sec_mismatch(const char
 
 	/* check whitelist - we may ignore it */
 	if (before &&
-	    secref_whitelist(secname, fromsec, elf->strtab + before->st_name))
+	    secref_whitelist(modname, secname, fromsec,
+			     elf->strtab + before->st_name))
 		return;
 
 	if (before && after) {

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Sam Ravnborg]

On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> CONFIG_RELOCATABLE modpost fix
> 
> Run modpost on vmlinux regardless of CONFIG_MODULES.
Below is my take on this one.
- Dropped -rR since this is default now
- Dropped subdir- assignment in scripts/Makefile since it is redundant
- Always pass vmlinux ti modpost so we have full updated info
- Print out number of modules being mod posted to distingush from
  vmlinux one
- use vmlinux as target name to enable nicer quiet command print

	Sam

diff --git a/Makefile b/Makefile
index 72ef9f6..6b86f4e 100644
--- a/Makefile
+++ b/Makefile
@@ -724,6 +724,7 @@ endif # ifdef CONFIG_KALLSYMS
 # vmlinux image - including updated kernel symbols
 vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) $(kallsyms.o) FORCE
 	$(call if_changed_rule,vmlinux__)
+	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost $@
 	$(Q)rm -f .old_version
 
 # The actual objects are generated when descending, 
diff --git a/scripts/Makefile b/scripts/Makefile
index d531a1f..ea41de8 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -19,7 +19,7 @@ # The following hostprogs-y programs are
 hostprogs-y += unifdef
 
 subdir-$(CONFIG_MODVERSIONS) += genksyms
-subdir-$(CONFIG_MODULES)     += mod
+subdir-y                     += mod
 
 # Let clean descend into subdirs
 subdir-	+= basic kconfig package
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 0a64688..9137df2 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -51,19 +51,25 @@ _modpost: $(modules)
 
 # Step 2), invoke modpost
 #  Includes step 3,4
-quiet_cmd_modpost = MODPOST
+quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
       cmd_modpost = scripts/mod/modpost            \
         $(if $(CONFIG_MODVERSIONS),-m)             \
 	$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a,)  \
 	$(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
 	$(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
 	$(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
-	$(filter-out FORCE,$^)
+	$(wildcard vmlinux) $(filter-out FORCE,$^)
 
 PHONY += __modpost
-__modpost: $(wildcard vmlinux) $(modules:.ko=.o) FORCE
+__modpost: $(modules:.ko=.o) FORCE
 	$(call cmd,modpost)
 
+quiet_cmd_kernel-mod = MODPOST $@
+      cmd_kernel-mod = $(cmd_modpost)
+
+vmlinux: FORCE
+	$(call cmd,kernel-mod)
+
 # Declare generated files as targets for modpost
 $(symverfile):         __modpost ;
 $(modules:.ko=.mod.c): __modpost ;

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Eric W. Biederman]

Sam Ravnborg <sam@ravnborg.org> writes:

> On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
>> CONFIG_RELOCATABLE modpost fix
>> 
>> Run modpost on vmlinux regardless of CONFIG_MODULES.
> Below is my take on this one.
> - Dropped -rR since this is default now
> - Dropped subdir- assignment in scripts/Makefile since it is redundant
> - Always pass vmlinux ti modpost so we have full updated info
> - Print out number of modules being mod posted to distingush from
>   vmlinux one
> - use vmlinux as target name to enable nicer quiet command print

Sam, Magnus: 

I'm dense.  Why do we want to run modpost if we are building a kernel
that doesn't support modules?

I haven't mucked with modpost at all so I don't have a good feel for
what it does, or why we want to run it.

My quick skimming says modpost is all about generating the module
version symbol scrambling.  Which if that is all it does means it is
senseless to run this without modules.

Eric

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Sam Ravnborg]

On Tue, Aug 08, 2006 at 01:59:03PM -0600, Eric W. Biederman wrote:
 
> Sam, Magnus: 
> 
> I'm dense.  Why do we want to run modpost if we are building a kernel
> that doesn't support modules?
> 
> I haven't mucked with modpost at all so I don't have a good feel for
> what it does, or why we want to run it.
> 
> My quick skimming says modpost is all about generating the module
> version symbol scrambling.  Which if that is all it does means it is
> senseless to run this without modules.

Recently modpost has been enhanced to do section reference checks.
So modpost is used to check that there is no references from .text to
.init.text - the latter will be freed by the kernel so we better avoid
it.

The consistency checks does a bit more than just that simple check but
this was enough reason to run it twice in the build process.

	Sam

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Magnus Damm]

Hi Sam,

Thanks for picking up the patch!

On Tue, 2006-08-08 at 17:16 +0200, Sam Ravnborg wrote:
> On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> > CONFIG_RELOCATABLE modpost fix
> > 
> > Run modpost on vmlinux regardless of CONFIG_MODULES. The modpost code is also
> > extended to ignore references from ".pci_fixup" to ".init.text".
> I've splitted the patch in two parts.
> First is this one (I slightly modified your version and removed trailing
> whitespaces).
> 
> 	Sam
> 
> commit 1f43a633dc485f90fddf667270179058a07b9d77
> Author: Magnus Damm <magnus@valinux.co.jp>
> Date:   Tue Aug 8 17:32:11 2006 +0900
> 
>     kbuild: ignore references from ".pci_fixup" to ".init.text"
>     
>     The modpost code is extended to ignore references
>     from ".pci_fixup" to ".init.text".
>     
>     Signed-off-by: Magnus Damm <magnus@valinux.co.jp>
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index dfde0e8..5028d46 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -581,8 +581,8 @@ static int strrcmp(const char *s, const 
>   *   fromsec = .data
>   *   atsym = *driver, *_template, *_sht, *_ops, *_probe, *probe_one
>   **/
> -static int secref_whitelist(const char *tosec, const char *fromsec,
> -			    const char *atsym)
> +static int secref_whitelist(const char *modname, const char *tosec,
> +			    const char *fromsec, const char *atsym)
>  {
>  	int f1 = 1, f2 = 1;
>  	const char **s;
> @@ -618,8 +618,15 @@ static int secref_whitelist(const char *
>  	for (s = pat2sym; *s; s++)
>  		if (strrcmp(atsym, *s) == 0)
>  			f1 = 1;
> +	if (f1 && f2)
> +		return 1;
>  
> -	return f1 && f2;
> +	/* Whitelist all references from .pci_fixup section if vmlinux */
> +	if (is_vmlinux(modname)) {
> +		if ((strcmp(fromsec, ".pci_fixup") == 0) &&
> +		    (strcmp(tosec, ".init.text") == 0))
> +		return 1;
> +	}
>  }

You forget to return a value which result in the following warning:

  HOSTCC  scripts/mod/modpost.o
scripts/mod/modpost.c: In function `secref_whitelist':
scripts/mod/modpost.c:630: warning: control reaches end of non-void
function
  HOSTCC  scripts/mod/sumversion.o

Cheers,

/magnus

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Magnus Damm]

Hi again Sam,

On Tue, 2006-08-08 at 20:39 +0200, Sam Ravnborg wrote:
> On Tue, Aug 08, 2006 at 05:32:11PM +0900, Magnus Damm wrote:
> > CONFIG_RELOCATABLE modpost fix
> > 
> > Run modpost on vmlinux regardless of CONFIG_MODULES.
> Below is my take on this one.
> - Dropped -rR since this is default now
> - Dropped subdir- assignment in scripts/Makefile since it is redundant
> - Always pass vmlinux ti modpost so we have full updated info
> - Print out number of modules being mod posted to distingush from
>   vmlinux one
> - use vmlinux as target name to enable nicer quiet command print

Your patch seems to work as expected if I add a return 0 at the end of
modpost.c:secref_whitelist(). I like how you printed out the number of
modules being processed. I have one minor comment about your patch:

Modpost seems to get run twice on vmlinux if the kernel is built with
"make all". I think it would be best to run modpost on vmlinux only when
vmlinux is built - never when modules are processed.

Thanks,

/ magnus

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Sam Ravnborg]

On Wed, Aug 09, 2006 at 10:32:36AM +0900, Magnus Damm wrote:
 
> Your patch seems to work as expected if I add a return 0 at the end of
> modpost.c:secref_whitelist(). I like how you printed out the number of
> modules being processed.
Thanks - fixed now. My gcc (3.4.6-r1 from Gentoo did not warn)

>I have one minor comment about your patch:
> 
> Modpost seems to get run twice on vmlinux if the kernel is built with
> "make all". I think it would be best to run modpost on vmlinux only when
> vmlinux is built - never when modules are processed.

Thesecond time modpost runs vmlinux is used to pick up symbol
information to check that all symbols are valid etc.
The alternative was to trust the symbols being read from Module.symvers
and that would be OK in most cases but I could imagine situations where
Module.symvers was deleted but vmlinux kept.

So therefore the more expensive solution to run modpost twice on vmlinux
was chosen.

	Sam

Re: [PATCH] CONFIG_RELOCATABLE modpost fix

[Magnus Damm]

On Wed, 2006-08-09 at 08:29 +0200, Sam Ravnborg wrote:
> On Wed, Aug 09, 2006 at 10:32:36AM +0900, Magnus Damm wrote:
>  
> > Your patch seems to work as expected if I add a return 0 at the end of
> > modpost.c:secref_whitelist(). I like how you printed out the number of
> > modules being processed.
> Thanks - fixed now. My gcc (3.4.6-r1 from Gentoo did not warn)

Interesting. I have the 3.4.6-r1 ebuild installed too, but I happened to
have the 3.3.6 profile selected by default. Which explains why things
still work as expected here.

> >I have one minor comment about your patch:
> > 
> > Modpost seems to get run twice on vmlinux if the kernel is built with
> > "make all". I think it would be best to run modpost on vmlinux only when
> > vmlinux is built - never when modules are processed.
> 
> Thesecond time modpost runs vmlinux is used to pick up symbol
> information to check that all symbols are valid etc.
> The alternative was to trust the symbols being read from Module.symvers
> and that would be OK in most cases but I could imagine situations where
> Module.symvers was deleted but vmlinux kept.
> 
> So therefore the more expensive solution to run modpost twice on vmlinux
> was chosen.

I understand. I'm not that worried about build performance, more the
fact that all the warnings from vmlinux will get spit out twice.

Thanks,

/ magnus