[PATCH v2] arm64: fpsimd: improve stacking logic in non-interruptible context
Catalin Marinas
catalin.marinas at arm.com
Thu Dec 8 07:50:55 PST 2016
Hi Ard,
On Wed, Dec 07, 2016 at 10:14:08AM +0000, Ard Biesheuvel wrote:
> void kernel_neon_begin_partial(u32 num_regs)
> {
> - if (in_interrupt()) {
> - struct fpsimd_partial_state *s = this_cpu_ptr(
> - in_irq() ? &hardirq_fpsimdstate : &softirq_fpsimdstate);
> + struct fpsimd_partial_state *s;
> + int level;
> +
> + preempt_disable();
> +
> + level = this_cpu_read(kernel_neon_nesting_level);
> + BUG_ON(level > 2);
> +
> + if (level > 0) {
> + s = this_cpu_ptr(nested_fpsimdstate);
>
> - BUG_ON(num_regs > 32);
> - fpsimd_save_partial_state(s, roundup(num_regs, 2));
> + WARN_ON_ONCE(num_regs > 32);
> + num_regs = min(roundup(num_regs, 2), 32U);
> +
> + fpsimd_save_partial_state(&s[level - 1], num_regs);
> } else {
> /*
> * Save the userland FPSIMD state if we have one and if we
> @@ -241,24 +256,29 @@ void kernel_neon_begin_partial(u32 num_regs)
> * that there is no longer userland FPSIMD state in the
> * registers.
> */
> - preempt_disable();
> if (current->mm &&
> !test_and_set_thread_flag(TIF_FOREIGN_FPSTATE))
> fpsimd_save_state(¤t->thread.fpsimd_state);
> this_cpu_write(fpsimd_last_state, NULL);
> }
> + this_cpu_write(kernel_neon_nesting_level, level + 1);
> }
I'm slightly confused with the potential race with an interrupt here.
Let's say the above is running in the process context, sets the
TIF_FOREIGN_FPSTATE but is interrupted before fpsimd_save_state(). The
interrupt handler calling kernel_neon_begin_partial() is seeing level 0
and TIF_FOREIGN_FPSTATE and decides that it is safe to corrupt the Neon
state without any further saving.
I think the kernel_neon_nesting_level should be incremented early on in
this function.
--
Catalin
More information about the linux-arm-kernel
mailing list