mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC PATCH 0/8] Module and ARM Module updates and fixes
@ 2020-06-17  3:43 David Dgien
  2020-06-17  3:43 ` [RFC PATCH 1/8] Makefile: Initialize and export KBUILD variables David Dgien
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:43 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

This series fixes various bugs and bit-rot issues with the module
loading code. It also ports a couple of modules features from the Linux
kernel: arch specific section fixups, and module PLTs for ARM modules,
to contain veneers for 'bl' instructions.

There are two things in this series I'm looking for feedback on:
Linux implements module_frob_arch_sections as a weak symbol for the
default case. I didn't see any other "weak" functions in barebox, so I
wasn't sure if using that was acceptable. Since the Kconfig
HAVE_MOD_ARCH_SPECIFIC already exists as part of the change, I just used
that to define a static inline default implementation, but using a weak
function would make that slightly cleaner.

And in the patch that added the init macros to module.h, I wasn't sure
if it would be okay to pollute init.h with the #ifndef MODULE
directives, so instead I just #undef'd all of the initcalls before
redefining them in module.h.  If it's okay to add the #ifndef MODULE to
init.h, that would be significantly cleaner than the current
implementation.

David Dgien (8):
  Makefile: Initialize and export KBUILD variables
  module: Add init macros to module.h
  module: Fix adding module to list after layout
  module: Fix module command registration
  module: Implement HAVE_MOD_ARCH_SPECIFIC
  arm: makefile: Fix compiler flag variable
  arm: elf: Add THM relocation types
  arm: module: Allow modules outside of bl range

 Makefile                      |  12 +-
 arch/arm/Kconfig              |  15 +++
 arch/arm/Makefile             |   6 +-
 arch/arm/cpu/Kconfig          |   1 +
 arch/arm/include/asm/elf.h    |   3 +
 arch/arm/include/asm/module.h |  44 ++++++-
 arch/arm/lib32/Makefile       |   1 +
 arch/arm/lib32/module-plts.c  | 229 ++++++++++++++++++++++++++++++++++
 arch/arm/lib32/module.c       |  14 +++
 arch/arm/lib32/module.lds     |   4 +
 common/Kconfig                |   8 ++
 common/module.c               |  60 +++++----
 include/asm-generic/module.h  |  49 ++++++++
 include/module.h              | 109 ++++++++++++++++
 14 files changed, 523 insertions(+), 32 deletions(-)
 create mode 100644 arch/arm/lib32/module-plts.c
 create mode 100644 arch/arm/lib32/module.lds
 create mode 100644 include/asm-generic/module.h

-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 1/8] Makefile: Initialize and export KBUILD variables
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
@ 2020-06-17  3:43 ` David Dgien
  2020-06-17  3:43 ` [RFC PATCH 2/8] module: Add init macros to module.h David Dgien
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:43 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

Initialize and export KBUILD KERNEL and MODULE variables. Otherwise,
appending to them elsewhere in the srctree will not be seen by the build
commands in Makefile.lib

Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 Makefile | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index c4a2519a1..f9457323e 100644
--- a/Makefile
+++ b/Makefile
@@ -395,6 +395,8 @@ BASH		= bash
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise $(CF)
 CFLAGS_KERNEL	=
 AFLAGS_KERNEL	=
+CFLAGS_MODULE	=
+AFLAGS_MODULE	=
 
 LDFLAGS_MODULE  = -T common/module.lds
 
@@ -418,6 +420,10 @@ KBUILD_CFLAGS   := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
                    -Werror=implicit-function-declaration -Werror=implicit-int \
                    -Os -pipe -Wmissing-prototypes -std=gnu89
 KBUILD_AFLAGS          := -D__ASSEMBLY__
+KBUILD_AFLAGS_KERNEL :=
+KBUILD_CFLAGS_KERNEL :=
+KBUILD_AFLAGS_MODULE := -DMODULE
+KBUILD_CFLAGS_MODULE := -DMODULE
 
 LDFLAGS_barebox	:= -Map barebox.map
 
@@ -432,8 +438,10 @@ export HOSTCXX CHECK CHECKFLAGS
 export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
 
 export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS
-export KBUILD_CFLAGS CFLAGS_KERNEL
-export KBUILD_AFLAGS AFLAGS_KERNEL
+export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
+export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE
+export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE
+export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
 export LDFLAGS_barebox
 export LDFLAGS_pbl
 
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/8] module: Add init macros to module.h
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
  2020-06-17  3:43 ` [RFC PATCH 1/8] Makefile: Initialize and export KBUILD variables David Dgien
@ 2020-06-17  3:43 ` David Dgien
  2020-06-17  3:43 ` [RFC PATCH 3/8] module: Fix adding module to list after layout David Dgien
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:43 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

Port module initialization macros and initcall redefinition macros from
Linux

Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 include/module.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/include/module.h b/include/module.h
index cea8c2e18..37c48f065 100644
--- a/include/module.h
+++ b/include/module.h
@@ -2,6 +2,7 @@
 #ifndef __MODULE_H
 #define __MODULE_H
 
+#include <init.h>
 #include <elf.h>
 #include <linux/compiler.h>
 #include <linux/export.h>
@@ -13,6 +14,96 @@
 
 #define MODULE_NAME_LEN (64 - sizeof(unsigned long))
 
