Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1456790 > unrolled thread

[PATCH v2 00/44] x86/dumpstack: rewrite x86 stack dump code

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/44] x86/dumpstack: rewrite x86 stack dump code Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 07/44] x86/dumpstack: remove extra brackets around "<EOE>" Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 11/44] x86/dumpstack: remove unnecessary stack pointer arguments Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 06/44] x86/dumpstack: add IRQ_USABLE_STACK_SIZE define Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 14/44] x86/asm/head:  put real return address on idle task stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 02/44] x86/asm/head: remove unused init_rsp variable Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 18/44] ftrace: remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 17/44] proc: fix return address printk conversion specifer in /proc/<pid>/stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 20/44] ftrace: add return address pointer to ftrace_ret_stack Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 15/44] perf/x86: check perf_callchain_store() error Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 04/44] x86/asm/head: use a common function for starting CPUs Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
      Re: [PATCH v2 04/44] x86/asm/head: use a common function for starting CPUs Nilay Vaish <nilayvaish@gmail.com> - 2016-08-05 17:50 +0200
        Re: [PATCH v2 04/44] x86/asm/head: use a common function for  starting CPUs Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 18:20 +0200
    [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack' Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
      Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack' Nilay Vaish <nilayvaish@gmail.com> - 2016-08-05 17:30 +0200
        Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' ->  'initial_stack' Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 18:10 +0200
          Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' ->  'initial_stack' Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-06 22:30 +0200
          Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' ->  'initial_stack' Borislav Petkov <bp@alien8.de> - 2016-08-06 22:50 +0200
            Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' ->  'initial_stack' Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-06 22:50 +0200
            Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack' Brian Gerst <brgerst@gmail.com> - 2016-08-07 00:30 +0200
    [PATCH v2 09/44] x86/dumpstack: fix x86_32 kernel_stack_pointer() previous stack access Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 22/44] x86/dumpstack/ftrace: convert dump_trace() callbacks to use ftrace_graph_ret_addr() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200
    [PATCH v2 12/44] x86: move _stext marker to before head code Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-05 00:40 +0200

Page 1 of 2  [1] 2  Next page →

#1456790 — [PATCH v2 00/44] x86/dumpstack: rewrite x86 stack dump code

There are a lot of changes since last time.  See the v2 changelog for
more details.

A git branch is available at:
  
  https://github.com/jpoimboe/linux unwind-v2

Based on tip/master.

v2 changelog:
- split up several of the patches and reorder them with lower-risk
  patches first
- add a lot more comments
- remove the 64-byte gap at the end of the irq stack
- fix some existing ftrace function graph unwinding issues
- fix an existing bug in kernel_stack_pointer()
- clarify the origins of the stack_info "next stack" pointers
- do visit_mask checking in get_stack_info() instead of in_*_stack()
- add some new unwinder warnings
- remove uses of test_and_set_bit()
- dont print regs->ip twice
- remove unwind_state.sp
- have unwind_get_return_address() validate the return address
- change /proc/pid/stack to use %pB
- several minor cleanups and fixes

----

The x86 stack dump code is a bit of a mess.  dump_trace() uses
callbacks, and each user of it seems to have slightly different
requirements, so there are several slightly different callbacks floating
around.

Also there are some upcoming features which will require more changes to
the stack dump code: reliable stack detection for live patching,
hardened user copy, and the DWARF unwinder.  Each of those features
would at least need more callbacks and/or callback interfaces, resulting
in a much bigger mess than what we have today.

Before doing all that, we should try to clean things up and replace
dump_trace() with something cleaner and more flexible.

The new unwinder is a simple state machine which was heavily inspired by
a suggestion from Andy Lutomirski:

  https://lkml.kernel.org/r/CALCETrUbNTqaM2LRyXGRx=kVLRPeY5A3Pc6k4TtQxF320rUT=w@mail.gmail.com

It's also similar to the libunwind API:

  http://www.nongnu.org/libunwind/man/libunwind(3).html

Some if its advantages:

- simplicity: no more callback sprawl and less code duplication.

- flexibility: allows the caller to stop and inspect the stack state at
  each step in the unwinding process.

- modularity: the unwinder code, console stack dump code, and stack
  metadata analysis code are all better separated so that changing one
  of them shouldn't have much of an impact on any of the others.


