2007-05-28 19:19:28

by Geert Uytterhoeven

[permalink] [raw]
Subject: [patch 1/2] m68k: runtime patching infrastructure

From: Roman Zippel <[email protected]>

Add the basic infrastructure to allow runtime patching of kernel and
modules to optimize a few functions with parameters, which are only
calculated once during bootup and are otherwise constant.
Use this for the conversion between virtual and physical addresses.

Signed-off-by: Roman Zippel <[email protected]>
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
arch/m68k/Makefile | 1 +
arch/m68k/kernel/Makefile | 3 +--
arch/m68k/kernel/module.c | 28 +++++++++++++++++++++++++++-
arch/m68k/kernel/module.lds | 7 +++++++
arch/m68k/kernel/vmlinux-std.lds | 5 +++++
arch/m68k/kernel/vmlinux-sun3.lds | 5 +++++
arch/m68k/mm/motorola.c | 3 +++
include/asm-m68k/module.h | 33 ++++++++++++++++++++++++++++++++-
include/asm-m68k/page.h | 29 ++++++++++++++++++++++++++---
9 files changed, 107 insertions(+), 7 deletions(-)

--- a/arch/m68k/Makefile
+++ b/arch/m68k/Makefile
@@ -19,6 +19,7 @@ COMPILE_ARCH = $(shell uname -m)
# override top level makefile
AS += -m68020
LDFLAGS := -m m68kelf
+LDFLAGS_MODULE += -T $(srctree)/arch/m68k/kernel/module.lds
ifneq ($(COMPILE_ARCH),$(ARCH))
# prefix for cross-compiling binaries
CROSS_COMPILE = m68k-linux-gnu-
--- a/arch/m68k/kernel/Makefile
+++ b/arch/m68k/kernel/Makefile
@@ -9,13 +9,12 @@ else
endif
extra-y += vmlinux.lds

-obj-y := entry.o process.o traps.o ints.o signal.o ptrace.o \
+obj-y := entry.o process.o traps.o ints.o signal.o ptrace.o module.o \
sys_m68k.o time.o semaphore.o setup.o m68k_ksyms.o devres.o

devres-y = ../../../kernel/irq/devres.o

obj-$(CONFIG_PCI) += bios32.o
-obj-$(CONFIG_MODULES) += module.o
obj-y$(CONFIG_MMU_SUN3) += dma.o # no, it's not a typo

EXTRA_AFLAGS := -traditional
--- a/arch/m68k/kernel/module.c
+++ b/arch/m68k/kernel/module.c
@@ -1,3 +1,9 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ */
+
#include <linux/moduleloader.h>
#include <linux/elf.h>
#include <linux/vmalloc.h>
@@ -11,6 +17,8 @@
#define DEBUGP(fmt...)
#endif

