| Message ID | 20240122-arm64-sve-sme-doc-v1-1-3d492e45265b@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/fp: Documentation cleanups and clarifications | expand |
On Mon, Jan 22, 2024 at 08:41:51PM +0000, Mark Brown wrote: > When we documented that we always clear state not shared with FPSIMD we Where / when? > didn't catch all of the places that mentioned that state might not be > cleared, remove a lingering reference. > > Reported-by: Edmund Grimley-Evans <edmund.grimley-evans@arm.com> > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > Documentation/arch/arm64/sve.rst | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/Documentation/arch/arm64/sve.rst b/Documentation/arch/arm64/sve.rst > index 0d9a426e9f85..b45a2da19bf1 100644 > --- a/Documentation/arch/arm64/sve.rst > +++ b/Documentation/arch/arm64/sve.rst > @@ -117,11 +117,6 @@ the SVE instruction set architecture. > * The SVE registers are not used to pass arguments to or receive results from > any syscall. > > -* In practice the affected registers/bits will be preserved or will be replaced > - with zeros on return from a syscall, but userspace should not make > - assumptions about this. The kernel behaviour may vary on a case-by-case > - basis. > - This was originally an intentionally conservative statement, to allow the kernel the flexibility to relax the register zeroing behaviour in the future. It would have permitted not always disabling a task's SVE across a syscall, for example. There were some concerns about security and testability that meant that we didn't use this flexibility to begin with. If we are making an irrevocable commitment not to use this flexibility ever, then this comment can go, but if we're not totally sure then I think it would be harmless to keep it (?) (Feel free to point me to the relevant past discussion that I may have missed.) [...] Cheers ---Dave
On Tue, Jan 23, 2024 at 03:44:23PM +0000, Dave Martin wrote: > On Mon, Jan 22, 2024 at 08:41:51PM +0000, Mark Brown wrote: > > When we documented that we always clear state not shared with FPSIMD we > Where / when? In the document that is being modified when it was written. > > -* In practice the affected registers/bits will be preserved or will be replaced > > - with zeros on return from a syscall, but userspace should not make > > - assumptions about this. The kernel behaviour may vary on a case-by-case > > - basis. > This was originally an intentionally conservative statement, to allow > the kernel the flexibility to relax the register zeroing behaviour in > the future. It would have permitted not always disabling a task's SVE > across a syscall, for example. There were some concerns about security > and testability that meant that we didn't use this flexibility to begin > with. > If we are making an irrevocable commitment not to use this flexibility > ever, then this comment can go, but if we're not totally sure then I > think it would be harmless to keep it (?) I think everyone except for Catalin had felt that the original discussion had concluded that there was a commitment to always clear the non-shared bits and was disappointed to learn that the documentation said otherwise. When I tried to take advantage of this as part of optimising the system call overhead for SVE there were eventually complaints. > (Feel free to point me to the relevant past discussion that I may have > missed.) See the discussion on my syscall optimisation series: https://lore.kernel.org/all/20220620124158.482039-8-broonie@kernel.org/
On Tue, Jan 23, 2024 at 05:31:58PM +0000, Mark Brown wrote: > On Tue, Jan 23, 2024 at 03:44:23PM +0000, Dave Martin wrote: > > On Mon, Jan 22, 2024 at 08:41:51PM +0000, Mark Brown wrote: > > > When we documented that we always clear state not shared with FPSIMD we > > > Where / when? > > In the document that is being modified when it was written. Ah, right, I see this: d09ee410a3c3 ("arm64/sve: Document our actual ABI for clearing registers on syscall") where the zeroing is made explicit. > > > > -* In practice the affected registers/bits will be preserved or will be replaced > > > - with zeros on return from a syscall, but userspace should not make > > > - assumptions about this. The kernel behaviour may vary on a case-by-case > > > - basis. > > > This was originally an intentionally conservative statement, to allow > > the kernel the flexibility to relax the register zeroing behaviour in > > the future. It would have permitted not always disabling a task's SVE > > across a syscall, for example. There were some concerns about security > > and testability that meant that we didn't use this flexibility to begin > > with. > > > If we are making an irrevocable commitment not to use this flexibility > > ever, then this comment can go, but if we're not totally sure then I > > think it would be harmless to keep it (?) > > I think everyone except for Catalin had felt that the original > discussion had concluded that there was a commitment to always clear the > non-shared bits and was disappointed to learn that the documentation > said otherwise. When I tried to take advantage of this as part of > optimising the system call overhead for SVE there were eventually > complaints. > > > (Feel free to point me to the relevant past discussion that I may have > > missed.) > > See the discussion on my syscall optimisation series: > > https://lore.kernel.org/all/20220620124158.482039-8-broonie@kernel.org/ I think my excuse would be that this was consciously left unresolved when SVE originally went upstream: the kernel played safe by always zeroing the bits, while userspace was told not to rely on this always happening in future. If the decision has effectively now been made to close the door permanently those optimisations, then I guess it makes sense to clean up the documentation to be as consistent as possible. I still feel that it is iffy practice for userspace to rely on the extra bits being zeroed -- I think the architecture hides this guarantee anyway whenever you go through a function call confirming to the regular procedure call standard (including the syscall wrappers). But there may not be a lot of point trying to put people off if we can't force them not to rely on it. Cheers ---Dave
On Tue, Jan 23, 2024 at 05:54:17PM +0000, Dave Martin wrote: > I still feel that it is iffy practice for userspace to rely on the > extra bits being zeroed -- I think the architecture hides this > guarantee anyway whenever you go through a function call confirming to > the regular procedure call standard (including the syscall wrappers). > But there may not be a lot of point trying to put people off if we > can't force them not to rely on it. I do tend to agree that the requirement to zero is excessively zealous and that the risk from relaxing it is minor (it's stricter than the function call ABI), I did leave a sysctl as a mechanism for restoring compatibility in the case where we did run into issues in my original series but I didn't expect to need it. If you convince everyone else I'd be happy to relax things but I don't super care either way.
On Tue, Jan 23, 2024 at 06:11:52PM +0000, Mark Brown wrote: > On Tue, Jan 23, 2024 at 05:54:17PM +0000, Dave Martin wrote: > > > I still feel that it is iffy practice for userspace to rely on the > > extra bits being zeroed -- I think the architecture hides this > > guarantee anyway whenever you go through a function call confirming to > > the regular procedure call standard (including the syscall wrappers). > > But there may not be a lot of point trying to put people off if we > > can't force them not to rely on it. > > I do tend to agree that the requirement to zero is excessively zealous > and that the risk from relaxing it is minor (it's stricter than the > function call ABI), I did leave a sysctl as a mechanism for restoring > compatibility in the case where we did run into issues in my original > series but I didn't expect to need it. If you convince everyone else > I'd be happy to relax things but I don't super care either way. [...] I don't feel that strongly about it. Ideally we'd have gone for the fully relaxed approach from the start, but it's hard to test whether "unspecified" registers aren't leaking data from somewhere they shouldn't. Given that the decision has been made anyway, the documentation should not send mixed messages, so: Reviewed-by: Dave Martin <Dave.Martin@arm.com>
diff --git a/Documentation/arch/arm64/sve.rst b/Documentation/arch/arm64/sve.rst index 0d9a426e9f85..b45a2da19bf1 100644 --- a/Documentation/arch/arm64/sve.rst +++ b/Documentation/arch/arm64/sve.rst @@ -117,11 +117,6 @@ the SVE instruction set architecture. * The SVE registers are not used to pass arguments to or receive results from any syscall. -* In practice the affected registers/bits will be preserved or will be replaced - with zeros on return from a syscall, but userspace should not make - assumptions about this. The kernel behaviour may vary on a case-by-case - basis. - * All other SVE state of a thread, including the currently configured vector length, the state of the PR_SVE_VL_INHERIT flag, and the deferred vector length (if any), is preserved across all syscalls, subject to the specific
When we documented that we always clear state not shared with FPSIMD we didn't catch all of the places that mentioned that state might not be cleared, remove a lingering reference. Reported-by: Edmund Grimley-Evans <edmund.grimley-evans@arm.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- Documentation/arch/arm64/sve.rst | 5 ----- 1 file changed, 5 deletions(-)