Josh Poimboeuf (44):
  x86/dumpstack: remove show_trace()
  x86/asm/head: remove unused init_rsp variable
  x86/asm/head: rename 'stack_start' -> 'initial_stack'
  x86/asm/head: use a common function for starting CPUs
  x86/dumpstack: make printk_stack_address() more generally useful
  x86/dumpstack: add IRQ_USABLE_STACK_SIZE define
  x86/dumpstack: remove extra brackets around "<EOE>"
  x86/dumpstack: fix irq stack bounds calculation in
    show_stack_log_lvl()
  x86/dumpstack: fix x86_32 kernel_stack_pointer() previous stack access
  x86/dumpstack: add get_stack_pointer() and get_frame_pointer()
  x86/dumpstack: remove unnecessary stack pointer arguments
  x86: move _stext marker to before head code
  x86/asm/head: remove useless zeroed word
  x86/asm/head:  put real return address on idle task stack
  perf/x86: check perf_callchain_store() error
  oprofile/x86: add regs->ip to oprofile trace
  proc: fix return address printk conversion specifer in
    /proc/<pid>/stack
  ftrace: remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config
  ftrace: only allocate the ret_stack 'fp' field when needed
  ftrace: add return address pointer to ftrace_ret_stack
  ftrace: add ftrace_graph_ret_addr() stack unwinding helpers
  x86/dumpstack/ftrace: convert dump_trace() callbacks to use
    ftrace_graph_ret_addr()
  ftrace/x86: implement HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
  x86/dumpstack/ftrace: mark function graph handler function as
    unreliable
  x86/dumpstack/ftrace: don't print unreliable addresses in
    print_context_stack_bp()
  x86/dumpstack: allow preemption in show_stack_log_lvl() and
    dump_trace()
  x86/dumpstack: simplify in_exception_stack()
  x86/dumpstack: add get_stack_info() interface
  x86/dumpstack: add recursion checking for all stacks
  x86/unwind: add new unwind interface and implementations
  perf/x86: convert perf_callchain_kernel() to use the new unwinder
  x86/stacktrace: convert save_stack_trace_*() to use the new unwinder
  oprofile/x86: convert x86_backtrace() to use the new unwinder
  x86/dumpstack: convert show_trace_log_lvl() to use the new unwinder
  x86/dumpstack: remove dump_trace() and related callbacks
  x86/entry/unwind: encode pt_regs pointer in frame pointer
  x86/unwind: detect syscall entry regs
  x86/dumpstack: print stack identifier on its own line
  x86/dumpstack: print any pt_regs found on the stack
  x86: remove 64-byte gap at end of irq stack
  x86/asm/head: standardize the end of the stack for idle tasks
  x86/unwind: warn on kernel stack corruption
  x86/unwind: warn on bad stack return address
  x86/unwind: warn if stack grows up

 Documentation/trace/ftrace-design.txt |  11 ++
 arch/arm/kernel/ftrace.c              |   2 +-
 arch/arm64/kernel/entry-ftrace.S      |   2 +-
 arch/arm64/kernel/ftrace.c            |   2 +-
 arch/blackfin/kernel/ftrace-entry.S   |   4 +-
 arch/blackfin/kernel/ftrace.c         |   2 +-
 arch/microblaze/kernel/ftrace.c       |   2 +-
 arch/mips/kernel/ftrace.c             |   4 +-
 arch/parisc/kernel/ftrace.c           |   2 +-
 arch/powerpc/kernel/ftrace.c          |   3 +-
 arch/s390/kernel/ftrace.c             |   3 +-
 arch/sh/kernel/ftrace.c               |   2 +-
 arch/sparc/Kconfig                    |   1 -
 arch/sparc/include/asm/ftrace.h       |   4 +
 arch/sparc/kernel/ftrace.c            |   2 +-
 arch/tile/kernel/ftrace.c             |   2 +-
 arch/x86/Kconfig                      |   1 -
 arch/x86/entry/calling.h              |  21 +++
 arch/x86/entry/entry_64.S             |  10 +-
 arch/x86/events/core.c                |  36 ++--
 arch/x86/include/asm/ftrace.h         |   3 +
 arch/x86/include/asm/kdebug.h         |   2 -
 arch/x86/include/asm/page_64_types.h  |  16 +-
 arch/x86/include/asm/realmode.h       |   2 +-
 arch/x86/include/asm/smp.h            |   3 -
 arch/x86/include/asm/stacktrace.h     | 114 ++++++------
 arch/x86/include/asm/unwind.h         | 104 +++++++++++
 arch/x86/kernel/Makefile              |   6 +
 arch/x86/kernel/acpi/sleep.c          |   2 +-
 arch/x86/kernel/cpu/common.c          |   2 +-
 arch/x86/kernel/dumpstack.c           | 272 ++++++++++++++---------------
 arch/x86/kernel/dumpstack_32.c        | 138 ++++++++-------
 arch/x86/kernel/dumpstack_64.c        | 319 ++++++++++------------------------
 arch/x86/kernel/ftrace.c              |   2 +-
 arch/x86/kernel/head_32.S             |   8 +-
 arch/x86/kernel/head_64.S             |  33 ++--
 arch/x86/kernel/ptrace.c              |   4 +-
 arch/x86/kernel/setup_percpu.c        |   2 +-
 arch/x86/kernel/smpboot.c             |   2 +-
 arch/x86/kernel/stacktrace.c          |  74 ++++----
 arch/x86/kernel/unwind_frame.c        | 222 +++++++++++++++++++++++
 arch/x86/kernel/unwind_guess.c        |  40 +++++
 arch/x86/kernel/vmlinux.lds.S         |   2 +-
 arch/x86/oprofile/backtrace.c         |  49 +++---
 fs/proc/base.c                        |   2 +-
 include/linux/ftrace.h                |  17 +-
 kernel/trace/Kconfig                  |   5 -
 kernel/trace/trace_functions_graph.c  |  67 ++++++-
 48 files changed, 977 insertions(+), 651 deletions(-)
 create mode 100644 arch/x86/include/asm/unwind.h
 create mode 100644 arch/x86/kernel/unwind_frame.c
 create mode 100644 arch/x86/kernel/unwind_guess.c

-- 
2.7.4

[toc] | [next] | [standalone]

#1456792 — [PATCH v2 07/44] x86/dumpstack: remove extra brackets around "<EOE>"

When starting the dump of an exception stack, it shows "<<EOE>>" instead
of "<EOE>".  print_trace_stack() already adds brackets, no need to add
them again.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/dumpstack_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 43023ae..7ea6ed0 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -199,7 +199,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
 
 			bp = ops->walk_stack(task, stack, bp, ops,
 					     data, stack_end, &graph);
