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

[Andrew Fish <afish@apple.com>]

Reviewed-by: Andrew Fish <afish@apple.com>

> On May 22, 2017, at 10:12 AM, Michael Kinney <michael.d.kinney@intel.com> wrote:
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=565
> 
> Fix NASM compatibility issues with XCODE5 tool chain.
> The XCODE5 tool chain for X64 builds using PIE (Position
> Independent Executable).  For most assembly sources using
> PIE mode does not cause any issues.
> 
> However, if assembly code is copied to a different address
> (such as AP startup code in the MpInitLib), then the
> X64 assembly source must be implemented to be compatible
> with PIE mode that uses RIP relative addressing.
> 
> The specific changes in this patch are:
> 
> * Use LEA instruction instead of MOV instruction to lookup
>  the addresses of functions.
> 
> * The assembly function RendezvousFunnelProc() is copied
>  below 1MB so it can be executed as part of the MpInitLib
>  AP startup sequence.  RendezvousFunnelProc() calls the
>  external function InitializeFloatingPointUnits().  The
>  absolute address of InitializeFloatingPointUnits() is
>  added to the MP_CPU_EXCHANGE_INFO structure that is passed
>  to RendezvousFunnelProc().
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.c          | 2 ++
> UefiCpuPkg/Library/MpInitLib/MpLib.h          | 1 +
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc    | 1 +
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 ++++----
> 4 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 407c44c..735e099 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -751,6 +751,8 @@ FillExchangeInfoData (
> 
>   ExchangeInfo->EnableExecuteDisable = IsBspExecuteDisableEnabled ();
> 
> +  ExchangeInfo->InitializeFloatingPointUnitsAddress = (UINTN)InitializeFloatingPointUnits;
> +
>   //
>   // Get the BSP's data of GDT and IDT
>   //
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 989b3f8..ea56412 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -177,6 +177,7 @@ typedef struct {
>   UINTN                 InitFlag;
>   CPU_INFO_IN_HOB       *CpuInfo;
>   CPU_MP_DATA           *CpuMpData;
> +  UINTN                 InitializeFloatingPointUnitsAddress;
> } MP_CPU_EXCHANGE_INFO;
> 
> #pragma pack()
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> index a63cd23..852281a 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> @@ -40,4 +40,5 @@ EnableExecuteDisableLocation  equ        LockLocation + 5Ch
> Cr3Location                   equ        LockLocation + 64h
> InitFlagLocation              equ        LockLocation + 6Ch
> CpuInfoLocation               equ        LockLocation + 74h
> +InitializeFloatingPointUnitsAddress equ  LockLocation + 84h
> 
> 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]
>     sub        rsp, 20h
>     call       rax               ; Call assembly function to initialize FPU per UEFI spec
>     add        rsp, 20h
> @@ -282,11 +282,11 @@ AsmRelocateApLoopEnd:
> ;-------------------------------------------------------------------------------------
> global ASM_PFX(AsmGetAddressMap)
> ASM_PFX(AsmGetAddressMap):
> -    mov        rax, ASM_PFX(RendezvousFunnelProc)
> +    lea        rax, [ASM_PFX(RendezvousFunnelProc)]
>     mov        qword [rcx], rax
>     mov        qword [rcx +  8h], LongModeStart - RendezvousFunnelProcStart
>     mov        qword [rcx + 10h], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> -    mov        rax, ASM_PFX(AsmRelocateApLoop)
> +    lea        rax, [ASM_PFX(AsmRelocateApLoop)]
>     mov        qword [rcx + 18h], rax
>     mov        qword [rcx + 20h], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
>     ret
> -- 
> 2.6.3.windows.1
>