LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: PPC upstream kernel ignored DABR bug
From: Jan Kratochvil @ 2007-11-28  8:59 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Paul Mackerras, Roland McGrath
In-Reply-To: <200711272335.36981.arnd@arndb.de>

On Tue, 27 Nov 2007 23:35:36 +0100, Arnd Bergmann wrote:
> On Monday 26 November 2007, Jan Kratochvil wrote:
> > Hi,
> > 
> > this testcase:
> >         http://people.redhat.com/jkratoch/dabr-lost.c
> > 
> > reproduces a PPC DABR kernel bug.  The variable `variable' should not get
> > modified as the thread modifying it should be caught by its DABR:
> > 
> > $ ./dabr-lost
> > TID 30914: DABR 0x10012a77 NIP 0x80f6ebb318
> > TID 30915: DABR 0x10012a77 NIP 0x80f6ebb318
> > TID 30916: DABR 0x10012a77 NIP 0x80f6ebb318
> > TID 30914: hitting the variable
> > TID 30915: hitting the variable
> > TID 30916: hitting the variable
> > variable found = 30916, caught TID = 30914
> > TID 30916: DABR 0x10012a77
> > Variable got modified by a thread which has DABR still set!
> > 
> 
> This sounds like a bug recently reported by Uli Weigand. BenH
> said he'd take a look, but it probably fell under the table.
> The problem found by Uli is that on certain processors (Cell/B.E.
> in his case), the DABRX register needs to be set in order for
> the DABR to take effect.

Please be aware DABR works fine if the same code runs just 1 (always) or
2 (sometimes) threads.  It starts failing with too many threads running:

$ ./dabr-lost
TID 32725: DABR 0x1001279f NIP 0xfecf41c
TID 32726: DABR 0x1001279f NIP 0xfecf41c
TID 32725: hitting the variable
variable found = -1, caught TID = 32725
TID 32726: hitting the variable
variable found = -1, caught TID = 32726
The kernel bug did not get reproduced - increase THREADS.

As I did not find any code in that kernel touching DABRX its value should not
be dependent on the number of threads running.


Regards,
Lace

^ permalink raw reply

* Re: [PATCH] PPC: CELLEB - fix potential NULL pointer dereference
From: Ishizaki Kou @ 2007-11-28  8:52 UTC (permalink / raw)
  To: gorcunov; +Cc: olof, linuxppc-dev, paulus, linux-kernel
In-Reply-To: <aa79d98a0711252346m583b652bocf72e11077da352e@mail.gmail.com>

> This patch adds checking for NULL value returned to prevent possible
> NULL pointer dereference.
> Also two unneeded 'return' are removed.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

Thanks, I tested your patch and it works.

My original code supposes that the device-tree is provided correctly,
so I omited such checks. (Sorry, it should have been commented.)

Should we check more strictly like your patch?

Best regards,
Kou Ishizaki

^ permalink raw reply

* Getting physical memory information from /proc/iomem
From: Rajasekaran Periyasamy @ 2007-11-28  6:32 UTC (permalink / raw)
  To: linuxppc-embedded


[-- Attachment #1.1: Type: text/plain, Size: 1580 bytes --]

Hi,
I have a MPC7447A board running linux 2.6.21. I want to get the physical RAM layout from /proc/iomem, but it is not proper as of x86 platform. Here is the output of my target's /proc/iomem.

[root@rootfs ~]# cat /proc/iomem
80000000-87ffffff : PCI hose 0 MEM Space 0
  86900000-869fffff : 0000:00:02.2
  86a00000-86afffff : 0000:00:02.2
.
.

  87f00000-87f7ffff : 0000:00:01.0
  87ffffc0-87ffffff : 0000:00:01.0
88000000-8fffffff : PCI hose 1 MEM Space 0
  8d3fe000-8d3fefff : 0001:01:05.2
  8d3ff000-8d3fffff : 0001:01:05.1
.
.

  8e000000-8effffff : 0001:01:04.0
  8f800000-8fffffff : 0001:01:04.0
fbe02000-fbe03fff : ethernet shared base
fbe04000-fbe04c17 : sdma 0 base
  fbe04000-fbe04c17 : sdma_regs
fbe06000-fbe06c17 : sdma 1 base
  fbe06000-fbe06c17 : sdma_regs
fbe08000-fbe08037 : mpsc 0 base
  fbe08000-fbe08037 : mpsc_regs
fbe09000-fbe09037 : mpsc 1 base
  fbe09000-fbe09037 : mpsc_regs
fbe0b200-fbe0b207 : brg 0 base
  fbe0b200-fbe0b207 : brg_regs
fbe0b208-fbe0b20f : brg 1 base
  fbe0b208-fbe0b20f : brg_regs
fbe0b400-fbe0b40b : mpsc routing base
  fbe0b400-fbe0b40b : mpsc_routing_regs
fbe0b800-fbe0b883 : sdma intr base
  fbe0b800-fbe0b883 : sdma_intr_regs
fbe0c000-fbe0c01f : mv64xxx i2c base
  fbe0c000-fbe0c01f : mv64xxx_i2c adapter
ff000000-ffffffff : physmap-flash.0
  ff000000-ffffffff : physmap-flash.0


Can you please anybody tell me how to interpret this to get physical RAM addresses. Or is there any patch available to get the details of System RAM from /proc/iomem ??

Thanks in advance,
Rajasekaran.P

[-- Attachment #1.2: Type: text/html, Size: 2246 bytes --]

^ permalink raw reply

* Re: [PATCH] PPC: CELLEB - fix potential NULL pointer dereference
From: Michael Ellerman @ 2007-11-28  9:48 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Olof Johansson, PPCML, Paul Mackerras, LKML
In-Reply-To: <aa79d98a0711252346m583b652bocf72e11077da352e@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]

On Mon, 2007-11-26 at 10:46 +0300, Cyrill Gorcunov wrote:
> This patch adds checking for NULL value returned to prevent possible
> NULL pointer dereference.
> Also two unneeded 'return' are removed.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> Any comments are welcome.

I guess it's good to be paranoid, but this is a little verbose:

        wi0 = of_get_property(node, "device-id", NULL);
+       if (unlikely((!wi0))) {
+               printk(KERN_ERR "PCI: device-id not found.\n");
+               goto error;
+       }
        wi1 = of_get_property(node, "vendor-id", NULL);
+       if (unlikely((!wi1))) {
+               printk(KERN_ERR "PCI: vendor-id not found.\n");
+               goto error;
+       }
        wi2 = of_get_property(node, "class-code", NULL);
+       if (unlikely((!wi2))) {
+               printk(KERN_ERR "PCI: class-code not found.\n");
+               goto error;
+       }
        wi3 = of_get_property(node, "revision-id", NULL);
+       if (unlikely((!wi3))) {
+               printk(KERN_ERR "PCI: revision-id not found.\n");
+               goto error;
+       }

Perhaps instead:

        wi0 = of_get_property(node, "device-id", NULL);
        wi1 = of_get_property(node, "vendor-id", NULL);
        wi2 = of_get_property(node, "class-code", NULL);
        wi3 = of_get_property(node, "revision-id", NULL);

       if (!wi0 || !wi1 || !wi2 || !wi3) {
               printk(KERN_ERR "PCI: Missing device tree properties.\n");
               goto error;
       }


cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 0/3] OF-platform PATA driver
From: Paul Mundt @ 2007-11-28  9:51 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: Arnd Bergmann, Jeff Garzik, linux-ide, linuxppc-dev
In-Reply-To: <20071127153708.GA12490@localhost.localdomain>

On Tue, Nov 27, 2007 at 06:37:08PM +0300, Anton Vorontsov wrote:
> Here is the second spin of the OF-platform PATA driver and
> related patches.
> 
So either the patches are missing, or I wasn't CC'ed on them. I'm going
to go out on a limb and assume the latter. If you wish me to Ack them,
I'm not going to start grovelling around list archives trying to find the
relevant posts.

This is now the second time this has happened, the first time I was only
made aware of this work as Jeff forwarded them along. CC'ing the authors
of code you are changing shouldn't be a cryptic requirement we have to
spell out in SubmittingPatches or so, especially if you're soliciting
Acked-bys. Please fix your development methodology, thanks.

^ permalink raw reply

* Re: [PATCH] [POWERPC] Emulate isel (Integer Select) instruction
From: Geert Uytterhoeven @ 2007-11-28 10:49 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20071127152317.GA5230@lixom.net>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 11545 bytes --]

On Tue, 27 Nov 2007, Olof Johansson wrote:
> On Tue, Nov 27, 2007 at 03:56:53PM +0100, Geert Uytterhoeven wrote:
> >  include/asm-powerpc/emulator.h |   60 ++++++++++++++++++++++++++++++++++++++
> 
> This name stood out as being a bit too generic, emulator could mean
> system support for running under some sort of emulator as well. How
> about emulated_ops.h?

Changed.

> > +config DEBUG_WARN_EMULATED
> > +	bool "Warn if emulated instructions are used"
> > +	depends on DEBUG_KERNEL && SYSFS
> > +	help
> > +	  This option will cause messages to be printed if an instruction is
> > +	  emulated.
> > +	  Counters for emulated instruction usages are always available under
> > +	  /sys/devices/system/cpu/cpu*/emulated/, irrespective of the state
> > +	  of this option.
> 
> How about making it a sysctl instead, so it can be flipped at runtime
> (but default off)?

Converted to a sysctl.

Subject: powerpc: Keep track of emulated instructions

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

powerpc: Keep track of emulated instructions

Counters for the various classes of emulated instructions are available under
/sys/devices/system/cpu/cpu*/emulated/.
Optionally (controlled by /proc/sys/kernel/cpu_emulation_warnings),
rate-limited warnings can be printed to the console when instructions are
emulated.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 arch/powerpc/kernel/align.c        |   17 ++++--
 arch/powerpc/kernel/sysfs.c        |  100 ++++++++++++++++++++++++++++++++++++-
 arch/powerpc/kernel/traps.c        |   24 ++++++++
 include/asm-powerpc/emulated_ops.h |   52 +++++++++++++++++++
 4 files changed, 186 insertions(+), 7 deletions(-)

--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -24,6 +24,7 @@
 #include <asm/system.h>
 #include <asm/cache.h>
 #include <asm/cputable.h>
+#include <asm/emulated_ops.h>
 
 struct aligninfo {
 	unsigned char len;
@@ -696,8 +697,10 @@ int fix_alignment(struct pt_regs *regs)
 	areg = dsisr & 0x1f;		/* register to update */
 
 #ifdef CONFIG_SPE
-	if ((instr >> 26) == 0x4)
+	if ((instr >> 26) == 0x4) {
+		WARN_EMULATE(spe);
 		return emulate_spe(regs, reg, instr);
+	}
 #endif
 
 	instr = (dsisr >> 10) & 0x7f;
@@ -731,17 +734,21 @@ int fix_alignment(struct pt_regs *regs)
 	/* A size of 0 indicates an instruction we don't support, with
 	 * the exception of DCBZ which is handled as a special case here
 	 */
-	if (instr == DCBZ)
+	if (instr == DCBZ) {
+		WARN_EMULATE(dcbz);
 		return emulate_dcbz(regs, addr);
+	}
 	if (unlikely(nb == 0))
 		return 0;
 
 	/* Load/Store Multiple instructions are handled in their own
 	 * function
 	 */
-	if (flags & M)
+	if (flags & M) {
+		WARN_EMULATE(multiple);
 		return emulate_multiple(regs, addr, reg, nb,
 					flags, instr, swiz);
+	}
 
 	/* Verify the address of the operand */
 	if (unlikely(user_mode(regs) &&
@@ -758,8 +765,10 @@ int fix_alignment(struct pt_regs *regs)
 	}
 
 	/* Special case for 16-byte FP loads and stores */
-	if (nb == 16)
+	if (nb == 16) {
+		WARN_EMULATE(fp_pair);
 		return emulate_fp_pair(regs, addr, reg, flags);
+	}
 
 	/* If we are loading, get the data from user space, else
 	 * get it from register values
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -8,6 +8,7 @@
 #include <linux/nodemask.h>
 #include <linux/cpumask.h>
 #include <linux/notifier.h>
+#include <linux/sysctl.h>
 
 #include <asm/current.h>
 #include <asm/processor.h>
@@ -19,6 +20,7 @@
 #include <asm/lppaca.h>
 #include <asm/machdep.h>
 #include <asm/smp.h>
+#include <asm/emulated_ops.h>
 
 static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
@@ -291,12 +293,101 @@ static struct sysdev_attribute pa6t_attr
 };
 
 
+#define SYSFS_EMULATED_SETUP(type)					\
+DEFINE_PER_CPU(atomic_long_t, emulated_ ## type);			\
+static ssize_t show_emulated_ ## type (struct sys_device *dev,		\
+				       char *buf)			\
+{									\
+	struct cpu *cpu = container_of(dev, struct cpu, sysdev);	\
+									\
+	return sprintf(buf, "%lu\n",					\
+		       atomic_long_read(&per_cpu(emulated_ ## type,	\
+					cpu->sysdev.id)));		\
+}									\
+									\
+static struct sysdev_attribute emulated_ ## type ## _attr = {		\
+	.attr = { .name = #type, .mode = 0400 },			\
+	.show = show_emulated_ ## type,					\
+};
+
+SYSFS_EMULATED_SETUP(dcba);
+SYSFS_EMULATED_SETUP(dcbz);
+SYSFS_EMULATED_SETUP(fp_pair);
+SYSFS_EMULATED_SETUP(mcrxr);
+SYSFS_EMULATED_SETUP(mfpvr);
+SYSFS_EMULATED_SETUP(multiple);
+SYSFS_EMULATED_SETUP(popcntb);
+SYSFS_EMULATED_SETUP(spe);
+SYSFS_EMULATED_SETUP(string);
+#ifdef CONFIG_MATH_EMULATION
+SYSFS_EMULATED_SETUP(math);
+#elif defined(CONFIG_8XX_MINIMAL_FPEMU)
+SYSFS_EMULATED_SETUP(8xx);
+#endif
+
+static struct attribute *emulated_attrs[] = {
+	&emulated_dcba_attr.attr,
+	&emulated_dcbz_attr.attr,
+	&emulated_fp_pair_attr.attr,
+	&emulated_mcrxr_attr.attr,
+	&emulated_mfpvr_attr.attr,
+	&emulated_multiple_attr.attr,
+	&emulated_popcntb_attr.attr,
+	&emulated_spe_attr.attr,
+	&emulated_string_attr.attr,
+#ifdef CONFIG_MATH_EMULATION
+	&emulated_math_attr.attr,
+#elif defined(CONFIG_8XX_MINIMAL_FPEMU)
+	&emulated_8xx_attr.attr,
+#endif
+	NULL
+};
+
+static struct attribute_group emulated_attr_group = {
+	.attrs = emulated_attrs,
+	.name = "emulated"
+};
+
+int sysctl_warn_emulated;
+
+#ifdef CONFIG_SYSCTL
+static ctl_table warn_emulated_ctl_table[]={
+	{
+		.procname	= "cpu_emulation_warnings",
+		.data		= &sysctl_warn_emulated,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
+	{}
+};
+
+static ctl_table warn_emulated_sysctl_root[] = {
+	{
+		.ctl_name	= CTL_KERN,
+		.procname	= "kernel",
+		.mode		= 0555,
+		.child		= warn_emulated_ctl_table,
+	},
+	{}
+};
+
+static inline void warn_emulated_sysctl_register(void)
+{
+	int res = register_sysctl_table(warn_emulated_sysctl_root);
+	printk("@@@ register_sysctl_table() returned %d\n", res);
+}
+#else /* !CONFIG_SYSCTL */
+static inline void warn_emulated_sysctl_register(void) {}
+#endif /* !CONFIG_SYSCTL */
+
+
 static void register_cpu_online(unsigned int cpu)
 {
 	struct cpu *c = &per_cpu(cpu_devices, cpu);
 	struct sys_device *s = &c->sysdev;
 	struct sysdev_attribute *attrs, *pmc_attrs;
-	int i, nattrs;
+	int i, nattrs, res;
 
 	if (!firmware_has_feature(FW_FEATURE_ISERIES) &&
 			cpu_has_feature(CPU_FTR_SMT))
@@ -339,6 +430,11 @@ static void register_cpu_online(unsigned
 
 	if (cpu_has_feature(CPU_FTR_DSCR))
 		sysdev_create_file(s, &attr_dscr);
+
+	res = sysfs_create_group(&s->kobj, &emulated_attr_group);
+	if (res)
+		pr_warning("Cannot create emulated sysfs group for cpu %u\n",
+			   cpu);
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
@@ -560,6 +656,8 @@ static int __init topology_init(void)
 			register_cpu_online(cpu);
 	}
 
+	warn_emulated_sysctl_register();
+
 	return 0;
 }
 subsys_initcall(topology_init);
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -53,6 +53,7 @@
 #include <asm/processor.h>
 #endif
 #include <asm/kexec.h>
+#include <asm/emulated_ops.h>
 
 #ifdef CONFIG_DEBUGGER
 int (*__debugger)(struct pt_regs *regs);
@@ -707,6 +708,13 @@ static int emulate_popcntb_inst(struct p
 	return 0;
 }
 
+void do_warn_emulate(const char *type)
+{
+	if (printk_ratelimit())
+		pr_warning("%s used emulated %s instruction\n", current->comm,
+			   type);
+}
+
 static int emulate_instruction(struct pt_regs *regs)
 {
 	u32 instword;
@@ -721,31 +729,38 @@ static int emulate_instruction(struct pt
 
 	/* Emulate the mfspr rD, PVR. */
 	if ((instword & INST_MFSPR_PVR_MASK) == INST_MFSPR_PVR) {
+		WARN_EMULATE(mfpvr);
 		rd = (instword >> 21) & 0x1f;
 		regs->gpr[rd] = mfspr(SPRN_PVR);
 		return 0;
 	}
 
 	/* Emulating the dcba insn is just a no-op.  */
-	if ((instword & INST_DCBA_MASK) == INST_DCBA)
+	if ((instword & INST_DCBA_MASK) == INST_DCBA) {
+		WARN_EMULATE(dcba);
 		return 0;
+	}
 
 	/* Emulate the mcrxr insn.  */
 	if ((instword & INST_MCRXR_MASK) == INST_MCRXR) {
 		int shift = (instword >> 21) & 0x1c;
 		unsigned long msk = 0xf0000000UL >> shift;
 
+		WARN_EMULATE(mcrxr);
 		regs->ccr = (regs->ccr & ~msk) | ((regs->xer >> shift) & msk);
 		regs->xer &= ~0xf0000000UL;
 		return 0;
 	}
 
 	/* Emulate load/store string insn. */
-	if ((instword & INST_STRING_GEN_MASK) == INST_STRING)
+	if ((instword & INST_STRING_GEN_MASK) == INST_STRING) {
+		WARN_EMULATE(string);
 		return emulate_string_inst(regs, instword);
+	}
 
 	/* Emulate the popcntb (Population Count Bytes) instruction. */
 	if ((instword & INST_POPCNTB_MASK) == INST_POPCNTB) {
+		WARN_EMULATE(popcntb);
 		return emulate_popcntb_inst(regs, instword);
 	}
 
@@ -929,6 +944,8 @@ void SoftwareEmulation(struct pt_regs *r
 
 #ifdef CONFIG_MATH_EMULATION
 	errcode = do_mathemu(regs);
+	if (errcode >= 0)
+		WARN_EMULATE(math);
 
 	switch (errcode) {
 	case 0:
@@ -950,6 +967,9 @@ void SoftwareEmulation(struct pt_regs *r
 
 #elif defined(CONFIG_8XX_MINIMAL_FPEMU)
 	errcode = Soft_emulate_8xx(regs);
+	if (errcode >= 0)
+		WARN_EMULATE(8xx);
+
 	switch (errcode) {
 	case 0:
 		emulate_single_step(regs);
--- /dev/null
+++ b/include/asm-powerpc/emulated_ops.h
@@ -0,0 +1,52 @@
+/*
+ *  Copyright 2007 Sony Corp.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef _ASM_POWERPC_EMULATED_OPS_H
+#define _ASM_POWERPC_EMULATED_OPS_H
+
+#include <linux/percpu.h>
+
+#include <asm/atomic.h>
+
+DECLARE_PER_CPU(atomic_long_t, emulated_dcba);
+DECLARE_PER_CPU(atomic_long_t, emulated_dcbz);
+DECLARE_PER_CPU(atomic_long_t, emulated_fp_pair);
+DECLARE_PER_CPU(atomic_long_t, emulated_mcrxr);
+DECLARE_PER_CPU(atomic_long_t, emulated_mfpvr);
+DECLARE_PER_CPU(atomic_long_t, emulated_multiple);
+DECLARE_PER_CPU(atomic_long_t, emulated_popcntb);
+DECLARE_PER_CPU(atomic_long_t, emulated_spe);
+DECLARE_PER_CPU(atomic_long_t, emulated_string);
+#ifdef CONFIG_MATH_EMULATION
+DECLARE_PER_CPU(atomic_long_t, emulated_math);
+#elif defined(CONFIG_8XX_MINIMAL_FPEMU)
+DECLARE_PER_CPU(atomic_long_t, emulated_8xx);
+#endif
+
+extern int sysctl_warn_emulated;
+extern void do_warn_emulate(const char *type);
+
+#define WARN_EMULATE(type)						\
+	do {								\
+		atomic_long_inc(&per_cpu(emulated_ ## type,		\
+					 raw_smp_processor_id()));	\
+		if (sysctl_warn_emulated)				\
+			do_warn_emulate(#type);				\
+	} while (0)
+
+
+#endif /* _ASM_POWERPC_EMULATED_OPS_H */

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* Re: pseries (power3) boot hang  (pageblock_nr_pages==0)
From: Mel Gorman @ 2007-11-28 10:52 UTC (permalink / raw)
  To: Will Schmidt; +Cc: linuxppc-dev, Stephen Rothwell, Linux Memory Management List
In-Reply-To: <1196105757.11297.11.camel@farscape.rchland.ibm.com>

On (26/11/07 13:35), Will Schmidt didst pronounce:
> 
> On Wed, 2007-11-21 at 22:03 +0000, Mel Gorman wrote:
> > On (21/11/07 15:55), Will Schmidt didst pronounce:
> > > Hi Folks, 
> > > 
> > > I imagine this would be properly fixed with something similar to the
> > > change for iSeries.  
> > 
> > Have you tried with the patch that fixed the iSeries boot problem?
> > Thanks for tracking down the problem to such a specific place.
> 
> I had not, but gave this patch a spin this morning, and it does the
> job.  :-)  

Brilliant.

> I was thinking (without really looking at it), that the
> iseries fix was in platform specific code.   Silly me. :-)
> 
> So for the record, this patch also fixes power3 pSeries systems.
> 
> fwiw:
> Tested-By:  Will Schmidt <will_schmidt@vnet.ibm.com>
> 

Thanks a lot for reporting and testing Will.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

^ permalink raw reply

* Re: [PATCH] PPC: CELLEB - fix potential NULL pointer dereference
From: Cyrill Gorcunov @ 2007-11-28 10:53 UTC (permalink / raw)
  To: michael; +Cc: Olof Johansson, PPCML, Paul Mackerras, LKML
In-Reply-To: <1196243333.2109.2.camel@concordia>

On 11/28/07, Michael Ellerman <michael@ellerman.id.au> wrote:
> On Mon, 2007-11-26 at 10:46 +0300, Cyrill Gorcunov wrote:
> > This patch adds checking for NULL value returned to prevent possible
> > NULL pointer dereference.
> > Also two unneeded 'return' are removed.
> >
> > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> > ---
> > Any comments are welcome.
>
> I guess it's good to be paranoid, but this is a little verbose:
>
>        wi0 = of_get_property(node, "device-id", NULL);
> +       if (unlikely((!wi0))) {
> +               printk(KERN_ERR "PCI: device-id not found.\n");
> +               goto error;
> +       }
>        wi1 = of_get_property(node, "vendor-id", NULL);
> +       if (unlikely((!wi1))) {
> +               printk(KERN_ERR "PCI: vendor-id not found.\n");
> +               goto error;
> +       }
>        wi2 = of_get_property(node, "class-code", NULL);
> +       if (unlikely((!wi2))) {
> +               printk(KERN_ERR "PCI: class-code not found.\n");
> +               goto error;
> +       }
>        wi3 = of_get_property(node, "revision-id", NULL);
> +       if (unlikely((!wi3))) {
> +               printk(KERN_ERR "PCI: revision-id not found.\n");
> +               goto error;
> +       }
>
> Perhaps instead:
>
>        wi0 = of_get_property(node, "device-id", NULL);
>        wi1 = of_get_property(node, "vendor-id", NULL);
>        wi2 = of_get_property(node, "class-code", NULL);
>        wi3 = of_get_property(node, "revision-id", NULL);
>
>       if (!wi0 || !wi1 || !wi2 || !wi3) {
>               printk(KERN_ERR "PCI: Missing device tree properties.\n");
>               goto error;
>       }

Hi Michael, yes that is much better (actually I was doubt about what form of
which the checking style to use - your form is much compact but mine does
show where *exactly* the problem appeared). So 'case that is the fake driver
your form is preferred ;) Ishizaki, could you use Michael's part then?

>
>
> cheers
>
> --
> Michael Ellerman
> OzLabs, IBM Australia Development Lab
>
> wwweb: http://michael.ellerman.id.au
> phone: +61 2 6212 1183 (tie line 70 21183)
>
> We do not inherit the earth from our ancestors,
> we borrow it from our children. - S.M.A.R.T Person
>
>

Cyrill

^ permalink raw reply

* Re: Unable to Read PPC440EPx Board ID thru Board Control and Status Registers (BCSR)
From: Stefan Roese @ 2007-11-28 10:51 UTC (permalink / raw)
  To: linuxppc-embedded
In-Reply-To: <639733.2036.qm@web45601.mail.sp1.yahoo.com>

On Wednesday 28 November 2007, Dell Query wrote:
> Oh is it 0x1C0002000?

Just to be sure here, we are talking about the AMCC Sequoia board, right?

> Where can I get the document? What I have is 0xC0002000 from
> ep440xc_um_amcc.pdf file that I get from the accompanying PPC440EPx
> resource CD.

Please take a look at the 440EPx data sheet. It has a nice table with an 
overview of the address maps (table 1). Here you will notice that the EBC has 
multiple maps, one starting at 0x1.c000.0000 and another one at 
0x1.f000.0000. Yes, these are 36bit physical addresses. In U-Boot these are 
mapped via the TLB to 0xc000.0000 and 0xf000.0000. So in U-Boot you are able 
to access the CPLD at 0xc0000000. But in Linux you have to map the 36bit 
address to get the virtual address which you need for accessing. And using 
arch/ppc you need to call ioremap64() with this 36bit address as parameter.

Hope this helps.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de
=====================================================================

^ permalink raw reply

* Re: [PATCH] PPC: CELLEB - fix potential NULL pointer dereference
From: Cyrill Gorcunov @ 2007-11-28 10:59 UTC (permalink / raw)
  To: michael; +Cc: Olof Johansson, PPCML, Paul Mackerras, LKML
In-Reply-To: <aa79d98a0711280253x3d1d28d6qa0cee0b731e807fe@mail.gmail.com>

On 11/28/07, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On 11/28/07, Michael Ellerman <michael@ellerman.id.au> wrote:
> > On Mon, 2007-11-26 at 10:46 +0300, Cyrill Gorcunov wrote:
> > > This patch adds checking for NULL value returned to prevent possible
> > > NULL pointer dereference.
> > > Also two unneeded 'return' are removed.
> > >
> > > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> > > ---
> > > Any comments are welcome.
> >
> > I guess it's good to be paranoid, but this is a little verbose:
> >
> >        wi0 = of_get_property(node, "device-id", NULL);
> > +       if (unlikely((!wi0))) {
> > +               printk(KERN_ERR "PCI: device-id not found.\n");
> > +               goto error;
> > +       }
> >        wi1 = of_get_property(node, "vendor-id", NULL);
> > +       if (unlikely((!wi1))) {
> > +               printk(KERN_ERR "PCI: vendor-id not found.\n");
> > +               goto error;
> > +       }
> >        wi2 = of_get_property(node, "class-code", NULL);
> > +       if (unlikely((!wi2))) {
> > +               printk(KERN_ERR "PCI: class-code not found.\n");
> > +               goto error;
> > +       }
> >        wi3 = of_get_property(node, "revision-id", NULL);
> > +       if (unlikely((!wi3))) {
> > +               printk(KERN_ERR "PCI: revision-id not found.\n");
> > +               goto error;
> > +       }
> >
> > Perhaps instead:
> >
> >        wi0 = of_get_property(node, "device-id", NULL);
> >        wi1 = of_get_property(node, "vendor-id", NULL);
> >        wi2 = of_get_property(node, "class-code", NULL);
> >        wi3 = of_get_property(node, "revision-id", NULL);
> >
> >       if (!wi0 || !wi1 || !wi2 || !wi3) {
> >               printk(KERN_ERR "PCI: Missing device tree properties.\n");
> >               goto error;
> >       }
>
> Hi Michael, yes that is much better (actually I was doubt about what form of
> which the checking style to use - your form is much compact but mine does
> show where *exactly* the problem appeared). So 'case that is the fake driver
> your form is preferred ;) Ishizaki, could you use Michael's part then?
>
> >
> >
> > cheers
> >
> > --
> > Michael Ellerman
> > OzLabs, IBM Australia Development Lab
> >
> > wwweb: http://michael.ellerman.id.au
> > phone: +61 2 6212 1183 (tie line 70 21183)
> >
> > We do not inherit the earth from our ancestors,
> > we borrow it from our children. - S.M.A.R.T Person
> >
> >
>
> Cyrill
>
Ishizaki I can update the patch if you needed. Should I?

Cyrill

^ permalink raw reply

* Re: [PATCH] powerpc: fix os-term usage on kernel panic
From: Olaf Hering @ 2007-11-28 11:00 UTC (permalink / raw)
  To: Will Schmidt; +Cc: linuxppc-dev, paulus
In-Reply-To: <1196208960.11297.26.camel@farscape.rchland.ibm.com>

On Tue, Nov 27, Will Schmidt wrote:

> > -void rtas_os_term(char *str)
> > +void rtas_panic_msg(char *str)

> > -	if (panic_timeout)
> > -		return;

This change is wrong. Booting with panic=123 really means the system
has to reboot in 123 seconds after a panic.
But, maybe this panic_timeout check was moved elsewhere.

Please revert this part of commit
a2b51812a4dc5db09ab4d4638d4d8ed456e2457e before 2.6.24 is released.

^ permalink raw reply

* Re: of_compat_cmp on various platforms
From: Stephen Rothwell @ 2007-11-28 11:45 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910711271946q1b600d97mf5abe71cc64bbf81@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

On Tue, 27 Nov 2007 22:46:16 -0500 "Jon Smirl" <jonsmirl@gmail.com> wrote:
>
> Is this right or should it be the same everywhere?
> 
> asm-powerpc/prom.h:#define of_compat_cmp(s1, s2, l)     strcasecmp((s1), (s2))
> asm-sparc/prom.h:#define of_compat_cmp(s1, s2, l)       strncmp((s1), (s2), (l))
> asm-sparc64/prom.h:#define of_compat_cmp(s1, s2, l)     strncmp((s1), (s2), (l))

The only reason these exist is because the sparc and powerpc versions
needed to be different when I was consolidating the of routines.  One of
the things we have to work on is finding the platforms/drivers where this
matters and fix them.

So, for now, these need to be there.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: PPC upstream kernel ignored DABR bug
From: Arnd Bergmann @ 2007-11-28 12:28 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: linuxppc-dev, Paul Mackerras, Roland McGrath
In-Reply-To: <20071128085907.GA19651@host0.dyn.jankratochvil.net>

On Wednesday 28 November 2007, Jan Kratochvil wrote:
> Please be aware DABR works fine if the same code runs just 1 (always) or
> 2 (sometimes) threads. =C2=A0It starts failing with too many threads runn=
ing:
>=20
> $ ./dabr-lost
> TID 32725: DABR 0x1001279f NIP 0xfecf41c
> TID 32726: DABR 0x1001279f NIP 0xfecf41c
> TID 32725: hitting the variable
> variable found =3D -1, caught TID =3D 32725
> TID 32726: hitting the variable
> variable found =3D -1, caught TID =3D 32726
> The kernel bug did not get reproduced - increase THREADS.
>=20
> As I did not find any code in that kernel touching DABRX its value should=
 not
> be dependent on the number of threads running.
>=20

Right, this is a different problem from the one reported by Uli.
=46rom what I can tell, your problem is that you set the DABR only
in one thread, so the other threads don't see it. DABR is saved
in the thread_struct, so setting it in one thread doesn't have
an impact on any other thread.

	Arnd <><

^ permalink raw reply

* Re: 2.6.24-rc3-mm2 - Build Failure on powerpc timerfd() undeclared
From: Kamalesh Babulal @ 2007-11-28 12:40 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linuxppc-dev, Balbir Singh, linux-kernel
In-Reply-To: <20071128034140.648383f0.akpm@linux-foundation.org>

Hi Andrew,

Kernel build fails, with build error

  CC      arch/powerpc/platforms/cell/spu_callbacks.o
In file included from arch/powerpc/platforms/cell/spu_callbacks.c:49:
include/asm/systbl.h:312: error: ‘sys_timerfd’ undeclared here (not in a function)
make[2]: *** [arch/powerpc/platforms/cell/spu_callbacks.o] Error 1
make[1]: *** [arch/powerpc/platforms/cell] Error 2
make: *** [arch/powerpc/platforms] Error 2

-- 
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.

^ permalink raw reply

* Re: PPC upstream kernel ignored DABR bug
From: Jan Kratochvil @ 2007-11-28 12:45 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Paul Mackerras, Roland McGrath
In-Reply-To: <200711281328.50386.arnd@arndb.de>

On Wed, 28 Nov 2007 13:28:48 +0100, Arnd Bergmann wrote:
> On Wednesday 28 November 2007, Jan Kratochvil wrote:
> > Please be aware DABR works fine if the same code runs just 1 (always) or
> > 2 (sometimes) threads.  It starts failing with too many threads running:
> > 
> > $ ./dabr-lost
> > TID 32725: DABR 0x1001279f NIP 0xfecf41c
> > TID 32726: DABR 0x1001279f NIP 0xfecf41c
> > TID 32725: hitting the variable
> > variable found = -1, caught TID = 32725
> > TID 32726: hitting the variable
> > variable found = -1, caught TID = 32726
> > The kernel bug did not get reproduced - increase THREADS.
> > 
> > As I did not find any code in that kernel touching DABRX its value should not
> > be dependent on the number of threads running.
> > 
> 
> Right, this is a different problem from the one reported by Uli.
> From what I can tell, your problem is that you set the DABR only
> in one thread, so the other threads don't see it. DABR is saved
> in the thread_struct, so setting it in one thread doesn't have
> an impact on any other thread.

It even prints out above:
	TID 32725: DABR 0x1001279f NIP 0xfecf41c
	TID 32726: DABR 0x1001279f NIP 0xfecf41c

that it wrote DABR in both the threads and it has also successfully read it
back from each thread specifically (according to its thread-specific TID).

for (threadi = 0; threadi < THREADS; threadi++)
    {
      pid_t tid = thread[threadi];

      setup (tid);
...
    }
static void setup (pid_t tid)
{
...
  l = ptrace (PTRACE_SET_DEBUGREG, tid, NULL, (void *) dabr);
...
}

Also if I would not set DABR specifically for each thread it would not work in
90% of cases for `THREADS == 2'.  And it would not work for `THREADS == 4' if
they are busylooping (therefore not in a syscall).
	TID 596: DABR 0x100127a7 NIP 0x10000dbc
	TID 597: DABR 0x100127a7 NIP 0x10000db0
	TID 598: DABR 0x100127a7 NIP 0x10000dac
	TID 599: DABR 0x100127a7 NIP 0x10000dbc
	TID 596: hitting the variable
	variable found = -1, caught TID = 596
	TID 599: hitting the variable
	variable found = -1, caught TID = 599
	TID 597: hitting the variable
	variable found = -1, caught TID = 597
	TID 598: hitting the variable
	variable found = -1, caught TID = 598
	The kernel bug got workarounded by WORKAROUND_SET_DABR_IN_SYSCALL.

