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 <d...@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 <d...@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
