Problem with module_init?

With the latest from Linus' tree, I get the following oops:

Oops: Kernel access of bad area, sig: 11 [#1]
Warp
Modules linked in: simple(+)
NIP: d14b8000 LR: c0001420 CTR: 00000000
REGS: cf2a1df0 TRAP: 0400   Not tainted  (2.6.29-pika)
MSR: 00029000 <EE,ME,CE>  CR: 24000022  XER: 00000000
TASK = cf896100[1673] 'insmod' THREAD: cf2a0000
GPR00: 00000000 cf2a1ea0 cf896100 d14b8000 00000001 d14b5360 ffffffff
00000000 GPR08: c03312fc c03312fc c0546018 c0546018 24000022 100872cc
0ffa7d00 007fff99 GPR16: 00400450 00800000 007fff00 0ffa7cec 00000000
ffffffff 00000000 1008323c GPR24: 10083254 00000000 c0330000 c0310000
c03110a4 d14b8000 cf2a0000 d14b5360 NIP [d14b8000] simple_init+0x0/0x8
[simple] LR [c0001420] do_one_initcall+0x34/0x1a0
Call Trace:
[cf2a1f20] [c0045e18] sys_init_module+0xb0/0x1b8
[cf2a1f40] [c000dbbc] ret_from_syscall+0x0/0x3c
Instruction dump:
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX <38600000> 4e800020 XXXXXXXX
XXXXXXXX ---[ end trace 9ac3b092cfc75a75 ]---

This is the entire driver:

#include <linux/module.h>

static int __init simple_init(void)
{
	return 0;
}
module_init(simple_init);

static void __exit simple_exit(void)
{
}
module_exit(simple_exit);

MODULE_DESCRIPTION("simple driver");
MODULE_LICENSE("GPL");

Has something changed for module?

Cheers,
   Sean

8d1cf34e7ad5c7738ce20d20bd7f002f562cb8b5 is first bad commit
commit 8d1cf34e7ad5c7738ce20d20bd7f002f562cb8b5
Author: Benjamin Herrenschmidt [off-list ref]
Date:   Thu Mar 19 19:34:08 2009 +0000

    powerpc/mm: Tweak PTE bit combination definitions
    
    This patch tweaks the way some PTE bit combinations are defined, in
    such a way that the 32 and 64-bit variant become almost identical
    and that will make it easier to bring in a new common pte-* file
    for the new variant of the Book3-E support.
    
    The combination of bits defining access to kernel pages are now
    clearly separated from the combination used by userspace and the
    core VM. The resulting generated code should remain identical
    unless I made a mistake. 
    Note: While at it, I removed a non-sensical statement related to
    CONFIG_KGDB in ppc_mmu_32.c which could cause kernel mappings to be
    user accessible when that option is enabled. Probably something
    that bitrot. 
    Signed-off-by: Benjamin Herrenschmidt [off-list ref]

:040000 040000 c105fb9641b078f1a8a60c2a7900ca2dd088b364
e90678246962d7a455aa27ba12814c11cd9b3c22 M      arch

On Tue, Mar 31, 2009 at 11:11:53PM -0400, Sean MacLennan wrote:

8d1cf34e7ad5c7738ce20d20bd7f002f562cb8b5 is first bad commit
commit 8d1cf34e7ad5c7738ce20d20bd7f002f562cb8b5
Author: Benjamin Herrenschmidt [off-list ref]
Date:   Thu Mar 19 19:34:08 2009 +0000

   powerpc/mm: Tweak PTE bit combination definitions
   
   This patch tweaks the way some PTE bit combinations are defined, in
   such a way that the 32 and 64-bit variant become almost identical
   and that will make it easier to bring in a new common pte-* file
   for the new variant of the Book3-E support.
   
   The combination of bits defining access to kernel pages are now
   clearly separated from the combination used by userspace and the
   core VM. The resulting generated code should remain identical
   unless I made a mistake. 
   Note: While at it, I removed a non-sensical statement related to
   CONFIG_KGDB in ppc_mmu_32.c which could cause kernel mappings to be
   user accessible when that option is enabled. Probably something
   that bitrot. 
   Signed-off-by: Benjamin Herrenschmidt [off-list ref]

:040000 040000 c105fb9641b078f1a8a60c2a7900ca2dd088b364
e90678246962d7a455aa27ba12814c11cd9b3c22 M      arch

I'm assuming this is the result of a git-bisect run?

Can I also assume you were loading the module on your Warp board?

Ben, have you tested module loads on 4xx since you redid this?

josh

On Wed, 1 Apr 2009 07:27:56 -0400
Josh Boyer [off-list ref] wrote:

I'm assuming this is the result of a git-bisect run?

Correct.

Can I also assume you were loading the module on your Warp board?

Correct again.

Cheers,
  Sean

On Wed, 2009-04-01 at 07:27 -0400, Josh Boyer wrote:

I'm assuming this is the result of a git-bisect run?

Can I also assume you were loading the module on your Warp board?

Ben, have you tested module loads on 4xx since you redid this?

I must shamefully admit almost never testing modules ... I always
netboot zImages with all the drivers I need built-in.

I suspect I just screwed up the definition of PAGE_KERNEL_EXEC or
something like that. I'll have a look.

Cheers,
Ben.

On Thu, 02 Apr 2009 09:24:43 +1100
"Benjamin Herrenschmidt" [off-list ref] wrote:

I suspect I just screwed up the definition of PAGE_KERNEL_EXEC or
something like that.

Yup, that is exactly what you did ;) You left out _PAGE_HWEXEC. The
following patch fixes the problem for me.