-			ops->stack(data, "<EOE>");
+			ops->stack(data, "EOE");
 			/*
 			 * We link to the next stack via the
 			 * second-to-last pointer (index -2 to end) in the
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456795 — [PATCH v2 11/44] x86/dumpstack: remove unnecessary stack pointer arguments

When calling show_stack_log_lvl() or dump_trace() with a regs argument,
providing a stack pointer or frame pointer is redundant.

Reviewed-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>d
---
 arch/x86/kernel/dumpstack.c    | 2 +-
 arch/x86/kernel/dumpstack_32.c | 2 +-
 arch/x86/kernel/dumpstack_64.c | 5 +----
 arch/x86/oprofile/backtrace.c  | 4 +---
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 68f42bb..692eecae 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -200,7 +200,7 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 
 void show_stack_regs(struct pt_regs *regs)
 {
-	show_stack_log_lvl(current, regs, (unsigned long *)regs->sp, regs->bp, "");
+	show_stack_log_lvl(current, regs, NULL, 0, "");
 }
 
 static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED;
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 358fe1c..c533b8b 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -122,7 +122,7 @@ void show_regs(struct pt_regs *regs)
 		u8 *ip;
 
 		pr_emerg("Stack:\n");
-		show_stack_log_lvl(NULL, regs, &regs->sp, 0, KERN_EMERG);
+		show_stack_log_lvl(NULL, regs, NULL, 0, KERN_EMERG);
 
 		pr_emerg("Code:");
 
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 3c5dbc0..491f2fd 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -283,9 +283,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
 void show_regs(struct pt_regs *regs)
 {
 	int i;
-	unsigned long sp;
 
-	sp = regs->sp;
 	show_regs_print_info(KERN_DEFAULT);
 	__show_regs(regs, 1);
 
@@ -300,8 +298,7 @@ void show_regs(struct pt_regs *regs)
 		u8 *ip;
 
 		printk(KERN_DEFAULT "Stack:\n");
-		show_stack_log_lvl(NULL, regs, (unsigned long *)sp,
-				   0, KERN_DEFAULT);
+		show_stack_log_lvl(NULL, regs, NULL, 0, KERN_DEFAULT);
 
 		printk(KERN_DEFAULT "Code: ");
 
diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c
index cb31a44..c594768 100644
--- a/arch/x86/oprofile/backtrace.c
+++ b/arch/x86/oprofile/backtrace.c
@@ -113,10 +113,8 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth)
 	struct stack_frame *head = (struct stack_frame *)frame_pointer(regs);
 
 	if (!user_mode(regs)) {
-		unsigned long stack = kernel_stack_pointer(regs);
 		if (depth)
-			dump_trace(NULL, regs, (unsigned long *)stack, 0,
-				   &backtrace_ops, &depth);
+			dump_trace(NULL, regs, NULL, 0, &backtrace_ops, &depth);
 		return;
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456797 — [PATCH v2 06/44] x86/dumpstack: add IRQ_USABLE_STACK_SIZE define

For reasons unknown, the x86_64 irq stack starts at an offset 64 bytes
from the end of the page.  At least make that explicit.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/page_64_types.h | 19 +++++++++++--------
 arch/x86/kernel/cpu/common.c         |  2 +-
 arch/x86/kernel/dumpstack_64.c       |  5 +----
 arch/x86/kernel/setup_percpu.c       |  2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/page_64_types.h b/arch/x86/include/asm/page_64_types.h
index 9215e05..6256baf 100644
--- a/arch/x86/include/asm/page_64_types.h
+++ b/arch/x86/include/asm/page_64_types.h
@@ -12,17 +12,20 @@
 #endif
 
 #define THREAD_SIZE_ORDER	(2 + KASAN_STACK_ORDER)
-#define THREAD_SIZE  (PAGE_SIZE << THREAD_SIZE_ORDER)
-#define CURRENT_MASK (~(THREAD_SIZE - 1))
+#define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)
+#define CURRENT_MASK		(~(THREAD_SIZE - 1))
 
-#define EXCEPTION_STACK_ORDER (0 + KASAN_STACK_ORDER)
-#define EXCEPTION_STKSZ (PAGE_SIZE << EXCEPTION_STACK_ORDER)
+#define EXCEPTION_STACK_ORDER	(0 + KASAN_STACK_ORDER)
+#define EXCEPTION_STKSZ		(PAGE_SIZE << EXCEPTION_STACK_ORDER)
 
-#define DEBUG_STACK_ORDER (EXCEPTION_STACK_ORDER + 1)
-#define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)
+#define DEBUG_STACK_ORDER	(EXCEPTION_STACK_ORDER + 1)
+#define DEBUG_STKSZ		(PAGE_SIZE << DEBUG_STACK_ORDER)
 
-#define IRQ_STACK_ORDER (2 + KASAN_STACK_ORDER)
-#define IRQ_STACK_SIZE (PAGE_SIZE << IRQ_STACK_ORDER)
+#define IRQ_STACK_ORDER		(2 + KASAN_STACK_ORDER)
+#define IRQ_STACK_SIZE		(PAGE_SIZE << IRQ_STACK_ORDER)
+
+/* FIXME: why? */
+#define IRQ_USABLE_STACK_SIZE	(IRQ_STACK_SIZE - 64)
 
 #define DOUBLEFAULT_STACK 1
 #define NMI_STACK 2
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 809eda0..8f3f7a4 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1281,7 +1281,7 @@ DEFINE_PER_CPU(struct task_struct *, current_task) ____cacheline_aligned =
 EXPORT_PER_CPU_SYMBOL(current_task);
 
 DEFINE_PER_CPU(char *, irq_stack_ptr) =
-	init_per_cpu_var(irq_stack_union.irq_stack) + IRQ_STACK_SIZE - 64;
+	init_per_cpu_var(irq_stack_union.irq_stack) + IRQ_USABLE_STACK_SIZE;
 
 DEFINE_PER_CPU(unsigned int, irq_count) __visible = -1;
 
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 9ee4520..43023ae 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -103,9 +103,6 @@ in_irq_stack(unsigned long *stack, unsigned long *irq_stack,
 	return (stack >= irq_stack && stack < irq_stack_end);
 }
 