+/* These are either module local, or the kernel's dummy ones. */
+extern int init_module(void);
+extern void cleanup_module(void);
+
+#ifndef MODULE
+/**
+ * module_init() - driver initialization entry point
+ * @x: function to be run at kernel boot time or module insertion
+ *
+ * module_init() will either be called during do_initcalls() (if
+ * builtin) or at module insertion time (if a module).  There can only
+ * be one per module.
+ */
+#define module_init(x)	device_initcall(x);
+
+/**
+ * module_exit() - driver exit entry point
+ * @x: function to be run when driver is removed
+ *
+ * module_exit() will wrap the driver clean-up code
+ * with cleanup_module() when used with rmmod when
+ * the driver is a module.  If the driver is statically
+ * compiled into the kernel, module_exit() has no effect.
+ * There can only be one per module.
+ */
+#define module_exit(x)	devshutdown_exitcall(x);
+
+#else /* MODULE */
+
+/*
+ * In most cases loadable modules do not need custom
+ * initcall levels. There are still some valid cases where
+ * a driver may be needed early if built in, and does not
+ * matter when built as a loadable module. Like bus
+ * snooping debug drivers.
+ */
+#undef core_initcall
+#undef postcore_initcall
+#undef console_initcall
+#undef postconsole_initcall
+#undef mem_initcall
+#undef mmu_initcall
+#undef postmmu_initcall
+#undef coredevice_initcall
+#undef fs_initcall
+#undef device_initcall
+#undef late_initcall
+
+#define core_initcall(fn)		module_init(fn)
+#define postcore_initcall(fn)		module_init(fn)
+#define console_initcall(fn)		module_init(fn)
+#define postconsole_initcall(fn)	module_init(fn)
+#define mem_initcall(fn)		module_init(fn)
+#define mmu_initcall(fn)		module_init(fn)
+#define postmmu_initcall(fn)		module_init(fn)
+#define coredevice_initcall(fn)		module_init(fn)
+#define fs_initcall(fn)			module_init(fn)
+#define device_initcall(fn)		module_init(fn)
+#define late_initcall(fn)		module_init(fn)
+
+#undef early_exitcall
+#undef predevshutdown_exitcall
+#undef devshutdown_exitcall
+#undef postdevshutdown_exitcall
+#undef prearchshutdown_exitcall
+#undef archshutdown_exitcall
+#undef postarchshutdown_exitcall
+
+#define early_exitcall(fn)		module_exit(fn)
+#define predevshutdown_exitcall(fn)	module_exit(fn)
+#define devshutdown_exitcall(fn)	module_exit(fn)
+#define postdevshutdown_exitcall(fn)	module_exit(fn)
+#define prearchshutdown_exitcall(fn)	module_exit(fn)
+#define archshutdown_exitcall(fn)	module_exit(fn)
+#define postarchshutdown_exitcall(fn)	module_exit(fn)
+
+/* Each module must use one module_init(). */
+#define module_init(initfn)					\
+	static inline initcall_t __maybe_unused __inittest(void)		\
+	{ return initfn; }					\
+	int init_module(void) __attribute__((alias(#initfn)));
+
+/* This is only required if you want to be unloadable. */
+#define module_exit(exitfn)					\
+	static inline exitcall_t __maybe_unused __exittest(void)		\
+	{ return exitfn; }					\
+	void cleanup_module(void) __attribute__((alias(#exitfn)));
+
+#endif
+
 #ifdef CONFIG_MODULES
 #include <asm/module.h>
 
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/8] module: Fix adding module to list after layout
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
  2020-06-17  3:43 ` [RFC PATCH 1/8] Makefile: Initialize and export KBUILD variables David Dgien
  2020-06-17  3:43 ` [RFC PATCH 2/8] module: Add init macros to module.h David Dgien
@ 2020-06-17  3:43 ` David Dgien
  2020-06-17  3:44 ` [RFC PATCH 4/8] module: Fix module command registration David Dgien
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:43 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

During load_module(), the 'this_module' section is relocated, but the
pointer to the module struct isn't updated to account account for the
move. Do so before adding the module to the module_list.

As a side effect of properly pointing to the relocated module struct, we
no longer need to manually search for and fixup the init_module symbol,
so remove that code.

Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 common/module.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/common/module.c b/common/module.c
index 829c12000..5ace544e0 100644
--- a/common/module.c
+++ b/common/module.c
@@ -297,13 +297,8 @@ struct module * load_module(void *mod_image, unsigned long len)
 		}
 	}
 
-	for (i = 0; i < numsyms; i++) {
-		if (!strcmp(strtab + sym[i].st_name, MODULE_SYMBOL_PREFIX "init_module")) {
-			printf("found init_module() at 0x%08x\n", sym[i].st_value);
-			module->init = (void *)sym[i].st_value;
-		}
-	}
-
+	/* Module has been moved */
+	module = (void *)sechdrs[modindex].sh_addr;
 	list_add_tail(&module->list, &module_list);
 
 	return module;
@@ -311,8 +306,6 @@ struct module * load_module(void *mod_image, unsigned long len)
 cleanup:
 	if (ptr)
 		free(ptr);
-	if (module)
-		free(module);
 
 	return NULL;
 }
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 4/8] module: Fix module command registration
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
                   ` (2 preceding siblings ...)
  2020-06-17  3:43 ` [RFC PATCH 3/8] module: Fix adding module to list after layout David Dgien
@ 2020-06-17  3:44 ` David Dgien
  2020-06-17  3:44 ` [RFC PATCH 5/8] module: Implement HAVE_MOD_ARCH_SPECIFIC David Dgien
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:44 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

Commit 1e4a948673d7 ("command: Use array of pointers to commands")
updated the command linker sections to contain an array of pointers to
the command structs, rather than the command structs themselves.
However, the load_module function was not updated to account for the
change to the linker section, so fix that here.

Additionally, commit 22bdecc9c591 ("lds: Move start/end address
variables into defines") moved the _start and _end symbols out of the
linker script and into the section macro defines, causing them to show
up in the module symbol table as well, so we take can advantage of those
to find the array of pointers.

Fixes: 1e4a948673d7 ("command: Use array of pointers to commands")
Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 common/module.c | 43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/common/module.c b/common/module.c
index 5ace544e0..b08df1199 100644
--- a/common/module.c
+++ b/common/module.c
@@ -176,6 +176,34 @@ static void layout_sections( struct module *mod,
 	debug("core_size: %ld\n", mod->core_size);
 }
 
+static void register_module_cmds(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex)
+{
+	Elf32_Sym *sym;
+	unsigned int numsyms;
+	unsigned int i;
+	struct command * const *cmd_start = NULL;
+	struct command * const *cmd_end = NULL;
+	struct command * const *cmd;
+
+	numsyms = sechdrs[symindex].sh_size / sizeof(Elf32_Sym);
+	sym = (void *)sechdrs[symindex].sh_addr;
+
+	for (i = 0; i < numsyms; i++) {
+		if (strcmp(strtab + sym[i].st_name, MODULE_SYMBOL_PREFIX "__barebox_cmd_start") == 0)
+			cmd_start = (struct command * const *)sym[i].st_value;
+
+		if (strcmp(strtab + sym[i].st_name, MODULE_SYMBOL_PREFIX "__barebox_cmd_end") == 0)
+			cmd_end = (struct command * const *)sym[i].st_value;
+	}
+
+	if (cmd_start && cmd_end) {
+		debug("found __barebox_cmd_start at 0x%08x\n", (uint32_t)cmd_start);
+		for (cmd = cmd_start; cmd != cmd_end; cmd++) {
+			register_command(*cmd);
+		}
+	}
+}
+
 LIST_HEAD(module_list);
 
 struct module * load_module(void *mod_image, unsigned long len)
@@ -183,8 +211,6 @@ struct module * load_module(void *mod_image, unsigned long len)
 	struct module *module = NULL;
 	Elf32_Ehdr *ehdr;		/* Elf header structure pointer     */
 	Elf32_Shdr *sechdrs;		/* Section header structure pointer */
-	Elf32_Sym *sym;
-	unsigned int numsyms;
 	char *strtab = 0;		/* String table pointer             */
 	int i;				/* Loop counter                     */
 	unsigned int strindex = 0;
@@ -193,7 +219,6 @@ struct module * load_module(void *mod_image, unsigned long len)
 	char *secstrings;
 	void *ptr = NULL;
 	int err;
-	int cmdindex;
 
 	if (len < sizeof(*ehdr))
 		return NULL;
@@ -285,17 +310,7 @@ struct module * load_module(void *mod_image, unsigned long len)
 			apply_relocate_add(sechdrs, strtab, symindex, i, module);
 	}
 
-	numsyms = sechdrs[symindex].sh_size / sizeof(Elf32_Sym);
-	sym = (void *)sechdrs[symindex].sh_addr;
-
-	cmdindex = find_sec(ehdr, sechdrs, secstrings, ".barebox_cmd");
-	if (cmdindex) {
-		struct command *cmd =(struct command *)sechdrs[cmdindex].sh_addr;
-		for (i = 0; i < sechdrs[cmdindex].sh_size / sizeof(struct command); i++) {
-			register_command(cmd);
-			cmd++;
-		}
-	}
+	register_module_cmds(sechdrs, strtab, symindex);
 
 	/* Module has been moved */
 	module = (void *)sechdrs[modindex].sh_addr;
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 5/8] module: Implement HAVE_MOD_ARCH_SPECIFIC
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
                   ` (3 preceding siblings ...)
  2020-06-17  3:44 ` [RFC PATCH 4/8] module: Fix module command registration David Dgien
