* Re: [PATCH 2/3] powerpc/44x: don't use tlbivax on AMP systems
From: Kumar Gala @ 2011-07-13 5:48 UTC (permalink / raw)
To: Tony Breeds; +Cc: Josh Boyer, LinuxPPC-dev
In-Reply-To: <20110705043803.GG13483@ozlabs.org>
On Jul 4, 2011, at 11:38 PM, Tony Breeds wrote:
> +#ifdef CONFIG_PPC_47x
> +void __init early_init_mmu_47x(void)
> +{
> +#ifdef CONFIG_SMP
> + unsigned long root = of_get_flat_dt_root();
> + if (of_get_flat_dt_prop(root, "cooperative-partition", NULL))
> + mmu_clear_feature(MMU_FTR_USE_TLBIVAX_BCAST);
> +#endif /* CONFIG_SMP */
> +}
> +#endif /* CONFIG_PPC_47x */
> +
Is this device tree prop spec'd anywhere?
- k
^ permalink raw reply
* [PATCH] hvc_console: Add kdb support
From: Anton Blanchard @ 2011-07-13 5:44 UTC (permalink / raw)
To: benh, jason.wessel; +Cc: kgdb-bugreport, linuxppc-dev
Add poll_get_char and poll_put_char for kdb. Enable kdb at boot with:
kgdboc=hvc0
or at runtime with:
echo hvc0 > /sys/module/kgdboc/parameters/kgdboc
Signed-off-by: Anton Blanchard <anton@samba.org>
---
A couple of things:
- I needed to enable CONFIG_KGDB_SERIAL_CONSOLE in order to
enable CONFIG_CONSOLE_POLL. It's somewhat confusing and I wonder
if we should always enable CONFIG_CONSOLE_POLL when CONFIG_KGDB
is enabled.
- I had to pull in linux/serial_core.h to get NO_POLL_CHAR, we
might want to move it somewhere more generic.
- Things are a little bit awkward because poll_get_char and
poll_put_char take a struct tty_driver instead of a struct tty,
I just grab the first entry in the ttys[] list.
FYI: This patch also needs "powerpc/pseries: Fix hvterm_raw_get_chars
to accept < 16 chars, fixing xmon" to be applied:
http://patchwork.ozlabs.org/patch/104475/
Index: linux-powerpc/drivers/tty/hvc/hvc_console.c
===================================================================
--- linux-powerpc.orig/drivers/tty/hvc/hvc_console.c 2011-07-13 15:08:15.853817380 +1000
+++ linux-powerpc/drivers/tty/hvc/hvc_console.c 2011-07-13 15:10:16.555874550 +1000
@@ -39,6 +39,7 @@
#include <linux/delay.h>
#include <linux/freezer.h>
#include <linux/slab.h>
+#include <linux/serial_core.h>
#include <asm/uaccess.h>
@@ -766,6 +767,39 @@ static int hvc_tiocmset(struct tty_struc
return hp->ops->tiocmset(hp, set, clear);
}
+#ifdef CONFIG_CONSOLE_POLL
+int hvc_poll_init(struct tty_driver *driver, int line, char *options)
+{
+ return 0;
+}
+
+static int hvc_poll_get_char(struct tty_driver *driver, int line)
+{
+ struct tty_struct *tty = driver->ttys[0];
+ struct hvc_struct *hp = tty->driver_data;
+ int n;
+ char ch;
+
+ n = hp->ops->get_chars(hp->vtermno, &ch, 1);
+
+ if (n == 0)
+ return NO_POLL_CHAR;
+
+ return ch;
+}
+
+static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch)
+{
+ struct tty_struct *tty = driver->ttys[0];
+ struct hvc_struct *hp = tty->driver_data;
+ int n;
+
+ do {
+ n = hp->ops->put_chars(hp->vtermno, &ch, 1);
+ } while (n <= 0);
+}
+#endif
+
static const struct tty_operations hvc_ops = {
.open = hvc_open,
.close = hvc_close,
@@ -776,6 +810,11 @@ static const struct tty_operations hvc_o
.chars_in_buffer = hvc_chars_in_buffer,
.tiocmget = hvc_tiocmget,
.tiocmset = hvc_tiocmset,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_init = hvc_poll_init,
+ .poll_get_char = hvc_poll_get_char,
+ .poll_put_char = hvc_poll_put_char,
+#endif
};
struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
^ permalink raw reply
* Re: [PATCH] powerpc: add denormalisation exception handling for POWER6/7
From: Michael Neuling @ 2011-07-13 5:30 UTC (permalink / raw)
To: Kumar Gala; +Cc: Paul Mackerras, miltonm, linuxppc-dev
In-Reply-To: <5E6FBB75-BE97-4D97-AEA2-EC8EAF1E0ECF@kernel.crashing.org>
In message <5E6FBB75-BE97-4D97-AEA2-EC8EAF1E0ECF@kernel.crashing.org> you wrote
:
>
> On Jul 11, 2011, at 12:52 AM, Michael Neuling wrote:
>
> > On POWER6 and POWER7 if the input operand to an instruction is a
> > denormalised single precision binary floating we can take a
> > denormalisation exception where it's expected that the hypervisor =
> (HV=3D1)
> > will fix up the inputs before the instruction is run.
> >=20
> > This adds code to handle this denormalisation exception for POWER6 and
> > POWER7.
> >=20
> > It also add a CONFIG_PPC_DENORMALISATION option and sets it in
> > pseries/ppc64_defconfig.=20
> >=20
> > This is useful on bare metal systems only. Based on patch from Milton
> > Miller.
> >=20
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> >=20
> > ---
> > arch/powerpc/Kconfig | 7 +
> > arch/powerpc/configs/ppc64_defconfig | 1=20
> > arch/powerpc/configs/pseries_defconfig | 1=20
> > arch/powerpc/include/asm/ppc-opcode.h | 2=20
> > arch/powerpc/include/asm/reg.h | 1=20
> > arch/powerpc/kernel/exceptions-64s.S | 125 =
> +++++++++++++++++++++++++++++++++
> > 6 files changed, 137 insertions(+)
> >=20
> > Index: linux-ozlabs/arch/powerpc/Kconfig
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > --- linux-ozlabs.orig/arch/powerpc/Kconfig
> > +++ linux-ozlabs/arch/powerpc/Kconfig
> > @@ -556,6 +556,13 @@ config SCHED_SMT
> > when dealing with POWER5 cpus at a cost of slightly increased
> > overhead in some places. If unsure say N here.
> >=20
> > +config PPC_DENORMALISATION
> > + bool "PowerPC denormalisation exception handling"
>
> Should this at least depend on PPC_BOOK3S_64 ?
Yep, thanks!
Mikey
^ permalink raw reply
* Re: [PATCH v2] powerpc32: Kexec support for PPC440X chipsets
From: Kumar Gala @ 2011-07-13 5:28 UTC (permalink / raw)
To: Suzuki K. Poulose
Cc: Sebastian Andrzej Siewior, kexec, lkml, Josh Boyer,
Paul Mackerras, linux ppc dev, Vivek Goyal
In-Reply-To: <20110712064356.28567.48722.stgit@suzukikp.in.ibm.com>
On Jul 12, 2011, at 1:44 AM, Suzuki K. Poulose wrote:
> Changes from V1: Uses a tmp mapping in the other address space to =
setup
> the 1:1 mapping (suggested by Sebastian Andrzej =
Siewior).
>=20
> Note: Should we do the same for kernel entry code for PPC44x ?
>=20
> This patch adds kexec support for PPC440 based chipsets.This work is =
based
> on the KEXEC patches for FSL BookE.
>=20
> The FSL BookE patch and the code flow could be found at the link =
below:
>=20
> http://patchwork.ozlabs.org/patch/49359/
>=20
> Steps:
>=20
> 1) Invalidate all the TLB entries except the one this code is run from
> 2) Create a tmp mapping for our code in the other address space and =
jump to it
> 3) Invalidate the entry we used
> 4) Create a 1:1 mapping for 0-2GiB in blocks of 256M
> 5) Jump to the new 1:1 mapping and invalidate the tmp mapping
>=20
> I have tested this patches on Ebony, Sequoia boards and Virtex on =
QEMU.
> It would be great if somebody could test this on the other boards.
>=20
> Signed-off-by: Suzuki Poulose <suzuki@in.ibm.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>=20
> arch/powerpc/Kconfig | 2=20
> arch/powerpc/include/asm/kexec.h | 2=20
> arch/powerpc/kernel/misc_32.S | 170 =
++++++++++++++++++++++++++++++++++++++
> 3 files changed, 172 insertions(+), 2 deletions(-)
>=20
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 423145a6..d04fae0 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -349,7 +349,7 @@ config ARCH_ENABLE_MEMORY_HOTREMOVE
>=20
> config KEXEC
> bool "kexec system call (EXPERIMENTAL)"
> - depends on (PPC_BOOK3S || FSL_BOOKE) && EXPERIMENTAL
> + depends on (PPC_BOOK3S || FSL_BOOKE || (44x && !SMP && !47x)) && =
EXPERIMENTAL
Is there something special about 47x that its not supported?
> help
> kexec is a system call that implements the ability to shutdown =
your
> current kernel, and to start another kernel. It is like a =
reboot
- k=
^ permalink raw reply
* Re: [PATCH] powerpc: add denormalisation exception handling for POWER6/7
From: Kumar Gala @ 2011-07-13 5:27 UTC (permalink / raw)
To: Michael Neuling; +Cc: Paul Mackerras, miltonm, linuxppc-dev
In-Reply-To: <11738.1310363538@neuling.org>
On Jul 11, 2011, at 12:52 AM, Michael Neuling wrote:
> On POWER6 and POWER7 if the input operand to an instruction is a
> denormalised single precision binary floating we can take a
> denormalisation exception where it's expected that the hypervisor =
(HV=3D1)
> will fix up the inputs before the instruction is run.
>=20
> This adds code to handle this denormalisation exception for POWER6 and
> POWER7.
>=20
> It also add a CONFIG_PPC_DENORMALISATION option and sets it in
> pseries/ppc64_defconfig.=20
>=20
> This is useful on bare metal systems only. Based on patch from Milton
> Miller.
>=20
> Signed-off-by: Michael Neuling <mikey@neuling.org>
>=20
> ---
> arch/powerpc/Kconfig | 7 +
> arch/powerpc/configs/ppc64_defconfig | 1=20
> arch/powerpc/configs/pseries_defconfig | 1=20
> arch/powerpc/include/asm/ppc-opcode.h | 2=20
> arch/powerpc/include/asm/reg.h | 1=20
> arch/powerpc/kernel/exceptions-64s.S | 125 =
+++++++++++++++++++++++++++++++++
> 6 files changed, 137 insertions(+)
>=20
> Index: linux-ozlabs/arch/powerpc/Kconfig
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- linux-ozlabs.orig/arch/powerpc/Kconfig
> +++ linux-ozlabs/arch/powerpc/Kconfig
> @@ -556,6 +556,13 @@ config SCHED_SMT
> when dealing with POWER5 cpus at a cost of slightly increased
> overhead in some places. If unsure say N here.
>=20
> +config PPC_DENORMALISATION
> + bool "PowerPC denormalisation exception handling"
Should this at least depend on PPC_BOOK3S_64 ?
> + default "n"
> + ---help---
> + Add support for handling denormalisation of single precision
> + values. Useful for bare metal only. If unsure say Y here.
> +
> config CMDLINE_BOOL
> bool "Default bootloader kernel arguments"
>=20
>=20
- k=
^ permalink raw reply
* [PATCH] powerpc/pseries: Fix hvterm_raw_get_chars to accept < 16 chars, fixing xmon
From: Anton Blanchard @ 2011-07-13 5:19 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
commit 4d2bb3f50036 (powerpc/pseries: Re-implement HVSI as part of
hvc_vio) changed udbg_getc to be based on hvterm_raw_get_chars.
Unfortunately hvterm_raw_get_chars returns -EAGAIN if you ask
for anything less than 16 characters. As a result xmon no longer
accepts input and prints a stream of junk to the screen.
The recent change highlights a problem that xmon on pseries VIO
has had all along, that it can drop input characters. The issue
is the hypervisor call does not take a count argument and can
return up to 16 characters.
This patch adds a per vterm buffer that we copy input data into
and give it out as requested.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-powerpc/drivers/tty/hvc/hvc_vio.c
===================================================================
--- linux-powerpc.orig/drivers/tty/hvc/hvc_vio.c 2011-07-13 14:37:16.251635811 +1000
+++ linux-powerpc/drivers/tty/hvc/hvc_vio.c 2011-07-13 14:48:33.843356126 +1000
@@ -71,41 +71,53 @@ struct hvterm_priv {
u32 termno; /* HV term number */
hv_protocol_t proto; /* Raw data or HVSI packets */
struct hvsi_priv hvsi; /* HVSI specific data */
+ spinlock_t buf_lock;
+ char buf[SIZE_VIO_GET_CHARS];
+ int left;
+ int offset;
};
static struct hvterm_priv *hvterm_privs[MAX_NR_HVC_CONSOLES];
-
/* For early boot console */
static struct hvterm_priv hvterm_priv0;
static int hvterm_raw_get_chars(uint32_t vtermno, char *buf, int count)
{
struct hvterm_priv *pv = hvterm_privs[vtermno];
- unsigned long got, i;
+ unsigned long i;
+ unsigned long flags;
+ int got;
if (WARN_ON(!pv))
return 0;
- /*
- * Vio firmware will read up to SIZE_VIO_GET_CHARS at its own discretion
- * so we play safe and avoid the situation where got > count which could
- * overload the flip buffer.
- */
- if (count < SIZE_VIO_GET_CHARS)
- return -EAGAIN;
-
- got = hvc_get_chars(pv->termno, buf, count);
+ spin_lock_irqsave(&pv->buf_lock, flags);
- /*
- * Work around a HV bug where it gives us a null
- * after every \r. -- paulus
- */
- for (i = 1; i < got; ++i) {
- if (buf[i] == 0 && buf[i-1] == '\r') {
- --got;
- if (i < got)
- memmove(&buf[i], &buf[i+1], got - i);
+ if (pv->left == 0) {
+ pv->offset = 0;
+ pv->left = hvc_get_chars(pv->termno, pv->buf, count);
+
+ /*
+ * Work around a HV bug where it gives us a null
+ * after every \r. -- paulus
+ */
+ for (i = 1; i < pv->left; ++i) {
+ if (pv->buf[i] == 0 && pv->buf[i-1] == '\r') {
+ --pv->left;
+ if (i < pv->left) {
+ memmove(&pv->buf[i], &pv->buf[i+1],
+ pv->left - i);
+ }
+ }
}
}
+
+ got = min(count, pv->left);
+ memcpy(buf, &pv->buf[pv->offset], got);
+ pv->offset += got;
+ pv->left -= got;
+
+ spin_unlock_irqrestore(&pv->buf_lock, flags);
+
return got;
}
@@ -266,6 +278,7 @@ static int __devinit hvc_vio_probe(struc
return -ENOMEM;
pv->termno = vdev->unit_address;
pv->proto = proto;
+ spin_lock_init(&pv->buf_lock);
hvterm_privs[termno] = pv;
hvsilib_init(&pv->hvsi, hvc_get_chars, hvc_put_chars,
pv->termno, 0);
@@ -406,6 +419,7 @@ void __init hvc_vio_init_early(void)
if (termno == NULL)
goto out;
hvterm_priv0.termno = *termno;
+ spin_lock_init(&hvterm_priv0.buf_lock);
hvterm_privs[0] = &hvterm_priv0;
/* Check the protocol */
@@ -447,6 +461,7 @@ void __init udbg_init_debug_lpar(void)
hvterm_privs[0] = &hvterm_priv0;
hvterm_priv0.termno = 0;
hvterm_priv0.proto = HV_PROTOCOL_RAW;
+ spin_lock_init(&hvterm_priv0.buf_lock)
udbg_putc = udbg_hvc_putc;
udbg_getc = udbg_hvc_getc;
udbg_getc_poll = udbg_hvc_getc_poll;
@@ -459,6 +474,7 @@ void __init udbg_init_debug_lpar_hvsi(vo
hvterm_privs[0] = &hvterm_priv0;
hvterm_priv0.termno = CONFIG_PPC_EARLY_DEBUG_HVSI_VTERMNO;
hvterm_priv0.proto = HV_PROTOCOL_HVSI;
+ spin_lock_init(&hvterm_priv0.buf_lock)
udbg_putc = udbg_hvc_putc;
udbg_getc = udbg_hvc_getc;
udbg_getc_poll = udbg_hvc_getc_poll;
^ permalink raw reply
* MPC8360RDK: Badness at kernel/softirq.c:122
From: ps2k @ 2011-07-13 4:13 UTC (permalink / raw)
To: linuxppc-dev
Hi,
When we are writing to the flash while data traffic is on we do see the
target rebooting with the following messages. Could some body please explain
what's going on here?
Any help is greatly appreciated.
Regards
psk
Call Trace:"
LR [c0005ea4] do_softirq+0x58/0x5c"
Badness at c0051360 [verbose debug info unavailable]"
NIP [c0051360] _local_bh_enable+0x40/0x84"
TASK = d772d430[1343] 'cp' THREAD: dc916000"
MSR: 00021032 <ME,IR,DR> CR: 44242482 XER: 20000000"
REGS: dc916860 TRAP: 0700 Not tainted (2.6.24-pwn-0.5.0.XXXX)"
NIP: c0051360 LR: c0005ea4 CTR: c01c0098"
------------[ cut here ]------------"
----------------------------------------
[d715c900] [c00060c0] do_softirq+0x58/0x5c"
Badness at net/core/skbuff.c:317"
[d715c8e0] [c0052670] __do_softirq+0x84/0xf8"
[d715c8c0] [c01c36fc] net_tx_action+0x84/0x17c"
[d715c8b0] [c01b9b2c] __kfree_skb+0x18/0xe8"
[d715c8a0] [0000012c] 0x12c (unreliable)"
Call Trace:"
LR [c01b9b2c] __kfree_skb+0x18/0xe8"
NIP [c01ba584] skb_release_all+0x68/0x124"
TASK = dc54c850[856] 'cp' THREAD: d715c000"
MSR: 00029032 <EE,ME,IR,DR> CR: 24242488 XER: 20000000"
REGS: d715c7f0 TRAP: 0700 Not tainted (2.6.24-pwn-0.5.0.XXXX)"
NIP: c01ba584 LR: c01b9b2c CTR: c01c3678"
------------[ cut here ]------------"
-----------------------
Badness at kernel/softirq.c:122"
Call Trace:"
LR [c00060c0] do_softirq+0x58/0x5c"
NIP [c0051f78] _local_bh_enable+0x40/0x84"
TASK = dc5587d0[1763] 'cp' THREAD: dc7ee000"
MSR: 00021032 <ME,IR,DR> CR: 44242482 XER: 00000000"
REGS: dc7ee850 TRAP: 0700 Not tainted (2.6.24-pwn-0.5.0.XXXX)"
NIP: c0051f78 LR: c00060c0 CTR: c0062040"
------------[ cut here ]------------"
-------------------------
Badness at kernel/softirq.c:122"
[d7114930] [c001108c] ret_from_except+0x0/0x14"
[d7114920] [c00065d4] do_IRQ+0xa8/0xc8"
[d7114910] [c0052190] irq_exit+0x60/0x80"
[d7114900] [c00060c0] do_softirq+0x58/0x5c (unreliable)"
Call Trace:"
LR [c00060c0] do_softirq+0x58/0x5c"
NIP [c0051f78] _local_bh_enable+0x40/0x84"
TASK = dc01c430[2468] 'cp' THREAD: d7114000"
MSR: 00021032 <ME,IR,DR> CR: 44242482 XER: 20000000"
REGS: d7114850 TRAP: 0700 Tainted: G D (2.6.24-pwn-0.5.0.XXXX)"
NIP: c0051f78 LR: c00060c0 CTR: c016b7f4"
------------[ cut here ]------------"
--
View this message in context: http://old.nabble.com/MPC8360RDK%3A-Badness-at-kernel-softirq.c%3A122-tp32050889p32050889.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] [v3] kexec-tools: ppc32: Fixup ThreadPointer for purgatory code
From: Simon Horman @ 2011-07-13 2:41 UTC (permalink / raw)
To: Suzuki K. Poulose
Cc: kexec, Josh Boyer, Paul Mackerras, linux ppc dev, Vivek Goyal
In-Reply-To: <20110712095029.18466.30735.stgit@suzukikp.in.ibm.com>
On Tue, Jul 12, 2011 at 03:20:40PM +0530, Suzuki K. Poulose wrote:
> PPC32 ELF ABI expects r2 to be loaded with Thread Pointer, which is 0x7000
> bytes past the end of TCB. Though the purgatory is single threaded, it uses
> TCB scratch space in vsnprintf(). This patch allocates a 1024byte TCB
> and populates the TP with the address accordingly.
>
> Changes from V2: Avoid address overflow in TP allocation.
> Changes from V1: Fixed the addr calculation for uImage support.
>
>
> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
> Cc: Ryan S. Arnold <rsa@us.ibm.com>
Thanks, applied.
^ permalink raw reply
* Re: [PATCH] 4xx: Add check_link to struct ppc4xx_pciex_hwops
From: Tony Breeds @ 2011-07-13 0:42 UTC (permalink / raw)
To: Josh Boyer; +Cc: Ayman El-Khashab, LinuxPPC-dev
In-Reply-To: <20110712180404.GC4203@zod.rchland.ibm.com>
On Tue, Jul 12, 2011 at 02:04:04PM -0400, Josh Boyer wrote:
> On Tue, Jul 12, 2011 at 12:40:07PM -0500, Ayman El-Khashab wrote:
> >On Fri, Jul 01, 2011 at 04:44:24PM +1000, Tony Breeds wrote:
> >> All current pcie controllers unconditionally use SDR to check the link and
> >> poll for reset.
> >
> >I was able to apply this patch and then modify the 460SX to
> >work correctly, so I think it is fine. There is only 1
> >comment below. So how does one supply a patch atop another
> >patch?
> >
> >Best,
> >Ayman
> >
> >> +static int __init ppc4xx_pciex_port_reset_sdr(struct ppc4xx_pciex_port *port)
> >> +{
> >> + printk(KERN_INFO "PCIE%d: Checking link...\n",
> >> + port->index);
> >
> >Its not a functional problem, but this printk belongs in the
> >check link if anywhere rather than the reset.
>
> I've got this queued in my tree locally. I can make that change before
> I push it out.
Thanks Josh. I really thought I'd fixed that before posting.
Yours Tony
^ permalink raw reply
* Re: [PATCH] 4xx: Add check_link to struct ppc4xx_pciex_hwops
From: Tony Breeds @ 2011-07-13 0:41 UTC (permalink / raw)
To: Ayman El-Khashab; +Cc: LinuxPPC-dev, Josh Boyer
In-Reply-To: <20110712221338.GA30666@crust.elkhashab.com>
On Tue, Jul 12, 2011 at 05:13:38PM -0500, Ayman El-Khashab wrote:
> Ok, so let me ask the following ... will it cause trouble if
> I swap the sequence of the calls to the following in xxx_port_init
>
> ppc4xx_pciex_port_init_mapping(...)
>
> and
>
> if (ppc4xx_pciex_hwops->check_link)...
>
> The reason is that at least on the 460SX, the link check is
> done via registers in the config space. But the init_mapping is
> needed to setup some of the DCRs to make the config space work.
> In my check_link, i map the config space do the link checks
> and then unmap since a superset of the space could be mapped
> later.
This is also what I do. IIRC ppc4xx_pciex_port_init_mapping() required
things that are setup between the 2 calls.
The double Mapping is fugly but I think it should be safe.
Yours Tony
^ permalink raw reply
* Re: [PATCH] 4xx: Add check_link to struct ppc4xx_pciex_hwops
From: Ayman El-Khashab @ 2011-07-12 22:13 UTC (permalink / raw)
To: Josh Boyer; +Cc: LinuxPPC-dev
In-Reply-To: <20110712180404.GC4203@zod.rchland.ibm.com>
On Tue, Jul 12, 2011 at 02:04:04PM -0400, Josh Boyer wrote:
> On Tue, Jul 12, 2011 at 12:40:07PM -0500, Ayman El-Khashab wrote:
> >On Fri, Jul 01, 2011 at 04:44:24PM +1000, Tony Breeds wrote:
> >> All current pcie controllers unconditionally use SDR to check the link and
> >> poll for reset.
> >
> >I was able to apply this patch and then modify the 460SX to
> >work correctly, so I think it is fine. There is only 1
> >comment below. So how does one supply a patch atop another
> >patch?
> >
> >Best,
> >Ayman
> >
> >> +static int __init ppc4xx_pciex_port_reset_sdr(struct ppc4xx_pciex_port *port)
> >> +{
> >> + printk(KERN_INFO "PCIE%d: Checking link...\n",
> >> + port->index);
> >
> >Its not a functional problem, but this printk belongs in the
> >check link if anywhere rather than the reset.
>
> I've got this queued in my tree locally. I can make that change before
> I push it out.
>
Ok, so let me ask the following ... will it cause trouble if
I swap the sequence of the calls to the following in xxx_port_init
ppc4xx_pciex_port_init_mapping(...)
and
if (ppc4xx_pciex_hwops->check_link)...
The reason is that at least on the 460SX, the link check is
done via registers in the config space. But the init_mapping is
needed to setup some of the DCRs to make the config space work.
In my check_link, i map the config space do the link checks
and then unmap since a superset of the space could be mapped
later.
Thanks,
ayman
^ permalink raw reply
* Please pull 'next' branch of 4xx tree
From: Josh Boyer @ 2011-07-12 20:41 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
Hi Ben,
A few fixes from Tony/Dave, a DTS update from Stefan, and a MAINTAINERS
update.
josh
The following changes since commit af9719c3062dfe216a0c3de3fa52be6d22b4456c:
powerpc: Use -mtraceback=no (2011-07-01 13:49:27 +1000)
are available in the git repository at:
ssh://master.kernel.org/pub/scm/linux/kernel/git/jwboyer/powerpc-4xx.git next
Dave Kleikamp (3):
powerpc/44x: don't use tlbivax on AMP systems
powerpc/44x: boot wrapper: allow kernel to load into non-zero address
powerpc/47x: allow kernel to be loaded in higher physical memory
Josh Boyer (2):
MAINTAINERS: Update PowerPC 4xx entry
powerpc/4xx: Move PCIE printk to proper function
Stefan Roese (1):
powerpc/44x: Use correct phy-address dt nodes on taishan.dts
Tony Breeds (1):
powerpc/4xx: Add check_link to struct ppc4xx_pciex_hwops
MAINTAINERS | 2 +-
arch/powerpc/Kconfig | 2 +-
arch/powerpc/boot/dts/taishan.dts | 4 +-
arch/powerpc/boot/treeboot-iss4xx.c | 23 ++++-
arch/powerpc/configs/44x/iss476-smp_defconfig | 6 +-
arch/powerpc/include/asm/mmu.h | 7 +-
arch/powerpc/kernel/head_44x.S | 42 ++++++--
arch/powerpc/kernel/setup_32.c | 2 +
arch/powerpc/mm/44x_mmu.c | 13 ++-
arch/powerpc/mm/tlb_hash32.c | 4 +
arch/powerpc/mm/tlb_nohash.c | 19 +++
arch/powerpc/sysdev/ppc4xx_pci.c | 147 ++++++++++++++-----------
12 files changed, 186 insertions(+), 85 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/47x: allow kernel to be loaded in higher physical memory
From: Suzuki Poulose @ 2011-07-12 20:22 UTC (permalink / raw)
To: Josh Boyer; +Cc: LinuxPPC-dev
In-Reply-To: <CA+5PVA6d9=PfsFbbFKyNSu6a+pUd0=ZBFVXzUYAYmg3hd5poXQ@mail.gmail.com>
On 07/12/11 18:57, Josh Boyer wrote:
> On Tue, Jul 5, 2011 at 2:18 AM, Suzuki Poulose<suzuki@in.ibm.com> wrote:
>> On 07/05/11 10:06, Tony Breeds wrote:
>>>
>>> From: Dave Kleikamp<shaggy@linux.vnet.ibm.com>
>>>
>>> The 44x code (which is shared by 47x) assumes the available physical
>>> memory
>>> begins at 0x00000000. This is not necessarily the case in an AMP
>>> environment.
>>>
>>> Support CONFIG_RELOCATABLE for 476 in order to allow the kernel to be
>>> loaded into a higher memory range.
>>
>> I think the code assumes, the kernel is loaded in 256M aligned page. You may
>> want to mention that in the description here.
>
> Suzie, do you have any other concerns with this code in regards to
> your kexec/kdump work for 4xx? It seems fairly self-contained to me,
> so I'd like to apply it but I want to make sure it is not going to
> majorly conflict with the work you're doing.
Please go ahead.
Thanks
Suzuki
^ permalink raw reply
* Re: softirqs are invoked while bottom halves are masked (was: Re: [PATCH] [PATCH] Fix deadlock in af_packet while stressing raw ethernet socket interface)
From: Ronny Meeus @ 2011-07-12 19:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: linuxppc-dev, netdev, afleming, Thomas De Schampheleire,
David Miller
In-Reply-To: <1310484474.2871.14.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Tue, Jul 12, 2011 at 5:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrot=
e:
> Le mardi 12 juillet 2011 =E0 14:03 +0200, Ronny Meeus a =E9crit :
>
>> Sorry for not mentioning we were using a patched kernel.
>> I was not aware that the code involved was patched by the FreeScale
>> patches we applied. The code found in the stack dumps is not
>> implemented in FSL specific files.
>>
>> While reading the code of af_packet I saw that the spin_lock_bh is
>> used in several places while this is not the case in the tpacket_rcv
>> function. Since we had a locking issue in that code, I thought that my
>> patch would be OK.
>> I was not aware that for that specific function (tpacket_rcv) a
>> different lock primitive must be used. A suggestion for improvement:
>> it would be better to document this pre-condition in the code.
>>
>> After doing the change you proposed our code now looks like:
>>
>> >---if (dev->features & NETIF_F_HW_QDISC) {
>> >--->---txq =3D dev_pick_tx(dev, skb);
>> >--->---local_bh_disable();
>> >--->---rc =3D dev_hard_start_xmit(skb, dev, txq);
>> >--->---local_bh_enable();
>> >--->---return rc;
>> >---}
>>
>> >---/* Disable soft irqs for various locks below. Also
>> >--- * stops preemption for RCU.
>> >--- */
>> >---rcu_read_lock_bh();
>>
>> but we still see the issue "BUG: sleeping function called from invalid c=
ontext":
>
> Of course you are if this is the only change you did.
>
>>
>> [ =A0 91.015989] BUG: sleeping function called from invalid context at
>> include/linux/skbuff.h:786
>> [ =A0 91.117096] in_atomic(): 1, irqs_disabled(): 0, pid: 1865, name: NM=
TX_T1842
>> [ =A0 91.200461] Call Trace:
>> [ =A0 91.229672] [ec58bbd0] [c000789c] show_stack+0x78/0x18c (unreliable=
)
>> [ =A0 91.305791] [ec58bc10] [c0022900] __might_sleep+0x100/0x118
>> [ =A0 91.372524] [ec58bc20] [c029f8d8] dpa_tx+0x128/0x758
>
>
> Please read again my mail :
>
> I said : "doing GFP_KERNEL allocations in dpa_tx() is wrong, for sure."
>
> I dont have this code, but I suspect it's using : skb_copy(skb,
> GFP_KERNEL)
>
> Just say no, use GFP_ATOMIC instead.
>
> Real question is : why skb_copy() is done, since its slow as hell.
>
>> [ =A0 91.431957] [ec58bc80] [c02d78ec] dev_hard_start_xmit+0x424/0x588
>> [ =A0 91.504952] [ec58bcc0] [c02d7ab0] dev_queue_xmit+0x60/0x3ac
>> [ =A0 91.571692] [ec58bcf0] [c0338d54] packet_sendmsg+0x8c4/0x988
>> [ =A0 91.639457] [ec58bd70] [c02c3838] sock_sendmsg+0x90/0xb4
>> [ =A0 91.703066] [ec58be40] [c02c4420] sys_sendto+0xdc/0x120
>> [ =A0 91.765646] [ec58bf10] [c02c57d0] sys_socketcall+0x148/0x210
>> [ =A0 91.833420] [ec58bf40] [c001084c] ret_from_syscall+0x0/0x3c
>> [ =A0 91.900153] --- Exception: c01 at 0x4824df00
>> [ =A0 91.900157] =A0 =A0 LR =3D 0x4828a030
>>
>
>
>
I have identified the piece of code, it was a call to skb_unshare. I
have changed it into GFP_ATOMIC.
>---if (skb_cloned(skb))
>--->---skb =3D skb_unshare(skb, GFP_ATOMIC);
After doing this change, I do not see the issue anymore. At least not
for the test I'm doing right now.
After seeing all your comments today, it might be that other issues popup l=
ater.
BTW Are there any good sites (or books) that document this part of the
Linux kernel?
Best regards,
Ronny
^ permalink raw reply
* Re: [PATCH] 4xx: Add check_link to struct ppc4xx_pciex_hwops
From: Josh Boyer @ 2011-07-12 18:04 UTC (permalink / raw)
To: Ayman El-Khashab; +Cc: LinuxPPC-dev
In-Reply-To: <20110712174007.GA26664@crust.elkhashab.com>
On Tue, Jul 12, 2011 at 12:40:07PM -0500, Ayman El-Khashab wrote:
>On Fri, Jul 01, 2011 at 04:44:24PM +1000, Tony Breeds wrote:
>> All current pcie controllers unconditionally use SDR to check the link and
>> poll for reset.
>
>I was able to apply this patch and then modify the 460SX to
>work correctly, so I think it is fine. There is only 1
>comment below. So how does one supply a patch atop another
>patch?
>
>Best,
>Ayman
>
>> +static int __init ppc4xx_pciex_port_reset_sdr(struct ppc4xx_pciex_port *port)
>> +{
>> + printk(KERN_INFO "PCIE%d: Checking link...\n",
>> + port->index);
>
>Its not a functional problem, but this printk belongs in the
>check link if anywhere rather than the reset.
I've got this queued in my tree locally. I can make that change before
I push it out.
josh
^ permalink raw reply
* Re: [PATCH] 4xx: Add check_link to struct ppc4xx_pciex_hwops
From: Ayman El-Khashab @ 2011-07-12 17:40 UTC (permalink / raw)
To: LinuxPPC-dev, Benjamin Herrenschmidt, Josh Boyer
In-Reply-To: <20110701064424.GE13483@ozlabs.org>
On Fri, Jul 01, 2011 at 04:44:24PM +1000, Tony Breeds wrote:
> All current pcie controllers unconditionally use SDR to check the link and
> poll for reset.
I was able to apply this patch and then modify the 460SX to
work correctly, so I think it is fine. There is only 1
comment below. So how does one supply a patch atop another
patch?
Best,
Ayman
> +static int __init ppc4xx_pciex_port_reset_sdr(struct ppc4xx_pciex_port *port)
> +{
> + printk(KERN_INFO "PCIE%d: Checking link...\n",
> + port->index);
Its not a functional problem, but this printk belongs in the
check link if anywhere rather than the reset.
> +
> + /* Wait for reset to complete */
> + if (ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS, 1 << 20, 0, 10)) {
> + printk(KERN_WARNING "PCIE%d: PGRST failed\n",
> + port->index);
> + return -1;
> + }
> + return 0;
> +}
^ permalink raw reply
* Re: softirqs are invoked while bottom halves are masked (was: Re: [PATCH] [PATCH] Fix deadlock in af_packet while stressing raw ethernet socket interface)
From: Eric Dumazet @ 2011-07-12 15:27 UTC (permalink / raw)
To: Ronny Meeus
Cc: linuxppc-dev, netdev, afleming, Thomas De Schampheleire,
David Miller
In-Reply-To: <CAMJ=MEeC1hoqufs7AfFRn3yJoC8mdw7v+14N+7e=wQuJefm4_w@mail.gmail.com>
Le mardi 12 juillet 2011 à 14:03 +0200, Ronny Meeus a écrit :
> Sorry for not mentioning we were using a patched kernel.
> I was not aware that the code involved was patched by the FreeScale
> patches we applied. The code found in the stack dumps is not
> implemented in FSL specific files.
>
> While reading the code of af_packet I saw that the spin_lock_bh is
> used in several places while this is not the case in the tpacket_rcv
> function. Since we had a locking issue in that code, I thought that my
> patch would be OK.
> I was not aware that for that specific function (tpacket_rcv) a
> different lock primitive must be used. A suggestion for improvement:
> it would be better to document this pre-condition in the code.
>
> After doing the change you proposed our code now looks like:
>
> >---if (dev->features & NETIF_F_HW_QDISC) {
> >--->---txq = dev_pick_tx(dev, skb);
> >--->---local_bh_disable();
> >--->---rc = dev_hard_start_xmit(skb, dev, txq);
> >--->---local_bh_enable();
> >--->---return rc;
> >---}
>
> >---/* Disable soft irqs for various locks below. Also
> >--- * stops preemption for RCU.
> >--- */
> >---rcu_read_lock_bh();
>
> but we still see the issue "BUG: sleeping function called from invalid context":
Of course you are if this is the only change you did.
>
> [ 91.015989] BUG: sleeping function called from invalid context at
> include/linux/skbuff.h:786
> [ 91.117096] in_atomic(): 1, irqs_disabled(): 0, pid: 1865, name: NMTX_T1842
> [ 91.200461] Call Trace:
> [ 91.229672] [ec58bbd0] [c000789c] show_stack+0x78/0x18c (unreliable)
> [ 91.305791] [ec58bc10] [c0022900] __might_sleep+0x100/0x118
> [ 91.372524] [ec58bc20] [c029f8d8] dpa_tx+0x128/0x758
Please read again my mail :
I said : "doing GFP_KERNEL allocations in dpa_tx() is wrong, for sure."
I dont have this code, but I suspect it's using : skb_copy(skb,
GFP_KERNEL)
Just say no, use GFP_ATOMIC instead.
Real question is : why skb_copy() is done, since its slow as hell.
> [ 91.431957] [ec58bc80] [c02d78ec] dev_hard_start_xmit+0x424/0x588
> [ 91.504952] [ec58bcc0] [c02d7ab0] dev_queue_xmit+0x60/0x3ac
> [ 91.571692] [ec58bcf0] [c0338d54] packet_sendmsg+0x8c4/0x988
> [ 91.639457] [ec58bd70] [c02c3838] sock_sendmsg+0x90/0xb4
> [ 91.703066] [ec58be40] [c02c4420] sys_sendto+0xdc/0x120
> [ 91.765646] [ec58bf10] [c02c57d0] sys_socketcall+0x148/0x210
> [ 91.833420] [ec58bf40] [c001084c] ret_from_syscall+0x0/0x3c
> [ 91.900153] --- Exception: c01 at 0x4824df00
> [ 91.900157] LR = 0x4828a030
>
^ permalink raw reply
* RE: RFC: top level compatibles for virtual platforms
From: Yoder Stuart-B08248 @ 2011-07-12 14:20 UTC (permalink / raw)
To: Wood Scott-B07421
Cc: Tabi Timur-B04825, Alexander Graf, linuxppc-dev@ozlabs.org,
Gala Kumar-B11780
In-Reply-To: <20110711160646.291e977e@schlenkerla.am.freescale.net>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Monday, July 11, 2011 4:07 PM
> To: Yoder Stuart-B08248
> Cc: Wood Scott-B07421; Tabi Timur-B04825; Grant Likely; Benjamin Herrensc=
hmidt; Gala Kumar-
> B11780; Alexander Graf; linuxppc-dev@ozlabs.org
> Subject: Re: RFC: top level compatibles for virtual platforms
>=20
> On Mon, 11 Jul 2011 15:41:35 -0500
> Yoder Stuart-B08248 <B08248@freescale.com> wrote:
>=20
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Monday, July 11, 2011 1:05 PM
> > >
> > > Just because Linux does it that way now doesn't mean it needs to.
> > > The interrupt controller has a compatible property. Match on it
> > > like any other device. You can find which one is the root interrupt
> > > controller by looking for nodes with the interrupt-controller
> > > property that doesn't have an explicit interrupt-parent (or an interr=
upts property? seems
> to be a conflict between ePAPR and the original interrupt mapping documen=
t).
> >
> > This may be the right long term thing to do, but restructuring how
> > Linux powerpc platforms work is a bigger effort. I was looking for an
> > incremental improvement over what we do now, which is pass a
> > compatible of MPC8544DS and P4080DS for these virtual platforms.
>=20
> A hack is usually easier than doing it right. :-)
>=20
> Though often the effort required for the latter is overstated, and the "r=
ight long term thing"
> never makes the jump to "short term plan".
>=20
> There are a few things that need to be driven off the device tree that cu=
rrently aren't --
> using some mechanism other than the standard device model, if necessary (=
or as a first step) -
> - and then we need a does-nothing default platform as the match of last r=
esort.
>=20
> > However, they _are_ compatible with MPC8544DS and P4080DS so maybe
> > leaving the compatible string alone is ok for now.
>=20
> The virtual platforms are not compatible with MPC8544DS or P4080DS. Only=
a subset of what is
> on those boards is provided. And in the case of direct device assignment=
, often the things
> that are present are incompatible (e.g.
> different type of eTSEC).
Hmm. Perhaps what we need is a real binding that defines specifically
what those compatibles mean. While not identical, a KVM
virtual machine is compatible in certain areas with those
boards.
The ePAPR defines the top level compatible as:
Specifies a list of platform architectures with which this
platform is compatible. This property can be used by
operating systems in selecting platform specific code.
1275 doesn't mention compatible on the root from what I can
see.
Stuart
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/47x: allow kernel to be loaded in higher physical memory
From: Josh Boyer @ 2011-07-12 13:27 UTC (permalink / raw)
To: Suzuki Poulose; +Cc: LinuxPPC-dev
In-Reply-To: <4E12ACD2.9050305@in.ibm.com>
On Tue, Jul 5, 2011 at 2:18 AM, Suzuki Poulose <suzuki@in.ibm.com> wrote:
> On 07/05/11 10:06, Tony Breeds wrote:
>>
>> From: Dave Kleikamp<shaggy@linux.vnet.ibm.com>
>>
>> The 44x code (which is shared by 47x) assumes the available physical
>> memory
>> begins at 0x00000000. =A0This is not necessarily the case in an AMP
>> environment.
>>
>> Support CONFIG_RELOCATABLE for 476 in order to allow the kernel to be
>> loaded into a higher memory range.
>
> I think the code assumes, the kernel is loaded in 256M aligned page. You =
may
> want to mention that in the description here.
Suzie, do you have any other concerns with this code in regards to
your kexec/kdump work for 4xx? It seems fairly self-contained to me,
so I'd like to apply it but I want to make sure it is not going to
majorly conflict with the work you're doing.
josh
^ permalink raw reply
* Re: [PATCH] mm: Fix output of total_ram.
From: Josh Boyer @ 2011-07-12 12:35 UTC (permalink / raw)
To: LinuxPPC-dev, Benjamin Herrenschmidt
In-Reply-To: <20110705044419.GA20597@ozlabs.org>
On Tue, Jul 5, 2011 at 12:44 AM, Tony Breeds <tony@bakeyournoodle.com> wrot=
e:
> On 32bit platforms that support >=3D 4GB memory total_ram was truncated.
> This creates a confusing printk:
> =A0 =A0 =A0 =A0Top of RAM: 0x100000000, Total RAM: 0x0
> Fix that:
> =A0 =A0 =A0 =A0Top of RAM: 0x100000000, Total RAM: 0x100000000
>
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
Acked-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
^ permalink raw reply
* Re: softirqs are invoked while bottom halves are masked
From: David Miller @ 2011-07-12 12:13 UTC (permalink / raw)
To: ronny.meeus
Cc: linuxppc-dev, afleming, patrickdepinguin+linuxppc, eric.dumazet,
netdev
In-Reply-To: <20110712.050817.1253941735409335652.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Tue, 12 Jul 2011 05:08:17 -0700 (PDT)
> From: Ronny Meeus <ronny.meeus@gmail.com>
> Date: Tue, 12 Jul 2011 14:03:04 +0200
>
>> but we still see the issue "BUG: sleeping function called from invalid context":
>>
>> [ 91.015989] BUG: sleeping function called from invalid context at
>> include/linux/skbuff.h:786
>> [ 91.117096] in_atomic(): 1, irqs_disabled(): 0, pid: 1865, name: NMTX_T1842
>> [ 91.200461] Call Trace:
>> [ 91.229672] [ec58bbd0] [c000789c] show_stack+0x78/0x18c (unreliable)
>> [ 91.305791] [ec58bc10] [c0022900] __might_sleep+0x100/0x118
>> [ 91.372524] [ec58bc20] [c029f8d8] dpa_tx+0x128/0x758
>
> Because this dpa driver's transmit method is doing something else that
> is not allowed in software interrupt context.
>
> You must remove all things that might sleep in this driver's
> ->ndo_start_xmit method, and I do mean everything.
Also this whole HW QOS feature bit facility is beyond bogus.
What if the user enables a qdisc that the hardware can't handle, or a
configuration of a hw supported qdisc that the hardware can't support?
What if I have packet classification and packet actions enabled in the
packet scheduler?
These changes are terrible, and we really need you guys to sort out
your problems with these changes yoursleves because your wounds are
entirely self-inflicted and totally not our problem.
^ permalink raw reply
* Re: softirqs are invoked while bottom halves are masked
From: David Miller @ 2011-07-12 12:08 UTC (permalink / raw)
To: ronny.meeus
Cc: linuxppc-dev, afleming, patrickdepinguin+linuxppc, eric.dumazet,
netdev
In-Reply-To: <CAMJ=MEeC1hoqufs7AfFRn3yJoC8mdw7v+14N+7e=wQuJefm4_w@mail.gmail.com>
From: Ronny Meeus <ronny.meeus@gmail.com>
Date: Tue, 12 Jul 2011 14:03:04 +0200
> but we still see the issue "BUG: sleeping function called from invalid context":
>
> [ 91.015989] BUG: sleeping function called from invalid context at
> include/linux/skbuff.h:786
> [ 91.117096] in_atomic(): 1, irqs_disabled(): 0, pid: 1865, name: NMTX_T1842
> [ 91.200461] Call Trace:
> [ 91.229672] [ec58bbd0] [c000789c] show_stack+0x78/0x18c (unreliable)
> [ 91.305791] [ec58bc10] [c0022900] __might_sleep+0x100/0x118
> [ 91.372524] [ec58bc20] [c029f8d8] dpa_tx+0x128/0x758
Because this dpa driver's transmit method is doing something else that
is not allowed in software interrupt context.
You must remove all things that might sleep in this driver's
->ndo_start_xmit method, and I do mean everything.
^ permalink raw reply
* Re: softirqs are invoked while bottom halves are masked (was: Re: [PATCH] [PATCH] Fix deadlock in af_packet while stressing raw ethernet socket interface)
From: Ronny Meeus @ 2011-07-12 12:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: linuxppc-dev, netdev, afleming, Thomas De Schampheleire,
David Miller
In-Reply-To: <1310465411.3314.6.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Tue, Jul 12, 2011 at 12:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wro=
te:
> Le mardi 12 juillet 2011 =E0 11:23 +0200, Thomas De Schampheleire a
> =E9crit :
>> Hi,
>>
>> I'm adding the linuxppc-dev mailing list since this may be pointing to
>> an irq/softirq problem in the powerpc architecture-specific code...
>
>>
>> Note that the reason we are seeing this problem, may be because the
>> kernel we are using contains some patches from Freescale.
>> Specifically, in dev_queue_xmit(), support is added for hardware queue
>> handling, just before entering the rcu_read_lock_bh():
>>
>
> Oh well, what a mess.
>
>> =A0 =A0 =A0 =A0 if (dev->features & NETIF_F_HW_QDISC) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 txq =3D dev_pick_tx(dev, skb);
>
>
>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return dev_hard_start_xmit(skb, dev, txq=
);
> =A0 =A0 =A0 =A0This need to be :
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0local_bh_disable();
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rc =3D dev_hard_start_xmit(skb, dev, txq);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0local_bh_enable();
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return rc;
>
>
>> =A0 =A0 =A0 =A0 }
>>
>> =A0 =A0 =A0 =A0 /* Disable soft irqs for various locks below. Also
>> =A0 =A0 =A0 =A0 =A0* stops preemption for RCU.
>> =A0 =A0 =A0 =A0 =A0*/
>> =A0 =A0 =A0 =A0 rcu_read_lock_bh();
>>
>> We just tried moving the escaping to dev_hard_start_xmit() after
>> taking the lock, but this gives a large number of other problems, e.g.
>>
>> [ =A0 78.662428] BUG: sleeping function called from invalid context at
>> mm/slab.c:3101
>> [ =A0 78.751004] in_atomic(): 1, irqs_disabled(): 0, pid: 1908, name:
>> send_eth_socket
>> [ =A0 78.839582] Call Trace:
>> [ =A0 78.868784] [ec537b70] [c000789c] show_stack+0x78/0x18c (unreliable=
)
>> [ =A0 78.944905] [ec537bb0] [c0022900] __might_sleep+0x100/0x118
>> [ =A0 79.011636] [ec537bc0] [c00facc4] kmem_cache_alloc+0x48/0x118
>> [ =A0 79.080446] [ec537be0] [c02cd0e8] __alloc_skb+0x50/0x130
>> [ =A0 79.144047] [ec537c00] [c02cdf5c] skb_copy+0x44/0xc8
>> [ =A0 79.203478] [ec537c20] [c029f904] dpa_tx+0x154/0x758
>
> doing GFP_KERNEL allocations in dpa_tx() is wrong, for sure.
>
>
>> [ =A0 79.262907] [ec537c80] [c02d78ec] dev_hard_start_xmit+0x424/0x588
>> [ =A0 79.335878] [ec537cc0] [c02d7aac] dev_queue_xmit+0x5c/0x3a4
>> [ =A0 79.402602] [ec537cf0] [c0338d4c] packet_sendmsg+0x8c4/0x988
>> [ =A0 79.470363] [ec537d70] [c02c3838] sock_sendmsg+0x90/0xb4
>> [ =A0 79.533960] [ec537e40] [c02c4420] sys_sendto+0xdc/0x120
>> [ =A0 79.596514] [ec537f10] [c02c57d0] sys_socketcall+0x148/0x210
>> [ =A0 79.664287] [ec537f40] [c001084c] ret_from_syscall+0x0/0x3c
>> [ =A0 79.731015] --- Exception: c01 at 0x48051f00
>> [ =A0 79.731019] =A0 =A0 LR =3D 0x4808e030
>>
>>
>> Note that this may just be the cause for us seeing this problem. If
>> indeed the main problem is irq_exit() invoking softirqs in a locked
>> context, then this patch adding hardware queue support is not really
>> relevant.
>
> irq_exit() is fine. This is because BH are not masked because of the
> Freescale patches.
>
> Really, suggesting an af_packet patch to solve a problem introduced in
> an out of tree patch is insane.
>
> You guys hould have clearly stated you were using an alien kernel.
>
>
>
>
Sorry for not mentioning we were using a patched kernel.
I was not aware that the code involved was patched by the FreeScale
patches we applied. The code found in the stack dumps is not
implemented in FSL specific files.
While reading the code of af_packet I saw that the spin_lock_bh is
used in several places while this is not the case in the tpacket_rcv
function. Since we had a locking issue in that code, I thought that my
patch would be OK.
I was not aware that for that specific function (tpacket_rcv) a
different lock primitive must be used. A suggestion for improvement:
it would be better to document this pre-condition in the code.
After doing the change you proposed our code now looks like:
>---if (dev->features & NETIF_F_HW_QDISC) {
>--->---txq =3D dev_pick_tx(dev, skb);
>--->---local_bh_disable();
>--->---rc =3D dev_hard_start_xmit(skb, dev, txq);
>--->---local_bh_enable();
>--->---return rc;
>---}
>---/* Disable soft irqs for various locks below. Also
>--- * stops preemption for RCU.
>--- */
>---rcu_read_lock_bh();
but we still see the issue "BUG: sleeping function called from invalid cont=
ext":
[ 91.015989] BUG: sleeping function called from invalid context at
include/linux/skbuff.h:786
[ 91.117096] in_atomic(): 1, irqs_disabled(): 0, pid: 1865, name: NMTX_T1=
842
[ 91.200461] Call Trace:
[ 91.229672] [ec58bbd0] [c000789c] show_stack+0x78/0x18c (unreliable)
[ 91.305791] [ec58bc10] [c0022900] __might_sleep+0x100/0x118
[ 91.372524] [ec58bc20] [c029f8d8] dpa_tx+0x128/0x758
[ 91.431957] [ec58bc80] [c02d78ec] dev_hard_start_xmit+0x424/0x588
[ 91.504952] [ec58bcc0] [c02d7ab0] dev_queue_xmit+0x60/0x3ac
[ 91.571692] [ec58bcf0] [c0338d54] packet_sendmsg+0x8c4/0x988
[ 91.639457] [ec58bd70] [c02c3838] sock_sendmsg+0x90/0xb4
[ 91.703066] [ec58be40] [c02c4420] sys_sendto+0xdc/0x120
[ 91.765646] [ec58bf10] [c02c57d0] sys_socketcall+0x148/0x210
[ 91.833420] [ec58bf40] [c001084c] ret_from_syscall+0x0/0x3c
[ 91.900153] --- Exception: c01 at 0x4824df00
[ 91.900157] LR =3D 0x4828a030
The FreeScale patch that introduced this code was created by Andy
Fleming <afleming@freescale.com> (Put in CC).
The purpose of the patch is:
"
Subject: [PATCH] net: Add support for handling queueing in hardware
The QDisc code does a bunch of locking which is unnecessary if
you have hardware which handles all of the queueing. Add
support for this, and skip over all of the queueing code if
the feature is enabled on a given device.
"
Ronny
^ permalink raw reply
* [PATCH] powerpc/44x: Use correct phy-address dt nodes on taishan.dts
From: Stefan Roese @ 2011-07-12 11:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Josh Boyer
Taishan (440GX) has the first PHY (EMAC2) mapped at PHY address 1
and the 2nd PHY (EMAC3) at PHY address 3. Use "phy-address" to
correctly describe this instead of "phy-map".
Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Josh Boyer <jwboyer@linux.vnet.ibm.com>
---
arch/powerpc/boot/dts/taishan.dts | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/boot/dts/taishan.dts b/arch/powerpc/boot/dts/taishan.dts
index 058438f..1657ad0 100644
--- a/arch/powerpc/boot/dts/taishan.dts
+++ b/arch/powerpc/boot/dts/taishan.dts
@@ -337,7 +337,7 @@
rx-fifo-size = <4096>;
tx-fifo-size = <2048>;
phy-mode = "rgmii";
- phy-map = <0x00000001>;
+ phy-address = <1>;
rgmii-device = <&RGMII0>;
rgmii-channel = <0>;
zmii-device = <&ZMII0>;
@@ -361,7 +361,7 @@
rx-fifo-size = <4096>;
tx-fifo-size = <2048>;
phy-mode = "rgmii";
- phy-map = <0x00000003>;
+ phy-address = <3>;
rgmii-device = <&RGMII0>;
rgmii-channel = <1>;
zmii-device = <&ZMII0>;
--
1.7.6
^ permalink raw reply related
* Re: [RFC PATCH 7/7] powerpc: Support RELOCATABLE kernel for PPC44x
From: Suzuki Poulose @ 2011-07-12 11:09 UTC (permalink / raw)
To: Michal Simek; +Cc: tmarri, linuxppc-dev, john.williams, arnd
In-Reply-To: <1308233668-24166-8-git-send-email-monstr@monstr.eu>
On 06/16/11 19:44, Michal Simek wrote:
> Changes:
> - Find out address where kernel runs
> - Create the first 256MB TLB from online detected address
>
> Limitations:
> - Kernel must be aligned to 256MB
>
> Backport:
> - Changes in page.h are backported from newer kernel version
>
> mmu_mapin_ram function has to reflect offset in memory start.
> memstart_addr and kernstart_addr are setup directly from asm
> code to ensure that only ppc44x is affected.
>
> Signed-off-by: Michal Simek<monstr@monstr.eu>
> ---
> arch/powerpc/Kconfig | 3 ++-
> arch/powerpc/include/asm/page.h | 7 ++++++-
> arch/powerpc/kernel/head_44x.S | 28 ++++++++++++++++++++++++++++
> arch/powerpc/mm/44x_mmu.c | 6 +++++-
> 4 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 45c9683..34c521e 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -796,7 +796,8 @@ config LOWMEM_CAM_NUM
>
> config RELOCATABLE
> bool "Build a relocatable kernel (EXPERIMENTAL)"
> - depends on EXPERIMENTAL&& ADVANCED_OPTIONS&& FLATMEM&& FSL_BOOKE
> + depends on EXPERIMENTAL&& ADVANCED_OPTIONS&& FLATMEM
> + depends on FSL_BOOKE || (44x&& !SMP)
> help
> This builds a kernel image that is capable of running at the
> location the kernel is loaded at (some alignment restrictions may
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 4940662..e813cc2 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -108,8 +108,13 @@ extern phys_addr_t kernstart_addr;
> #define pfn_to_kaddr(pfn) __va((pfn)<< PAGE_SHIFT)
> #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr)>> PAGE_SHIFT)
>
> -#define __va(x) ((void *)((unsigned long)(x) + PAGE_OFFSET - MEMORY_START))
> +#ifdef CONFIG_BOOKE
> +#define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) - PHYSICAL_START + KERNELBASE))
> +#define __pa(x) ((unsigned long)(x) + PHYSICAL_START - KERNELBASE)
> +#else
> +#define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + PAGE_OFFSET - MEMORY_START))
> #define __pa(x) ((unsigned long)(x) - PAGE_OFFSET + MEMORY_START)
> +#endif
>
> /*
> * Unfortunately the PLT is in the BSS in the PPC32 ELF ABI,
> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
> index d80ce05..6a63d32 100644
> --- a/arch/powerpc/kernel/head_44x.S
> +++ b/arch/powerpc/kernel/head_44x.S
> @@ -59,6 +59,17 @@ _ENTRY(_start);
> * of abatron_pteptrs
> */
> nop
> +
> +#ifdef CONFIG_RELOCATABLE
> + bl jump /* Find our address */
> + nop
> +jump: mflr r25 /* Make it accessible */
> + /* just for and */
> + lis r26, 0xfffffff0@h
> + ori r26, r26, 0xfffffff0@l
> + and. r21, r25, r26
> +#endif
Hmm. So we are assuming we are running from a 1:1 mapping at the entry.
It is much more safe to read our tlb entry and use the RPN instead.
> +#ifdef CONFIG_RELOCATABLE
> + /* load physical address where kernel runs */
> + mr r4,r21
> +#else
> /* Kernel is at PHYSICAL_START */
> lis r4,PHYSICAL_START@h
> ori r4,r4,PHYSICAL_START@l
> +#endif
>
> /* Load the kernel PID = 0 */
> li r0,0
> @@ -258,6 +274,18 @@ skpinv: addi r4,r4,1 /* Increment */
> mr r5,r29
> mr r6,r28
> mr r7,r27
> +
> +#ifdef CONFIG_RELOCATABLE
> + /* save kernel and memory start */
> + lis r25,kernstart_addr@h
> + ori r25,r25,kernstart_addr@l
> + stw r21,4(r25)
1) You have to use ERPN value in the higher word of kernel_start_addr.
2) You have to account for the (KERNEL_BASE - PAGE_OFFSET) shift for kernel_start_addr.
> +
> + lis r25,memstart_addr@h
> + ori r25,r25,memstart_addr@l
> + stw r21,4(r25)
> +#endif
> +
Suzuki
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox