[PATCH 0/5] Simplify kernel-mode NEON

[Catalin Marinas]

On Fri, Aug 04, 2017 at 02:21:52PM +0100, Ard Biesheuvel wrote:
> On 4 August 2017 at 14:12, Catalin Marinas <catalin.marinas at arm.com> wrote:
> > On Fri, Aug 04, 2017 at 01:25:03PM +0100, Ard Biesheuvel wrote:
> >> On 4 August 2017 at 13:22, Catalin Marinas <catalin.marinas at arm.com> wrote:
> >> > On Fri, Aug 04, 2017 at 01:08:58PM +0100, Catalin Marinas wrote:
> >> >> On Thu, Aug 03, 2017 at 05:23:18PM +0100, Dave P Martin wrote:
> >> >> > This series depends on Ard's v4.14 crypto rework [2].
> >> > [...]
> >> >> > [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/520664.html
> >> > [...]
> >> >> > Ard Biesheuvel (1):
> >> >> >   arm64: neon: replace generic definition of may_use_simd()
> >> >> >
> >> >> > Dave Martin (4):
> >> >> >   arm64: neon: Add missing header guard in <asm/neon.h>
> >> >> >   arm64: fpsimd: Consistently use __this_cpu_ ops where appropriate
> >> >> >   arm64: neon: Allow EFI runtime services to use FPSIMD in irq context
> >> >> >   arm64: neon: Remove support for nested or hardirq kernel-mode NEON
> >> >>
> >> >> Queued for 4.14. Thanks.
> >> >
> >> > ... and reverted.
> >> >
> >> > I now realised that it doesn't build without Ard's crypto series (I had
> >> > the wrong impression it is just a performance impact). I get errors
> >> > like:
> >> >
> >> > arch/arm64/crypto/sha1-ce-glue.c|41 col 2| error: implicit declaration
> >> > of function ‘kernel_neon_begin_partial’
> >>
> >> Ah yes. Apologies for failing to mention that. Apparently, there are
> >> in fact build time cross-dependencies that I forgot about.
> >>
> >> To avoid delaying this unnecessarily, we just alias
> >> kernel_neon_begin_partial() to kernel_neon_begin() and ignore the
> >> argument. We can remove that again when everything has gone upstream.
> >
> > But would kernel_neon_begin() be called in an interrupt context from the
> > crypto code? I'd like the arm64 for-next/core branch (to be based on
> > -rc4 next week) to still be fully working.
> 
> Well, theoretically. In the new situation we allow KMN from process
> and softirq context, but the latter only if no process context KMN is
> in progress. This last rule could be violated by the old crypto code,
> but the chances of actually hitting that are very low: if your wifi
> chip lacks a h/w implementation of CCMP (which is uncommon these
> days), and it gets invoked while KMN crypto is being used on the same
> core, i.e., for IPsec or block encryption, you may get a splat. It
> wouldn't corrupt your encrypted file system.

Just to make sure I understand correctly, even if may_use_simd() returns
false, the crypto code could still call kernel_neon_begin_partial() in
an interrupt context. I guess this is still fine with this series until
the last patch when we would get a BUG_ON (if we define
kernel_neon_begin_partial to kernel_neon_begin).

> Perhaps we could stick it on a separate branch for this cycle (with
> the suggested tweak) and get it pulled it into -next separately? That
> way, you can forward it as a late pull, after Herbert's stuff gets in.

I'll push it to a branch for now (with a fix to be able to build) and
look at the merge later (I don't think I'll ever hit the BUG_ON with my
hardware but you never know; linux-next should be fine since the other
crypto patches are merged). In hindsight, we should have put your crypto
patches on a common branch but Herbert already merged them on top of
others.

-- 
Catalin