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.126.1570725790215474554 for ; Thu, 10 Oct 2019 09:43:10 -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-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CA0833D962; Thu, 10 Oct 2019 16:43:09 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-48.rdu2.redhat.com [10.10.120.48]) by smtp.corp.redhat.com (Postfix) with ESMTP id 839BB6061E; Thu, 10 Oct 2019 16:43:08 +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> From: "Laszlo Ersek" Message-ID: <08f939d6-f980-aa4b-e867-dc719ec91ae1@redhat.com> Date: Thu, 10 Oct 2019 18:43:07 +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: <4A89E2EF3DFEDB4C8BFDE51014F606A14E51572A@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 10 Oct 2019 16:43:09 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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\SecureBootConfigDxe\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\SecureBootConfigDxe\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 = 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. Thanks Laszlo