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

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

[Michael Kinney]

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

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

[Fan, Jeff]

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

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

[Kinney, Michael D]

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