Skip to contentSkip to conversation

framework for building modules externally

PATCH In review Robert Millan 4 participants 22 messages Nov 1, 2008 – Apr 13, 2009 Kbuild

lore
Share & tools
lore

Robert Millan

author
#0 lore raw

PATCH 1/1 framework for building modules externally

Hi,

Attached patch makes it possible to build modules externally, by:

- Installing headers in system include dir.

- Exporting two ABI tags (a build-time macro and a run-time variable) for run-time comparison.

- Exporting a makefile with COMMON_*FLAGS variables.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

2008-11-01 Robert Millan <[email protected]>

* Makefile.in (include_DATA): New variable. (build_flags.mk): New target.
(pkgdata_DATA): Add `build_flags.mk'. (install-local): Install $(include_DATA) files in $(includedir).

* include/grub/misc.h (GRUB_ABI): New macro (initialize as 0). (grub_abi): New variable prototype.
* kern/main.c (grub_abi): New variable (initialize as GRUB_ABI).

Makefile.in +20 -0
Index: Makefile.in
===================================================================
@@ -152,6 +152,13 @@ ifeq (, $(UNIFONT_HEX))
 else
 pkgdata_DATA += unicode.pff ascii.pff
 
+include_DATA = $(shell find $(srcdir)/include -name \*.h | sed -e 's,^$(srcdir)/,,g') \
+			$(srcdir)/include/grub/cpu
+
+pkgdata_DATA += build_flags.mk
+build_flags.mk:
+	echo -en "COMMON_ASFLAGS=$(COMMON_ASFLAGS)\nCOMMON_CFLAGS=$(COMMON_CFLAGS)\nCOMMON_LDFLAGS=$(COMMON_LDFLAGS)\n" > $@
+
 # Arrows and lines are needed to draw the menu, so we always include them
 UNICODE_ARROWS=0x2190-0x2193
 UNICODE_LINES=0x2501-0x251B

@@ -226,6 +233,19 @@ install-local: all
 	  dest="`echo $$file | sed 's,.*/,,'`"; \
 	  $(INSTALL_DATA) $$dir$$file $(DESTDIR)$(libdir)/grub/$$dest; \
 	done
+	$(mkinstalldirs) $(DESTDIR)$(includedir)
+	@list='$(include_DATA)'; \
+	for file in $$list; do \
+	  if test -f "$$file"; then dir=; else dir="$(srcdir)/"; fi; \
+	  dest="`echo $$file | sed 's,include/,,'`"; \
+	  destdir="`echo $$dest | sed 's,\(^\|/\)[^/]*$$,,g'`"; \
+	  $(mkinstalldirs) $(DESTDIR)$(includedir)/$$destdir; \
+	  if test -f "$$dir$$file"; then \
+	    $(INSTALL_DATA) $$dir$$file $(DESTDIR)$(includedir)/$$dest; \
+	  elif test -L "$$dir$$file"; then \
+	    cp -d $$dir$$file $(DESTDIR)$(includedir)/$$dest; \
+	  fi; \
+	done
 
 install-strip:
 	$(MAKE) INSTALL_PROGRAM="$(INSTALL_STRIP_PROGRAM)" install
main.c +2 -0
Index: kern/main.c
===================================================================
@@ -28,6 +28,8 @@
 #include <grub/env.h>
 #include <grub/mm.h>
 
+grub_uint32_t grub_abi = GRUB_ABI;
+
 void
 grub_module_iterate (int (*hook) (struct grub_module_header *header))
 {
grub/misc.h +4 -0
Index: include/grub/misc.h
===================================================================
@@ -25,6 +25,10 @@
 #include <grub/symbol.h>
 #include <grub/err.h>
 
+/* Increase this number every time GRUB ABI is changed.  */
+#define GRUB_ABI	0
+extern grub_uint32_t EXPORT_VAR(grub_abi);
+
 #define ALIGN_UP(addr, align) (((grub_uint64_t)addr + align - 1) & ~(align - 1))
 
 #define grub_dprintf(condition, fmt, args...) grub_real_dprintf(__FILE__, __LINE__, condition, fmt, ## args);
author
#1 lore raw
Quoted 11 lines · Robert Millan

Hi,

Attached patch makes it possible to build modules externally, by:

- Installing headers in system include dir.

- Exporting two ABI tags (a build-time macro and a run-time variable) for run-time comparison.

- Exporting a makefile with COMMON_*FLAGS variables.

Looks like I missed some important details. The headers and some macros are not enough, as we need to reproduce part of the build system to build a module externally. This new patch provides the following:

In pkgdata: genmodsrc.sh genmk.rb
In pkglib: config.h grub_script.tab.h build_env.mk
In includedir: header files

How to use:

cat > Makefile.in << EOF
srcdir = /usr/share/grub
builddir = /usr/lib/grub/i386-pc
include $(builddir)/build_env.mk

pkglib_MODULES += mymod.mod
mymod_mod_SOURCES = mymod.c
mymod_mod_CFLAGS = $(COMMON_CFLAGS) mymod_mod_LDFLAGS = $(COMMON_LDFLAGS)
EOF
ruby /usr/share/grub/genmk.rb < Makefile.in > Makefile make

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

2008-11-01 Robert Millan <[email protected]>

* Makefile.in (PKGLIB): Add $(pkglib_BUILDDIR). (PKGDATA): Add $(pkgdata_SRCDIR).
(pkglib_BUILDDIR): New variable.
(pkgdata_SRCDIR): New variable.
(build_env.mk): New target.
(include_DATA): New variable.
(install-local): Install $(include_DATA) files in $(includedir).

* include/grub/misc.h (GRUB_ABI): New macro (initialize as 0). (grub_abi): New variable prototype.
* kern/main.c (grub_abi): New variable (initialize as GRUB_ABI).

Makefile.in +31 -2
Index: Makefile.in
===================================================================
@@ -105,8 +105,8 @@
 MKFILES = $(patsubst %.rmk,%.mk,$(RMKFILES))
 
 PKGLIB = $(pkglib_IMAGES) $(pkglib_MODULES) $(pkglib_PROGRAMS) \
-	$(pkglib_DATA) $(lib_DATA)
-PKGDATA = $(pkgdata_DATA)
+	$(pkglib_DATA) $(lib_DATA) $(pkglib_BUILDDIR)
+PKGDATA = $(pkgdata_DATA) $(pkgdata_SRCDIR)
 PROGRAMS = $(bin_UTILITIES) $(sbin_UTILITIES)
 SCRIPTS = $(bin_SCRIPTS) $(sbin_SCRIPTS) $(grub-mkconfig_SCRIPTS)
 

@@ -163,6 +163,22 @@
 	ruby $(srcdir)/util/unifont2pff.rb 0x0-0x7f $(UNICODE_ARROWS) $(UNICODE_LINES) $(UNIFONT_HEX) > $@
 endif
 
+# Used for building modules externally
+pkglib_BUILDDIR += build_env.mk
+build_env.mk: Makefile
+	(\
+	echo "TARGET_CC=$(TARGET_CC)" ; \
+	echo "TARGET_CFLAGS=$(TARGET_CFLAGS)" ; \
+	echo "TARGET_CPPFLAGS=$(TARGET_CPPFLAGS) -I$(pkglibdir)" ; \
+	echo "STRIP=$(STRIP)" ; \
+	echo "COMMON_ASFLAGS=$(COMMON_ASFLAGS)" ; \
+	echo "COMMON_CFLAGS=$(COMMON_CFLAGS)" ; \
+	echo "COMMON_LDFLAGS=$(COMMON_LDFLAGS)"\
+	) > $@
+pkglib_BUILDDIR += config.h grub_script.tab.h
+pkgdata_SRCDIR += genmodsrc.sh genmk.rb
+include_DATA += $(shell find include -name \*.h) include/grub/cpu
+
 all-local: $(PROGRAMS) $(PKGLIB) $(PKGDATA) $(SCRIPTS) $(MKFILES)
 
 install: install-local

@@ -175,6 +191,19 @@
 	  dest="`echo $$file | sed 's,.*/,,'`"; \
 	  $(INSTALL_DATA) $$dir$$file $(DESTDIR)$(pkglibdir)/$$dest; \
 	done
+	$(mkinstalldirs) $(DESTDIR)$(includedir)
+	@list='$(include_DATA)'; \
+	for file in $$list; do \
+	  if test -f "$$file"; then dir=; else dir="$(srcdir)/"; fi; \
+	  dest="`echo $$file | sed 's,include/,,'`"; \
+	  destdir="`echo $$dest | sed 's,\(^\|/\)[^/]*$$,,g'`"; \
+	  $(mkinstalldirs) $(DESTDIR)$(includedir)/$$destdir; \
+	  if test -f "$$dir$$file"; then \
+	    $(INSTALL_DATA) $$dir$$file $(DESTDIR)$(includedir)/$$dest; \
+	  elif test -L "$$dir$$file"; then \
+	    cp -d $$dir$$file $(DESTDIR)$(includedir)/$$dest; \
+	  fi; \
+	done
 	$(mkinstalldirs) $(DESTDIR)$(pkgdatadir)
 	@list='$(PKGDATA)'; \
 	for file in $$list; do \
main.c +2 -0
Index: kern/main.c
===================================================================
@@ -28,6 +28,8 @@
 #include <grub/env.h>
 #include <grub/mm.h>
 
+grub_uint32_t grub_abi = GRUB_ABI;
+
 void
 grub_module_iterate (int (*hook) (struct grub_module_header *header))
 {
grub/misc.h +4 -0
Index: include/grub/misc.h
===================================================================
@@ -25,6 +25,10 @@
 #include <grub/symbol.h>
 #include <grub/err.h>
 
+/* Increase this number every time GRUB ABI is changed.  */
+#define GRUB_ABI	0
+extern grub_uint32_t EXPORT_VAR(grub_abi);
+
 #define ALIGN_UP(addr, align) (((grub_uint64_t)addr + align - 1) & ~(align - 1))
 
 #define grub_dprintf(condition, fmt, args...) grub_real_dprintf(__FILE__, __LINE__, condition, fmt, ## args);
#2 lore raw
Quoted 7 lines · Robert Millan
Quoted 6 lines · Robert Millan

Hi,

Attached patch makes it possible to build modules externally, by:

- Installing headers in system include dir.

Quoted 8 lines
Quoted 4 lines

- Exporting two ABI tags (a build-time macro and a run-time variable) for run-time comparison.

- Exporting a makefile with COMMON_*FLAGS variables.

Looks like I missed some important details. The headers and some macros are not enough, as we need to reproduce part of the build system to build a module externally. This new patch provides the following:

Interesting idea. However how do you define GRUB ABI :) ? What is it, kernel, certain set of modules, or how :) ?

Versioning of the modules would be a good idea indeed to make sure mismatched modules are not tried together.

author
#3 lore raw
Quoted 11 lines · Vesa Jääskeläinen
Quoted 10 lines · Robert Millan
Quoted 9 lines · Robert Millan

Hi,

Attached patch makes it possible to build modules externally, by:

- Installing headers in system include dir.

- Exporting two ABI tags (a build-time macro and a run-time variable) for run-time comparison.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

- Exporting a makefile with COMMON_*FLAGS variables.

Looks like I missed some important details. The headers and some macros are not enough, as we need to reproduce part of the build system to build a module externally. This new patch provides the following:

Interesting idea. However how do you define GRUB ABI :) ? What is it, kernel, certain set of modules, or how :) ?

Anything that can possibly break an external module. I.e. any interface exported by either the kernel or any of the modules.

(I expect we'd bump this number a lot)

Quoted 2 lines

Versioning of the modules would be a good idea indeed to make sure mismatched modules are not tried together.

This should never happen if user always runs grub-install (which he should do anyway unless he's looking for trouble). My idea for external modules is that a separate package can put them in $pkglibdir and then grub-install will automaticaly include them the next time it's called.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
#4 lore raw
Quoted 16 lines · Robert Millan
Quoted 15 lines · Vesa Jääskeläinen
Quoted 14 lines · Robert Millan
Quoted 10 lines · Robert Millan

Hi,

Attached patch makes it possible to build modules externally, by:

- Installing headers in system include dir.

- Exporting two ABI tags (a build-time macro and a run-time variable) for run-time comparison.

- Exporting a makefile with COMMON_*FLAGS variables.

Looks like I missed some important details. The headers and some macros are not enough, as we need to reproduce part of the build system to build a module externally. This new patch provides the following:

Quoted 7 lines
Quoted 2 lines

Interesting idea. However how do you define GRUB ABI :) ? What is it, kernel, certain set of modules, or how :) ?

Anything that can possibly break an external module. I.e. any interface exported by either the kernel or any of the modules.

(I expect we'd bump this number a lot)

Did you have some plan how to automate this ABI versioning :) ? If it is not automated I can almost guarantee that we get it wrong at one point of time :)

Quoted 7 lines
Quoted 2 lines

Versioning of the modules would be a good idea indeed to make sure mismatched modules are not tried together.

This should never happen if user always runs grub-install (which he should do anyway unless he's looking for trouble). My idea for external modules is that a separate package can put them in $pkglibdir and then grub-install will automaticaly include them the next time it's called.

How do you treat differences in ABI ?

Dis-allow loading of module with different value of ABI ? Or were you planning that module itself adapts to different versions of GRUB 2 ABI's?

author
#5 lore raw
Quoted 4 lines · Vesa Jääskeläinen

Did you have some plan how to automate this ABI versioning :) ? If it is not automated I can almost guarantee that we get it wrong at one point of time :)

It's not a big problem, at least not for my intended use of this feature. Debian packages can use strong versioned dependencies to prevent breakage. Also, I wouldn't mind reviewing the ChangeLog for "missed" ABI bumps before I upload a package.

And it surely would have been bumped several times between official releases.

Quoted 8 lines
Quoted 7 lines
Quoted 2 lines

Versioning of the modules would be a good idea indeed to make sure mismatched modules are not tried together.

This should never happen if user always runs grub-install (which he should do anyway unless he's looking for trouble). My idea for external modules is that a separate package can put them in $pkglibdir and then grub-install will automaticaly include them the next time it's called.

Quoted 4 lines

How do you treat differences in ABI ?

Dis-allow loading of module with different value of ABI ? Or were you planning that module itself adapts to different versions of GRUB 2 ABI's?

The module itself could:

GRUB_MOD_INIT(foo)
{
if (grub_abi != GRUB_ABI)

    {
      grub_printf ("abi mismatch!\n");
      return;
    }

/* register our commands/terminals/disks/whatever */
}

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
#6 lore raw
Quoted 12 lines · Robert Millan
Quoted 6 lines

...
How do you treat differences in ABI ?

Dis-allow loading of module with different value of ABI ? Or were you planning that module itself adapts to different versions of GRUB 2 ABI's?

The module itself could:

GRUB_MOD_INIT(foo)
{
if (grub_abi != GRUB_ABI)

Quoted 9 lines
    {
      grub_printf ("abi mismatch!\n");
      return;
    }

/* register our commands/terminals/disks/whatever */
}

Alternative: Export a symbol describing the ABI version in kernel ("grub_abi_3_14"). Access this symbol in each module (this can be hidden in GRUB_MOD_INIT).

Then loading a module with wrong ABI version fails due to undefined symbol.

Signature · 1 line
Christian Franke
author
#7 lore raw
Quoted 3 lines · Christian Franke

Alternative: Export a symbol describing the ABI version in kernel ("grub_abi_3_14").

That requires more bytes than a 32-bit integer.

Quoted 2 lines

Access this symbol in each module (this can be hidden in GRUB_MOD_INIT).

And this adds code in every module, but for non-external modules we already have reassurance that their abi is consistent (users should never bypass grub-install; if they do, it's likely going to break for them anyway).

I understand there's a minor benefit for programmers of external modules, but both things are at the expense of extra size to kernel and core.img modules. External modules will only provide non-essential functionality, so it's not a problem they have to check the ABI IMHO.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
#8 lore raw
Quoted 8 lines · Robert Millan
Quoted 2 lines · Christian Franke

Alternative: Export a symbol describing the ABI version in kernel ("grub_abi_3_14").

That requires more bytes than a 32-bit integer.

The symbol value does not matter, the symbol name string provides enough space to describe any version.

GCC apparently uses a similar method ("_gcc/gxx_personality_*") to ensure that the correct libgcc is linked.

Quoted 13 lines
Quoted 2 lines

Access this symbol in each module (this can be hidden in GRUB_MOD_INIT).

And this adds code in every module, but for non-external modules we already have reassurance that their abi is consistent (users should never bypass grub-install; if they do, it's likely going to break for them anyway).

I understand there's a minor benefit for programmers of external modules, but both things are at the expense of extra size to kernel and core.img modules. External modules will only provide non-essential functionality, so it's not a problem they have to check the ABI IMHO.

I agree, such a check is not necessary for the non-external modules.

But, exporting one extra "grub_abi_VERSION" symbol would only require one entry in the kernel symbol table and no other code.

Then, a module author has the option to check the ABI version by importing this symbol (possibly by simply adding '-u grub_abi_VERSION' to ld command)

Signature · 5 lines
Christian Franke


PS: The current use of __FILE__ may also add extra unexpected size: For packaging, configure is often run outside of $srcdir with a absolute path name. This may result in long __FILE__ strings, like
/home/maintainer/packaging/grub/tmp/grub-1.96+20081105-1/src/grub-1.96/kern/disk.c
author
#9 lore raw

__FILE__ (Re: [PATCH] framework for building modules externally)

Quoted 5 lines · Christian Franke

PS: The current use of __FILE__ may also add extra unexpected size: For packaging, configure is often run outside of $srcdir with a absolute path name. This may result in long __FILE__ strings, like /home/maintainer/packaging/grub/tmp/grub-1.96+20081105-1/src/grub-1.96/kern/disk.c

This has annoyed me for a while. Do you know a proper fix?

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
#10 lore raw

Re: __FILE__ (Re: [PATCH] framework for building modules externally)

Quoted 9 lines · Robert Millan
Quoted 3 lines · Christian Franke

PS: The current use of __FILE__ may also add extra unexpected size: For packaging, configure is often run outside of $srcdir with a absolute path name. This may result in long __FILE__ strings, like /home/maintainer/packaging/grub/tmp/grub-1.96+20081105-1/src/grub-1.96/kern/disk.c

This has annoyed me for a while. Do you know a proper fix?

There is apparently no option to modify __FILE__ expansion. Therefore, the relative file name has to be specified in the module itself.

See attached patch for a possible fix: Each module using grub_dprintf (here disk.c) may specify its name in 'this_file'. When all modules are changed, the '#define this_file' and all #undefs can be removed.

Christian

include/grub/misc.h +3 -1
index 15c18f5..36e4bcc 100644
@@ -27,7 +27,9 @@
 
 #define ALIGN_UP(addr, align) (((grub_uint64_t)addr + align - 1) & ~(align - 1))
 
-#define grub_dprintf(condition, fmt, args...) grub_real_dprintf(__FILE__, __LINE__, condition, fmt, ## args);
+#define grub_dprintf(condition, fmt, args...) grub_real_dprintf(this_file, __LINE__, condition, fmt, ## args);
+#define this_file __FILE__
+
 /* XXX: If grub_memmove is too slow, we must implement grub_memcpy.  */
 #define grub_memcpy(d,s,n)	grub_memmove ((d), (s), (n))
kern/disk.c +3 -0
index ed82506..039c011 100644
@@ -25,6 +25,9 @@
 #include <grub/time.h>
 #include <grub/file.h>
 
+#undef this_file
+static const char this_file[] = "kern/disk.c";
+
 #define	GRUB_CACHE_TIMEOUT	2
 
 /* The last time the disk was used.  */
author
#11 lore raw

Re: __FILE__ (Re: [PATCH] framework for building modules externally)

Quoted 4 lines · Christian Franke

See attached patch for a possible fix: Each module using grub_dprintf (here disk.c) may specify its name in 'this_file'. When all modules are changed, the '#define this_file' and all #undefs can be removed.

This adds redundancy which later makes it more work to move code around and rename files, etc. Why not make it part of the build system? I.e. to build foo.c, you need gcc -Dfile=foo.c foo.c ?

But then again, since gcc is free software perhaps it'd make more sense to add new candy there? (e.g. __RELATIVE_FILE__ or so).

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
#12 lore raw

Re: __FILE__ (Re: [PATCH] framework for building modules externally)

Quoted 10 lines · Robert Millan
Quoted 2 lines · Christian Franke

See attached patch for a possible fix: Each module using grub_dprintf (here disk.c) may specify its name in 'this_file'. When all modules are changed, the '#define this_file' and all #undefs can be removed.

This adds redundancy which later makes it more work to move code around and rename files, etc. Why not make it part of the build system? I.e. to build foo.c, you need gcc -Dfile=foo.c foo.c ?

Rarely used, but makes sense.

Even leaving the source as is and using 'gcc -D__FILE__=foo.c ...' would work, but then gcc emits a warning.

Quoted 4 lines

But then again, since gcc is free software perhaps it'd make more sense to add new candy there? (e.g. __RELATIVE_FILE__ or so).

... and/or a new option '-ffile_var_relative_to=PREFIX' which removes PREFIX (e.g. "$srcdir/") from __FILE__ expansions. Then such a fix could easily be added also to other existing projects, without any sourcecode changes.

Christian

author
#13 lore raw
In reply to Christian Franke
Quoted 4 lines · Christian Franke

Then, a module author has the option to check the ABI version by importing this symbol
(possibly by simply adding '-u grub_abi_VERSION' to ld command)

How would the whole picture look like? Sounds like it'd mean less complexity in external modules at the expense of more complexity in the build system.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
#14 lore raw
Quoted 10 lines · Robert Millan
Quoted 3 lines · Christian Franke

Then, a module author has the option to check the ABI version by importing this symbol (possibly by simply adding '-u grub_abi_VERSION' to ld command)

How would the whole picture look like? Sounds like it'd mean less complexity in external modules at the expense of more complexity in the build system.

Here a first proof-of-concept patch, uses hello.c as the external module.

If kernel is compiled with other GRUB_ABI_VER_SYM, insmod fails with:
"error: the symbol 'grub_abi_ver_1_96' not found"

Christian

hello/hello.c +2 -0
index 70cbf60..4eb2483 100644
@@ -25,6 +25,8 @@
 #include <grub/dl.h>
 #include <grub/normal.h>
 
+GRUB_ABI_VER_CHECK;
+
 static grub_err_t
 grub_cmd_hello (struct grub_arg_list *state __attribute__ ((unused)),
 		int argc __attribute__ ((unused)),
include/grub/dl.h +7 -0
index bdde089..3efc05e 100644
@@ -93,4 +93,11 @@ void *EXPORT_FUNC(grub_dl_resolve_symbol) (const char *name);
 grub_err_t grub_arch_dl_check_header (void *ehdr);
 grub_err_t grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr);
 
+#define GRUB_ABI_VER_SYM  grub_abi_ver_1_96
+
+extern char EXPORT_VAR(GRUB_ABI_VER_SYM);
+
+#define GRUB_ABI_VER_CHECK \
+char * grub_abi_ver_check = &GRUB_ABI_VER_SYM
+
 #endif /* ! GRUB_DL_H */
kern/dl.c +2 -0
index b483134..32591f4 100644
@@ -29,6 +29,8 @@
 #include <grub/env.h>
 #include <grub/cache.h>
 
+char GRUB_ABI_VER_SYM;
+
 #if GRUB_CPU_SIZEOF_VOID_P == 4
 
 typedef Elf32_Word Elf_Word;
author
#15 lore raw
Quoted 5 lines · Christian Franke

Here a first proof-of-concept patch, uses hello.c as the external module.

If kernel is compiled with other GRUB_ABI_VER_SYM, insmod fails with:
"error: the symbol 'grub_abi_ver_1_96' not found"

Ah, I thought it'd be more complex. Seems fine to me.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
author
#16 lore raw
In reply to Christian Franke
Quoted 9 lines · Christian Franke
Quoted 4 lines

How would the whole picture look like? Sounds like it'd mean less complexity
in external modules at the expense of more complexity in the build system.

Here a first proof-of-concept patch, uses hello.c as the external module.

If kernel is compiled with other GRUB_ABI_VER_SYM, insmod fails with:
"error: the symbol 'grub_abi_ver_1_96' not found"

Ah, it's much simpler than I thought.

Quoted 1 line

+#define GRUB_ABI_VER_SYM grub_abi_ver_1_96

I assume between releases we'd add a counter after the version number? Like grub_abi_ver_1_96_X or so?

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
#17 lore raw
Quoted 11 lines · Robert Millan
Quoted 8 lines · Christian Franke
Quoted 3 lines

How would the whole picture look like? Sounds like it'd mean less complexity in external modules at the expense of more complexity in the build system.

Here a first proof-of-concept patch, uses hello.c as the external module.

If kernel is compiled with other GRUB_ABI_VER_SYM, insmod fails with:
"error: the symbol 'grub_abi_ver_1_96' not found"

Quoted 10 lines

Ah, it's much simpler than I thought.

Quoted 2 lines

+#define GRUB_ABI_VER_SYM grub_abi_ver_1_96

I assume between releases we'd add a counter after the version number? Like grub_abi_ver_1_96_X or so?

Yes. Symbol should be changed with each incompatible ABI change. Not necessary for simple ABI enhancements like new functions.

Christian

#18 lore raw
In reply to Robert Millan
Quoted 4 lines · Robert Millan
Quoted 1 line · Christian Franke

Alternative: Export a symbol describing the ABI version in kernel ("grub_abi_3_14").

That requires more bytes than a 32-bit integer.

You export grub_abi anyway. _3_14 is 4 bytes instead of 4 but saves. grub_abi_* can be a fictive variable by adding something like

      {"grub_abi_<..>", 0},

to gensymlist.sh.in

Quoted 12 lines
Quoted 2 lines

Access this symbol in each module (this can be hidden in GRUB_MOD_INIT).

And this adds code in every module, but for non-external modules we already have reassurance that their abi is consistent (users should never bypass grub-install; if they do, it's likely going to break for them anyway).

I understand there's a minor benefit for programmers of external modules, but both things are at the expense of extra size to kernel and core.img modules. External modules will only provide non-essential functionality, so it's not a problem they have to check the ABI IMHO.

Signature · 2 lines
Regards
Vladimir 'phcoder' Serbinenko
author
#19 lore raw
Quoted 10 lines · phcoder
Quoted 5 lines · Robert Millan
Quoted 2 lines · Christian Franke

Alternative: Export a symbol describing the ABI version in kernel ("grub_abi_3_14").

That requires more bytes than a 32-bit integer.

You export grub_abi anyway. _3_14 is 4 bytes instead of 4 but saves. grub_abi_* can be a fictive variable by adding something like

      {"grub_abi_<..>", 0},

to gensymlist.sh.in

I've been thinking that this doesn't solve all the problems anyway.

*.lst files are generated in official grub tree, and it's not easy for external packages to override that. Perhaps we should do it at the source level and not bother about ABI anymore... :-/

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
author
#20 lore raw
In reply to Robert Millan

Hi,

Since it looks like the ABI part inspires more need for debate/discussion, I propose merging the rest first. Support for ABI tracking is not essential for building external modules (in Debian, it's barely necessary).

Here's a new patch. If nobody objects, I'll commit it in a few days.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

2008-11-01 Robert Millan <[email protected]>

* Makefile.in (PKGLIB): Add $(pkglib_BUILDDIR). (PKGDATA): Add $(pkgdata_SRCDIR).
(pkglib_BUILDDIR): New variable.
(pkgdata_SRCDIR): New variable.
(build_env.mk): New target.
(include_DATA): New variable.
(install-local): Install $(include_DATA) files in $(includedir).

Makefile.in +31 -2
Index: Makefile.in
===================================================================
@@ -105,8 +105,8 @@
 MKFILES = $(patsubst %.rmk,%.mk,$(RMKFILES))
 
 PKGLIB = $(pkglib_IMAGES) $(pkglib_MODULES) $(pkglib_PROGRAMS) \
-	$(pkglib_DATA) $(lib_DATA)
-PKGDATA = $(pkgdata_DATA)
+	$(pkglib_DATA) $(lib_DATA) $(pkglib_BUILDDIR)
+PKGDATA = $(pkgdata_DATA) $(pkgdata_SRCDIR)
 PROGRAMS = $(bin_UTILITIES) $(sbin_UTILITIES)
 SCRIPTS = $(bin_SCRIPTS) $(sbin_SCRIPTS) $(grub-mkconfig_SCRIPTS)
 

@@ -163,6 +163,22 @@
 	ruby $(srcdir)/util/unifont2pff.rb 0x0-0x7f $(UNICODE_ARROWS) $(UNICODE_LINES) $(UNIFONT_HEX) > $@
 endif
 
+# Used for building modules externally
+pkglib_BUILDDIR += build_env.mk
+build_env.mk: Makefile
+	(\
+	echo "TARGET_CC=$(TARGET_CC)" ; \
+	echo "TARGET_CFLAGS=$(TARGET_CFLAGS)" ; \
+	echo "TARGET_CPPFLAGS=$(TARGET_CPPFLAGS) -I$(pkglibdir)" ; \
+	echo "STRIP=$(STRIP)" ; \
+	echo "COMMON_ASFLAGS=$(COMMON_ASFLAGS)" ; \
+	echo "COMMON_CFLAGS=$(COMMON_CFLAGS)" ; \
+	echo "COMMON_LDFLAGS=$(COMMON_LDFLAGS)"\
+	) > $@
+pkglib_BUILDDIR += config.h grub_script.tab.h
+pkgdata_SRCDIR += genmodsrc.sh genmk.rb
+include_DATA += $(shell find include -name \*.h) include/grub/cpu
+
 all-local: $(PROGRAMS) $(PKGLIB) $(PKGDATA) $(SCRIPTS) $(MKFILES)
 
 install: install-local

@@ -175,6 +191,19 @@
 	  dest="`echo $$file | sed 's,.*/,,'`"; \
 	  $(INSTALL_DATA) $$dir$$file $(DESTDIR)$(pkglibdir)/$$dest; \
 	done
+	$(mkinstalldirs) $(DESTDIR)$(includedir)
+	@list='$(include_DATA)'; \
+	for file in $$list; do \
+	  if test -f "$$file"; then dir=; else dir="$(srcdir)/"; fi; \
+	  dest="`echo $$file | sed 's,include/,,'`"; \
+	  destdir="`echo $$dest | sed 's,\(^\|/\)[^/]*$$,,g'`"; \
+	  $(mkinstalldirs) $(DESTDIR)$(includedir)/$$destdir; \
+	  if test -f "$$dir$$file"; then \
+	    $(INSTALL_DATA) $$dir$$file $(DESTDIR)$(includedir)/$$dest; \
+	  elif test -L "$$dir$$file"; then \
+	    cp -d $$dir$$file $(DESTDIR)$(includedir)/$$dest; \
+	  fi; \
+	done
 	$(mkinstalldirs) $(DESTDIR)$(pkgdatadir)
 	@list='$(PKGDATA)'; \
 	for file in $$list; do \
author
#21 lore raw
Quoted 8 lines · Robert Millan

Hi,

Since it looks like the ABI part inspires more need for debate/discussion, I propose merging the rest first. Support for ABI tracking is not essential for building external modules (in Debian, it's barely necessary).

Here's a new patch. If nobody objects, I'll commit it in a few days.

Committed.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."