From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp01.apple.com (ma1-aaemail-dr-lapp01.apple.com [17.171.2.60]) by mx.groups.io with SMTP id smtpd.web12.127.1570724962325085310 for ; Thu, 10 Oct 2019 09:29:23 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=Hm62ui8j; spf=pass (domain: apple.com, ip: 17.171.2.60, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp01.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp01.apple.com (8.16.0.27/8.16.0.27) with SMTP id x9AGLxOx034840; Thu, 10 Oct 2019 09:29:20 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=YAIf5WylMgZMswkiJcrxLKS8T0wFFol+8eoT8SeQ8Lo=; b=Hm62ui8jjoE+scYC/BjY73qe9AKrQY3dI9SASPxURP8ATFaitXefFeKePmHTRVGudwNh gSNnL6dbP5qkhoFg8MWseraHHeV7wKEzQ79LuGxvBE6AhBcsXil7afXTx3zSOrgf3xH1 oPq2VPJuSAzrEOdAps5eXFtj3AUotZ2KSFYFgQGLzdARkofe7NPBcpgO0OTWZTN39vMG HTolrq+vgtcVgYgjC8LM2f4Be/1fk1n74a42ztlqNGpceeEK7GtgncOE7GRsOlonQjkO Y43HwPxKJMFmBuS585+W9HOussW9he1nCdf0lL/eP0bnp8U4IVT+di+LTDQKreCis5M9 yg== Received: from ma1-mtap-s01.corp.apple.com (ma1-mtap-s01.corp.apple.com [17.40.76.5]) by ma1-aaemail-dr-lapp01.apple.com with ESMTP id 2vesu6byqv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Thu, 10 Oct 2019 09:29:20 -0700 Received: from nwk-mmpp-sz13.apple.com (nwk-mmpp-sz13.apple.com [17.128.115.216]) by ma1-mtap-s01.corp.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPS id <0PZ600DVO34VRM80@ma1-mtap-s01.corp.apple.com>; Thu, 10 Oct 2019 09:29:20 -0700 (PDT) Received: from process_milters-daemon.nwk-mmpp-sz13.apple.com by nwk-mmpp-sz13.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) id <0PZ60090024B2L00@nwk-mmpp-sz13.apple.com>; Thu, 10 Oct 2019 09:29:19 -0700 (PDT) X-Va-A: X-Va-T-CD: 281f8fa03638b888ab0035439e9fe9bb X-Va-E-CD: 7089de0e78478859cd10a9847375a06a X-Va-R-CD: e4a2608e06c747242500bd6e6ccfdb6f X-Va-CD: 0 X-Va-ID: 85afad66-e27d-4ee1-b10a-88ce3ad5d63a X-V-A: X-V-T-CD: 281f8fa03638b888ab0035439e9fe9bb X-V-E-CD: 7089de0e78478859cd10a9847375a06a X-V-R-CD: e4a2608e06c747242500bd6e6ccfdb6f X-V-CD: 0 X-V-ID: 2db554a0-11a4-4540-8e9d-4931de527605 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-10-10_05:,, signatures=0 Received: from [17.235.61.64] (unknown [17.235.61.64]) by nwk-mmpp-sz13.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPSA id <0PZ6009PG34TGCE0@nwk-mmpp-sz13.apple.com>; Thu, 10 Oct 2019 09:29:19 -0700 (PDT) Sender: afish@apple.com From: "Andrew Fish" Message-id: <1F7679F4-4AE7-4795-9850-E4144B96B419@apple.com> MIME-version: 1.0 (Mac OS X Mail 13.0 \(3594.4.17\)) Subject: Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain Date: Thu, 10 Oct 2019 09:29:17 -0700 In-reply-to: <4A89E2EF3DFEDB4C8BFDE51014F606A14E51572A@SHSMSX104.ccr.corp.intel.com> Cc: "devel@edk2.groups.io" , "lersek@redhat.com" , Jordan Justen 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> <1e3de9da-1218-588a-4632-0263d6f64de5@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E51572A@SHSMSX104.ccr.corp.intel.com> X-Mailer: Apple Mail (2.3594.4.17) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-10-10_05:,, signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_E085E118-7D1C-43DD-B66F-A4F322614A54" --Apple-Mail=_E085E118-7D1C-43DD-B66F-A4F322614A54 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Liming, I'm OK with not regressing OVMF, I understand the concept of technical deb= t.=20 I'd point out that this is likely a generic issue with the SecurityPkg tha= t was worked around in OVMF.=20 Also it looks to me like passing those flags violate the calling conventio= n [1]. Maybe that is why the linker had an issue?=20 int drbg_add(const void *buf, int num, double randomness); >>From what I can tell without the flag randomness gets passed in XMM2, with= the flags it gets passed on the stack. Seems clang is not happy with that.= But the optimizer violates the calling conventions too, so as long as noth= ing is public with a floating point API it should not break anything.=20 Yikes I crashed clang passing in -mno-mmx -mno-sse. It might be a good ide= a to not pass -mno-mmx -mno-sse to clang in general for any target that is = x86_64-pc-win32. [1] UEFI Spec... Return values that fix into 64-bits are returned in the Rax register. If t= he return value does not fit within 64-bits, then the caller must allocate = and pass a pointer for the return value as the first argument, Rcx. Subsequ= ent arguments are then shifted one argument to the right, so for example ar= gument one would be passed in Rdx. User-defined types to be returned must b= e 1,2,4,8,16,32, or 64 bits in length. The registers Rax, Rcx Rdx R8, R9, R10, R11, and XMM0-XMM5 are volatile an= d are, therefore, destroyed on function calls. The registers RBX, RBP, RDI, RSI, R12, R13, R14, R15, and XMM6-XMM15 are c= onsidered nonvolatile and must be saved and restored by a function that use= s them. Function pointers are pointers to the label of the respective function and= don=E2=80=99t require special treatment. A caller must always call with th= e stack 16-byte aligned. For MMX, XMM and floating-point values, return values that can fit into 64= -bits are returned through RAX (including MMX types). However, XMM 128-bit = types, floats, and doubles are returned in XMM0. The floating point status = register is not saved by the target function. Floating-point and double-pre= cision arguments are passed in XMM0 - XMM3 (up to 4) with the integer slot = (RCX, RDX, R8, and R9) that would normally be used for that cardinal slot b= eing ignored (see example) and vice versa. XMM types are never passed by im= mediate value but rather a pointer will be passed to memory allocated by th= e caller. MMX types will be passed as if they were integers of the same siz= e. Callees must not unmask exceptions without providing correct exception h= andlers. In addition, unless otherwise specified by the function definition, all ot= her CPU registers (including MMX and XMM) are preserved. Thanks, Andrew Fish > On Oct 10, 2019, at 5:18 AM, Gao, Liming wrote: >=20 >=20 >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Laszlo E= rsek >> 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 cha= in - >>=20 >> Hi Andrew, >>=20 >> On 10/09/19 18:22, Andrew Fish wrote: >>=20 >>> 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. >>=20 >> The origin of those build flags in OVMF is commit 4a8266f570ef >> ("OvmfPkg: Work around issue seen with kvm + grub2 (efi)", 2010-12-31): >>=20 >> OvmfPkg: Work around issue seen with kvm + grub2 (efi) >>=20 >> When OVMF is run with kvm and grub2 (efi), an exception >> occurs when mmx/sse registers are accessed. >>=20 >> As a work around, this change eliminates firmware usage >> of these register types. >>=20 >> First, only the BaseMemoryLib implementation >> MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf >> is used. >>=20 >> Second, the GCC compiler is passes the additional >> '-mno-mmx -mno-sse' parameters. >>=20 >> The eternal problem with this kind of workaround is that we never know >> when it becomes safe to remove. >>=20 >> 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.) >=20 > Yes. I agree it is hard to decide to remove this option, because we don'= t know its impact.=20 > With the option -mno-mmx -mno-sse, it will cause CLANG9 linker failure l= ike below. So, I say=20 > this patch is to support the different linker. >=20 > 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=3D10 /ALIGN:32 /FILEALIGN:32 /SECTION= :.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT > RY:_ModuleEntryPoint /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BAS= E:0 /DEBUG:GHASH /lldmap @d:\allpkg\edk2\Build\O > vmf3264\DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBootCon= figDxe\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 >=20 > Thanks > Liming >>=20 >> Thanks >> Laszlo >>=20 >>=20 --Apple-Mail=_E085E118-7D1C-43DD-B66F-A4F322614A54 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Liming,
I'm OK with not regressing OVMF, I under= stand the concept of technical debt. 

