[PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
From: Joerg Roedel We want x86_tss.sp0 point to the entry stack later to use it as a trampoline stack for other kernel entry points besides SYSENTER. So store the task stack pointer in x86_tss.sp1, which is otherwise unused by the hardware, as Linux doesn't make use of Ring 1. Signed-off-by: Joerg Roedel --- arch/x86/kernel/asm-offsets_32.c | 9 +++-- arch/x86/kernel/process_32.c | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c index 15b3f45..82826f2 100644 --- a/arch/x86/kernel/asm-offsets_32.c +++ b/arch/x86/kernel/asm-offsets_32.c @@ -46,9 +46,14 @@ void foo(void) OFFSET(saved_context_gdt_desc, saved_context, gdt_desc); BLANK(); - /* Offset from the entry stack to task stack stored in TSS */ + /* +* Offset from the entry stack to task stack stored in TSS. Kernel entry +* happens on the per-cpu entry-stack, and the asm code switches to the +* task-stack pointer stored in x86_tss.sp1, which is a copy of +* task->thread.sp0 where entry code can find it. +*/ DEFINE(TSS_entry2task_stack, - offsetof(struct cpu_entry_area, tss.x86_tss.sp0) - + offsetof(struct cpu_entry_area, tss.x86_tss.sp1) - offsetofend(struct cpu_entry_area, entry_stack_page.stack)); #ifdef CONFIG_STACKPROTECTOR diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index 0ae659d..ec62cc7 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -290,6 +290,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) this_cpu_write(cpu_current_top_of_stack, (unsigned long)task_stack_page(next_p) + THREAD_SIZE); + /* SYSENTER reads the task-stack from tss.sp1 */ + this_cpu_write(cpu_tss_rw.x86_tss.sp1, next_p->thread.sp0); /* * Restore %gs if needed (which is common) -- 2.7.4
Re: [PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
On Tue, Jul 17, 2018 at 12:05 AM, Joerg Roedel wrote: > On Fri, Jul 13, 2018 at 04:17:40PM -0700, Andy Lutomirski wrote: >> I re-read it again. How about keeping TSS_entry_stack but making it >> be the offset from the TSS to the entry stack. Then do the arithmetic >> in asm. > > Hmm, I think its better to keep the arithmetic in the C file for better > readability. How about renaming it to TSS_entry2task_stack? That's okay with me. > > > Regards, > > Joerg
Re: [PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
On Fri, Jul 13, 2018 at 04:17:40PM -0700, Andy Lutomirski wrote: > I re-read it again. How about keeping TSS_entry_stack but making it > be the offset from the TSS to the entry stack. Then do the arithmetic > in asm. Hmm, I think its better to keep the arithmetic in the C file for better readability. How about renaming it to TSS_entry2task_stack? Regards, Joerg
Re: [PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
On Fri, Jul 13, 2018 at 10:19 AM, Andy Lutomirski wrote: > On Fri, Jul 13, 2018 at 2:48 AM, Joerg Roedel wrote: >> On Thu, Jul 12, 2018 at 01:49:13PM -0700, Andy Lutomirski wrote: >>> > On Jul 11, 2018, at 4:29 AM, Joerg Roedel wrote: >>> >/* Offset from the sysenter stack to tss.sp0 */ >>> > -DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, >>> > tss.x86_tss.sp0) - >>> > +DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, >>> > tss.x86_tss.sp1) - >>> > offsetofend(struct cpu_entry_area, entry_stack_page.stack)); >>> > >>> >>> The code reads differently. Did you perhaps mean TSS_task_stack? >> >> Well, the offset name came from TSS_sysenter_sp0, which was the offset >> from the sysenter_sp0 (==sysenter-stack) to the task stack in TSS, now >> sysenter_sp0 became entry_stack, because its used for all entry points >> and not only sysenter. So with the old convention the naming makes still >> sense, no? >> > > Trying to parse it certainly makes my brain hurt a bit. This is the > offset from the entry stack to sp1, where sp1 is the location of the > pointer to the task stack. > > Maybe all the arithmetic could go in entry_32.S and the asm-offset > name could just be TSS_sp1, just like on 64-bit? > I re-read it again. How about keeping TSS_entry_stack but making it be the offset from the TSS to the entry stack. Then do the arithmetic in asm.
Re: [PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
On Fri, Jul 13, 2018 at 2:48 AM, Joerg Roedel wrote: > On Thu, Jul 12, 2018 at 01:49:13PM -0700, Andy Lutomirski wrote: >> > On Jul 11, 2018, at 4:29 AM, Joerg Roedel wrote: >> >/* Offset from the sysenter stack to tss.sp0 */ >> > -DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, >> > tss.x86_tss.sp0) - >> > +DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, >> > tss.x86_tss.sp1) - >> > offsetofend(struct cpu_entry_area, entry_stack_page.stack)); >> > >> >> The code reads differently. Did you perhaps mean TSS_task_stack? > > Well, the offset name came from TSS_sysenter_sp0, which was the offset > from the sysenter_sp0 (==sysenter-stack) to the task stack in TSS, now > sysenter_sp0 became entry_stack, because its used for all entry points > and not only sysenter. So with the old convention the naming makes still > sense, no? > Trying to parse it certainly makes my brain hurt a bit. This is the offset from the entry stack to sp1, where sp1 is the location of the pointer to the task stack. Maybe all the arithmetic could go in entry_32.S and the asm-offset name could just be TSS_sp1, just like on 64-bit? --Andy
Re: [PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
On Thu, Jul 12, 2018 at 01:49:13PM -0700, Andy Lutomirski wrote: > > On Jul 11, 2018, at 4:29 AM, Joerg Roedel wrote: > >/* Offset from the sysenter stack to tss.sp0 */ > > -DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, > > tss.x86_tss.sp0) - > > +DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, > > tss.x86_tss.sp1) - > > offsetofend(struct cpu_entry_area, entry_stack_page.stack)); > > > > The code reads differently. Did you perhaps mean TSS_task_stack? Well, the offset name came from TSS_sysenter_sp0, which was the offset from the sysenter_sp0 (==sysenter-stack) to the task stack in TSS, now sysenter_sp0 became entry_stack, because its used for all entry points and not only sysenter. So with the old convention the naming makes still sense, no? > Also, the “top of task stack” is a bit weird on 32-bit due to vm86. > Can you document *exactly* what goes in sp1? Will do, thanks for your feedback! Joerg
Re: [PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
> On Jul 11, 2018, at 4:29 AM, Joerg Roedel wrote: > > From: Joerg Roedel > We want x86_tss.sp0 point to the entry stack later to use > it as a trampoline stack for other kernel entry points > besides SYSENTER. Makes sense: sp0 will be the entry stack. But: > > >/* Offset from the sysenter stack to tss.sp0 */ > -DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, tss.x86_tss.sp0) > - > +DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, tss.x86_tss.sp1) > - > offsetofend(struct cpu_entry_area, entry_stack_page.stack)); > The code reads differently. Did you perhaps mean TSS_task_stack? Also, the “top of task stack” is a bit weird on 32-bit due to vm86. Can you document *exactly* what goes in sp1?
[PATCH 03/39] x86/entry/32: Load task stack from x86_tss.sp1 in SYSENTER handler
From: Joerg Roedel We want x86_tss.sp0 point to the entry stack later to use it as a trampoline stack for other kernel entry points besides SYSENTER. So store the task stack pointer in x86_tss.sp1, which is otherwise unused by the hardware, as Linux doesn't make use of Ring 1. Signed-off-by: Joerg Roedel --- arch/x86/kernel/asm-offsets_32.c | 2 +- arch/x86/kernel/process_32.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c index ab2d949..36d77d3 100644 --- a/arch/x86/kernel/asm-offsets_32.c +++ b/arch/x86/kernel/asm-offsets_32.c @@ -47,7 +47,7 @@ void foo(void) BLANK(); /* Offset from the sysenter stack to tss.sp0 */ - DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, tss.x86_tss.sp0) - + DEFINE(TSS_entry_stack, offsetof(struct cpu_entry_area, tss.x86_tss.sp1) - offsetofend(struct cpu_entry_area, entry_stack_page.stack)); #ifdef CONFIG_STACKPROTECTOR diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index 0ae659d..ec62cc7 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -290,6 +290,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) this_cpu_write(cpu_current_top_of_stack, (unsigned long)task_stack_page(next_p) + THREAD_SIZE); + /* SYSENTER reads the task-stack from tss.sp1 */ + this_cpu_write(cpu_tss_rw.x86_tss.sp1, next_p->thread.sp0); /* * Restore %gs if needed (which is common) -- 2.7.4