@ 2020-06-17  3:44 ` David Dgien
  2020-06-17  3:44 ` [RFC PATCH 6/8] arm: makefile: Fix compiler flag variable David Dgien
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:44 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

Implement HAVE_MOD_ARCH_SPECIFIC Kconfig and module_frob_arch_sections()
function prototype from Linux module subsystem.
module_frob_arch_sections() should be implemented by any architecture
that selects HAVE_MOD_ARCH_SPECIFIC, and is called from load_module()

Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 common/Kconfig               |  8 ++++++
 common/module.c              |  6 +++++
 include/asm-generic/module.h | 49 ++++++++++++++++++++++++++++++++++++
 include/module.h             | 18 +++++++++++++
 4 files changed, 81 insertions(+)
 create mode 100644 include/asm-generic/module.h

diff --git a/common/Kconfig b/common/Kconfig
index ac282d895..9bade375e 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -326,6 +326,14 @@ config MODULES
 	  way to compile modules and the list of exported symbols to actually
 	  make use of modules is short to nonexistent
 
+config HAVE_MOD_ARCH_SPECIFIC
+	depends on MODULES
+	bool
+	help
+	  The arch uses struct mod_arch_specific to store data.  Many arches
+	  just need a simple module loader without arch specific data - those
+	  should not enable this.
+
 config KALLSYMS
 	depends on HAS_KALLSYMS
 	bool "kallsyms"
diff --git a/common/module.c b/common/module.c
index b08df1199..964369755 100644
--- a/common/module.c
+++ b/common/module.c
@@ -271,6 +271,12 @@ struct module * load_module(void *mod_image, unsigned long len)
 		goto cleanup;
 	}
 
+	/* Allow arches to frob section contents and sizes.  */
+	err = module_frob_arch_sections(ehdr, sechdrs,
+					secstrings, module);
+	if (err < 0)
+		goto cleanup;
+
 	/* Determine total sizes, and put offsets in sh_entsize.  For now
 	   this is done generically; there doesn't appear to be any
 	   special cases for the architectures. */
diff --git a/include/asm-generic/module.h b/include/asm-generic/module.h
new file mode 100644
index 000000000..98e1541b7
--- /dev/null
+++ b/include/asm-generic/module.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_GENERIC_MODULE_H
+#define __ASM_GENERIC_MODULE_H
+
+/*
+ * Many architectures just need a simple module
+ * loader without arch specific data.
+ */
+#ifndef CONFIG_HAVE_MOD_ARCH_SPECIFIC
+struct mod_arch_specific
+{
+};
+#endif
+
+#ifdef CONFIG_64BIT
+#define Elf_Shdr	Elf64_Shdr
+#define Elf_Phdr	Elf64_Phdr
+#define Elf_Sym		Elf64_Sym
+#define Elf_Dyn		Elf64_Dyn
+#define Elf_Ehdr	Elf64_Ehdr
+#define Elf_Addr	Elf64_Addr
+#ifdef CONFIG_MODULES_USE_ELF_REL
+#define Elf_Rel		Elf64_Rel
+#endif
+#ifdef CONFIG_MODULES_USE_ELF_RELA
+#define Elf_Rela	Elf64_Rela
+#endif
+#define ELF_R_TYPE(X)	ELF64_R_TYPE(X)
+#define ELF_R_SYM(X)	ELF64_R_SYM(X)
+
+#else /* CONFIG_64BIT */
+
+#define Elf_Shdr	Elf32_Shdr
+#define Elf_Phdr	Elf32_Phdr
+#define Elf_Sym		Elf32_Sym
+#define Elf_Dyn		Elf32_Dyn
+#define Elf_Ehdr	Elf32_Ehdr
+#define Elf_Addr	Elf32_Addr
+#ifdef CONFIG_MODULES_USE_ELF_REL
+#define Elf_Rel		Elf32_Rel
+#endif
+#ifdef CONFIG_MODULES_USE_ELF_RELA
+#define Elf_Rela	Elf32_Rela
+#endif
+#define ELF_R_TYPE(X)	ELF32_R_TYPE(X)
+#define ELF_R_SYM(X)	ELF32_R_SYM(X)
+#endif
+
+#endif /* __ASM_GENERIC_MODULE_H */
diff --git a/include/module.h b/include/module.h
index 37c48f065..2751c86cd 100644
--- a/include/module.h
+++ b/include/module.h
@@ -142,6 +142,24 @@ int apply_relocate_add(Elf_Shdr *sechdrs,
 		       unsigned int symindex,
 		       unsigned int relsec,
 		       struct module *mod);
