* [PATCH 0/9] x86/entry/head: standardize the end of the stack
@ 2016-09-20 20:02 Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 1/9] x86/entry/head/32: use local labels Josh Poimboeuf
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
Thanks to all the recent x86 entry code refactoring, most tasks' kernel
stacks start at the same offset right below their saved pt_regs,
regardless of which syscall was used to enter the kernel. That creates
a nice convention which makes it straightforward to identify the end of
the stack, which can be useful [*] for the unwinder.
But there a few places where tasks don't yet follow the convention.
This patch set finishes the job.
[*] This will be useful for three upcoming proposed features:
- Detecting corrupt stacks in the unwinder (which will also be a force
for ensuring this end of stack convention continues to be followed in
the future).
- Printing all saved pt_regs on the stack during an oops/warning.
- Validating stacks in the livepatch consistency model.
Josh Poimboeuf (9):
x86/entry/head/32: use local labels
x86/entry/32: rename 'error_code' to 'common_exception'
x86/entry/32: fix the end of the stack for newly forked tasks
x86/head/32: fix the end of the stack for idle tasks
x86/smp: fix initial idle stack location on 32-bit
x86/asm/head: use a common function for starting CPUs
x86/head: put real return address on idle task stack
x86/head: fix the end of the stack for idle tasks
x86: move _stext marker to before head code
arch/x86/entry/entry_32.S | 120 ++++++++++++++++++++++++------------------
arch/x86/kernel/head_32.S | 49 ++++++++++-------
arch/x86/kernel/head_64.S | 42 +++++++--------
arch/x86/kernel/smpboot.c | 4 +-
arch/x86/kernel/vmlinux.lds.S | 2 +-
5 files changed, 122 insertions(+), 95 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/9] x86/entry/head/32: use local labels
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-21 0:57 ` Andy Lutomirski
2016-09-20 20:02 ` [PATCH 2/9] x86/entry/32: rename 'error_code' to 'common_exception' Josh Poimboeuf
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
Add the local label prefix to all non-function named labels in head_32.S
and entry_32.S. In addition to decluttering the symbol table, it also
will help stack traces to be more sensible. For example, the last
reported function in the idle task stack trace will be startup_32_smp()
instead of is486().
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/entry/entry_32.S | 55 ++++++++++++++++++++++++-----------------------
arch/x86/kernel/head_32.S | 32 +++++++++++++--------------
2 files changed, 44 insertions(+), 43 deletions(-)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index b75a8bc..378e912 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -64,7 +64,7 @@
# define preempt_stop(clobbers) DISABLE_INTERRUPTS(clobbers); TRACE_IRQS_OFF
#else
# define preempt_stop(clobbers)
-# define resume_kernel restore_all
+# define resume_kernel .Lrestore_all
#endif
.macro TRACE_IRQS_IRET
@@ -255,7 +255,7 @@ ENTRY(ret_from_fork)
/* When we fork, we trace the syscall return in the child, too. */
movl %esp, %eax
call syscall_return_slowpath
- jmp restore_all
+ jmp .Lrestore_all
/* kernel thread */
1: movl %edi, %eax
@@ -300,19 +300,19 @@ ENTRY(resume_userspace)
TRACE_IRQS_OFF
movl %esp, %eax
call prepare_exit_to_usermode
- jmp restore_all
+ jmp .Lrestore_all
END(ret_from_exception)
#ifdef CONFIG_PREEMPT
ENTRY(resume_kernel)
DISABLE_INTERRUPTS(CLBR_ANY)
-need_resched:
+.Lneed_resched:
cmpl $0, PER_CPU_VAR(__preempt_count)
- jnz restore_all
+ jnz .Lrestore_all
testl $X86_EFLAGS_IF, PT_EFLAGS(%esp) # interrupts off (exception path) ?
- jz restore_all
+ jz .Lrestore_all
call preempt_schedule_irq
- jmp need_resched
+ jmp .Lneed_resched
END(resume_kernel)
#endif
@@ -333,7 +333,7 @@ GLOBAL(__begin_SYSENTER_singlestep_region)
*/
ENTRY(xen_sysenter_target)
addl $5*4, %esp /* remove xen-provided frame */
- jmp sysenter_past_esp
+ jmp .Lsysenter_past_esp
#endif
/*
@@ -370,7 +370,7 @@ ENTRY(xen_sysenter_target)
*/
ENTRY(entry_SYSENTER_32)
movl TSS_sysenter_sp0(%esp), %esp
-sysenter_past_esp:
+.Lsysenter_past_esp:
pushl $__USER_DS /* pt_regs->ss */
pushl %ebp /* pt_regs->sp (stashed in bp) */
pushfl /* pt_regs->flags (except IF = 0) */
@@ -501,11 +501,11 @@ ENTRY(entry_INT80_32)
call do_int80_syscall_32
.Lsyscall_32_done:
-restore_all:
+.Lrestore_all:
TRACE_IRQS_IRET
-restore_all_notrace:
+.Lrestore_all_notrace:
#ifdef CONFIG_X86_ESPFIX32
- ALTERNATIVE "jmp restore_nocheck", "", X86_BUG_ESPFIX
+ ALTERNATIVE "jmp .Lrestore_nocheck", "", X86_BUG_ESPFIX
movl PT_EFLAGS(%esp), %eax # mix EFLAGS, SS and CS
/*
@@ -517,22 +517,23 @@ restore_all_notrace:
movb PT_CS(%esp), %al
andl $(X86_EFLAGS_VM | (SEGMENT_TI_MASK << 8) | SEGMENT_RPL_MASK), %eax
cmpl $((SEGMENT_LDT << 8) | USER_RPL), %eax
- je ldt_ss # returning to user-space with LDT SS
+ je .Lldt_ss # returning to user-space with LDT SS
#endif
-restore_nocheck:
+.Lrestore_nocheck:
RESTORE_REGS 4 # skip orig_eax/error_code
-irq_return:
+.Lirq_return:
INTERRUPT_RETURN
+
.section .fixup, "ax"
ENTRY(iret_exc )
pushl $0 # no error code
pushl $do_iret_error
jmp error_code
.previous
- _ASM_EXTABLE(irq_return, iret_exc)
+ _ASM_EXTABLE(.Lirq_return, iret_exc)
#ifdef CONFIG_X86_ESPFIX32
-ldt_ss:
+.Lldt_ss:
/*
* Setup and switch to ESPFIX stack
*
@@ -561,7 +562,7 @@ ldt_ss:
*/
DISABLE_INTERRUPTS(CLBR_EAX)
lss (%esp), %esp /* switch to espfix segment */
- jmp restore_nocheck
+ jmp .Lrestore_nocheck
#endif
ENDPROC(entry_INT80_32)
@@ -881,7 +882,7 @@ ftrace_call:
popl %edx
popl %ecx
popl %eax
-ftrace_ret:
+.Lftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
ftrace_graph_call:
@@ -951,7 +952,7 @@ GLOBAL(ftrace_regs_call)
popl %gs
addl $8, %esp /* Skip orig_ax and ip */
popf /* Pop flags at end (no addl to corrupt flags) */
- jmp ftrace_ret
+ jmp .Lftrace_ret
popf
jmp ftrace_stub
@@ -962,7 +963,7 @@ ENTRY(mcount)
jb ftrace_stub /* Paging not enabled yet? */
cmpl $ftrace_stub, ftrace_trace_function
- jnz trace
+ jnz .Ltrace
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
cmpl $ftrace_stub, ftrace_graph_return
jnz ftrace_graph_caller
@@ -975,7 +976,7 @@ ftrace_stub:
ret
/* taken from glibc */
-trace:
+.Ltrace:
pushl %eax
pushl %ecx
pushl %edx
@@ -1114,7 +1115,7 @@ ENTRY(nmi)
movl %ss, %eax
cmpw $__ESPFIX_SS, %ax
popl %eax
- je nmi_espfix_stack
+ je .Lnmi_espfix_stack
#endif
pushl %eax # pt_regs->orig_ax
@@ -1130,7 +1131,7 @@ ENTRY(nmi)
/* Not on SYSENTER stack. */
call do_nmi
- jmp restore_all_notrace
+ jmp .Lrestore_all_notrace
.Lnmi_from_sysenter_stack:
/*
@@ -1141,10 +1142,10 @@ ENTRY(nmi)
movl PER_CPU_VAR(cpu_current_top_of_stack), %esp
call do_nmi
movl %ebp, %esp
- jmp restore_all_notrace
+ jmp .Lrestore_all_notrace
#ifdef CONFIG_X86_ESPFIX32
-nmi_espfix_stack:
+.Lnmi_espfix_stack:
/*
* create the pointer to lss back
*/
@@ -1162,7 +1163,7 @@ nmi_espfix_stack:
call do_nmi
RESTORE_REGS
lss 12+4(%esp), %esp # back to espfix stack
- jmp irq_return
+ jmp .Lirq_return
#endif
END(nmi)
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 5f40126..617fba2 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -247,19 +247,19 @@ page_pde_offset = (__PAGE_OFFSET >> 20);
#ifdef CONFIG_PARAVIRT
/* This is can only trip for a broken bootloader... */
cmpw $0x207, pa(boot_params + BP_version)
- jb default_entry
+ jb .Ldefault_entry
/* Paravirt-compatible boot parameters. Look to see what architecture
we're booting under. */
movl pa(boot_params + BP_hardware_subarch), %eax
cmpl $num_subarch_entries, %eax
- jae bad_subarch
+ jae .Lbad_subarch
movl pa(subarch_entries)(,%eax,4), %eax
subl $__PAGE_OFFSET, %eax
jmp *%eax
-bad_subarch:
+.Lbad_subarch:
WEAK(lguest_entry)
WEAK(xen_entry)
/* Unknown implementation; there's really
@@ -269,14 +269,14 @@ WEAK(xen_entry)
__INITDATA
subarch_entries:
- .long default_entry /* normal x86/PC */
+ .long .Ldefault_entry /* normal x86/PC */
.long lguest_entry /* lguest hypervisor */
.long xen_entry /* Xen hypervisor */
- .long default_entry /* Moorestown MID */
+ .long .Ldefault_entry /* Moorestown MID */
num_subarch_entries = (. - subarch_entries) / 4
.previous
#else
- jmp default_entry
+ jmp .Ldefault_entry
#endif /* CONFIG_PARAVIRT */
#ifdef CONFIG_HOTPLUG_CPU
@@ -316,7 +316,7 @@ ENTRY(startup_32_smp)
call load_ucode_ap
#endif
-default_entry:
+.Ldefault_entry:
#define CR0_STATE (X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | \
X86_CR0_NE | X86_CR0_WP | X86_CR0_AM | \
X86_CR0_PG)
@@ -346,7 +346,7 @@ default_entry:
pushfl
popl %eax # get EFLAGS
testl $X86_EFLAGS_ID,%eax # did EFLAGS.ID remained set?
- jz enable_paging # hw disallowed setting of ID bit
+ jz .Lenable_paging # hw disallowed setting of ID bit
# which means no CPUID and no CR4
xorl %eax,%eax
@@ -356,13 +356,13 @@ default_entry:
movl $1,%eax
cpuid
andl $~1,%edx # Ignore CPUID.FPU
- jz enable_paging # No flags or only CPUID.FPU = no CR4
+ jz .Lenable_paging # No flags or only CPUID.FPU = no CR4
movl pa(mmu_cr4_features),%eax
movl %eax,%cr4
testb $X86_CR4_PAE, %al # check if PAE is enabled
- jz enable_paging
+ jz .Lenable_paging
/* Check if extended functions are implemented */
movl $0x80000000, %eax
@@ -370,7 +370,7 @@ default_entry:
/* Value must be in the range 0x80000001 to 0x8000ffff */
subl $0x80000001, %eax
cmpl $(0x8000ffff-0x80000001), %eax
- ja enable_paging
+ ja .Lenable_paging
/* Clear bogus XD_DISABLE bits */
call verify_cpu
@@ -379,7 +379,7 @@ default_entry:
cpuid
/* Execute Disable bit supported? */
btl $(X86_FEATURE_NX & 31), %edx
- jnc enable_paging
+ jnc .Lenable_paging
/* Setup EFER (Extended Feature Enable Register) */
movl $MSR_EFER, %ecx
@@ -389,7 +389,7 @@ default_entry:
/* Make changes effective */
wrmsr
-enable_paging:
+.Lenable_paging:
/*
* Enable paging
@@ -418,7 +418,7 @@ enable_paging:
*/
movb $4,X86 # at least 486
cmpl $-1,X86_CPUID
- je is486
+ je .Lis486
/* get vendor info */
xorl %eax,%eax # call CPUID with 0 -> return vendor ID
@@ -429,7 +429,7 @@ enable_paging:
movl %ecx,X86_VENDOR_ID+8 # last 4 chars
orl %eax,%eax # do we have processor info as well?
- je is486
+ je .Lis486
movl $1,%eax # Use the CPUID instruction to get CPU type
cpuid
@@ -443,7 +443,7 @@ enable_paging:
movb %cl,X86_MASK
movl %edx,X86_CAPABILITY
-is486:
+.Lis486:
movl $0x50022,%ecx # set AM, WP, NE and MP
movl %cr0,%eax
andl $0x80000011,%eax # Save PG,PE,ET
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/9] x86/entry/32: rename 'error_code' to 'common_exception'
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 1/9] x86/entry/head/32: use local labels Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks Josh Poimboeuf
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
The 'error_code' label is awkwardly named, especially when it shows up
in a stack trace. Move it to its own local function and rename it to
'common_exception', analagous to the existing 'common_interrupt'.
This also makes related stack traces more sensible.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/entry/entry_32.S | 43 +++++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 378e912..deef561 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -528,7 +528,7 @@ ENTRY(entry_INT80_32)
ENTRY(iret_exc )
pushl $0 # no error code
pushl $do_iret_error
- jmp error_code
+ jmp common_exception
.previous
_ASM_EXTABLE(.Lirq_return, iret_exc)
@@ -659,7 +659,7 @@ ENTRY(coprocessor_error)
ASM_CLAC
pushl $0
pushl $do_coprocessor_error
- jmp error_code
+ jmp common_exception
END(coprocessor_error)
ENTRY(simd_coprocessor_error)
@@ -673,14 +673,14 @@ ENTRY(simd_coprocessor_error)
#else
pushl $do_simd_coprocessor_error
#endif
- jmp error_code
+ jmp common_exception
END(simd_coprocessor_error)
ENTRY(device_not_available)
ASM_CLAC
pushl $-1 # mark this as an int
pushl $do_device_not_available
- jmp error_code
+ jmp common_exception
END(device_not_available)
#ifdef CONFIG_PARAVIRT
@@ -694,59 +694,59 @@ ENTRY(overflow)
ASM_CLAC
pushl $0
pushl $do_overflow
- jmp error_code
+ jmp common_exception
END(overflow)
ENTRY(bounds)
ASM_CLAC
pushl $0
pushl $do_bounds
- jmp error_code
+ jmp common_exception
END(bounds)
ENTRY(invalid_op)
ASM_CLAC
pushl $0
pushl $do_invalid_op
- jmp error_code
+ jmp common_exception
END(invalid_op)
ENTRY(coprocessor_segment_overrun)
ASM_CLAC
pushl $0
pushl $do_coprocessor_segment_overrun
- jmp error_code
+ jmp common_exception
END(coprocessor_segment_overrun)
ENTRY(invalid_TSS)
ASM_CLAC
pushl $do_invalid_TSS
- jmp error_code
+ jmp common_exception
END(invalid_TSS)
ENTRY(segment_not_present)
ASM_CLAC
pushl $do_segment_not_present
- jmp error_code
+ jmp common_exception
END(segment_not_present)
ENTRY(stack_segment)
ASM_CLAC
pushl $do_stack_segment
- jmp error_code
+ jmp common_exception
END(stack_segment)
ENTRY(alignment_check)
ASM_CLAC
pushl $do_alignment_check
- jmp error_code
+ jmp common_exception
END(alignment_check)
ENTRY(divide_error)
ASM_CLAC
pushl $0 # no error code
pushl $do_divide_error
- jmp error_code
+ jmp common_exception
END(divide_error)
#ifdef CONFIG_X86_MCE
@@ -754,7 +754,7 @@ ENTRY(machine_check)
ASM_CLAC
pushl $0
pushl machine_check_vector
- jmp error_code
+ jmp common_exception
END(machine_check)
#endif
@@ -762,7 +762,7 @@ ENTRY(spurious_interrupt_bug)
ASM_CLAC
pushl $0
pushl $do_spurious_interrupt_bug
- jmp error_code
+ jmp common_exception
END(spurious_interrupt_bug)
#ifdef CONFIG_XEN
@@ -1026,7 +1026,7 @@ return_to_handler:
ENTRY(trace_page_fault)
ASM_CLAC
pushl $trace_do_page_fault
- jmp error_code
+ jmp common_exception
END(trace_page_fault)
#endif
@@ -1034,7 +1034,10 @@ ENTRY(page_fault)
ASM_CLAC
pushl $do_page_fault
ALIGN
-error_code:
+ jmp common_exception
+END(page_fault)
+
+common_exception:
/* the function address is in %gs's slot on the stack */
pushl %fs
pushl %es
@@ -1063,7 +1066,7 @@ error_code:
movl %esp, %eax # pt_regs pointer
call *%edi
jmp ret_from_exception
-END(page_fault)
+END(common_exception)
ENTRY(debug)
/*
@@ -1180,14 +1183,14 @@ END(int3)
ENTRY(general_protection)
pushl $do_general_protection
- jmp error_code
+ jmp common_exception
END(general_protection)
#ifdef CONFIG_KVM_GUEST
ENTRY(async_page_fault)
ASM_CLAC
pushl $do_async_page_fault
- jmp error_code
+ jmp common_exception
END(async_page_fault)
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 1/9] x86/entry/head/32: use local labels Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 2/9] x86/entry/32: rename 'error_code' to 'common_exception' Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-21 1:10 ` Brian Gerst
2016-09-20 20:02 ` [PATCH 4/9] x86/head/32: fix the end of the stack for idle tasks Josh Poimboeuf
` (5 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
Thanks to all the recent x86 entry code refactoring, most tasks' kernel
stacks start at the same offset right below their saved pt_regs,
regardless of which syscall was used to enter the kernel. That creates
a nice convention which makes it straightforward to identify the end of
the stack, which can be useful for the unwinder to verify the stack is
sane.
Calling schedule_tail() directly breaks that convention because its an
asmlinkage function so its argument has to be pushed on the stack. Add
a wrapper which creates a proper "end of stack" frame header before the
call.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/entry/entry_32.S | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index deef561..f0a7444 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -44,6 +44,7 @@
#include <asm/alternative-asm.h>
#include <asm/asm.h>
#include <asm/smap.h>
+#include <asm/frame.h>
.section .entry.text, "ax"
@@ -237,6 +238,23 @@ ENTRY(__switch_to_asm)
END(__switch_to_asm)
/*
+ * The unwinder expects the last frame on the stack to always be at the same
+ * offset from the end of the page, which allows it to validate the stack.
+ * Calling schedule_tail() directly would break that convention because its an
+ * asmlinkage function so its argument has to be pushed on the stack. This
+ * wrapper creates a proper "end of stack" frame header before the call.
+ */
+ENTRY(schedule_tail_wrapper)
+ FRAME_BEGIN
+
+ pushl %eax
+ call schedule_tail
+ popl %eax
+
+ FRAME_END
+ ret
+ENDPROC(schedule_tail_wrapper)
+/*
* A newly forked process directly context switches into this address.
*
* eax: prev task we switched from
@@ -244,9 +262,7 @@ END(__switch_to_asm)
* edi: kernel thread arg
*/
ENTRY(ret_from_fork)
- pushl %eax
- call schedule_tail
- popl %eax
+ call schedule_tail_wrapper
testl %ebx, %ebx
jnz 1f /* kernel threads are uncommon */
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/9] x86/head/32: fix the end of the stack for idle tasks
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
` (2 preceding siblings ...)
2016-09-20 20:02 ` [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 5/9] x86/smp: fix initial idle stack location on 32-bit Josh Poimboeuf
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
The frame at the end of each idle task stack is inconsistent with real
task stacks, which have a stack frame header and a real return address
before the pt_regs area. This inconsistency can be confusing for stack
unwinders. It also hides useful information about what asm code was
involved in calling into C.
Fix that by changing the initial code jumps to calls. Also add infinite
loops after the calls to make it clear that the calls don't return, and
to hang if they do.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/kernel/head_32.S | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 617fba2..7aac1b9 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -288,7 +288,8 @@ num_subarch_entries = (. - subarch_entries) / 4
ENTRY(start_cpu0)
movl initial_stack, %ecx
movl %ecx, %esp
- jmp *(initial_code)
+ call *(initial_code)
+1: jmp 1b
ENDPROC(start_cpu0)
#endif
@@ -469,8 +470,9 @@ ENTRY(startup_32_smp)
xorl %eax,%eax # Clear LDT
lldt %ax
- pushl $0 # fake return address for unwinder
- jmp *(initial_code)
+ call *(initial_code)
+1: jmp 1b
+ENDPROC(startup_32_smp)
#include "verify_cpu.S"
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/9] x86/smp: fix initial idle stack location on 32-bit
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
` (3 preceding siblings ...)
2016-09-20 20:02 ` [PATCH 4/9] x86/head/32: fix the end of the stack for idle tasks Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 6/9] x86/asm/head: use a common function for starting CPUs Josh Poimboeuf
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
On 32-bit, the initial idle stack calculation doesn't take into account
the TOP_OF_KERNEL_STACK_PADDING, making the stack end address
inconsistent with other tasks on 32-bit.
Reviewed-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/kernel/smpboot.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 039790b..9345072 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -963,9 +963,7 @@ static int do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
int cpu0_nmi_registered = 0;
unsigned long timeout;
- idle->thread.sp = (unsigned long) (((struct pt_regs *)
- (THREAD_SIZE + task_stack_page(idle))) - 1);
-
+ idle->thread.sp = (unsigned long)task_pt_regs(idle);
early_gdt_descr.address = (unsigned long)get_cpu_gdt_table(cpu);
initial_code = (unsigned long)start_secondary;
initial_stack = idle->thread.sp;
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/9] x86/asm/head: use a common function for starting CPUs
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
` (4 preceding siblings ...)
2016-09-20 20:02 ` [PATCH 5/9] x86/smp: fix initial idle stack location on 32-bit Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 7/9] x86/head: put real return address on idle task stack Josh Poimboeuf
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
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 | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index c98a559..3621ad2 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -264,13 +264,17 @@ 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
+ jmp start_cpu
+ENDPROC(secondary_startup_64)
+
+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
@@ -299,7 +303,7 @@ ENTRY(secondary_startup_64)
pushq $__KERNEL_CS # set correct cs
pushq %rax # target address in negative space
lretq
-ENDPROC(secondary_startup_64)
+ENDPROC(start_cpu)
#include "verify_cpu.S"
@@ -307,15 +311,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/9] x86/head: put real return address on idle task stack
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
` (5 preceding siblings ...)
2016-09-20 20:02 ` [PATCH 6/9] x86/asm/head: use a common function for starting CPUs Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 8/9] x86/head: fix the end of the stack for idle tasks Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 9/9] x86: move _stext marker to before head code Josh Poimboeuf
8 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
The frame at the end of each idle task stack has a zeroed 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. It also hides useful information about what asm code
was involved in calling into C.
Make it a real address by using the side effect of a call instruction to
push the instruction pointer on the stack.
Reviewed-by: Andy Lutomirski <luto@kernel.org>
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 3621ad2..c90f481 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -298,8 +298,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 8/9] x86/head: fix the end of the stack for idle tasks
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
` (6 preceding siblings ...)
2016-09-20 20:02 ` [PATCH 7/9] x86/head: put real return address on idle task stack Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 9/9] x86: move _stext marker to before head code Josh Poimboeuf
8 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
Thanks to all the recent x86 entry code refactoring, most tasks' kernel
stacks start at the same offset right below their saved pt_regs,
regardless of which syscall was used to enter the kernel. That creates
a nice convention which makes it straightforward to identify the end of
the stack, which can be useful for the unwinder to verify the stack is
sane.
However, the boot CPU's idle "swapper" task doesn't follow that
convention. Fix that by starting its stack at a sizeof(pt_regs) offset
from the end of the stack page.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/kernel/head_32.S | 9 ++++++++-
arch/x86/kernel/head_64.S | 15 +++++++--------
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 7aac1b9..d87bec4 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -62,6 +62,8 @@
#define PAGE_TABLE_SIZE(pages) ((pages) / PTRS_PER_PGD)
#endif
+#define SIZEOF_PTREGS 17*4
+
/*
* Number of possible pages in the lowmem region.
*
@@ -706,7 +708,12 @@ ENTRY(initial_page_table)
.data
.balign 4
ENTRY(initial_stack)
- .long init_thread_union+THREAD_SIZE
+ /*
+ * The SIZEOF_PTREGS gap is a convention which helps the in-kernel
+ * unwinder reliably detect the end of the stack.
+ */
+ .long init_thread_union + THREAD_SIZE - SIZEOF_PTREGS - \
+ TOP_OF_KERNEL_STACK_PADDING;
__INITRODATA
int_msg:
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index c90f481..ec332e9 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -65,13 +65,8 @@ startup_64:
* tables and then reload them.
*/
- /*
- * Setup stack for verify_cpu(). "-8" because initial_stack 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
+ /* Set up the stack for verify_cpu(), similar to initial_stack below */
+ leaq (__end_init_task - SIZEOF_PTREGS)(%rip), %rsp
/* Sanitize CPU configuration */
call verify_cpu
@@ -328,7 +323,11 @@ ENDPROC(start_cpu0)
GLOBAL(initial_gs)
.quad INIT_PER_CPU_VAR(irq_stack_union)
GLOBAL(initial_stack)
- .quad init_thread_union+THREAD_SIZE-8
+ /*
+ * The SIZEOF_PTREGS gap is a convention which helps the in-kernel
+ * unwinder reliably detect the end of the stack.
+ */
+ .quad init_thread_union + THREAD_SIZE - SIZEOF_PTREGS
__FINITDATA
bad_address:
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 9/9] x86: move _stext marker to before head code
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
` (7 preceding siblings ...)
2016-09-20 20:02 ` [PATCH 8/9] x86/head: fix the end of the stack for idle tasks Josh Poimboeuf
@ 2016-09-20 20:02 ` Josh Poimboeuf
8 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-20 20:02 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin
Cc: x86, linux-kernel, Andy Lutomirski, Linus Torvalds, Brian Gerst,
Peter Zijlstra, Nilay Vaish
When core_kernel_text() is used to determine whether an address on a
task's stack trace is a kernel text address, it incorrectly returns
false for early text addresses for the head code between the _text and
_stext markers. Among other things, this can cause the unwinder to
behave incorrectly when unwinding to x86 head code.
Head code is text code too, so mark it as such. This seems to match the
intent of other users of the _stext symbol, and it also seems consistent
with what other architectures are already doing.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/kernel/vmlinux.lds.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 9297a00..1d9b636 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -91,10 +91,10 @@ SECTIONS
/* Text and read-only data */
.text : AT(ADDR(.text) - LOAD_OFFSET) {
_text = .;
+ _stext = .;
/* bootstrapping code */
HEAD_TEXT
. = ALIGN(8);
- _stext = .;
TEXT_TEXT
SCHED_TEXT
LOCK_TEXT
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/9] x86/entry/head/32: use local labels
2016-09-20 20:02 ` [PATCH 1/9] x86/entry/head/32: use local labels Josh Poimboeuf
@ 2016-09-21 0:57 ` Andy Lutomirski
2016-09-21 2:52 ` Josh Poimboeuf
0 siblings, 1 reply; 17+ messages in thread
From: Andy Lutomirski @ 2016-09-21 0:57 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Thomas Gleixner, linux-kernel, H . Peter Anvin, Nilay Vaish,
Ingo Molnar, Brian Gerst, X86 ML, Peter Zijlstra, Linus Torvalds
On Sep 20, 2016 10:03 AM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote:
>
> Add the local label prefix to all non-function named labels in head_32.S
> and entry_32.S. In addition to decluttering the symbol table, it also
> will help stack traces to be more sensible. For example, the last
> reported function in the idle task stack trace will be startup_32_smp()
> instead of is486().
>
I think that restore_all, at least, should stay. It's a common tail
for lots of functions. I haven't checked this patch for other cases
where the new label is worse than the old for debugging.
--Andy
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
2016-09-20 20:02 ` [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks Josh Poimboeuf
@ 2016-09-21 1:10 ` Brian Gerst
2016-09-21 3:25 ` Josh Poimboeuf
0 siblings, 1 reply; 17+ messages in thread
From: Brian Gerst @ 2016-09-21 1:10 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
the arch/x86 maintainers, Linux Kernel Mailing List,
Andy Lutomirski, Linus Torvalds, Peter Zijlstra, Nilay Vaish
On Tue, Sep 20, 2016 at 4:02 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> Thanks to all the recent x86 entry code refactoring, most tasks' kernel
> stacks start at the same offset right below their saved pt_regs,
> regardless of which syscall was used to enter the kernel. That creates
> a nice convention which makes it straightforward to identify the end of
> the stack, which can be useful for the unwinder to verify the stack is
> sane.
>
> Calling schedule_tail() directly breaks that convention because its an
> asmlinkage function so its argument has to be pushed on the stack. Add
> a wrapper which creates a proper "end of stack" frame header before the
> call.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
> arch/x86/entry/entry_32.S | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index deef561..f0a7444 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -44,6 +44,7 @@
> #include <asm/alternative-asm.h>
> #include <asm/asm.h>
> #include <asm/smap.h>
> +#include <asm/frame.h>
>
> .section .entry.text, "ax"
>
> @@ -237,6 +238,23 @@ ENTRY(__switch_to_asm)
> END(__switch_to_asm)
>
> /*
> + * The unwinder expects the last frame on the stack to always be at the same
> + * offset from the end of the page, which allows it to validate the stack.
> + * Calling schedule_tail() directly would break that convention because its an
> + * asmlinkage function so its argument has to be pushed on the stack. This
> + * wrapper creates a proper "end of stack" frame header before the call.
> + */
> +ENTRY(schedule_tail_wrapper)
> + FRAME_BEGIN
> +
> + pushl %eax
> + call schedule_tail
> + popl %eax
> +
> + FRAME_END
> + ret
> +ENDPROC(schedule_tail_wrapper)
> +/*
> * A newly forked process directly context switches into this address.
> *
> * eax: prev task we switched from
> @@ -244,9 +262,7 @@ END(__switch_to_asm)
> * edi: kernel thread arg
> */
> ENTRY(ret_from_fork)
> - pushl %eax
> - call schedule_tail
> - popl %eax
> + call schedule_tail_wrapper
>
> testl %ebx, %ebx
> jnz 1f /* kernel threads are uncommon */
> --
> 2.7.4
>
Dropping asmlinkage from schedule_tail() would be a better option if possible.
--
Brian Gerst
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/9] x86/entry/head/32: use local labels
2016-09-21 0:57 ` Andy Lutomirski
@ 2016-09-21 2:52 ` Josh Poimboeuf
0 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-21 2:52 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Thomas Gleixner, linux-kernel, H . Peter Anvin, Nilay Vaish,
Ingo Molnar, Brian Gerst, X86 ML, Peter Zijlstra, Linus Torvalds
On Tue, Sep 20, 2016 at 05:57:16PM -0700, Andy Lutomirski wrote:
> On Sep 20, 2016 10:03 AM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote:
> >
> > Add the local label prefix to all non-function named labels in head_32.S
> > and entry_32.S. In addition to decluttering the symbol table, it also
> > will help stack traces to be more sensible. For example, the last
> > reported function in the idle task stack trace will be startup_32_smp()
> > instead of is486().
> >
>
> I think that restore_all, at least, should stay. It's a common tail
> for lots of functions. I haven't checked this patch for other cases
> where the new label is worse than the old for debugging.
Yes, for restore_all, I think you're right. I'll change it back.
--
Josh
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
2016-09-21 1:10 ` Brian Gerst
@ 2016-09-21 3:25 ` Josh Poimboeuf
2016-09-21 3:33 ` Josh Poimboeuf
2016-09-21 6:39 ` Andy Lutomirski
0 siblings, 2 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-21 3:25 UTC (permalink / raw)
To: Brian Gerst
Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
the arch/x86 maintainers, Linux Kernel Mailing List,
Andy Lutomirski, Linus Torvalds, Peter Zijlstra, Nilay Vaish
On Tue, Sep 20, 2016 at 09:10:55PM -0400, Brian Gerst wrote:
> Dropping asmlinkage from schedule_tail() would be a better option if possible.
My understanding is that it's still needed for ia64. AFAICT, ia64
relies on schedule_tail() having the syscall_linkage function attribute.
>From the gcc manual:
This attribute is used to modify the IA64 calling convention by
marking all input registers as live at all function exits. This makes
it possible to restart a system call after an interrupt without having
to save/restore the input registers. This also prevents kernel data
from leaking into application code.
And the ia64 entry code has some similar language:
/*
* Invoke schedule_tail(task) while preserving in0-in7, which may be needed
* in case a system call gets restarted.
*/
GLOBAL_ENTRY(ia64_invoke_schedule_tail)
...
--
Josh
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
2016-09-21 3:25 ` Josh Poimboeuf
@ 2016-09-21 3:33 ` Josh Poimboeuf
2016-09-21 6:39 ` Andy Lutomirski
1 sibling, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-21 3:33 UTC (permalink / raw)
To: Brian Gerst
Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin,
the arch/x86 maintainers, Linux Kernel Mailing List,
Andy Lutomirski, Linus Torvalds, Peter Zijlstra, Nilay Vaish
On Tue, Sep 20, 2016 at 10:25:16PM -0500, Josh Poimboeuf wrote:
> On Tue, Sep 20, 2016 at 09:10:55PM -0400, Brian Gerst wrote:
> > Dropping asmlinkage from schedule_tail() would be a better option if possible.
>
> My understanding is that it's still needed for ia64. AFAICT, ia64
> relies on schedule_tail() having the syscall_linkage function attribute.
> From the gcc manual:
>
> This attribute is used to modify the IA64 calling convention by
> marking all input registers as live at all function exits. This makes
> it possible to restart a system call after an interrupt without having
> to save/restore the input registers. This also prevents kernel data
> from leaking into application code.
>
> And the ia64 entry code has some similar language:
>
> /*
> * Invoke schedule_tail(task) while preserving in0-in7, which may be needed
> * in case a system call gets restarted.
> */
> GLOBAL_ENTRY(ia64_invoke_schedule_tail)
> ...
(But I should note that I'm a complete ia64 neophyte, so I could be
misreading that...)
--
Josh
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
2016-09-21 3:25 ` Josh Poimboeuf
2016-09-21 3:33 ` Josh Poimboeuf
@ 2016-09-21 6:39 ` Andy Lutomirski
2016-09-21 11:35 ` Josh Poimboeuf
1 sibling, 1 reply; 17+ messages in thread
From: Andy Lutomirski @ 2016-09-21 6:39 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Thomas Gleixner, H. Peter Anvin, Nilay Vaish,
the arch/x86 maintainers, Ingo Molnar, Linux Kernel Mailing List,
Brian Gerst, Peter Zijlstra, Linus Torvalds
On Sep 20, 2016 5:25 PM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote:
>
> On Tue, Sep 20, 2016 at 09:10:55PM -0400, Brian Gerst wrote:
> > Dropping asmlinkage from schedule_tail() would be a better option if possible.
>
> My understanding is that it's still needed for ia64. AFAICT, ia64
> relies on schedule_tail() having the syscall_linkage function attribute.
> From the gcc manual:
>
> This attribute is used to modify the IA64 calling convention by
> marking all input registers as live at all function exits. This makes
> it possible to restart a system call after an interrupt without having
> to save/restore the input registers. This also prevents kernel data
> from leaking into application code.
/me needs to excise this from i386. The amount of BS code involved to
avoid a whopping *six* register saves per syscall was absurd.
>
> And the ia64 entry code has some similar language:
>
> /*
> * Invoke schedule_tail(task) while preserving in0-in7, which may be needed
> * in case a system call gets restarted.
> */
> GLOBAL_ENTRY(ia64_invoke_schedule_tail)
> ...
That comment has to be wrong. What syscall could possibly be
restarted across schedule_tail()? It's a brand new thread and has
literally never done a syscall.
There may be another reason that the registers are live there, but I
generally do my best to never look at ia64 asm code.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
2016-09-21 6:39 ` Andy Lutomirski
@ 2016-09-21 11:35 ` Josh Poimboeuf
0 siblings, 0 replies; 17+ messages in thread
From: Josh Poimboeuf @ 2016-09-21 11:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Thomas Gleixner, H. Peter Anvin, Nilay Vaish,
the arch/x86 maintainers, Ingo Molnar, Linux Kernel Mailing List,
Brian Gerst, Peter Zijlstra, Linus Torvalds
On Tue, Sep 20, 2016 at 11:39:35PM -0700, Andy Lutomirski wrote:
> > And the ia64 entry code has some similar language:
> >
> > /*
> > * Invoke schedule_tail(task) while preserving in0-in7, which may be needed
> > * in case a system call gets restarted.
> > */
> > GLOBAL_ENTRY(ia64_invoke_schedule_tail)
> > ...
>
> That comment has to be wrong. What syscall could possibly be
> restarted across schedule_tail()? It's a brand new thread and has
> literally never done a syscall.
Hm, yeah, that comment doesn't make any sense.
> There may be another reason that the registers are live there, but I
> generally do my best to never look at ia64 asm code.
Yeah, I'm just going to turn around, pretend I never saw it, and slowly
walk away...
--
Josh
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-09-21 11:35 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20 20:02 [PATCH 0/9] x86/entry/head: standardize the end of the stack Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 1/9] x86/entry/head/32: use local labels Josh Poimboeuf
2016-09-21 0:57 ` Andy Lutomirski
2016-09-21 2:52 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 2/9] x86/entry/32: rename 'error_code' to 'common_exception' Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks Josh Poimboeuf
2016-09-21 1:10 ` Brian Gerst
2016-09-21 3:25 ` Josh Poimboeuf
2016-09-21 3:33 ` Josh Poimboeuf
2016-09-21 6:39 ` Andy Lutomirski
2016-09-21 11:35 ` Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 4/9] x86/head/32: fix the end of the stack for idle tasks Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 5/9] x86/smp: fix initial idle stack location on 32-bit Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 6/9] x86/asm/head: use a common function for starting CPUs Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 7/9] x86/head: put real return address on idle task stack Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 8/9] x86/head: fix the end of the stack for idle tasks Josh Poimboeuf
2016-09-20 20:02 ` [PATCH 9/9] x86: move _stext marker to before head code Josh Poimboeuf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.