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

[Andrew Fish <afish@apple.com>]

> 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