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.