(I found out now WORKAROUND_SET_DABR_IN_SYSCALL only reduces the probability of
the failure, it is not a 100% workaround of the problem in the testcase.)


There is some tricky kernel code around it but I did not try to debug it:

struct task_struct *__switch_to(struct task_struct *prev,
	struct task_struct *new)
{
...
	if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr)) {
		set_dabr(new->thread.dabr);
		__get_cpu_var(current_dabr) = new->thread.dabr;
	}
...
}



Regards,
Jan

^ permalink raw reply

* Re: [PATCH 0/3] OF-platform PATA driver
From: Anton Vorontsov @ 2007-11-28 13:24 UTC (permalink / raw)
  To: Paul Mundt; +Cc: Arnd Bergmann, Jeff Garzik, linux-ide, linuxppc-dev
In-Reply-To: <20071128095151.GA29107@linux-sh.org>

On Wed, Nov 28, 2007 at 06:51:51PM +0900, Paul Mundt wrote:
> On Tue, Nov 27, 2007 at 06:37:08PM +0300, Anton Vorontsov wrote:
> > Here is the second spin of the OF-platform PATA driver and
> > related patches.
> > 
> So either the patches are missing, or I wasn't CC'ed on them. I'm going
> to go out on a limb and assume the latter. If you wish me to Ack them,
> I'm not going to start grovelling around list archives trying to find the
> relevant posts.