-static const unsigned long irq_stack_size =
-	(IRQ_STACK_SIZE - 64) / sizeof(unsigned long);
-
 enum stack_type {
 	STACK_IS_UNKNOWN,
 	STACK_IS_NORMAL,
@@ -133,7 +130,7 @@ analyze_stack(int cpu, struct task_struct *task, unsigned long *stack,
 		return STACK_IS_NORMAL;
 
 	*stack_end = irq_stack;
-	irq_stack = irq_stack - irq_stack_size;
+	irq_stack -= (IRQ_USABLE_STACK_SIZE / sizeof(long));
 
 	if (in_irq_stack(stack, irq_stack, *stack_end))
 		return STACK_IS_IRQ;
diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
index 7a40e06..be6b571 100644
--- a/arch/x86/kernel/setup_percpu.c
+++ b/arch/x86/kernel/setup_percpu.c
@@ -246,7 +246,7 @@ void __init setup_per_cpu_areas(void)
 #ifdef CONFIG_X86_64
 		per_cpu(irq_stack_ptr, cpu) =
 			per_cpu(irq_stack_union.irq_stack, cpu) +
-			IRQ_STACK_SIZE - 64;
+			IRQ_USABLE_STACK_SIZE;
 #endif
 #ifdef CONFIG_NUMA
 		per_cpu(x86_cpu_to_node_map, cpu) =
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456798 — [PATCH v2 14/44] x86/asm/head: put real return address on idle task stack

The frame at the end of each idle task stack has a zero instead of a
real return address.  This is inconsistent with real task stacks, which
have a real return address at that spot.  This inconsistency can be
confusing for stack unwinders.

Make it a real address by using the side effect of a call instruction to
push the instruction pointer on the stack.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/head_64.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index ac6e27e..c910c27 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -296,8 +296,9 @@ ENTRY(start_cpu)
 	 *	REX.W + FF /5 JMP m16:64 Jump far, absolute indirect,
 	 *		address given in m16:64.
 	 */
-	movq	initial_code(%rip),%rax
-	pushq	$0		# fake return address to stop unwinder
+	call	1f		# put return address on stack for unwinder
+1:	xorq	%rbp, %rbp	# clear frame pointer
+	movq	initial_code(%rip), %rax
 	pushq	$__KERNEL_CS	# set correct cs
 	pushq	%rax		# target address in negative space
 	lretq
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456799 — [PATCH v2 02/44] x86/asm/head: remove unused init_rsp variable

The init_rsp variable is unused.  Remove it.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/realmode.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/include/asm/realmode.h b/arch/x86/include/asm/realmode.h
index 9c6b890..b2b407b 100644
--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -44,7 +44,6 @@ struct trampoline_header {
 extern struct real_mode_header *real_mode_header;
 extern unsigned char real_mode_blob_end[];
 
-extern unsigned long init_rsp;
 extern unsigned long initial_code;
 extern unsigned long initial_gs;
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456800 — [PATCH v2 18/44] ftrace: remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config

Make HAVE_FUNCTION_GRAPH_FP_TEST a normal define, independent from
kconfig.  This removes some config file pollution and simplifies the
checking for the fp test.

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/arm64/kernel/entry-ftrace.S     | 2 +-
 arch/blackfin/kernel/ftrace-entry.S  | 4 ++--
 arch/sparc/Kconfig                   | 1 -
 arch/sparc/include/asm/ftrace.h      | 4 ++++
 arch/x86/Kconfig                     | 1 -
 arch/x86/include/asm/ftrace.h        | 1 +
 kernel/trace/Kconfig                 | 5 -----
 kernel/trace/trace_functions_graph.c | 2 +-
 8 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index 0f03a8f..aef02d2 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -219,7 +219,7 @@ ENDPROC(ftrace_graph_caller)
  *
  * Run ftrace_return_to_handler() before going back to parent.
  * @fp is checked against the value passed by ftrace_graph_caller()
- * only when CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST is enabled.
+ * only when HAVE_FUNCTION_GRAPH_FP_TEST is enabled.
  */
 ENTRY(return_to_handler)
 	save_return_regs
diff --git a/arch/blackfin/kernel/ftrace-entry.S b/arch/blackfin/kernel/ftrace-entry.S
index 28d0595..3b8bdcb 100644
--- a/arch/blackfin/kernel/ftrace-entry.S
+++ b/arch/blackfin/kernel/ftrace-entry.S
@@ -169,7 +169,7 @@ ENTRY(_ftrace_graph_caller)
 	r0 = sp;	/* unsigned long *parent */
 	r1 = [sp];	/* unsigned long self_addr */
 # endif
-# ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
+# ifdef HAVE_FUNCTION_GRAPH_FP_TEST
 	r2 = fp;	/* unsigned long frame_pointer */
 # endif
 	r0 += 16;	/* skip the 4 local regs on stack */
@@ -190,7 +190,7 @@ ENTRY(_return_to_handler)
 	[--sp] = r1;
 
 	/* get original return address */
-# ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST
+# ifdef HAVE_FUNCTION_GRAPH_FP_TEST
 	r0 = fp;	/* Blackfin is sane, so omit this */
 # endif
 	call _ftrace_return_to_handler;
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 546293d..b0a6ab2 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -55,7 +55,6 @@ config SPARC64
 	def_bool 64BIT
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_GRAPH_TRACER
-	select HAVE_FUNCTION_GRAPH_FP_TEST
 	select HAVE_KRETPROBES
 	select HAVE_KPROBES
 	select HAVE_RCU_TABLE_FREE if SMP
diff --git a/arch/sparc/include/asm/ftrace.h b/arch/sparc/include/asm/ftrace.h
index 3192a8e..62755a3 100644
--- a/arch/sparc/include/asm/ftrace.h
+++ b/arch/sparc/include/asm/ftrace.h
@@ -9,6 +9,10 @@
 void _mcount(void);
 #endif
 
+#endif /* CONFIG_MCOUNT */
+
+#if defined(CONFIG_SPARC64) && !defined(CC_USE_FENTRY)
+#define HAVE_FUNCTION_GRAPH_FP_TEST
 #endif
 
 #ifdef CONFIG_DYNAMIC_FTRACE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2fa5585..69267bd 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -108,7 +108,6 @@ config X86
 	select HAVE_EXIT_THREAD
 	select HAVE_FENTRY			if X86_64
 	select HAVE_FTRACE_MCOUNT_RECORD
-	select HAVE_FUNCTION_GRAPH_FP_TEST
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
 	select HAVE_GENERIC_DMA_COHERENT	if X86_32
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index a4820d4..37f67cb 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -6,6 +6,7 @@
 # define MCOUNT_ADDR		((unsigned long)(__fentry__))
 #else
 # define MCOUNT_ADDR		((unsigned long)(mcount))
+# define HAVE_FUNCTION_GRAPH_FP_TEST
 #endif
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index f4b86e8..ba33267 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -24,11 +24,6 @@ config HAVE_FUNCTION_GRAPH_TRACER
 	help
 	  See Documentation/trace/ftrace-design.txt
 
-config HAVE_FUNCTION_GRAPH_FP_TEST
-	bool
-	help
-	  See Documentation/trace/ftrace-design.txt
-
 config HAVE_DYNAMIC_FTRACE
 	bool
 	help
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 7363ccf..fc173cd 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -204,7 +204,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
 		return;
 	}
 
-#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY)
+#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
 	/*
 	 * The arch may choose to record the frame pointer used
 	 * and check it here to make sure that it is what we expect it
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456801 — [PATCH v2 17/44] proc: fix return address printk conversion specifer in /proc/<pid>/stack

When printing call return addresses found on a stack, /proc/<pid>/stack
can sometimes give a confusing result.  If the call instruction was the
last instruction in the function (which can happen when calling a
noreturn function), '%pS' will incorrectly display the name of the
function which happens to be next in the object code, rather than the
name of the actual calling function.

Use '%pB' instead, which was created for this exact purpose.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 fs/proc/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 31370da..cc4d81c 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -483,7 +483,7 @@ static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns,
 		save_stack_trace_tsk(task, &trace);
 
 		for (i = 0; i < trace.nr_entries; i++) {
-			seq_printf(m, "[<%pK>] %pS\n",
+			seq_printf(m, "[<%pK>] %pB\n",
 				   (void *)entries[i], (void *)entries[i]);
 		}
 		unlock_trace(task);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456802 — [PATCH v2 20/44] ftrace: add return address pointer to ftrace_ret_stack

Storing this value will help prevent unwinders from getting out of sync
with the function graph tracer ret_stack.

Note that an array of 50 ftrace_ret_stack structs is allocated for every
task.  So when an arch implements this, it will add either 200 or 400
bytes of memory usage per task (depending on whether it's a 32-bit or
64-bit platform).

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 Documentation/trace/ftrace-design.txt | 11 +++++++++++
 arch/arm/kernel/ftrace.c              |  2 +-
 arch/arm64/kernel/ftrace.c            |  2 +-
 arch/blackfin/kernel/ftrace.c         |  2 +-
 arch/microblaze/kernel/ftrace.c       |  2 +-
 arch/mips/kernel/ftrace.c             |  4 ++--
 arch/parisc/kernel/ftrace.c           |  2 +-
 arch/powerpc/kernel/ftrace.c          |  3 ++-
 arch/s390/kernel/ftrace.c             |  3 ++-
 arch/sh/kernel/ftrace.c               |  2 +-
 arch/sparc/kernel/ftrace.c            |  2 +-
 arch/tile/kernel/ftrace.c             |  2 +-
 arch/x86/kernel/ftrace.c              |  2 +-
 include/linux/ftrace.h                |  5 ++++-
 kernel/trace/trace_functions_graph.c  |  5 ++++-
 15 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index dd5f916..a273dd0 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -203,6 +203,17 @@ along to ftrace_push_return_trace() instead of a stub value of 0.
 
 Similarly, when you call ftrace_return_to_handler(), pass it the frame pointer.
 
+HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
+--------------------------------
+
+An arch may pass in a pointer to the return address on the stack.  This
+prevents potential stack unwinding issues where the unwinder gets out of
+sync with ret_stack and the wrong addresses are reported by
+ftrace_graph_ret_addr().
+
+Adding support for it is easy: just define the macro in asm/ftrace.h and
+pass the return address pointer as the 'retp' argument to
+ftrace_push_return_trace().
 
 HAVE_FTRACE_NMI_ENTER
 ---------------------
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 709ee1d..3f17594 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -218,7 +218,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 	}
 
 	err = ftrace_push_return_trace(old, self_addr, &trace.depth,
-				       frame_pointer);
+				       frame_pointer, NULL);
 	if (err == -EBUSY) {
 		*parent = old;
 		return;
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index ebecf9a..40ad08a 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -138,7 +138,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 		return;
 
 	err = ftrace_push_return_trace(old, self_addr, &trace.depth,
-				       frame_pointer);
+				       frame_pointer, NULL);
 	if (err == -EBUSY)
 		return;
 	else
diff --git a/arch/blackfin/kernel/ftrace.c b/arch/blackfin/kernel/ftrace.c
index 095de0f..8dad758 100644
--- a/arch/blackfin/kernel/ftrace.c
+++ b/arch/blackfin/kernel/ftrace.c
@@ -107,7 +107,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 		return;
 
 	if (ftrace_push_return_trace(*parent, self_addr, &trace.depth,
-	                             frame_pointer) == -EBUSY)
+				     frame_pointer, NULL) == -EBUSY)
 		return;
 
 	trace.func = self_addr;
diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c
index fc7b48a..d57563c 100644
--- a/arch/microblaze/kernel/ftrace.c
+++ b/arch/microblaze/kernel/ftrace.c
@@ -63,7 +63,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 		return;
 	}
 
-	err = ftrace_push_return_trace(old, self_addr, &trace.depth, 0);
+	err = ftrace_push_return_trace(old, self_addr, &trace.depth, 0, NULL);
 	if (err == -EBUSY) {
 		*parent = old;
 		return;
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 937c54b..30a3b75 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -382,8 +382,8 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,
 	if (unlikely(faulted))
 		goto out;
 
-	if (ftrace_push_return_trace(old_parent_ra, self_ra, &trace.depth, fp)
-	    == -EBUSY) {
+	if (ftrace_push_return_trace(old_parent_ra, self_ra, &trace.depth, fp,
+				     NULL) == -EBUSY) {
 		*parent_ra_addr = old_parent_ra;
 		return;
 	}
diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
index a828a0a..5a5506a 100644
--- a/arch/parisc/kernel/ftrace.c
+++ b/arch/parisc/kernel/ftrace.c
@@ -48,7 +48,7 @@ static void __hot prepare_ftrace_return(unsigned long *parent,
 		return;
 
         if (ftrace_push_return_trace(old, self_addr, &trace.depth,
-			0 ) == -EBUSY)
+				     0, NULL) == -EBUSY)
                 return;
 
 	/* activate parisc_return_to_handler() as return point */
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 1123a4d..d49b757 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -592,7 +592,8 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip)
 	if (!ftrace_graph_entry(&trace))
 		goto out;
 
-	if (ftrace_push_return_trace(parent, ip, &trace.depth, 0) == -EBUSY)
+	if (ftrace_push_return_trace(parent, ip, &trace.depth, 0,
+				     NULL) == -EBUSY)
 		goto out;
 
 	parent = return_hooker;
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 0f7bfeb..60a8a4e 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -209,7 +209,8 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip)
 	/* Only trace if the calling function expects to. */
 	if (!ftrace_graph_entry(&trace))
 		goto out;