I'd point out that this is likely a generic is= sue with the SecurityPkg that was worked around in OVMF. 

Also it looks to me like pass= ing those flags violate the calling convention [1]. Maybe that is why the l= inker had an issue? 
int drbg_add(const void *buf= , int num, double randomness);

From what I can tell without the flag randomness gets passed = in XMM2, with the flags it gets passed on the stack. Seems clang is not hap= py with that. But the optimizer violates the calling conventions too, so as= long as nothing is public with a floating point API it should not break an= ything. 

Yik= es I crashed clang passing in -mno-mmx -mno-sse. It might be a good id= ea to not pass -mno-mmx -mno-sse to clang in general for any target that is=  x86_64-pc-win32.

[1] UEFI Spec...
Retur= n values that fix into 64-bits are returned in the Rax register. If the ret= urn value does not fit within 64-bits, then the caller must allocate and pa= ss a pointer for the return value as the first argument, Rcx. Subsequent ar= guments are then shifted one argument to the right, so for example argument= one would be passed in Rdx. User-defined types to be returned must be 1,2,= 4,8,16,32, or 64 bits in length.
The registers Rax, Rc= x Rdx R8, R9, R10, R11, and XMM0-XMM5 are volatile and are, therefore, dest= royed on function calls.
The registers RBX, RBP, RDI, = RSI, R12, R13, R14, R15, and XMM6-XMM15 are considered nonvolatile and must= be saved and restored by a function that uses them.
F= unction pointers are pointers to the label of the respective function and d= on=E2=80=99t require special treatment. A caller must always call with the = stack 16-byte aligned.
For MMX, XMM and floating-point= values, return values that can fit into 64-bits are returned through RAX (= including MMX types). However, XMM 128-bit types, floats, and doubles are r= eturned in XMM0. The floating point status register is not saved by the tar= get function. Floating-point and double-precision arguments are passed in X= MM0 - XMM3 (up to 4) with the integer slot (RCX, RDX, R8, and R9) that woul= d normally be used for that cardinal slot being ignored (see example) and v= ice versa. XMM types are never passed by immediate value but rather a point= er will be passed to memory allocated by the caller. MMX types will be pass= ed as if they were integers of the same size. Callees must not unmask excep= tions without providing correct exception handlers.
In= addition, unless otherwise specified by the function definition, all other= CPU registers (including MMX and XMM) are preserved.