Point taken, fair enough. I'll Cc you explicitly next time.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: NULL dereference in clockevents_program_event
From: Johannes Berg @ 2007-11-27 23:52 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linuxppc-dev list, Ingo Molnar, Paul Mackerras, Linux Kernel list
In-Reply-To: <1195502804.19479.6.camel@johannes.berg>

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]


> During a hibernate cycle on my G5, while machine was powering down after
> saving the image, I just had a NULL dereference  in
> clockevents_program_event when accessing dev->mode, dev was NULL.
> 
> Unfortunately the machine rebooted before I was able to write down more
> than the fact that it was called from tick_program_event(); the problem
> doesn't seem to be easily reproducible.
> 
> From what I can see when doing the same thing, the shutdown attempts to
> offline all CPUs. Because the snapshot was actually saved to disk and
> the machine was shutting down I guess that the it happened at that time,
> but I have no idea what else to do to debug this.
> 
> I have
> 
> | CONFIG_HIGH_RES_TIMERS=y
> | CONFIG_NO_HZ=y
> 
> in this config.

I just got the same or a similar thing again and was able to write down
more of the stack dump:

NIP clock_events_program_event+0x20
LR  tick_program_event+0x64
Stack:
hr_timer_interrupt+0x211
timer_interrupt+0xc4
decrementer_common+0x110

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: 2.6.24-rc3-mm2 - Build Failure on powerpc timerfd() undeclared
From: Arnd Bergmann @ 2007-11-28 13:32 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Andrew Morton, Balbir Singh, Paul Mackerras, linux-kernel,
	Kamalesh Babulal