-	if (ftrace_push_return_trace(parent, ip, &trace.depth, 0) == -EBUSY)
+	if (ftrace_push_return_trace(parent, ip, &trace.depth, 0,
+				     NULL) == -EBUSY)
 		goto out;
 	parent = (unsigned long) return_to_handler;
 out:
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 38993e0..95eccd4 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -382,7 +382,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 		return;
 	}
 
-	err = ftrace_push_return_trace(old, self_addr, &trace.depth, 0);
+	err = ftrace_push_return_trace(old, self_addr, &trace.depth, 0, NULL);
 	if (err == -EBUSY) {
 		__raw_writel(old, parent);
 		return;
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index 0a2d2dd..6bcff69 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -131,7 +131,7 @@ unsigned long prepare_ftrace_return(unsigned long parent,
 		return parent + 8UL;
 
 	if (ftrace_push_return_trace(parent, self_addr, &trace.depth,
-				     frame_pointer) == -EBUSY)
+				     frame_pointer, NULL) == -EBUSY)
 		return parent + 8UL;
 
 	trace.func = self_addr;
diff --git a/arch/tile/kernel/ftrace.c b/arch/tile/kernel/ftrace.c
index 4a57208..b827a41 100644
--- a/arch/tile/kernel/ftrace.c
+++ b/arch/tile/kernel/ftrace.c
@@ -184,7 +184,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 	*parent = return_hooker;
 
 	err = ftrace_push_return_trace(old, self_addr, &trace.depth,
-				       frame_pointer);
+				       frame_pointer, NULL);
 	if (err == -EBUSY) {
 		*parent = old;
 		return;
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index d036cfb..ae3b1fb 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -1029,7 +1029,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
 	}
 
 	if (ftrace_push_return_trace(old, self_addr, &trace.depth,
-		    frame_pointer) == -EBUSY) {
+				     frame_pointer, NULL) == -EBUSY) {
 		*parent = old;
 		return;
 	}
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 4ad9ccc..483e02a 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -798,6 +798,9 @@ struct ftrace_ret_stack {
 #ifdef HAVE_FUNCTION_GRAPH_FP_TEST
 	unsigned long fp;
 #endif
+#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
+	unsigned long *retp;
+#endif
 };
 
 /*
@@ -809,7 +812,7 @@ extern void return_to_handler(void);
 
 extern int
 ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth,
-			 unsigned long frame_pointer);
+			 unsigned long frame_pointer, unsigned long *retp);
 
 /*
  * Sometimes we don't want to trace a function with the function
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 0e03ed0..f7212ec 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -119,7 +119,7 @@ print_graph_duration(struct trace_array *tr, unsigned long long duration,
 /* Add a function return address to the trace stack on thread info.*/
 int
 ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth,
