Re: [edk2-devel] [Patch 11/12] OvmfPkg: Enable CLANG9 tool chain

["Andrew Fish" <afish@apple.com>]

[-- Attachment #1: Type: text/plain, Size: 6381 bytes --]

Liming,

I'm OK with not regressing OVMF, I understand the concept of technical debt. 

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

Also it looks to me like passing those flags violate the calling convention [1]. Maybe that is why the linker 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 happy 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 anything. 

Yikes I crashed clang passing in -mno-mmx -mno-sse. It might be a good idea 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 the 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. Subsequent arguments 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, Rcx Rdx R8, R9, R10, R11, and XMM0-XMM5 are volatile and are, therefore, destroyed 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.
Function pointers are pointers to the label of the respective function and don’t 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 returned in XMM0. The floating point status register is not saved by the target function. Floating-point and double-precision 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 being ignored (see example) and vice versa. XMM types are never passed by immediate value but rather a pointer will be passed to memory allocated by the caller. MMX types will be passed as if they were integers of the same size. Callees must not unmask exceptions 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 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
> 
> Thanks
> Liming
>> 
>> Thanks
>> Laszlo
>> 
>> 


[-- Attachment #2: Type: text/html, Size: 20931 bytes --]