RE: [PATCH] tools/nolibc: x86: Remove `r8`, `r9` and `r10` from the clobber list

From: David Laight <David.Laight@ACULAB.COM>
To: 'Michael Matz' <matz@suse.de>, Willy Tarreau <w@1wt.eu>
Cc: Borislav Petkov <bp@alien8.de>,
	Ammar Faizi <ammar.faizi@students.amikom.ac.id>,
	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>,
	Andy Lutomirski <luto@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "x86@kernel.org" <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, 19 Oct 2021 09:06:40 +0000	[thread overview]
Message-ID: <01650203956e4f13adf1feed85fc36a3@AcuMS.aculab.com> (raw)
In-Reply-To: <alpine.LSU.2.20.2110131601000.26294@wotan.suse.de>

From: Michael Matz
> Sent: 13 October 2021 17:24
> 
> Hello,
> 
> On Wed, 13 Oct 2021, Willy Tarreau wrote:
> 
> > On Wed, Oct 13, 2021 at 04:20:55PM +0200, Borislav Petkov wrote:
> > > On Wed, Oct 13, 2021 at 04:07:23PM +0200, Willy Tarreau wrote:
> > > > Yes I agree with the "potentially" here. If it can potentially be (i.e.
> > > > the kernel is allowed by contract to later change the way it's currently
> > > > done) then we have to save them even if it means lower code efficiency.
> > > >
> > > > If, however, the kernel performs such savings on purpose because it is
> > > > willing to observe a stricter saving than the AMD64 ABI, we can follow
> > > > it but only once it's written down somewhere that it is by contract and
> > > > will not change.
> > >
> > > Right, and Micha noted that such a change to the document can be done.
> >
> > great.
> >
> > > And we're basically doing that registers restoring anyway, in POP_REGS.
> >
> > That's what I based my analysis on when I wanted to verify Ammar's
> > finding. I would tend to think that if we're burning cycles popping
> > plenty of registers it's probably for a reason, maybe at least a good
> > one, which is that it's the only way to make sure we're not leaking
> > internal kernel data! This is not a concern for kernel->kernel nor
> > user->user calls but for user->kernel calls it definitely is one, and
> > I don't think we could relax that series of pop without causing leaks
> > anyway.
> 
> It might also be interesting to know that while the wording of the psABI
> was indeed intended to imply that all argument registers are potentially
> clobbered (like with normal calls) glibc's inline assembler to call
> syscalls relies on most registers to actually be preserved:
> 
> # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
> ...
> #define internal_syscall6(number, arg1, arg2, arg3, arg4, arg5, arg6) \
> ({                                                                      \
>     unsigned long int resultvar;                                        \
>     TYPEFY (arg6, __arg6) = ARGIFY (arg6);                              \
>     TYPEFY (arg5, __arg5) = ARGIFY (arg5);                              \
>     TYPEFY (arg4, __arg4) = ARGIFY (arg4);                              \
>     TYPEFY (arg3, __arg3) = ARGIFY (arg3);                              \
>     TYPEFY (arg2, __arg2) = ARGIFY (arg2);                              \
>     TYPEFY (arg1, __arg1) = ARGIFY (arg1);                              \
>     register TYPEFY (arg6, _a6) asm ("r9") = __arg6;                    \
>     register TYPEFY (arg5, _a5) asm ("r8") = __arg5;                    \
>     register TYPEFY (arg4, _a4) asm ("r10") = __arg4;                   \
>     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                   \
>     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                   \
>     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                   \
>     asm volatile (                                                      \
>     "syscall\n\t"                                                       \
>     : "=a" (resultvar)                                                  \
>     : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4),         \
>       "r" (_a5), "r" (_a6)                                              \
>     : "memory", REGISTERS_CLOBBERED_BY_SYSCALL);                        \
>     (long int) resultvar;                                               \
> })
> 
> 
> Note in particular the missing clobbers or outputs of any of the argument
> regs.

What about all the AVX registers?
These are normally caller-saved - so are unlikely to be live in a gcc stub.
But glibc is unlikely to keep the clobber list up do date - even if they
were ever added.

While the kernel can't return 'junk' in the AVX registers, it may be
significantly cheaper to zero the registers on at least some code paths.

The same is true for the rxx registers.
Executing 'xor %rxx,%rxx' is faster than 'pop $rxx'.
Especially since the xor cam all be execute in parallel.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)