LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Lai Jiangshan <laijs@linux.alibaba.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	linux-kernel@vger.kernel.org
Cc: Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, Joerg Roedel <jroedel@suse.de>,
	Youquan Song <youquan.song@intel.com>,
	Tony Luck <tony.luck@intel.com>, Juergen Gross <jgross@suse.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>
Subject: Re: [PATCH 17/24] x86/entry: Introduce struct ist_regs
Date: Thu, 9 Sep 2021 21:31:30 -0700	[thread overview]
Message-ID: <9fe0e26b-5fb7-4521-a01e-8edd8d5cb74c@zytor.com> (raw)
In-Reply-To: <eb294b5d-82f2-be80-b3e3-db556c155d95@linux.alibaba.com>

Note: the examples in this email all compiled with:

gcc -O2 -mpreferred-stack-boundary=3 -mcmodel=kernel

The disassembly has been slightly simplified.


On 9/9/21 5:18 PM, Lai Jiangshan wrote:
> 
> This patch was over designed.
> 
> In ASM code, we can easily save results in the callee-saved registers.
> For example, rc3 is saved in %r14, gsbase info is saved in %rbx.
> 
> And in C code, we can't save results in registers.  And I thought there was
> no place to save the results because the CR3 and gsbase are not kernel's.
> So I extended the pt_regs to ist_regs to save the results.
> 
> But it was incorrect.  The results can be saved in percpu data at the 
> end of
> paranoid_entry() after the CR3/gsbase are settled down.  And the results
> can be read at the beginning of paranoid_exit() before the CR3/gsbase are
> switched to the interrupted context's.
> 

OK, count me confused. Of course you can save results in caller-saved 
registers in C; it is kind of what they do:


extern void bar(void);

unsigned long foo(unsigned long v)
{
	bar();
	return v;
}

0000000000000000 <foo>:
    0:   41 54                   push   %r12
    2:   49 89 fc                mov    %rdi,%r12
    5:   e8 00 00 00 00          callq  bar
    a:   4c 89 e0                mov    %r12,%rax
    d:   41 5c                   pop    %r12
    f:   c3                      retq

Now, if you need to specify *which* registers, you have to declare them 
as global register variables - NOT local (which have completely 
different semantics). This also means that you (probably) want to 
isolate this code into its own compilation unit, because it will prevent 
any other code in the same .c file from using that register as well.

For example:

register unsigned long r14 asm("%r14");
unsigned long foo(unsigned long v)
{
	r14 = v;
	bar();
	v = r14;
	return v;
}

0000000000000000 <foo>:
    0:   49 89 fe                mov    %rdi,%r14
    3:   e8 00 00 00 00          callq  bar
    8:   4c 89 f0                mov    %r14,%rax
    b:   c3                      retq

WARNING: This also means that gcc will happily discard the old value in 
%r14, so if you need it preserved you have to do so explicitly; if you 
are called direct from assembly and are happy to lose the value then the 
above code is fine -- and it is even slightly more efficient!

For preserving the old r14, in this case:

register unsigned long r14 asm("%r14");
unsigned long foo(unsigned long v)
{
	unsigned long saved_r14 = r14;
	r14 = v;
	bar();
	v = r14;
	r14 = saved_r14;
	return v;
}

0000000000000000 <foo>:
    0:   53                      push   %rbx
    1:   4c 89 f3                mov    %r14,%rbx
    4:   49 89 fe                mov    %rdi,%r14
    7:   e8 00 00 00 00          callq  bar
    c:   4c 89 f0                mov    %r14,%rax
    f:   49 89 de                mov    %rbx,%r14
   12:   5b                      pop    %rbx
   13:   c3                      retq


HOWEVER, if you are relying on not using the stack, then using C code is 
probably very much not a good idea. It is very hard to guarantee that 
just because the C compiler is *currently* not using a stack, that it 
won't do so *in the future*.

Again, finally, local register variables DO NOT WORK, this does NOT do 
what you expect:

unsigned long foo(unsigned long v)
{
	register unsigned long r14 asm("%r14") = v;
	bar();
	return r14;
}