+
+#ifdef CONFIG_HAVE_MOD_ARCH_SPECIFIC
+/* Adjust arch-specific sections.  Return 0 on success.  */
+int module_frob_arch_sections(Elf_Ehdr *hdr,
+			      Elf_Shdr *sechdrs,
+			      char *secstrings,
+			      struct module *mod);
+#else
+static inline
+int module_frob_arch_sections(Elf_Ehdr *hdr,
+			      Elf_Shdr *sechdrs,
+			      char *secstrings,
+			      struct module *mod)
+{
+	return 0;
+}
+#endif
+
 #endif /* CONFIG_MODULES */
 
 extern struct list_head module_list;
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 6/8] arm: makefile: Fix compiler flag variable
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
                   ` (4 preceding siblings ...)
  2020-06-17  3:44 ` [RFC PATCH 5/8] module: Implement HAVE_MOD_ARCH_SPECIFIC David Dgien
@ 2020-06-17  3:44 ` David Dgien
  2020-06-17  3:44 ` [RFC PATCH 7/8] arm: elf: Add THM relocation types David Dgien
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:44 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

In order for ARM modules to compile with the correct relocation types,
they must be built without -fPIE. Move -fPIE from KBUILD_CPPFLAGS to
KBUILD_CFLAGS_KERNEL so that the flag is only included when compiling
files being built into the barebox image.

Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 arch/arm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 6c7373c20..c18a1d802 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -139,7 +139,7 @@ LDFLAGS_barebox += --gc-sections
 LDFLAGS_pbl += --gc-sections
 
 # early code often runs at addresses we are not linked at
-KBUILD_CPPFLAGS += -fPIE
+KBUILD_CFLAGS_KERNEL += -fPIE
 
 ifdef CONFIG_RELOCATABLE
 LDFLAGS_barebox += -pie
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 7/8] arm: elf: Add THM relocation types
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
                   ` (5 preceding siblings ...)
  2020-06-17  3:44 ` [RFC PATCH 6/8] arm: makefile: Fix compiler flag variable David Dgien
@ 2020-06-17  3:44 ` David Dgien
  2020-06-17  3:44 ` [RFC PATCH 8/8] arm: module: Allow modules outside of bl range David Dgien
  2020-06-17 13:45 ` [RFC PATCH 0/8] Module and ARM Module updates and fixes Sascha Hauer
  8 siblings, 0 replies; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:44 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 arch/arm/include/asm/elf.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
index b98b3e52a..3def56769 100644
--- a/arch/arm/include/asm/elf.h
+++ b/arch/arm/include/asm/elf.h
@@ -30,6 +30,9 @@ typedef struct user_fp elf_fpregset_t;
 #define R_ARM_CALL	28
 #define R_ARM_JUMP24	29
 
+#define R_ARM_THM_CALL		10
+#define R_ARM_THM_JUMP24	30
+
 /*
  * These are used to set parameters in the core dumps.
  */
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 8/8] arm: module: Allow modules outside of bl range
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
                   ` (6 preceding siblings ...)
  2020-06-17  3:44 ` [RFC PATCH 7/8] arm: elf: Add THM relocation types David Dgien
@ 2020-06-17  3:44 ` David Dgien
  2020-06-17 13:52   ` Sascha Hauer
  2020-06-17 13:45 ` [RFC PATCH 0/8] Module and ARM Module updates and fixes Sascha Hauer
  8 siblings, 1 reply; 14+ messages in thread
From: David Dgien @ 2020-06-17  3:44 UTC (permalink / raw)
  To: barebox; +Cc: David Dgien

Unlike the Linux kernel, barebox does not have a dedicated heap for
storing modules. Therefore, if the system memory configuration places
the general heap further away than can be reached by a 'bl' instruction
(24 bits of address, or 16 MiB), then the module relocations will fail
due to being out of range.

Allocate PLTs when loading modules so that jumps and calls whose
targets are too far away for their relative offsets to be encoded
in the instructions themselves can be bounced via veneers in the
module's PLT. The modules will use slightly more memory, but after
rounding up to page size, the actual memory footprint is usually
the same.

Adoption of Linux commits:

66e94ba3c8ea ARM: kernel: avoid brute force search on PLT generation
1031a7e674d1 ARM: kernel: sort relocation sections before allocating PLTs
05123fef0982 ARM: kernel: allocate PLT entries only for external symbols
35fa91eed817 ARM: kernel: merge core and init PLTs
7d485f647c1f ARM: 8220/1: allow modules outside of bl range

Signed-off-by: David Dgien <dgienda125@gmail.com>
---
 arch/arm/Kconfig              |  15 +++
 arch/arm/Makefile             |   4 +
 arch/arm/cpu/Kconfig          |   1 +
 arch/arm/include/asm/module.h |  44 ++++++-
 arch/arm/lib32/Makefile       |   1 +
 arch/arm/lib32/module-plts.c  | 229 ++++++++++++++++++++++++++++++++++
 arch/arm/lib32/module.c       |  14 +++
 arch/arm/lib32/module.lds     |   4 +
 8 files changed, 306 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/lib32/module-plts.c
 create mode 100644 arch/arm/lib32/module.lds

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index dfb18777b..95fd8ecfe 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -477,4 +477,19 @@ config ARM_PSCI_DEBUG
 	  putc function.
 	  Only use for debugging.
 
+config ARM_MODULE_PLTS
+	bool "Use PLTs to allow loading modules placed far from barebox image"
+	depends on MODULES
+	select QSORT
+	help
+	  Allocate PLTs when loading modules so that jumps and calls whose
+	  targets are too far away for their relative offsets to be encoded
+	  in the instructions themselves can be bounced via veneers in the
+	  module's PLT. The modules will use slightly more memory, but after
+	  rounding up to page size, the actual memory footprint is usually
+	  the same.
+
+	  Say y if your memory configuration puts the heap to far away from the
+	  barebox image, causing relocation out of range errors
+
 endmenu
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index c18a1d802..6ba0a6261 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -18,6 +18,10 @@ AS		+= -EL
 LD		+= -EL
 endif
 
