linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	Kees Cook <keescook@chromium.org>, Borislav Petkov <bp@alien8.de>,
	X86 ML <x86@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	Alexei Starovoitov <ast@plumgrid.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Will Drewry <wad@chromium.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v4 RESEND] x86/asm/entry/32, selftests: Add 'test_syscall_vdso' test
Date: Tue, 15 Sep 2015 11:14:36 -0700	[thread overview]
Message-ID: <CALCETrXgddhM7FTiD2Dih9fgfvU3Wb908GbOOwJOarHSwa-yLg@mail.gmail.com> (raw)
In-Reply-To: <20150915055958.GA16947@gmail.com>

On Sep 14, 2015 11:00 PM, "Ingo Molnar" <mingo@kernel.org> wrote:
>
>
> * Andy Lutomirski <luto@amacapital.net> wrote:
>
> > On Sep 14, 2015 1:15 AM, "Ingo Molnar" <mingo@kernel.org> wrote:
> > >
> > >
> > > * Denys Vlasenko <dvlasenk@redhat.com> wrote:
> > >
> > > > >> +                       /* INT80 syscall entrypoint can be used by
> > > > >> +                        * 64-bit programs too, unlike SYSCALL/SYSENTER.
> > > > >> +                        * Therefore it must preserve R12+
> > > > >> +                        * (they are callee-saved registers in 64-bit C ABI).
> > > > >> +                        *
> > > > >> +                        * This was probably historically not intended,
> > > > >> +                        * but R8..11 are clobbered (cleared to 0).
> > > > >> +                        * IOW: they are the only registers which aren't
> > > > >> +                        * preserved across INT80 syscall.
> > > > >> +                        */
> > > > >> +                       if (*r64 == 0 && num <= 11)
> > > > >> +                               continue;
> > > > >
> > > > > Ugh.  I'll change my big entry patchset to preserve these and maybe to
> > > > > preserve all of the 64-bit regs.
> > > >
> > > > If you do that, this won't change the ABI: we don't _promise_
> > > > to save them. If we accidentally do, that means nothing.
> > >
> > > Argh, that's dangerous nonsense! You _still_ don't seem to understand what the
> > > Linux ABI means and how to change code that implements it...
> >
> > I think Denys might be taking about R8-R11 here.  If we change them
> > from clobbered to saved, that's probably fine.  Certainly I have to
> > save R12-R15 -- my v1 is just buggy there.  I was too deep in
> > __kernel_vsyscall when I wrote that code, and I wasn't thinking about
> > the raw int $0x80 entry variant.
> >
> > I'd be rather surprised if anything broke if we started preserving
> > R8-R11 instead of zeroing them.
>
> Well, read the statement:
>
>   " If you do that, this won't change the ABI: we don't _promise_
>     to save them. If we accidentally do, that means nothing. "
>
> of _course_ it means everything: if we preserve R8-R11 and any app learns to rely
> on it, it becomes an ABI.

Right, it changes the ABI in a way that we can't undo, but it probably
doesn't break the old ABI.

Certainly for v2, I'll try to preserve the old behavior exactly.  If
we change it later to preserve all high regs, that'll be a separate
patch.

--Andy

  reply	other threads:[~2015-09-15 18:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-07 15:56 [PATCH v4 RESEND] x86/asm/entry/32, selftests: Add 'test_syscall_vdso' test Denys Vlasenko
2015-09-09 22:01 ` Andy Lutomirski
2015-09-10 19:04   ` Denys Vlasenko
2015-09-10 19:17     ` Andy Lutomirski
2015-09-14  8:26       ` Ingo Molnar
2015-09-14 17:54         ` Andy Lutomirski
2015-09-14  8:15     ` Ingo Molnar
2015-09-14 17:55       ` Andy Lutomirski
2015-09-15  5:59         ` Ingo Molnar
2015-09-15 18:14           ` Andy Lutomirski [this message]
2015-09-16  9:51             ` Ingo Molnar
2015-09-15  6:05 ` Ingo Molnar

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=CALCETrXgddhM7FTiD2Dih9fgfvU3Wb908GbOOwJOarHSwa-yLg@mail.gmail.com \
    --to=luto@amacapital.net \
    --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=mingo@kernel.org \
    --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 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).