All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Alexei Starovoitov <ast@plumgrid.com>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] x86: change THREAD_INFO definition to not depend on KERNEL_STACK_OFFSET
Date: Tue, 24 Mar 2015 19:09:04 +0100	[thread overview]
Message-ID: <20150324180904.GA15333@gmail.com> (raw)
In-Reply-To: <1426785469-15125-1-git-send-email-dvlasenk@redhat.com>


* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> This changes THREAD_INFO definition and all its callsites
> so that they do not count stack position from
> (top of stack - KERNEL_STACK_OFFSET), but from top of stack.
> 
> Semi-mysterious expressions THREAD_INFO(%rsp,RIP) - "why RIP??"
> are now replaced by more logical THREAD_INFO(%rsp,SIZEOF_PTREGS) -
> "calculate thread_info's address using information that
> rsp is SIZEOF_PTREGS bytes below top of stack".
> 
> While at it, replace "(off)-THREAD_SIZE(reg)" with equivalent
> "((off)-THREAD_SIZE)(reg)". The form without parentheses
> falsely looks like we invoke THREAD_SIZE() macro.
> 
> Improve comment atop THREAD_INFO macro definition.
> 
> This patch does not change generated code (verified by objdump).

> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -207,10 +207,12 @@ static inline unsigned long current_stack_pointer(void)
>  	_ASM_SUB $(THREAD_SIZE-KERNEL_STACK_OFFSET),reg ;
>  
>  /*
> - * Same if PER_CPU_VAR(kernel_stack) is, perhaps with some offset, already in
> - * a certain register (to be used in assembler memory operands).
> + * ASM operand which evaluates to thread_info address
> + * if it is known that "reg" is exactly "off" bytes below stack top.
> + * Example (fetch thread_info->fieldname):
> + *  mov TI_fieldname+THREAD_INFO(reg, off),%eax
>   */
> -#define THREAD_INFO(reg, off) KERNEL_STACK_OFFSET+(off)-THREAD_SIZE(reg)
> +#define THREAD_INFO(reg, off) ((off)-THREAD_SIZE)(reg)

We need more assembly hackers, so I have improved this still somewhat 
cryptic comment to:

/*
 * ASM operand which evaluates to a 'thread_info' address of
 * the current task, if it is known that "reg" is exactly "off"
 * bytes below the top of the stack currently.
 *
 * ( The kernel stack's size is known at build time, it is usually
 *   2 or 4 pages, and the bottom  of the kernel stack contains
 *   the thread_info structure. So to access the thread_info very
 *   quickly from assembly code we can calculate down from the
 *   top of the kernel stack to the bottom, using constant,
 *   build-time calculations only. )
 *
 * For example, to fetch the current thread_info->flags value into %eax
 * on x86-64 defconfig kernels:
 *
 *      mov TI_flags+THREAD_INFO(%rsp, SIZEOF_PTREGS), %eax
 *
 * will translate to:
 *
 *      8b 84 24 b8 c0 ff ff      mov    -0x3f48(%rsp), %eax
 *
 * which is below the current RSP by almost 16K.
 */
#define THREAD_INFO(reg, off) ((off)-THREAD_SIZE)(reg)

Agreed?

Thanks,

	Ingo

  parent reply	other threads:[~2015-03-24 18:09 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-19 17:17 [PATCH 1/5] x86: change THREAD_INFO definition to not depend on KERNEL_STACK_OFFSET Denys Vlasenko
2015-03-19 17:17 ` [PATCH 2/5] x86: get rid of KERNEL_STACK_OFFSET Denys Vlasenko
2015-03-20 16:21   ` Borislav Petkov
2015-03-25  9:10   ` [tip:x86/asm] x86/asm/entry: Get " tip-bot for Denys Vlasenko
2015-03-19 17:17 ` [PATCH 3/5] x86/entry_64.S: use PUSH insns to build pt_regs on stack Denys Vlasenko
2015-03-20 16:35   ` Borislav Petkov
2015-03-25  9:11   ` [tip:x86/asm] x86/asm/entry/64: Use PUSH instructions " tip-bot for Denys Vlasenko
2015-03-19 17:17 ` [PATCH 4/5] x86/entry_64.S: get rid of FIXUP_TOP_OF_STACK/RESTORE_TOP_OF_STACK Denys Vlasenko
2015-03-20 16:38   ` Borislav Petkov
2015-03-25  9:11   ` [tip:x86/asm] x86/asm/entry/64: Get rid of the FIXUP_TOP_OF_STACK /RESTORE_TOP_OF_STACK macros tip-bot for Denys Vlasenko
2015-03-19 17:17 ` [PATCH 5/5] x86/entry_64.S: get rid of int_ret_from_sys_call_fixup Denys Vlasenko
2015-03-20 16:39   ` Borislav Petkov
2015-03-25  9:11   ` [tip:x86/asm] x86/asm/entry/64: Get " tip-bot for Denys Vlasenko
2015-03-20 10:30 ` [PATCH 1/5] x86: change THREAD_INFO definition to not depend on KERNEL_STACK_OFFSET Borislav Petkov
2015-03-20 22:27 ` Andy Lutomirski
2015-03-24 18:09 ` Ingo Molnar [this message]
2015-03-24 18:43 ` [PATCH] x86/asm/entry/64: Improve the THREAD_INFO() macro explanation Ingo Molnar
2015-03-24 18:50   ` Denys Vlasenko
2015-03-24 19:07     ` Andy Lutomirski
2015-03-24 19:20   ` Borislav Petkov
2015-03-25  9:12   ` [tip:x86/asm] " tip-bot for Ingo Molnar
2015-03-24 18:44 ` [PATCH] x86/asm/entry/64: Merge the field offset into the THREAD_INFO() macro Ingo Molnar
2015-03-24 18:50   ` Denys Vlasenko
2015-03-24 19:29     ` Ingo Molnar
2015-03-24 19:34       ` Denys Vlasenko
2015-03-24 19:08   ` Andy Lutomirski
2015-03-25  9:12   ` [tip:x86/asm] " tip-bot for Ingo Molnar
2015-03-24 18:44 ` [PATCH] x86/asm/entry/64: Rename THREAD_INFO() to ASM_ASM_THREAD_INFO_MEMOP() Ingo Molnar
2015-03-24 19:24   ` Borislav Petkov
2015-03-24 19:34     ` Ingo Molnar
2015-03-25  9:13   ` [tip:x86/asm] x86/asm/entry/64: Rename THREAD_INFO() to ASM_THREAD_INFO() tip-bot for Ingo Molnar
2015-03-25  9:10 ` [tip:x86/asm] x86/asm/entry/64: Change the THREAD_INFO() definition to not depend on KERNEL_STACK_OFFSET tip-bot for Denys Vlasenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150324180904.GA15333@gmail.com \
    --to=mingo@kernel.org \
    --cc=ast@plumgrid.com \
    --cc=bp@alien8.de \
    --cc=dvlasenk@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=oleg@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=wad@chromium.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.