+ifeq ($(CONFIG_ARM_MODULE_PLTS),y)
+LDFLAGS_MODULE   += -T $(srctree)/arch/arm/lib32/module.lds
+endif
+
 # Unaligned access is not supported when MMU is disabled, so given how
 # at least some of the code would be executed with MMU off, lets be
 # conservative and instruct the compiler not to generate any unaligned
diff --git a/arch/arm/cpu/Kconfig b/arch/arm/cpu/Kconfig
index 6b4fed526..f9f52a625 100644
--- a/arch/arm/cpu/Kconfig
+++ b/arch/arm/cpu/Kconfig
@@ -6,6 +6,7 @@ config PHYS_ADDR_T_64BIT
 config CPU_32
 	bool
 	select HAS_MODULES
+	select HAVE_MOD_ARCH_SPECIFIC
 	select HAS_DMA
 	select HAVE_PBL_IMAGE
 
diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
index 5b4d1a3f3..4d7039a7a 100644
--- a/arch/arm/include/asm/module.h
+++ b/arch/arm/include/asm/module.h
@@ -1,13 +1,45 @@
 #ifndef _ASM_ARM_MODULE_H
 #define _ASM_ARM_MODULE_H
 
-struct mod_arch_specific
-{
-	int foo;
+#include <asm-generic/module.h>
+
+struct unwind_table;
+
+#ifdef CONFIG_ARM_UNWIND
+enum {
+	ARM_SEC_INIT,
+	ARM_SEC_DEVINIT,
+	ARM_SEC_CORE,
+	ARM_SEC_EXIT,
+	ARM_SEC_DEVEXIT,
+	ARM_SEC_HOT,
+	ARM_SEC_UNLIKELY,
+	ARM_SEC_MAX,
 };
+#endif
 
-#define Elf_Shdr	Elf32_Shdr
-#define Elf_Sym		Elf32_Sym
-#define Elf_Ehdr	Elf32_Ehdr
+struct mod_arch_specific {
+#ifdef CONFIG_ARM_UNWIND
+	struct unwind_table *unwind[ARM_SEC_MAX];
+#endif
+#ifdef CONFIG_ARM_MODULE_PLTS
+	struct elf32_shdr	*plt;
+	int			plt_count;
+#endif
+};
+
+struct module;
+u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val);
+
+#ifndef CONFIG_ARM_MODULE_PLTS
+static inline
+int module_frob_arch_sections(Elf_Ehdr *hdr,
+			      Elf_Shdr *sechdrs,
+			      char *secstrings,
+			      struct module *mod)
+{
+	return 0;
+}
+#endif
 
 #endif /* _ASM_ARM_MODULE_H */
diff --git a/arch/arm/lib32/Makefile b/arch/arm/lib32/Makefile
index 597bc0790..ec6a3aea6 100644
--- a/arch/arm/lib32/Makefile
+++ b/arch/arm/lib32/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_ARM_OPTIMZED_STRING_FUNCTIONS)	+= memset.o
 obj-$(CONFIG_ARM_UNWIND) += unwind.o
 obj-$(CONFIG_ARM_SEMIHOSTING) += semihosting-trap.o semihosting.o
 obj-$(CONFIG_MODULES) += module.o
+obj-$(CONFIG_ARM_MODULE_PLTS) += module-plts.o
 extra-y += barebox.lds
 
 pbl-y	+= lib1funcs.o
