Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Hongbo Zhang <hongbo dot zhang at linaro dot org>]

On 11 June 2018 at 20:34, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> On 06/11/2018 03:16 PM, Hongbo Zhang wrote:
>>
>> Phecda is HXT semiconductor's CPU core, this patch adds memory operation
>> ifuncs for it: sharing the same optimized implementation with Qualcomm's
>> Falkor core.
>>
>> 2018-06-07  Minfeng Kang <minfeng.kang@hxt-semitech.com>
>>             Hongbo Zhang <hongbo.zhang@linaro.org>
>>
>>         * sysdeps/aarch64/multiarch/memcpy.c (libc_ifunc): reuse
>>         __memcpy_falkor for phecda core.
>>         * sysdeps/aarch64/multiarch/memmove.c (libc_ifunc): reuse
>>         __memmove_falkor for phecda core.
>>         * sysdeps/aarch64/multiarch/memset.c (libc_ifunc): reuse
>>         __memset_falkor for phecda core.
>>         * sysdeps/unix/sysv/linux/aarch64/cpu-features.c: add MIDR entry
>>         for phecda core.
>>         * sysdeps/unix/sysv/linux/aarch64/cpu-features.h (IS_PHECDA): add
>>         macro to identify phecda core.
>
>
> Thank you for the patch.  Your contributions are covered by the Linaro
> copyright assignment, but you have also attributed Minfeng Kang with part of
> this patch.  Can you (or Carlos/Joseph) please clarify the copyright
> assignment status for Minfeng?  I don't have access to the assignment file
> to confirm myself.
>
> Thanks,
> Siddhesh
>
Hi Siddhesh,
Thank you for the quick review.
It is my first time to upstream a glibc patch, rules seem a bit
different from kernel/uboot etc.
If it is up to me to clarify the copyright, how to do it, add a
Co-authored-by tag?

Thanks
>
>> ---
>>   sysdeps/aarch64/multiarch/memcpy.c             | 2 +-
>>   sysdeps/aarch64/multiarch/memmove.c            | 2 +-
>>   sysdeps/aarch64/multiarch/memset.c             | 7 ++++---
>>   sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 1 +
>>   sysdeps/unix/sysv/linux/aarch64/cpu-features.h | 3 +++
>>   5 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/sysdeps/aarch64/multiarch/memcpy.c
>> b/sysdeps/aarch64/multiarch/memcpy.c
>> index b94c655..4a04a63 100644
>> --- a/sysdeps/aarch64/multiarch/memcpy.c
>> +++ b/sysdeps/aarch64/multiarch/memcpy.c
>> @@ -36,7 +36,7 @@ extern __typeof (__redirect_memcpy) __memcpy_falkor
>> attribute_hidden;
>>   libc_ifunc (__libc_memcpy,
>>               (IS_THUNDERX (midr)
>>              ? __memcpy_thunderx
>> -            : (IS_FALKOR (midr)
>> +            : (IS_FALKOR (midr) || IS_PHECDA (midr)
>>                 ? __memcpy_falkor
>>                 : (IS_THUNDERX2 (midr) || IS_THUNDERX2PA (midr)
>>                   ? __memcpy_thunderx2
>> diff --git a/sysdeps/aarch64/multiarch/memmove.c
>> b/sysdeps/aarch64/multiarch/memmove.c
>> index afd8dd2..e69d816 100644
>> --- a/sysdeps/aarch64/multiarch/memmove.c
>> +++ b/sysdeps/aarch64/multiarch/memmove.c
>> @@ -35,7 +35,7 @@ extern __typeof (__redirect_memmove) __memmove_falkor
>> attribute_hidden;
>>   libc_ifunc (__libc_memmove,
>>               (IS_THUNDERX (midr)
>>              ? __memmove_thunderx
>> -            : (IS_FALKOR (midr)
>> +            : (IS_FALKOR (midr) || IS_PHECDA (midr)
>>                 ? __memmove_falkor
>>                 : __memmove_generic)));
>>   diff --git a/sysdeps/aarch64/multiarch/memset.c
>> b/sysdeps/aarch64/multiarch/memset.c
>> index 6df93d5..d74ed3a 100644
>> --- a/sysdeps/aarch64/multiarch/memset.c
>> +++ b/sysdeps/aarch64/multiarch/memset.c
>> @@ -31,9 +31,10 @@ extern __typeof (__redirect_memset) __libc_memset;
>>   extern __typeof (__redirect_memset) __memset_falkor attribute_hidden;
>>   extern __typeof (__redirect_memset) __memset_generic attribute_hidden;
>>   -libc_ifunc (__libc_memset, (IS_FALKOR (midr) && zva_size == 64
>> -                           ? __memset_falkor
>> -                           : __memset_generic));
>> +libc_ifunc (__libc_memset,
>> +           ((IS_FALKOR (midr) || IS_PHECDA (midr)) && zva_size == 64
>> +            ? __memset_falkor
>> +            : __memset_generic));
>>     # undef memset
>>   strong_alias (__libc_memset, memset);
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> index 33b87a8..203f839 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> @@ -35,6 +35,7 @@ static struct cpu_list cpu_list[] = {
>>         {"thunderxt88",  0x430F0A10},
>>         {"thunderx2t99",   0x431F0AF0},
>>         {"thunderx2t99p1", 0x420F5160},
>> +      {"phecda",        0x680F0000},
>>         {"generic",      0x0}
>>   };
>>   diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> index cde655b..eb35adf 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> @@ -49,6 +49,9 @@
>>   #define IS_FALKOR(midr) (MIDR_IMPLEMENTOR(midr) == 'Q'
>> \
>>                           && MIDR_PARTNUM(midr) == 0xc00)
>>   +#define IS_PHECDA(midr) (MIDR_IMPLEMENTOR(midr) == 'h'
>> \
>> +                        && MIDR_PARTNUM(midr) == 0x000)
>> +
>>   struct cpu_features
>>   {
>>     uint64_t midr_el1;
>>
>