Re: MIPS panic in 2.6.39 (bisected to 7eaceaccab5f)

Re: MIPS panic in 2.6.39 (bisected to 7eaceaccab5f)

[Ralf Baechle]

On Fri, May 27, 2011 at 08:55:13AM +0100, Ralf Baechle wrote:

> > Have you guys been able to reproduce the problem?
> 
> Staring at the disassembly was good enough, I think.  The commit you
> bisected is restructuring some of the hardware probing code for Malta and
> seems to result in gcmp_present being set without _gcmp_base having been
> assigned, thus the null pointer dereference.

Can you test below patch?  Thanks,

  Ralf

Since af3a1f6f4813907e143f87030cde67a9971db533 the Malta code does no
longer probe for presence of GCMP if CMP is not configured.  This means
that the variable gcmp_present well be left at its default value of -1
which normally is meant to indicate that GCMP has not yet been mmapped.
This non-zero value is now interpreted as GCMP being present resulting
in a write attempt to a GCMP register resulting in a crash.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/include/asm/smp-ops.h          |   41 +++++++++++++++++++++++++++--
 arch/mips/mipssim/sim_setup.c            |   17 ++++++------
 arch/mips/mti-malta/malta-init.c         |   13 ++++-----
 arch/mips/pmc-sierra/msp71xx/msp_setup.c |    8 ++---
 4 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/arch/mips/include/asm/smp-ops.h b/arch/mips/include/asm/smp-ops.h
index 9e09af3..48b03ff 100644
--- a/arch/mips/include/asm/smp-ops.h
+++ b/arch/mips/include/asm/smp-ops.h
@@ -56,8 +56,43 @@ static inline void register_smp_ops(struct plat_smp_ops *ops)
 
 #endif /* !CONFIG_SMP */
 
-extern struct plat_smp_ops up_smp_ops;
-extern struct plat_smp_ops cmp_smp_ops;
-extern struct plat_smp_ops vsmp_smp_ops;
+static inline int register_up_smp_ops(void)
+{
+#ifdef CONFIG_SMP_UP
+	extern struct plat_smp_ops up_smp_ops;
+
+	register_smp_ops(&up_smp_ops);
+
+	return 0;
+#else
+	return -ENODEV;
+#endif
+}
+
+static inline int register_cmp_smp_ops(void)
+{
+#ifdef CONFIG_MIPS_CMP
+	extern struct plat_smp_ops cmp_smp_ops;
+
+	register_smp_ops(&cmp_smp_ops);
+
+	return 0;
+#else
+	return -ENODEV;
+#endif
+}
+
+static inline int register_vsmp_smp_ops(void)
+{
+#ifdef CONFIG_MIPS_MT_SMP
+	extern struct plat_smp_ops vsmp_smp_ops;
+
+	register_smp_ops(&vsmp_smp_ops);
+
+	return 0;
+#else
+	return -ENODEV;
+#endif
+}
 
 #endif /* __ASM_SMP_OPS_H */
diff --git a/arch/mips/mipssim/sim_setup.c b/arch/mips/mipssim/sim_setup.c
index 55f22a3..1970069 100644
--- a/arch/mips/mipssim/sim_setup.c
+++ b/arch/mips/mipssim/sim_setup.c
@@ -59,18 +59,17 @@ void __init prom_init(void)
 
 	prom_meminit();
 
-#ifdef CONFIG_MIPS_MT_SMP
-	if (cpu_has_mipsmt)
-		register_smp_ops(&vsmp_smp_ops);
-	else
-		register_smp_ops(&up_smp_ops);
-#endif
+	if (cpu_has_mipsmt) {
+		if (!register_vsmp_smp_ops())
+			return;
+
 #ifdef CONFIG_MIPS_MT_SMTC
-	if (cpu_has_mipsmt)
 		register_smp_ops(&ssmtc_smp_ops);
-	else
-		register_smp_ops(&up_smp_ops);
+			return;
 #endif
+	}
+
+	register_up_smp_ops();
 }
 
 static void __init serial_init(void)
diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
index 31180c3..4163d09e 100644
--- a/arch/mips/mti-malta/malta-init.c
+++ b/arch/mips/mti-malta/malta-init.c
@@ -358,15 +358,14 @@ void __init prom_init(void)
 #ifdef CONFIG_SERIAL_8250_CONSOLE
 	console_config();
 #endif
-#ifdef CONFIG_MIPS_CMP
 	/* Early detection of CMP support */
 	if (gcmp_probe(GCMP_BASE_ADDR, GCMP_ADDRSPACE_SZ))
-		register_smp_ops(&cmp_smp_ops);
-	else
-#endif
-#ifdef CONFIG_MIPS_MT_SMP
-		register_smp_ops(&vsmp_smp_ops);
-#endif
+		if (!register_cmp_smp_ops())
+			return;
+
+	if (!register_vsmp_smp_ops())
+		return;
+
 #ifdef CONFIG_MIPS_MT_SMTC
 	register_smp_ops(&msmtc_smp_ops);
 #endif
diff --git a/arch/mips/pmc-sierra/msp71xx/msp_setup.c b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
index 2413ea6..0abfbe0 100644
--- a/arch/mips/pmc-sierra/msp71xx/msp_setup.c
+++ b/arch/mips/pmc-sierra/msp71xx/msp_setup.c
@@ -228,13 +228,11 @@ void __init prom_init(void)
 	 */
 	msp_serial_setup();
 
