Re: [PATCH v6 06/37] kselftest/arm64: Add a test program to exercise the syscall ABI

[Catalin Marinas <catalin.marinas@arm.com>]

On Thu, Dec 09, 2021 at 07:13:00PM +0000, Mark Brown wrote:
> On Thu, Dec 09, 2021 at 05:05:41PM +0000, Catalin Marinas wrote:
> > On Mon, Nov 15, 2021 at 03:28:04PM +0000, Mark Brown wrote:
> 
> > > +// SPDX-License-Identifier: GPL-2.0-only
> 
> > Nitpick: I think GPL-2.0 is sufficient (i.e. no '-only' suffix), though
> > about a quarter seem to use the -only variant.
> 
> Yeah, it's that because it's the default for kernel stuff.  Easier to
> make it restrictive and then relax later?

My point was that IIUC GPL-2.0 is equivalent to GPL-2.0-only (not to be
confused with GPL-2.0+). Anyway, it's fine by me to keep the -only if
you want. It seems that we have nearly the same amount of both
throughout the kernel.

> > > +	/*
> > > +	 * After a syscall the low 128 bits of the Z registers should
> > > +	 * be preserved and the rest be zeroed.
> > > +	 */
> 
> > That's the current behaviour I think but the sve.rst doc states the
> > values after syscall are 'unspecified' (same for the P regs). Should we
> > tighten the doc as well?
> 
> I think so if this goes in as is.  There was some debate at the time
> that SVE was originally merged, with a strong desire from some people to
> make sure that this behaves consistently on every syscall.  I've copied
> in Mark Rutland and Marc Zyngier who I think have opinions here.
> 
> > A downside with forcing zero is that it may prevent us from some
> > optimisations in the future. Currently we do an sve_user_discard() on
> > the syscall entry path and disable SVE but we could instead do this only
> > on context switch or when the kernel used Neon.
> 
> Yes, this is limiting our options for performance work since we need to
> at least take the cost of zeroing the non-shared state on every syscall,
> though there's still options for choosing not to disable SVE all the
> time (I've got a patch already I need to do a bit more work on). 

Some people seem to be pretty sensitive to the syscall latency, so I'd
like to keep the option to optimise this path if it bites us.

> The
> currently documented behaviour is in line with AAPCS here so you do have
> to wonder how likely it is that someone will rely on the zeroing.  On
> the other hand anything like only zeroing the state on context switch
> would mean that it's more likely that userspace bugs with something
> forgetting that the state might be cleared will be intermittent and most
> likely hard to reproduce which will make people miserable.  There's a
> good chance that bugs will be wrong answers rather than something more
> immediate like a fault which really doesn't help there.

If we eventually optimise this path, we could add an option to
force-zero the SVE regs on syscall for debugging purposes. But even
without this, such software may run into problems. By AAPCS, if the
callee doesn't take any SVE arguments, it doesn't need to preserve any
of the registers, though it may choose to. Let's take a gettimeofday
libc call, it may or may not end up in the kernel. When it's handled by
the vDSO, all the SVE regs are preserved but not when doing the syscall.
Something like a futex call would be even less predictable.

In an optimised kernel, the context switch indeed adds to the entropy
but the user can already hit such problems with the current more
consistent behaviour.

-- 
Catalin