From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5973DC433EF for ; Wed, 20 Apr 2022 07:23:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B057F6B0078; Wed, 20 Apr 2022 03:23:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A8E166B007B; Wed, 20 Apr 2022 03:23:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 907FC6B007D; Wed, 20 Apr 2022 03:23:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.26]) by kanga.kvack.org (Postfix) with ESMTP id 7C0B16B0078 for ; Wed, 20 Apr 2022 03:23:56 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id 48C46120B96 for ; Wed, 20 Apr 2022 07:23:56 +0000 (UTC) X-FDA: 79376418072.06.0C5C90D Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by imf21.hostedemail.com (Postfix) with ESMTP id 41E1B1C0019 for ; Wed, 20 Apr 2022 07:23:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1650439435; x=1681975435; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=VZ2QcDDsn09EZJ9ChcAfXKxOwF3NNkLHqhHF7SzxBPU=; b=C455mpaa4d+5mS0ZQdN4b7Y9Qzh3rR1iLcdeDOwK/bOFq80BVUteowEK N4mHgohNempxHkwm4JeQMTdGOWAirVAAhQdfDOi1vFd5WcbEApBANMaWt QePd7T9ky2rtYtT3uzJWXybaDbh81DxxyCRw4fQnDAMvoEAaE2ReTRj6t SK6QPoOSwh2R64tqctwplz75HjEJqkqyqVP2PkEkmxCYXMBvRg+Zpi8TN 2WoJ1JdGWXEvFu9JVmh6mGq58Oxp/RsOToUAXFga97MTTKWYvkEipqQcF QHNA+4EZF9S1GTSvjY/tbY/3+BG5RmG+h5c456918lyj8vzc8+bw8ThLy A==; X-IronPort-AV: E=McAfee;i="6400,9594,10322"; a="326861333" X-IronPort-AV: E=Sophos;i="5.90,274,1643702400"; d="scan'208";a="326861333" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2022 00:23:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,274,1643702400"; d="scan'208";a="555076861" Received: from xpf.sh.intel.com ([10.239.182.112]) by orsmga007.jf.intel.com with ESMTP; 20 Apr 2022 00:23:49 -0700 Date: Wed, 20 Apr 2022 15:22:32 +0800 From: Pengfei Xu To: Reinette Chatre Cc: shuah@kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com, sandipan@linux.ibm.com, fweimer@redhat.com, desnesn@linux.vnet.ibm.com, mingo@kernel.org, bauerman@linux.ibm.com, mpe@ellerman.id.au, msuchanek@suse.de, linux-mm@kvack.org, chang.seok.bae@intel.com, bp@suse.de, tglx@linutronix.de, hpa@zytor.com, x86@kernel.org, luto@kernel.org, heng.su@intel.com Subject: Re: [PATCH V2 1/4] selftests: Provide local define of __cpuid_count() Message-ID: References: <7c49dbfe5bab04389ed84c516fcbfe31d66df880.1647360971.git.reinette.chatre@intel.com> <50067c2d-5563-7d8c-f992-5fef787d4d38@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=C455mpaa; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf21.hostedemail.com: domain of pengfei.xu@intel.com has no SPF policy when checking 134.134.136.100) smtp.mailfrom=pengfei.xu@intel.com X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 41E1B1C0019 X-Stat-Signature: 5sogji3b58qabg3z859cbjy96eixdm7z X-HE-Tag: 1650439434-602829 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2022-04-19 at 15:34:11 -0700, Reinette Chatre wrote: > Hi Pengfei, > > On 4/18/2022 9:31 PM, Pengfei Xu wrote: > > On 2022-04-18 at 09:04:33 -0700, Reinette Chatre wrote: > >> Hi Pengfei, > >> > >> On 4/16/2022 12:52 AM, Pengfei Xu wrote: > >>> 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 > >>>> Signed-off-by: Reinette Chatre > >>>> --- > >>>> 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 > >> > >> I encountered this also and that is why this patch contains the "Note to > >> maintainers" above. It is not clear to me whether you considered the note > >> since your response does not acknowledge it. > >> > > Sorry, I just made a suggestion to fix this problem mentioned by the script. > > I didn't notice and reply for the note. > > > >>> ... > >>> +#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 > >> > >> More information available in > >> https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html#Statement-Exprs > >> but from what I understand it does not apply to this macro. Even so, I do > >> not know what checkpatch.pl uses to determine that this is a "Macro with > >> complex values". > >> > > Checked checkpatch.pl and it seems to suggest using ({ }) for any asm macro > > definition. > > > >>> > >>> 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 > >>> " > >> > >> Sure, I can do so. > >> > > I just made a suggestion to fix the problem reported by the checkpatch.pl. > > But I didn't think deeply enough before: I'm not sure is there any real > > improvment or help after the fix. > > In this case I would prefer to not implicitly follow the checkpatch.pl without > understanding what the concern is. > > The goal of this change is to make the __cpuid_count() macro available > within kselftest and it does so by duplicating gcc's __cpuid_count() macro. > > The macro style is not unique and you would, for example, encounter the same > checkpatch.pl complaint if you run: > ./scripts/checkpatch.pl -f arch/x86/kernel/fpu/xstate.h Ok, no question from my side. Thanks! --Pengfei > > Reinette