+#ifdef CONFIG_MODULES
+
void *module_alloc(unsigned long size)
{
if (size == 0)
@@ -118,11 +126,29 @@ int apply_relocate_add(Elf32_Shdr *sechd

int module_finalize(const Elf_Ehdr *hdr,
const Elf_Shdr *sechdrs,
- struct module *me)
+ struct module *mod)
{
+ module_fixup(mod, mod->arch.fixup_start, mod->arch.fixup_end);
+
return 0;
}

void module_arch_cleanup(struct module *mod)
{
}
+
+#endif /* CONFIG_MODULES */
+
+void module_fixup(struct module *mod, struct m68k_fixup_info *start,
+ struct m68k_fixup_info *end)
+{
+ struct m68k_fixup_info *fixup;
+
+ for (fixup = start; fixup < end; fixup++) {
+ switch (fixup->type) {
+ case m68k_fixup_memoffset:
+ *(u32 *)fixup->addr = m68k_memoffset;
+ break;
+ }
+ }
+}
--- /dev/null
+++ b/arch/m68k/kernel/module.lds
@@ -0,0 +1,7 @@
+SECTIONS {
+ .m68k_fixup : {
+ __start_fixup = .;
+ *(.m68k_fixup)
+ __stop_fixup = .;
+ }
+}
--- a/arch/m68k/kernel/vmlinux-std.lds
+++ b/arch/m68k/kernel/vmlinux-std.lds
@@ -60,6 +60,11 @@ SECTIONS
__con_initcall_start = .;
.con_initcall.init : { *(.con_initcall.init) }
__con_initcall_end = .;
+ .m68k_fixup : {
+ __start_fixup = .;
+ *(.m68k_fixup)
+ __stop_fixup = .;
+ }
SECURITY_INIT
#ifdef CONFIG_BLK_DEV_INITRD
. = ALIGN(8192);
--- a/arch/m68k/kernel/vmlinux-sun3.lds
+++ b/arch/m68k/kernel/vmlinux-sun3.lds
@@ -54,6 +54,11 @@ __init_begin = .;
__con_initcall_start = .;
.con_initcall.init : { *(.con_initcall.init) }
__con_initcall_end = .;
+ .m68k_fixup : {
+ __start_fixup = .;
+ *(.m68k_fixup)
+ __stop_fixup = .;
+ }
SECURITY_INIT
#ifdef CONFIG_BLK_DEV_INITRD
. = ALIGN(8192);
--- a/arch/m68k/mm/motorola.c
+++ b/arch/m68k/mm/motorola.c
@@ -222,6 +222,9 @@ void __init paging_init(void)
pgprot_val(protection_map[i]) |= _PAGE_CACHE040;
}

+ module_fixup(NULL, __start_fixup, __stop_fixup);
+ flush_icache();
+
/*
* Map the physical memory available into the kernel virtual
* address space. It may allocate some memory for page
--- a/include/asm-m68k/module.h
+++ b/include/asm-m68k/module.h
@@ -1,7 +1,38 @@
#ifndef _ASM_M68K_MODULE_H
#define _ASM_M68K_MODULE_H
-struct mod_arch_specific { };
+
+struct mod_arch_specific {
+ struct m68k_fixup_info *fixup_start, *fixup_end;
+};
+
+#define MODULE_ARCH_INIT { \
+ .fixup_start = __start_fixup, \
+ .fixup_end = __stop_fixup, \
+}
+
#define Elf_Shdr Elf32_Shdr
#define Elf_Sym Elf32_Sym
#define Elf_Ehdr Elf32_Ehdr
+
+
+enum m68k_fixup_type {
+ m68k_fixup_memoffset,
+};
+
+struct m68k_fixup_info {
+ enum m68k_fixup_type type;
+ void *addr;
+};
+
+#define m68k_fixup(type, addr) \
+ " .section \".m68k_fixup\",\"aw\"\n" \
+ " .long " #type "," #addr "\n" \
+ " .previous\n"
+
+extern struct m68k_fixup_info __start_fixup[], __stop_fixup[];
+
+struct module;
+extern void module_fixup(struct module *mod, struct m68k_fixup_info *start,
+ struct m68k_fixup_info *end);
+
#endif /* _ASM_M68K_MODULE_H */
--- a/include/asm-m68k/page.h
+++ b/include/asm-m68k/page.h
@@ -27,6 +27,8 @@

#ifndef __ASSEMBLY__

+#include <asm/module.h>
+
#define get_user_page(vaddr) __get_free_page(GFP_KERNEL)
#define free_user_page(page, addr) free_page(addr)

@@ -114,14 +116,35 @@ typedef struct { unsigned long pgprot; }

#ifndef __ASSEMBLY__

+extern unsigned long m68k_memoffset;
+
#ifndef CONFIG_SUN3

#define WANT_PAGE_VIRTUAL
#ifdef CONFIG_SINGLE_MEMORY_CHUNK
-extern unsigned long m68k_memoffset;