In-Reply-To: <474D61CF.6080400@linux.vnet.ibm.com>

On Wednesday 28 November 2007, Kamalesh Babulal wrote:
> Kernel build fails, with build error
>=20
> =C2=A0 CC =C2=A0 =C2=A0 =C2=A0arch/powerpc/platforms/cell/spu_callbacks.o
> In file included from arch/powerpc/platforms/cell/spu_callbacks.c:49:
> include/asm/systbl.h:312: error: =E2=80=98sys_timerfd=E2=80=99 undeclared=
 here (not in a function)
> make[2]: *** [arch/powerpc/platforms/cell/spu_callbacks.o] Error 1
> make[1]: *** [arch/powerpc/platforms/cell] Error 2
> make: *** [arch/powerpc/platforms] Error 2
>=20

I guess all architectures except x86 are currently broken because they
reference the old sys_timerfd function. This patch should add the missing
bits to powerpc.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

=2D--

Disclaimer: Not tested at all, just applied common sense.
Disclaimer2: conflicts with the sys_indirect kernel implementation
sent by paulus last week.

diff --git a/include/asm-powerpc/systbl.h b/include/asm-powerpc/systbl.h
index 11d5383..b029368 100644
=2D-- a/include/asm-powerpc/systbl.h
+++ b/include/asm-powerpc/systbl.h
@@ -309,7 +309,9 @@ SYSCALL_SPU(getcpu)
 COMPAT_SYS(epoll_pwait)
 COMPAT_SYS_SPU(utimensat)
 COMPAT_SYS_SPU(signalfd)
=2DCOMPAT_SYS_SPU(timerfd)
+COMPAT_SYS_SPU(timerfd_create)
 SYSCALL_SPU(eventfd)
 COMPAT_SYS_SPU(sync_file_range2)
 COMPAT_SYS(fallocate)
+COMPAT_SYS_SPU(sys_timerfd_settime)
+COMPAT_SYS_SPU(sys_timerfd_gettime)
diff --git a/include/asm-powerpc/unistd.h b/include/asm-powerpc/unistd.h
index 97d82b6..4ba2d20 100644
=2D-- a/include/asm-powerpc/unistd.h
+++ b/include/asm-powerpc/unistd.h
@@ -328,14 +328,16 @@
 #define __NR_epoll_pwait	303
 #define __NR_utimensat		304
 #define __NR_signalfd		305
=2D#define __NR_timerfd		306
+#define __NR_timerfd_create	306
 #define __NR_eventfd		307
 #define __NR_sync_file_range2	308
 #define __NR_fallocate		309
+#define __NR_sys_timerfd_settime 310
+#define __NR_sys_timerfd_gettime 311
=20
 #ifdef __KERNEL__
=20
=2D#define __NR_syscalls		310
+#define __NR_syscalls		312
=20
 #define __NR__exit __NR_exit
 #define NR_syscalls	__NR_syscalls

^ permalink raw reply

* [PATCH] IB/ehca: Fix static rate if path faster than link
From: Joachim Fenkes @ 2007-11-28 13:46 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG
  Cc: Stefan Roscher, Christoph Raisch, Marcus Eder

