[Trivial PATCH 12/33] macintosh: Convert use of typedef ctl_table to struct ctl_table
This typedef is unnecessary and should just be removed. Signed-off-by: Joe Perches --- drivers/macintosh/mac_hid.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/macintosh/mac_hid.c b/drivers/macintosh/mac_hid.c index 6a82388..80d30e8 100644 --- a/drivers/macintosh/mac_hid.c +++ b/drivers/macintosh/mac_hid.c @@ -181,7 +181,7 @@ static void mac_hid_stop_emulation(void) mac_hid_destroy_emumouse(); } -static int mac_hid_toggle_emumouse(ctl_table *table, int write, +static int mac_hid_toggle_emumouse(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) { @@ -214,7 +214,7 @@ static int mac_hid_toggle_emumouse(ctl_table *table, int write, } /* file(s) in /proc/sys/dev/mac_hid */ -static ctl_table mac_hid_files[] = { +static struct ctl_table mac_hid_files[] = { { .procname = "mouse_button_emulation", .data = &mouse_emulate_buttons, @@ -240,7 +240,7 @@ static ctl_table mac_hid_files[] = { }; /* dir in /proc/sys/dev */ -static ctl_table mac_hid_dir[] = { +static struct ctl_table mac_hid_dir[] = { { .procname = "mac_hid", .maxlen = 0, @@ -251,7 +251,7 @@ static ctl_table mac_hid_dir[] = { }; /* /proc/sys/dev itself, in case that is not there yet */ -static ctl_table mac_hid_root_dir[] = { +static struct ctl_table mac_hid_root_dir[] = { { .procname = "dev", .maxlen = 0, -- 1.8.1.2.459.gbcd45b4.dirty ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[Trivial PATCH 05/33] powerpc: idle: Convert use of typedef ctl_table to struct ctl_table
This typedef is unnecessary and should just be removed. Signed-off-by: Joe Perches --- arch/powerpc/kernel/idle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c index 939ea7e..d7216c9 100644 --- a/arch/powerpc/kernel/idle.c +++ b/arch/powerpc/kernel/idle.c @@ -85,7 +85,7 @@ int powersave_nap; /* * Register the sysctl to set/clear powersave_nap. */ -static ctl_table powersave_nap_ctl_table[]={ +static struct ctl_table powersave_nap_ctl_table[] = { { .procname = "powersave-nap", .data = &powersave_nap, @@ -95,7 +95,7 @@ static ctl_table powersave_nap_ctl_table[]={ }, {} }; -static ctl_table powersave_nap_sysctl_root[] = { +static struct ctl_table powersave_nap_sysctl_root[] = { { .procname = "kernel", .mode = 0555, -- 1.8.1.2.459.gbcd45b4.dirty ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[Trivial PATCH 00/33] Remove uses of typedef ctl_table
It's clearer to use struct ctl_table instead Joe Perches (33): arm: kernel: isa: Convert use of typedef ctl_table to struct ctl_table frv: Convert use of typedef ctl_table to struct ctl_table ia64: crash: Convert use of typedef ctl_table to struct ctl_table mips: lasat: sysctl: Convert use of typedef ctl_table to struct ctl_table powerpc: idle: Convert use of typedef ctl_table to struct ctl_table s390: Convert use of typedef ctl_table to struct ctl_table tile: proc: Convert use of typedef ctl_table to struct ctl_table x86: vdso: Convert use of typedef ctl_table to struct ctl_table cdrom: Convert use of typedef ctl_table to struct ctl_table char: Convert use of typedef ctl_table to struct ctl_table infiniband: Convert use of typedef ctl_table to struct ctl_table macintosh: Convert use of typedef ctl_table to struct ctl_table md: Convert use of typedef ctl_table to struct ctl_table sgi: xpc: Convert use of typedef ctl_table to struct ctl_table parport: Convert use of typedef ctl_table to struct ctl_table scsi_sysctl: Convert use of typedef ctl_table to struct ctl_table coda: Convert use of typedef ctl_table to struct ctl_table fscache: Convert use of typedef ctl_table to struct ctl_table lockd: Convert use of typedef ctl_table to struct ctl_table nfs: Convert use of typedef ctl_table to struct ctl_table inotify: Convert use of typedef ctl_table to struct ctl_table ntfs: Convert use of typedef ctl_table to struct ctl_table ocfs2: Convert use of typedef ctl_table to struct ctl_table quota: Convert use of typedef ctl_table to struct ctl_table xfs: Convert use of typedef ctl_table to struct ctl_table fs: Convert use of typedef ctl_table to struct ctl_table key: Convert use of typedef ctl_table to struct ctl_table ipv6: Convert use of typedef ctl_table to struct ctl_table ndisc: Convert use of typedef ctl_table to struct ctl_table ipc: Convert use of typedef ctl_table to struct ctl_table sysctl: Convert use of typedef ctl_table to struct ctl_table mm: Convert use of typedef ctl_table to struct ctl_table security: keys: Convert use of typedef ctl_table to struct ctl_table arch/arm/kernel/isa.c | 6 ++-- arch/frv/kernel/pm.c | 8 +++--- arch/frv/kernel/sysctl.c | 4 +-- arch/ia64/kernel/crash.c | 4 +-- arch/ia64/kernel/perfmon.c | 6 ++-- arch/mips/lasat/sysctl.c | 14 - arch/powerpc/kernel/idle.c | 4 +-- arch/s390/appldata/appldata_base.c | 16 +-- arch/s390/kernel/debug.c | 4 +-- arch/s390/mm/cmm.c | 6 ++-- arch/tile/kernel/proc.c| 4 +-- arch/x86/vdso/vdso32-setup.c | 4 +-- drivers/cdrom/cdrom.c | 10 +++ drivers/char/hpet.c| 6 ++-- drivers/char/ipmi/ipmi_poweroff.c | 6 ++-- drivers/char/random.c | 8 +++--- drivers/char/rtc.c | 6 ++-- drivers/infiniband/core/ucma.c | 2 +- drivers/macintosh/mac_hid.c| 8 +++--- drivers/md/md.c| 6 ++-- drivers/misc/sgi-xp/xpc_main.c | 6 ++-- drivers/parport/procfs.c | 58 +++--- drivers/scsi/scsi_sysctl.c | 6 ++-- fs/coda/sysctl.c | 4 +-- fs/dcache.c| 2 +- fs/drop_caches.c | 2 +- fs/eventpoll.c | 2 +- fs/file_table.c| 4 +-- fs/fscache/main.c | 4 +-- fs/inode.c | 2 +- fs/lockd/svc.c | 6 ++-- fs/nfs/nfs4sysctl.c| 6 ++-- fs/nfs/sysctl.c| 6 ++-- fs/notify/inotify/inotify_user.c | 2 +- fs/ntfs/sysctl.c | 4 +-- fs/ocfs2/stackglue.c | 8 +++--- fs/quota/dquot.c | 6 ++-- fs/xfs/xfs_sysctl.c| 26 - include/linux/key.h| 2 +- include/net/ipv6.h | 4 +-- include/net/ndisc.h| 2 +- ipc/ipc_sysctl.c | 14 - ipc/mq_sysctl.c| 10 +++ kernel/sysctl.c| 2 +- kernel/utsname_sysctl.c| 6 ++-- mm/page-writeback.c| 2 +- mm/page_alloc.c| 15 +- security/keys/sysctl.c | 2 +- 48 files changed, 174 insertions(+), 171 deletions(-) -- 1.8.1.2.459.gbcd45b4.dirty ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] powerpc: add Book E support to 64-bit hibernation
On 06/13/2013 04:55:43 AM, Wang Dongsheng-B40534 wrote: > > +#else > > + /* Save SPRGs */ > > + RESTORE_SPRG(0) > > + RESTORE_SPRG(1) > > + RESTORE_SPRG(2) > > + RESTORE_SPRG(3) > > + RESTORE_SPRG(4) > > + RESTORE_SPRG(5) > > + RESTORE_SPRG(6) > > + RESTORE_SPRG(7) > > Why do we need this on book3e and not on book3s? > Book3e: SPRG1 used save paca, SPRG2 be defined SPRN_SPRG_TLB_EXFRAME,... I think those register should be save, even now some SPRG register not be use. Are those expected/allowed to change as a result of the restore? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [BUG] PCI related panic on powerpc based board with 3.10-rcX
On 06/13/2013 02:21:24 AM, Rojhalat Ibrahim wrote: On Wednesday 12 June 2013 16:50:26 Scott Wood wrote: > On 06/12/2013 03:19:30 AM, Rojhalat Ibrahim wrote: > > On Tuesday 11 June 2013 12:28:59 Scott Wood wrote: > > > Yes, I figured it was non-PCIe because the code change that you said > > > helped was on the non-PCIe branch of the if/else. Generally it's > > > > good > > > > > to explicitly mention the chip you're using, though. > > > > > > fsl_setup_indirect_pci should be renamed to fsl_setup_indirect_pcie. > > > Your patch above should be applied, and fsl_setup_indirect_pcie > > > > should > > > > > be moved into the booke/86xx ifdef to avoid an unused function > > > > warning. > > > > > -Scott > > > > How about this patch? It uses setup_indirect_pci for the PCI case in > > mpc83xx_add_bridge. Additionally it adds a check in > > fsl_setup_indirect_pci > > to only use the modified read function in case of PCIe. > > If we're adding the check to fsl_setup_indirect_pci, there's no need to > change the 83xx call back to setup_indirect_pci. I see that 85xx is > also callirng fsl_setup_indirect_pci for both; it'd be good to be > consistent. > > In any case, can you send a proper patch with a signoff and commit > message? > > -Scott Where is it called for 85xx? As far as I can tell fsl_setup_indirect_pci is called exactly once in fsl_add_bridge and nowhere else (after applying the proposed patch). fsl_add_bridge() is where it's called for 85xx. For 83xx the decision between PCI and PCIe has already been made at the point where the setup function is called. So IMO it doesn't make sense to call fsl_setup_indirect_pci and do the check again. Moreover PCIe on 83xx uses a completely different set of functions. My concern is consistency. E.g. if 85xx is using fsl_setup_indirect_pci for both, but 83xx isn't, then a developer using 83xx could end up breaking 85xx by introducing another PCIe dependency in fsl_setup_indirect_pci. Or an 85xx developer could put something non-PCIe-related in fsl_setup_indirect_pci that 83xx would benefit from. Alternatively, you could call it fsl_setup_indirect_pcie, and move the PCIe check into fsl_add_bridge(). -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 15/27] powerpc/eeh: I/O chip EEH state retrieval
On Thu, Jun 13, 2013 at 02:42:17PM +1000, Benjamin Herrenschmidt wrote: >On Thu, 2013-06-13 at 12:26 +0800, Gavin Shan wrote: >> So the answer is we can do it by makeing the assumption that f/w won't >> return valid delay and we're going to use default value (1 second) for >> guest on powernv or phyp, or we keep the delay here. > >Ok, at the very least then change the name to "unavailable_delay" or >something explicit like that then :-) > Ok. >BTW. I've already applied patches 1 and 2 to my tree, you don't have to >resend those. They'll show up today or tomorrow when I push my next >branch out. > Ok. Thanks, Ben. Thanks, Gavin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix stack overflow crash in resume_kernel when ftracing
On Fri, 2013-06-14 at 00:31 +1000, Michael Ellerman wrote: > On Thu, 2013-06-13 at 08:45 -0400, Steven Rostedt wrote: > > On Thu, 2013-06-13 at 21:04 +1000, Michael Ellerman wrote: > > > > > Although that should be sufficient to fix the bug, we also mark the > > > runlatch routines as notrace. They are called very early in the > > > exception entry and we are asking for trouble tracing them. They are > > > also fairly uninteresting and tracing them just adds unnecessary > > > overhead. > > > > Note, I usually lean towards tracing everything that can be traced, and > > only adding notrace to things that will actually cause a crash. If you > > don't like them to be traced, you can always do: > > Yeah fair enough. In this case I think we don't want to trace it. > Although it doesn't cause a crash right now (at least after part 1 of > this patch), it's being called at a time when things are fragile, and > it's possible we could get bitten again some other way if the > surrounding code changes. > I wont push to trace it, as if it is fragile code, then it's best not to risk it. Tracing is always a second class citizen in the kernel ;-) > > > echo '*__ppc64_runlatch_*' > > > /sys/kernel/debug/tracing/set_ftrace_notrace > > > > and that will keep them from being traced. You can also add it to the > > kernel command line with: ftrace_notrace=*__ppc64_runlatch_* which will > > also disable them on boot up. > > > > Also trace-cmd has: > > > > trace-cmd record -p function_graph -n '*__ppc64_runlatch_*' > > > > that will do the same thing. > > > > Hmm, I should add a way to disable things that are usually considered > > noise. Perhaps add something like: > > > > > > FTRACE_DEFAULT_OFF(__ppc64_runlatch_on); > > > > That adds the function to a different section that places it into > > another file that keeps it from being traced, but can be enabled when > > you want it to. > > Yeah that would be cool. > > Personally I'm just using shell scripts that poke the files in sysfs, > and I'm often running on different boxes, so the more that just works > without extra config by me the better. Agreed, which is why I hope to get something like this in. Perhaps I'll aim for 3.12. Then we can look at all the functions with "notrace" and see which can be converted. Thanks, -- Steve ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix stack overflow crash in resume_kernel when ftracing
On Thu, 2013-06-13 at 08:45 -0400, Steven Rostedt wrote: > On Thu, 2013-06-13 at 21:04 +1000, Michael Ellerman wrote: > > > Although that should be sufficient to fix the bug, we also mark the > > runlatch routines as notrace. They are called very early in the > > exception entry and we are asking for trouble tracing them. They are > > also fairly uninteresting and tracing them just adds unnecessary > > overhead. > > Note, I usually lean towards tracing everything that can be traced, and > only adding notrace to things that will actually cause a crash. If you > don't like them to be traced, you can always do: Yeah fair enough. In this case I think we don't want to trace it. Although it doesn't cause a crash right now (at least after part 1 of this patch), it's being called at a time when things are fragile, and it's possible we could get bitten again some other way if the surrounding code changes. > echo '*__ppc64_runlatch_*' > > /sys/kernel/debug/tracing/set_ftrace_notrace > > and that will keep them from being traced. You can also add it to the > kernel command line with: ftrace_notrace=*__ppc64_runlatch_* which will > also disable them on boot up. > > Also trace-cmd has: > > trace-cmd record -p function_graph -n '*__ppc64_runlatch_*' > > that will do the same thing. > > Hmm, I should add a way to disable things that are usually considered > noise. Perhaps add something like: > > > FTRACE_DEFAULT_OFF(__ppc64_runlatch_on); > > That adds the function to a different section that places it into > another file that keeps it from being traced, but can be enabled when > you want it to. Yeah that would be cool. Personally I'm just using shell scripts that poke the files in sysfs, and I'm often running on different boxes, so the more that just works without extra config by me the better. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix stack overflow crash in resume_kernel when ftracing
On Thu, 2013-06-13 at 21:04 +1000, Michael Ellerman wrote: > Although that should be sufficient to fix the bug, we also mark the > runlatch routines as notrace. They are called very early in the > exception entry and we are asking for trouble tracing them. They are > also fairly uninteresting and tracing them just adds unnecessary > overhead. Note, I usually lean towards tracing everything that can be traced, and only adding notrace to things that will actually cause a crash. If you don't like them to be traced, you can always do: echo '*__ppc64_runlatch_*' > /sys/kernel/debug/tracing/set_ftrace_notrace and that will keep them from being traced. You can also add it to the kernel command line with: ftrace_notrace=*__ppc64_runlatch_* which will also disable them on boot up. Also trace-cmd has: trace-cmd record -p function_graph -n '*__ppc64_runlatch_*' that will do the same thing. Hmm, I should add a way to disable things that are usually considered noise. Perhaps add something like: FTRACE_DEFAULT_OFF(__ppc64_runlatch_on); That adds the function to a different section that places it into another file that keeps it from being traced, but can be enabled when you want it to. -- Steve ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/sysfs: disable hotplug for the boot cpu
On Wed, Jun 12, 2013 at 01:25:22PM +1000, Benjamin Herrenschmidt wrote: > On Mon, 2013-06-03 at 18:43 +0800, Zhao Chenhui wrote: > > On Sat, Jun 01, 2013 at 07:49:44AM +1000, Benjamin Herrenschmidt wrote: > > > On Tue, 2013-05-28 at 15:59 +0800, Zhao Chenhui wrote: > > > > Some features depend on the boot cpu, for instance, hibernate/suspend. > > > > So disable hotplug for the boot cpu. > > > > > > Don't we have code to "move" the boot CPU around when that happens ? > > > > > > Ben. > > > > > > > Currently, the code in generic_cpu_disable() likes this: > > > > if (cpu == boot_cpuid) > > > > return -EBUSY; > > But the code in pseries/hotplug-cpu.c doesn't, we just "move" the boot > CPU around when that happens. Any reason we can't do that generically ? > > Cheers, > Ben. > Some multicore SoCs firstly boot up the cpu0 after warm reset. In some suspend/resume cases, SoC will do a warm reset when resuming. In order to ensure that the suspending and resuming is running on a same cpu, cpu0 should be the last cpu to suspend. Here, cpu0 is the boot_cpuid. -Chenhui > > If the dying cpu is the boot cpu, it will return -EBUSY. In the subsequent > > error handling, > > cpu_notify_nofail(CPU_DOWN_FAILED) in _cpu_down() will be called. > > Unfortunately, some > > cpu notifier callbacks handled CPU_DOWN_PREPARE, but not CPU_DOWN_FAILED, > > such as sched_cpu_inactive(). > > So it will cause issues. > > > > If we set the hotpluggable for the boot cpu, we can prevent user > > applications from disabling the boot cpu. > > > > -Chenhui > > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: Fix stack overflow crash in resume_kernel when ftracing
It's possible for us to crash when running with ftrace enabled, eg: Bad kernel stack pointer bd12 at c000a454 cpu 0x3: Vector: 300 (Data Access) at [cffe3d40] pc: c000a454: resume_kernel+0x34/0x60 lr: c000335c: performance_monitor_common+0x15c/0x180 sp: bd12 msr: 80001032 dar: bd12 dsisr: 4200 If we look at current's stack (paca->__current->stack) we see it is equal to c002ecab. Our stack is 16K, and comparing to paca->kstack (c002ecab3e30) we can see that we have overflowed our kernel stack. This leads to us writing over our struct thread_info, and in this case we have corrupted thread_info->flags and set _TIF_EMULATE_STACK_STORE. Dumping the stack we see: 3:mon> t c002ecab [c002ecab] c002131c .performance_monitor_exception+0x5c/0x70 [c002ecab0080] c000335c performance_monitor_common+0x15c/0x180 --- Exception: f01 (Performance Monitor) at c00fb2ec .trace_hardirqs_off+0x1c/0x30 [c002ecab0370] c016fdb0 .trace_graph_entry+0xb0/0x280 (unreliable) [c002ecab0410] c003d038 .prepare_ftrace_return+0x98/0x130 [c002ecab04b0] c000a920 .ftrace_graph_caller+0x14/0x28 [c002ecab0520] c00d6b58 .idle_cpu+0x18/0x90 [c002ecab05a0] c000a934 .return_to_handler+0x0/0x34 [c002ecab0620] c001e660 .timer_interrupt+0x160/0x300 [c002ecab06d0] c00025dc decrementer_common+0x15c/0x180 --- Exception: 901 (Decrementer) at c00104d4 .arch_local_irq_restore+0x74/0xa0 [c002ecab09c0] c00fe044 .trace_hardirqs_on+0x14/0x30 (unreliable) [c002ecab0fb0] c016fe3c .trace_graph_entry+0x13c/0x280 [c002ecab1050] c003d038 .prepare_ftrace_return+0x98/0x130 [c002ecab10f0] c000a920 .ftrace_graph_caller+0x14/0x28 [c002ecab1160] c00161f0 .__ppc64_runlatch_on+0x10/0x40 [c002ecab11d0] c000a934 .return_to_handler+0x0/0x34 --- Exception: 901 (Decrementer) at c00104d4 .arch_local_irq_restore+0x74/0xa0 ... and so on __ppc64_runlatch_on() is called from RUNLATCH_ON in the exception entry path. At that point the irq state is not consistent, ie. interrupts are hard disabled (by the exception entry), but the paca soft-enabled flag may be out of sync. This leads to the local_irq_restore() in trace_graph_entry() actually enabling interrupts, which we do not want. Because we have not yet reprogrammed the decrementer we immediately take another decrementer exception, and recurse. The fix is twofold. Firstly make sure we call DISABLE_INTS before calling RUNLATCH_ON. The badly named DISABLE_INTS actually reconciles the irq state in the paca with the hardware, making it safe again to call local_irq_save/restore(). Although that should be sufficient to fix the bug, we also mark the runlatch routines as notrace. They are called very early in the exception entry and we are asking for trouble tracing them. They are also fairly uninteresting and tracing them just adds unnecessary overhead. Signed-off-by: Michael Ellerman --- arch/powerpc/include/asm/exception-64s.h |2 +- arch/powerpc/kernel/process.c|4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h index 8e5fae8..46793b5 100644 --- a/arch/powerpc/include/asm/exception-64s.h +++ b/arch/powerpc/include/asm/exception-64s.h @@ -513,7 +513,7 @@ label##_common: \ */ #define STD_EXCEPTION_COMMON_ASYNC(trap, label, hdlr)\ EXCEPTION_COMMON(trap, label, hdlr, ret_from_except_lite, \ -FINISH_NAP;RUNLATCH_ON;DISABLE_INTS) +FINISH_NAP;DISABLE_INTS;RUNLATCH_ON) /* * When the idle code in power4_idle puts the CPU into NAP mode, diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index b0f3e3f..076d124 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -1369,7 +1369,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack) #ifdef CONFIG_PPC64 /* Called with hard IRQs off */ -void __ppc64_runlatch_on(void) +void notrace __ppc64_runlatch_on(void) { struct thread_info *ti = current_thread_info(); unsigned long ctrl; @@ -1382,7 +1382,7 @@ void __ppc64_runlatch_on(void) } /* Called with hard IRQs off */ -void __ppc64_runlatch_off(void) +void notrace __ppc64_runlatch_off(void) { struct thread_info *ti = current_thread_info(); unsigned long ctrl; -- 1.7.10.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/2] powerpc: add Book E support to 64-bit hibernation
> -Original Message- > From: Wood Scott-B07421 > Sent: Thursday, June 13, 2013 6:04 AM > To: Wang Dongsheng-B40534 > Cc: b...@kernel.crashing.org; johan...@sipsolutions.net; an...@enomsg.org; > ga...@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; Wang Dongsheng- > B40534 > Subject: Re: [PATCH 1/2] powerpc: add Book E support to 64-bit > hibernation > > On 06/09/2013 05:37:39 AM, Wang Dongsheng wrote: > > /* these macros rely on the save area being > > * pointed to by r11 */ > > + > > +#define SAVE_SPR(register) \ > > + mfspr r0,SPRN_##register ;\ > > + std r0,SL_##register(r11) > > +#define RESTORE_SPR(register) \ > > + ld r0,SL_##register(r11) ;\ > > + mtspr SPRN_##register,r0 > > +#define RESTORE_SPRG(n)\ > > + ld r0,SL_SPRG##n(r11) ;\ > > + mtsprg n,r0 > > #define SAVE_SPECIAL(special) \ > > mf##special r0 ;\ > > std r0, SL_##special(r11) > > Is there a particular SPR that you're trying to save, for which > SAVE_SPECIAL doesn't work? > Yes, like pid, tcr. > > +#else > > + /* Save SPRGs */ > > + RESTORE_SPRG(0) > > + RESTORE_SPRG(1) > > + RESTORE_SPRG(2) > > + RESTORE_SPRG(3) > > + RESTORE_SPRG(4) > > + RESTORE_SPRG(5) > > + RESTORE_SPRG(6) > > + RESTORE_SPRG(7) > > Why do we need this on book3e and not on book3s? > Book3e: SPRG1 used save paca, SPRG2 be defined SPRN_SPRG_TLB_EXFRAME,... I think those register should be save, even now some SPRG register not be use. Book3s: Sorry, I not clear why book3s not do this. I think Anton or Ben could know the reason. > > + > > + RESTORE_SPECIAL(MSR) > > + > > + /* Restore TCR and clear any pending bits in TSR. */ > > + RESTORE_SPR(TCR) > > + lis r0, (TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS)@h > > + mtspr SPRN_TSR,r0 > > Please be internally consistent with whitespace after commas, even if the > rest of the file is already inconsistent. :-P > Thanks. > > + > > + /* Kick decrementer */ > > + li r0,1 > > + mtdec r0 > > Why doesn't book3s need to kick the decrementer? > Sorry, I not clear why book3s not do this. I think Anton or Ben could know the reason. > -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH powerpc] Set cpu sibling mask before online cpu
On Tue, 2013-06-11 at 14:03 +0530, Srivatsa S. Bhat wrote: > On 06/11/2013 12:30 PM, Benjamin Herrenschmidt wrote: > > On Thu, 2013-05-16 at 18:20 +0800, Li Zhong wrote: > >> It seems following race is possible: > >> > > > > .../... > > > >>vdso_getcpu_init(); > >> #endif > >> - notify_cpu_starting(cpu); > >> - set_cpu_online(cpu, true); > >>/* Update sibling maps */ > >>base = cpu_first_thread_sibling(cpu); > >>for (i = 0; i < threads_per_core; i++) { > >> - if (cpu_is_offline(base + i)) > >> + if (cpu_is_offline(base + i) && (cpu != base + i)) > >>continue; > >>cpumask_set_cpu(cpu, cpu_sibling_mask(base + i)); > >>cpumask_set_cpu(base + i, cpu_sibling_mask(cpu)); > >> @@ -667,6 +665,10 @@ __cpuinit void start_secondary(void *unused) > >>} > >>of_node_put(l2_cache); > >> > >> + smp_wmb(); > >> + notify_cpu_starting(cpu); > >> + set_cpu_online(cpu, true); > >> + > > > > So we could have an online CPU with an empty sibling mask. Now we can > > have a sibling that isn't online ... Is that ok ? > > I think it is OK. We do the same thing on x86 as well - we set up the > sibling links before calling notify_cpu_starting() and setting the cpu > in the cpu_online_mask. In fact, there is even a comment explicitly > noting that order: > > arch/x86/kernel/smpboot.c: > 220 /* > 221 * This must be done before setting cpu_online_mask > 222 * or calling notify_cpu_starting. > 223 */ > 224 set_cpu_sibling_map(raw_smp_processor_id()); > 225 wmb(); > 226 > 227 notify_cpu_starting(cpuid); > 228 > 229 /* > 230 * Allow the master to continue. > 231 */ > 232 cpumask_set_cpu(cpuid, cpu_callin_mask); > > > So I agree with Li Zhong's solution. > > [Arch-specific CPU hotplug code consolidation efforts such as [1] would > have weeded out such nasty bugs.. I guess we should revive that patchset > sometime soon.] > Thank you both for the review and comments. Good to know that it matches that of x86, and there is a patchset consolidating the code. With the patches in [1], it seems we only need the line to include the "to be onlined cpu" in this patch. Thanks, Zhong > Regards, > Srivatsa S. Bhat > > [1]. https://lwn.net/Articles/500185/ > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc/pci: Fix setup of Freescale PCI / PCIe controllers
Commit 50d8f87d2b3 (powerpc/fsl-pci Make PCIe hotplug work with Freescale PCIe controllers) does not handle non-PCIe controllers properly, which causes a panic during boot for certain configurations. This patch fixes the issue for 83xx devices by calling the proper setup function. For booke/86xx devices a check is added to differentiate between PCI and PCIe controllers. Reported-by: Michael Guntsche Cc: Scott Wood Signed-off-by: Rojhalat Ibrahim --- arch/powerpc/sysdev/fsl_pci.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 028ac1f..45670df 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -97,22 +97,23 @@ static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int devfn, return indirect_read_config(bus, devfn, offset, len, val); } -static struct pci_ops fsl_indirect_pci_ops = +static struct pci_ops fsl_indirect_pcie_ops = { .read = fsl_indirect_read_config, .write = indirect_write_config, }; +#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) + static void __init fsl_setup_indirect_pci(struct pci_controller* hose, resource_size_t cfg_addr, resource_size_t cfg_data, u32 flags) { setup_indirect_pci(hose, cfg_addr, cfg_data, flags); - hose->ops = &fsl_indirect_pci_ops; + if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) /* PCIe */ + hose->ops = &fsl_indirect_pcie_ops; } -#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) - #define MAX_PHYS_ADDR_BITS 40 static u64 pci64_dma_offset = 1ull << MAX_PHYS_ADDR_BITS; @@ -814,8 +815,8 @@ int __init mpc83xx_add_bridge(struct device_node *dev) if (ret) goto err0; } else { - fsl_setup_indirect_pci(hose, rsrc_cfg.start, - rsrc_cfg.start + 4, 0); + setup_indirect_pci(hose, rsrc_cfg.start, + rsrc_cfg.start + 4, 0); } printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. " -- 1.8.1.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [BUG] PCI related panic on powerpc based board with 3.10-rcX
On Wednesday 12 June 2013 16:50:26 Scott Wood wrote: > On 06/12/2013 03:19:30 AM, Rojhalat Ibrahim wrote: > > On Tuesday 11 June 2013 12:28:59 Scott Wood wrote: > > > Yes, I figured it was non-PCIe because the code change that you said > > > helped was on the non-PCIe branch of the if/else. Generally it's > > > > good > > > > > to explicitly mention the chip you're using, though. > > > > > > fsl_setup_indirect_pci should be renamed to fsl_setup_indirect_pcie. > > > Your patch above should be applied, and fsl_setup_indirect_pcie > > > > should > > > > > be moved into the booke/86xx ifdef to avoid an unused function > > > > warning. > > > > > -Scott > > > > How about this patch? It uses setup_indirect_pci for the PCI case in > > mpc83xx_add_bridge. Additionally it adds a check in > > fsl_setup_indirect_pci > > to only use the modified read function in case of PCIe. > > If we're adding the check to fsl_setup_indirect_pci, there's no need to > change the 83xx call back to setup_indirect_pci. I see that 85xx is > also callirng fsl_setup_indirect_pci for both; it'd be good to be > consistent. > > In any case, can you send a proper patch with a signoff and commit > message? > > -Scott Where is it called for 85xx? As far as I can tell fsl_setup_indirect_pci is called exactly once in fsl_add_bridge and nowhere else (after applying the proposed patch). For 83xx the decision between PCI and PCIe has already been made at the point where the setup function is called. So IMO it doesn't make sense to call fsl_setup_indirect_pci and do the check again. Moreover PCIe on 83xx uses a completely different set of functions. I'll send the proper patch in a separate mail. Rojhalat ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev