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

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Jeff,

I agree.  I thought of a 2nd method to pass the address through the 
structure that is shared between the C and assembly code.  I will
work on a 2nd version of the patch that is both XIP and PIE compatible.

Mike

> -----Original Message-----
> From: Fan, Jeff
> Sent: Sunday, May 21, 2017 8:10 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> Cc: Andrew Fish <afish@apple.com>
> Subject: RE: [Patch] UefiCpuPkg/MpInitLib: Fix X64 XCODE5/NASM compatibility issues
> 
> Mike,
> 
> This piece of code will be shared for CPU PEI/DXE MP driver. If PEI is 64bit, this
> piece of code maybe located on FLASH.
> 
> We cannot change the value in InitialzeFloatingPointUnitsAddress if the code located
> on FALSH in AsmGetAddressMap().
> 
> We need to update the InitialzeFloatingPointUnitsAddress after this piece of code
> copied into AP reset vector in memory
> in BackupAndPrepareWakeupBuffer().
> 
> Thanks!
> Jeff
> 
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Monday, May 22, 2017 3:05 AM
> To: edk2-devel@lists.01.org
> Cc: Andrew Fish; Fan, Jeff; Kinney, Michael D
> Subject: [Patch] UefiCpuPkg/MpInitLib: Fix X64 XCODE5/NASM compatibility issues
> 
> 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() must
>   resolved and saved to a data element that is part of
>   RendezvousFunnelProc()  before RendezvousFunnelProc() is
>   copied below 1MB.  This work is done in AsmGetAddressMap().
> 
> 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/X64/MpFuncs.nasm | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index fa54d01..c943a09 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,8 @@
> CProcedureInvoke:
>      push       rbp
>      mov        rbp, rsp
> 
> -    mov        rax, ASM_PFX(InitializeFloatingPointUnits)
> +    lea        rax, [InitialzeFloatingPointUnitsAddress]
> +    mov        rax, qword [rax]
>      sub        rsp, 20h
>      call       rax               ; Call assembly function to initialize FPU per UEFI
> spec
>      add        rsp, 20h
> @@ -219,6 +220,10 @@ CProcedureInvoke:
>      add        rsp, 20h
>      jmp        $                 ; Should never reach here
> 
> +InitialzeFloatingPointUnitsAddress:
> +    DQ 0                         ; Provide storage for absolute adddress of
> +                                 ; the InitializeFloatingPointUnits()
> +function
> +
>  RendezvousFunnelProcEnd:
> 
>  ;------------------------------------------------------------------------------------
> -
> @@ -282,11 +287,18 @@ AsmRelocateApLoopEnd:
>  ;------------------------------------------------------------------------------------
> -
>  global ASM_PFX(AsmGetAddressMap)
>  ASM_PFX(AsmGetAddressMap):
> -    mov        rax, ASM_PFX(RendezvousFunnelProc)
> +    ; Save absolute address of InitializeFloatingPointUnits() in data element
> +    ; within the RendezvousFunnelProc template.  This provides the address of
> +    ; the InitializeFloatingPointUnits() function to the RendezvousFunnelProc
> +    ; after it has been copied below 1MB
> +    lea        rax, [ASM_PFX(InitializeFloatingPointUnits)]
> +    mov        qword [InitialzeFloatingPointUnitsAddress], rax
> +
> +    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