The formula would yield -1 for this, which is wrong in a bad way (max
throttling). Clamp to 0, which is the correct value.

Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
---

This fixes another regression introduced in rc3.
Please review and apply for 2.6.24-rc4. Thanks!

 drivers/infiniband/hw/ehca/ehca_av.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_av.c b/drivers/infiniband/hw/ehca/ehca_av.c
index 453eb99..f7782c8 100644
--- a/drivers/infiniband/hw/ehca/ehca_av.c
+++ b/drivers/infiniband/hw/ehca/ehca_av.c
@@ -76,8 +76,12 @@ int ehca_calc_ipd(struct ehca_shca *shca, int port,
 
 	link = ib_width_enum_to_int(pa.active_width) * pa.active_speed;
 
-	/* IPD = round((link / path) - 1) */
-	*ipd = ((link + (path >> 1)) / path) - 1;
+	if (path >= link)
+		/* no need to throttle if path faster than link */
+		*ipd = 0;
+	else
+		/* IPD = round((link / path) - 1) */
+		*ipd = ((link + (path >> 1)) / path) - 1;
 
 	return 0;
 }
-- 
1.5.2

^ permalink raw reply related

* [BUG] 2.6.24-rc3-mm2 soft lockup while running tbench
From: Kamalesh Babulal @ 2007-11-28 14:33 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linuxppc-dev, Balbir Singh, linux-kernel
In-Reply-To: <20071128034140.648383f0.akpm@linux-foundation.org>

Hi Andrew,

while running tbench on the powerpc with 2.6.24-rc3-mm2 softlock up occurs

BUG: soft lockup - CPU#0 stuck for 11s! [tbench:12183]
NIP: c0000000000ac978 LR: c0000000000acff0 CTR: c00000000005c648
REGS: C00000076F0F3200 TRAP: 0901   Not tainted  (2.6.24-rc3-mm2-autotest)
MSR: 8000000000009032 <EE,ME,IR,DR>  CR: 44000482  XER: 00000000
TASK = C00000076F4BC000[12183] 'tbench' THREAD: C00000076F0F0000 CPU: 0
NIP [c0000000000ac978] .get_page_from_freelist+0x1cc/0x754
LR [c0000000000acff0] .__alloc_pages+0xb0/0x3a8
Call Trace:
[c00000076f0f3480] [c00000076f0f3560] 0xc00000076f0f3560 (unreliable)
[c00000076f0f3590] [c0000000000acff0] .__alloc_pages+0xb0/0x3a8
[c00000076f0f3680] [c0000000000ce2e4] .alloc_pages_current+0xa8/0xc8
[c00000076f0f3710] [c0000000000ac6ec] .__get_free_pages+0x20/0x70
[c00000076f0f3790] [c0000000000d75c8] .__kmalloc_node_track_caller+0x60/0x148
[c00000076f0f3840] [c0000000002c22b0] .__alloc_skb+0x98/0x184
[c00000076f0f38f0] [c000000000306cd8] .tcp_sendmsg+0x1fc/0xe24
[c00000076f0f3a10] [c0000000002b963c] .sock_sendmsg+0xe4/0x128
[c00000076f0f3c10] [c0000000002ba4ec] .sys_sendto+0xd4/0x120
[c00000076f0f3d90] [c0000000002df2f8] .compat_sys_socketcall+0x148/0x214
[c00000076f0f3e30] [c00000000000872c] syscall_exit+0x0/0x40
Instruction dump:
720b0001 eb970000 40820070 72000002 4182000c e8bc0000 48000018 72080004 
4182000c e8bc0008 48000008 e8bc0010 <e8c10078> 7f83e378 7de407b4 7e078378 

-- 
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.

^ permalink raw reply

* [patch 1/7] ps3: Make bus_id and dev_id u64
From: Geert Uytterhoeven @ 2007-11-28 14:31 UTC (permalink / raw)
  To: Linux/PPC Development; +Cc: Geert Uytterhoeven, Linux kernel mailing list
In-Reply-To: <20071128143142.580732000@pademelon.sonytel.be>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 9746 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

ps3: Make bus_id and dev_id u64.

These IDs are 64-bit in the repository, and the special storage notification
device has a device ID of ULONG_MAX.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 arch/powerpc/platforms/ps3/device-init.c |    4 ++--
 arch/powerpc/platforms/ps3/mm.c          |    8 ++++----
 arch/powerpc/platforms/ps3/platform.h    |   12 ++++++------
 arch/powerpc/platforms/ps3/repository.c  |   21 ++++++++-------------
 arch/powerpc/platforms/ps3/system-bus.c  |   14 +++++++-------
 drivers/net/ps3_gelic_net.c              |    4 ++--
 include/asm-powerpc/ps3.h                |    4 ++--
 7 files changed, 31 insertions(+), 36 deletions(-)

--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -297,7 +297,7 @@ static int ps3_storage_wait_for_device(c
 		u64 dev_port;
 	} *notify_event;
 