-#define __pa(vaddr) ((unsigned long)(vaddr)+m68k_memoffset)
-#define __va(paddr) ((void *)((unsigned long)(paddr)-m68k_memoffset))
+static inline unsigned long ___pa(void *vaddr)
+{
+ unsigned long paddr;
+ asm (
+ "1: addl #0,%0\n"
+ m68k_fixup(%c2, 1b+2)
+ : "=r" (paddr)
+ : "0" (vaddr), "i" (m68k_fixup_memoffset));
+ return paddr;
+}
+#define __pa(vaddr) ___pa((void *)(vaddr))
+static inline void *__va(unsigned long paddr)
+{
+ void *vaddr;
+ asm (
+ "1: subl #0,%0\n"
+ m68k_fixup(%c2, 1b+2)
+ : "=r" (vaddr)
+ : "0" (paddr), "i" (m68k_fixup_memoffset));
+ return vaddr;
+}
+
#else
#define __pa(vaddr) virt_to_phys((void *)(vaddr))
#define __va(paddr) phys_to_virt((unsigned long)(paddr))

--
Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2007-05-30 00:38:36

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

On Mon, 28 May 2007 21:16:31 +0200
Geert Uytterhoeven <[email protected]> wrote:

> --- a/include/asm-m68k/module.h
> +++ b/include/asm-m68k/module.h
> @@ -1,7 +1,38 @@
> #ifndef _ASM_M68K_MODULE_H
> #define _ASM_M68K_MODULE_H
> -struct mod_arch_specific { };
> +
> +struct mod_arch_specific {
> + struct m68k_fixup_info *fixup_start, *fixup_end;
> +};

Here we use struct m68k_fixup_info.

> +#define MODULE_ARCH_INIT { \
> + .fixup_start = __start_fixup, \
> + .fixup_end = __stop_fixup, \
> +}
> +
> #define Elf_Shdr Elf32_Shdr
> #define Elf_Sym Elf32_Sym
> #define Elf_Ehdr Elf32_Ehdr
> +
> +
> +enum m68k_fixup_type {
> + m68k_fixup_memoffset,
> +};
> +
> +struct m68k_fixup_info {
> + enum m68k_fixup_type type;
> + void *addr;
> +};

and later we define it.

How come it doesn't spit warnings?

I think it could be tightened up even if it happens not to warn?

> +#define m68k_fixup(type, addr) \
> + " .section \".m68k_fixup\",\"aw\"\n" \
> + " .long " #type "," #addr "\n" \
> + " .previous\n"
> +
> +extern struct m68k_fixup_info __start_fixup[], __stop_fixup[];
> +
> +struct module;
> +extern void module_fixup(struct module *mod, struct m68k_fixup_info *start,
> + struct m68k_fixup_info *end);

2007-05-30 05:48:37

by Eric Dumazet

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

Andrew Morton a ?crit :
> On Mon, 28 May 2007 21:16:31 +0200
> Geert Uytterhoeven <[email protected]> wrote:
>
>> --- a/include/asm-m68k/module.h
>> +++ b/include/asm-m68k/module.h
>> @@ -1,7 +1,38 @@
>> #ifndef _ASM_M68K_MODULE_H
>> #define _ASM_M68K_MODULE_H
>> -struct mod_arch_specific { };
>> +
>> +struct mod_arch_specific {
>> + struct m68k_fixup_info *fixup_start, *fixup_end;
>> +};
>
> Here we use struct m68k_fixup_info.
>
>> +#define MODULE_ARCH_INIT { \
>> + .fixup_start = __start_fixup, \
>> + .fixup_end = __stop_fixup, \
>> +}
>> +
>> #define Elf_Shdr Elf32_Shdr
>> #define Elf_Sym Elf32_Sym
>> #define Elf_Ehdr Elf32_Ehdr
>> +
>> +
>> +enum m68k_fixup_type {
>> + m68k_fixup_memoffset,
>> +};
>> +
>> +struct m68k_fixup_info {
>> + enum m68k_fixup_type type;
>> + void *addr;
>> +};
>
> and later we define it.
>
> How come it doesn't spit warnings?
>
> I think it could be tightened up even if it happens not to warn?


struct a {
struct not_yet_defined *start, *end;
};

struct not_yet_defined {
void *foo;
};

Is a valid and gives no warnings.

Still I didnt tried to compile a m68k kernel, so I guess I shouldnt speak here :)