Thanks,
Andrew Fish

On Oct 10, 2019= , at 5:18 AM, Gao, Liming <liming.gao@intel.com> 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 <= ;afish@apple.com>; Gao= , Liming <liming.gao@= 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 Fi= sh wrote:

I thought the thing we were discussing was compiler flags.
S= pecifically -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 g= ets compiled [1].
it looks like -mno-mmx -mno-sse just down g= rade floating point
instructions that get used. Thus it seems= like either we have some
code doing float and that code shou= ld set -mno-mmx -mno-sse, or the
-mno-mmx -mno-sse should be = set generically.

The origin of th= ose build flags in OVMF is commit 4a8266f570ef
("OvmfPkg: Wor= k around issue seen with kvm + grub2 (efi)", 2010-12-31):
   OvmfPkg: Work around issue seen with kvm + gr= ub2 (efi)

   When OVMF is run w= ith kvm and grub2 (efi), an exception
   occur= s when mmx/sse registers are accessed.

 &= nbsp; As a work around, this change eliminates firmware usage
   of these register types.

   First, only the BaseMemoryLib implementation
   MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLib= RepStr.inf
   is used.

   Second, the GCC compiler is passes the additional<= br class=3D"">   '-mno-mmx -mno-sse' parameters.

The eternal problem with this kind of workaround is that = we never know
when it becomes safe to remove.
<= br class=3D"">It would be nice to understand the exact details around the p= roblem.
Given that the commit is from 2010, I assume the issu= e is difficult to
reproduce with recent components (KVM, grub= 2). On the other hand, if we
just remove the flags (which we = should, in a perfect world), someone
could report a bug in th= ree 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.&nbs= p;
With the opti= on -mno-mmx -mno-sse, it will cause CLANG9 linker failure like below. So, I= say 
this patch is to support the different lin= ker.

0.      Program arguments: C:= \Program Files\LLVM\bin\lld-link.EXE /OUT:d:\allpkg\edk2\Build\Ovmf3264\DEB= UG_CLANG9\X64\Se
cur= ityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigDxe\DEBUG\= SecureBootConfigDxe.dll /NOLOGO /NODEFAULT
LIB /IGNORE:4001 /OPT:REF /OPT:ICF=3D10 /ALIGN:32 /FILE= ALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /Machine:X64 /DLL /ENT<= br style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 1= 2px; font-style: normal; font-variant-caps: normal; font-weight: normal; le= tter-spacing: normal; text-align: start; text-indent: 0px; text-transform: = none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0p= x; text-decoration: none;" class=3D"">RY:_ModuleEntryPoint /SUBSYSTE= M:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DEBUG:GHASH /lldmap @d:\allp= kg\edk2\Build\O
vmf3264= \DEBUG_CLANG9\X64\SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\Sec= ureBootConfigDxe\OUTPUT\static_library
_files.lst
1.      Running pass 'Function Pass Manager= ' on module 'ld-temp.o'.
2.      Running pass 'X86 FP Stackifier' on func= tion '@drbg_add'
#0 = 0x00007ff696bad7f8 C:\Program Files\LLVM\bin\lld-link.EXE 0x148d7f8 C:\Prog= ram Files\LLVM\bin\lld-link.EXE 0x142ba7f

Thanks
Liming

Thanks
= Laszlo

<= br class=3D"">
--Apple-Mail=_E085E118-7D1C-43DD-B66F-A4F322614A54--