linux-kernel - Re: [PATCH V2 1/4] selftests: Provide local define of __cpuid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 16 Apr 2022 15:52:00 +0800
From:   Pengfei Xu <pengfei.xu@...el.com>
To:     Reinette Chatre <reinette.chatre@...el.com>
Cc:     shuah@...nel.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, dave.hansen@...ux.intel.com,
        sandipan@...ux.ibm.com, fweimer@...hat.com,
        desnesn@...ux.vnet.ibm.com, mingo@...nel.org,
        bauerman@...ux.ibm.com, mpe@...erman.id.au, msuchanek@...e.de,
        linux-mm@...ck.org, chang.seok.bae@...el.com, bp@...e.de,
        tglx@...utronix.de, hpa@...or.com, x86@...nel.org, luto@...nel.org,
        heng.su@...el.com
Subject: Re: [PATCH V2 1/4] selftests: Provide local define of __cpuid_count()

On 2022-03-15 at 09:44:25 -0700, Reinette Chatre wrote:
> Some selftests depend on information provided by the CPUID instruction.
> To support this dependency the selftests implement private wrappers for
> CPUID.
> 
> Duplication of the CPUID wrappers should be avoided.
> 
> Both gcc and clang/LLVM provide __cpuid_count() macros but neither
> the macro nor its header file are available in all the compiler
> versions that need to be supported by the selftests. __cpuid_count()
> as provided by gcc is available starting with gcc v4.4, so it is
> not available if the latest tests need to be run in all the
> environments required to support kernels v4.9 and v4.14 that
> have the minimal required gcc v3.2.
> 
> Provide a centrally defined macro for __cpuid_count() to help
> eliminate the duplicate CPUID wrappers while continuing to
> compile in older environments.
> 
> Suggested-by: Shuah Khan <skhan@...uxfoundation.org>
> Signed-off-by: Reinette Chatre <reinette.chatre@...el.com>
> ---
> Note to maintainers:
> - Macro is identical to the one provided by gcc, but not liked by
>   checkpatch.pl with message "Macros with complex values should
>   be enclosed in parentheses". Similar style is used in kernel,
>   for example in arch/x86/kernel/fpu/xstate.h.
> 
>  tools/testing/selftests/kselftest.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index f1180987492c..898d7b2fac6c 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -52,6 +52,21 @@
> + * have __cpuid_count().
> + */
> +#ifndef __cpuid_count
> +#define __cpuid_count(level, count, a, b, c, d)				\
> +	__asm__ __volatile__ ("cpuid\n\t"				\
> +			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
> +			      : "0" (level), "2" (count))
> +#endif
   Linux C check tool "scripts/checkpatch.pl" shows an error:
"
ERROR: Macros with complex values should be enclosed in parentheses
...
+#define __cpuid_count(level, count, a, b, c, d)                        \
+       __asm__ __volatile__ ("cpuid\n\t"                               \
+                             : "=a" (a), "=b" (b), "=c" (c), "=d" (d)  \
+                             : "0" (level), "2" (count))
"
Googling:
https://www.google.com/search?q=Macros+with+complex+values+should+be+enclosed+in+parentheses&rlz=1C1GCEB_enUS884US884&oq=Macros+with+complex+values+should+be+enclosed+in+parentheses&aqs=chrome.0.69i59j0i5i30l2.313j0j7&sourceid=chrome&ie=UTF-8
-> https://stackoverflow.com/questions/8142280/why-do-we-need-parentheses-around-block-macro

Could we fix it as follow, shall we?
"
#ifndef __cpuid_count
#define __cpuid_count(level, count, a, b, c, d) ({			\
	__asm__ __volatile__ ("cpuid\n\t"				\
			      : "=a" (a), "=b" (b), "=c" (c), "=d" (d)	\
			      : "0" (level), "2" (count))		\
})
#endif
"
Thanks!
--Pengfei