diff --git a/arch/arm/lib32/module-plts.c b/arch/arm/lib32/module-plts.c
new file mode 100644
index 000000000..53cf6b11c
--- /dev/null
+++ b/arch/arm/lib32/module-plts.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2014-2017 Linaro Ltd. <ard.biesheuvel@linaro.org>
+ */
+
+#include <common.h>
+#include <elf.h>
+#include <module.h>
+#include <qsort.h>
+
+#include <asm/opcodes.h>
+
+#define PLT_ENT_STRIDE		32
+#define PLT_ENT_COUNT		(PLT_ENT_STRIDE / sizeof(u32))
+#define PLT_ENT_SIZE		(sizeof(struct plt_entries) / PLT_ENT_COUNT)
+
+#ifdef CONFIG_THUMB2_BAREBOX
+#define PLT_ENT_LDR		__opcode_to_mem_thumb32(0xf8dff000 | \
+							(PLT_ENT_STRIDE - 4))
+#else
+#define PLT_ENT_LDR		__opcode_to_mem_arm(0xe59ff000 | \
+						    (PLT_ENT_STRIDE - 8))
+#endif
+
+struct plt_entries {
+	u32	ldr[PLT_ENT_COUNT];
+	u32	lit[PLT_ENT_COUNT];
+};
+
+u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val)
+{
+	struct plt_entries *plt = (struct plt_entries *)mod->arch.plt->sh_addr;
+	int idx = 0;
+
+	/*
+	 * Look for an existing entry pointing to 'val'. Given that the
+	 * relocations are sorted, this will be the last entry we allocated.
+	 * (if one exists).
+	 */
+	if (mod->arch.plt_count > 0) {
+		plt += (mod->arch.plt_count - 1) / PLT_ENT_COUNT;
+		idx = (mod->arch.plt_count - 1) % PLT_ENT_COUNT;
+
+		if (plt->lit[idx] == val)
+			return (u32)&plt->ldr[idx];
+
+		idx = (idx + 1) % PLT_ENT_COUNT;
+		if (!idx)
+			plt++;
+	}
+
+	mod->arch.plt_count++;
+	BUG_ON(mod->arch.plt_count * PLT_ENT_SIZE > mod->arch.plt->sh_size);
+
+	if (!idx)
+		/* Populate a new set of entries */
+		*plt = (struct plt_entries){
+			{ [0 ... PLT_ENT_COUNT - 1] = PLT_ENT_LDR, },
+			{ val, }
+		};
+	else
+		plt->lit[idx] = val;
+
+	return (u32)&plt->ldr[idx];
+}
+
+#define cmp_3way(a,b)	((a) < (b) ? -1 : (a) > (b))
+
+static int cmp_rel(const void *a, const void *b)
+{
+	const Elf32_Rel *x = a, *y = b;
+	int i;
+
+	/* sort by type and symbol index */
+	i = cmp_3way(ELF32_R_TYPE(x->r_info), ELF32_R_TYPE(y->r_info));
+	if (i == 0)
+		i = cmp_3way(ELF32_R_SYM(x->r_info), ELF32_R_SYM(y->r_info));
+	return i;
+}
+
+static bool is_zero_addend_relocation(Elf32_Addr base, const Elf32_Rel *rel)
+{
+	u32 *tval = (u32 *)(base + rel->r_offset);
+
+	/*
+	 * Do a bitwise compare on the raw addend rather than fully decoding
+	 * the offset and doing an arithmetic comparison.
+	 * Note that a zero-addend jump/call relocation is encoded taking the
+	 * PC bias into account, i.e., -8 for ARM and -4 for Thumb2.
+	 */
+	switch (ELF32_R_TYPE(rel->r_info)) {
+		u16 upper, lower;
+
+	case R_ARM_THM_CALL:
+	case R_ARM_THM_JUMP24:
+		upper = __mem_to_opcode_thumb16(((u16 *)tval)[0]);
+		lower = __mem_to_opcode_thumb16(((u16 *)tval)[1]);
+
+		return (upper & 0x7ff) == 0x7ff && (lower & 0x2fff) == 0x2ffe;
+
+	case R_ARM_CALL:
+	case R_ARM_PC24:
+	case R_ARM_JUMP24:
+		return (__mem_to_opcode_arm(*tval) & 0xffffff) == 0xfffffe;
+	}
+	BUG();
+}
+
+static bool duplicate_rel(Elf32_Addr base, const Elf32_Rel *rel, int num)
+{
+	const Elf32_Rel *prev;
+
+	/*
+	 * Entries are sorted by type and symbol index. That means that,
+	 * if a duplicate entry exists, it must be in the preceding
+	 * slot.
+	 */
+	if (!num)
+		return false;
+
+	prev = rel + num - 1;
+	return cmp_rel(rel + num, prev) == 0 &&
+	       is_zero_addend_relocation(base, prev);
+}
+
+/* Count how many PLT entries we may need */
+static unsigned int count_plts(const Elf32_Sym *syms, Elf32_Addr base,
+			       const Elf32_Rel *rel, int num, Elf32_Word dstidx)
+{
+	unsigned int ret = 0;
+	const Elf32_Sym *s;
+	int i;
+
+	for (i = 0; i < num; i++) {
+		switch (ELF32_R_TYPE(rel[i].r_info)) {
+		case R_ARM_CALL:
+		case R_ARM_PC24:
+		case R_ARM_JUMP24:
+		case R_ARM_THM_CALL:
+		case R_ARM_THM_JUMP24:
+			/*
+			 * We only have to consider branch targets that resolve
+			 * to symbols that are defined in a different section.
+			 * This is not simply a heuristic, it is a fundamental
+			 * limitation, since there is no guaranteed way to emit
+			 * PLT entries sufficiently close to the branch if the
+			 * section size exceeds the range of a branch
+			 * instruction. So ignore relocations against defined
+			 * symbols if they live in the same section as the
+			 * relocation target.
+			 */
+			s = syms + ELF32_R_SYM(rel[i].r_info);
+			if (s->st_shndx == dstidx)
+				break;
+
+			/*
+			 * Jump relocations with non-zero addends against
+			 * undefined symbols are supported by the ELF spec, but
+			 * do not occur in practice (e.g., 'jump n bytes past
+			 * the entry point of undefined function symbol f').
+			 * So we need to support them, but there is no need to
+			 * take them into consideration when trying to optimize
+			 * this code. So let's only check for duplicates when
+			 * the addend is zero.
+			 */
+			if (!is_zero_addend_relocation(base, rel + i) ||
+			    !duplicate_rel(base, rel, i))
+				ret++;
+		}
+	}
+	return ret;
+}
+
+int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
+			      char *secstrings, struct module *mod)
+{
+	unsigned long plts = 0;
+	Elf32_Shdr *s, *sechdrs_end = sechdrs + ehdr->e_shnum;
+	Elf32_Sym *syms = NULL;
+
+	/*
+	 * To store the PLTs, we expand the .text section for core module code
+	 * and for initialization code.
+	 */
+	for (s = sechdrs; s < sechdrs_end; ++s) {
+		if (strcmp(".plt", secstrings + s->sh_name) == 0)
+			mod->arch.plt = s;
+		else if (s->sh_type == SHT_SYMTAB)
+			syms = (Elf32_Sym *)s->sh_addr;
+	}
+
+	if (!mod->arch.plt) {
+		pr_err("%s: module PLT section missing\n", mod->name);
+		return -ENOEXEC;
+	}
+	if (!syms) {
+		pr_err("%s: module symtab section missing\n", mod->name);
+		return -ENOEXEC;
+	}
+
+	for (s = sechdrs + 1; s < sechdrs_end; ++s) {
+		Elf32_Rel *rels = (void *)ehdr + s->sh_offset;
+		int numrels = s->sh_size / sizeof(Elf32_Rel);
+		Elf32_Shdr *dstsec = sechdrs + s->sh_info;
+
+		if (s->sh_type != SHT_REL)
+			continue;
+
+		/* ignore relocations that operate on non-exec sections */
+		if (!(dstsec->sh_flags & SHF_EXECINSTR))
+			continue;
+
+		/* sort by type and symbol index */
+		/* n.b. Barebox qsort instead of Linux sort */
+		qsort(rels, numrels, sizeof(Elf32_Rel), cmp_rel);
+
+		plts += count_plts(syms, dstsec->sh_addr, rels,	numrels, s->sh_info);
+	}
+
+	mod->arch.plt->sh_type = SHT_NOBITS;
+	mod->arch.plt->sh_flags = SHF_EXECINSTR | SHF_ALLOC;
+	mod->arch.plt->sh_addralign = PLT_ENT_STRIDE;
+	mod->arch.plt->sh_size = round_up(plts * PLT_ENT_SIZE,
+					  sizeof(struct plt_entries));
+	mod->arch.plt_count = 0;
+
+	pr_debug("%s: plt=%x\n", __func__, mod->arch.plt->sh_size);
+	return 0;
+}
diff --git a/arch/arm/lib32/module.c b/arch/arm/lib32/module.c
index be7965d59..3ded9896b 100644
--- a/arch/arm/lib32/module.c
+++ b/arch/arm/lib32/module.c
@@ -64,6 +64,20 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
 				offset -= 0x04000000;
 
 			offset += sym->st_value - loc;
