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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3163 bytes --]

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?

> > +	/*
> > +	 * 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).  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.

> Of course, we also run the risk of the current behavior (zeroing)
> becoming the de-facto ABI.

All current kernels do the zeroing so given that SVE hardware is
starting to roll out widely and that usage is likely to be with kernels
that are already available, some people will doubtless backport any
performance work that changes the behaviour but it's never going to be
everyone.  At the very least we I think we need to decide soon if we
really do want the consistent behaviour we currently have and update the
ABI document if we do, if not there's more options we can include in
performance work.

Deciding that the current behaviour is in fact our ABI is definitely the
safest thing and the path of least resistance, though it's not ideal
from a performance point of view.  My sense is that we would probably be
fine changing things but there are real risks and my sense is that
there's going to be substantial pushback based on those which will
result in us sticking with the current behaviour.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]