2007-05-30 07:06:21

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

On Wed, 30 May 2007, Eric Dumazet wrote:
> Andrew Morton a ?crit :
> > On Mon, 28 May 2007 21:16:31 +0200
> > Geert Uytterhoeven <[email protected]> wrote:
> >
> > > --- a/include/asm-m68k/module.h
> > > +++ b/include/asm-m68k/module.h
> > > @@ -1,7 +1,38 @@
> > > #ifndef _ASM_M68K_MODULE_H
> > > #define _ASM_M68K_MODULE_H
> > > -struct mod_arch_specific { };
> > > +
> > > +struct mod_arch_specific {
> > > + struct m68k_fixup_info *fixup_start, *fixup_end;
> > > +};
> >
> > Here we use struct m68k_fixup_info.
> >
> > > +#define MODULE_ARCH_INIT { \
> > > + .fixup_start = __start_fixup, \
> > > + .fixup_end = __stop_fixup, \
> > > +}
> > > +
> > > #define Elf_Shdr Elf32_Shdr
> > > #define Elf_Sym Elf32_Sym
> > > #define Elf_Ehdr Elf32_Ehdr
> > > +
> > > +
> > > +enum m68k_fixup_type {
> > > + m68k_fixup_memoffset,
> > > +};
> > > +
> > > +struct m68k_fixup_info {
> > > + enum m68k_fixup_type type;
> > > + void *addr;
> > > +};
> >
> > and later we define it.
> >
> > How come it doesn't spit warnings?
> >
> > I think it could be tightened up even if it happens not to warn?
>
>
> struct a {
> struct not_yet_defined *start, *end;
> };
>
> struct not_yet_defined {
> void *foo;
> };
>
> Is a valid and gives no warnings.

I was puzzled by this as well, as there were no compiler warnings...

Apparently you get a warning only if the _first_ occurrence of a struct
is declared inside a parameter list of a function.

So even

struct a {
struct not_yet_defined *start, *end;
};

extern void f(struct not_yet_defined *start);

struct not_yet_defined {
void *foo;
};

doesn't give a warning, because the first occurence is inside the
definition of struct a.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2007-05-30 08:23:55

by Andreas Schwab

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

Geert Uytterhoeven <[email protected]> writes:

> I was puzzled by this as well, as there were no compiler warnings...
>
> Apparently you get a warning only if the _first_ occurrence of a struct
> is declared inside a parameter list of a function.

The parameter list has its own scope.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2007-05-30 08:40:36

by Joerg Dorchain

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

On Wed, May 30, 2007 at 09:06:08AM +0200, Geert Uytterhoeven wrote:
[...]
> > >
> > > I think it could be tightened up even if it happens not to warn?
> >
> >
> > struct a {
> > struct not_yet_defined *start, *end;
> > };
> >
> > struct not_yet_defined {
> > void *foo;
> > };
> >
> > Is a valid and gives no warnings.
>
> I was puzzled by this as well, as there were no compiler warnings...

Pointers are (at least on m68k) of known size, so the compiler knows how
much space the struct occupies.

Type checking is by definition futile with void * pointer, but for all
other cases the compiler has all types and sizes it needs at this point.

The actual dereferencing of the symbol table is done by the linker,
which also knows all locations and sizes it needs.

Actually, this is the only way to define circular referencing
structures.

A one-pass-compiler-linker would run into problems.

Joerg, trying to recall compiler construction lessons