-			 unsigned long frame_pointer)
+			 unsigned long frame_pointer, unsigned long *retp)
 {
 	unsigned long long calltime;
 	int index;
@@ -174,6 +174,9 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth,
 #ifdef HAVE_FUNCTION_GRAPH_FP_TEST
 	current->ret_stack[index].fp = frame_pointer;
 #endif
+#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
+	current->ret_stack[index].retp = retp;
+#endif
 	*depth = current->curr_ret_stack;
 
 	return 0;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456803 — [PATCH v2 15/44] perf/x86: check perf_callchain_store() error

Add a check to perf_callchain_kernel() so that it returns early if the
callchain entry array is already full.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/events/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index d0efb5c..c1319ac 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2277,7 +2277,8 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
 		return;
 	}
 
-	perf_callchain_store(entry, regs->ip);
+	if (perf_callchain_store(entry, regs->ip))
+		return;
 
 	dump_trace(NULL, regs, NULL, 0, &backtrace_ops, entry);
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1456804 — [PATCH v2 04/44] x86/asm/head: use a common function for starting CPUs

There are two different pieces of code for starting a CPU: start_cpu0()
and the end of secondary_startup_64().  They're identical except for the
stack setup.  Combine the common parts into a shared start_cpu()
function.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/head_64.S | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index aa10a53..8822c20 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -264,13 +264,15 @@ ENTRY(secondary_startup_64)
 	movl	$MSR_GS_BASE,%ecx
 	movl	initial_gs(%rip),%eax
 	movl	initial_gs+4(%rip),%edx
-	wrmsr	
+	wrmsr
 
 	/* rsi is pointer to real mode structure with interesting info.
 	   pass it to C */
 	movq	%rsi, %rdi
-	
-	/* Finally jump to run C code and to be on real kernel address
+
+ENTRY(start_cpu)
+	/*
+	 * Jump to run C code and to be on a real kernel address.
 	 * Since we are running on identity-mapped space we have to jump
 	 * to the full 64bit address, this is only possible as indirect
 	 * jump.  In addition we need to ensure %cs is set so we make this
@@ -307,15 +309,11 @@ ENDPROC(secondary_startup_64)
 /*
  * Boot CPU0 entry point. It's called from play_dead(). Everything has been set
  * up already except stack. We just set up stack here. Then call
- * start_secondary().
+ * start_secondary() via start_cpu().
  */
 ENTRY(start_cpu0)
-	movq initial_stack(%rip),%rsp
-	movq	initial_code(%rip),%rax
-	pushq	$0		# fake return address to stop unwinder
-	pushq	$__KERNEL_CS	# set correct cs
-	pushq	%rax		# target address in negative space
-	lretq
+	movq	initial_stack(%rip), %rsp
+	jmp	start_cpu
 ENDPROC(start_cpu0)
 #endif
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1457184 — Re: [PATCH v2 04/44] x86/asm/head: use a common function for starting CPUs

On 4 August 2016 at 17:22, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> There are two different pieces of code for starting a CPU: start_cpu0()
> and the end of secondary_startup_64().  They're identical except for the
> stack setup.  Combine the common parts into a shared start_cpu()
> function.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  arch/x86/kernel/head_64.S | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index aa10a53..8822c20 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -264,13 +264,15 @@ ENTRY(secondary_startup_64)
>         movl    $MSR_GS_BASE,%ecx
>         movl    initial_gs(%rip),%eax
>         movl    initial_gs+4(%rip),%edx
> -       wrmsr
> +       wrmsr
>
>         /* rsi is pointer to real mode structure with interesting info.
>            pass it to C */
>         movq    %rsi, %rdi
> -
> -       /* Finally jump to run C code and to be on real kernel address
> +
> +ENTRY(start_cpu)
> +       /*
> +        * Jump to run C code and to be on a real kernel address.
>          * Since we are running on identity-mapped space we have to jump
>          * to the full 64bit address, this is only possible as indirect
>          * jump.  In addition we need to ensure %cs is set so we make this
> @@ -307,15 +309,11 @@ ENDPROC(secondary_startup_64)
>  /*
>   * Boot CPU0 entry point. It's called from play_dead(). Everything has been set
>   * up already except stack. We just set up stack here. Then call
> - * start_secondary().
> + * start_secondary() via start_cpu().
>   */
>  ENTRY(start_cpu0)
> -       movq initial_stack(%rip),%rsp
> -       movq    initial_code(%rip),%rax
> -       pushq   $0              # fake return address to stop unwinder
> -       pushq   $__KERNEL_CS    # set correct cs
> -       pushq   %rax            # target address in negative space
> -       lretq
> +       movq    initial_stack(%rip), %rsp
> +       jmp     start_cpu
>  ENDPROC(start_cpu0)
>  #endif
>

I have small suggestion here.  To me jumping from start_cpu0 into the
middle of secondary_startup_64 just seems strange.  May be we can
define separate ENTRY and ENDPROC pair for start_cpu and jump there
from start_cpu0 and also from secondary_startup_64.

--
Nilay

[toc] | [prev] | [next] | [standalone]

#1457201 — Re: [PATCH v2 04/44] x86/asm/head: use a common function for starting CPUs

