On Thu, Nov 29, 2018 at 07:09:10PM +0100, Ard Biesheuvel wrote:
> On Thu, 29 Nov 2018 at 18:00, Dave Martin <Dave.Martin@xxxxxxx> 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