-#ifdef CONFIG_MIPS_MT_SMP
-	register_smp_ops(&vsmp_smp_ops);
-#endif
-
+	if (register_vsmp_smp_ops()) {
 #ifdef CONFIG_MIPS_MT_SMTC
-	register_smp_ops(&msp_smtc_smp_ops);
+		register_smp_ops(&msp_smtc_smp_ops);
 #endif
+	}
 
 #ifdef CONFIG_PMCTWILED
 	/*

Re: MIPS panic in 2.6.39 (bisected to 7eaceaccab5f)

[Rob Landley]

On 05/27/2011 09:00 AM, Ralf Baechle wrote:
> On Fri, May 27, 2011 at 08:55:13AM +0100, Ralf Baechle wrote:
> 
>>> Have you guys been able to reproduce the problem?
>>
>> Staring at the disassembly was good enough, I think.  The commit you
>> bisected is restructuring some of the hardware probing code for Malta and
>> seems to result in gcmp_present being set without _gcmp_base having been
>> assigned, thus the null pointer dereference.
> 
> Can you test below patch?  Thanks,

arch/mips/mti-malta/malta-init.c: In function 'prom_init':
arch/mips/mti-malta/malta-init.c:363: error: implicit declaration of
function 'register_cmp_smp_ops'
arch/mips/mti-malta/malta-init.c:366: error: implicit declaration of
function 'register_vsmp_smp_ops'
make[2]: *** [arch/mips/mti-malta/malta-init.o] Error 1

Rob

Re: MIPS panic in 2.6.39 (bisected to 7eaceaccab5f)

[Rob Landley]

On 05/27/2011 09:00 AM, Ralf Baechle wrote:
> On Fri, May 27, 2011 at 08:55:13AM +0100, Ralf Baechle wrote:
> 
>>> Have you guys been able to reproduce the problem?
>>
>> Staring at the disassembly was good enough, I think.  The commit you
>> bisected is restructuring some of the hardware probing code for Malta and
>> seems to result in gcmp_present being set without _gcmp_base having been
>> assigned, thus the null pointer dereference.
> 
> Can you test below patch?  Thanks,
> 
>   Ralf
> 
> Since af3a1f6f4813907e143f87030cde67a9971db533 the Malta code does no
> longer probe for presence of GCMP if CMP is not configured.  This means
> that the variable gcmp_present well be left at its default value of -1
> which normally is meant to indicate that GCMP has not yet been mmapped.
> This non-zero value is now interpreted as GCMP being present resulting
> in a write attempt to a GCMP register resulting in a crash.
> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

patch patch patch...

> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
> index 31180c3..4163d09e 100644
> --- a/arch/mips/mti-malta/malta-init.c
> +++ b/arch/mips/mti-malta/malta-init.c

Your missing hunk at the top of this file is:

@@ -29,6 +29,7 @@
 #include <asm/system.h>
 #include <asm/cacheflush.h>
 #include <asm/traps.h>
+#include <asm/smp-ops.h>

 #include <asm/gcmpregs.h>
 #include <asm/mips-boards/prom.h>

And then the patch works!  Yay!  Thank you.

Signed-off-by: Rob Landley <rob@landley.net>

Rob

Re: MIPS panic in 2.6.39 (bisected to 7eaceaccab5f)

[Ralf Baechle]

On Sat, May 28, 2011 at 05:48:35AM -0500, Rob Landley wrote:

> Your missing hunk at the top of this file is:
> 
> @@ -29,6 +29,7 @@
>  #include <asm/system.h>
>  #include <asm/cacheflush.h>
>  #include <asm/traps.h>
> +#include <asm/smp-ops.h>
> 
>  #include <asm/gcmpregs.h>
>  #include <asm/mips-boards/prom.h>
> 
> And then the patch works!  Yay!  Thank you.

Thanks Rob!  I fixed that and the patch is now in the MIPS git.

  Ralf

Re: MIPS panic in 2.6.39 (bisected to 7eaceaccab5f)

[Rob Landley]

On 05/28/2011 11:28 AM, Ralf Baechle wrote:
> On Sat, May 28, 2011 at 05:48:35AM -0500, Rob Landley wrote:
> 
>> Your missing hunk at the top of this file is:
>>
>> @@ -29,6 +29,7 @@
>>  #include <asm/system.h>
>>  #include <asm/cacheflush.h>
>>  #include <asm/traps.h>
>> +#include <asm/smp-ops.h>
>>
>>  #include <asm/gcmpregs.h>
>>  #include <asm/mips-boards/prom.h>
>>
>> And then the patch works!  Yay!  Thank you.
> 
> Thanks Rob!  I fixed that and the patch is now in the MIPS git.
> 
>   Ralf

Do you think it's a candidate for stable?

Ro

Re: MIPS panic in 2.6.39 (bisected to 7eaceaccab5f)

[Ralf Baechle]

On Sat, May 28, 2011 at 02:56:35PM -0500, Rob Landley wrote:

> > Thanks Rob!  I fixed that and the patch is now in the MIPS git.
> > 
> >   Ralf
> 
> Do you think it's a candidate for stable?

I'm afraid so it is indeed.  I copied the patch to the -stable branch
right away.

  Ralf