+
+			/*
+			 * Route through a PLT entry if 'offset' exceeds the
+			 * supported range. Note that 'offset + loc + 8'
+			 * contains the absolute jump target, i.e.,
+			 * @sym + addend, corrected for the +8 PC bias.
+			 */
+			if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS) &&
+			    (offset <= (s32)0xfe000000 ||
+			     offset >= (s32)0x02000000))
+				offset = get_module_plt(module, loc,
+							offset + loc + 8)
+					 - loc - 8;
+
 			if (offset & 3 ||
 			    offset <= (s32)0xfe000000 ||
 			    offset >= (s32)0x02000000) {
diff --git a/arch/arm/lib32/module.lds b/arch/arm/lib32/module.lds
new file mode 100644
index 000000000..0dd204608
--- /dev/null
+++ b/arch/arm/lib32/module.lds
@@ -0,0 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+SECTIONS {
+	.plt : { BYTE(0) }
+}
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/8] Module and ARM Module updates and fixes
  2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
                   ` (7 preceding siblings ...)
  2020-06-17  3:44 ` [RFC PATCH 8/8] arm: module: Allow modules outside of bl range David Dgien
@ 2020-06-17 13:45 ` Sascha Hauer
  2020-06-18  1:54   ` David Dgien
  8 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2020-06-17 13:45 UTC (permalink / raw)
  To: David Dgien; +Cc: barebox

Hi David,

On Tue, Jun 16, 2020 at 11:43:56PM -0400, David Dgien wrote:
> This series fixes various bugs and bit-rot issues with the module
> loading code. It also ports a couple of modules features from the Linux
> kernel: arch specific section fixups, and module PLTs for ARM modules,
> to contain veneers for 'bl' instructions.
> 
> There are two things in this series I'm looking for feedback on:
> Linux implements module_frob_arch_sections as a weak symbol for the
> default case. I didn't see any other "weak" functions in barebox, so I
> wasn't sure if using that was acceptable.

For things that are really mutually exclusive like different
implementations on different architectures I think weak functions are
ok. They are not ok as a quick hack for hooking something into something
though.

> Since the Kconfig
> HAVE_MOD_ARCH_SPECIFIC already exists as part of the change, I just used
> that to define a static inline default implementation, but using a weak
> function would make that slightly cleaner.
> 
> And in the patch that added the init macros to module.h, I wasn't sure
> if it would be okay to pollute init.h with the #ifndef MODULE
> directives, so instead I just #undef'd all of the initcalls before
> redefining them in module.h.  If it's okay to add the #ifndef MODULE to
> init.h, that would be significantly cleaner than the current
> implementation.

I think it's ok to add #ifndef MODULE to init.h

Anyway, what do you need modules for? Do you have a good reason or is it
just for the fun of it?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 8/8] arm: module: Allow modules outside of bl range
  2020-06-17  3:44 ` [RFC PATCH 8/8] arm: module: Allow modules outside of bl range David Dgien
@ 2020-06-17 13:52   ` Sascha Hauer
  0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-06-17 13:52 UTC (permalink / raw)
  To: David Dgien; +Cc: barebox

On Tue, Jun 16, 2020 at 11:44:04PM -0400, David Dgien wrote:
> Unlike the Linux kernel, barebox does not have a dedicated heap for
> storing modules. Therefore, if the system memory configuration places
> the general heap further away than can be reached by a 'bl' instruction
> (24 bits of address, or 16 MiB), then the module relocations will fail
> due to being out of range.

At least the tlsf malloc implementation allows for multiple heaps. It
might be easier to just add another heap just below the barebox image.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/8] Module and ARM Module updates and fixes
  2020-06-17 13:45 ` [RFC PATCH 0/8] Module and ARM Module updates and fixes Sascha Hauer
@ 2020-06-18  1:54   ` David Dgien
  2020-06-18 13:10     ` Sascha Hauer
  0 siblings, 1 reply; 14+ messages in thread
From: David Dgien @ 2020-06-18  1:54 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

On Wed, Jun 17, 2020 at 03:45:38PM +0200, Sascha Hauer wrote:
> Hi David,
> 
> On Tue, Jun 16, 2020 at 11:43:56PM -0400, David Dgien wrote:
> > This series fixes various bugs and bit-rot issues with the module
> > loading code. It also ports a couple of modules features from the Linux
> > kernel: arch specific section fixups, and module PLTs for ARM modules,
> > to contain veneers for 'bl' instructions.
> > 
> > There are two things in this series I'm looking for feedback on:
> > Linux implements module_frob_arch_sections as a weak symbol for the
> > default case. I didn't see any other "weak" functions in barebox, so I
> > wasn't sure if using that was acceptable.
> 
> For things that are really mutually exclusive like different
> implementations on different architectures I think weak functions are
> ok. They are not ok as a quick hack for hooking something into something
> though.
> 

I'll make the change to a weak function here in a v2, since it will be a
bit cleaner.

> > Since the Kconfig
> > HAVE_MOD_ARCH_SPECIFIC already exists as part of the change, I just used
> > that to define a static inline default implementation, but using a weak
> > function would make that slightly cleaner.
> > 
> > And in the patch that added the init macros to module.h, I wasn't sure
> > if it would be okay to pollute init.h with the #ifndef MODULE
> > directives, so instead I just #undef'd all of the initcalls before
> > redefining them in module.h.  If it's okay to add the #ifndef MODULE to
> > init.h, that would be significantly cleaner than the current
> > implementation.
> 
> I think it's ok to add #ifndef MODULE to init.h

Same as above.

> 
> Anyway, what do you need modules for? Do you have a good reason or is it
> just for the fun of it?

I'm working on a project that wants to use barebox as a very lightweight
OS replacement. We're using modules to allow loading user code with
controlled access to hw interfaces via exported driver symbols.

-- David Dgien

> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/8] Module and ARM Module updates and fixes
  2020-06-18  1:54   ` David Dgien
@ 2020-06-18 13:10     ` Sascha Hauer
  2020-06-22 17:52       ` Masahiro Yamada
  0 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2020-06-18 13:10 UTC (permalink / raw)
  To: David Dgien; +Cc: barebox

