Re: [PATCH 17/18] x86/asm/64: Remove thread_struct::sp0

Brian Gerst
2017-10-28T00:53:02Z

On Thu, Oct 26, 2017 at 4:26 AM, Andy Lutomirski <luto@kernel.org> wrote:
> On x86_64, we can easily calculate sp0 when needed instead of
> storing it in thread_struct.
>
> On x86_32, a similar cleanup would be possible, but it would require
> cleaning up the vm86 code first, and that can wait for a later
> cleanup series.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> arch/x86/include/asm/compat.h | 1 +
> arch/x86/include/asm/processor.h | 33 +++++++++++++++------------------
> arch/x86/include/asm/switch_to.h | 6 ++++++
> arch/x86/kernel/process_64.c | 1 -
> 4 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
> index 5343c19814b3..948b6d8ec46f 100644
> --- a/arch/x86/include/asm/compat.h
> +++ b/arch/x86/include/asm/compat.h
> @@ -6,6 +6,7 @@
> */
> #include <linux/types.h>
> #include <linux/sched.h>
> +#include <linux/sched/task_stack.h>
> #include <asm/processor.h>
> #include <asm/user32.h>
> #include <asm/unistd.h>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index ad59cec14239..562c575d8bc3 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -430,7 +430,9 @@ typedef struct {
> struct thread_struct {
> /* Cached TLS descriptors: */
> struct desc_struct tls_array[GDT_ENTRY_TLS_ENTRIES];
> +#ifdef CONFIG_X86_32
> unsigned long sp0;
> +#endif
> unsigned long sp;
> #ifdef CONFIG_X86_32
> unsigned long sysenter_cs;
> @@ -797,6 +799,13 @@ static inline void spin_lock_prefetch(const void *x)
>
> #define task_top_of_stack(task) ((unsigned long)(task_pt_regs(task) + 1))
>
> +#define task_pt_regs(task) \
> +({ \
> + unsigned long __ptr = (unsigned long)task_stack_page(task); \
> + __ptr += THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING; \
> + ((struct pt_regs *)__ptr) - 1; \
> +})
> +
> #ifdef CONFIG_X86_32
> /*
> * User space process size: 3GB (default).
> @@ -816,23 +825,6 @@ static inline void spin_lock_prefetch(const void *x)
> .addr_limit = KERNEL_DS, \
> }
>
> -/*
> - * TOP_OF_KERNEL_STACK_PADDING reserves 8 bytes on top of the ring0 stack.
> - * This is necessary to guarantee that the entire "struct pt_regs"
> - * is accessible even if the CPU haven't stored the SS/ESP registers
> - * on the stack (interrupt gate does not save these registers
> - * when switching to the same priv ring).
> - * Therefore beware: accessing the ss/esp fields of the
> - * "struct pt_regs" is possible, but they may contain the
> - * completely wrong values.
> - */
> -#define task_pt_regs(task) \
> -({ \
> - unsigned long __ptr = (unsigned long)task_stack_page(task); \
> - __ptr += THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING; \
> - ((struct pt_regs *)__ptr) - 1; \
> -})
> -
> #define KSTK_ESP(task) (task_pt_regs(task)->sp)
>
> #else
> @@ -865,12 +857,17 @@ static inline void spin_lock_prefetch(const void *x)
> #define STACK_TOP TASK_SIZE_LOW
> #define STACK_TOP_MAX TASK_SIZE_MAX
>
> +#ifdef CONFIG_X86_32
> #define INIT_THREAD { \
> .sp0 = TOP_OF_INIT_STACK, \
> .addr_limit = KERNEL_DS, \
> }
> +#else
> +#define INIT_THREAD { \
> + .addr_limit = KERNEL_DS, \
> +}
> +#endif

There is already a separate INIT_THREAD for 32-bit. Just delete the sp0 member.

--
Brian Gerst