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.web12.7831.1570787877863062649 for ; Fri, 11 Oct 2019 02:57:58 -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-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 705CF30024DA; Fri, 11 Oct 2019 09:57:57 +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 3B9895C21A; Fri, 11 Oct 2019 09:57:56 +0000 (UTC) Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain - To: devel@edk2.groups.io, liming.gao@intel.com, Andrew Fish Cc: "Justen, Jordan L" 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> <1e3de9da-1218-588a-4632-0263d6f64de5@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E51572A@SHSMSX104.ccr.corp.intel.com> <08f939d6-f980-aa4b-e867-dc719ec91ae1@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E515A71@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: Date: Fri, 11 Oct 2019 11:57:55 +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: <4A89E2EF3DFEDB4C8BFDE51014F606A14E515A71@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Fri, 11 Oct 2019 09:57:57 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/11/19 03:47, Liming Gao wrote: > Laszlo: > >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Friday, October 11, 2019 12:43 AM >> To: devel@edk2.groups.io; Gao, Liming ; Andrew Fish >> >> Cc: Justen, Jordan L >> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain - >> >> Hi Liming, >> >> On 10/10/19 14:18, Liming Gao wrote: >>>> -----Original Message----- >>>> From: devel@edk2.groups.io On Behalf Of Laszlo >> Ersek >>>> Sent: Thursday, October 10, 2019 3:35 PM >>>> To: Andrew Fish ; Gao, Liming >>>> Cc: devel@edk2.groups.io >>>> Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool >> chain - >>>> >>>> Hi Andrew, >>>> >>>> On 10/09/19 18:22, Andrew Fish wrote: >>>> >>>>> I thought the thing we were discussing was compiler flags. >>>>> Specifically -mno-mmx -mno-sse. It seems to me if OVMF requires >>>>> -mno-mmx -mno-sse then it is a bug in the tools_def.txt definition >>>>> for those compilers? As far as I can tell -mno-implicit-float should >>>>> prevent the optimizer from using floating point. The -mno-mmx >>>>> -mno-sse flags most change how floating point code gets compiled [1]. >>>>> it looks like -mno-mmx -mno-sse just down grade floating point >>>>> instructions that get used. Thus it seems like either we have some >>>>> code doing float and that code should set -mno-mmx -mno-sse, or the >>>>> -mno-mmx -mno-sse should be set generically. >>>> >>>> The origin of those build flags in OVMF is commit 4a8266f570ef >>>> ("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31): >>>> >>>> OvmfPkg: Work around issue seen with kvm + grub2 (efi) >>>> >>>> When OVMF is run with kvm and grub2 (efi), an exception >>>> occurs when mmx/sse registers are accessed. >>>> >>>> As a work around, this change eliminates firmware usage >>>> of these register types. >>>> >>>> First, only the BaseMemoryLib implementation >>>> MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf >>>> is used. >>>> >>>> Second, the GCC compiler is passes the additional >>>> '-mno-mmx -mno-sse' parameters. >>>> >>>> The eternal problem with this kind of workaround is that we never know >>>> when it becomes safe to remove. >>>> >>>> It would be nice to understand the exact details around the problem. >>>> Given that the commit is from 2010, I assume the issue is difficult to >>>> reproduce with recent components (KVM, grub2). On the other hand, if >> we >>>> just remove the flags (which we should, in a perfect world), someone >>>> could report a bug in three months: "my host distro upgraded the OVMF >>>> package to the next edk2-stableYYYYMM tag, and now my virtual machine, >>>> which runs a distro from 2009, no longer boots". (We've seen similar >>>> reports before.) >>> >>> Yes. I agree it is hard to decide to remove this option, because we don't >> know its impact. >>> With the option -mno-mmx -mno-sse, it will cause CLANG9 linker failure like >> below. So, I say >>> this patch is to support the different linker. >>> >>> 0. Program arguments: C:\Program Files\LLVM\bin\lld-link.EXE >> /OUT:d:\allpkg\edk2\Build\Ovmf3264\DEBUG_CLANG9\X64\Se >>> >> curityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDx >> e\DEBUG\SecureBootConfigDxe.dll /NOLOGO /NODEFAULT >>> LIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /ALIGN:32 /FILEALIGN:32 >> /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT >>> RY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER >> /SAFESEH:NO /BASE:0 /DEBUG:GHASH /lldmap @d:\allpkg\edk2\Build\O >>> >> vmf3264\DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBo >> otConfigDxe\SecureBootConfigDxe\OUTPUT\static_library >>> _files.lst >>> 1. Running pass 'Function Pass Manager' on module 'ld-temp.o'. >>> 2. Running pass 'X86 FP Stackifier' on function '@drbg_add' >>> #0 0x00007ff696bad7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x148d7f8 >> C:\Program Files\LLVM\bin\lld-link.EXE 0x142ba7f >> >> can you please try the following: >> >> (a) replace this patch with two patches, as follows >> >> (b) in the first patch, only rework >> >> !if $(TOOL_CHAIN_TAG) != "XCODE5" >> GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse >> !endif >> >> into: >> >> GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse >> XCODE:*_*_*_CC_FLAGS = >> >> (c) in the second patch, append: >> >> CLANGPE:*_*_*_CC_FLAGS = >> > This way clear CLANG9 CC flags. It forbids CLANG9 to inherit other GCC flags. > Below flags are not applied into CLANG9. The purpose is not to include > specific -mno-mmx -mno-sse option, and still inherit other GCC flags. > So, I have to use $(TOOL_CHAIN_TAG) check. > > MSFT:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > INTEL:*_*_*_CC_FLAGS = /D DISABLE_NEW_DEPRECATED_INTERFACES > GCC:*_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES > >> and also introduce >> >> CLANGPE: *_*_*_DLINK_FLAGS = /ALIGN:4096 >> >> >> If (b) preserves the intended effect for the XCODE5 toolchain, and (c) >> does the right thing for the CLANG9 toolchain as well, then that >> approach would be preferable to the currently proposed patch#11. >> >> If the above does *not* work, then I'm fine with the currently proposed >> patch, but then please update the commit message; it should say that >> "-mno-mmx -mno-sse" is *excluded* for CLANG9. >> > Yes. I will update the commit message. OK. With the commit message updated, for this patch: Reviewed-by: Laszlo Ersek For later, please consider the GENUINE_GCC suggestion (for BUILDRULEFAMILY) in my other message in the present thread: http://mid.mail-archive.com/43b303eb-fd74-7480-aa6d-6907300af3e0@redhat.com https://edk2.groups.io/g/devel/message/48808 If you think that the "BaseTools/Conf/tools_def.template" patch that I suggested there is acceptable, then I would like to update the OVMF DSC files (later) as follows: GENUINE_GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse CLANG:*_*_*_CC_FLAGS = -mno-mmx -mno-sse /* say nothing about XCODE */ /* say nothing CLANGPE */ Thanks Laszlo