Attachments:
(No filename) (954.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2007-05-30 11:19:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

On Wed, 30 May 2007, Joerg Dorchain wrote:
> On Wed, May 30, 2007 at 09:06:08AM +0200, Geert Uytterhoeven wrote:
> [...]
> > > >
> > > > I think it could be tightened up even if it happens not to warn?
> > >
> > >
> > > struct a {
> > > struct not_yet_defined *start, *end;
> > > };
> > >
> > > struct not_yet_defined {
> > > void *foo;
> > > };
> > >
> > > Is a valid and gives no warnings.
> >
> > I was puzzled by this as well, as there were no compiler warnings...
>
> Pointers are (at least on m68k) of known size, so the compiler knows how
> much space the struct occupies.
>
> Type checking is by definition futile with void * pointer, but for all
> other cases the compiler has all types and sizes it needs at this point.
>
> The actual dereferencing of the symbol table is done by the linker,
> which also knows all locations and sizes it needs.

True.

> Actually, this is the only way to define circular referencing
> structures.

No, you have forward declarations for that. These are missing here.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2007-05-30 11:41:10

by Joerg Dorchain

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

On Wed, May 30, 2007 at 01:19:45PM +0200, Geert Uytterhoeven wrote:
>
> > Actually, this is the only way to define circular referencing
> > structures.
>
> No, you have forward declarations for that. These are missing here.

I am no gcc expert, but might this be considred an implicit declaration?
IIRC the compiler warns if later the type does noch match. Does it keep
quiet when it matches?

Bye,

Joerg


Attachments:
(No filename) (409.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2007-05-30 18:05:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure



On Wed, 30 May 2007, Geert Uytterhoeven wrote:
>
> Apparently you get a warning only if the _first_ occurrence of a struct
> is declared inside a parameter list of a function.

This is normal C behaviour (and afaik, the "inside a parameter list" part
is actually just _modern_ C, not traditional C. I _think_ traditional C
had a single global namespace for struct names, and no scoping at all).

You can use a _pointer_ to a structure without declaring the structure
itself, and this is totally standard C behaviour. The very use of the
pointer will tell the C compiler that such a structure exists, and since
you only use the pointer, the compiler doesn't care what the struct
_looks_ like. It just adds it to its list of known structures.

In fact, you can often use such a pointer without _ever_ declaring the
structure at all. The structure may not even _exist_. It may be just a way
to get type safety, and every time you want to actually use the pointer,
you have to explicitly cast it to something else.

(Example: in <stdio.h> you can do

typedef struct dummy_made_up_struct FILE;

and make sure that everybody ever uses a "FILE *f", and trying to create a
"FILE f" would be a compile-time error, because that "struct
dummy_made_up_struct" simply doesn't even exist, and all the real users
will cast it to some internal thing)

The reason the "inside a parameter list" is special is that modern C (I
think through some obscure C++ reason, but I'm not sure) considers the
function parameter list to be inside function scope, and that includes the
types, not just the parameter names.

So when you do

extern function(struct hello *);

without (forward-)declaring "struch hello" earlier, then the compiler will
still create a "struct hello" internally, but since the scope of that
declaration is purely just that function prototype itself, it will be
forgotten immediately afterwards, and a subsequent use of "struct hello"
will be considered a _different_ "struct hello", since it's in different
scope.

So you generally never need to forward-declare structures, unless you only
then use them inside function prototypes. In which case you would just do

struct hello;

to tell the compiler that you have a "struct hello" that you haven't
actually declared yet, and now it's in scope _outside_ the function
prototype, and everybody is happy and agrees that they are using the same
"struct hello".

Linus

2007-06-03 15:57:22

by Wouter Verhelst

[permalink] [raw]
Subject: Re: [patch 1/2] m68k: runtime patching infrastructure

On Tue, May 29, 2007 at 05:38:18PM -0700, Andrew Morton wrote:
[...]
> > +struct mod_arch_specific {
> > + struct m68k_fixup_info *fixup_start, *fixup_end;
> > +};
>
> Here we use struct m68k_fixup_info.
[...]
> > +struct m68k_fixup_info {
> > + enum m68k_fixup_type type;
> > + void *addr;
> > +};
>
> and later we define it.
>
> How come it doesn't spit warnings?

Because otherwise you couldn't create linked lists:

struct foo {
void* data;
struct foo* next;
};

At that point it hasn't been defined yet but it is being used. This is
legal, so the compiler can't create a warning for that.

Which is not to say that it's a nice coding style, but that's a
different matter.

--
Shaw's Principle:
Build a system that even a fool can use, and only a fool will
want to use it.