LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [patch] ppc: Fix gamecube build
From: Simon Horman @ 2010-05-13 15:25 UTC (permalink / raw)
  To: linuxppc-dev, kexec; +Cc: Maxim Uvarov, Sebastian Andrzej Siewior

This fixes the kexec-build on ppc32 when
the --game-cube option is supplied to ./configure.
It seems to have bit-rotted a little.

Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Cc: Maxim Uvarov <muvarov@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

Index: kexec-tools/kexec/arch/ppc/kexec-elf-ppc.c
===================================================================
--- kexec-tools.orig/kexec/arch/ppc/kexec-elf-ppc.c	2010-05-14 00:18:25.000000000 +0900
+++ kexec-tools/kexec/arch/ppc/kexec-elf-ppc.c	2010-05-14 00:20:58.000000000 +0900
@@ -162,17 +162,9 @@ int elf_ppc_load(int argc, char **argv,
 	unsigned long max_addr, hole_addr;
 	struct mem_phdr *phdr;
 	size_t size;
-	unsigned long long *rsvmap_ptr;
-	struct bootblock *bb_ptr;
-	unsigned int nr_segments;
-	unsigned long my_kernel, my_dt_offset;
-	unsigned long my_stack, my_backup_start;
 #ifdef CONFIG_PPC64
 	unsigned long toc_addr;
 #endif
-	unsigned int slave_code[256 / sizeof(unsigned int)], master_entry;
-	unsigned char *seg_buf = NULL;
-	off_t seg_size = 0;
 #ifdef WITH_GAMECUBE
 	int target_is_gamecube = 1;
 	char *arg_buf;
@@ -183,13 +175,21 @@ int elf_ppc_load(int argc, char **argv,
 	unsigned char *setup_start;
 	uint32_t setup_size;
 #else
+	unsigned long long *rsvmap_ptr;
+	struct bootblock *bb_ptr;
+	unsigned int nr_segments;
+	unsigned long my_kernel, my_dt_offset;
+	unsigned long my_stack, my_backup_start;
+	unsigned int slave_code[256 / sizeof(unsigned int)], master_entry;
+	unsigned char *seg_buf = NULL;
+	off_t seg_size = 0;
 	int target_is_gamecube = 0;
 	unsigned int addr;
 	unsigned long dtb_addr;
+#endif
 #define FIXUP_ENTRYS	(20)
 	char *fixup_nodes[FIXUP_ENTRYS + 1];
 	int cur_fixup = 0;
-#endif
 	int opt;
 
 	command_line = NULL;
Index: kexec-tools/kexec/arch/ppc/kexec-ppc.c
===================================================================
--- kexec-tools.orig/kexec/arch/ppc/kexec-ppc.c	2010-05-14 00:18:25.000000000 +0900
+++ kexec-tools/kexec/arch/ppc/kexec-ppc.c	2010-05-14 00:20:58.000000000 +0900
@@ -26,6 +26,11 @@
 
 #include "config.h"
 
+uint64_t rmo_top;
+unsigned long long crash_base, crash_size;
+unsigned int rtas_base, rtas_size;
+int max_memory_ranges;
+
 #ifdef WITH_GAMECUBE
 #define MAX_MEMORY_RANGES  64
 static struct memory_range memory_range[MAX_MEMORY_RANGES];
@@ -46,15 +51,11 @@ static int get_memory_ranges_gc(struct m
 }
 #else
 static int use_new_dtb;
-int max_memory_ranges;
 static int nr_memory_ranges, nr_exclude_ranges;
 static struct memory_range *exclude_range;
 static struct memory_range *memory_range;
 static struct memory_range *base_memory_range;
 static uint64_t memory_max;
-uint64_t rmo_top;
-unsigned long long crash_base, crash_size;
-unsigned int rtas_base, rtas_size;
 
 /*
  * Count the memory nodes under /proc/device-tree and populate the

^ permalink raw reply

* Re: ftrace syscalls, PowerPC: Fixes and PowerPC raw syscall tracepoint implementation
From: Steven Rostedt @ 2010-05-13 16:06 UTC (permalink / raw)
  To: Ian Munsie
  Cc: Frederic Weisbecker, Jason Baron, linux-kernel, Ingo Molnar,
	Paul Mackerras, linuxppc-dev
In-Reply-To: <1273736594-19320-1-git-send-email-imunsie@au1.ibm.com>

Frederic,

I'm fine with these patches, but since you mainly did the syscall work,
I'll let you take them.

The patches that touch the PowerPC code needs an acked-by from Ben or
Paul.

-- Steve


On Thu, 2010-05-13 at 17:43 +1000, Ian Munsie wrote:
> This patch series implements raw system call tracepoints on PowerPC that can be
> used with ftrace and perf. Some problems with the generic ftrace syscall
> tracepoint code have also been addressed.
> 
> The patches are based upon Ben's powerpc/next tree merged with tip/tracing/core
> 
> Patch #1 removes all ftrace syscall events that fail to map the system call
> name from the system call metadata with the system call's number, preventing
> the events which will not work from showing up in perf list and removing them
> from /sys/kernel/debug/tracing/events/syscalls.
> 
> Patches #2 and #3 allow for archs with unusual system call tables (#2) or
> unusual symbol names (#3) to override the appropriate functions so that they
> can still work with ftrace syscalls.
> 
> Patch #4 implements the actual raw system call tracepoints that ftrace syscalls
> builds upon, allowing all of the system calls to be used with the raw_syscalls
> events category and most to be used with the syscalls category.
> 
> 
> Not all the raw_syscalls are currently mapped to ftrace syscalls - the syscalls
> defined in /arch/powerpc/include/asm/syscalls.h do not use the SYSCALL_DEFINE
> class of macros and as such have no meta data, likewise some of the ppc_*
> syscalls have assembly wrappers. These are on their way, but I wanted to put
> the work I have done so far out first.
> 
> Some of those syscalls have different return types than the __SYSCALL_DEFINE
> macro uses (unsigned long, int, time_t) and some have different prefixes (ppc,
> ppc64) - I didn't particularly want to change them straight over without asking
> the list first, and I certainly don't want to change the return types. I see
> that Jason Baron ran into similar issues, but his "add compat syscall support"
> patches have yet to be merged, and do not tackle the differing return types.
> 

^ permalink raw reply

* Re: ftrace syscalls, PowerPC: Fixes and PowerPC raw syscall tracepoint implementation
From: Frederic Weisbecker @ 2010-05-13 16:12 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jason Baron, linux-kernel, Ingo Molnar, Paul Mackerras,
	Ian Munsie, linuxppc-dev
In-Reply-To: <1273766771.27703.1069.camel@gandalf.stny.rr.com>

On Thu, May 13, 2010 at 12:06:11PM -0400, Steven Rostedt wrote:
> Frederic,
> 
> I'm fine with these patches, but since you mainly did the syscall work,
> I'll let you take them.


Sure.


 
> The patches that touch the PowerPC code needs an acked-by from Ben or
> Paul.
> 
> -- Steve



Ok,

Thanks.

^ permalink raw reply

* Re: [PATCH v2 2/2] powerpc new toolchains fix (crt.S)
From: Segher Boessenkool @ 2010-05-13 17:27 UTC (permalink / raw)
  To: Maxim Uvarov; +Cc: lists, kexec, horms, linuxppc-dev
In-Reply-To: <20100512070849.9572.84728.stgit@muvarov>

> Linker does not provide some vital functions when building  
> freestanding
> applications with a new toolchain,

That's because the compiler provides those functions, not the linker.

> so we have to provide our own CRT.

...in libgcc.  Why don't you link against that?

> p.s.
> Without the CRT we won't see any build errors (since the purgatory is
> linked with --no-undefined), but the purgatory code won't work,

It would be nice if the build would check if anything unexpectedly
undefined is left.


Segher

^ permalink raw reply

* Re: [PATCH v2 2/2] powerpc new toolchains fix (crt.S)
From: Anton Vorontsov @ 2010-05-13 19:07 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Maxim Uvarov, lists, horms, kexec, linuxppc-dev
In-Reply-To: <15016EE5-7B40-40C9-9E45-58409644CE17@kernel.crashing.org>

On Thu, May 13, 2010 at 07:27:59PM +0200, Segher Boessenkool wrote:
> >Linker does not provide some vital functions when building
> >freestanding
> >applications with a new toolchain,
> 
> That's because the compiler provides those functions, not the linker.
> 
> >so we have to provide our own CRT.
> 
> ...in libgcc.  Why don't you link against that?

For the same reason we don't link kernel against libgcc?
I.e. just a matter of taste. There are two camps: those who
argue against libgcc, and those who argue for libgcc.

Hey! You were in this thread! :-)

http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg17224.html

Purgatory is just a small (tiny) piece of code that runs
between kernels. Whether use libgcc or not, personally, I have
no strong opinion. Kexec didn't use it, so we kept it the same
way.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check
From: Kumar Gala @ 2010-05-13 19:14 UTC (permalink / raw)
  To: Li Yang; +Cc: linuxppc-dev, Jin Qing
In-Reply-To: <1273221514-8683-1-git-send-email-leoli@freescale.com>


On May 7, 2010, at 3:38 AM, Li Yang wrote:

> Check the user/supervisor execution permission base on the code =
address.
> This fixes the following oops on module loading or removing.
>=20
> Unable to handle kernel paging request for instruction fetch
> Faulting instruction address: 0xf938d040
> Oops: Kernel access of bad area, sig: 11 [#1]
>=20
> Signed-off-by: Li Yang <leoli@freescale.com>
> Signed-off-by: Jin Qing <b24347@freescale.com>
> ---
> arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)

applied to merge.

I've updated the commit message to be a bit more clear on why we need to =
do this.

Thanks for figuring this out.

- k=

^ permalink raw reply

* Re: "event-scan failed" logflood
From: Benjamin Herrenschmidt @ 2010-05-13 21:28 UTC (permalink / raw)
  To: pacman; +Cc: linuxppc-dev
In-Reply-To: <20100513111603.3352.qmail@kosh.dhis.org>

On Thu, 2010-05-13 at 06:16 -0500, pacman@kosh.dhis.org wrote:
> Benjamin Herrenschmidt writes:
> > 
> > Well, first it should be called once per second, not 60 times per
> > second, so something is wrong there...
> 
> Actually I think it was happening a lot more than 60 times per second, and
> klogd was losing most of the messages because they came too fast. When
> running the new kernel, vmstat shows 170000 context switches per second (and
> CPU usage never going below 5%) even when everything should be idle.
> 
> My next experiment was to hack rtas_init to return 0 unconditionally. Now
> everything still runs fine, and the context switches per second are back down
> in the 2-digit range.
> 
> > 
> > Then, it wouldn't surprise me if the Pegasos RTAS didn't implement
> > the event scan properly. Maybe we failed silently before that ? I
> > doubt it's necessary there anyways.
> 
> If I'm reading things correctly, rtas-event-scan-rate is the key to the
> frequency of attempts. And rtasd.c didn't expect this:
> 
> $ od -t x1 /proc/device-tree/rtas/rtas-event-scan-rate
> 0000000 00 00 00 00
> 0000004

Ok, that gives us a good thing to use to not do the scanning then. If
that's 0 then we assume it's a bogus OF and we disable the scan
completely. I'll do a patch later today, though feel free to beat me to
it.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check
From: Benjamin Herrenschmidt @ 2010-05-13 21:46 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, Jin Qing
In-Reply-To: <0710E9DF-66DF-4D24-95DE-A04E6D869826@kernel.crashing.org>

On Thu, 2010-05-13 at 14:14 -0500, Kumar Gala wrote:
> On May 7, 2010, at 3:38 AM, Li Yang wrote:
> 
> > Check the user/supervisor execution permission base on the code address.
> > This fixes the following oops on module loading or removing.
> > 
> > Unable to handle kernel paging request for instruction fetch
> > Faulting instruction address: 0xf938d040
> > Oops: Kernel access of bad area, sig: 11 [#1]
> > 
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Signed-off-by: Jin Qing <b24347@freescale.com>
> > ---
> > arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
> > 1 files changed, 10 insertions(+), 3 deletions(-)
> 
> applied to merge.
> 
> I've updated the commit message to be a bit more clear on why we need to do this.

Not looking at the code right now ... but do we have the same issue on
64e ?

Cheers,
Ben.

^ permalink raw reply

* Re: ftrace syscalls, PowerPC: Fixes and PowerPC raw syscall tracepoint implementation
From: Benjamin Herrenschmidt @ 2010-05-13 23:55 UTC (permalink / raw)
  To: rostedt
  Cc: Frederic Weisbecker, Jason Baron, linux-kernel, Ingo Molnar,
	Paul Mackerras, Ian Munsie, linuxppc-dev
In-Reply-To: <1273766771.27703.1069.camel@gandalf.stny.rr.com>

On Thu, 2010-05-13 at 12:06 -0400, Steven Rostedt wrote:
> Frederic,
> 
> I'm fine with these patches, but since you mainly did the syscall work,
> I'll let you take them.
> 
> The patches that touch the PowerPC code needs an acked-by from Ben or
> Paul.

Done :-)

Cheers,
Ben.

> -- Steve
> 
> 
> On Thu, 2010-05-13 at 17:43 +1000, Ian Munsie wrote:
> > This patch series implements raw system call tracepoints on PowerPC that can be
> > used with ftrace and perf. Some problems with the generic ftrace syscall
> > tracepoint code have also been addressed.
> > 
> > The patches are based upon Ben's powerpc/next tree merged with tip/tracing/core
> > 
> > Patch #1 removes all ftrace syscall events that fail to map the system call
> > name from the system call metadata with the system call's number, preventing
> > the events which will not work from showing up in perf list and removing them
> > from /sys/kernel/debug/tracing/events/syscalls.
> > 
> > Patches #2 and #3 allow for archs with unusual system call tables (#2) or
> > unusual symbol names (#3) to override the appropriate functions so that they
> > can still work with ftrace syscalls.
> > 
> > Patch #4 implements the actual raw system call tracepoints that ftrace syscalls
> > builds upon, allowing all of the system calls to be used with the raw_syscalls
> > events category and most to be used with the syscalls category.
> > 
> > 
> > Not all the raw_syscalls are currently mapped to ftrace syscalls - the syscalls
> > defined in /arch/powerpc/include/asm/syscalls.h do not use the SYSCALL_DEFINE
> > class of macros and as such have no meta data, likewise some of the ppc_*
> > syscalls have assembly wrappers. These are on their way, but I wanted to put
> > the work I have done so far out first.
> > 
> > Some of those syscalls have different return types than the __SYSCALL_DEFINE
> > macro uses (unsigned long, int, time_t) and some have different prefixes (ppc,
> > ppc64) - I didn't particularly want to change them straight over without asking
> > the list first, and I certainly don't want to change the return types. I see
> > that Jason Baron ran into similar issues, but his "add compat syscall support"
> > patches have yet to be merged, and do not tackle the differing return types.
> > 
> 

^ permalink raw reply

* Re: [PATCH 3/4] ftrace syscalls: Allow arch specific syscall symbol matching
From: Benjamin Herrenschmidt @ 2010-05-13 23:54 UTC (permalink / raw)
  To: Ian Munsie
  Cc: Frederic Weisbecker, Jason Baron, linux-kernel, Steven Rostedt,
	Ingo Molnar, Paul Mackerras, linuxppc-dev
In-Reply-To: <1273736594-19320-4-git-send-email-imunsie@au1.ibm.com>

On Thu, 2010-05-13 at 17:43 +1000, Ian Munsie wrote:
> From: Ian Munsie <imunsie@au1.ibm.com>
> 
> Some architectures have unusual symbol names and the generic code to
> match the symbol name with the function name for the syscall metadata
> will fail. For example, symbols on PPC64 start with a period and the
> generic code will fail to match them.
> 
> This patch splits out the match logic into a standalone weak function
> that can be overridden on archs with unusual symbol names.
> 
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> ---

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Ian, I assume you will implement the support for the "special" ppc_*
syscalls via a subsequent patch and not a respin of this one right ?

Cheers,
Ben.

>  Documentation/trace/ftrace-design.txt |    3 +++
>  include/linux/ftrace.h                |    1 +
>  kernel/trace/trace_syscalls.c         |   19 ++++++++++++-------
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
> index 8369a1c..3936d5f 100644
> --- a/Documentation/trace/ftrace-design.txt
> +++ b/Documentation/trace/ftrace-design.txt
> @@ -247,6 +247,9 @@ You need very few things to get the syscalls tracing in an arch.
>  - If the system call table on this arch is more complicated than a simple array
>    of addresses of the system calls, implement an arch_syscall_addr to return
>    the address of a given system call.
> +- If the symbol names of the system calls do not match the function names on
> +  this arch, implement an arch_syscall_match_sym_name with the appropriate
> +  logic to return true if the function name corresponds with the symbol name.
>  - Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
>  
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index e0ae83b..26ad1f5 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -534,6 +534,7 @@ static inline void trace_hw_branch_oops(void) {}
>  #ifdef CONFIG_FTRACE_SYSCALLS
>  
>  unsigned long arch_syscall_addr(int nr);
> +bool arch_syscall_match_sym_name(const char *sym, const char *name);
>  
>  #endif /* CONFIG_FTRACE_SYSCALLS */
>  
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 1c231d0..ebbc74d 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -32,13 +32,7 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
>  	kallsyms_lookup(syscall, NULL, NULL, NULL, str);
>  
>  	for ( ; start < stop; start++) {
> -		/*
> -		 * Only compare after the "sys" prefix. Archs that use
> -		 * syscall wrappers may have syscalls symbols aliases prefixed
> -		 * with "SyS" instead of "sys", leading to an unwanted
> -		 * mismatch.
> -		 */
> -		if (start->name && !strcmp(start->name + 3, str + 3))
> +		if (start->name && arch_syscall_match_sym_name(str, start->name))
>  			return start;
>  	}
>  	return NULL;
> @@ -408,6 +402,17 @@ unsigned long __init __weak arch_syscall_addr(int nr)
>  	return (unsigned long)sys_call_table[nr];
>  }
>  
> +bool __weak arch_syscall_match_sym_name(const char *sym, const char *name)
> +{
> +	/*
> +	 * Only compare after the "sys" prefix. Archs that use
> +	 * syscall wrappers may have syscalls symbols aliases prefixed
> +	 * with "SyS" instead of "sys", leading to an unwanted
> +	 * mismatch.
> +	 */
> +	return (!strcmp(sym + 3, name + 3));
> +}
> +
>  int __init init_ftrace_syscalls(void)
>  {
>  	struct syscall_metadata *meta;

^ permalink raw reply

* Re: [PATCH 1/2] Fix kexec on powerpc32
From: Christian Kujau @ 2010-05-14  0:43 UTC (permalink / raw)
  To: Maxim Uvarov; +Cc: kexec, uvarov, linuxppc-dev, horms
In-Reply-To: <AANLkTik1aRVvqj871qOmGctuP_WjvSFwhU1w4gpJoqp1@mail.gmail.com>

On Thu, 13 May 2010 at 15:50, Maxim Uvarov wrote:
> zImage should not work with kexec. Only vmlinux.

"should"?

PPC64 has something nice there:

  > kexec/arch/ppc64/kexec-zImage-ppc64.c:
  > fprintf(stderr, "zImage support is still broken\n");

....but I could not figure out where this is called from.

Anyway, tried vmlinux now and it's working, yay!

Thanks a lot, Maxim!

Christian.
-- 
BOFH excuse #363:

Out of cards on drive D:

^ permalink raw reply

* RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check
From: Liu Dave-R63238 @ 2010-05-14  0:53 UTC (permalink / raw)
  To: Kumar Gala, Li Yang-R58472; +Cc: linuxppc-dev, Jin Qing-B24347
In-Reply-To: <0710E9DF-66DF-4D24-95DE-A04E6D869826@kernel.crashing.org>

> I've updated the commit message to be a bit more clear on why=20
> we need to do this.

I'm curious why the _PAGE_EXEC have different definition in pte-book3e.h
and pte-fsl-booke.h?

It is UX permission in pte-book3e, but is SX permission in
pte-fsl-booke.h.

Thanks, Dave

^ permalink raw reply

* Re: [PATCH 4/4] trace, powerpc: Implement raw syscall tracepoints on PowerPC
From: Ian Munsie @ 2010-05-14  2:03 UTC (permalink / raw)
  To: michael
  Cc: Frederic Weisbecker, Jason Baron, linux-kernel, Steven Rostedt,
	Ingo Molnar, Paul Mackerras, linuxppc-dev
In-Reply-To: <1273752571.8067.12.camel@concordia>

Excerpts from Michael Ellerman's message of Thu May 13 22:09:31 +1000 2010:
> On Thu, 2010-05-13 at 17:43 +1000, Ian Munsie wrote:
> > From: Ian Munsie <imunsie@au.ibm.com>
> 
> Hi Ian,
> 
> Just a few comments ..
> 
> > This patch implements the raw syscall tracepoints on PowerPC required
> > for ftrace syscalls.
> 
> OK. It also adds a bunch of code under CONFIG_FTRACE_*, so does it
> implement raw syscall tracepoints _and_ hook them up to ftrace?

Yes, that's correct. CONFIG_FTRACE_SYSCALLS depends solely on, and is
the primary consumer of, HAVE_SYSCALL_TRACEPOINTS. It makes little sense
to me to provide the raw syscall tracepoints without exporting the
syscall table for ftrace syscalls to use - otherwise they would be
available to select in make config, but broken.

> > To minimise reworking existing code, I slightly re-ordered the thread
> > info flags such that the new TIF_SYSCALL_TRACEPOINT bit would still fit
> > within the 16 bits of the andi instruction's UI field.
> 
> Which andi instruction? That could use a bit more explaining.

The ones under /arch/powerpc/kernel/entry_{32,64}.S using andi to
and the _TIF_SYSCALL_T_OR_A with the thread flags to see if system call
tracing is enabled.
 
For instance, from entry_64.S:
	ld	r10,TI_FLAGS(r11)
	andi.	r11,r10,_TIF_SYSCALL_T_OR_A   <-- that one
	bne-	syscall_dotrace
.......
syscall_dotrace:
	bl	.save_nvgprs
	addi	r3,r1,STACK_FRAME_OVERHEAD
	bl	.do_syscall_trace_enter

And similarly elsewhere in the same file:
	ld	r9,TI_FLAGS(r12)
	li	r11,-_LAST_ERRNO
	andi.	r0,r9,(_TIF_SYSCALL_T_OR_A|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
	bne-	syscall_exit_work
.......
syscall_exit_work:
.......
	bl	.save_nvgprs
	addi	r3,r1,STACK_FRAME_OVERHEAD
	bl	.do_syscall_trace_leave

entry_32.S contains very similar assembly to the above.

The alternative to renumbering the thread flags would be to rework the
assembly to use and. instead of andi. to avoid having to squeeze that
flag into 16 bits.

> > In the case of 64bit PowerPC, arch_syscall_addr and
> > arch_syscall_match_sym_name are overridden to allow ftrace syscalls to
> > work given the unusual system call table structure and symbol names that
> > start with a period.
> 
> Not unusual, just different (ie. better) than x86 ;)

Fair enough ;)

> > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> > index 23913e9..4098105 100644
> > --- a/arch/powerpc/include/asm/syscall.h
> > +++ b/arch/powerpc/include/asm/syscall.h
> > @@ -15,6 +15,15 @@
> >  
> >  #include <linux/sched.h>
> >  
> > +/* ftrace syscalls requires exporting the sys_call_table */
> > +#ifdef CONFIG_FTRACE_SYSCALLS
> > +#ifdef CONFIG_PPC64
> > +extern const unsigned long long *sys_call_table;
> 
> I'm not sure why this is ULL ? UL and ULL are both 64 bits (on 64bit),
> and it would save you this ifdef block and a cast in
> arch_syscall_addr().

Good point - I was just following the format from
/arch/powerpc/kernel/systbl.S, which uses pairs of .llong on PPC64 and
single .long on PPC32. Does the assembler treat .llong different from
.long on 64bit?

> > +#else /* !CONFIG_PPC64 */
> > +extern const unsigned long *sys_call_table;
> > +#endif /* CONFIG_PPC64 */
> > +#endif /* CONFIG_FTRACE_SYSCALLS */
> > +
> >  static inline long syscall_get_nr(struct task_struct *task,
> >                    struct pt_regs *regs)
> >  {
> > diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> > index aa9d383..e7a8af2 100644
> > --- a/arch/powerpc/include/asm/thread_info.h
> > +++ b/arch/powerpc/include/asm/thread_info.h
> > @@ -110,7 +110,8 @@ static inline struct thread_info *current_thread_info(void)
> >  #define TIF_NOERROR        12    /* Force successful syscall return */
> >  #define TIF_NOTIFY_RESUME    13    /* callback before returning to user */
> >  #define TIF_FREEZE        14    /* Freezing for suspend */
> > -#define TIF_RUNLATCH        15    /* Is the runlatch enabled? */
> > +#define TIF_SYSCALL_TRACEPOINT    15    /* syscall tracepoint instrumentation */
> > +#define TIF_RUNLATCH        16    /* Is the runlatch enabled? */
> 
> I don't grok why this is good or safe, not that it isn't but please tell
> me why it is :)

Ok - now I could be wrong on this, but AFAICT the flags are only used
internally within the kernel by name and not exported to userspace (ie,
not part of the kernel ABI).  That specific flag is only set and cleared
within /arch/powerpc/kernel/process.c so recompiling the kernel should
be sufficient to change those instances of TIF_RUNLATCH from 15 to 16.

> > diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> > index 8773263..9c404bb 100644
> > --- a/arch/powerpc/kernel/Makefile
> > +++ b/arch/powerpc/kernel/Makefile
> > @@ -98,6 +98,7 @@ obj64-$(CONFIG_AUDIT)        += compat_audit.o
> >  
> >  obj-$(CONFIG_DYNAMIC_FTRACE)    += ftrace.o
> >  obj-$(CONFIG_FUNCTION_GRAPH_TRACER)    += ftrace.o
> > +obj-$(CONFIG_FTRACE_SYSCALLS)    += ftrace.o
> 
> You're following the existing pattern there, but it's a little odd.
> Seems like those three config options should really all depend on
> something common and that should trigger the build of ftrace.c

There isn't anything in the arch specific ftrace.c that depends purely on
ftrace. It's all stuff that depends on the above specific ftrace options
that have some arch specific implementation.

> > diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
> > index ce1f3e4..b34171e 100644
> > --- a/arch/powerpc/kernel/ftrace.c
> > +++ b/arch/powerpc/kernel/ftrace.c
> > @@ -22,6 +22,7 @@
> >  #include <asm/cacheflush.h>
> >  #include <asm/code-patching.h>
> >  #include <asm/ftrace.h>
> > +#include <asm/syscall.h>
> >  
> > 
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> > @@ -600,3 +601,15 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
> >      }
> >  }
> >  #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> > +
> > +#if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_PPC64)
> 
> Does 32-bit just work using the existing routines? Or do we not support
> it on 32-bit (though that's not what your Kconfig change said).

It should work with the existing routines - I still need to test this on
feugo to make sure, but following from /arch/powerpc/kernel/systbl.S it
seems that the symbol names should match the function names and the
system call table is a trivial lookup table, both of which will work
with the generic implementation.

> > +unsigned long __init arch_syscall_addr(int nr)
> > +{
> > +    return (unsigned long)sys_call_table[nr*2];
> 
> That's the cast I was referring to.

Ok, I'll change that in v2.

> > +}
> > +
> > +inline bool arch_syscall_match_sym_name(const char *sym, const char *name)
> > +{
> > +    return (!strcmp(sym + 4, name + 3));
> 
> So the +4 is to skip ".sys" and the +3 is to skip "sys" ? That could use
> a comment IMHO :)

Yeah that's right, I'll add a comment there to clarify that.

> 
> cheers


Thanks for the feedback,
-Ian

^ permalink raw reply

* Re: [PATCH 3/4] ftrace syscalls: Allow arch specific syscall symbol matching
From: Ian Munsie @ 2010-05-14  2:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Frederic Weisbecker, Jason Baron, linux-kernel, Steven Rostedt,
	Ingo Molnar, Paul Mackerras, linuxppc-dev
In-Reply-To: <1273794896.21352.203.camel@pasglop>

Excerpts from Benjamin Herrenschmidt's message of Fri May 14 09:54:56 +1000 2010:
> On Thu, 2010-05-13 at 17:43 +1000, Ian Munsie wrote:
> > From: Ian Munsie <imunsie@au1.ibm.com>
> > 
> > Some architectures have unusual symbol names and the generic code to
> > match the symbol name with the function name for the syscall metadata
> > will fail. For example, symbols on PPC64 start with a period and the
> > generic code will fail to match them.
> > 
> > This patch splits out the match logic into a standalone weak function
> > that can be overridden on archs with unusual symbol names.
> > 
> > Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> > ---
> 
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> Ian, I assume you will implement the support for the "special" ppc_*
> syscalls via a subsequent patch and not a respin of this one right ?
> 
> Cheers,
> Ben.

Yes, that will be in a separate patch or two.

Cheers,
-Ian

^ permalink raw reply

* RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check
From: Liu Dave-R63238 @ 2010-05-14  2:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Kumar Gala; +Cc: linuxppc-dev, Jin Qing-B24347
In-Reply-To: <1273787172.21352.198.camel@pasglop>

> Not looking at the code right now ... but do we have the same=20
> issue on 64e ?

Aaron pointed the issue on FSL BookE.
http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079738.html

^ permalink raw reply

* [PATCH v2] kexec-tools, ppc64: Fix segfault parsing DR memory property
From: Matt Evans @ 2010-05-14  3:07 UTC (permalink / raw)
  To: kexec; +Cc: linuxppc-dev, Michael Neuling, Simon Horman

add_dyn_reconf_usable_mem_property() iterates over memory spans
in /ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory and intersects
these with usablemem_rgns ranges.  In doing so it used an unchecked
fixed-size array which will overrun on machines with lots of LMBs.

This patch removes the fixed-sized arrays from
add_dyn_reconf_usable_mem_property() and add_usable_mem_property(), in lieu of
malloc/realloc/free.

Signed-off-by: Matt Evans <matt@ozlabs.org>
---
 kexec/arch/ppc64/fs2dt.c |   82 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c
index 762bf04..4400f13 100644
--- a/kexec/arch/ppc64/fs2dt.c
+++ b/kexec/arch/ppc64/fs2dt.c
@@ -37,7 +37,7 @@
 #define NAMESPACE 16384		/* max bytes for property names */
 #define INIT_TREE_WORDS 65536	/* Initial num words for prop values */
 #define MEMRESERVE 256		/* max number of reserved memory blocks */
-#define MAX_MEMORY_RANGES 1024
+#define MEM_RANGE_CHUNK_SZ 2048 /* Initial num dwords for mem ranges */
 
 static char pathname[MAXPATH], *pathstart;
 static char propnames[NAMESPACE] = { 0 };
@@ -148,7 +148,8 @@ static void add_dyn_reconf_usable_mem_property(int fd)
 {
 	char fname[MAXPATH], *bname;
 	uint64_t buf[32];
-	uint64_t ranges[2*MAX_MEMORY_RANGES];
+	uint64_t *ranges;
+	int ranges_size = MEM_RANGE_CHUNK_SZ;
 	uint64_t base, end, loc_base, loc_end;
 	size_t i, rngs_cnt, range;
 	int rlen = 0;
@@ -165,6 +166,11 @@ static void add_dyn_reconf_usable_mem_property(int fd)
 		die("unrecoverable error: error seeking in \"%s\": %s\n",
 			pathname, strerror(errno));
 
+	ranges = malloc(ranges_size*8);
+	if (!ranges)
+		die("unrecoverable error: can't alloc %d bytes for ranges.\n",
+		    ranges_size*8);
+
 	rlen = 0;
 	for (i = 0; i < num_of_lmbs; i++) {
 		if (read(fd, buf, 24) < 0)
@@ -180,24 +186,57 @@ static void add_dyn_reconf_usable_mem_property(int fd)
 
 		rngs_cnt = 0;
 		for (range = 0; range < usablemem_rgns.size; range++) {
+			int add = 0;
 			loc_base = usablemem_rgns.ranges[range].start;
 			loc_end = usablemem_rgns.ranges[range].end;
 			if (loc_base >= base && loc_end <= end) {
-				ranges[rlen++] = loc_base;
-				ranges[rlen++] = loc_end - loc_base;
-				rngs_cnt++;
+				add = 1;
 			} else if (base < loc_end && end > loc_base) {
 				if (loc_base < base)
 					loc_base = base;
 				if (loc_end > end)
 					loc_end = end;
+				add = 1;
+			}
+
+			if (add) {
+				if (rlen >= (ranges_size-2)) {
+					ranges_size += MEM_RANGE_CHUNK_SZ;
+					ranges = realloc(ranges, ranges_size*8);
+					if (!ranges)
+						die("unrecoverable error: can't"
+						    " realloc %d bytes for"
+						    " ranges.\n",
+						    ranges_size*8);
+				}
 				ranges[rlen++] = loc_base;
 				ranges[rlen++] = loc_end - loc_base;
 				rngs_cnt++;
 			}
 		}
-		/* Store the count of (base, size) duple */
-		ranges[tmp_indx] = rngs_cnt;
+		if (rngs_cnt == 0) {
+			/* We still need to add a counter for every LMB because
+			 * the kernel parsing code is dumb.  We just have
+			 * a zero in this case, with no following base/len.
+			 */
+			ranges[tmp_indx] = 0;
+			/* rlen is already just tmp_indx+1 as we didn't write
+			 * anything.  Check array size here, as we'll probably
+			 * go on for a while writing zeros now.
+			 */
+			if (rlen >= (ranges_size-1)) {
+				ranges_size += MEM_RANGE_CHUNK_SZ;
+				ranges = realloc(ranges, ranges_size*8);
+				if (!ranges)
+					die("unrecoverable error: can't"
+					    " realloc %d bytes for"
+					    " ranges.\n",
+					    ranges_size*8);
+			}
+		} else {
+			/* Store the count of (base, size) duple */
+			ranges[tmp_indx] = rngs_cnt;
+		}
 	}
 		
 	rlen = rlen * sizeof(uint64_t);
@@ -210,7 +249,8 @@ static void add_dyn_reconf_usable_mem_property(int fd)
 	*dt++ = propnum("linux,drconf-usable-memory");
 	if ((rlen >= 8) && ((unsigned long)dt & 0x4))
 		dt++;
-	memcpy(dt, &ranges, rlen);
+	memcpy(dt, ranges, rlen);
+	free(ranges);
 	dt += (rlen + 3)/4;
 }
 
@@ -218,7 +258,8 @@ static void add_usable_mem_property(int fd, size_t len)
 {
 	char fname[MAXPATH], *bname;
 	uint64_t buf[2];
-	uint64_t ranges[2*MAX_MEMORY_RANGES];
+	uint64_t *ranges;
+	int ranges_size = MEM_RANGE_CHUNK_SZ;
 	uint64_t base, end, loc_base, loc_end;
 	size_t range;
 	int rlen = 0;
@@ -247,17 +288,33 @@ static void add_usable_mem_property(int fd, size_t len)
 	base = buf[0];
 	end = base + buf[1];
 
+	ranges = malloc(ranges_size*8);
+	if (!ranges)
+		die("unrecoverable error: can't alloc %d bytes for ranges.\n",
+		    ranges_size*8);
+
 	for (range = 0; range < usablemem_rgns.size; range++) {
+		int add = 0;
 		loc_base = usablemem_rgns.ranges[range].start;
 		loc_end = usablemem_rgns.ranges[range].end;
 		if (loc_base >= base && loc_end <= end) {
-			ranges[rlen++] = loc_base;
-			ranges[rlen++] = loc_end - loc_base;
+			add = 1;
 		} else if (base < loc_end && end > loc_base) {
 			if (loc_base < base)
 				loc_base = base;
 			if (loc_end > end)
 				loc_end = end;
+			add = 1;
+		}
+		if (add) {
+			if (rlen >= (ranges_size-2)) {
+				ranges_size += MEM_RANGE_CHUNK_SZ;
+				ranges = realloc(ranges, ranges_size*8);
+				if (!ranges)
+					die("unrecoverable error: can't realloc"
+					    "%d bytes for ranges.\n",
+					    ranges_size*8);
+			}
 			ranges[rlen++] = loc_base;
 			ranges[rlen++] = loc_end - loc_base;
 		}
@@ -283,7 +340,8 @@ static void add_usable_mem_property(int fd, size_t len)
 	*dt++ = propnum("linux,usable-memory");
 	if ((rlen >= 8) && ((unsigned long)dt & 0x4))
 		dt++;
-	memcpy(dt,&ranges,rlen);
+	memcpy(dt, ranges, rlen);
+	free(ranges);
 	dt += (rlen + 3)/4;
 }
 
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check
From: Benjamin Herrenschmidt @ 2010-05-14  3:36 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, Jin Qing
In-Reply-To: <0710E9DF-66DF-4D24-95DE-A04E6D869826@kernel.crashing.org>

On Thu, 2010-05-13 at 14:14 -0500, Kumar Gala wrote:
> On May 7, 2010, at 3:38 AM, Li Yang wrote:
> 
> > Check the user/supervisor execution permission base on the code address.
> > This fixes the following oops on module loading or removing.
> > 
> > Unable to handle kernel paging request for instruction fetch
> > Faulting instruction address: 0xf938d040
> > Oops: Kernel access of bad area, sig: 11 [#1]
> > 
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > Signed-off-by: Jin Qing <b24347@freescale.com>
> > ---
> > arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
> > 1 files changed, 10 insertions(+), 3 deletions(-)
> 
> applied to merge.
> 
> I've updated the commit message to be a bit more clear on why we need to do this.
> 
> Thanks for figuring this out.

Should I pull & send to Linus ?

Cheers,
Ben.

^ permalink raw reply

* RE: [PATCH] pte_fsl_booke: fix instruction TLB error permission check
From: Benjamin Herrenschmidt @ 2010-05-14  3:42 UTC (permalink / raw)
  To: Liu Dave-R63238; +Cc: Jin Qing-B24347, linuxppc-dev, Li Yang-R58472
In-Reply-To: <D7CCA83BB0796C49BC0BB53B6AB12089B974F9@zch01exm21.fsl.freescale.net>

On Fri, 2010-05-14 at 08:53 +0800, Liu Dave-R63238 wrote:
> > I've updated the commit message to be a bit more clear on why 
> > we need to do this.
> 
> I'm curious why the _PAGE_EXEC have different definition in pte-book3e.h
> and pte-fsl-booke.h?
> 
> It is UX permission in pte-book3e, but is SX permission in
> pte-fsl-booke.h.

Oh well, there's a whole history here :-)

I'm not 100% sure of the "old" fsl-booke, but pte-book3e uses UX since
that's what _PAGE_EXEC really represents from a kernel standpoint,
user-execute.

On book3e, we need to account for the HW tablewalk, which means that the
PTE can be loaded as-is by the HW. We thus don't get a chance to test
whether we are user or supervisor or change the bits accordingly. We
thus need _PAGE_EXEC to fall right into UX since that's really what the
kernel manipulates it for.

It also means that user pages will not get execute permission in
supervisor mode which is a good thing :-)

For kernel-execute, we have a specific permission, which should be
obtained via PAGE_KERNEL_EXEC in vmalloc_exec().

Cheers,
Ben.

^ permalink raw reply

* [PATCH 1/2] powerpc, kexec: Fix race in kexec shutdown
From: Michael Neuling @ 2010-05-14  3:57 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, kexec, Anton Blanchard, jlarrew
In-Reply-To: <1273559307.55666.645438163872.qpush@pale>

In kexec_prepare_cpus, the primary CPU IPIs the secondary CPUs to
kexec_smp_down().  kexec_smp_down() calls kexec_smp_wait() which sets
the hw_cpu_id() to -1.  The primary does this while leaving IRQs on
which means the primary can take a timer interrupt which can lead to
the IPIing one of the secondary CPUs (say, for a scheduler re-balance)
but since the secondary CPU now has a hw_cpu_id = -1, we IPI CPU
-1... Kaboom!

We are hitting this case regularly on POWER7 machines.  

There is also a second race, where the primary will tear down the MMU
mappings before knowing the secondaries have entered real mode.  

Also, the secondaries are clearing out any pending IPIs before
guaranteeing that no more will be received.  

This changes kexec_prepare_cpus() so that we turn off IRQs in the
primary CPU much earlier.  It adds a paca flag to say that the
secondaries have entered the kexec_smp_down() IPI and turned off IRQs,
rather than overloading hw_cpu_id with -1.  This new paca flag is
again used to in indicate when the secondaries has entered real mode.

It also ensures that all CPUs have their IRQs off before we clear out
any pending IPI requests (in kexec_cpu_down()) to ensure there are no
trailing IPIs left unacknowledged.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---

 arch/powerpc/include/asm/kexec.h       |    4 ++
 arch/powerpc/include/asm/paca.h        |    1 
 arch/powerpc/kernel/asm-offsets.c      |    1 
 arch/powerpc/kernel/machine_kexec_64.c |   48 +++++++++++++++++++++++----------
 arch/powerpc/kernel/misc_64.S          |    8 +++--
 arch/powerpc/kernel/paca.c             |    2 +
 6 files changed, 47 insertions(+), 17 deletions(-)

Index: linux-2.6-ozlabs/arch/powerpc/include/asm/kexec.h
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/include/asm/kexec.h
+++ linux-2.6-ozlabs/arch/powerpc/include/asm/kexec.h
@@ -31,6 +31,10 @@
 #define KEXEC_ARCH KEXEC_ARCH_PPC
 #endif
 
+#define KEXEC_STATE_NONE 0
+#define KEXEC_STATE_IRQS_OFF 1
+#define KEXEC_STATE_REAL_MODE 2
+
 #ifndef __ASSEMBLY__
 #include <linux/cpumask.h>
 #include <asm/reg.h>
Index: linux-2.6-ozlabs/arch/powerpc/include/asm/paca.h
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/include/asm/paca.h
+++ linux-2.6-ozlabs/arch/powerpc/include/asm/paca.h
@@ -82,6 +82,7 @@ struct paca_struct {
 	s16 hw_cpu_id;			/* Physical processor number */
 	u8 cpu_start;			/* At startup, processor spins until */
 					/* this becomes non-zero. */
+	u8 kexec_state;		/* set when kexec down has irqs off */
 #ifdef CONFIG_PPC_STD_MMU_64
 	struct slb_shadow *slb_shadow_ptr;
 
Index: linux-2.6-ozlabs/arch/powerpc/kernel/asm-offsets.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/asm-offsets.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/asm-offsets.c
@@ -184,6 +184,7 @@ int main(void)
 #endif /* CONFIG_PPC_STD_MMU_64 */
 	DEFINE(PACAEMERGSP, offsetof(struct paca_struct, emergency_sp));
 	DEFINE(PACAHWCPUID, offsetof(struct paca_struct, hw_cpu_id));
+	DEFINE(PACAKEXECSTATE, offsetof(struct paca_struct, kexec_state));
 	DEFINE(PACA_STARTPURR, offsetof(struct paca_struct, startpurr));
 	DEFINE(PACA_STARTSPURR, offsetof(struct paca_struct, startspurr));
 	DEFINE(PACA_USER_TIME, offsetof(struct paca_struct, user_time));
Index: linux-2.6-ozlabs/arch/powerpc/kernel/machine_kexec_64.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/machine_kexec_64.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/machine_kexec_64.c
@@ -155,33 +155,38 @@ void kexec_copy_flush(struct kimage *ima
 
 #ifdef CONFIG_SMP
 
-/* FIXME: we should schedule this function to be called on all cpus based
- * on calling the interrupts, but we would like to call it off irq level
- * so that the interrupt controller is clean.
- */
+static int kexec_all_irq_disabled = 0;
+
 static void kexec_smp_down(void *arg)
 {
+	local_irq_disable();
+	mb(); /* make sure our irqs are disabled before we say they are */
+	get_paca()->kexec_state = KEXEC_STATE_IRQS_OFF;
+	while(kexec_all_irq_disabled == 0)
+		cpu_relax();
+	mb(); /* make sure all irqs are disabled before this */
+	/*
+	 * Now every CPU has IRQs off, we can clear out any pending
+	 * IPIs and be sure that no more will come in after this.
+	 */
 	if (ppc_md.kexec_cpu_down)
 		ppc_md.kexec_cpu_down(0, 1);
 
-	local_irq_disable();
 	kexec_smp_wait();
 	/* NOTREACHED */
 }
 
-static void kexec_prepare_cpus(void)
+static void kexec_prepare_cpus_wait(int wait_state)
 {
 	int my_cpu, i, notified=-1;
 
-	smp_call_function(kexec_smp_down, NULL, /* wait */0);
 	my_cpu = get_cpu();
-
-	/* check the others cpus are now down (via paca hw cpu id == -1) */
+	/* check the others cpus are now down (via paca kexec_irqs_off == 1) */
 	for (i=0; i < NR_CPUS; i++) {
 		if (i == my_cpu)
 			continue;
 
-		while (paca[i].hw_cpu_id != -1) {
+		while (paca[i].kexec_state < wait_state) {
 			barrier();
 			if (!cpu_possible(i)) {
 				printk("kexec: cpu %d hw_cpu_id %d is not"
@@ -201,20 +206,35 @@ static void kexec_prepare_cpus(void)
 			}
 			if (i != notified) {
 				printk( "kexec: waiting for cpu %d (physical"
-						" %d) to go down\n",
-						i, paca[i].hw_cpu_id);
+						" %d) to enter %i state\n",
+					i, paca[i].hw_cpu_id, wait_state);
 				notified = i;
 			}
 		}
 	}
+	mb();
+}
+
+static void kexec_prepare_cpus(void)
+{
+
+	smp_call_function(kexec_smp_down, NULL, /* wait */0);
+	local_irq_disable();
+	mb(); /* make sure IRQs are disabled before we say they are */
+	get_paca()->kexec_state = KEXEC_STATE_IRQS_OFF;
+
+	kexec_prepare_cpus_wait(KEXEC_STATE_IRQS_OFF);
+	/* we are sure every CPU has IRQs off at this point */
+	kexec_all_irq_disabled = 1;
 
 	/* after we tell the others to go down */
 	if (ppc_md.kexec_cpu_down)
 		ppc_md.kexec_cpu_down(0, 0);
 
-	put_cpu();
+	/* Before removing MMU mapings make sure all CPUs have entered real mode */
+	kexec_prepare_cpus_wait(KEXEC_STATE_REAL_MODE);
 
-	local_irq_disable();
+	put_cpu();
 }
 
 #else /* ! SMP */
Index: linux-2.6-ozlabs/arch/powerpc/kernel/misc_64.S
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/misc_64.S
+++ linux-2.6-ozlabs/arch/powerpc/kernel/misc_64.S
@@ -24,6 +24,7 @@
 #include <asm/asm-offsets.h>
 #include <asm/cputable.h>
 #include <asm/thread_info.h>
+#include <asm/kexec.h>
 
 	.text
 
@@ -471,6 +472,10 @@ _GLOBAL(kexec_wait)
 1:	mflr	r5
 	addi	r5,r5,kexec_flag-1b
 
+	li	r4,KEXEC_STATE_REAL_MODE
+	stb	r4,PACAKEXECSTATE(r13)
+	SYNC
+
 99:	HMT_LOW
 #ifdef CONFIG_KEXEC		/* use no memory without kexec */
 	lwz	r4,0(r5)
@@ -494,14 +499,11 @@ kexec_flag:
  * note: this is a terminal routine, it does not save lr
  *
  * get phys id from paca
- * set paca id to -1 to say we got here
  * switch to real mode
  * join other cpus in kexec_wait(phys_id)
  */
 _GLOBAL(kexec_smp_wait)
 	lhz	r3,PACAHWCPUID(r13)
-	li	r4,-1
-	sth	r4,PACAHWCPUID(r13)	/* let others know we left */
 	bl	real_mode
 	b	.kexec_wait
 
Index: linux-2.6-ozlabs/arch/powerpc/kernel/paca.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/paca.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/paca.c
@@ -18,6 +18,7 @@
 #include <asm/pgtable.h>
 #include <asm/iseries/lpar_map.h>
 #include <asm/iseries/hv_types.h>
+#include <asm/kexec.h>
 
 /* This symbol is provided by the linker - let it fill in the paca
  * field correctly */
@@ -97,6 +98,7 @@ void __init initialise_paca(struct paca_
 	new_paca->kernelbase = (unsigned long) _stext;
 	new_paca->kernel_msr = MSR_KERNEL;
 	new_paca->hw_cpu_id = 0xffff;
+	new_paca->kexec_state = KEXEC_STATE_NONE;
 	new_paca->__current = &init_task;
 #ifdef CONFIG_PPC_STD_MMU_64
 	new_paca->slb_shadow_ptr = &slb_shadow[cpu];

^ permalink raw reply

* [PATCH 2/2] powerpc, kdump: Fix race in kdump shutdown
From: Michael Neuling @ 2010-05-14  3:57 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, kexec, Anton Blanchard, jlarrew
In-Reply-To: <1273559307.55666.645438163872.qpush@pale>

When we are crashing, the crashing/primary CPU IPIs the secondaries to
turn off IRQs, go into real mode and wait in kexec_wait.  While this
is happening, the primary tears down all the MMU maps.  Unfortunately
the primary doesn't check to make sure the secondaries have entered
real mode before doing this.

On PHYP machines, the secondaries can take a long time shutting down
the IRQ controller as RTAS calls are need.  These RTAS calls need to
be serialised which resilts in the secondaries contending in
lock_rtas() and hence taking a long time to shut down.

We've hit this on large POWER7 machines, where some secondaries are
still waiting in lock_rtas(), when the primary tears down the HPTEs.

This patch makes sure all secondaries are in real mode before the
primary tears down the MMU.  It uses the new kexec_state entry in the
paca.  It times out if the secondaries don't reach real mode after
10sec.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---

 arch/powerpc/kernel/crash.c |   28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Index: linux-2.6-ozlabs/arch/powerpc/kernel/crash.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/crash.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/crash.c
@@ -162,6 +162,33 @@ static void crash_kexec_prepare_cpus(int
 	/* Leave the IPI callback set */
 }
 
+/* wait for all the CPUs to hit real mode but timeout if they don't come in */
+static void crash_kexec_wait_realmode(int cpu)
+{
+	unsigned int msecs;
+	int i;
+
+	/* check the others cpus are now down (via paca kexec_irqs_off == 1) */
+	msecs = 10000;
+	for (i=0; i < NR_CPUS && msecs > 0; i++) {
+		if (i == cpu)
+			continue;
+
+		while (paca[i].kexec_state < KEXEC_STATE_REAL_MODE) {
+			barrier();
+			if (!cpu_possible(i)) {
+				break;
+			}
+			if (!cpu_online(i)) {
+				break;
+			}
+			msecs--;
+			mdelay(1);
+		}
+	}
+	mb();
+}
+
 /*
  * This function will be called by secondary cpus or by kexec cpu
  * if soft-reset is activated to stop some CPUs.
@@ -419,6 +446,7 @@ void default_machine_crash_shutdown(stru
 	crash_kexec_prepare_cpus(crashing_cpu);
 	cpu_set(crashing_cpu, cpus_in_crash);
 	crash_kexec_stop_spus();
+	crash_kexec_wait_realmode(crashing_cpu);
 	if (ppc_md.kexec_cpu_down)
 		ppc_md.kexec_cpu_down(1, 0);
 }

^ permalink raw reply

* Re: [PATCH] pte_fsl_booke: fix instruction TLB error permission check
From: Kumar Gala @ 2010-05-14  4:19 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Jin Qing
In-Reply-To: <1273808175.21352.481.camel@pasglop>


On May 13, 2010, at 10:36 PM, Benjamin Herrenschmidt wrote:

> On Thu, 2010-05-13 at 14:14 -0500, Kumar Gala wrote:
>> On May 7, 2010, at 3:38 AM, Li Yang wrote:
>>=20
>>> Check the user/supervisor execution permission base on the code =
address.
>>> This fixes the following oops on module loading or removing.
>>>=20
>>> Unable to handle kernel paging request for instruction fetch
>>> Faulting instruction address: 0xf938d040
>>> Oops: Kernel access of bad area, sig: 11 [#1]
>>>=20
>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>> Signed-off-by: Jin Qing <b24347@freescale.com>
>>> ---
>>> arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
>>> 1 files changed, 10 insertions(+), 3 deletions(-)
>>=20
>> applied to merge.
>>=20
>> I've updated the commit message to be a bit more clear on why we need =
to do this.
>>=20
>> Thanks for figuring this out.
>=20
> Should I pull & send to Linus ?

No, I've got another patch to fix a FTRACE issue with SMP CPU bringup.  =
Will post a tree for you here shortly.

- k=

^ permalink raw reply

* [PATCH] powerpc/fsl-booke: Move loadcam_entry back to asm code to fix SMP ftrace
From: Kumar Gala @ 2010-05-14  4:25 UTC (permalink / raw)
  To: linuxppc-dev

When we build with ftrace enabled its possible that loadcam_entry would
have used the stack pointer (even though the code doesn't need it).  We
call loadcam_entry in __secondary_start before the stack is setup.  To
ensure that loadcam_entry doesn't use the stack pointer the easiest
solution is to just have it in asm code.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/kernel/asm-offsets.c |    8 ++++++++
 arch/powerpc/mm/fsl_booke_mmu.c   |   25 +++----------------------
 arch/powerpc/mm/mmu_decl.h        |   10 +++++++++-
 arch/powerpc/mm/tlb_nohash_low.S  |   28 ++++++++++++++++++++++++++++
 4 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index c09138d..b894721 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -447,6 +447,14 @@ int main(void)
 	DEFINE(PGD_T_LOG2, PGD_T_LOG2);
 	DEFINE(PTE_T_LOG2, PTE_T_LOG2);
 #endif
+#ifdef CONFIG_FSL_BOOKE
+	DEFINE(TLBCAM_SIZE, sizeof(struct tlbcam));
+	DEFINE(TLBCAM_MAS0, offsetof(struct tlbcam, MAS0));
+	DEFINE(TLBCAM_MAS1, offsetof(struct tlbcam, MAS1));
+	DEFINE(TLBCAM_MAS2, offsetof(struct tlbcam, MAS2));
+	DEFINE(TLBCAM_MAS3, offsetof(struct tlbcam, MAS3));
+	DEFINE(TLBCAM_MAS7, offsetof(struct tlbcam, MAS7));
+#endif
 
 #ifdef CONFIG_KVM_EXIT_TIMING
 	DEFINE(VCPU_TIMING_EXIT_TBU, offsetof(struct kvm_vcpu,
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 1ed6b52..cdc7526 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -2,7 +2,7 @@
  * Modifications by Kumar Gala (galak@kernel.crashing.org) to support
  * E500 Book E processors.
  *
- * Copyright 2004 Freescale Semiconductor, Inc
+ * Copyright 2004,2010 Freescale Semiconductor, Inc.
  *
  * This file contains the routines for initializing the MMU
  * on the 4xx series of chips.
@@ -56,19 +56,13 @@
 
 unsigned int tlbcam_index;
 
-#define NUM_TLBCAMS	(64)
 
 #if defined(CONFIG_LOWMEM_CAM_NUM_BOOL) && (CONFIG_LOWMEM_CAM_NUM >= NUM_TLBCAMS)
 #error "LOWMEM_CAM_NUM must be less than NUM_TLBCAMS"
 #endif
 
-struct tlbcam {
-	u32	MAS0;
-	u32	MAS1;
-	unsigned long	MAS2;
-	u32	MAS3;
-	u32	MAS7;
-} TLBCAM[NUM_TLBCAMS];
+#define NUM_TLBCAMS	(64)
+struct tlbcam TLBCAM[NUM_TLBCAMS];
 
 struct tlbcamrange {
 	unsigned long start;
@@ -109,19 +103,6 @@ unsigned long p_mapped_by_tlbcam(phys_addr_t pa)
 	return 0;
 }
 
-void loadcam_entry(int idx)
-{
-	mtspr(SPRN_MAS0, TLBCAM[idx].MAS0);
-	mtspr(SPRN_MAS1, TLBCAM[idx].MAS1);
-	mtspr(SPRN_MAS2, TLBCAM[idx].MAS2);
-	mtspr(SPRN_MAS3, TLBCAM[idx].MAS3);
-
-	if (mmu_has_feature(MMU_FTR_BIG_PHYS))
-		mtspr(SPRN_MAS7, TLBCAM[idx].MAS7);
-
-	asm volatile("isync;tlbwe;isync" : : : "memory");
-}
-
 /*
  * Set up one of the I/D BAT (block address translation) register pairs.
  * The parameters are not checked; in particular size must be a power
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index d49a775..0591f25 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -149,7 +149,15 @@ extern unsigned long mmu_mapin_ram(unsigned long top);
 extern void MMU_init_hw(void);
 extern unsigned long mmu_mapin_ram(unsigned long top);
 extern void adjust_total_lowmem(void);
-
+extern void loadcam_entry(unsigned int index);
+
+struct tlbcam {
+	u32	MAS0;
+	u32	MAS1;
+	unsigned long	MAS2;
+	u32	MAS3;
+	u32	MAS7;
+};
 #elif defined(CONFIG_PPC32)
 /* anything 32-bit except 4xx or 8xx */
 extern void MMU_init_hw(void);
diff --git a/arch/powerpc/mm/tlb_nohash_low.S b/arch/powerpc/mm/tlb_nohash_low.S
index bbdc5b5..8656ecf 100644
--- a/arch/powerpc/mm/tlb_nohash_low.S
+++ b/arch/powerpc/mm/tlb_nohash_low.S
@@ -271,3 +271,31 @@ _GLOBAL(set_context)
 #else
 #error Unsupported processor type !
 #endif
+
+#if defined(CONFIG_FSL_BOOKE)
+/*
+ * extern void loadcam_entry(unsigned int index)
+ *
+ * Load TLBCAM[index] entry in to the L2 CAM MMU
+ */
+_GLOBAL(loadcam_entry)
+	LOAD_REG_ADDR(r4, TLBCAM)
+	mulli	r5,r3,TLBCAM_SIZE
+	add	r3,r5,r4
+	lwz	r4,TLBCAM_MAS0(r3)
+	mtspr	SPRN_MAS0,r4
+	lwz	r4,TLBCAM_MAS1(r3)
+	mtspr	SPRN_MAS1,r4
+	PPC_LL	r4,TLBCAM_MAS2(r3)
+	mtspr	SPRN_MAS2,r4
+	lwz	r4,TLBCAM_MAS3(r3)
+	mtspr	SPRN_MAS3,r4
+BEGIN_MMU_FTR_SECTION
+	lwz	r4,TLBCAM_MAS7(r3)
+	mtspr	SPRN_MAS7,r4
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_BIG_PHYS)
+	isync
+	tlbwe
+	isync
+	blr
+#endif
-- 
1.6.0.6

^ permalink raw reply related

* [git pull] Please pull powerpc.git merge branch
From: Kumar Gala @ 2010-05-14  4:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

The following changes since commit 131c6c9eddfa252e376edb4aeff9c7fe1b96a798:
  Benjamin Herrenschmidt (1):
        Merge commit 'kumar/merge' into merge

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git merge

Kumar Gala (1):
      powerpc/fsl-booke: Move loadcam_entry back to asm code to fix SMP ftrace

Li Yang (1):
      powerpc/fsl-booke: Fix InstructionTLBError execute permission check

 arch/powerpc/kernel/asm-offsets.c    |    8 ++++++++
 arch/powerpc/kernel/head_fsl_booke.S |   13 ++++++++++---
 arch/powerpc/mm/fsl_booke_mmu.c      |   25 +++----------------------
 arch/powerpc/mm/mmu_decl.h           |   10 +++++++++-
 arch/powerpc/mm/tlb_nohash_low.S     |   28 ++++++++++++++++++++++++++++
 5 files changed, 58 insertions(+), 26 deletions(-)

^ permalink raw reply

* [PATCH 1/2] powerpc, kexec: Fix race in kexec shutdown
From: Michael Neuling @ 2010-05-14  5:40 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, kexec, Anton Blanchard, jlarrew
In-Reply-To: <20100514035700.8B6EDCED6E@localhost.localdomain>

In kexec_prepare_cpus, the primary CPU IPIs the secondary CPUs to
kexec_smp_down().  kexec_smp_down() calls kexec_smp_wait() which sets
the hw_cpu_id() to -1.  The primary does this while leaving IRQs on
which means the primary can take a timer interrupt which can lead to
the IPIing one of the secondary CPUs (say, for a scheduler re-balance)
but since the secondary CPU now has a hw_cpu_id = -1, we IPI CPU
-1... Kaboom!

We are hitting this case regularly on POWER7 machines.  

There is also a second race, where the primary will tear down the MMU
mappings before knowing the secondaries have entered real mode.  

Also, the secondaries are clearing out any pending IPIs before
guaranteeing that no more will be received.  

This changes kexec_prepare_cpus() so that we turn off IRQs in the
primary CPU much earlier.  It adds a paca flag to say that the
secondaries have entered the kexec_smp_down() IPI and turned off IRQs,
rather than overloading hw_cpu_id with -1.  This new paca flag is
again used to in indicate when the secondaries has entered real mode.

It also ensures that all CPUs have their IRQs off before we clear out
any pending IPI requests (in kexec_cpu_down()) to ensure there are no
trailing IPIs left unacknowledged.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
Oops, missed quilt refresh in the last version...
---

 arch/powerpc/include/asm/kexec.h       |    4 ++
 arch/powerpc/include/asm/paca.h        |    1 
 arch/powerpc/kernel/asm-offsets.c      |    1 
 arch/powerpc/kernel/machine_kexec_64.c |   48 +++++++++++++++++++++++----------
 arch/powerpc/kernel/misc_64.S          |    8 +++--
 arch/powerpc/kernel/paca.c             |    2 +
 6 files changed, 47 insertions(+), 17 deletions(-)

Index: linux-2.6-ozlabs/arch/powerpc/include/asm/kexec.h
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/include/asm/kexec.h
+++ linux-2.6-ozlabs/arch/powerpc/include/asm/kexec.h
@@ -31,6 +31,10 @@
 #define KEXEC_ARCH KEXEC_ARCH_PPC
 #endif
 
+#define KEXEC_STATE_NONE 0
+#define KEXEC_STATE_IRQS_OFF 1
+#define KEXEC_STATE_REAL_MODE 2
+
 #ifndef __ASSEMBLY__
 #include <linux/cpumask.h>
 #include <asm/reg.h>
Index: linux-2.6-ozlabs/arch/powerpc/include/asm/paca.h
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/include/asm/paca.h
+++ linux-2.6-ozlabs/arch/powerpc/include/asm/paca.h
@@ -82,6 +82,7 @@ struct paca_struct {
 	s16 hw_cpu_id;			/* Physical processor number */
 	u8 cpu_start;			/* At startup, processor spins until */
 					/* this becomes non-zero. */
+	u8 kexec_state;		/* set when kexec down has irqs off */
 #ifdef CONFIG_PPC_STD_MMU_64
 	struct slb_shadow *slb_shadow_ptr;
 
Index: linux-2.6-ozlabs/arch/powerpc/kernel/asm-offsets.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/asm-offsets.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/asm-offsets.c
@@ -184,6 +184,7 @@ int main(void)
 #endif /* CONFIG_PPC_STD_MMU_64 */
 	DEFINE(PACAEMERGSP, offsetof(struct paca_struct, emergency_sp));
 	DEFINE(PACAHWCPUID, offsetof(struct paca_struct, hw_cpu_id));
+	DEFINE(PACAKEXECSTATE, offsetof(struct paca_struct, kexec_state));
 	DEFINE(PACA_STARTPURR, offsetof(struct paca_struct, startpurr));
 	DEFINE(PACA_STARTSPURR, offsetof(struct paca_struct, startspurr));
 	DEFINE(PACA_USER_TIME, offsetof(struct paca_struct, user_time));
Index: linux-2.6-ozlabs/arch/powerpc/kernel/machine_kexec_64.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/machine_kexec_64.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/machine_kexec_64.c
@@ -155,33 +155,38 @@ void kexec_copy_flush(struct kimage *ima
 
 #ifdef CONFIG_SMP
 
-/* FIXME: we should schedule this function to be called on all cpus based
- * on calling the interrupts, but we would like to call it off irq level
- * so that the interrupt controller is clean.
- */
+static int kexec_all_irq_disabled = 0;
+
 static void kexec_smp_down(void *arg)
 {
+	local_irq_disable();
+	mb(); /* make sure our irqs are disabled before we say they are */
+	get_paca()->kexec_state = KEXEC_STATE_IRQS_OFF;
+	while(kexec_all_irq_disabled == 0)
+		cpu_relax();
+	mb(); /* make sure all irqs are disabled before this */
+	/*
+	 * Now every CPU has IRQs off, we can clear out any pending
+	 * IPIs and be sure that no more will come in after this.
+	 */
 	if (ppc_md.kexec_cpu_down)
 		ppc_md.kexec_cpu_down(0, 1);
 
-	local_irq_disable();
 	kexec_smp_wait();
 	/* NOTREACHED */
 }
 
-static void kexec_prepare_cpus(void)
+static void kexec_prepare_cpus_wait(int wait_state)
 {
 	int my_cpu, i, notified=-1;
 
-	smp_call_function(kexec_smp_down, NULL, /* wait */0);
 	my_cpu = get_cpu();
-
-	/* check the others cpus are now down (via paca hw cpu id == -1) */
+	/* Make sure each CPU has atleast made it to the state we need */
 	for (i=0; i < NR_CPUS; i++) {
 		if (i == my_cpu)
 			continue;
 
-		while (paca[i].hw_cpu_id != -1) {
+		while (paca[i].kexec_state < wait_state) {
 			barrier();
 			if (!cpu_possible(i)) {
 				printk("kexec: cpu %d hw_cpu_id %d is not"
@@ -201,20 +206,35 @@ static void kexec_prepare_cpus(void)
 			}
 			if (i != notified) {
 				printk( "kexec: waiting for cpu %d (physical"
-						" %d) to go down\n",
-						i, paca[i].hw_cpu_id);
+						" %d) to enter %i state\n",
+					i, paca[i].hw_cpu_id, wait_state);
 				notified = i;
 			}
 		}
 	}
+	mb();
+}
+
+static void kexec_prepare_cpus(void)
+{
+
+	smp_call_function(kexec_smp_down, NULL, /* wait */0);
+	local_irq_disable();
+	mb(); /* make sure IRQs are disabled before we say they are */
+	get_paca()->kexec_state = KEXEC_STATE_IRQS_OFF;
+
+	kexec_prepare_cpus_wait(KEXEC_STATE_IRQS_OFF);
+	/* we are sure every CPU has IRQs off at this point */
+	kexec_all_irq_disabled = 1;
 
 	/* after we tell the others to go down */
 	if (ppc_md.kexec_cpu_down)
 		ppc_md.kexec_cpu_down(0, 0);
 
-	put_cpu();
+	/* Before removing MMU mapings make sure all CPUs have entered real mode */
+	kexec_prepare_cpus_wait(KEXEC_STATE_REAL_MODE);
 
-	local_irq_disable();
+	put_cpu();
 }
 
 #else /* ! SMP */
Index: linux-2.6-ozlabs/arch/powerpc/kernel/misc_64.S
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/misc_64.S
+++ linux-2.6-ozlabs/arch/powerpc/kernel/misc_64.S
@@ -24,6 +24,7 @@
 #include <asm/asm-offsets.h>
 #include <asm/cputable.h>
 #include <asm/thread_info.h>
+#include <asm/kexec.h>
 
 	.text
 
@@ -471,6 +472,10 @@ _GLOBAL(kexec_wait)
 1:	mflr	r5
 	addi	r5,r5,kexec_flag-1b
 
+	li	r4,KEXEC_STATE_REAL_MODE
+	stb	r4,PACAKEXECSTATE(r13)
+	SYNC
+
 99:	HMT_LOW
 #ifdef CONFIG_KEXEC		/* use no memory without kexec */
 	lwz	r4,0(r5)
@@ -494,14 +499,11 @@ kexec_flag:
  * note: this is a terminal routine, it does not save lr
  *
  * get phys id from paca
- * set paca id to -1 to say we got here
  * switch to real mode
  * join other cpus in kexec_wait(phys_id)
  */
 _GLOBAL(kexec_smp_wait)
 	lhz	r3,PACAHWCPUID(r13)
-	li	r4,-1
-	sth	r4,PACAHWCPUID(r13)	/* let others know we left */
 	bl	real_mode
 	b	.kexec_wait
 
Index: linux-2.6-ozlabs/arch/powerpc/kernel/paca.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/paca.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/paca.c
@@ -18,6 +18,7 @@
 #include <asm/pgtable.h>
 #include <asm/iseries/lpar_map.h>
 #include <asm/iseries/hv_types.h>
+#include <asm/kexec.h>
 
 /* This symbol is provided by the linker - let it fill in the paca
  * field correctly */
@@ -97,6 +98,7 @@ void __init initialise_paca(struct paca_
 	new_paca->kernelbase = (unsigned long) _stext;
 	new_paca->kernel_msr = MSR_KERNEL;
 	new_paca->hw_cpu_id = 0xffff;
+	new_paca->kexec_state = KEXEC_STATE_NONE;
 	new_paca->__current = &init_task;
 #ifdef CONFIG_PPC_STD_MMU_64
 	new_paca->slb_shadow_ptr = &slb_shadow[cpu];

^ permalink raw reply

* [PATCH 2/2] powerpc, kdump: Fix race in kdump shutdown
From: Michael Neuling @ 2010-05-14  5:40 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, kexec, Anton Blanchard, jlarrew
In-Reply-To: <20100514035700.8B6EDCED6E@localhost.localdomain>

When we are crashing, the crashing/primary CPU IPIs the secondaries to
turn off IRQs, go into real mode and wait in kexec_wait.  While this
is happening, the primary tears down all the MMU maps.  Unfortunately
the primary doesn't check to make sure the secondaries have entered
real mode before doing this.

On PHYP machines, the secondaries can take a long time shutting down
the IRQ controller as RTAS calls are need.  These RTAS calls need to
be serialised which resilts in the secondaries contending in
lock_rtas() and hence taking a long time to shut down.

We've hit this on large POWER7 machines, where some secondaries are
still waiting in lock_rtas(), when the primary tears down the HPTEs.

This patch makes sure all secondaries are in real mode before the
primary tears down the MMU.  It uses the new kexec_state entry in the
paca.  It times out if the secondaries don't reach real mode after
10sec.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---

 arch/powerpc/kernel/crash.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Index: linux-2.6-ozlabs/arch/powerpc/kernel/crash.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/crash.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/crash.c
@@ -162,6 +162,32 @@ static void crash_kexec_prepare_cpus(int
 	/* Leave the IPI callback set */
 }
 
+/* wait for all the CPUs to hit real mode but timeout if they don't come in */
+static void crash_kexec_wait_realmode(int cpu)
+{
+	unsigned int msecs;
+	int i;
+
+	msecs = 10000;
+	for (i=0; i < NR_CPUS && msecs > 0; i++) {
+		if (i == cpu)
+			continue;
+
+		while (paca[i].kexec_state < KEXEC_STATE_REAL_MODE) {
+			barrier();
+			if (!cpu_possible(i)) {
+				break;
+			}
+			if (!cpu_online(i)) {
+				break;
+			}
+			msecs--;
+			mdelay(1);
+		}
+	}
+	mb();
+}
+
 /*
  * This function will be called by secondary cpus or by kexec cpu
  * if soft-reset is activated to stop some CPUs.
@@ -412,6 +438,7 @@ void default_machine_crash_shutdown(stru
 	crash_kexec_prepare_cpus(crashing_cpu);
 	cpu_set(crashing_cpu, cpus_in_crash);
 	crash_kexec_stop_spus();
+	crash_kexec_wait_realmode(crashing_cpu);
 	if (ppc_md.kexec_cpu_down)
 		ppc_md.kexec_cpu_down(1, 0);
 }

^ permalink raw reply


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