Laszlo: >-----Original Message----- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Friday, October 11, 2019 12:43 AM >To: devel@edk2.groups.io; Gao, Liming <liming....@intel.com>; Andrew Fish ><af...@apple.com> >Cc: Justen, Jordan L <jordan.l.jus...@intel.com> >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 <devel@edk2.groups.io> On Behalf Of Laszlo >Ersek >>> Sent: Thursday, October 10, 2019 3:35 PM >>> To: Andrew Fish <af...@apple.com>; Gao, Liming <liming....@intel.com> >>> 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. Thanks Liming >Thanks >Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48772): https://edk2.groups.io/g/devel/message/48772 Mute This Topic: https://groups.io/mt/34464936/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