On Fri, Aug 05, 2016 at 10:41:15AM -0500, Nilay Vaish wrote:
> On 4 August 2016 at 17:22, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > There are two different pieces of code for starting a CPU: start_cpu0()
> > and the end of secondary_startup_64().  They're identical except for the
> > stack setup.  Combine the common parts into a shared start_cpu()
> > function.
> >
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > ---
> >  arch/x86/kernel/head_64.S | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> > index aa10a53..8822c20 100644
> > --- a/arch/x86/kernel/head_64.S
> > +++ b/arch/x86/kernel/head_64.S
> > @@ -264,13 +264,15 @@ ENTRY(secondary_startup_64)
> >         movl    $MSR_GS_BASE,%ecx
> >         movl    initial_gs(%rip),%eax
> >         movl    initial_gs+4(%rip),%edx
> > -       wrmsr
> > +       wrmsr
> >
> >         /* rsi is pointer to real mode structure with interesting info.
> >            pass it to C */
> >         movq    %rsi, %rdi
> > -
> > -       /* Finally jump to run C code and to be on real kernel address
> > +
> > +ENTRY(start_cpu)
> > +       /*
> > +        * Jump to run C code and to be on a real kernel address.
> >          * Since we are running on identity-mapped space we have to jump
> >          * to the full 64bit address, this is only possible as indirect
> >          * jump.  In addition we need to ensure %cs is set so we make this
> > @@ -307,15 +309,11 @@ ENDPROC(secondary_startup_64)
> >  /*
> >   * Boot CPU0 entry point. It's called from play_dead(). Everything has been set
> >   * up already except stack. We just set up stack here. Then call
> > - * start_secondary().
> > + * start_secondary() via start_cpu().
> >   */
> >  ENTRY(start_cpu0)
> > -       movq initial_stack(%rip),%rsp
> > -       movq    initial_code(%rip),%rax
> > -       pushq   $0              # fake return address to stop unwinder
> > -       pushq   $__KERNEL_CS    # set correct cs
> > -       pushq   %rax            # target address in negative space
> > -       lretq
> > +       movq    initial_stack(%rip), %rsp
> > +       jmp     start_cpu
> >  ENDPROC(start_cpu0)
> >  #endif
> >
> 
> I have small suggestion here.  To me jumping from start_cpu0 into the
> middle of secondary_startup_64 just seems strange.  May be we can
> define separate ENTRY and ENDPROC pair for start_cpu and jump there
> from start_cpu0 and also from secondary_startup_64.

Yeah, that might be better.  But then again, it would also be strange to
add a jump at the end of secondary_startup_64, when it could instead
just fall through.

Maybe I should do as you suggest, but instead of the jump, add a comment
that it falls through to start_cpu()?

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1456805 — [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack'

The 'stack_start' variable is similar in usage to 'initial_code' and
'initial_gs': they're all stored in head_64.S and they're all updated by
SMP and ACPI suspend before starting a CPU.

Rename it to 'initial_stack' to be consistent with the others.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/realmode.h | 1 +
 arch/x86/include/asm/smp.h      | 3 ---
 arch/x86/kernel/acpi/sleep.c    | 2 +-
 arch/x86/kernel/head_32.S       | 8 ++++----
 arch/x86/kernel/head_64.S       | 9 ++++-----
 arch/x86/kernel/smpboot.c       | 2 +-
 6 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/realmode.h b/arch/x86/include/asm/realmode.h
index b2b407b..677a671 100644
--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -46,6 +46,7 @@ extern unsigned char real_mode_blob_end[];
 
 extern unsigned long initial_code;
 extern unsigned long initial_gs;
+extern unsigned long initial_stack;
 
 extern unsigned char real_mode_blob[];
 extern unsigned char real_mode_relocs[];
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index ebd0c16..19980b3 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -39,9 +39,6 @@ DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid);
 DECLARE_EARLY_PER_CPU_READ_MOSTLY(int, x86_cpu_to_logical_apicid);
 #endif
 