0000000000000000 <foo>:
    0:   41 54                   push   %r12
    2:   49 89 fc                mov    %rdi,%r12
    5:   e8 00 00 00 00          callq  bar
    a:   4c 89 e0                mov    %r12,%rax
    d:   41 5c                   pop    %r12
    f:   c3                      retq




  parent reply	other threads:[~2021-09-10  4:32 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31 17:50 [PATCH 00/24] x86/entry/64: Convert a bunch of ASM entry code into C code Lai Jiangshan
2021-08-31 17:50 ` [PATCH 01/24] x86/traps: Remove stack-protector from traps.c Lai Jiangshan
2021-08-31 17:50 ` [PATCH 02/24] x86/traps: Move arch/x86/kernel/traps.c to arch/x86/entry/ Lai Jiangshan
2021-09-02  8:09   ` Joerg Roedel
2021-09-02  9:21     ` Lai Jiangshan
2021-09-02 10:50       ` Peter Zijlstra
2021-09-02 11:54         ` Lai Jiangshan
2021-09-02 12:05           ` Peter Zijlstra
2021-09-02 13:34             ` Peter Zijlstra
2021-09-02 17:05               ` Nick Desaulniers
2021-09-02 17:19                 ` Miguel Ojeda
2021-09-02 17:23                   ` Nick Desaulniers
2021-09-03  7:36                 ` Martin Liška
2021-09-07 21:12                   ` Nick Desaulniers
2021-09-08  7:33                     ` Martin Liška
2021-08-31 17:50 ` [PATCH 03/24] x86/traps: Move declaration of native_irq_return_iret up Lai Jiangshan
2021-08-31 17:50 ` [PATCH 04/24] x86/entry: Expose the address of .Lgs_change to traps.c Lai Jiangshan
2021-09-02  9:14   ` Peter Zijlstra
2021-09-02  9:20     ` Lai Jiangshan
2021-08-31 17:50 ` [PATCH 05/24] x86/entry: Introduce __entry_text for entry code written in C Lai Jiangshan
2021-08-31 19:34   ` Peter Zijlstra
2021-09-01  0:23     ` Lai Jiangshan
2021-08-31 17:50 ` [PATCH 06/24] x86/entry: Move PTI_USER_* to arch/x86/include/asm/processor-flags.h Lai Jiangshan
2021-08-31 17:50 ` [PATCH 07/24] x86: Mark __native_read_cr3() & native_write_cr3() as __always_inline Lai Jiangshan
2021-08-31 17:50 ` [PATCH 08/24] x86/traps: Add C verion of SWITCH_TO_KERNEL_CR3 as switch_to_kernel_cr3() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 09/24] x86/traps: Add fence_swapgs_{user,kernel}_entry() Lai Jiangshan
2021-09-02  9:25   ` Peter Zijlstra
2021-08-31 17:50 ` [PATCH 10/24] x86/traps: Move pt_regs only in fixup_bad_iret() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 11/24] x86/entry: Replace the most of asm code of error_entry to C code Lai Jiangshan
2021-09-02 10:16   ` Peter Zijlstra
2021-09-02 12:08     ` Lai Jiangshan
2021-08-31 17:50 ` [PATCH 12/24] x86/traps: Reconstruct pt_regs on task stack directly in fixup_bad_iret() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 13/24] x86/traps: Mark sync_regs() and fixup_bad_iret() as static __always_inline Lai Jiangshan
2021-08-31 17:50 ` [PATCH 14/24] x86/entry: Make paranoid_exit() callable Lai Jiangshan
2021-08-31 17:50 ` [PATCH 15/24] x86/entry: Call paranoid_exit() in asm_exc_nmi() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 16/24] x86/entry: Use skip_rdi instead of save_ret for PUSH_AND_CLEAR_REGS Lai Jiangshan
2021-08-31 17:50 ` [PATCH 17/24] x86/entry: Introduce struct ist_regs Lai Jiangshan
2021-09-10  0:18   ` Lai Jiangshan
2021-09-10  0:36     ` Lai Jiangshan
2021-09-10  4:31     ` H. Peter Anvin [this message]
2021-09-10  7:13       ` Lai Jiangshan
2021-09-10  7:14         ` H. Peter Anvin
2021-09-10  4:50     ` H. Peter Anvin
2021-09-10  4:51       ` H. Peter Anvin
2021-08-31 17:50 ` [PATCH 18/24] x86/entry: Add the C version ist_switch_to_kernel_cr3() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 19/24] x86/entry: Add the C version ist_restore_cr3() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 20/24] x86/entry: Add the C version get_percpu_base() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 21/24] x86/entry: Add the C version ist_switch_to_kernel_gsbase() Lai Jiangshan
2021-08-31 17:50 ` [PATCH 22/24] x86/entry: Implement and use do_paranoid_entry() and paranoid_exit() Lai Jiangshan
2021-09-02 10:33   ` Peter Zijlstra
2021-09-02 10:42     ` Lai Jiangshan
2021-09-02 12:02       ` Peter Zijlstra
2021-09-02 11:58     ` Lai Jiangshan
2021-09-02 12:29       ` Joerg Roedel
2021-08-31 17:50 ` [PATCH 23/24] x86/entry: Remove the unused ASM macros Lai Jiangshan
2021-08-31 17:50 ` [PATCH 24/24] x86/syscall/64: Move the checking for sysret to C code Lai Jiangshan
2021-09-10  7:20   ` Nikolay Borisov
2021-09-10  7:30     ` Lai Jiangshan
2021-08-31 20:44 ` [PATCH 00/24] x86/entry/64: Convert a bunch of ASM entry code into " Peter Zijlstra
2021-09-02  6:28   ` Lai Jiangshan
2021-09-02  7:44     ` Peter Zijlstra
2021-09-02 10:50 ` [PATCH 25/24] x86/traps: Rewrite native_load_gs_index in " Lai Jiangshan
2021-09-08  1:38   ` H. Peter Anvin
2021-09-08  4:42     ` H. Peter Anvin
2021-09-08  5:00       ` H. Peter Anvin
2021-09-08  7:12         ` Lai Jiangshan
2021-09-09 23:16           ` H. Peter Anvin
2021-09-13 20:01   ` Andy Lutomirski
2021-09-14  2:04     ` Lai Jiangshan
2021-09-14  8:14       ` Peter Zijlstra
2021-09-14  8:17         ` Borislav Petkov
2021-09-14  8:40         ` Lai Jiangshan

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=9fe0e26b-5fb7-4521-a01e-8edd8d5cb74c@zytor.com \
    --to=hpa@zytor.com \
    --cc=bp@alien8.de \
    --cc=jgross@suse.com \
    --cc=jiangshanlai@gmail.com \
    --cc=jroedel@suse.de \
    --cc=laijs@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    --cc=youquan.song@intel.com \
    --subject='Re: [PATCH 17/24] x86/entry: Introduce struct ist_regs' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).