On Mon, Nov 28, 2016 at 12:06:24PM +0000, Catalin Marinas wrote:

On Mon, Nov 28, 2016 at 11:47:26AM +0000, Dave P Martin wrote:

quoted

On Sat, Nov 26, 2016 at 11:30:42AM +0000, Catalin Marinas wrote:

quoted

On Fri, Nov 25, 2016 at 08:45:02PM +0000, Ard Biesheuvel wrote:

quoted

On 25 November 2016 at 19:39, Dave Martin [off-list ref] wrote:

quoted

--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c

@@ -282,11 +282,26 @@ static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate);
  */
 void kernel_neon_begin_partial(u32 num_regs)
 {
+       preempt_disable();
+
+       /*
+        * For now, we have no special storage for SVE registers in
+        * interrupt context, so always save the userland SVE state
+        * if there is any, even for interrupts.
+        */
+       if (IS_ENABLED(CONFIG_ARM64_SVE) && (elf_hwcap & HWCAP_SVE) &&
+           current->mm &&
+           !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE)) {
+               fpsimd_save_state(&current->thread.fpsimd_state);
+               this_cpu_write(fpsimd_last_state, NULL);
+       }
+

I am having trouble understanding why we need all of this if we don't
support SVE in the kernel. Could you elaborate?

Dave knows all the details but a reason is that touching a Neon register
zeros the upper SVE state in the same vector register. So we can't
safely save/restore just the Neon part without corrupting the SVE state.

This is right -- this also means that EFI services can trash the upper
bits of an SVE vector register (as a side-effect of FPSIMD/NEON usage).

It's overkill to save/restore absolutely everything -- I ignore num_regs
for example -- but I wanted to keep things as simple as possible
initially.

Without looking at your patches in detail, could we mandate in the ABI
that the SVE state is lost on the user/kernel syscall boundary? I guess
even for the PCS, the SVE state is caller-saved, so there shouldn't be
an additional cost to user. On interrupts, however, we'd have to
preserve the SVE state but if we do this on entry/exit points, the
kernel_neon_*() functions would not have to deal with any SVE state (and
even ignore it completely if in interrupt).

See [RFC PATCH 24/29] arm64/sve: Discard SVE state on system call.

Currently, kernel_neon_begin_partial() doesn't take advandage of this --
discarding the state is deferred until sched-out, and anyway I don't
check for TIF_SVE in kernel_neon_begin_partial().  There's definitely
some room for improvement.

the requirements for syscall and sigcontext modifications.

Agreed -- I wanted to get this series posted so skipped that for now,
but I plan to include a Documentation patch alongside the final series.

Cheers
---Dave