Re: [PATCH] x86: Add Hygon Dhyana support.

[Carlos O'Donell <carlos at redhat dot com>]

On 12/10/18 5:26 AM, fanjinke wrote:
> This patch fix Hygon Dhyana processor CPU Vendor ID detection problem
> in glibc sysdep module, current glibc codes doesn't recognize Dhyana
> CPU Vendor ID("HygonGenuine") and set kind to arch_kind_other, which
> result to incorrect zero value for __cache_sysconf() syscall. As Hygon
> Dhyana share most architecture feature as AMD Family 17h, this patch
> add Hygon CPU Vendor ID check and setup kind to arch_kind_amd and reuse
> AMD code path, which lead to correct return value in __cache_sysconf()
> syscall. we run the glibc test suite for both Hygon Dhyana and AMD EPYC
> and found no failure case.
> 
> Background:
> Chengdu Haiguang IC Design Co., Ltd (Hygon) is a Joint Venture between
> AMD and Haiguang Information Technology Co.,Ltd., aims at providing high
> performance x86 processor for China server market. Its first generation
> processor codename is Dhyana, which originates from AMD technology and
> shares most of the architecture with AMD's family 17h, but with different
> CPU Vendor ID("HygonGenuine")/Family series number(Family 18h).
> 
> Related Hygon kernel patch can be found on
> http://lkml.kernel.org/r/5ce86123a7b9dad925ac583d88d2f921040e859b.
> 1538583282.git.puwen@hygon.cn
> 
> Very appreciate everyone who has helped review and fix issues.

Do you have copyright assignment papers filed with the FSF?

This one-line change doesn't need copyright papers filed, but if
you need to make more compilcated changes, then you will need
them.

> *Tested for Hygon:*
> Before patching the results of hygon's tests:
> Summary of test results:
>       6 FAIL
>    5927 PASS
>      29 UNSUPPORTED
>      17 XFAIL
>       2 XPASS
> Makefile:403: recipe for target 'tests' failed
> make[1]: *** [tests] Error 1
> make[1]: Leaving directory
> '/home/higon/space/src/opensrc/glibc/glibc/glibc'
> Makefile:9: recipe for target 'check' failed
> make: *** [check] Error 2
> 
> And the result of testing after patching:
> Summary of test results:
>    5945 PASS
>      17 UNSUPPORTED
>      17 XFAIL
>       2 XPASS
> make[1]: Leaving directory
> '/home/higon/space/src/opensrc/glibc/glibc/glibc'
> 
> *Tested for AMD:*
> The test results did not change before and after patching:
> Summary of test results:
>    5951 PASS
>      11 UNSUPPORTED
>      17 XFAIL
>       2 XPASS
> make[1]: Leaving directory '/home/higon/glibc'
> 
> ChangeLog:
> 
> 2018-12-10  fanjinke  <fanjinke@hygon.cn>
> 
>         * sysdeps/x86/cpu-features.c (init_cpu_features):
> ---
>  sysdeps/x86/cpu-features.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 5f9eefd408..86e78bfb78 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -425,8 +425,9 @@ init_cpu_features (struct cpu_features *cpu_features)
>  	cpu_features->feature[index_arch_Prefer_No_AVX512]
>  	  |= bit_arch_Prefer_No_AVX512;
>      }
> -  /* This spells out "AuthenticAMD".  */
> -  else if (ebx == 0x68747541 && ecx == 0x444d4163 && edx == 0x69746e65)
> +  /* This spells out "AuthenticAMD" or "HygonGenuine". */
> +  else if ((ebx == 0x68747541 && ecx == 0x444d4163 && edx == 0x69746e65)
> +	|| (ebx == 0x6f677948 && ecx == 0x656e6975 && edx == 0x6e65476e))
>      {
>        unsigned int extended_model;
  
This looks fine to me, and reuses the AMD code.

I'd like to hear from Amit if this is OK for him, since he'll have to
maintain this code as things diverge.

Amit, Any objections?

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

-- 
Cheers,
Carlos.