* Re: [Next] CPU Hotplug test failures on powerpc
From: Sachin Sant @ 2009-12-16 6:41 UTC (permalink / raw)
To: Xiaotian Feng
Cc: Peter Zijlstra, linux-kernel, Linux/PPC Development, linux-next,
Ingo Molnar
In-Reply-To: <7b6bb4a50912152225p4f5dde13re83c439407c16eaf@mail.gmail.com>
Xiaotian Feng wrote:
> Does this testcase hotplug cpu 0 off?
>
No, i don't think so. It skips cpu0 during online/offline
process.
thanks
-Sachin
--
---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------
^ permalink raw reply
* Re: [Next] CPU Hotplug test failures on powerpc
From: Xiaotian Feng @ 2009-12-16 6:25 UTC (permalink / raw)
To: Sachin Sant
Cc: Peter Zijlstra, linux-kernel, Linux/PPC Development, linux-next,
Ingo Molnar
In-Reply-To: <4B2224C7.1020908@in.ibm.com>
On Fri, Dec 11, 2009 at 6:53 PM, Sachin Sant <sachinp@in.ibm.com> wrote:
> While executing cpu_hotplug(from autotest) tests against latest
> next on a power6 box, the machine locks up. A soft reset shows
> the following trace
>
> cpu 0x0: Vector: 100 (System Reset) at [c00000000c9333d0]
> =C2=A0 pc: c0000000003433d8: .find_next_bit+0x54/0xc4
> =C2=A0 lr: c000000000342f10: .cpumask_next_and+0x4c/0x94
> =C2=A0 sp: c00000000c933650
> =C2=A0msr: 8000000000089032
> =C2=A0current =3D 0xc00000000c173840
> =C2=A0paca =C2=A0 =C2=A0=3D 0xc000000000bc2600
> =C2=A0 pid =C2=A0 =3D 2602, comm =3D hotplug06.top.s
> enter ? for help
> [link register =C2=A0 ] c000000000342f10 .cpumask_next_and+0x4c/0x94
> [c00000000c933650] c0000000000e9f34 .cpuset_cpus_allowed_locked+0x38/0x74
> (unreliable)
> [c00000000c9336e0] c000000000090074 .move_task_off_dead_cpu+0xc4/0x1ac
> [c00000000c9337a0] c0000000005e4e5c .migration_call+0x304/0x830
> [c00000000c933880] c0000000005e0880 .notifier_call_chain+0x68/0xe0
> [c00000000c933920] c00000000012a92c ._cpu_down+0x210/0x34c
> [c00000000c933a90] c00000000012aad8 .cpu_down+0x70/0xa8
> [c00000000c933b20] c000000000525940 .store_online+0x54/0x894
> [c00000000c933bb0] c000000000463430 .sysdev_store+0x3c/0x50
> [c00000000c933c20] c0000000001f8320 .sysfs_write_file+0x124/0x18c
> [c00000000c933ce0] c00000000017edac .vfs_write+0xd4/0x1fc
> [c00000000c933d80] c00000000017efdc .SyS_write+0x58/0xa0
> [c00000000c933e30] c0000000000085b4 syscall_exit+0x0/0x40
> --- Exception: c01 (System Call) at 00000fff9fa8a8f8
> SP (fffe7aef200) is in userspace
> 0:mon> e
> cpu 0x0: Vector: 100 (System Reset) at [c00000000c9333d0]
> =C2=A0 pc: c0000000003433d8: .find_next_bit+0x54/0xc4
> =C2=A0 lr: c000000000342f10: .cpumask_next_and+0x4c/0x94
> =C2=A0 sp: c00000000c933650
> =C2=A0msr: 8000000000089032
> =C2=A0current =3D 0xc00000000c173840
> =C2=A0paca =C2=A0 =C2=A0=3D 0xc000000000bc2600
> =C2=A0 pid =C2=A0 =3D 2602, comm =3D hotplug06.top.s
>
Does this testcase hotplug cpu 0 off?
> Last few messages from the dmesg log shows
>
> 0:mon> <4>IRQ 17 affinity broken off cpu 0
> <4>IRQ 18 affinity broken off cpu 0
> <4>IRQ 19 affinity broken off cpu 0
> <4>IRQ 264 affinity broken off cpu 0
> <4>cpu 0 (hwid 0) Ready to die...
> <7>clockevent: decrementer mult[83126e97] shift[32] cpu[0]
> <4>Processor 0 found.
> <4>IRQ 17 affinity broken off cpu 1
> <4>IRQ 18 affinity broken off cpu 1
> <4>IRQ 19 affinity broken off cpu 1
> <4>IRQ 264 affinity broken off cpu 1
> <4>cpu 1 (hwid 1) Ready to die...
> <7>clockevent: decrementer mult[83126e97] shift[32] cpu[1]
> <4>Processor 1 found.
> <4>cpu 1 (hwid 1) Ready to die...
> <7>clockevent: decrementer mult[83126e97] shift[32] cpu[1]
> <4>Processor 1 found.
> <4>cpu 1 (hwid 1) Ready to die...
> <6>process 2423 (bash) no longer affine to cpu1
> <7>clockevent: decrementer mult[83126e97] shift[32] cpu[1]
> <4>Processor 1 found.
> <4>cpu 1 (hwid 1) Ready to die...
> <7>clockevent: decrementer mult[83126e97] shift[32] cpu[1]
> <4>Processor 1 found.
> <4>cpu 1 (hwid 1) Ready to die...
> <7>clockevent: decrementer mult[83126e97] shift[32] cpu[1]
> <4>Processor 1 found.
> <4>cpu 1 (hwid 1) Ready to die...
> <3>INFO: RCU detected CPU 0 stall (t=3D1000 jiffies)
> <3>INFO: RCU detected CPU 0 stall (t=3D4000 jiffies)
> 0:mon>
>
> After some debugging a possible suspect seems to be commit
> 6ad4c18.. : sched: Fix balance vs hotplug race
>
> If i revert this patch i am able to execute the tests on this
> power6 without any issues.
> But at the same time the above patch is required to solve the
> cpu hotplug related race on x86_64(as a side note this same
> x86_64 issue can be recreated against latest Linus git as well)
> that i reported here :
>
> http://marc.info/?l=3Dlinux-kernel&m=3D125802682922299&w=3D2
>
> I will try few more iterations with and without the above
> patch just to make sure i have the correct results.
>
> If someone has a suggestion let me know.
>
> Thanks
> -Sachin
>
>
> --
>
> ---------------------------------
> Sachin Sant
> IBM Linux Technology Center
> India Systems and Technology Labs
> Bangalore, India
> ---------------------------------
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" i=
n
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at =C2=A0http://www.tux.org/lkml/
>
^ permalink raw reply
* Re: [Next] CPU Hotplug test failures on powerpc
From: Sachin Sant @ 2009-12-16 5:38 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, linux-next, linux-kernel, Linux/PPC Development
In-Reply-To: <1260889402.4165.434.camel@twins>
Peter Zijlstra wrote:
> Could you try the below?
>
No luck. Still the same issue. The mask values don't change.
Thanks
-Sachin
> ---
> init/main.c | 7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index 4051d75..4be7de2 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -369,12 +369,6 @@ static void __init smp_init(void)
> {
> unsigned int cpu;
>
> - /*
> - * Set up the current CPU as possible to migrate to.
> - * The other ones will be done by cpu_up/cpu_down()
> - */
> - set_cpu_active(smp_processor_id(), true);
> -
> /* FIXME: This should be done in userspace --RR */
> for_each_present_cpu(cpu) {
> if (num_online_cpus() >= setup_max_cpus)
> @@ -486,6 +480,7 @@ static void __init boot_cpu_init(void)
> int cpu = smp_processor_id();
> /* Mark the boot cpu "present", "online" etc for SMP and UP case */
> set_cpu_online(cpu, true);
> + set_cpu_active(cpu, true);
> set_cpu_present(cpu, true);
> set_cpu_possible(cpu, true);
> }
>
>
>
--
---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------
^ permalink raw reply
* [PATCH -tip tracing/kprobes] PPC: Powerpc port of the kprobe-based event tracer
From: Mahesh Salgaonkar @ 2009-12-16 4:39 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Masami Hiramatsu, Mahesh Salgaonkar
In-Reply-To: <20091216043619.963539987@mars.in.ibm.com>
This patch ports the kprobe-based event tracer to powerpc. This patch
is based in x86 port. This brings powerpc on par with x86.
Port the following API's to ppc for accessing registers and stack entries
from pt_regs.
- regs_query_register_offset(const char *name)
Query the offset of "name" register.
- regs_query_register_name(unsigned int offset)
Query the name of register by its offset.
- regs_get_register(struct pt_regs *regs, unsigned int offset)
Get the value of a register by its offset.
- regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr)
Check the address is in the kernel stack.
- regs_get_kernel_stack_nth(struct pt_regs *reg, unsigned int nth)
Get Nth entry of the kernel stack. (N >= 0)
- regs_get_argument_nth(struct pt_regs *reg, unsigned int nth)
Get Nth argument at function call. (N >= 0)
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhiramat@redhat.com>
---
arch/powerpc/include/asm/ptrace.h | 64 +++++++++++++++++
arch/powerpc/kernel/ptrace.c | 141 ++++++++++++++++++++++++++++++++++++++
kernel/trace/Kconfig | 2
3 files changed, 206 insertions(+), 1 deletion(-)
Index: linux-2.6-tip/arch/powerpc/include/asm/ptrace.h
===================================================================
--- linux-2.6-tip.orig/arch/powerpc/include/asm/ptrace.h
+++ linux-2.6-tip/arch/powerpc/include/asm/ptrace.h
@@ -83,6 +83,7 @@ struct pt_regs {
#define instruction_pointer(regs) ((regs)->nip)
#define user_stack_pointer(regs) ((regs)->gpr[1])
+#define kernel_stack_pointer(regs) ((regs)->gpr[1])
#define regs_return_value(regs) ((regs)->gpr[3])
#ifdef CONFIG_SMP
@@ -131,6 +132,69 @@ do { \
} while (0)
#endif /* __powerpc64__ */
+/* Query offset/name of register from its name/offset */
+#include <linux/stddef.h>
+#include <linux/thread_info.h>
+extern int regs_query_register_offset(const char *name);
+extern const char *regs_query_register_name(unsigned int offset);
+/* Get Nth argument at function call */
+extern unsigned long regs_get_argument_nth(struct pt_regs *regs,
+ unsigned int n);
+#define MAX_REG_OFFSET (offsetof(struct pt_regs, result))
+
+/**
+ * regs_get_register() - get register value from its offset
+ * @regs: pt_regs from which register value is gotten
+ * @offset: offset number of the register.
+ *
+ * regs_get_register returns the value of a register whose offset from @regs.
+ * The @offset is the offset of the register in struct pt_regs.
+ * If @offset is bigger than MAX_REG_OFFSET, this returns 0.
+ */
+static inline unsigned long regs_get_register(struct pt_regs *regs,
+ unsigned int offset)
+{
+ if (unlikely(offset > MAX_REG_OFFSET))
+ return 0;
+ return *(unsigned long *)((unsigned long)regs + offset);
+}
+
+/**
+ * regs_within_kernel_stack() - check the address in the stack
+ * @regs: pt_regs which contains kernel stack pointer.
+ * @addr: address which is checked.
+ *
+ * regs_within_kernel_stack() checks @addr is within the kernel stack page(s).
+ * If @addr is within the kernel stack, it returns true. If not, returns false.
+ */
+
+static inline bool regs_within_kernel_stack(struct pt_regs *regs,
+ unsigned long addr)
+{
+ return ((addr & ~(THREAD_SIZE - 1)) ==
+ (kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1)));
+}
+
+/**
+ * regs_get_kernel_stack_nth() - get Nth entry of the stack
+ * @regs: pt_regs which contains kernel stack pointer.
+ * @n: stack entry number.
+ *
+ * regs_get_kernel_stack_nth() returns @n th entry of the kernel stack which
+ * is specified by @regs. If the @n th entry is NOT in the kernel stack,
+ * this returns 0.
+ */
+static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
+ unsigned int n)
+{
+ unsigned long *addr = (unsigned long *)kernel_stack_pointer(regs);
+ addr += n;
+ if (regs_within_kernel_stack(regs, (unsigned long)addr))
+ return *addr;
+ else
+ return 0;
+}
+
/*
* These are defined as per linux/ptrace.h, which see.
*/
Index: linux-2.6-tip/arch/powerpc/kernel/ptrace.c
===================================================================
--- linux-2.6-tip.orig/arch/powerpc/kernel/ptrace.c
+++ linux-2.6-tip/arch/powerpc/kernel/ptrace.c
@@ -39,6 +39,147 @@
#include <asm/system.h>
/*
+ * The parameter save area on the stack is used to store arguments being passed
+ * to callee function and is located at fixed offset from stack pointer.
+ */
+#ifdef CONFIG_PPC32
+#define PARAMETER_SAVE_AREA_OFFSET 24 /* bytes */
+#else /* CONFIG_PPC32 */
+#define PARAMETER_SAVE_AREA_OFFSET 48 /* bytes */
+#endif
+
+struct pt_regs_offset {
+ const char *name;
+ int offset;
+};
+
+#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs, r)}
+#define REG_OFFSET_END {.name = NULL, .offset = 0}
+
+static const struct pt_regs_offset regoffset_table[] = {
+ REG_OFFSET_NAME(gpr[0]),
+ REG_OFFSET_NAME(gpr[1]),
+ REG_OFFSET_NAME(gpr[2]),
+ REG_OFFSET_NAME(gpr[3]),
+ REG_OFFSET_NAME(gpr[4]),
+ REG_OFFSET_NAME(gpr[5]),
+ REG_OFFSET_NAME(gpr[6]),
+ REG_OFFSET_NAME(gpr[7]),
+ REG_OFFSET_NAME(gpr[8]),
+ REG_OFFSET_NAME(gpr[9]),
+ REG_OFFSET_NAME(gpr[10]),
+ REG_OFFSET_NAME(gpr[11]),
+ REG_OFFSET_NAME(gpr[12]),
+ REG_OFFSET_NAME(gpr[13]),
+ REG_OFFSET_NAME(gpr[14]),
+ REG_OFFSET_NAME(gpr[15]),
+ REG_OFFSET_NAME(gpr[16]),
+ REG_OFFSET_NAME(gpr[17]),
+ REG_OFFSET_NAME(gpr[18]),
+ REG_OFFSET_NAME(gpr[19]),
+ REG_OFFSET_NAME(gpr[20]),
+ REG_OFFSET_NAME(gpr[21]),
+ REG_OFFSET_NAME(gpr[22]),
+ REG_OFFSET_NAME(gpr[23]),
+ REG_OFFSET_NAME(gpr[24]),
+ REG_OFFSET_NAME(gpr[25]),
+ REG_OFFSET_NAME(gpr[26]),
+ REG_OFFSET_NAME(gpr[27]),
+ REG_OFFSET_NAME(gpr[28]),
+ REG_OFFSET_NAME(gpr[29]),
+ REG_OFFSET_NAME(gpr[30]),
+ REG_OFFSET_NAME(gpr[31]),
+ REG_OFFSET_NAME(nip),
+ REG_OFFSET_NAME(msr),
+ REG_OFFSET_NAME(orig_gpr3),
+ REG_OFFSET_NAME(ctr),
+ REG_OFFSET_NAME(link),
+ REG_OFFSET_NAME(xer),
+ REG_OFFSET_NAME(ccr),
+#ifdef CONFIG_PPC64
+ REG_OFFSET_NAME(softe),
+#else
+ REG_OFFSET_NAME(mq),
+#endif
+ REG_OFFSET_NAME(trap),
+ REG_OFFSET_NAME(dar),
+ REG_OFFSET_NAME(dsisr),
+ REG_OFFSET_NAME(result),
+ REG_OFFSET_END,
+};
+
+/**
+ * regs_query_register_offset() - query register offset from its name
+ * @name: the name of a register
+ *
+ * regs_query_register_offset() returns the offset of a register in struct
+ * pt_regs from its name. If the name is invalid, this returns -EINVAL;
+ */
+int regs_query_register_offset(const char *name)
+{
+ const struct pt_regs_offset *roff;
+ for (roff = regoffset_table; roff->name != NULL; roff++)
+ if (!strcmp(roff->name, name))
+ return roff->offset;
+ return -EINVAL;
+}
+
+/**
+ * regs_query_register_name() - query register name from its offset
+ * @offset: the offset of a register in struct pt_regs.
+ *
+ * regs_query_register_name() returns the name of a register from its
+ * offset in struct pt_regs. If the @offset is invalid, this returns NULL;
+ */
+const char *regs_query_register_name(unsigned int offset)
+{
+ const struct pt_regs_offset *roff;
+ for (roff = regoffset_table; roff->name != NULL; roff++)
+ if (roff->offset == offset)
+ return roff->name;
+ return NULL;
+}
+
+static const int arg_offs_table[] = {
+ [0] = offsetof(struct pt_regs, gpr[3]),
+ [1] = offsetof(struct pt_regs, gpr[4]),
+ [2] = offsetof(struct pt_regs, gpr[5]),
+ [3] = offsetof(struct pt_regs, gpr[6]),
+ [4] = offsetof(struct pt_regs, gpr[7]),
+ [5] = offsetof(struct pt_regs, gpr[8]),
+ [6] = offsetof(struct pt_regs, gpr[9]),
+ [7] = offsetof(struct pt_regs, gpr[10])
+};
+
+/**
+ * regs_get_argument_nth() - get Nth argument at function call
+ * @regs: pt_regs which contains registers at function entry.
+ * @n: argument number.
+ *
+ * regs_get_argument_nth() returns @n th argument of a function call.
+ * Since usually the kernel stack will be changed right after function entry,
+ * you must use this at function entry. If the @n th entry is NOT in the
+ * kernel stack or pt_regs, this returns 0.
+ */
+unsigned long regs_get_argument_nth(struct pt_regs *regs, unsigned int n)
+{
+ if (n < ARRAY_SIZE(arg_offs_table))
+ return *(unsigned long *)((char *)regs + arg_offs_table[n]);
+ else {
+ /*
+ * If more arguments are passed that can be stored in
+ * registers, the remaining arguments are stored in the
+ * parameter save area located at fixed offset from stack
+ * pointer.
+ * Following the PowerPC ABI, the first few arguments are
+ * actually passed in registers (r3-r10), with equivalent space
+ * left unused in the parameter save area.
+ */
+ n += (PARAMETER_SAVE_AREA_OFFSET / sizeof(unsigned long));
+ return regs_get_kernel_stack_nth(regs, n);
+ }
+}
+/*
* does not yet catch signals sent when the child dies.
* in exit.c or in signal.c.
*/
Index: linux-2.6-tip/kernel/trace/Kconfig
===================================================================
--- linux-2.6-tip.orig/kernel/trace/Kconfig
+++ linux-2.6-tip/kernel/trace/Kconfig
@@ -464,7 +464,7 @@ config BLK_DEV_IO_TRACE
config KPROBE_EVENT
depends on KPROBES
- depends on X86
+ depends on X86 || PPC
bool "Enable kprobes-based dynamic events"
select TRACING
default y
^ permalink raw reply
* Re: [POWERPC] add U-Boot bootcount driver.
From: David Gibson @ 2009-12-16 0:02 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <20091216024730.455b90fd@vitb-lp>
On Wed, Dec 16, 2009 at 02:47:30AM +0300, Vitaly Bordug wrote:
>
> From: Heiko Schocher <hs@denx.de>
>
> This driver provides (read/write) access to the
> U-Boot bootcounter via PROC FS or sysFS.
>
> in u-boot, it uses a 8 byte mem area (it must hold the value over a
> soft reset of course), for storing a bootcounter (it counts many soft
> resets are done, on hard reset it starts with 0). If the bootcountvalue
> exceeds the value in the env variable "bootlimit", and alternative
> bootcmd stored in the env variable "altbootcmd" is run.
>
> The bootcountregister gets configured via DTS.
> for example on the mgsuvd board:
>
> bootcount@0x3eb0 {
> device_type = "bootcount";
No device_type.
> compatible = "uboot,bootcount";
> reg = <0x3eb0 0x08>;
> };
This area should also be in the flattened tree's reserved map.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* [POWERPC] add U-Boot bootcount driver.
From: Vitaly Bordug @ 2009-12-15 23:47 UTC (permalink / raw)
To: linuxppc-dev
From: Heiko Schocher <hs@denx.de>
This driver provides (read/write) access to the
U-Boot bootcounter via PROC FS or sysFS.
in u-boot, it uses a 8 byte mem area (it must hold the value over a
soft reset of course), for storing a bootcounter (it counts many soft
resets are done, on hard reset it starts with 0). If the bootcountvalue
exceeds the value in the env variable "bootlimit", and alternative
bootcmd stored in the env variable "altbootcmd" is run.
The bootcountregister gets configured via DTS.
for example on the mgsuvd board:
bootcount@0x3eb0 {
device_type = "bootcount";
compatible = "uboot,bootcount";
reg = <0x3eb0 0x08>;
};
This driver is tested on the mgcoge(82xx) and mgsuvd(8xx) board.
Signed-off-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Wolfgang Denk <wd@denx.de>
Signed-off-by: Vitaly Bordyug <vitb@kernel.crashing.org>
---
I think there is no reason not to have this in mainline. Thoughts? And
I'm not sure what is right direction to push this - it's representation
of u-boot feature in fact, pretty useful tho.
arch/powerpc/boot/dts/mgcoge.dts | 6 +
arch/powerpc/boot/dts/mgsuvd.dts | 6 +
arch/powerpc/configs/mgsuvd_defconfig | 1
drivers/char/Kconfig | 6 +
drivers/char/Makefile | 1
drivers/char/bootcount.c | 222
+++++++++++++++++++++++++++++++++ 6 files changed, 242 insertions(+),
0 deletions(-) create mode 100644 drivers/char/bootcount.c
diff --git a/arch/powerpc/boot/dts/mgcoge.dts
b/arch/powerpc/boot/dts/mgcoge.dts index 0ce9664..7136fee 100644
--- a/arch/powerpc/boot/dts/mgcoge.dts
+++ b/arch/powerpc/boot/dts/mgcoge.dts
@@ -215,6 +215,12 @@
linux,network-index = <2>;
fsl,cpm-command = <0x16200300>;
};
+
+ bootcount@0x80f4 {
+ device_type = "bootcount";
+ compatible = "uboot,bootcount";
+ reg = <0x80f4 0x08>;
+ };
};
PIC: interrupt-controller@10c00 {
diff --git a/arch/powerpc/boot/dts/mgsuvd.dts
b/arch/powerpc/boot/dts/mgsuvd.dts index e4fc53a..59216a7 100644
--- a/arch/powerpc/boot/dts/mgsuvd.dts
+++ b/arch/powerpc/boot/dts/mgsuvd.dts
@@ -158,6 +158,12 @@
fsl,cpm-command = <0x80>;
fixed-link = <0 0 10 0 0>;
};
+
+ bootcount@0x3eb0 {
+ device_type = "bootcount";
+ compatible = "uboot,bootcount";
+ reg = <0x3eb0 0x08>;
+ };
};
};
};
diff --git a/arch/powerpc/configs/mgsuvd_defconfig
b/arch/powerpc/configs/mgsuvd_defconfig index 43c3c4f..298ced6 100644
--- a/arch/powerpc/configs/mgsuvd_defconfig
+++ b/arch/powerpc/configs/mgsuvd_defconfig
@@ -626,6 +626,7 @@ CONFIG_GEN_RTC=y
# CONFIG_GEN_RTC_X is not set
# CONFIG_R3964 is not set
# CONFIG_RAW_DRIVER is not set
+CONFIG_BOOTCOUNT=y
# CONFIG_TCG_TPM is not set
# CONFIG_I2C is not set
# CONFIG_SPI is not set
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 6aad99e..5fe2d6c 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -1049,6 +1049,12 @@ config MAX_RAW_DEVS
Default is 256. Increase this number in case you need lots of
raw devices.
+config BOOTCOUNT
+ tristate "UBoot Bootcount driver"
+ depends on PPC
+ help
+ The UBoot Bootcount driver ...
+
config HPET
bool "HPET - High Precision Event Timer" if (X86 || IA64)
default n
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index 19a79dd..e3e2b1a 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -98,6 +98,7 @@ obj-$(CONFIG_NSC_GPIO) += nsc_gpio.o
obj-$(CONFIG_CS5535_GPIO) += cs5535_gpio.o
obj-$(CONFIG_GPIO_TB0219) += tb0219.o
obj-$(CONFIG_TELCLOCK) += tlclk.o
+obj-$(CONFIG_BOOTCOUNT) += bootcount.o
obj-$(CONFIG_MWAVE) += mwave/
obj-$(CONFIG_AGP) += agp/
diff --git a/drivers/char/bootcount.c b/drivers/char/bootcount.c
new file mode 100644
index 0000000..62fcabd
--- /dev/null
+++ b/drivers/char/bootcount.c
@@ -0,0 +1,222 @@
+/*
+ * This driver gives access(read/write) to the bootcounter used by
u-boot.
+ * Access is supported via procFS and sysFS.
+ *
+ * Copyright 2008 DENX Software Engineering GmbH
+ * Author: Heiko Schocher <hs@denx.de>
+ * Based on work from: Steffen Rumler (Steffen.Rumler@siemens.com)
+ *
+ * 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; either version 2 of the License, or (at
your
+ * option) any later version.
+ */
+
+#include <linux/mm.h>
+#include <linux/mman.h>
+#include <linux/init.h>
+#include <linux/capability.h>
+#include <linux/ptrace.h>
+#include <linux/device.h>
+
+#include <asm/uaccess.h>
+#include <asm/io.h>
+
+#include <linux/of_platform.h>
+
+#ifndef CONFIG_PROC_FS
+#error "PROC FS support must be switched-on"
+#endif
+
+
+#define UBOOT_BOOTCOUNT_MAGIC_OFFSET 0x04 /*
offset of magic number */ +#define
UBOOT_BOOTCOUNT_MAGIC 0xB001C041 /* magic number
value */ + +#define
UBOOT_BOOTCOUNT_PROC_ENTRY "driver/bootcount" /* PROC FS
entry under '/proc' */ + +/*
+ * This macro frees the machine specific function from bounds checking
and
+ * this like that...
+ */
+#define PRINT_PROC(fmt,args...) \
+ do { \
+ *len += sprintf( buffer+*len, fmt, ##args ); \
+ if (*begin + *len > offset + size) \
+ return( 0 ); \
+ if (*begin + *len < offset) { \
+ *begin += *len; \
+ *len = 0; \
+ } \
+ } while(0)
+
+void __iomem *mem = NULL;
+/*
+ * read U-Boot bootcounter
+ */
+static int
+read_bootcounter_info(char *buffer, int *len, off_t * begin, off_t
offset,
+ int size)
+{
+ unsigned long magic;
+ unsigned long counter;
+
+
+ magic = *((unsigned long *) (mem +
UBOOT_BOOTCOUNT_MAGIC_OFFSET));
+ counter = *((unsigned long *) (mem));
+
+ if (magic == UBOOT_BOOTCOUNT_MAGIC) {
+ PRINT_PROC ("%lu\n", counter);
+ } else {
+ PRINT_PROC ("bad magic: 0x%lu != 0x%lu\n", magic,
+ (unsigned long)UBOOT_BOOTCOUNT_MAGIC);
+ }
+
+ return 1;
+}
+
+/*
+ * read U-Boot bootcounter (wrapper)
+ */
+static int
+read_bootcounter(char *buffer, char **start, off_t offset, int size,
+ int *eof, void *arg)
+{
+ int len = 0;
+ off_t begin = 0;
+
+
+ *eof = read_bootcounter_info(buffer, &len, &begin, offset,
size); +
+ if (offset >= begin + len)
+ return 0;
+
+ *start = buffer + (offset - begin);
+ return size < begin + len - offset ? size : begin + len -
offset; +}
+
+/*
+ * write new value to U-Boot bootcounter
+ */
+static int
+write_bootcounter(struct file *file, const char *buffer, unsigned long
count,
+ void *data)
+{
+ unsigned long magic;
+ unsigned long *counter_ptr;
+
+
+ magic = *((unsigned long *) (mem +
UBOOT_BOOTCOUNT_MAGIC_OFFSET));
+ counter_ptr = (unsigned long *) (mem);
+
+ if (magic == UBOOT_BOOTCOUNT_MAGIC)
+ *counter_ptr = simple_strtol(buffer, NULL, 10);
+ else
+ return -EINVAL;
+
+ return count;
+}
+
+/* helper for the sysFS */
+static int show_str_bootcount(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ int ret = 0;
+ off_t begin = 0;
+
+ read_bootcounter_info(buf, &ret, &begin, 0, 20);
+ return ret;
+}
+static int store_str_bootcount(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ write_bootcounter(NULL, buf, count, NULL);
+ return count;
+}
+static DEVICE_ATTR(bootcount, S_IWUSR | S_IRUGO, show_str_bootcount,
store_str_bootcount); +
+static int __devinit bootcount_probe(struct of_device *ofdev,
+ const struct of_device_id
*match) +{
+ struct device_node *np = NULL;
+ struct proc_dir_entry *bootcount;
+
+ printk("%s (%d) %s: ", __FILE__, __LINE__, __FUNCTION__);
+ np = of_find_node_by_type(np, "bootcount");
+ if (!np) {
+ printk("%s no node, trying compatible node\n",
__FUNCTION__);
+ np = of_find_compatible_node(NULL, NULL,
"uboot,bootcount");
+ if (!np) {
+ printk("%s no node\n", __FUNCTION__);
+ return -ENODEV;
+ }
+ }
+ mem = of_iomap(np, 0);
+ if (mem == NULL) {
+ printk("%s couldnt map register.\n", __FUNCTION__);
+ }
+
+ /* init ProcFS */
+ if ((bootcount =
+ create_proc_entry(UBOOT_BOOTCOUNT_PROC_ENTRY, 0600,
+ NULL)) == NULL) {
+
+ printk(KERN_ERR "\n%s (%d): cannot create /proc/%s\n",
+ __FILE__, __LINE__,
UBOOT_BOOTCOUNT_PROC_ENTRY);
+ } else {
+
+ bootcount->read_proc = read_bootcounter;
+ bootcount->write_proc = write_bootcounter;
+ printk("created \"/proc/%s\"\n",
UBOOT_BOOTCOUNT_PROC_ENTRY);
+ }
+
+ if (device_create_file(&ofdev->dev, &dev_attr_bootcount))
+ printk("%s couldnt register sysFS entry.\n",
__FUNCTION__); +
+ return 0;
+}
+
+static int bootcount_remove(struct of_device *ofdev)
+{
+ BUG();
+ return 0;
+}
+
+static const struct of_device_id bootcount_match[] = {
+ {
+ .compatible = "uboot,bootcount",
+ },
+ {},
+};
+
+static struct of_platform_driver bootcount_driver = {
+ .driver = {
+ .name = "bootcount",
+ },
+ .match_table = bootcount_match,
+ .probe = bootcount_probe,
+ .remove = bootcount_remove,
+};
+
+
+static int __init uboot_bootcount_init(void)
+{
+ of_register_platform_driver(&bootcount_driver);
+ return 0;
+}
+
+static void __exit uboot_bootcount_cleanup(void)
+{
+ if (mem != NULL)
+ iounmap(mem);
+ remove_proc_entry(UBOOT_BOOTCOUNT_PROC_ENTRY, NULL);
+}
+
+module_init(uboot_bootcount_init);
+module_exit(uboot_bootcount_cleanup);
+
+MODULE_LICENSE ("GPL");
+MODULE_AUTHOR ("Steffen Rumler <steffen.rumler@siemens.com>");
+MODULE_DESCRIPTION ("Provide (read/write) access to the U-Boot
bootcounter via PROC FS");
^ permalink raw reply related
* [PATCH] powerpc/85xx: Fix oops during MSI driver probe on MPC85xxMDS boards
From: Anton Vorontsov @ 2009-12-15 22:58 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
MPC85xx chips report the wrong value in feature reporting register,
and that causes the following oops:
Unable to handle kernel paging request for data at address 0x00000c00
Faulting instruction address: 0xc0019294
Oops: Kernel access of bad area, sig: 11 [#1]
MPC8569 MDS
Modules linked in:
[...]
NIP [c0019294] mpic_set_irq_type+0x2f0/0x368
LR [c0019124] mpic_set_irq_type+0x180/0x368
Call Trace:
[ef851d60] [c0019124] mpic_set_irq_type+0x180/0x368 (unreliable)
[ef851d90] [c007958c] __irq_set_trigger+0x44/0xd4
[ef851db0] [c007b550] set_irq_type+0x40/0x7c
[ef851dc0] [c0004a60] irq_create_of_mapping+0xb4/0x114
[ef851df0] [c0004af0] irq_of_parse_and_map+0x30/0x40
[ef851e20] [c0405678] fsl_of_msi_probe+0x1a0/0x328
[ef851e60] [c02e6438] of_platform_device_probe+0x5c/0x84
[...]
This is because mpic_alloc() assigns wrong values to
mpic->isu_{size,shift,mask}, and things eventually break when
_mpic_irq_read() is trying to use them.
This patch fixes the issue by enabling MPIC_BROKEN_FRR_NIRQS quirk.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index c5028a2..6491f7c 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -338,7 +338,8 @@ static void __init mpc85xx_mds_pic_init(void)
}
mpic = mpic_alloc(np, r.start,
- MPIC_PRIMARY | MPIC_WANTS_RESET | MPIC_BIG_ENDIAN,
+ MPIC_PRIMARY | MPIC_WANTS_RESET | MPIC_BIG_ENDIAN |
+ MPIC_BROKEN_FRR_NIRQS,
0, 256, " OpenPIC ");
BUG_ON(mpic == NULL);
of_node_put(np);
--
1.6.3.3
^ permalink raw reply related
* Problem with mini-PCI-E slot on P2020RDB
From: Felix Radensky @ 2009-12-15 21:25 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org, Aggrwal Poonam-B10812, Kumar Gala
Hi,
I'm trying to use mini-PCI-E WLAN card on P2020RDB running 2.6.32, but
so far without success.
ath9k driver identifies the device, I can run ifconfig, iwconfig and
hostapd on wlan0, but device is not
getting any interrupts, so I suspect the interrupt configuration is
wrong. Atheros ath9k driver reports:
phy0: Atheros AR9280 MAC/BB Rev:2 AR5133 RF Rev:d0: mem=0xf1060000, irq=16
The mapping for irq 16 is:
irq: irq 1 on host /soc@ffe00000/pic@40000 mapped to virtual irq 16
According to /proc/interrupts:
CPU0
16: 0 OpenPIC Edge ath9k
The same problem happens if Atheros card is plugged (with adapter) into
regular PCI-E slot.
It seems that p2020rdb device tree is missing interrupt-map-mask and
interrupt-map properties
in PCI-E nodes.
I've tried running kernel from latest FSL BSP for this board (based on
2.6.32-rc3). The device tree
has the interrupt-map-mask and interrupt-map properties, and interrupt
mapping is different:
irq: irq 0 on host /soc@ffe00000/pic@40000 mapped to virtual irq 16
In /proc/interrups I see
CPU0
16: 100001 OpenPIC Level ath9k
However, when ath9k driver is loaded I get this:
irq 16: nobody cared (try booting with the "irqpoll" option)
Call Trace:
[efbefa40] [c00074b0] show_stack+0x4c/0x16c (unreliable)
[efbefa70] [c0073970] __report_bad_irq+0x38/0xd0
[efbefa90] [c0073bd4] note_interrupt+0x1cc/0x22c
[efbefac0] [c00747d0] handle_fasteoi_irq+0xf4/0x128
[efbefae0] [c0004eb8] do_IRQ+0xc8/0xf4
[efbefb00] [c001081c] ret_from_except+0x0/0x18
[efbefbc0] [00000000] (null)
[efbefc10] [c0004d24] do_softirq+0x60/0x64
[efbefc20] [c0044670] irq_exit+0x88/0xa8
[efbefc30] [c0004ebc] do_IRQ+0xcc/0xf4
[efbefc50] [c001081c] ret_from_except+0x0/0x18
[efbefd10] [c00730b4] __setup_irq+0x320/0x39c
[efbefd30] [c0073214] request_threaded_irq+0xe4/0x148
[efbefd60] [f2244218] ath_pci_probe+0x1b0/0x3a4 [ath9k]
[efbefda0] [c01c386c] local_pci_probe+0x24/0x34
[efbefdb0] [c01c3bc0] pci_device_probe+0x84/0xa8
[efbefde0] [c01e86b8] driver_probe_device+0xa8/0x1a8
[efbefe00] [c01e8874] __driver_attach+0xbc/0xc0
[efbefe20] [c01e7d88] bus_for_each_dev+0x70/0xac
[efbefe50] [c01e84d8] driver_attach+0x24/0x34
[efbefe60] [c01e7504] bus_add_driver+0xb8/0x278
[efbefe90] [c01e8bec] driver_register+0x84/0x178
[efbefeb0] [c01c3e6c] __pci_register_driver+0x54/0xe4
[efbefed0] [f2244434] ath_pci_init+0x28/0x38 [ath9k]
[efbefee0] [f215702c] ath9k_init+0x2c/0x100 [ath9k]
[efbefef0] [c0001d34] do_one_initcall+0x3c/0x1e8
[efbeff20] [c006f9f0] sys_init_module+0xf8/0x220
[efbeff40] [c00101c4] ret_from_syscall+0x0/0x3c
handlers:
[<f223badc>] (ath_isr+0x0/0x1b4 [ath9k])
Disabling IRQ #16
Atheros card plugged into regular PCI-E slot works OK in FSL BSP.
Any help in resolving this is much appreciated.
Thanks.
Felix.
^ permalink raw reply
* [git pull] Please pull powerpc.git next branch
From: Kumar Gala @ 2009-12-15 20:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
The following changes since
commit e090aa80321b64c3b793f3b047e31ecf1af9538d:
Benjamin Herrenschmidt (1):
powerpc: Fix usage of 64-bit instruction in 32-bit altivec code
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git next
Anton Vorontsov (4):
powerpc/fsl_pci: Fix P2P bridge handling for MPC83xx PCIe controllers
powerpc/83xx/suspend: Clear deep_sleeping after devices resume
powerpc/83xx/suspend: Save and restore SICRL, SICRH and SCCR
powerpc/83xx: Add power management support for MPC8315E-RDB boards
Dmitry Eremin-Solenikov (4):
powerpc/83xx: mpc8349emitx - add gpio controller declarations
powerpc/83xx: mpc8349emitx - populate I2C busses in device tree
powerpc/83xx: mpc8349emitx - add OF descriptions of LocalBus devices
powerpc/83xx: mpc8349emitx - add leds-gpio binding
Felix Radensky (1):
powerpc/85xx: Workaround MPC8572/MPC8536 GPIO 1 errata.
Mark Ware (1):
powerpc/cpm2_pic: Allow correct flow_types for port C interrupts
Peter Korsgaard (1):
powerpc/gpio: support gpio_to_irq()
Sebastian Andrzej Siewior (1):
powerpc/fsl: try to explain why the interrupt numbers are off by 16
Documentation/powerpc/dts-bindings/fsl/mpic.txt | 42 ++++++++++++
arch/powerpc/boot/dts/mpc8315erdb.dts | 27 ++++++++
arch/powerpc/boot/dts/mpc8349emitx.dts | 82 ++++++++++++++++++++++-
arch/powerpc/include/asm/gpio.h | 5 +-
arch/powerpc/platforms/83xx/suspend.c | 52 ++++++++++++++-
arch/powerpc/sysdev/cpm2_pic.c | 28 ++++++--
arch/powerpc/sysdev/fsl_pci.c | 8 ++-
arch/powerpc/sysdev/mpc8xxx_gpio.c | 21 ++++++-
8 files changed, 248 insertions(+), 17 deletions(-)
create mode 100644 Documentation/powerpc/dts-bindings/fsl/mpic.txt
^ permalink raw reply
* Re: [PATCH] PowerPC: const intspec pointers
From: Grant Likely @ 2009-12-15 19:50 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912151159.40628.roman.fietze@telemotive.de>
On Tue, Dec 15, 2009 at 3:59 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
> Hallo Grant,
>
> On Friday 11 December 2009 07:13:45 Grant Likely wrote:
>
>> BTW, if you're interested, there is a driver for the SCLPC FIFO about
>> to be merged into 2.6.33. =A0It's in Ben's tree waiting to be pulled
>> into mainline.
>
> I've had a "look" into it. This means I rewrote it in some parts to
> get it running.
>
> First I tried to do it in nice little steps, so the git commits are
> clean and simple. But with my knowldege about the SCLPC, the code your
> old SCLPC test driver and my 2.4.25 FPGA driver using SCLPS and
> BestComm, one of the commits just got a big rewrite including fixes
> for quite some bugs.
>
> The driver is now running using DMA TX and RX. Without BestComm it's
> running partly. I'm really windering if this code ever ran before?
> There are some historical or future items in there like measurement of
> the times, a list_header maybe for future request queueing, and so on
Yes, I'm using the driver in a couple of projects. It works for me
for both RX and TX (although TX+DMA has been troublesome). I'll
double check to make sure I've merged all of my patches for the
driver.
The test driver on the other hand is pretty poor code. Don't expect
much from it other than some hints. There's a reason I didn't merge
that chunk.
> If you or Ben are interested in my work I can post the patches
> here. Of course just to get some comments, because the driver can't be
> ready in the state it is. The commits would go on top of Ben's next
> branch.
Yes, please post the patches and cc: me. I'll review, test, and make comme=
nts.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH v0] Crypto: Talitos: re-initialize async_tx descriptors
From: Dan Williams @ 2009-12-15 18:23 UTC (permalink / raw)
To: Suresh Vishnu-B05022
Cc: herbert@gondor.apana.org.au, Ira Snyder, Tabi Timur-B04825,
linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
linuxppc-dev@ozlabs.org, linux-crypto@vger.kernel.org,
Li Yang-R58472
In-Reply-To: <76952DF81420A349876E087D089DF034562C75@zin33exm21.fsl.freescale.net>
Suresh Vishnu-B05022 wrote:
> > On Mon, Dec 14, 2009 at 6:33 AM, Vishnu Suresh <Vishnu@freescale.com>
> wrote:
> > > The async_tx descriptors contains dangling pointers.
> > > Hence, re-initialize them to NULL before use.
> > >
> > > Signed-off-by: Vishnu Suresh <Vishnu@freescale.com>
> > > ---
> > > o. Rebased to linux-next as of 20091214
> > >
> > > drivers/crypto/talitos.c | 3 +++
> > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> > > index 87f06be..9e261c6 100644
> > > --- a/drivers/crypto/talitos.c
> > > +++ b/drivers/crypto/talitos.c
> > > @@ -952,6 +952,9 @@ static struct dma_async_tx_descriptor *
> talitos_prep_dma_xor(
> > > return NULL;
> > > }
> > > dma_async_tx_descriptor_init(&new->async_tx, &xor_chan->common);
> > > + new->async_tx.parent = NULL;
> > > + new->async_tx.next = NULL;
> > > +
> > >
> > > desc = &new->hwdesc;
> > > /* Set destination: Last pointer pair */
>
> > These two values are owned by the async_tx api, drivers are not
> > supposed to touch them.
> I have sent this patch and the similar one for fsldma seperately,
> so that if the changes are needed and can be done in
> dma_async_tx_descriptor_init(),
> these patches can be ignored.
> > Both iop_adma and the new ppx4xx driver
> > (which use the async_tx channel switching capability) get away without
> > touching these fields which makes me suspect there is a
> > misunderstanding/bug somewhere else in the talitos implementation.
>
> This bug does not occur on all the platforms. The occurrance is random.
> This occurs when a channel switch between two different devices are
> present.
> This same initialization is required in case of fsldma as well.
> In case of fsldma/talitosXOR, there are two DMA channels (on same
> device) and single XOR channel (on another device).
>
> When used without fsldma, this driver works fine.
> Does iop_adma and ppx4xx work in similar enviroment?
Yes, iop_adma when running on iop3xx hardware has one xor channel and
two memcpy channels.
The bug may very well be in the fsldma driver, but the workarounds are
just band aids. I would be more comfortable with dropping the three
workaround patches and simply adding "depends on !FSL_DMA" to the
CRYPTO_DEV_TALITOS option until the true fix can be developed.
--
Dan
^ permalink raw reply
* Re: [Next] CPU Hotplug test failures on powerpc
From: Peter Zijlstra @ 2009-12-15 15:03 UTC (permalink / raw)
To: Sachin Sant; +Cc: Ingo Molnar, linux-next, linux-kernel, Linux/PPC Development
In-Reply-To: <4B279370.5050800@in.ibm.com>
Could you try the below?
---
init/main.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/init/main.c b/init/main.c
index 4051d75..4be7de2 100644
--- a/init/main.c
+++ b/init/main.c
@@ -369,12 +369,6 @@ static void __init smp_init(void)
{
unsigned int cpu;
=20
- /*
- * Set up the current CPU as possible to migrate to.
- * The other ones will be done by cpu_up/cpu_down()
- */
- set_cpu_active(smp_processor_id(), true);
-
/* FIXME: This should be done in userspace --RR */
for_each_present_cpu(cpu) {
if (num_online_cpus() >=3D setup_max_cpus)
@@ -486,6 +480,7 @@ static void __init boot_cpu_init(void)
int cpu =3D smp_processor_id();
/* Mark the boot cpu "present", "online" etc for SMP and UP case */
set_cpu_online(cpu, true);
+ set_cpu_active(cpu, true);
set_cpu_present(cpu, true);
set_cpu_possible(cpu, true);
}
^ permalink raw reply related
* Re: [Next] CPU Hotplug test failures on powerpc
From: Sachin Sant @ 2009-12-15 13:47 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, linux-next, linux-kernel, Linux/PPC Development
In-Reply-To: <1260873827.4165.362.camel@twins>
Peter Zijlstra wrote:
>> I added some debug statements within the above code.
>> This is a 2 cpu machine.
>>
>> XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
>> XMON dest_cpu = 1024
>> XMON dest_cpu = 1024 . dead_cpu = 1
>> XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
>> XMON dest_cpu = 1024
>> XMON dest_cpu = 1024 . dead_cpu = 1
>> XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
>> XMON dest_cpu = 1024
>> XMON dest_cpu = 1024 . dead_cpu = 1
>>
>> Seems to me that the control is stuck in an infinite loop and hence the
>> machine appears to be in hung state. The dest_cpu value is always 1024
>> and never changes, which result in an infinite loop.
>>
>> In working scenario the o/p is something on the following lines
>>
>> XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
>> XMON dest_cpu = 0
>> XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
>> XMON dest_cpu = 0
>> XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
>> XMON dest_cpu = 0
>>
>> Let me know if i should try to record any specific value ?
>>
>
> Could you possibly print the two masks themselves? cpumask_scnprintf()
> and friend come in handy for this.
>
> The dest_cpu=1024 thing seem to suggest the intersection between
> p->cpus_allowed and cpu_active_mask is empty for some reason, even
> though we forcefully reset p->cpus_allowed to the full set using
> cpuset_cpus_allowed_locked().
>
So here is the data related to the two masks.
cpu_active_mask = 00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000
XMON dest_cpu = 1024
while p->cpus_allowed = 00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000001
XMON dest_cpu = 1024
In working scenario the above data looks like
cpu_active_mask = 00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000002
XMON dest_cpu = 1
while p->cpus_allowed = 00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000002
XMON dest_cpu = 1
hope i got the data correct.
Thanks
-Sachin
--
---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------
^ permalink raw reply
* Re: [v10 PATCH 8/9]: pSeries: implement pSeries processor idle module
From: Arun R Bharadwaj @ 2009-12-15 11:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-arch, Peter Zijlstra, linux-kernel, linux-acpi,
Venkatesh Pallipadi, Arun Bharadwaj, Ingo Molnar, linuxppc-dev
In-Reply-To: <1259920852.2076.1274.camel@pasglop>
* Benjamin Herrenschmidt <benh@kernel.crashing.org> [2009-12-04 21:00:52]:
> On Fri, 2009-12-04 at 13:45 +0530, Arun R Bharadwaj wrote:
>
> >
> > Hi Ben,
> >
> > I forgot to attach the patch which enables cpuidle for the rest of the
> > POWER platforms. Attaching it below.
> >
> > So for these platforms, ppc_md.power_save will be called from from the
> > cpuidle_idle_call idle loop itself. Also, this cpuidle_idle_call is
> > not a pseries specific idle loop. It is a common loop for Intel and
> > PPC which use cpuidle infrastructure.
>
> Ok, so there was a missing piece in the puzzle ;-)
>
> I'll review asap.
>
Hi Ben,
Did you get time to review this?
thanks
arun
> Cheers,
> Ben.
>
> > arun
> >
> >
> >
> > This patch enables cpuidle for the rest of the POWER platforms like
> > 44x, Cell, Pasemi etc.
> >
> > Signed-off-by: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/include/asm/system.h | 2 ++
> > arch/powerpc/kernel/idle.c | 28 ++++++++++++++++++++++++++++
> > arch/powerpc/kernel/setup_32.c | 8 ++++++--
> > arch/powerpc/platforms/44x/idle.c | 2 ++
> > arch/powerpc/platforms/cell/pervasive.c | 2 ++
> > arch/powerpc/platforms/pasemi/idle.c | 2 ++
> > arch/powerpc/platforms/ps3/setup.c | 2 ++
> > 7 files changed, 44 insertions(+), 2 deletions(-)
> >
> > Index: linux.trees.git/arch/powerpc/include/asm/system.h
> > ===================================================================
> > --- linux.trees.git.orig/arch/powerpc/include/asm/system.h
> > +++ linux.trees.git/arch/powerpc/include/asm/system.h
> > @@ -551,8 +551,10 @@ void cpu_idle_wait(void);
> >
> > #ifdef CONFIG_CPU_IDLE
> > extern void update_smt_snooze_delay(int snooze);
> > +extern void setup_cpuidle_ppc(void);
> > #else
> > static inline void update_smt_snooze_delay(int snooze) {}
> > +static inline void setup_cpuidle_ppc(void) {}
> > #endif
> >
> > #endif /* __KERNEL__ */
> > Index: linux.trees.git/arch/powerpc/kernel/idle.c
> > ===================================================================
> > --- linux.trees.git.orig/arch/powerpc/kernel/idle.c
> > +++ linux.trees.git/arch/powerpc/kernel/idle.c
> > @@ -129,6 +129,34 @@ void default_idle(void)
> > HMT_very_low();
> > }
> >
> > +#ifdef CONFIG_CPU_IDLE
> > +DEFINE_PER_CPU(struct cpuidle_device, ppc_idle_devices);
> > +struct cpuidle_driver cpuidle_ppc_driver = {
> > + .name = "cpuidle_ppc",
> > +};
> > +
> > +static void ppc_idle_loop(struct cpuidle_device *dev, struct cpuidle_state *st)
> > +{
> > + ppc_md.power_save();
> > +}
> > +
> > +void setup_cpuidle_ppc(void)
> > +{
> > + struct cpuidle_device *dev;
> > + int cpu;
> > +
> > + cpuidle_register_driver(&cpuidle_ppc_driver);
> > +
> > + for_each_online_cpu(cpu) {
> > + dev = &per_cpu(ppc_idle_devices, cpu);
> > + dev->cpu = cpu;
> > + dev->states[0].enter = ppc_idle_loop;
> > + dev->state_count = 1;
> > + cpuidle_register_device(dev);
> > + }
> > +}
> > +#endif
> > +
> > int powersave_nap;
> >
> > #ifdef CONFIG_SYSCTL
> > Index: linux.trees.git/arch/powerpc/kernel/setup_32.c
> > ===================================================================
> > --- linux.trees.git.orig/arch/powerpc/kernel/setup_32.c
> > +++ linux.trees.git/arch/powerpc/kernel/setup_32.c
> > @@ -133,14 +133,18 @@ notrace void __init machine_init(unsigne
> >
> > #ifdef CONFIG_6xx
> > if (cpu_has_feature(CPU_FTR_CAN_DOZE) ||
> > - cpu_has_feature(CPU_FTR_CAN_NAP))
> > + cpu_has_feature(CPU_FTR_CAN_NAP)) {
> > ppc_md.power_save = ppc6xx_idle;
> > + setup_cpuidle_ppc();
> > + }
> > #endif
> >
> > #ifdef CONFIG_E500
> > if (cpu_has_feature(CPU_FTR_CAN_DOZE) ||
> > - cpu_has_feature(CPU_FTR_CAN_NAP))
> > + cpu_has_feature(CPU_FTR_CAN_NAP)) {
> > ppc_md.power_save = e500_idle;
> > + setup_cpuidle_ppc();
> > + }
> > #endif
> > if (ppc_md.progress)
> > ppc_md.progress("id mach(): done", 0x200);
> > Index: linux.trees.git/arch/powerpc/platforms/44x/idle.c
> > ===================================================================
> > --- linux.trees.git.orig/arch/powerpc/platforms/44x/idle.c
> > +++ linux.trees.git/arch/powerpc/platforms/44x/idle.c
> > @@ -24,6 +24,7 @@
> > #include <linux/of.h>
> > #include <linux/kernel.h>
> > #include <asm/machdep.h>
> > +#include <asm/system.h>
> >
> > static int mode_spin;
> >
> > @@ -46,6 +47,7 @@ int __init ppc44x_idle_init(void)
> > /* If we are not setting spin mode
> > then we set to wait mode */
> > ppc_md.power_save = &ppc44x_idle;
> > + setup_cpuidle_ppc();
> > }
> >
> > return 0;
> > Index: linux.trees.git/arch/powerpc/platforms/cell/pervasive.c
> > ===================================================================
> > --- linux.trees.git.orig/arch/powerpc/platforms/cell/pervasive.c
> > +++ linux.trees.git/arch/powerpc/platforms/cell/pervasive.c
> > @@ -35,6 +35,7 @@
> > #include <asm/pgtable.h>
> > #include <asm/reg.h>
> > #include <asm/cell-regs.h>
> > +#include <asm/system.h>
> >
> > #include "pervasive.h"
> >
> > @@ -128,5 +129,6 @@ void __init cbe_pervasive_init(void)
> > }
> >
> > ppc_md.power_save = cbe_power_save;
> > + setup_cpuidle_ppc();
> > ppc_md.system_reset_exception = cbe_system_reset_exception;
> > }
> > Index: linux.trees.git/arch/powerpc/platforms/pasemi/idle.c
> > ===================================================================
> > --- linux.trees.git.orig/arch/powerpc/platforms/pasemi/idle.c
> > +++ linux.trees.git/arch/powerpc/platforms/pasemi/idle.c
> > @@ -27,6 +27,7 @@
> > #include <asm/machdep.h>
> > #include <asm/reg.h>
> > #include <asm/smp.h>
> > +#include <asm/system.h>
> >
> > #include "pasemi.h"
> >
> > @@ -81,6 +82,7 @@ static int __init pasemi_idle_init(void)
> >
> > ppc_md.system_reset_exception = pasemi_system_reset_exception;
> > ppc_md.power_save = modes[current_mode].entry;
> > + setup_cpuidle_ppc();
> > printk(KERN_INFO "Using PA6T idle loop (%s)\n", modes[current_mode].name);
> >
> > return 0;
> > Index: linux.trees.git/arch/powerpc/platforms/ps3/setup.c
> > ===================================================================
> > --- linux.trees.git.orig/arch/powerpc/platforms/ps3/setup.c
> > +++ linux.trees.git/arch/powerpc/platforms/ps3/setup.c
> > @@ -33,6 +33,7 @@
> > #include <asm/prom.h>
> > #include <asm/lv1call.h>
> > #include <asm/ps3gpu.h>
> > +#include <asm/system.h>
> >
> > #include "platform.h"
> >
> > @@ -214,6 +215,7 @@ static void __init ps3_setup_arch(void)
> > prealloc_ps3flash_bounce_buffer();
> >
> > ppc_md.power_save = ps3_power_save;
> > + setup_cpuidle_ppc();
> > ps3_os_area_init();
> >
> > DBG(" <- %s:%d\n", __func__, __LINE__);
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-arch" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* RE: [PATCH v0] Crypto: Talitos: re-initialize async_tx descriptors
From: Suresh Vishnu-B05022 @ 2009-12-15 11:03 UTC (permalink / raw)
To: Dan Williams
Cc: herbert, Tabi Timur-B04825, linux-kernel, linux-raid,
linuxppc-dev, linux-crypto, Li Yang-R58472
In-Reply-To: <e9c3a7c20912142329j6603402ah2963d075419efa1c@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]
> On Mon, Dec 14, 2009 at 6:33 AM, Vishnu Suresh <Vishnu@freescale.com> wrote:
> > The async_tx descriptors contains dangling pointers.
> > Hence, re-initialize them to NULL before use.
> >
> > Signed-off-by: Vishnu Suresh <Vishnu@freescale.com>
> > ---
> > o. Rebased to linux-next as of 20091214
> >
> > drivers/crypto/talitos.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> > index 87f06be..9e261c6 100644
> > --- a/drivers/crypto/talitos.c
> > +++ b/drivers/crypto/talitos.c
> > @@ -952,6 +952,9 @@ static struct dma_async_tx_descriptor * talitos_prep_dma_xor(
> > return NULL;
> > }
> > dma_async_tx_descriptor_init(&new->async_tx, &xor_chan->common);
> > + new->async_tx.parent = NULL;
> > + new->async_tx.next = NULL;
> > +
> >
> > desc = &new->hwdesc;
> > /* Set destination: Last pointer pair */
> These two values are owned by the async_tx api, drivers are not
> supposed to touch them.
I have sent this patch and the similar one for fsldma seperately,
so that if the changes are needed and can be done in dma_async_tx_descriptor_init(),
these patches can be ignored.
> Both iop_adma and the new ppx4xx driver
> (which use the async_tx channel switching capability) get away without
> touching these fields which makes me suspect there is a
> misunderstanding/bug somewhere else in the talitos implementation.
This bug does not occur on all the platforms. The occurrance is random.
This occurs when a channel switch between two different devices are present.
This same initialization is required in case of fsldma as well.
In case of fsldma/talitosXOR, there are two DMA channels (on same device) and single XOR channel (on another device).
When used without fsldma, this driver works fine.
Does iop_adma and ppx4xx work in similar enviroment?
> > Also that dma_async_tx_descriptor_init() is unexpected in the hot
> > path, it's only needed at initial descriptor allocation. End result I
> > think this driver needs some more time to brew.
> --
> Dan
[-- Attachment #2: Type: text/html, Size: 3265 bytes --]
^ permalink raw reply
* Re: [PATCH] PowerPC: const intspec pointers
From: Roman Fietze @ 2009-12-15 10:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <fa686aa40912102213m5b2a0092i3c25d73b914ece93@mail.gmail.com>
Hallo Grant,
On Friday 11 December 2009 07:13:45 Grant Likely wrote:
> BTW, if you're interested, there is a driver for the SCLPC FIFO about
> to be merged into 2.6.33. It's in Ben's tree waiting to be pulled
> into mainline.
I've had a "look" into it. This means I rewrote it in some parts to
get it running.
=46irst I tried to do it in nice little steps, so the git commits are
clean and simple. But with my knowldege about the SCLPC, the code your
old SCLPC test driver and my 2.4.25 FPGA driver using SCLPS and
BestComm, one of the commits just got a big rewrite including fixes
for quite some bugs.
The driver is now running using DMA TX and RX. Without BestComm it's
running partly. I'm really windering if this code ever ran before?
There are some historical or future items in there like measurement of
the times, a list_header maybe for future request queueing, and so on.
If you or Ben are interested in my work I can post the patches
here. Of course just to get some comments, because the driver can't be
ready in the state it is. The commits would go on top of Ben's next
branch.
Roman
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply
* Re: [Next] CPU Hotplug test failures on powerpc
From: Peter Zijlstra @ 2009-12-15 10:43 UTC (permalink / raw)
To: Sachin Sant; +Cc: Ingo Molnar, linux-next, linux-kernel, Linux/PPC Development
In-Reply-To: <4B275A6B.9030200@in.ibm.com>
On Tue, 2009-12-15 at 15:14 +0530, Sachin Sant wrote:
> Benjamin Herrenschmidt wrote:
> >> static void move_task_off_dead_cpu(int dead_cpu, struct task_struct *p=
)
> >> {
> >> int dest_cpu;
> >> const struct cpumask *nodemask =3D cpumask_of_node(cpu_to_node=
(dead_cpu));
> >>
> >> again:
> >> /* Look for allowed, online CPU in same node. */
> >> for_each_cpu_and(dest_cpu, nodemask, cpu_active_mask)
> >> if (cpumask_test_cpu(dest_cpu, &p->cpus_allowed))
> >> goto move;
> >>
> >> /* Any allowed, online CPU? */
> >> dest_cpu =3D cpumask_any_and(&p->cpus_allowed, cpu_active_mask=
);
> >> if (dest_cpu < nr_cpu_ids)
> >> goto move;
> >>
> >> /* No more Mr. Nice Guy. */
> >> if (dest_cpu >=3D nr_cpu_ids) {
> >> cpuset_cpus_allowed_locked(p, &p->cpus_allowed);
> >> =3D=3D=3D=3D> dest_cpu =3D cpumask_any_and(cpu_active_mask, =
&p->cpus_allowed);
> >>
> >> /*
> >> * Don't tell them about moving exiting tasks or
> >> * kernel threads (both mm NULL), since they never
> >> * leave kernel.
> >> */
> >> if (p->mm && printk_ratelimit()) {
> >> pr_info("process %d (%s) no longer affine to c=
pu%d\n",
> >> task_pid_nr(p), p->comm, dead_cpu);
> >> }
> >> }
> >>
> >> move:
> >> /* It can have affinity changed while we were choosing. */
> >> if (unlikely(!__migrate_task_irq(p, dead_cpu, dest_cpu)))
> >> goto again;
> >> }
> >>
> >> Both masks, p->cpus_allowed and cpu_active_mask are stable in that p
> >> won't go away since we hold the tasklist_lock (in migrate_list_tasks),
> >> and cpu_active_mask is static storage, so WTH is it going funny on?
> >> =20
> I added some debug statements within the above code.=20
> This is a 2 cpu machine.
>=20
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1 . nr_cpu_ids =3D 2
> XMON dest_cpu =3D 1024=20
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1 . nr_cpu_ids =3D 2
> XMON dest_cpu =3D 1024=20
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1 . nr_cpu_ids =3D 2
> XMON dest_cpu =3D 1024=20
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1
>=20
> Seems to me that the control is stuck in an infinite loop and hence the
> machine appears to be in hung state. The dest_cpu value is always 1024
> and never changes, which result in an infinite loop.
>=20
> In working scenario the o/p is something on the following lines
>=20
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1 . nr_cpu_ids =3D 2
> XMON dest_cpu =3D 0=20
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1 . nr_cpu_ids =3D 2
> XMON dest_cpu =3D 0=20
> XMON dest_cpu =3D 1024 . dead_cpu =3D 1 . nr_cpu_ids =3D 2
> XMON dest_cpu =3D 0=20
>=20
> Let me know if i should try to record any specific value ?
Could you possibly print the two masks themselves? cpumask_scnprintf()
and friend come in handy for this.
The dest_cpu=3D1024 thing seem to suggest the intersection between
p->cpus_allowed and cpu_active_mask is empty for some reason, even
though we forcefully reset p->cpus_allowed to the full set using
cpuset_cpus_allowed_locked().
/me goes re-read the cpu_active_map code, this really shouldn't happen.
^ permalink raw reply
* Re: [Next] CPU Hotplug test failures on powerpc
From: Sachin Sant @ 2009-12-15 9:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Peter Zijlstra
Cc: Linux/PPC Development, Ingo Molnar, linux-next, linux-kernel
In-Reply-To: <1260825420.2217.40.camel@pasglop>
Benjamin Herrenschmidt wrote:
>> static void move_task_off_dead_cpu(int dead_cpu, struct task_struct *p)
>> {
>> int dest_cpu;
>> const struct cpumask *nodemask = cpumask_of_node(cpu_to_node(dead_cpu));
>>
>> again:
>> /* Look for allowed, online CPU in same node. */
>> for_each_cpu_and(dest_cpu, nodemask, cpu_active_mask)
>> if (cpumask_test_cpu(dest_cpu, &p->cpus_allowed))
>> goto move;
>>
>> /* Any allowed, online CPU? */
>> dest_cpu = cpumask_any_and(&p->cpus_allowed, cpu_active_mask);
>> if (dest_cpu < nr_cpu_ids)
>> goto move;
>>
>> /* No more Mr. Nice Guy. */
>> if (dest_cpu >= nr_cpu_ids) {
>> cpuset_cpus_allowed_locked(p, &p->cpus_allowed);
>> ====> dest_cpu = cpumask_any_and(cpu_active_mask, &p->cpus_allowed);
>>
>> /*
>> * Don't tell them about moving exiting tasks or
>> * kernel threads (both mm NULL), since they never
>> * leave kernel.
>> */
>> if (p->mm && printk_ratelimit()) {
>> pr_info("process %d (%s) no longer affine to cpu%d\n",
>> task_pid_nr(p), p->comm, dead_cpu);
>> }
>> }
>>
>> move:
>> /* It can have affinity changed while we were choosing. */
>> if (unlikely(!__migrate_task_irq(p, dead_cpu, dest_cpu)))
>> goto again;
>> }
>>
>> Both masks, p->cpus_allowed and cpu_active_mask are stable in that p
>> won't go away since we hold the tasklist_lock (in migrate_list_tasks),
>> and cpu_active_mask is static storage, so WTH is it going funny on?
>>
I added some debug statements within the above code.
This is a 2 cpu machine.
XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
XMON dest_cpu = 1024
XMON dest_cpu = 1024 . dead_cpu = 1
XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
XMON dest_cpu = 1024
XMON dest_cpu = 1024 . dead_cpu = 1
XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
XMON dest_cpu = 1024
XMON dest_cpu = 1024 . dead_cpu = 1
Seems to me that the control is stuck in an infinite loop and hence the
machine appears to be in hung state. The dest_cpu value is always 1024
and never changes, which result in an infinite loop.
In working scenario the o/p is something on the following lines
XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
XMON dest_cpu = 0
XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
XMON dest_cpu = 0
XMON dest_cpu = 1024 . dead_cpu = 1 . nr_cpu_ids = 2
XMON dest_cpu = 0
Let me know if i should try to record any specific value ?
Thanks
-Sachin
--
---------------------------------
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
---------------------------------
^ permalink raw reply
* Re: [PATCH v0] Crypto: Talitos: re-initialize async_tx descriptors
From: Dan Williams @ 2009-12-15 7:29 UTC (permalink / raw)
To: Vishnu Suresh
Cc: herbert, B04825, linux-kernel, linux-raid, linuxppc-dev,
linux-crypto, R58472
In-Reply-To: <1260797602-7476-1-git-send-email-Vishnu@freescale.com>
On Mon, Dec 14, 2009 at 6:33 AM, Vishnu Suresh <Vishnu@freescale.com> wrote=
:
> The async_tx descriptors contains dangling pointers.
> Hence, re-initialize them to NULL before use.
>
> Signed-off-by: Vishnu Suresh <Vishnu@freescale.com>
> ---
> o. Rebased to linux-next as of 20091214
>
> =A0drivers/crypto/talitos.c | =A0 =A03 +++
> =A01 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index 87f06be..9e261c6 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -952,6 +952,9 @@ static struct dma_async_tx_descriptor * talitos_prep_=
dma_xor(
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return NULL;
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0dma_async_tx_descriptor_init(&new->async_tx, &xor_chan->co=
mmon);
> + =A0 =A0 =A0 new->async_tx.parent =3D NULL;
> + =A0 =A0 =A0 new->async_tx.next =3D NULL;
> +
>
> =A0 =A0 =A0 =A0desc =3D &new->hwdesc;
> =A0 =A0 =A0 =A0/* Set destination: Last pointer pair */
These two values are owned by the async_tx api, drivers are not
supposed to touch them. Both iop_adma and the new ppx4xx driver
(which use the async_tx channel switching capability) get away without
touching these fields which makes me suspect there is a
misunderstanding/bug somewhere else in the talitos implementation.
Also that dma_async_tx_descriptor_init() is unexpected in the hot
path, it's only needed at initial descriptor allocation. End result I
think this driver needs some more time to brew.
--
Dan
^ permalink raw reply
* [PATCH] powerpc: Fix MSI support on U4 bridge PCIe slot
From: Benjamin Herrenschmidt @ 2009-12-15 1:31 UTC (permalink / raw)
To: linuxppc-dev
On machines using the Apple U4 bridge (AKA IBM CPC945) PCIe interface such
as the latest generation G5 machines x16 slot or the x16 slot of the
PowerStation, MSIs are currently broken (and will oops when enabling).
This fixes the oops and implements proper support for those. Instead of
using the PCIe <-> HT bridge conversion, on such slots we need to use
a bunch of magic registers in the bridge as the MSI target, encoding
the interrupt number in the low bits of the address itself
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/sysdev/mpic_msi.c | 11 ++++++++-
arch/powerpc/sysdev/mpic_u3msi.c | 46 ++++++++++++++++++++++++++++++++-----
2 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/sysdev/mpic_msi.c b/arch/powerpc/sysdev/mpic_msi.c
index 1d44eee..0f67cd7 100644
--- a/arch/powerpc/sysdev/mpic_msi.c
+++ b/arch/powerpc/sysdev/mpic_msi.c
@@ -39,7 +39,12 @@ static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic)
pr_debug("mpic: found U3, guessing msi allocator setup\n");
- /* Reserve source numbers we know are reserved in the HW */
+ /* Reserve source numbers we know are reserved in the HW.
+ *
+ * This is a bit of a mix of U3 and U4 reserves but that's going
+ * to work fine, we have plenty enugh numbers left so let's just
+ * mark anything we don't like reserved.
+ */
for (i = 0; i < 8; i++)
msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, i);
@@ -49,6 +54,10 @@ static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic)
for (i = 100; i < 105; i++)
msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, i);
+ for (i = 124; i < mpic->irq_count; i++)
+ msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, i);
+
+
np = NULL;
while ((np = of_find_all_nodes(np))) {
pr_debug("mpic: mapping hwirqs for %s\n", np->full_name);
diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index d3caf23..bcbfe79 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -64,12 +64,12 @@ static u64 read_ht_magic_addr(struct pci_dev *pdev, unsigned int pos)
return addr;
}
-static u64 find_ht_magic_addr(struct pci_dev *pdev)
+static u64 find_ht_magic_addr(struct pci_dev *pdev, unsigned int hwirq)
{
struct pci_bus *bus;
unsigned int pos;
- for (bus = pdev->bus; bus; bus = bus->parent) {
+ for (bus = pdev->bus; bus && bus->self; bus = bus->parent) {
pos = pci_find_ht_capability(bus->self, HT_CAPTYPE_MSI_MAPPING);
if (pos)
return read_ht_magic_addr(bus->self, pos);
@@ -78,13 +78,41 @@ static u64 find_ht_magic_addr(struct pci_dev *pdev)
return 0;
}
+static u64 find_u4_magic_addr(struct pci_dev *pdev, unsigned int hwirq)
+{
+ struct pci_controller *hose = pci_bus_to_host(pdev->bus);
+
+ /* U4 PCIe MSIs need to write to the special register in
+ * the bridge that generates interrupts. There should be
+ * theorically a register at 0xf8005000 where you just write
+ * the MSI number and that triggers the right interrupt, but
+ * unfortunately, this is busted in HW, the bridge endian swaps
+ * the value and hits the wrong nibble in the register.
+ *
+ * So instead we use another register set which is used normally
+ * for converting HT interrupts to MPIC interrupts, which decodes
+ * the interrupt number as part of the low address bits
+ *
+ * This will not work if we ever use more than one legacy MSI in
+ * a block but we never do. For one MSI or multiple MSI-X where
+ * each interrupt address can be specified separately, it works
+ * just fine.
+ */
+ if (of_device_is_compatible(hose->dn, "u4-pcie") ||
+ of_device_is_compatible(hose->dn, "U4-pcie"))
+ return 0xf8004000 | (hwirq << 4);
+
+ return 0;
+}
+
static int u3msi_msi_check_device(struct pci_dev *pdev, int nvec, int type)
{
if (type == PCI_CAP_ID_MSIX)
pr_debug("u3msi: MSI-X untested, trying anyway.\n");
/* If we can't find a magic address then MSI ain't gonna work */
- if (find_ht_magic_addr(pdev) == 0) {
+ if (find_ht_magic_addr(pdev, 0) == 0 &&
+ find_u4_magic_addr(pdev, 0) == 0) {
pr_debug("u3msi: no magic address found for %s\n",
pci_name(pdev));
return -ENXIO;
@@ -118,10 +146,6 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
u64 addr;
int hwirq;
- addr = find_ht_magic_addr(pdev);
- msg.address_lo = addr & 0xFFFFFFFF;
- msg.address_hi = addr >> 32;
-
list_for_each_entry(entry, &pdev->msi_list, list) {
hwirq = msi_bitmap_alloc_hwirqs(&msi_mpic->msi_bitmap, 1);
if (hwirq < 0) {
@@ -129,6 +153,12 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
return hwirq;
}
+ addr = find_ht_magic_addr(pdev, hwirq);
+ if (addr == 0)
+ addr = find_u4_magic_addr(pdev, hwirq);
+ msg.address_lo = addr & 0xFFFFFFFF;
+ msg.address_hi = addr >> 32;
+
virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
if (virq == NO_IRQ) {
pr_debug("u3msi: failed mapping hwirq 0x%x\n", hwirq);
@@ -143,6 +173,8 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
pr_debug("u3msi: allocated virq 0x%x (hw 0x%x) addr 0x%lx\n",
virq, hwirq, (unsigned long)addr);
+ printk("u3msi: allocated virq 0x%x (hw 0x%x) addr 0x%lx\n",
+ virq, hwirq, (unsigned long)addr);
msg.data = hwirq;
write_msi_msg(virq, &msg);
^ permalink raw reply related
* Re: [Next] CPU Hotplug test failures on powerpc
From: Benjamin Herrenschmidt @ 2009-12-14 21:17 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Linux/PPC Development, Ingo Molnar, linux-next, linux-kernel
In-Reply-To: <1260793182.4165.223.camel@twins>
On Mon, 2009-12-14 at 13:19 +0100, Peter Zijlstra wrote:
> > >> cpu 0x0: Vector: 100 (System Reset) at [c00000000c9333d0]
> > >> pc: c0000000003433d8: .find_next_bit+0x54/0xc4
> > >> lr: c000000000342f10: .cpumask_next_and+0x4c/0x94
> > >> sp: c00000000c933650
> > >> msr: 8000000000089032
> > >> current = 0xc00000000c173840
> > >> paca = 0xc000000000bc2600
> > >> pid = 2602, comm = hotplug06.top.s
> > >> enter ? for help
> > >> [link register ] c000000000342f10 .cpumask_next_and+0x4c/0x94
> > >> [c00000000c933650] c0000000000e9f34 .cpuset_cpus_allowed_locked+0x38/0x74 (unreliable)
> > >> [c00000000c9336e0] c000000000090074 .move_task_off_dead_cpu+0xc4/0x1ac
> > >> [c00000000c9337a0] c0000000005e4e5c .migration_call+0x304/0x830
> > >> [c00000000c933880] c0000000005e0880 .notifier_call_chain+0x68/0xe0
> > >> [c00000000c933920] c00000000012a92c ._cpu_down+0x210/0x34c
> > >> [c00000000c933a90] c00000000012aad8 .cpu_down+0x70/0xa8
> > >> [c00000000c933b20] c000000000525940 .store_online+0x54/0x894
> > >> [c00000000c933bb0] c000000000463430 .sysdev_store+0x3c/0x50
> > >> [c00000000c933c20] c0000000001f8320 .sysfs_write_file+0x124/0x18c
> > >> [c00000000c933ce0] c00000000017edac .vfs_write+0xd4/0x1fc
> > >> [c00000000c933d80] c00000000017efdc .SyS_write+0x58/0xa0
> > >> [c00000000c933e30] c0000000000085b4 syscall_exit+0x0/0x40
> > >> --- Exception: c01 (System Call) at 00000fff9fa8a8f8
> > >> SP (fffe7aef200) is in userspace
> > >> 0:mon> e
> > >> cpu 0x0: Vector: 100 (System Reset) at [c00000000c9333d0]
> > >> pc: c0000000003433d8: .find_next_bit+0x54/0xc4
> > >> lr: c000000000342f10: .cpumask_next_and+0x4c/0x94
> > >> sp: c00000000c933650
> > >> msr: 8000000000089032
> > >> current = 0xc00000000c173840
> > >> paca = 0xc000000000bc2600
> > >> pid = 2602, comm = hotplug06.top.s
> > >>
>
> OK so how do I read that above thing? What's a System Reset? Is that
> like the x86 triple fault thing?
Nah, it's an NMI that throws you into xmon. Basically, the machine was
hung and Sachin interrupted it with an NMI to see what was going on. The
above is the backtrace. It was at the moment of the NMI inside
find_next_bit() called from cpumask_next_and() etc...
> >From what I can make of it, its in move_task_off_dead_cpu(), right after
> having called cpuset_cpus_allowed_locked(), doing that cpumask_any_and()
> call.
Yes, it looks like it.
> static void move_task_off_dead_cpu(int dead_cpu, struct task_struct *p)
> {
> int dest_cpu;
> const struct cpumask *nodemask = cpumask_of_node(cpu_to_node(dead_cpu));
>
> again:
> /* Look for allowed, online CPU in same node. */
> for_each_cpu_and(dest_cpu, nodemask, cpu_active_mask)
> if (cpumask_test_cpu(dest_cpu, &p->cpus_allowed))
> goto move;
>
> /* Any allowed, online CPU? */
> dest_cpu = cpumask_any_and(&p->cpus_allowed, cpu_active_mask);
> if (dest_cpu < nr_cpu_ids)
> goto move;
>
> /* No more Mr. Nice Guy. */
> if (dest_cpu >= nr_cpu_ids) {
> cpuset_cpus_allowed_locked(p, &p->cpus_allowed);
> ====> dest_cpu = cpumask_any_and(cpu_active_mask, &p->cpus_allowed);
>
> /*
> * Don't tell them about moving exiting tasks or
> * kernel threads (both mm NULL), since they never
> * leave kernel.
> */
> if (p->mm && printk_ratelimit()) {
> pr_info("process %d (%s) no longer affine to cpu%d\n",
> task_pid_nr(p), p->comm, dead_cpu);
> }
> }
>
> move:
> /* It can have affinity changed while we were choosing. */
> if (unlikely(!__migrate_task_irq(p, dead_cpu, dest_cpu)))
> goto again;
> }
>
> Both masks, p->cpus_allowed and cpu_active_mask are stable in that p
> won't go away since we hold the tasklist_lock (in migrate_list_tasks),
> and cpu_active_mask is static storage, so WTH is it going funny on?
Sachin, this is 100% reproduceable right ? You should be able to
sprinkle it with some xmon_printf() (rather than printk, just add a
prototype extern void xmon_printf(const char *fmt,...); somewhere, this
has the advantage of being fully synchronous and will print out even if
the printk sem is held.
Cheers,
Ben.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH] powerpc: handle VSX alignment faults correctly in little-endian mode
From: Michael Neuling @ 2009-12-14 21:05 UTC (permalink / raw)
To: Neil Campbell; +Cc: linuxppc-dev
In-Reply-To: <4B2646F9.4000203@linux.vnet.ibm.com>
> This patch fixes the handling of VSX alignment faults in little-endian
> mode (the current code assumes the processor is in big-endian mode).
>
> The patch also makes the handlers clear the top 8 bytes of the register
> when handling an 8 byte VSX load.
>
> This is based on 2.6.32.
>
> Signed-off-by: Neil Campbell <neilc@linux.vnet.ibm.com>
Thanks for this Neil!
Acked-by: Michael Neuling <mikey@neuling.org>
> Cc: <stable@kernel.org>
> ---
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index a5b632e..f0c624f 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -642,10 +642,14 @@ static int emulate_spe(struct pt_regs *regs, unsigned i
nt reg,
> */
> static int emulate_vsx(unsigned char __user *addr, unsigned int reg,
> unsigned int areg, struct pt_regs *regs,
> - unsigned int flags, unsigned int length)
> + unsigned int flags, unsigned int length,
> + unsigned int elsize)
> {
> char *ptr;
> + unsigned long *lptr;
> int ret = 0;
> + int sw = 0;
> + int i, j;
>
> flush_vsx_to_thread(current);
>
> @@ -654,19 +658,35 @@ static int emulate_vsx(unsigned char __user *addr, unsi
gned int reg,
> else
> ptr = (char *) ¤t->thread.vr[reg - 32];
>
> - if (flags & ST)
> - ret = __copy_to_user(addr, ptr, length);
> - else {
> - if (flags & SPLT){
> - ret = __copy_from_user(ptr, addr, length);
> - ptr += length;
> + lptr = (unsigned long *) ptr;
> +
> + if (flags & SW)
> + sw = elsize-1;
> +
> + for (j = 0; j < length; j += elsize) {
> + for (i = 0; i < elsize; ++i) {
> + if (flags & ST)
> + ret |= __put_user(ptr[i^sw], addr + i);
> + else
> + ret |= __get_user(ptr[i^sw], addr + i);
> }
> - ret |= __copy_from_user(ptr, addr, length);
> + ptr += elsize;
> + addr += elsize;
> }
> - if (flags & U)
> - regs->gpr[areg] = regs->dar;
> - if (ret)
> +
> + if (!ret) {
> + if (flags & U)
> + regs->gpr[areg] = regs->dar;
> +
> + /* Splat load copies the same data to top and bottom 8 bytes */
> + if (flags & SPLT)
> + lptr[1] = lptr[0];
> + /* For 8 byte loads, zero the top 8 bytes */
> + else if (!(flags & ST) && (8 == length))
> + lptr[1] = 0;
> + } else
> return -EFAULT;
> +
> return 1;
> }
> #endif
> @@ -767,16 +787,25 @@ int fix_alignment(struct pt_regs *regs)
>
> #ifdef CONFIG_VSX
> if ((instruction & 0xfc00003e) == 0x7c000018) {
> - /* Additional register addressing bit (64 VSX vs 32 FPR/GPR */
> + unsigned int elsize;
> +
> + /* Additional register addressing bit (64 VSX vs 32 FPR/GPR) */
> reg |= (instruction & 0x1) << 5;
> /* Simple inline decoder instead of a table */
> + /* VSX has only 8 and 16 byte memory accesses */
> + nb = 8;
> if (instruction & 0x200)
> nb = 16;
> - else if (instruction & 0x080)
> - nb = 8;
> - else
> - nb = 4;
> +
> + /* Vector stores in little-endian mode swap individual
> + elements, so process them separately */
> + elsize = 4;
> + if (instruction & 0x80)
> + elsize = 8;
> +
> flags = 0;
> + if (regs->msr & MSR_LE)
> + flags |= SW;
> if (instruction & 0x100)
> flags |= ST;
> if (instruction & 0x040)
> @@ -787,7 +816,7 @@ int fix_alignment(struct pt_regs *regs)
> nb = 8;
> }
> PPC_WARN_EMULATED(vsx);
> - return emulate_vsx(addr, reg, areg, regs, flags, nb);
> + return emulate_vsx(addr, reg, areg, regs, flags, nb, elsize);
> }
> #endif
> /* A size of 0 indicates an instruction we don't support, with
>
^ permalink raw reply
* Re: [PATCH] powerpc/mm: fix typo of cpumask_clear_cpu()
From: Benjamin Herrenschmidt @ 2009-12-14 20:18 UTC (permalink / raw)
To: Li Yang; +Cc: linuxppc-dev
In-Reply-To: <1260795709-16165-1-git-send-email-leoli@freescale.com>
On Mon, 2009-12-14 at 21:01 +0800, Li Yang wrote:
> The function name of cpumask_clear_cpu was not correct.
>
> Reported-by: Jin Qing <b24347@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> This also implies that the CONFIG_HOTPLUG_CPU was never tested.
> We are trying to add cpu hotplug for SMP suspend, but seeing the
> following error(on 2.6.31 with context patches applied).
> Any idea or suggestion?
Hotplug hass indeed never been tested on BookE as we lack a platform
that supports it :-)
As you log, it's useless since you haven't compiled verbose BUG info in
your kernel so the message indicating the file/line of the error is
absent.
Ben.
> ------------[ cut here ]------------
> Badness at c00161b0 [verbose debug info unavailable]
> NIP: c00161b0 LR: c0016190 CTR: c0038f7c
> REGS: eec61e10 TRAP: 0700 Not tainted (2.6.31-00040-g7c92556-dirty)
> MSR: 00021000 <ME,CE> CR: 22280028 XER: 00000000
> TASK = eec54980[0] 'swapper' THREAD: eec60000 CPU: 1
> GPR00: 00000001 eec61ec0 eec54980 c0562ea0 eecad500 00000000 00000000 00000001
> GPR08: 00eed000 00000000 00000001 eecad67c 00001ca8 00000000 00021000 eec60040
> GPR16: eec54b0c c05208a0 c055fbe8 00000001 ffffffff c0560000 c0562ea0 00000004
> GPR24: eec60000 00000001 00000000 c0562e80 eec60000 c05291f8 eecad500 c05291f8
> NIP [c00161b0] switch_mmu_context+0x54/0x520
> LR [c0016190] switch_mmu_context+0x34/0x520
> Call Trace:
> [eec61ec0] [c00709c8] tick_program_event+0x50/0x60 (unreliable)
> [eec61f20] [c03beb54] schedule+0x2bc/0x7bc
> [eec61fa0] [c0008a8c] cpu_idle+0x160/0x170
> [eec61fc0] [c03c4be0] start_secondary+0x2d0/0x2e8
> [eec61ff0] [c0001c9c] __secondary_start+0x30/0x84
> Instruction dump:
> 543c0024 7ec3b378 833c0008 483ab1bd 813e0184 2f9d0000 39290001 913e0184
> 419e001c 813d0184 7d200034 5400d97e <0f000000> 3929ffff 913d0184 3d20c055
> MMU: More active contexts than CPUs ! (3 vs 2)
> MMU: More active contexts than CPUs ! (3 vs 2)
>
>
> arch/powerpc/mm/mmu_context_nohash.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
> index be4f34c..1044a63 100644
> --- a/arch/powerpc/mm/mmu_context_nohash.c
> +++ b/arch/powerpc/mm/mmu_context_nohash.c
> @@ -353,7 +353,7 @@ static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
> read_lock(&tasklist_lock);
> for_each_process(p) {
> if (p->mm)
> - cpu_mask_clear_cpu(cpu, mm_cpumask(p->mm));
> + cpumask_clear_cpu(cpu, mm_cpumask(p->mm));
> }
> read_unlock(&tasklist_lock);
> break;
^ permalink raw reply
* Re: [Patch 1/1] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
From: Roland McGrath @ 2009-12-14 19:26 UTC (permalink / raw)
To: prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, Frederic Weisbecker,
David Gibson, linuxppc-dev, Alan Stern, paulus
In-Reply-To: <20091214180324.GA18406@in.ibm.com>
> Yes, it does unset MSR_SE bit in single_step_dabr_instruction()
> irrespective of whether it was previously enabled through
> user_enable_single_step(). This could be mitigated with the use of a
> separate flag which can be used to conditionally unset MSR_SE, however
> given further concerns about pre-emption (as expressed by you below),
> I'm afraid of substantial revamp of the user-space semantics.
There is already TIF_SINGLESTEP set by user_enable_single_step. So for
that aspect, it is probably relatively straightforward to cover that
interaction. The code has to be pretty exact and will merit some comments
about subtleties, but I suspect the actual new code required will be just a
tiny amount.
> Kprobes has been tested to work simultaneously with hw-breakpoints. KGDB
> has not been ported yet to use the hw-breakpoint interfaces (KGDB had
> issues in it, that prevented it from being tested during our
> development...though its maintainer has begun showing interest
> recently).
>
> Xmon was (and I believe is still) in a state where data breakpoints did
> not work. It needs to be ported too, to benefit from the hw-breakpoint
> interfaces.
That is not really what I meant at all. That is good stuff to work out.
But I just meant the interactions with kprobes/kgdb's use of single-stepping,
the direct analogy to the user_enable_single_step issue.
> I must admit that the issue of pre-emption [...]
I understand the reason for using stepping. (I have advised in the past
that I thought this magical implicit step logic was too hairy to roll in
under the covers and that a low-level facility expressing the different
hardware semantics to a kernel API would be OK. I do agree with the
motivation of cross-arch uniformity of the semantics. I don't object to
making it magically right--I just expressed general skepticism/fear about
getting that right so that I didn't want to try writing that magic. Now
I'm just responding about the particular details I've noticed about that
can of worms. It's certainly great if you can resolve all that. But I'll
note that I am still by no means confident that the details I have raised
cover all the worms in that can.)
What remains less than clear is how preemption relates. For any per-thread
hw_breakpoint, there is no high-level reason to care one way or the other.
The thread, its HW breakpoints, its register state including state of
stepping, are all part of per-thread state and no reason to do any less (or
more) preemption than normally happens.
> Disabling pre-emption is necessary to ensure that hw-breakpoints are
> enabled immediately after the causative instruction has finished
> execution (the control flow may go astray if pre-emption occurs between
> i1 and i2).
I don't understand what "go astray" means here. The only thing I can think
of is the effect on any per-cpu variables you are using in hw_breakpoint
implementation.
Thanks,
Roland
^ permalink raw reply
* Re: [Patch 1/1] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
From: K.Prasad @ 2009-12-14 18:03 UTC (permalink / raw)
To: Roland McGrath
Cc: Michael Neuling, Benjamin Herrenschmidt, Frederic Weisbecker,
David Gibson, linuxppc-dev, Alan Stern, paulus
In-Reply-To: <20091214005648.82BA31DE@magilla.sf.frob.com>
On Sun, Dec 13, 2009 at 04:56:48PM -0800, Roland McGrath wrote:
> I can't see anything you've done to keep this use of MSR_SE in the
> user-mode register state from interfering with user_enable_single_step().
> It looks to me like you'd swallow the normal step indications.
>
Yes, it does unset MSR_SE bit in single_step_dabr_instruction()
irrespective of whether it was previously enabled through
user_enable_single_step(). This could be mitigated with the use of a
separate flag which can be used to conditionally unset MSR_SE, however
given further concerns about pre-emption (as expressed by you below),
I'm afraid of substantial revamp of the user-space semantics.
> Likewise I'm not very clear on the interaction with kprobes, kgdb,
> or whatnot for kernel-mode cases. But I'll leave those concerns to
> others, since I know more about the user-mode situations.
>
Kprobes has been tested to work simultaneously with hw-breakpoints. KGDB
has not been ported yet to use the hw-breakpoint interfaces (KGDB had
issues in it, that prevented it from being tested during our
development...though its maintainer has begun showing interest
recently).
Xmon was (and I believe is still) in a state where data breakpoints did
not work. It needs to be ported too, to benefit from the hw-breakpoint
interfaces.
> Back to the user-mode case, is it really reasonable to disable
> preemption in hw_breakpoint_handler and leave it so across returning
> to user mode? (Is that even possible? I thought user mode was
> always preemptible.) That is done very casually with little comment
> in hw_breakpoint_handler and single_step_dabr_instruction, but it
> seems like an extremely deep and magical thing that merits more
> explanation. I guess the need for it has to do with the per_cpu
> variable you're using, but the whole situation is not very clear on
> first reading. Even for kernel mode, what does this mean when the
> stepped instruction does a page fault?
>
I must admit that the issue of pre-emption should have been given more
thought. Suppose there's a stream of user-space instructions "i1, i2, i3,
.....i<n>" and if 'i2' instruction can cause a hw-breakpoint exception,
then there exists a small window between i1 and i2 where pre-emption is
disabled (while a schedule operation could have taken place otherwise).
Disabling pre-emption is necessary to ensure that hw-breakpoints are
enabled immediately after the causative instruction has finished
execution (the control flow may go astray if pre-emption occurs between
i1 and i2). The root cause of this behaviour is a combination of
'trigger-before-execute' behaviour (for data-exceptions) and a desire
for 'continuous' exceptions (as opposed to one-shot behaviour seen in
ptrace). The per-cpu variable 'last_hit_bp' just helps identify a
single-step exception resulting from a hbp_handler vs other sources.
Resorting to one-shot behaviour (which is the easiest workaround
available) will break the desired uniformity in behaviour for
hw-breakpoint interfaces - say every register_user_<> interface must be
accompanied by a unregister_<> interface, etc. Post perf-events'
integration, ensuring a one-shot behaviour might also have its own
bunch of undesirable consequences (such as circular locks), that must be
overcome.
Unless I see a way to re-instate the breakpoints (surviving a
pre-emption), I will send out a new patch that resorts to a one-shot
behaviour for user-space (kernel-space is fine though).
Thank you for the insightful comments!
K.Prasad
^ 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