On 10/11/19 03:47, Liming Gao wrote: > 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.
OK. With the commit message updated, for this patch: Reviewed-by: Laszlo Ersek <ler...@redhat.com> For later, please consider the GENUINE_GCC suggestion (for BUILDRULEFAMILY) in my other message in the present thread: 43b303eb-fd74-7480-aa6d-6907300af3e0@redhat.com">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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48811): https://edk2.groups.io/g/devel/message/48811 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] -=-=-=-=-=-=-=-=-=-=-=-