Cheers,
   Sean

diff --git a/arch/powerpc/include/asm/pte-common.h b/arch/powerpc/include/asm/pte-common.h
index d9740e8..a84f248 100644
--- a/arch/powerpc/include/asm/pte-common.h
+++ b/arch/powerpc/include/asm/pte-common.h

@@ -167,7 +167,8 @@ extern unsigned long bad_call_to_PMD_PAGE_SIZE(void);
 #endif
 
 /* Make modules code happy. We don't set RO yet */
-#define PAGE_KERNEL_EXEC	PAGE_KERNEL_X
+// #define PAGE_KERNEL_EXEC	PAGE_KERNEL_X
+#define PAGE_KERNEL_EXEC __pgprot(_PAGE_BASE | _PAGE_KERNEL_RW | _PAGE_EXEC | _PAGE_HWEXEC)
 
 /* Advertise special mapping type for AGP */
 #define PAGE_AGP		(PAGE_KERNEL_NC)

On Thu, 2009-04-02 at 00:08 -0400, Sean MacLennan wrote:

On Thu, 02 Apr 2009 09:24:43 +1100
"Benjamin Herrenschmidt" [off-list ref] wrote:

quoted

I suspect I just screwed up the definition of PAGE_KERNEL_EXEC or
something like that.

Yup, that is exactly what you did ;) You left out _PAGE_HWEXEC. The
following patch fixes the problem for me.

The proper fix is for PAGE_KERNEL_X to have _PAGE_HWEXEC. I'll fix that.

Cheers,
Ben.

On Thu, 02 Apr 2009 15:26:02 +1100
Benjamin Herrenschmidt [off-list ref] wrote:

The proper fix is for PAGE_KERNEL_X to have _PAGE_HWEXEC. I'll fix
that.

That may not be enough. I made that change (adding _PAGE_HWEXEC to
PAGE_KERNEL_X) and it works for some drivers, but I am still crashing in
one driver. Although, that *could* be unrelated bug.

Cheers,
   Sean

On Thu, 2 Apr 2009 00:39:05 -0400
"Sean MacLennan" [off-list ref] wrote:

Although, that *could* be unrelated bug.

It *is* an unrelated bug. dma_alloc_coherent now requires a device
where before it was optional. Carry on.

Cheers,
   Sean

On Thu, 2009-04-02 at 00:51 -0400, Sean MacLennan wrote:

On Thu, 2 Apr 2009 00:39:05 -0400
"Sean MacLennan" [off-list ref] wrote:

quoted

Although, that *could* be unrelated bug.

It *is* an unrelated bug. dma_alloc_coherent now requires a device
where before it was optional. Carry on.

Right, that's new in .30

Cheers,
Ben.

Cheers,
   Sean