crash in init_ipic_sysfs on efika

I cant reproduce this bug on my board, but:

The global primary_ipic in arch/powerpc/sysdev/ipic.c can remain NULL if
ipic_init() fails. init_ipic_sysfs() will crash in that case.

Something like this may fix it:

Index: linux-2.6.25-rc6/arch/powerpc/sysdev/ipic.c
===================================================================

--- linux-2.6.25-rc6.orig/arch/powerpc/sysdev/ipic.c
+++ linux-2.6.25-rc6/arch/powerpc/sysdev/ipic.c

@@ -906,7 +906,7 @@ static int __init init_ipic_sysfs(void)
 {
        int rc;
 
-       if (!primary_ipic->regs)
+       if (!primary_ipic || !primary_ipic->regs)
                return -ENODEV;
        printk(KERN_DEBUG "Registering ipic with sysfs...\n");
 

ok boot eth:192.168.2.52,inst32 inst32 console=ttyPSC0,115200

SuSE Linux zImage starting: loaded at 00040000-00d24cbc (0/0/01003ed8;
sp: 017ffe80)
uncompressing ELF header done. (00000100 bytes)
Allocated 008584d4 bytes for kernel @ 02000000
Allocated 00a963a2 bytes for initrd @ 02859000
uncompressing kernel done. (004faeb8 bytes)
entering kernel at 02010000(2859000/a963a2/01003ed8)
OF stdout device is: /failsafe
command line: console=ttyPSC0,115200
memory layout at init:
  alloc_bottom : 032f0000
  alloc_top    : 30000000
  alloc_top_hi : f000c000
  rmo_top      : 30000000
  ram_top      : f000c000
Looking for displays
instantiating rtas at 0x07ffb000 ... done
Applying EFIKA device tree fixups
Fixing bestcomm interrupts property
Adding Ethernet MDIO node
Adding Ethernet PHY node
copying OF device tree ...
Building dt strings...
Building dt structure...
Device tree strings 0x032f1000 -> 0x032f17e0
Device tree struct  0x032f2000 -> 0x03315000
Calling quiesce ...
returning from prom_init
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
Memory: 113264k/131072k available (4808k kernel code, 17648k reserved,
164k data, 461k bss, 228k init)
Security Framework initialized
AppArmor: AppArmor initialized
AppArmor: Unable to log event (1505) to audit subsys
AppArmor: Registered secondary security module name="capability"
AppArmor: Unable to log event (1505) to audit subsys
Capability LSM initialized as secondary

41 more lines

Olaf Hering writes:

I cant reproduce this bug on my board, but:

The global primary_ipic in arch/powerpc/sysdev/ipic.c can remain NULL if
ipic_init() fails. init_ipic_sysfs() will crash in that case.

Something like this may fix it:

Is this needed for 2.6.25?  Is the system at all usable if ipic_init
fails?

Paul.

On Mon, Mar 17, Olaf Hering wrote:

ipic_init() is not called for every board.
This change for the used config fixes the crash as well.

@@ -168,14 +168,14 @@ CONFIG_PPC_MULTIPLATFORM=y
 # CONFIG_PPC_86xx is not set
 CONFIG_CLASSIC32=y
 CONFIG_PPC_CHRP=y
-CONFIG_PPC_MPC512x=y
-CONFIG_PPC_MPC5121=y
-CONFIG_MPC5121_ADS=y
+# CONFIG_PPC_MPC512x is not set
+# CONFIG_PPC_MPC5121 is not set
+# CONFIG_MPC5121_ADS is not set
 CONFIG_PPC_MPC52xx=y
-CONFIG_PPC_MPC5200_SIMPLE=y
+# CONFIG_PPC_MPC5200_SIMPLE is not set
 CONFIG_PPC_EFIKA=y
 # CONFIG_PPC_LITE5200 is not set
-CONFIG_PPC_MPC5200_BUGFIX=y
+# CONFIG_PPC_MPC5200_BUGFIX is not set
 CONFIG_PPC_PMAC=y
 # CONFIG_PPC_CELL is not set
 # CONFIG_PPC_CELL_NATIVE is not set

On Wed, Mar 19, Paul Mackerras wrote:

Olaf Hering writes:

quoted

I cant reproduce this bug on my board, but:

The global primary_ipic in arch/powerpc/sysdev/ipic.c can remain NULL if
ipic_init() fails. init_ipic_sysfs() will crash in that case.

Something like this may fix it:

Is this needed for 2.6.25?  Is the system at all usable if ipic_init
fails?

CONFIG_PPC_MPC51* needs to be disabled for pmac/chrp/bplan .configs.

A full featured ppc32_defconfig will probably catch such errors.

This was a good catch. IPIC is the controller on the MPC83xx type boards (and
MPC5121E) so building a kernel with this controller code in there, but without
the controller present, should gladly fail.

However removing the boards from the config does not fix this obvious error. I
like your patch better than changing the config options to disable entire
swathes of board support :D

What I'm concerned about is that the IPIC code is registering with sysfs even
though there is no controller present. How on EARTH does it even get that far?
If ipic_init fails, no further code should be run from this file, in my view.

subsys_initcall(init_ipic_sysfs); <-- this, in my eyes, is the culprit. If
init_ipic() runs, init_ipic_sysfs should be called from that, not left for
some further subsystem to blindly try and register sysfs nodes for devices
which may not even be present..

I'd love someone to explain to me how this works and why, and why it isn't
braindead? :D

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Olaf Hering wrote:

9 more quoted lines

On Mar 17, 2008, at 2:53 PM, Olaf Hering wrote:

We should look and see if we can change init_ipic_sysfs() from a subsys_initcall() to just getting called at the end of ipic_init().

- k

Kumar Gala writes:

We should look and see if we can change init_ipic_sysfs() from a  
subsys_initcall() to just getting called at the end of ipic_init().

If init_ipic is called from init_IRQ then I would expect that's way
too early to be calling sysfs stuff.

Paul.

On Mon, Mar 17, 2008 at 1:53 PM, Olaf Hering [off-list ref] wrote:

 I cant reproduce this bug on my board, but:

 The global primary_ipic in arch/powerpc/sysdev/ipic.c can remain NULL if
 ipic_init() fails. init_ipic_sysfs() will crash in that case.

 Something like this may fix it:

 Index: linux-2.6.25-rc6/arch/powerpc/sysdev/ipic.c
 ===================================================================
 --- linux-2.6.25-rc6.orig/arch/powerpc/sysdev/ipic.c
 +++ linux-2.6.25-rc6/arch/powerpc/sysdev/ipic.c
 @@ -906,7 +906,7 @@ static int __init init_ipic_sysfs(void)
  {
        int rc;

 -       if (!primary_ipic->regs)
 +       if (!primary_ipic || !primary_ipic->regs)
                return -ENODEV;
        printk(KERN_DEBUG "Registering ipic with sysfs...\n");

Confirmed, this patch fixes the problem.  Paulus or Kumar, can you
please pick it up for .25?

Thanks,
g.

 ok boot eth:192.168.2.52,inst32 inst32 console=ttyPSC0,115200

 SuSE Linux zImage starting: loaded at 00040000-00d24cbc (0/0/01003ed8;
 sp: 017ffe80)
 uncompressing ELF header done. (00000100 bytes)
 Allocated 008584d4 bytes for kernel @ 02000000
 Allocated 00a963a2 bytes for initrd @ 02859000
 uncompressing kernel done. (004faeb8 bytes)
 entering kernel at 02010000(2859000/a963a2/01003ed8)
 OF stdout device is: /failsafe
 command line: console=ttyPSC0,115200
 memory layout at init:
  alloc_bottom : 032f0000
  alloc_top    : 30000000
  alloc_top_hi : f000c000
  rmo_top      : 30000000
  ram_top      : f000c000
 Looking for displays
 instantiating rtas at 0x07ffb000 ... done
 Applying EFIKA device tree fixups
 Fixing bestcomm interrupts property
 Adding Ethernet MDIO node
 Adding Ethernet PHY node
 copying OF device tree ...
 Building dt strings...
 Building dt structure...
 Device tree strings 0x032f1000 -> 0x032f17e0
 Device tree struct  0x032f2000 -> 0x03315000
 Calling quiesce ...
 returning from prom_init
 Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
 Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
 Memory: 113264k/131072k available (4808k kernel code, 17648k reserved,
 164k data, 461k bss, 228k init)
 Security Framework initialized
 AppArmor: AppArmor initialized
 AppArmor: Unable to log event (1505) to audit subsys

50 more quoted lines

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Grant Likely writes:

Confirmed, this patch fixes the problem.  Paulus or Kumar, can you
please pick it up for .25?

Sure, will do.  I thought about putting it in the last batch but I
wanted an ack from you.

Anyone else got any last-minute things for 2.6.25?

Paul.

On Fri, 21 Mar 2008, Paul Mackerras wrote:

Grant Likely writes:

quoted

Confirmed, this patch fixes the problem.  Paulus or Kumar, can you
please pick it up for .25?

Sure, will do.  I thought about putting it in the last batch but I
wanted an ack from you.

Anyone else got any last-minute things for 2.6.25?

since you are going to send something to linus let me update the fsl
defconfigs and send you a pull request.

- k

On Thu, 2008-03-20 at 11:32 +0000, Matt Sealey wrote:

subsys_initcall(init_ipic_sysfs); <-- this, in my eyes, is the culprit. If
init_ipic() runs, init_ipic_sysfs should be called from that, not left for
some further subsystem to blindly try and register sysfs nodes for devices
which may not even be present..

I'd love someone to explain to me how this works and why, and why it isn't
braindead? :D

init_ipic() is called way too early to be able to manpipulate sysfs.

Ben.

Is the MPC5200B PSC-AC97 driver in there?

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Paul Mackerras wrote:

Grant Likely writes:

quoted

Confirmed, this patch fixes the problem.  Paulus or Kumar, can you
please pick it up for .25?

Sure, will do.  I thought about putting it in the last batch but I
wanted an ack from you.

Anyone else got any last-minute things for 2.6.25?

Paul.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

On Fri, Mar 21, 2008 at 7:14 AM, Matt Sealey [off-list ref] wrote:

Is the MPC5200B PSC-AC97 driver in there?

Audio drivers need to go in via one of the ALSA developers and I
haven't been paying close enough attention to know if it has not in
yet.

g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

On Fri, Mar 21, 2008 at 12:51:46PM -0600, Grant Likely wrote:

On Fri, Mar 21, 2008 at 7:14 AM, Matt Sealey [off-list ref] wrote:

quoted

Is the MPC5200B PSC-AC97 driver in there?

Audio drivers need to go in via one of the ALSA developers and I
haven't been paying close enough attention to know if it has not in
yet.

BTW, it was reported to me that the ethernet drivers don't get
autoloaded by udev. Is this a failure of udev missing the of_plateform
support, or a deficiency of the ethernet drivers ? 

Friendly,

Sven Luther

On Sat, 2008-03-22 at 16:25 +0100, Sven Luther wrote:

BTW, it was reported to me that the ethernet drivers don't get
autoloaded by udev. Is this a failure of udev missing the of_plateform
support, or a deficiency of the ethernet drivers ? 

As far as I know, they _are_ being loaded properly. Otherwise, the
Fedora installer wouldn't work.

-- 
dwmw2

On Sun, Mar 23, 2008 at 09:00:41AM +0000, David Woodhouse wrote:

On Sat, 2008-03-22 at 16:25 +0100, Sven Luther wrote:

quoted

BTW, it was reported to me that the ethernet drivers don't get
autoloaded by udev. Is this a failure of udev missing the of_plateform
support, or a deficiency of the ethernet drivers ? 

As far as I know, they _are_ being loaded properly. Otherwise, the
Fedora installer wouldn't work.

Yes, but you probably have the of_plateform udev patch, no ? 

Friendly,

Sven Luther

On Sun, 2008-03-23 at 11:12 +0100, Sven Luther wrote:

On Sun, Mar 23, 2008 at 09:00:41AM +0000, David Woodhouse wrote:

quoted

On Sat, 2008-03-22 at 16:25 +0100, Sven Luther wrote:

quoted

BTW, it was reported to me that the ethernet drivers don't get
autoloaded by udev. Is this a failure of udev missing the of_plateform
support, or a deficiency of the ethernet drivers ? 

As far as I know, they _are_ being loaded properly. Otherwise, the
Fedora installer wouldn't work.

Yes, but you probably have the of_plateform udev patch, no ? 

Not any more. That was merged a while ago, wasn't it?

-- 
dwmw2

On Sun, Mar 23, 2008 at 10:13:12AM +0000, David Woodhouse wrote:

On Sun, 2008-03-23 at 11:12 +0100, Sven Luther wrote:

quoted

On Sun, Mar 23, 2008 at 09:00:41AM +0000, David Woodhouse wrote:

quoted

On Sat, 2008-03-22 at 16:25 +0100, Sven Luther wrote:

quoted

BTW, it was reported to me that the ethernet drivers don't get
autoloaded by udev. Is this a failure of udev missing the of_plateform
support, or a deficiency of the ethernet drivers ? 

As far as I know, they _are_ being loaded properly. Otherwise, the
Fedora installer wouldn't work.

Yes, but you probably have the of_plateform udev patch, no ? 

Not any more. That was merged a while ago, wasn't it?

So, the only explanation would be if debian had a very outdated udev
which does not include this patch.

Friendly,

Sven Luther

Kumar Gala writes:

since you are going to send something to linus let me update the fsl
defconfigs and send you a pull request.

OK -- I'm about to send Linus a pull request now in case he feels like
releasing 2.6.25 this weekend.  I'll send him another pull request
with your defconfig update and whatever else has turned up when I get
your pull request.

Paul.

My assessment of the situation is: not at all.

-- 
Matt Sealey [off-list ref]
Genesi, Manager, Developer Relations

Grant Likely wrote:

On Fri, Mar 21, 2008 at 7:14 AM, Matt Sealey [off-list ref] wrote:

quoted

Is the MPC5200B PSC-AC97 driver in there?

Audio drivers need to go in via one of the ALSA developers and I
haven't been paying close enough attention to know if it has not in
yet.

g.

On Mar 24, 2008, at 3:02 AM, Paul Mackerras wrote:

Kumar Gala writes:

quoted

since you are going to send something to linus let me update the fsl
defconfigs and send you a pull request.

OK -- I'm about to send Linus a pull request now in case he feels like
releasing 2.6.25 this weekend.  I'll send him another pull request
with your defconfig update and whatever else has turned up when I get
your pull request.

ok and sent you a pull request.

- k