-	pr_debug(" -> %s:%u: (%u:%u:%u)\n", __func__, __LINE__, repo->bus_id,
+	pr_debug(" -> %s:%u: (%lu:%lu:%u)\n", __func__, __LINE__, repo->bus_id,
 		 repo->dev_id, repo->dev_type);
 
 	buf = kzalloc(512, GFP_KERNEL);
@@ -384,7 +384,7 @@ static int ps3_storage_wait_for_device(c
 
 		if (notify_event->dev_id == repo->dev_id &&
 		    notify_event->dev_type == PS3_DEV_TYPE_NOACCESS) {
-			pr_debug("%s:%u: no access: dev_id %u\n", __func__,
+			pr_debug("%s:%u: no access: dev_id %lu\n", __func__,
 				 __LINE__, repo->dev_id);
 			break;
 		}
--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -359,7 +359,7 @@ static unsigned long dma_sb_lpar_to_bus(
 static void  __maybe_unused _dma_dump_region(const struct ps3_dma_region *r,
 	const char *func, int line)
 {
-	DBG("%s:%d: dev        %u:%u\n", func, line, r->dev->bus_id,
+	DBG("%s:%d: dev        %lu:%lu\n", func, line, r->dev->bus_id,
 		r->dev->dev_id);
 	DBG("%s:%d: page_size  %u\n", func, line, r->page_size);
 	DBG("%s:%d: bus_addr   %lxh\n", func, line, r->bus_addr);
@@ -394,7 +394,7 @@ struct dma_chunk {
 static void _dma_dump_chunk (const struct dma_chunk* c, const char* func,
 	int line)
 {
-	DBG("%s:%d: r.dev        %u:%u\n", func, line,
+	DBG("%s:%d: r.dev        %lu:%lu\n", func, line,
 		c->region->dev->bus_id, c->region->dev->dev_id);
 	DBG("%s:%d: r.bus_addr   %lxh\n", func, line, c->region->bus_addr);
 	DBG("%s:%d: r.page_size  %u\n", func, line, c->region->page_size);
@@ -658,7 +658,7 @@ static int dma_sb_region_create(struct p
 	BUG_ON(!r);
 
 	if (!r->dev->bus_id) {
-		pr_info("%s:%d: %u:%u no dma\n", __func__, __LINE__,
+		pr_info("%s:%d: %lu:%lu no dma\n", __func__, __LINE__,
 			r->dev->bus_id, r->dev->dev_id);
 		return 0;
 	}
@@ -724,7 +724,7 @@ static int dma_sb_region_free(struct ps3
 	BUG_ON(!r);
 
 	if (!r->dev->bus_id) {
-		pr_info("%s:%d: %u:%u no dma\n", __func__, __LINE__,
+		pr_info("%s:%d: %lu:%lu no dma\n", __func__, __LINE__,
 			r->dev->bus_id, r->dev->dev_id);
 		return 0;
 	}
--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -95,7 +95,7 @@ enum ps3_dev_type {
 
 int ps3_repository_read_bus_str(unsigned int bus_index, const char *bus_str,
 	u64 *value);
-int ps3_repository_read_bus_id(unsigned int bus_index, unsigned int *bus_id);
+int ps3_repository_read_bus_id(unsigned int bus_index, u64 *bus_id);
 int ps3_repository_read_bus_type(unsigned int bus_index,
 	enum ps3_bus_type *bus_type);
 int ps3_repository_read_bus_num_dev(unsigned int bus_index,
@@ -119,7 +119,7 @@ enum ps3_reg_type {
 int ps3_repository_read_dev_str(unsigned int bus_index,
 	unsigned int dev_index, const char *dev_str, u64 *value);
 int ps3_repository_read_dev_id(unsigned int bus_index, unsigned int dev_index,
-	unsigned int *dev_id);
+	u64 *dev_id);
 int ps3_repository_read_dev_type(unsigned int bus_index,
 	unsigned int dev_index, enum ps3_dev_type *dev_type);
 int ps3_repository_read_dev_intr(unsigned int bus_index,
@@ -138,12 +138,12 @@ int ps3_repository_read_dev_reg(unsigned
 /* repository bus enumerators */
 
 struct ps3_repository_device {
-	enum ps3_bus_type bus_type;
 	unsigned int bus_index;
-	unsigned int bus_id;
-	enum ps3_dev_type dev_type;
 	unsigned int dev_index;
-	unsigned int dev_id;
+	enum ps3_bus_type bus_type;
+	enum ps3_dev_type dev_type;
+	u64 bus_id;
+	u64 dev_id;
 };
 
 static inline struct ps3_repository_device *ps3_repository_bump_device(
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -168,18 +168,15 @@ int ps3_repository_read_bus_str(unsigned
 		value, 0);
 }
 
-int ps3_repository_read_bus_id(unsigned int bus_index, unsigned int *bus_id)
+int ps3_repository_read_bus_id(unsigned int bus_index, u64 *bus_id)
 {
 	int result;
-	u64 v1;
-	u64 v2; /* unused */
 
 	result = read_node(PS3_LPAR_ID_PME,
 		make_first_field("bus", bus_index),
 		make_field("id", 0),
 		0, 0,
-		&v1, &v2);
-	*bus_id = v1;
+		bus_id, NULL);
 	return result;
 }
 
@@ -225,18 +222,16 @@ int ps3_repository_read_dev_str(unsigned
 }
 
 int ps3_repository_read_dev_id(unsigned int bus_index, unsigned int dev_index,
-	unsigned int *dev_id)
+	u64 *dev_id)
 {
 	int result;
-	u64 v1;
 
 	result = read_node(PS3_LPAR_ID_PME,
 		make_first_field("bus", bus_index),
 		make_field("dev", dev_index),
 		make_field("id", 0),
 		0,
-		&v1, 0);
-	*dev_id = v1;
+		dev_id, 0);
 	return result;
 }
 
@@ -332,7 +327,7 @@ int ps3_repository_find_device(struct ps
 		return result;
 	}
 
-	pr_debug("%s:%d: bus_type %u, bus_index %u, bus_id %u, num_dev %u\n",
+	pr_debug("%s:%d: bus_type %u, bus_index %u, bus_id %lu, num_dev %u\n",
 		__func__, __LINE__, tmp.bus_type, tmp.bus_index, tmp.bus_id,
 		num_dev);
 
@@ -387,7 +382,7 @@ int ps3_repository_find_device(struct ps
 		return result;
 	}
 
-	pr_debug("%s:%d: found: dev_type %u, dev_index %u, dev_id %u\n",
+	pr_debug("%s:%d: found: dev_type %u, dev_index %u, dev_id %lu\n",
 		__func__, __LINE__, tmp.dev_type, tmp.dev_index, tmp.dev_id);
 
 	*repo = tmp;
@@ -1034,7 +1029,7 @@ static int dump_device_info(struct ps3_r
 			continue;
 		}
 
-		pr_debug("%s:%d  (%u:%u): dev_type %u, dev_id %u\n", __func__,
+		pr_debug("%s:%d  (%u:%u): dev_type %u, dev_id %lu\n", __func__,
 			__LINE__, repo->bus_index, repo->dev_index,
 			repo->dev_type, repo->dev_id);
 
@@ -1091,7 +1086,7 @@ int ps3_repository_dump_bus_info(void)
 			continue;
 		}
 
-		pr_debug("%s:%d bus_%u: bus_type %u, bus_id %u, num_dev %u\n",
+		pr_debug("%s:%d bus_%u: bus_type %u, bus_id %lu, num_dev %u\n",
 			__func__, __LINE__, repo.bus_index, repo.bus_type,
 			repo.bus_id, num_dev);
 
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -42,8 +42,8 @@ struct {
 	int gpu;
 } static usage_hack;
 
-static int ps3_is_device(struct ps3_system_bus_device *dev,
-			 unsigned int bus_id, unsigned int dev_id)
+static int ps3_is_device(struct ps3_system_bus_device *dev, u64 bus_id,
+			 u64 dev_id)
 {
 	return dev->bus_id == bus_id && dev->dev_id == dev_id;
 }
@@ -182,8 +182,8 @@ int ps3_open_hv_device(struct ps3_system
 	case PS3_MATCH_ID_SYSTEM_MANAGER:
 		pr_debug("%s:%d: unsupported match_id: %u\n", __func__,
 			__LINE__, dev->match_id);
-		pr_debug("%s:%d: bus_id: %u\n", __func__,
-			__LINE__, dev->bus_id);
+		pr_debug("%s:%d: bus_id: %lu\n", __func__, __LINE__,
+			dev->bus_id);
 		BUG();
 		return -EINVAL;
 
@@ -220,8 +220,8 @@ int ps3_close_hv_device(struct ps3_syste
 	case PS3_MATCH_ID_SYSTEM_MANAGER:
 		pr_debug("%s:%d: unsupported match_id: %u\n", __func__,
 			__LINE__, dev->match_id);
-		pr_debug("%s:%d: bus_id: %u\n", __func__,
-			__LINE__, dev->bus_id);
+		pr_debug("%s:%d: bus_id: %lu\n", __func__, __LINE__,
+			dev->bus_id);
 		BUG();
 		return -EINVAL;
 
@@ -240,7 +240,7 @@ EXPORT_SYMBOL_GPL(ps3_close_hv_device);
 static void _dump_mmio_region(const struct ps3_mmio_region* r,
 	const char* func, int line)
 {
-	pr_debug("%s:%d: dev       %u:%u\n", func, line, r->dev->bus_id,
+	pr_debug("%s:%d: dev       %lu:%lu\n", func, line, r->dev->bus_id,
 		r->dev->dev_id);
 	pr_debug("%s:%d: bus_addr  %lxh\n", func, line, r->bus_addr);
 	pr_debug("%s:%d: len       %lxh\n", func, line, r->len);
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -58,11 +58,11 @@ static inline struct device *ctodev(stru
 {
 	return &card->dev->core;
 }
-static inline unsigned int bus_id(struct gelic_net_card *card)
+static inline u64 bus_id(struct gelic_net_card *card)
 {
 	return card->dev->bus_id;
 }
-static inline unsigned int dev_id(struct gelic_net_card *card)
+static inline u64 dev_id(struct gelic_net_card *card)
 {
 	return card->dev->dev_id;
 }
--- a/include/asm-powerpc/ps3.h
+++ b/include/asm-powerpc/ps3.h
@@ -344,8 +344,8 @@ struct ps3_system_bus_device {
 	enum ps3_match_id match_id;
 	enum ps3_system_bus_device_type dev_type;
 
-	unsigned int bus_id;              /* SB */
-	unsigned int dev_id;              /* SB */
+	u64 bus_id;                       /* SB */
+	u64 dev_id;                       /* SB */
 	unsigned int interrupt_id;        /* SB */
 	struct ps3_dma_region *d_region;  /* SB, IOC0 */
 	struct ps3_mmio_region *m_region; /* SB, IOC0*/

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 0/7] PS3 notification device patches for 2.6.25
From: Geert Uytterhoeven @ 2007-11-28 14:31 UTC (permalink / raw)
  To: Linux/PPC Development; +Cc: Linux kernel mailing list

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

	Hi all,

Here are my PS3 notification device patches for 2.6.25:
  [1] ps3: Make bus_id and dev_id u64
  [2] ps3: Add ps3_repository_find_device_by_id()
  [3] ps3: Use the HV's storage device notification mechanism properly
  [4] ps3: Add repository polling loop to work around hypervisor bug
  [5] ps3: Kill unused ps3_repository_bump_device()
  [6] ps3: Refactor ps3_repository_find_device()
  [7] ps3: denoise arch/powerpc/platforms/ps3/repository.c

These patches correct the use of the PS3 notification device mechanism, which
allows to wait until a storage device is ready.

Please review, thanks!

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 2/7] ps3: Add ps3_repository_find_device_by_id()
From: Geert Uytterhoeven @ 2007-11-28 14:31 UTC (permalink / raw)
  To: Linux/PPC Development; +Cc: Geert Uytterhoeven, Linux kernel mailing list
In-Reply-To: <20071128143142.580732000@pademelon.sonytel.be>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3948 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

ps3: Add ps3_repository_find_device_by_id()

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 arch/powerpc/platforms/ps3/platform.h   |    2 
 arch/powerpc/platforms/ps3/repository.c |   77 ++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -153,6 +153,8 @@ static inline struct ps3_repository_devi
 	return repo;
 }
 int ps3_repository_find_device(struct ps3_repository_device *repo);
+int ps3_repository_find_device_by_id(struct ps3_repository_device *repo,
+				     u64 bus_id, u64 dev_id);
 int ps3_repository_find_devices(enum ps3_bus_type bus_type,
 	int (*callback)(const struct ps3_repository_device *repo));
 int ps3_repository_find_bus(enum ps3_bus_type bus_type, unsigned int from,
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -389,6 +389,83 @@ int ps3_repository_find_device(struct ps
 	return 0;
 }
 
+int ps3_repository_find_device_by_id(struct ps3_repository_device *repo,
+				     u64 bus_id, u64 dev_id)
+{
+	int result = -ENODEV;
+	struct ps3_repository_device tmp;
+	unsigned int num_dev;
+
+	pr_debug(" -> %s:%u: find device by id %lu:%lu\n", __func__, __LINE__,
+		 bus_id, dev_id);
+
+	for (tmp.bus_index = 0; tmp.bus_index < 10; tmp.bus_index++) {
+		result = ps3_repository_read_bus_id(tmp.bus_index,
+						    &tmp.bus_id);
+		if (result) {
+			pr_debug("%s:%u read_bus_id(%u) failed\n", __func__,
+				 __LINE__, tmp.bus_index);
+			return result;
+		}
+
+		if (tmp.bus_id == bus_id)
+			goto found_bus;
+
+		pr_debug("%s:%u: skip, bus_id %lu\n", __func__, __LINE__,
+			 tmp.bus_id);
+	}
+	pr_debug(" <- %s:%u: bus not found\n", __func__, __LINE__);
+	return result;
+
+found_bus:
+	result = ps3_repository_read_bus_type(tmp.bus_index, &tmp.bus_type);
+	if (result) {
+		pr_debug("%s:%u read_bus_type(%u) failed\n", __func__,
+			 __LINE__, tmp.bus_index);
+		return result;
+	}
+
+	result = ps3_repository_read_bus_num_dev(tmp.bus_index, &num_dev);
+	if (result) {
+		pr_debug("%s:%u read_bus_num_dev failed\n", __func__,
+			 __LINE__);
+		return result;
+	}
+
+	for (tmp.dev_index = 0; tmp.dev_index < num_dev; tmp.dev_index++) {
+		result = ps3_repository_read_dev_id(tmp.bus_index,
+						    tmp.dev_index,
+						    &tmp.dev_id);
+		if (result) {
+			pr_debug("%s:%u read_dev_id(%u:%u) failed\n", __func__,
+				 __LINE__, tmp.bus_index, tmp.dev_index);
+			return result;
+		}
+
+		if (tmp.dev_id == dev_id)
+			goto found_dev;
+
+		pr_debug("%s:%u: skip, dev_id %lu\n", __func__, __LINE__,
+			 tmp.dev_id);
+	}
+	pr_debug(" <- %s:%u: dev not found\n", __func__, __LINE__);
+	return result;
+
+found_dev:
+	result = ps3_repository_read_dev_type(tmp.bus_index, tmp.dev_index,
+					      &tmp.dev_type);
+	if (result) {
+		pr_debug("%s:%u read_dev_type failed\n", __func__, __LINE__);
+		return result;
+	}
+
+	pr_debug(" <- %s:%u: found: type (%u:%u) index (%u:%u) id (%lu:%lu)\n",
+		 __func__, __LINE__, tmp.bus_type, tmp.dev_type, tmp.bus_index,
+		 tmp.dev_index, tmp.bus_id, tmp.dev_id);
+	*repo = tmp;
+	return 0;
+}
+
 int __devinit ps3_repository_find_devices(enum ps3_bus_type bus_type,
 	int (*callback)(const struct ps3_repository_device *repo))
 {

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 4/7] ps3: Add repository polling loop to work around hypervisor bug
From: Geert Uytterhoeven @ 2007-11-28 14:31 UTC (permalink / raw)
  To: Linux/PPC Development; +Cc: Geert Uytterhoeven, Linux kernel mailing list
In-Reply-To: <20071128143142.580732000@pademelon.sonytel.be>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2998 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

ps3: Add repository polling loop to work around hypervisor bug

On some firmware versions (e.g. 1.90), the storage device may not show up
in the repository immediately after receiving the notification message.
Add a small polling loop to make sure we don't miss it.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 arch/powerpc/platforms/ps3/device-init.c |   47 ++++++++++++++++++++++---------
 1 files changed, 34 insertions(+), 13 deletions(-)

--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -581,6 +581,39 @@ static int ps3_notification_read_write(s
 	return 0;
 }
 
+static void ps3_find_and_add_device(u64 bus_id, u64 dev_id)
+{
+	struct ps3_repository_device repo;
+	int res;
+	unsigned int retries;
+	unsigned long rem;
+
+	/*
+	 * On some firmware versions (e.g. 1.90), the device may not show up
+	 * in the repository immediately
+	 */
+	for (retries = 0; retries < 10; retries++) {
+		res = ps3_repository_find_device_by_id(&repo, bus_id, dev_id);
+		if (!res)
+			goto found;
+
+		rem = msleep_interruptible(100);
+		if (rem)
+			break;
+	}
+	pr_warning("%s:%u: device %lu:%lu not found\n", __func__, __LINE__,
+		   bus_id, dev_id);
+	return;
+
+found:
+	if (retries)
+		pr_debug("%s:%u: device %lu:%lu found after %u retries\n",
+			 __func__, __LINE__, bus_id, dev_id, retries);
+
+	ps3_register_repository_device(&repo);
+	return;
+}
+
 
 /**
  * ps3_probe_thread - Background repository probing at system startup.
@@ -595,7 +628,6 @@ static int ps3_notification_read_write(s
 static int ps3_probe_thread(void *data)
 {
 	struct ps3_notification_device dev;
-	struct ps3_repository_device repo;
 	int res;
 	unsigned int irq;
 	u64 lpar;
@@ -677,18 +709,7 @@ static int ps3_probe_thread(void *data)
 			continue;
 		}
 
-		res = ps3_repository_find_device_by_id(&repo, dev.sbd.bus_id,
-						       notify_event->dev_id);
-		if (res) {
-			pr_warning("%s:%u: device %lu:%lu not found\n",
-				   __func__, __LINE__, dev.sbd.bus_id,
-				   notify_event->dev_id);
-			continue;
-		}
-
-		pr_debug("%s:%u: device %lu:%lu found\n", __func__, __LINE__,
-			 dev.sbd.bus_id, notify_event->dev_id);
-		ps3_register_repository_device(&repo);
+		ps3_find_and_add_device(dev.sbd.bus_id, notify_event->dev_id);
 
 	} while (!kthread_should_stop());
 

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply

* [patch 3/7] ps3: Use the HVs storage device notification mechanism properly
From: Geert Uytterhoeven @ 2007-11-28 14:31 UTC (permalink / raw)
  To: Linux/PPC Development; +Cc: Geert Uytterhoeven, Linux kernel mailing list
In-Reply-To: <20071128143142.580732000@pademelon.sonytel.be>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 15780 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

ps3: Use the HV's storage device notification mechanism properly

The hypervisor has a storage device notification mechanism to wait until a
storage device is ready. Unfortunately the storage device probing code used
this mechanism in an incorrect way, needing a polling loop and handling of
devices that are not yet ready.

This change corrects this by:
  - First waiting for the reception of an asynchronous notification that a new
    storage device became ready,
  - Then looking up the storage device in the device repository.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
 arch/powerpc/platforms/ps3/device-init.c |  395 +++++++++++++++----------------
 arch/powerpc/platforms/ps3/platform.h    |    2 
 arch/powerpc/platforms/ps3/repository.c  |   29 --
 3 files changed, 194 insertions(+), 232 deletions(-)

--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -238,166 +238,6 @@ static int __init ps3_setup_vuart_device
 	return result;
 }
 
-static int ps3stor_wait_for_completion(u64 dev_id, u64 tag,
-				       unsigned int timeout)
-{
-	int result = -1;
-	unsigned int retries = 0;
-	u64 status;
-
-	for (retries = 0; retries < timeout; retries++) {
-		result = lv1_storage_check_async_status(dev_id, tag, &status);
-		if (!result)
-			break;
-
-		msleep(1);
-	}
-
-	if (result)
-		pr_debug("%s:%u: check_async_status: %s, status %lx\n",
-			 __func__, __LINE__, ps3_result(result), status);
-
-	return result;
-}
-
-/**
- * ps3_storage_wait_for_device - Wait for a storage device to become ready.
- * @repo: The repository device to wait for.
- *
- * Uses the hypervisor's storage device notification mechanism to wait until
- * a storage device is ready.  The device notification mechanism uses a
- * psuedo device (id = -1) to asynchronously notify the guest when storage
- * devices become ready.  The notification device has a block size of 512
- * bytes.
- */
-
-static int ps3_storage_wait_for_device(const struct ps3_repository_device *repo)
-{
-	int error = -ENODEV;
-	int result;
-	const u64 notification_dev_id = (u64)-1LL;
-	const unsigned int timeout = HZ;
-	u64 lpar;
-	u64 tag;
-	void *buf;
-	enum ps3_notify_type {
-		notify_device_ready = 0,
-		notify_region_probe = 1,
-		notify_region_update = 2,
-	};
-	struct {
-		u64 operation_code;	/* must be zero */
-		u64 event_mask;		/* OR of 1UL << enum ps3_notify_type */
-	} *notify_cmd;
-	struct {
-		u64 event_type;		/* enum ps3_notify_type */
-		u64 bus_id;
-		u64 dev_id;
-		u64 dev_type;
-		u64 dev_port;
-	} *notify_event;
-
-	pr_debug(" -> %s:%u: (%lu:%lu:%u)\n", __func__, __LINE__, repo->bus_id,
-		 repo->dev_id, repo->dev_type);
-
-	buf = kzalloc(512, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	lpar = ps3_mm_phys_to_lpar(__pa(buf));
-	notify_cmd = buf;
-	notify_event = buf;
-
-	result = lv1_open_device(repo->bus_id, notification_dev_id, 0);
-	if (result) {
-		printk(KERN_ERR "%s:%u: lv1_open_device %s\n", __func__,
-		       __LINE__, ps3_result(result));
-		goto fail_free;
-	}
-
-	/* Setup and write the request for device notification. */
-
-	notify_cmd->operation_code = 0; /* must be zero */
-	notify_cmd->event_mask = 1UL << notify_region_probe;
-
-	result = lv1_storage_write(notification_dev_id, 0, 0, 1, 0, lpar,
-				   &tag);
-	if (result) {
-		printk(KERN_ERR "%s:%u: write failed %s\n", __func__, __LINE__,
-		       ps3_result(result));
-		goto fail_close;
-	}
-
-	/* Wait for the write completion */
-
-	result = ps3stor_wait_for_completion(notification_dev_id, tag,
-					     timeout);
-	if (result) {
-		printk(KERN_ERR "%s:%u: write not completed %s\n", __func__,
-		       __LINE__, ps3_result(result));
-		goto fail_close;
-	}
-
-	/* Loop here processing the requested notification events. */
-
-	while (1) {
-		memset(notify_event, 0, sizeof(*notify_event));
-
-		result = lv1_storage_read(notification_dev_id, 0, 0, 1, 0,
-					  lpar, &tag);
-		if (result) {
-			printk(KERN_ERR "%s:%u: write failed %s\n", __func__,
-			       __LINE__, ps3_result(result));
-			break;
-		}
-
-		result = ps3stor_wait_for_completion(notification_dev_id, tag,
-						     timeout);
-		if (result) {
-			printk(KERN_ERR "%s:%u: read not completed %s\n",
-			       __func__, __LINE__, ps3_result(result));
-			break;
-		}
-
-		pr_debug("%s:%d: notify event (%u:%u:%u): event_type 0x%lx, "
-			 "port %lu\n", __func__, __LINE__, repo->bus_index,
-			 repo->dev_index, repo->dev_type,
-			 notify_event->event_type, notify_event->dev_port);
-
-		if (notify_event->event_type != notify_region_probe ||
-		    notify_event->bus_id != repo->bus_id) {
-			pr_debug("%s:%u: bad notify_event: event %lu, "
-				 "dev_id %lu, dev_type %lu\n",
-				 __func__, __LINE__, notify_event->event_type,
-				 notify_event->dev_id, notify_event->dev_type);
-			break;
-		}
-
-		if (notify_event->dev_id == repo->dev_id &&
-		    notify_event->dev_type == repo->dev_type) {
-			pr_debug("%s:%u: device ready (%u:%u:%u)\n", __func__,
-				 __LINE__, repo->bus_index, repo->dev_index,
-				 repo->dev_type);
-			error = 0;
-			break;
-		}
-
-		if (notify_event->dev_id == repo->dev_id &&
-		    notify_event->dev_type == PS3_DEV_TYPE_NOACCESS) {
-			pr_debug("%s:%u: no access: dev_id %lu\n", __func__,
-				 __LINE__, repo->dev_id);
-			break;
-		}
-	}
-
-fail_close:
-	lv1_close_device(repo->bus_id, notification_dev_id);
-fail_free:
-	kfree(buf);
-	pr_debug(" <- %s:%u\n", __func__, __LINE__);
-	return error;
-}
-
 static int ps3_setup_storage_dev(const struct ps3_repository_device *repo,
 				 enum ps3_match_id match_id)
 {
@@ -449,16 +289,6 @@ static int ps3_setup_storage_dev(const s
 		goto fail_find_interrupt;
 	}
 
-	/* FIXME: Arrange to only do this on a 'cold' boot */
-
-	result = ps3_storage_wait_for_device(repo);
-	if (result) {
-		printk(KERN_ERR "%s:%u: storage_notification failed %d\n",
-		       __func__, __LINE__, result);
-		result = -ENODEV;
-		goto fail_probe_notification;
-	}
-
 	for (i = 0; i < num_regions; i++) {
 		unsigned int id;
 		u64 start, size;
@@ -494,7 +324,6 @@ static int ps3_setup_storage_dev(const s
 
 fail_device_register:
 fail_read_region:
-fail_probe_notification:
 fail_find_interrupt:
 	kfree(p);
 fail_malloc:
@@ -659,56 +488,219 @@ static int ps3_register_repository_devic
 	return result;
 }
 
+
+#define PS3_NOTIFICATION_DEV_ID		ULONG_MAX
+#define PS3_NOTIFICATION_INTERRUPT_ID	0
+
+struct ps3_notification_device {
+	struct ps3_system_bus_device sbd;
+	spinlock_t lock;
+	u64 tag;
+	u64 lv1_status;
+	struct completion done;
+};
+
+enum ps3_notify_type {
+	notify_device_ready = 0,
+	notify_region_probe = 1,
+	notify_region_update = 2,
+};
+
+struct ps3_notify_cmd {
+	u64 operation_code;		/* must be zero */
+	u64 event_mask;			/* OR of 1UL << enum ps3_notify_type */
+};
+
+struct ps3_notify_event {
+	u64 event_type;			/* enum ps3_notify_type */
+	u64 bus_id;
+	u64 dev_id;
+	u64 dev_type;
+	u64 dev_port;
+};
+
+static irqreturn_t ps3_notification_interrupt(int irq, void *data)
+{
+	struct ps3_notification_device *dev = data;
+	int res;
+	u64 tag, status;
+
+	spin_lock(&dev->lock);
+	res = lv1_storage_get_async_status(PS3_NOTIFICATION_DEV_ID, &tag,
+					   &status);
+	if (tag != dev->tag)
+		pr_err("%s:%u: tag mismatch, got %lx, expected %lx\n",
+		       __func__, __LINE__, tag, dev->tag);
+
+	if (res) {
+		pr_err("%s:%u: res %d status 0x%lx\n", __func__, __LINE__, res,
+		       status);
+	} else {
+		pr_debug("%s:%u: completed, status 0x%lx\n", __func__,
+			 __LINE__, status);
+		dev->lv1_status = status;
+		complete(&dev->done);
+	}
+	spin_unlock(&dev->lock);
+	return IRQ_HANDLED;
+}
+
+static int ps3_notification_read_write(struct ps3_notification_device *dev,
+				       u64 lpar, int write)
+{
+	const char *op = write ? "write" : "read";
+	unsigned long flags;
+	int res;
+
+	init_completion(&dev->done);
+	spin_lock_irqsave(&dev->lock, flags);
+	res = write ? lv1_storage_write(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
+					&dev->tag)
+		    : lv1_storage_read(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
+				       &dev->tag);
+	spin_unlock_irqrestore(&dev->lock, flags);
+	if (res) {
+		pr_err("%s:%u: %s failed %d\n", __func__, __LINE__, op, res);
+		return -EPERM;
+	}
+	pr_debug("%s:%u: notification %s issued\n", __func__, __LINE__, op);
+
+	res = wait_for_completion_interruptible(&dev->done);
+	if (res) {
+		pr_debug("%s:%u: interrupted %s\n", __func__, __LINE__, op);
+		return res;
+	}
+
+	if (dev->lv1_status) {
+		pr_err("%s:%u: %s not completed, status 0x%lx\n", __func__,
+		       __LINE__, op, dev->lv1_status);
+		return -EIO;
+	}
+	pr_debug("%s:%u: notification %s completed\n", __func__, __LINE__, op);
+
+	return 0;
+}
+
+
 /**
  * ps3_probe_thread - Background repository probing at system startup.
  *
  * This implementation only supports background probing on a single bus.
+ * It uses the hypervisor's storage device notification mechanism to wait until
+ * a storage device is ready.  The device notification mechanism uses a
+ * pseudo device to asynchronously notify the guest when storage devices become
+ * ready.  The notification device has a block size of 512 bytes.
  */
 
 static int ps3_probe_thread(void *data)
 {
-	struct ps3_repository_device *repo = data;
-	int result;
-	unsigned int ms = 250;
+	struct ps3_notification_device dev;
+	struct ps3_repository_device repo;
+	int res;
+	unsigned int irq;
+	u64 lpar;
+	void *buf;
+	struct ps3_notify_cmd *notify_cmd;
+	struct ps3_notify_event *notify_event;
 
 	pr_debug(" -> %s:%u: kthread started\n", __func__, __LINE__);
 
-	do {
-		try_to_freeze();
+	buf = kzalloc(512, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	lpar = ps3_mm_phys_to_lpar(__pa(buf));
+	notify_cmd = buf;
+	notify_event = buf;
+
+	/* dummy system bus device */
+	dev.sbd.bus_id = (u64)data;
+	dev.sbd.dev_id = PS3_NOTIFICATION_DEV_ID;
+	dev.sbd.interrupt_id = PS3_NOTIFICATION_INTERRUPT_ID;
+
+	res = lv1_open_device(dev.sbd.bus_id, dev.sbd.dev_id, 0);
+	if (res) {
+		pr_err("%s:%u: lv1_open_device %s\n", __func__, __LINE__,
+		       ps3_result(res));
+		goto fail_free;
+	}
+
+	res = ps3_sb_event_receive_port_setup(&dev.sbd, PS3_BINDING_CPU_ANY,
+					      &irq);
+	if (res) {
+		pr_err("%s:%u: ps3_sb_event_receive_port_setup failed %d\n",
+		       __func__, __LINE__, res);
+	       goto fail_close_device;
+	}
+
+	spin_lock_init(&dev.lock);
+
+	res = request_irq(irq, ps3_notification_interrupt, IRQF_DISABLED,
+			  "ps3_notification", &dev);
+	if (res) {
+		pr_err("%s:%u: request_irq failed %d\n", __func__, __LINE__,
+		       res);
+		goto fail_sb_event_receive_port_destroy;
+	}
 
-		pr_debug("%s:%u: probing...\n", __func__, __LINE__);
+	/* Setup and write the request for device notification. */
+	notify_cmd->operation_code = 0; /* must be zero */
+	notify_cmd->event_mask = 1UL << notify_region_probe;
 
-		do {
-			result = ps3_repository_find_device(repo);
+	res = ps3_notification_read_write(&dev, lpar, 1);
+	if (res)
+		goto fail_free_irq;
 
-			if (result == -ENODEV)
-				pr_debug("%s:%u: nothing new\n", __func__,
-					__LINE__);
-			else if (result)
-				pr_debug("%s:%u: find device error.\n",
-					__func__, __LINE__);
-			else {
-				pr_debug("%s:%u: found device (%u:%u:%u)\n",
-					 __func__, __LINE__, repo->bus_index,
-					 repo->dev_index, repo->dev_type);
-				ps3_register_repository_device(repo);
-				ps3_repository_bump_device(repo);
-				ms = 250;
-			}
-		} while (!result);
+	/* Loop here processing the requested notification events. */
+	do {
+		try_to_freeze();
 
-		pr_debug("%s:%u: ms %u\n", __func__, __LINE__, ms);
+		memset(notify_event, 0, sizeof(*notify_event));
 
-		if ( ms > 60000)
+		res = ps3_notification_read_write(&dev, lpar, 0);
+		if (res)
 			break;
 
-		msleep_interruptible(ms);
+		pr_debug("%s:%u: notify event type 0x%lx bus id %lu dev id %lu"
+			 " type %lu port %lu\n", __func__, __LINE__,
+			 notify_event->event_type, notify_event->bus_id,
+			 notify_event->dev_id, notify_event->dev_type,
+			 notify_event->dev_port);
 
-		/* An exponential backoff. */
-		ms <<= 1;
+		if (notify_event->event_type != notify_region_probe ||
+		    notify_event->bus_id != dev.sbd.bus_id) {
+			pr_warning("%s:%u: bad notify_event: event %lu, "
+				   "dev_id %lu, dev_type %lu\n",
+				   __func__, __LINE__, notify_event->event_type,
+				   notify_event->dev_id,
+				   notify_event->dev_type);
+			continue;
+		}
+
+		res = ps3_repository_find_device_by_id(&repo, dev.sbd.bus_id,
+						       notify_event->dev_id);
+		if (res) {
+			pr_warning("%s:%u: device %lu:%lu not found\n",
+				   __func__, __LINE__, dev.sbd.bus_id,
+				   notify_event->dev_id);
+			continue;
+		}
+
+		pr_debug("%s:%u: device %lu:%lu found\n", __func__, __LINE__,
+			 dev.sbd.bus_id, notify_event->dev_id);
+		ps3_register_repository_device(&repo);
 
 	} while (!kthread_should_stop());
 
+fail_free_irq:
+	free_irq(irq, &dev);
+fail_sb_event_receive_port_destroy:
+	ps3_sb_event_receive_port_destroy(&dev.sbd, irq);
+fail_close_device:
+	lv1_close_device(dev.sbd.bus_id, dev.sbd.dev_id);
+fail_free:
+	kfree(buf);
+
 	pr_debug(" <- %s:%u: kthread finished\n", __func__, __LINE__);
 
 	return 0;
@@ -723,7 +715,7 @@ static int __init ps3_start_probe_thread
 {
 	int result;
 	struct task_struct *task;
-	static struct ps3_repository_device repo; /* must be static */
+	struct ps3_repository_device repo;
 
 	pr_debug(" -> %s:%d\n", __func__, __LINE__);
 
@@ -746,7 +738,8 @@ static int __init ps3_start_probe_thread
 		return -ENODEV;
 	}
 
-	task = kthread_run(ps3_probe_thread, &repo, "ps3-probe-%u", bus_type);
+	task = kthread_run(ps3_probe_thread, (void *)repo.bus_id,
+			   "ps3-probe-%u", bus_type);
 
 	if (IS_ERR(task)) {
 		result = PTR_ERR(task);
--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -89,8 +89,6 @@ enum ps3_dev_type {
 	PS3_DEV_TYPE_STOR_ROM = TYPE_ROM,	/* 5 */
 	PS3_DEV_TYPE_SB_GPIO = 6,
 	PS3_DEV_TYPE_STOR_FLASH = TYPE_RBC,	/* 14 */
-	PS3_DEV_TYPE_STOR_DUMMY = 32,
-	PS3_DEV_TYPE_NOACCESS = 255,
 };
 
 int ps3_repository_read_bus_str(unsigned int bus_index, const char *bus_str,
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -344,35 +344,6 @@ int ps3_repository_find_device(struct ps
 		return result;
 	}
 
-	if (tmp.bus_type == PS3_BUS_TYPE_STORAGE) {
-		/*
-		 * A storage device may show up in the repository before the
-		 * hypervisor has finished probing its type and regions
-		 */
-		unsigned int num_regions;
-
-		if (tmp.dev_type == PS3_DEV_TYPE_STOR_DUMMY) {
-			pr_debug("%s:%u storage device not ready\n", __func__,
-				 __LINE__);
-			return -ENODEV;
-		}
-
-		result = ps3_repository_read_stor_dev_num_regions(tmp.bus_index,
-								  tmp.dev_index,
-								  &num_regions);
-		if (result) {
-			pr_debug("%s:%d read_stor_dev_num_regions failed\n",
-				 __func__, __LINE__);
-			return result;
-		}
-
-		if (!num_regions) {
-			pr_debug("%s:%u storage device has no regions yet\n",
-				 __func__, __LINE__);
-			return -ENODEV;
-		}
-	}
-
 	result = ps3_repository_read_dev_id(tmp.bus_index, tmp.dev_index,
 		&tmp.dev_id);
 

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

^ permalink raw reply


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