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

[Brian Gerst]

On Thu, Oct 26, 2017 at 4:26 AM, Andy Lutomirski <luto@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  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