Received-SPF: pass (google.com: domain of linux-crypto+bounces-734-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20231208113218.3001940-6-ardb@google.com> <170231871028.1857077.10318072500676133330.b4-ty@kernel.org>
In-Reply-To: <170231871028.1857077.10318072500676133330.b4-ty@kernel.org>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Tue, 12 Dec 2023 09:16:13 +0100
Message-ID: <CAMj1kXE=2H_Jqyuv_gDp6QS2g2Vzdgf=fngp=ZXEUBXKOYWnbQ@mail.gmail.com>
Subject: Re: [PATCH v4 0/4] arm64: Run kernel mode NEON with preemption enabled
To: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@google.com>, linux-arm-kernel@lists.infradead.org, 
	catalin.marinas@arm.com, kernel-team@android.com, 
	Mark Rutland <mark.rutland@arm.com>, Sebastian Andrzej Siewior <bigeasy@linutronix.de>, 
	Eric Biggers <ebiggers@google.com>, Kees Cook <keescook@chromium.org>, 
	Marc Zyngier <maz@kernel.org>, Mark Brown <broonie@kernel.org>, linux-crypto@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

On Mon, 11 Dec 2023 at 21:27, Will Deacon <will@kernel.org> wrote:
>
> Hey Ard,
>
> On Fri, 8 Dec 2023 12:32:19 +0100, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > Currently, kernel mode NEON (SIMD) support is implemented in a way that
> > requires preemption to be disabled while the SIMD registers are live.
> > The reason for this is that those registers are not in the set that is
> > preserved/restored on exception entry/exit and context switch, as this
> > would impact performance generally, even for workloads where kernel mode
> > SIMD is not the bottleneck.
> >
> > [...]
>
> I applied the first three patches to for-next/fpsimd:
>

Thanks

> [1/4] arm64: fpsimd: Drop unneeded 'busy' flag
>       https://git.kernel.org/arm64/c/e109130b0e5e
> [2/4] arm64: fpsimd: Preserve/restore kernel mode NEON at context switch
>       https://git.kernel.org/arm64/c/1e3a3de1ff6c

I spotted a typo in the commit log of this patch:

TIF_USING_KMODE_FPSIMD -> TIF_KERNEL_FPSTATE


> [3/4] arm64: fpsimd: Implement lazy restore for kernel mode FPSIMD
>       https://git.kernel.org/arm64/c/035262623959
>
> It would be nice to have an Ack from Herbert on the last one so that
> he's aware of the possible conflicts.
>
> The other thing I tangentially wondered about is what happens now if code
> calls uaccess routines (e.g. get_user()) within a kernel_neon_{begin,end}
> section? I think previously the fact that preemption had to be disabled
> would've caused the might_fault() to explode, but now I suppose the BUG_ON()
> in kernel_neon_begin() will save us. Is that right?
>

Not sure what you mean by 'save us'. Is there anything fundamentally
wrong with doing user access from a kernel mode NEON region if
preemption remains enabled?

The BUG_ON() will only catch uses from hardirq/NMI context, or cases
where FP/SIMD is not implemented/enabled in the first place so it will
not trigger on a user access.