Re: [PATCH 03/15] Add HAS_RTM

[Andreas Jaeger <aj at suse dot com>]

On 12/23/2012 04:02 AM, Andi Kleen wrote:
> From: "H.J. Lu" <hjl.tools@gmail.com>
>
> Add cpuid bits for RTM (Restricted Transactional Memory) and code
> to query cpuid 7.
> ---
>   ChangeLog.tsx                        |    9 +++++++++
>   sysdeps/x86_64/multiarch/init-arch.c |    7 +++++++
>   sysdeps/x86_64/multiarch/init-arch.h |   10 ++++++++++
>   3 files changed, 26 insertions(+), 0 deletions(-)
>   create mode 100644 ChangeLog.tsx
>
> diff --git a/ChangeLog.tsx b/ChangeLog.tsx
> new file mode 100644
> index 0000000..392b444
> --- /dev/null
> +++ b/ChangeLog.tsx
> @@ -0,0 +1,9 @@
> +2012-12-11  H.J. Lu  <hongjiu.lu@intel.com>
> +
> +	* sysdeps/x86_64/multiarch/init-arch.c (__init_cpu_features):
> +	Initialize COMMON_CPUID_INDEX_7 element.
> +	* sysdeps/x86_64/multiarch/init-arch.h (bit_RTM): New macro.
> +	(index_RTM): Likewise.
> +	(CPUID_RTM): Likewise.
> +	(HAS_RTM): Likewise.
> +	(COMMON_CPUID_INDEX_7): New enum.
> diff --git a/sysdeps/x86_64/multiarch/init-arch.c b/sysdeps/x86_64/multiarch/init-arch.c
> index 71a862c..d38878a 100644
> --- a/sysdeps/x86_64/multiarch/init-arch.c
> +++ b/sysdeps/x86_64/multiarch/init-arch.c
> @@ -143,6 +143,13 @@ __init_cpu_features (void)
>     else
>       kind = arch_kind_other;
>
> +  if (__cpu_features.max_cpuid >= 7)
> +    __cpuid_count (7, 0,
> +		   __cpu_features.cpuid[COMMON_CPUID_INDEX_7].eax,
> +		   __cpu_features.cpuid[COMMON_CPUID_INDEX_7].ebx,
> +		   __cpu_features.cpuid[COMMON_CPUID_INDEX_7].ecx,
> +		   __cpu_features.cpuid[COMMON_CPUID_INDEX_7].edx);
> +
>     /* Can we call xgetbv?  */
>     if (CPUID_OSXSAVE)
>       {
> diff --git a/sysdeps/x86_64/multiarch/init-arch.h b/sysdeps/x86_64/multiarch/init-arch.h
> index 79ff1fe..9ae2a9c 100644
> --- a/sysdeps/x86_64/multiarch/init-arch.h
> +++ b/sysdeps/x86_64/multiarch/init-arch.h
> @@ -26,6 +26,8 @@
>   #define bit_FMA4_Usable			(1 << 8)
>
>   /* CPUID Feature flags.  */
> +
> +/* COMMON_CPUID_INDEX_1.  */
>   #define bit_SSE2	(1 << 26)
>   #define bit_SSSE3	(1 << 9)
>   #define bit_SSE4_1	(1 << 19)
> @@ -36,6 +38,9 @@
>   #define bit_FMA		(1 << 12)
>   #define bit_FMA4	(1 << 16)
>
> +/* COMMON_CPUID_INDEX_7.  */
> +#define bit_RTM		(1 << 11)
> +
>   /* XCR0 Feature flags.  */
>   #define bit_XMM_state  (1 << 1)
>   #define bit_YMM_state  (2 << 1)
> @@ -49,6 +54,7 @@
>   # define index_SSE4_1	COMMON_CPUID_INDEX_1*CPUID_SIZE+CPUID_ECX_OFFSET
>   # define index_SSE4_2	COMMON_CPUID_INDEX_1*CPUID_SIZE+CPUID_ECX_OFFSET
>   # define index_AVX	COMMON_CPUID_INDEX_1*CPUID_SIZE+CPUID_ECX_OFFSET
> +# define index_RTM	COMMON_CPUID_INDEX_7*CPUID_SIZE+CPUID_ECX_OFFSET

Should this really be ECX? Below you're using ebx.

>   # define index_Fast_Rep_String		FEATURE_INDEX_1*FEATURE_SIZE
>   # define index_Fast_Copy_Backward	FEATURE_INDEX_1*FEATURE_SIZE
> @@ -67,6 +73,7 @@
>   enum
>     {
>       COMMON_CPUID_INDEX_1 = 0,
> +    COMMON_CPUID_INDEX_7,
>       COMMON_CPUID_INDEX_80000001,	/* for AMD */
>       /* Keep the following line at the end.  */
>       COMMON_CPUID_INDEX_MAX
> @@ -138,6 +145,8 @@ extern const struct cpu_features *__get_cpu_features (void)
>     HAS_CPUID_FLAG (COMMON_CPUID_INDEX_1, ecx, bit_FMA)
>   # define CPUID_FMA4 \
>     HAS_CPUID_FLAG (COMMON_CPUID_INDEX_80000001, ecx, bit_FMA4)
> +# define CPUID_RTM \
> +  HAS_CPUID_FLAG (COMMON_CPUID_INDEX_7, ebx, bit_RTM)
>
>   /* HAS_* evaluates to true if we may use the feature at runtime.  */
>   # define HAS_SSE2	HAS_CPU_FEATURE (COMMON_CPUID_INDEX_1, edx, bit_SSE2)
> @@ -145,6 +154,7 @@ extern const struct cpu_features *__get_cpu_features (void)
>   # define HAS_SSSE3	HAS_CPU_FEATURE (COMMON_CPUID_INDEX_1, ecx, bit_SSSE3)
>   # define HAS_SSE4_1	HAS_CPU_FEATURE (COMMON_CPUID_INDEX_1, ecx, bit_SSE4_1)
>   # define HAS_SSE4_2	HAS_CPU_FEATURE (COMMON_CPUID_INDEX_1, ecx, bit_SSE4_2)
> +# define HAS_RTM	HAS_CPU_FEATURE (COMMON_CPUID_INDEX_7, ebx, bit_RTM)
>
>   # define index_Fast_Rep_String		FEATURE_INDEX_1
>   # define index_Fast_Copy_Backward	FEATURE_INDEX_1

This patch could go in separately IMO, provided it has been tested 
properly.  Seeing the typo above, I suggest that you add a testcase to 
test-multiarch.c as well so that the glibc testsuite will check for the 
flag.

Andreas
--
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126