On Thu, Nov 29, 2018 at 07:09:10PM +0100, Ard Biesheuvel wrote: > On Thu, 29 Nov 2018 at 18:00, Dave Martin <dave.mar...@arm.com> wrote: > > > > On Tue, Nov 27, 2018 at 06:08:58PM +0800, Jackie Liu wrote:
[...] > > > +static struct xor_block_template xor_block_arm64 = { > > > + .name = "arm64_neon", > > > + .do_2 = xor_neon_2, > > > + .do_3 = xor_neon_3, > > > + .do_4 = xor_neon_4, > > > + .do_5 = xor_neon_5 > > > +}; > > > +#undef XOR_TRY_TEMPLATES > > > +#define XOR_TRY_TEMPLATES \ > > > + do { \ > > > + xor_speed(&xor_block_8regs); \ > > > + xor_speed(&xor_block_32regs); \ > > > + if (cpu_has_neon()) { \ > > > + xor_speed(&xor_block_arm64);\ > > > + } \ > > > + } while (0) > > > > Should there be a may_use_simd() check somewhere? > > > > If we invoke this in a softirq I don't see what prevents us from > > corrupting the task's NEON state. > > > > (The check might be in some surrounding glue code that I missed...) > > > > There is no check. This code should simply not be called from > non-process context, same as the RAID56 code. > > This is not terribly robust, obviously, but appears to be common > practice in this realm of the kernel. Fair enough -- I was just curious. If this goes wrong, we should get a clear splat in kernel_neon_begin() anyway. I'd be more concerned if we could just end up scribbling over the NEON state silently. Cheers ---Dave