On Wed, Jun 17, 2020 at 09:54:55PM -0400, David Dgien wrote:
> Hi Sascha,
> 
> On Wed, Jun 17, 2020 at 03:45:38PM +0200, Sascha Hauer wrote:
> > Hi David,
> > 
> > On Tue, Jun 16, 2020 at 11:43:56PM -0400, David Dgien wrote:
> > > This series fixes various bugs and bit-rot issues with the module
> > > loading code. It also ports a couple of modules features from the Linux
> > > kernel: arch specific section fixups, and module PLTs for ARM modules,
> > > to contain veneers for 'bl' instructions.
> > > 
> > > There are two things in this series I'm looking for feedback on:
> > > Linux implements module_frob_arch_sections as a weak symbol for the
> > > default case. I didn't see any other "weak" functions in barebox, so I
> > > wasn't sure if using that was acceptable.
> > 
> > For things that are really mutually exclusive like different
> > implementations on different architectures I think weak functions are
> > ok. They are not ok as a quick hack for hooking something into something
> > though.
> > 
> 
> I'll make the change to a weak function here in a v2, since it will be a
> bit cleaner.
> 
> > > Since the Kconfig
> > > HAVE_MOD_ARCH_SPECIFIC already exists as part of the change, I just used
> > > that to define a static inline default implementation, but using a weak
> > > function would make that slightly cleaner.
> > > 
> > > And in the patch that added the init macros to module.h, I wasn't sure
> > > if it would be okay to pollute init.h with the #ifndef MODULE
> > > directives, so instead I just #undef'd all of the initcalls before
> > > redefining them in module.h.  If it's okay to add the #ifndef MODULE to
> > > init.h, that would be significantly cleaner than the current
> > > implementation.
> > 
> > I think it's ok to add #ifndef MODULE to init.h
> 
> Same as above.
> 
> > 
> > Anyway, what do you need modules for? Do you have a good reason or is it
> > just for the fun of it?
> 
> I'm working on a project that wants to use barebox as a very lightweight
> OS replacement. 
> We're using modules to allow loading user code with
> controlled access to hw interfaces via exported driver symbols.

So barebOS again, we had that as an April fools joke once :)

You'll probably miss things like interrupts, paging and multitasking
very soon.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/8] Module and ARM Module updates and fixes
  2020-06-18 13:10     ` Sascha Hauer
@ 2020-06-22 17:52       ` Masahiro Yamada
  0 siblings, 0 replies; 14+ messages in thread
From: Masahiro Yamada @ 2020-06-22 17:52 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List, David Dgien

On Thu, Jun 18, 2020 at 10:10 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Wed, Jun 17, 2020 at 09:54:55PM -0400, David Dgien wrote:
> > Hi Sascha,
> >
> > On Wed, Jun 17, 2020 at 03:45:38PM +0200, Sascha Hauer wrote:
> > > Hi David,
> > >
> > > On Tue, Jun 16, 2020 at 11:43:56PM -0400, David Dgien wrote:
> > > > This series fixes various bugs and bit-rot issues with the module
> > > > loading code. It also ports a couple of modules features from the Linux
> > > > kernel: arch specific section fixups, and module PLTs for ARM modules,
> > > > to contain veneers for 'bl' instructions.
> > > >
> > > > There are two things in this series I'm looking for feedback on:
> > > > Linux implements module_frob_arch_sections as a weak symbol for the
> > > > default case. I didn't see any other "weak" functions in barebox, so I
> > > > wasn't sure if using that was acceptable.
> > >
> > > For things that are really mutually exclusive like different
> > > implementations on different architectures I think weak functions are
> > > ok. They are not ok as a quick hack for hooking something into something
> > > though.
> > >
> >
> > I'll make the change to a weak function here in a v2, since it will be a
> > bit cleaner.
> >
> > > > Since the Kconfig
> > > > HAVE_MOD_ARCH_SPECIFIC already exists as part of the change, I just used
> > > > that to define a static inline default implementation, but using a weak
> > > > function would make that slightly cleaner.
> > > >
> > > > And in the patch that added the init macros to module.h, I wasn't sure
> > > > if it would be okay to pollute init.h with the #ifndef MODULE
> > > > directives, so instead I just #undef'd all of the initcalls before
> > > > redefining them in module.h.  If it's okay to add the #ifndef MODULE to
> > > > init.h, that would be significantly cleaner than the current
> > > > implementation.
> > >
> > > I think it's ok to add #ifndef MODULE to init.h
> >
> > Same as above.
> >
> > >
> > > Anyway, what do you need modules for? Do you have a good reason or is it
> > > just for the fun of it?
> >
> > I'm working on a project that wants to use barebox as a very lightweight
> > OS replacement.
> > We're using modules to allow loading user code with
> > controlled access to hw interfaces via exported driver symbols.
>
> So barebOS again, we had that as an April fools joke once :)
>
> You'll probably miss things like interrupts, paging and multitasking
> very soon.
>
> Sascha


Does it make sense to choose y/m
depending on how often they are used?

- commands and drivers that are used quite often   ->  built-in
- commands and drivers that are sometimes used     ->  modules



-- 
Best Regards
Masahiro Yamada

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-06-22 17:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-17  3:43 [RFC PATCH 0/8] Module and ARM Module updates and fixes David Dgien
2020-06-17  3:43 ` [RFC PATCH 1/8] Makefile: Initialize and export KBUILD variables David Dgien
2020-06-17  3:43 ` [RFC PATCH 2/8] module: Add init macros to module.h David Dgien
2020-06-17  3:43 ` [RFC PATCH 3/8] module: Fix adding module to list after layout David Dgien
2020-06-17  3:44 ` [RFC PATCH 4/8] module: Fix module command registration David Dgien
2020-06-17  3:44 ` [RFC PATCH 5/8] module: Implement HAVE_MOD_ARCH_SPECIFIC David Dgien
2020-06-17  3:44 ` [RFC PATCH 6/8] arm: makefile: Fix compiler flag variable David Dgien
2020-06-17  3:44 ` [RFC PATCH 7/8] arm: elf: Add THM relocation types David Dgien
2020-06-17  3:44 ` [RFC PATCH 8/8] arm: module: Allow modules outside of bl range David Dgien
2020-06-17 13:52   ` Sascha Hauer
2020-06-17 13:45 ` [RFC PATCH 0/8] Module and ARM Module updates and fixes Sascha Hauer
2020-06-18  1:54   ` David Dgien
2020-06-18 13:10     ` Sascha Hauer
2020-06-22 17:52       ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox