linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
To: Willy Tarreau <w@1wt.eu>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Linux Kernel Mailing List <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, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] tools/nolibc: x86: Remove `r8`, `r9` and `r10` from the clobber list
Date: Tue, 12 Oct 2021 15:36:44 +0700	[thread overview]
Message-ID: <20211012083644.543775-1-ammarfaizi2@gmail.com> (raw)
In-Reply-To: <20211012052822.GA28951@1wt.eu>

On Tue, Oct 12, 2021 at 12:28 PM Willy Tarreau <w@1wt.eu> wrote:
>
> Hello Ammar,
>
> First, thanks for your patch. I have a few questions below.
>
> On Mon, Oct 11, 2021 at 11:03:44AM +0700, Ammar Faizi wrote:
> > Linux x86-64 syscall only clobbers rax, rcx and r11 (and "memory").
> >
> >   - rax for the return value.
> >   - rcx to save the return address.
> >   - r11 to save the rflags.
> >
> > Other registers are preserved.
> >
> > Having r8, r9 and r10 in the syscall clobber list is harmless, but this
> > results in a missed-optimization.
> >
> > As the syscall doesn't clobber r8-r10, GCC should be allowed to reuse
> > their value after the syscall returns to userspace. But since they are
> > in the clobber list, GCC will always miss this opportunity.
>
> I agree with your conclusion about this but need to be perfectly sure
> about the exact clobber list since I got a different impression when
> implementing the calls. I got the impression that these ones were
> clobbered by reading entry_64.S below:
>
>  * Registers on entry:
>  * rax  system call number
>  * rcx  return address
>  * r11  saved rflags (note: r11 is callee-clobbered register in C ABI)
>  * rdi  arg0
>  * rsi  arg1
>  * rdx  arg2
>  * r10  arg3 (needs to be moved to rcx to conform to C ABI)
>  * r8   arg4
>  * r9   arg5
>  * (note: r12-r15, rbp, rbx are callee-preserved in C ABI)
>
> See this last note ? Indicating that r12-15, rbp and rbx are preserved
> made me think it's not the case for the other ones, but that might of
> course be a misunderstanding.
>
> And calling.h says this:
>
>  x86 function call convention, 64-bit:
>  -------------------------------------
>   arguments           |  callee-saved      | extra caller-saved | return
>  [callee-clobbered]   |                    | [callee-clobbered] |
>  ---------------------------------------------------------------------------
>  rdi rsi rdx rcx r8-9 | rbx rbp [*] r12-15 | r10-11             | rax, rdx [**]
>
> Note that it's indicated "function call convention", not "syscall",
> leaving it open to have extra restrictions on syscalls. Later by
> reading the POP_REGS macro called with pop_rdi=0 and skipr11rcx=1
> on syscall leave, it seems to restore everything but r11, rcx, rax
> and rdi (which is restored last with rsp).
>
> As such, could you please add in your commit message a link to the
> location where you found that authoritative information above it there
> is a better place (i.e. one that doesn't require to read all the macros)?
> This would significantly help anyone facing the same doubts about this
> in the future.

Hi Willy,

I have tried to search for the documentation about this one, but I
couldn't find any. Checking at `Documentation/x86/entry_64.rst`, but
it doesn't tell anything relevant.

Background story, I browsed the nolibc.h file and found extra clobbers
for Linux x86-64 syscall which I think they are unneccesary (r8, r9
and r10).

This finding had me worried a bit, because I have written syscall in
inline ASM, based on my understanding the required clobbers are "rcx",
"r11" and "memory". So in my mind was "Is my app safe?".

So I opened a discussion on Stack Overflow, yesterday:
https://stackoverflow.com/questions/69515893/when-does-linux-x86-64-syscall-clobber-r8-r9-and-r10

While waiting for answers on Stack Overflow, I also checked "entry_64.S"
and "calling.h". Now I strongly believe syscall only clobbers 3
registers, they are rax, rcx and r11. The answer and comments on SO
support it as well.

These two macros do the job:

	// saves all GPRs and zero some of them
	PUSH_AND_CLEAR_REGS

	// restore all GPRs, except rdi, r11, rcx
	POP_REGS pop_rdi=0 skip_r11rcx=1

	// later rdi is also restored.

My stance comes from SO, Telegram group discussion, and reading source
code. Therefore, I don't think I can attach the link to it as
"authoritative information". Or can I?

When I sent this patch, I also added entry_64.S's maintainers to CC
list. In hope they can help to at least acknowledge it. Mainly because
I can't find the documentation from Linux that tells about this.

Andy, Thomas, Ingo, Borislav, H. Peter.

Could one of you shed some light so that I can attach the link to your
message in the commit message?

Thanks,

-- 
Ammar Faizi

  reply	other threads:[~2021-10-12  8:37 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-11  4:03 [PATCH] tools/nolibc: x86: Remove `r8`, `r9` and `r10` from the clobber list Ammar Faizi
2021-10-12  5:28 ` Willy Tarreau
2021-10-12  8:36   ` Ammar Faizi [this message]
2021-10-12  9:06     ` Willy Tarreau
2021-10-12 20:29       ` Borislav Petkov
2021-10-12 21:51         ` Borislav Petkov
2021-10-12 22:23         ` Ammar Faizi
2021-10-13  3:01           ` Willy Tarreau
2021-10-13  3:32             ` Ammar Faizi
2021-10-13  3:34               ` Ammar Faizi
2021-10-13  3:37                 ` Ammar Faizi
2021-10-13 12:43           ` Borislav Petkov
2021-10-13 12:51             ` Willy Tarreau
2021-10-13 13:06               ` Borislav Petkov
2021-10-13 14:07                 ` Willy Tarreau
2021-10-13 14:20                   ` Borislav Petkov
2021-10-13 14:24                     ` Willy Tarreau
2021-10-13 16:24                       ` Michael Matz
2021-10-13 16:30                         ` Willy Tarreau
2021-10-13 16:51                           ` Andy Lutomirski
2021-10-13 16:52                           ` Borislav Petkov
2021-10-14  8:44                             ` Ammar Faizi
2021-10-14 12:44                             ` Michael Matz
2021-10-14 14:31                               ` Borislav Petkov
2021-10-19  9:06                         ` David Laight
2021-10-23 20:40             ` H. Peter Anvin
2021-10-12 21:21       ` David Laight
2021-10-12 23:02         ` Subject: " Ammar Faizi
2021-10-13  9:03 ` [PATCH v2] " Ammar Faizi
2021-10-15  8:25   ` [PATCH 0/2] Fix clobber list and startup code bug Ammar Faizi
2021-10-15  8:25     ` [PATCH 1/2] tools/nolibc: x86: Remove `r8`, `r9` and `r10` from the clobber list Ammar Faizi
2021-10-18  5:52       ` Willy Tarreau
2021-10-15  8:25     ` [PATCH 2/2] tools/nolibc: x86-64: Fix startup code bug Ammar Faizi
2021-10-15  8:57       ` Ammar Faizi
2021-10-15  9:26         ` Bedirhan KURT
2021-10-15  9:58           ` Ammar Faizi
2021-10-15  9:41         ` Louvian Lyndal
2021-10-18  4:58       ` Willy Tarreau
2021-10-18  6:53         ` Ammar Faizi
2021-10-23 13:27           ` Ammar Faizi
2021-10-23 13:43             ` Willy Tarreau
2021-10-24  2:11               ` [PATCHSET v2 0/2] tools/nolibc: Fix startup code bug and small improvement Ammar Faizi
2021-10-24  2:11                 ` [PATCH 1/2] tools/nolibc: x86-64: Fix startup code bug Ammar Faizi
2021-10-24  2:11                 ` [PATCH 2/2] tools/nolibc: x86-64: Use `mov $60,%eax` instead of `mov $60,%rax` Ammar Faizi
2021-10-24 11:41                 ` [PATCHSET v2 0/2] tools/nolibc: Fix startup code bug and small improvement Willy Tarreau

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=20211012083644.543775-1-ammarfaizi2@gmail.com \
    --to=ammar.faizi@students.amikom.ac.id \
    --cc=aou@eecs.berkeley.edu \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=tglx@linutronix.de \
    --cc=w@1wt.eu \
    --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 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).