Re: [Patch V2] UefiCpuPkg/MpInitLib: Fix X64 XCODE5/NASM compatibility issues

[hpa@zytor.com]

On June 6, 2017 2:15:08 PM PDT, Andrew Fish <afish@apple.com> wrote:
>
>> On Jun 6, 2017, at 2:05 PM, hpa@zytor.com wrote:
>> 
>> On June 6, 2017 1:49:34 PM PDT, Andrew Fish <afish@apple.com> wrote:
>>> 
>>>> On Jun 6, 2017, at 12:41 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>> 
>>>> On 05/22/17 19:08, Fan, Jeff wrote:
>>>>> 
>>>>> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>>> b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>>>>> index fa54d01..0b14a53 100644
>>>>> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>>>>> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>>>>> @@ -1,5 +1,5 @@
>>>>> 
>>>
>;------------------------------------------------------------------------------
>>> ; -; Copyright (c) 2015 - 2016, Intel Corporation. All rights
>>> reserved.<BR>
>>>>> +; Copyright (c) 2015 - 2017, Intel Corporation. All rights 
>>>>> +reserved.<BR>
>>>>> ; This program and the accompanying materials  ; are licensed and
>>> made available under the terms and conditions of the BSD License  ;
>>> which accompanies this distribution.  The full text of the license
>may
>>> be found at @@ -201,7 +201,7 @@ CProcedureInvoke:
>>>>>    push       rbp
>>>>>    mov        rbp, rsp
>>>>> 
>>>>> -    mov        rax, ASM_PFX(InitializeFloatingPointUnits)
>>>>> +    mov        rax, qword [esi +
>>> InitializeFloatingPointUnitsAddress]
>>> 
>>> Does nasm remove the need for the ASM_PFX() macro? That macro hides
>if
>>> C is decorating with a _ prefix. 
>>> 
>>> Also given it is a #define (equ) why do we use camel case vs. all
>caps?
>>> 
>>> Thanks,
>>> 
>>> Andrew Fish
>>> 
>>>>>    sub        rsp, 20h
>>>>>    call       rax               ; Call assembly function to
>>> initialize FPU per UEFI spec
>>>>>    add        rsp, 20h
>>>> 
>>>> FYI, the qword specifier is unnecessary since you are already
>>> specifying
>>>> rax.
>>>> 
>>>> However, why not simply drop the use of rax entirely and do:
>>>> 
>>>> 	call [esi + InitializeFloatingPointUnitsAddress]
>>>> 
>>>> (Also: is this *really* supposed to be esi and not rsi?  The former
>>>> means a 32-bit address.)
>>>> 
>>>> 	-hpa
>>>> _______________________________________________
>>>> edk2-devel mailing list
>>>> edk2-devel@lists.01.org
>>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> 
>> The NASM command-line option --prefix _ should do exactly that.
>
>How does it know when to prefix the _? That could break dead stripping
>with the Xcode linker. L is used to imply local symbol, don't dead
>strip. 
>
>Thanks,
>
>Andrew Fish
>
>> -- 
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.

It appends the underscore to global or external variables.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.