-/* Static state in head.S used to set up a CPU */
-extern unsigned long stack_start; /* Initial stack pointer address */
-
 struct task_struct;
 
 struct smp_ops {
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index adb3eaf..4858733 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -99,7 +99,7 @@ int x86_acpi_suspend_lowlevel(void)
 	saved_magic = 0x12345678;
 #else /* CONFIG_64BIT */
 #ifdef CONFIG_SMP
-	stack_start = (unsigned long)temp_stack + sizeof(temp_stack);
+	initial_stack = (unsigned long)temp_stack + sizeof(temp_stack);
 	early_gdt_descr.address =
 			(unsigned long)get_cpu_gdt_table(smp_processor_id());
 	initial_gs = per_cpu_offset(smp_processor_id());
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 6f8902b..5f40126 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -94,7 +94,7 @@ RESERVE_BRK(pagetables, INIT_MAP_SIZE)
  */
 __HEAD
 ENTRY(startup_32)
-	movl pa(stack_start),%ecx
+	movl pa(initial_stack),%ecx
 	
 	/* test KEEP_SEGMENTS flag to see if the bootloader is asking
 		us to not reload segments */
@@ -286,7 +286,7 @@ num_subarch_entries = (. - subarch_entries) / 4
  * start_secondary().
  */
 ENTRY(start_cpu0)
-	movl stack_start, %ecx
+	movl initial_stack, %ecx
 	movl %ecx, %esp
 	jmp  *(initial_code)
 ENDPROC(start_cpu0)
@@ -307,7 +307,7 @@ ENTRY(startup_32_smp)
 	movl %eax,%es
 	movl %eax,%fs
 	movl %eax,%gs
-	movl pa(stack_start),%ecx
+	movl pa(initial_stack),%ecx
 	movl %eax,%ss
 	leal -__PAGE_OFFSET(%ecx),%esp
 
@@ -703,7 +703,7 @@ ENTRY(initial_page_table)
 
 .data
 .balign 4
-ENTRY(stack_start)
+ENTRY(initial_stack)
 	.long init_thread_union+THREAD_SIZE
 
 __INITRODATA
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 9f8efc9..aa10a53 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -226,7 +226,7 @@ ENTRY(secondary_startup_64)
 	movq	%rax, %cr0
 
 	/* Setup a boot time stack */
-	movq stack_start(%rip), %rsp
+	movq initial_stack(%rip), %rsp
 
 	/* zero EFLAGS after setting rsp */
 	pushq $0
@@ -310,7 +310,7 @@ ENDPROC(secondary_startup_64)
  * start_secondary().
  */
 ENTRY(start_cpu0)
-	movq stack_start(%rip),%rsp
+	movq initial_stack(%rip),%rsp
 	movq	initial_code(%rip),%rax
 	pushq	$0		# fake return address to stop unwinder
 	pushq	$__KERNEL_CS	# set correct cs
@@ -319,15 +319,14 @@ ENTRY(start_cpu0)
 ENDPROC(start_cpu0)
 #endif
 
-	/* SMP bootup changes these two */
+	/* Both SMP bootup and ACPI suspend change these variables */
 	__REFDATA
 	.balign	8
 	GLOBAL(initial_code)
 	.quad	x86_64_start_kernel
 	GLOBAL(initial_gs)
 	.quad	INIT_PER_CPU_VAR(irq_stack_union)
-
-	GLOBAL(stack_start)
+	GLOBAL(initial_stack)
 	.quad  init_thread_union+THREAD_SIZE-8
 	.word  0
 	__FINITDATA
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2a6e84a..2fbd8d7 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -962,7 +962,7 @@ static int do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
 
 	early_gdt_descr.address = (unsigned long)get_cpu_gdt_table(cpu);
 	initial_code = (unsigned long)start_secondary;
-	stack_start  = idle->thread.sp;
+	initial_stack  = idle->thread.sp;
 
 	/*
 	 * Enable the espfix hack for this CPU
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1457163 — Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack'

On 4 August 2016 at 17:21, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> The 'stack_start' variable is similar in usage to 'initial_code' and
> 'initial_gs': they're all stored in head_64.S and they're all updated by
> SMP and ACPI suspend before starting a CPU.
>
> Rename it to 'initial_stack' to be consistent with the others.
>

May be change the following line as well:

./arch/x86/kernel/head_64.S:69:     * Setup stack for verify_cpu().
"-8" because stack_start is defined


--
Nilay

[toc] | [prev] | [next] | [standalone]

#1457199 — Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack'

On Fri, Aug 05, 2016 at 10:28:39AM -0500, Nilay Vaish wrote:
> On 4 August 2016 at 17:21, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > The 'stack_start' variable is similar in usage to 'initial_code' and
> > 'initial_gs': they're all stored in head_64.S and they're all updated by
> > SMP and ACPI suspend before starting a CPU.
> >
> > Rename it to 'initial_stack' to be consistent with the others.
> >
> 
> May be change the following line as well:
> 
> ./arch/x86/kernel/head_64.S:69:     * Setup stack for verify_cpu().
> "-8" because stack_start is defined

Ah, yeah, I missed that one.

And also, now that I see that comment, and the line below it:

  leaq    (__end_init_task - 8)(%rip), %rsp

The 8 should be changed to SIZEOF_PTREGS in a later patch
("x86/asm/head: standardize the end of the stack for idle tasks").

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1457325 — Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack'

On Sat, Aug 06, 2016 at 07:25:21AM +0200, Borislav Petkov wrote:
> On Fri, Aug 05, 2016 at 11:01:57AM -0500, Josh Poimboeuf wrote:
> > The 8 should be changed to SIZEOF_PTREGS in a later patch
> > ("x86/asm/head: standardize the end of the stack for idle tasks").
> 
> But SIZEOF_PTREGS is 21*8. I don't understand.

I was referring to this patch:

  [PATCH v2 41/44] x86/asm/head: standardize the end of the stack for idle tasks
  https://lkml.kernel.org/r/98f297ffbc2a23131f08c5c77c4db974e0de2ad3.1470345772.git.jpoimboe@redhat.com

It changes the stack end offset from 8 to SIZEOF_PTREGS, so idle tasks
will have the same end of stack address that other tasks do.  I was
thinking we should make a similar change here, for consistency:

	/*
	 * Setup stack for verify_cpu(). "-8" because stack_start is defined
	 * this way, see below. Our best guess is a NULL ptr for stack
	 * termination heuristics and we don't want to break anything which
	 * might depend on it (kgdb, ...).
	 */
	leaq	(__end_init_task - 8)(%rip), %rsp

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1457342 — Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack'

On Fri, Aug 05, 2016 at 11:01:57AM -0500, Josh Poimboeuf wrote:
> The 8 should be changed to SIZEOF_PTREGS in a later patch
> ("x86/asm/head: standardize the end of the stack for idle tasks").

But SIZEOF_PTREGS is 21*8. I don't understand.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1457345 — Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack'

On Sat, Aug 06, 2016 at 09:15:30AM -0400, Brian Gerst wrote:
> On Sat, Aug 6, 2016 at 1:25 AM, Borislav Petkov <bp@alien8.de> wrote:
> > On Fri, Aug 05, 2016 at 11:01:57AM -0500, Josh Poimboeuf wrote:
> >> The 8 should be changed to SIZEOF_PTREGS in a later patch
> >> ("x86/asm/head: standardize the end of the stack for idle tasks").
> >
> > But SIZEOF_PTREGS is 21*8. I don't understand.
> 
> This patch is only for the boot cpu's idle thread.  All other kernel
> threads, including idle threads for the secondary cpus, already have
> the pt_regs area reserved.

Ah, you're right, it does only affect the boot CPU's idle thread.

(To be clear, I think you're talking about patch 41/44, and not the
temporary stack for the verify_cpu() call which I referred to above.)

> My best guess for the current 8 byte
> padding is to make sure thread_info is calculated properly (by masking
> off the low bits from RSP).
>
> Also, this fix should be applied to 32-bit, but make sure to account
> for TOP_OF_KERNEL_STACK_PADDING.

Ok.

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1457391 — Re: [PATCH v2 03/44] x86/asm/head: rename 'stack_start' -> 'initial_stack'

On Sat, Aug 6, 2016 at 1:25 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Aug 05, 2016 at 11:01:57AM -0500, Josh Poimboeuf wrote:
>> The 8 should be changed to SIZEOF_PTREGS in a later patch
>> ("x86/asm/head: standardize the end of the stack for idle tasks").
>
> But SIZEOF_PTREGS is 21*8. I don't understand.

This patch is only for the boot cpu's idle thread.  All other kernel
threads, including idle threads for the secondary cpus, already have
the pt_regs area reserved.  My best guess for the current 8 byte
padding is to make sure thread_info is calculated properly (by masking
off the low bits from RSP).

Also, this fix should be applied to 32-bit, but make sure to account
for TOP_OF_KERNEL_STACK_PADDING.

--
Brian Gerst

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web