[PATCH] raid6/altivec: adding vpermxor implementation for raid6 Q syndrome

[Matt Brown]

Hi Daniel,

Just to respond to your comments,

The inline asm line cannot be formatted over multiple lines due to the
unrolling process, but we can take out the volatile.

The pagefault_disable() also seems to be an old method of disabling
preemption, but no longer actually works to disable preemption.
Preempt_disable should be used instead now. So the use of
pagefault_disable() in crc32d-vpmsum_glue.c is actually a bug.

Thanks,
Matt

On Tue, Apr 4, 2017 at 11:51 AM, Daniel Axtens <dja at axtens.net> wrote:
> Hi Matt,
>
>> Woops, totally missed that big chunk of makefile in the commit.
>> I had a chat with Oliver last week about the backwards compatibility stuff.
>> This will work for all versions >= 207S.
>>
>> From what I can tell there is almost no difference between
>> pagefault_disable() and preempt_disable(), but I'll follow that up
>> when I'm in the office next.
>
> Cool, good to know.
>
> See you when you're next in!
>
> Regards,
> Daniel
>
>>
>> Thanks for the review,
>>
>> Matt
>>
>> On Tue, Apr 4, 2017 at 7:44 AM, Daniel Axtens <dja at axtens.net> wrote:
>>>> In that function, the flow is:
>>>>  pagefault_disable();
>>>>  enable_kernel_altivec();
>>>>  <vectorised function>
>>>>  pagefault_enable();
>>>>
>>>> There are a few things that it would be nice (but by no means essential)
>>>> to find out:
>>>>  - what is the difference between pagefault and prempt enable/disable
>>>>  - is it required to disable altivec after the end of the function or
>>>>    can we leave that enabled?
>>>
>>> Answering my own question here, dc4fbba11e46 ("powerpc: Create
>>> disable_kernel_{fp,altivec,vsx,spe}()") adds the disable_ function, and
>>> it's a no-op except under debug conditions. So it should stay.
>>>
>>> Regards,
>>> Daniel
>>>
>>>
>>>>
>>>>> +
>>>>> +int raid6_have_altivec_vpermxor(void);
>>>>> +#if $# == 1
>>>>> +int raid6_have_altivec_vpermxor(void)
>>>>> +{
>>>>> +    /* Check if CPU has both altivec and the vpermxor instruction*/
>>>> Please add a space: s|ion*/|ion */|
>>>>> +# ifdef __KERNEL__
>>>>> +    return (cpu_has_feature(CONFIG_ALTIVEC) &&
>>>>> +            cpu_has_feature(CPU_FTR_ARCH_207S));
>>>> I assume this is future-proof - an ISA 3.00 cpu will advertise 2.07S
>>>> compat?
>>>>
>>>>> +# else
>>>>> +    return 1;
>>>>> +#endif
>>>>> +
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> +const struct raid6_calls raid6_vpermxor$# = {
>>>>> +    raid6_vpermxor$#_gen_syndrome,
>>>>> +    NULL,
>>>>> +    raid6_have_altivec_vpermxor,
>>>>> +    "vpermxor$#",
>>>>> +    0
>>>>> +};
>>>>> +#endif
>>>>> --
>>>>> 2.9.3
>>>>
>>>> Apart from that this patch looks good and I expect I will be able to
>>>> formally Review v2.
>>>>
>>>> Regards,
>>>> Daniel