From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.groups.io with SMTP id smtpd.web10.7576.1570786648372269587 for ; Fri, 11 Oct 2019 02:37:28 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BE9F189AC4; Fri, 11 Oct 2019 09:37:27 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-177.rdu2.redhat.com [10.10.120.177]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7C1F3196B2; Fri, 11 Oct 2019 09:37:26 +0000 (UTC) Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain To: "Gao, Liming" References: <1569570395-11240-1-git-send-email-liming.gao@intel.com> <1569570395-11240-12-git-send-email-liming.gao@intel.com> <4c27bdc4-60b4-26bf-c416-c02b69ef8051@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E5124FA@SHSMSX104.ccr.corp.intel.com> <7fc791fe-9d08-9763-ecc9-529e88b621c6@redhat.com> <767711D5-7C33-4703-8E97-F4F5B5A6BD5F@apple.com> <6f49899d-b5e4-70ad-82e5-08c5a8649ac8@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E514EAB@SHSMSX104.ccr.corp.intel.com> Cc: Andrew Fish , "devel@edk2.groups.io" , Tom Lendacky From: "Laszlo Ersek" Message-ID: <43b303eb-fd74-7480-aa6d-6907300af3e0@redhat.com> Date: Fri, 11 Oct 2019 11:37:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E514EAB@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 11 Oct 2019 09:37:28 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Liming, On 10/09/19 16:44, Gao, Liming wrote: > The difference between XCODE/CLANG and GCCXX is the linker. Current > patches are introduced for the different linker. Clang supports most > usage of GCC compiler. So, CLANG and XCODE uses GCC family. When I > enable XCODE or CLANG tool chain in the platform, I don't find other > incompatible behavior with GCC compiler. So, I think it is safe to let > CLANG inherit GCC option except for these two cases. When you make new > changes, and verify them with GCC compiler, that's enough. You don't > need to specially verify them with XCODE or CLANG. In most case, GCC > compiler pass, XCODE or CLANG can also pass. I'd like to provide a counter-example for this. Consider the issue that Tom reported, at http://mid.mail-archive.com/8eb55d97-0ba3-c217-a160-c24730b9f036@amd.com https://edk2.groups.io/g/devel/message/48762 in point (2). (Both links point to the same message, just in different archives.) Let me summarize that problem. - In BZ#849, an XCODE toolchain bug was reported, and a workaround was requested. - The workaround was commit 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass XCODE5 tool chain", 2018-01-16). - The workaround is binary patching (self-modification) in the exception handler's assembly code. - Unfortunately, this code is also linked into SEC modules, which run from flash. Therefore, self-modification is not permitted, and the workaround is not good enough for SEC. (Nor for PEI modules that run from flash.) Now, let's consider how we could mitigate this issue for *temporarily*, for all toolchains *except* XCODE5, until the issue is fixed. Clearly, toolchains other than XCODE5 have no problems with the original assembly code (i.e., with the 64-bit absolute address relocations). Therefore, we could consider reverting the commit, and capturing the results of the revert in a *separate* NASM source file. This source file (that is, the original, pre-2db0ccc2d7fe assembly code) would apply to all toolchain families, except the XCODE family. Let's say the new NASM source files would be called - X64/ExceptionHandlerAsmGeneric.nasm -- all except XCODE, - X64/ExceptionHandlerAsmXcode.nasm -- XCODE, replacing the current file - X64/ExceptionHandlerAsm.nasm So, how can we use the new files, in the INF file UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf ? We could attempt, > [Sources.X64] > X64/ExceptionHandlerAsmGeneric.nasm | GCC > X64/ExceptionHandlerAsmXcode.nasm | XCODE > X64/ExceptionHandlerAsmGeneric.nasm | INTEL > X64/ExceptionHandlerAsmGeneric.nasm | MSFT Unfortunately, this does not work. While it solves the issue on GCC, INTEL and MSFT, it breaks on XCODE: XCODE picks up *both* the GCC line and the XCODE line. And that's because XCODE inherits GCC, and there is presently no way to express "real GCC only". But, at least, this suggests a solution too. In "BaseTools/Conf/tools_def.template", for *every* toolchain that specifies FAMILY=GCC, we should spell out an explicit BUILDRULEFAMILY. Like this: > @@ -1995,6 +1995,7 @@ > # > #################################################################################### > *_GCC48_*_*_FAMILY = GCC > +*_GCC48_*_*_BUILDRULEFAMILY = GENUINE_GCC > > *_GCC48_*_MAKE_PATH = DEF(GCC_HOST_PREFIX)make > *_GCC48_*_*_DLL = ENV(GCC48_DLL) > @@ -2134,6 +2135,7 @@ > # > #################################################################################### > *_GCC49_*_*_FAMILY = GCC > +*_GCC49_*_*_BUILDRULEFAMILY = GENUINE_GCC > > *_GCC49_*_MAKE_PATH = DEF(GCC_HOST_PREFIX)make > *_GCC49_*_*_DLL = ENV(GCC49_DLL) > @@ -2280,6 +2282,7 @@ > # > #################################################################################### > *_GCC5_*_*_FAMILY = GCC > +*_GCC5_*_*_BUILDRULEFAMILY = GENUINE_GCC > > *_GCC5_*_MAKE_PATH = DEF(GCC_HOST_PREFIX)make > *_GCC5_*_*_DLL = ENV(GCC5_DLL) > @@ -2437,6 +2440,7 @@ > # > #################################################################################### > *_CLANG35_*_*_FAMILY = GCC > +*_CLANG35_*_*_BUILDRULEFAMILY = CLANG > > *_CLANG35_*_MAKE_PATH = make > *_CLANG35_*_*_DLL = ENV(CLANG35_DLL) > @@ -2517,6 +2521,8 @@ > # > #################################################################################### > *_CLANG38_*_*_FAMILY = GCC > +*_CLANG38_*_*_BUILDRULEFAMILY = CLANG > + > *_CLANG38_*_MAKE_PATH = make > *_CLANG38_*_*_DLL = ENV(CLANG38_DLL) > *_CLANG38_*_ASL_PATH = DEF(UNIX_IASL_BIN) And then we could write: > [Sources.X64] > X64/ExceptionHandlerAsmGeneric.nasm | GENUINE_GCC > X64/ExceptionHandlerAsmGeneric.nasm | CLANG > X64/ExceptionHandlerAsmXcode.nasm | XCODE > X64/ExceptionHandlerAsmGeneric.nasm | INTEL > X64/ExceptionHandlerAsmGeneric.nasm | MSFT replacing plain "GCC", which XCODE inherits, with "GENUINE_GCC" and "CLANG", neither of which XCODE inherits. Would you accept the above patch, for "BaseTools/Conf/tools_def.template"? Thanks, Laszlo