Re: [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Gao, Liming
> Sent: Friday, September 21, 2018 8:56 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry
> function run the same position
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191
> 
> Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp
> _SmiHandler
> is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved
> into rax,
> then call it. But, this code doesn't work in XCODE5 tool chain. Because
> XCODE5
> doesn't generated the absolute address in the EFI image. So, rax stores the
> relative address. Once this logic is moved to another place, it will not work.
> ;   jmp     _SmiHandler                 ; instruction is not needed
> ...
>     mov     rax, ASM_PFX(CpuSmmDebugEntry)
>     call    rax
> 
> Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support
> XCODE5.
> One tricky way is selected to fix it. Although SmiEntry logic is copied to
> another place and run, but here jmp _SmiHandler is enabled to jmp the
> original
> code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative
> address.
>     mov     rax, strict qword 0         ;   mov     rax, _SmiHandler
> _SmiHandlerAbsAddr:
>     jmp     rax
> ...
>     call    ASM_PFX(CpuSmmDebugEntry)
> 
> Now, BZ 1191 raises the issue that SmiHandler should run in the copied
> address,
> can't run in the common address. So, jmp _SmiHandler is required to be
> removed,
> the code is kept to run in copied address. And, the relative address is
> requried to be fixed up to the absolute address. The necessary changes
> should
> not affect the behavior of platforms that already consume
> PiSmmCpuDxeSmm.
> OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has
> been verified.
> ...
>     mov     rax, strict qword 0         ;   call
> ASM_PFX(CpuSmmDebugEntry)
> CpuSmmDebugEntryAbsAddr:
>     call    rax
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 42
> ++++++++++++++++++++++-------
>  1 file changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> index 315d0f8..815f95b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr:
>      mov     gs, eax
>      mov     ax, [rbx + DSC_SS]
>      mov     ss, eax
> -    mov     rax, strict qword 0         ;   mov     rax,
> _SmiHandler
> -_SmiHandlerAbsAddr:
> -    jmp     rax
> 
>  _SmiHandler:
>      mov     rbx, [rsp + 0x8]             ; rcx <- CpuIndex
> @@ -189,13 +186,19 @@ _SmiHandler:
>      add     rsp, -0x20
> 
>      mov     rcx, rbx
> -    call    ASM_PFX(CpuSmmDebugEntry)
> +    mov     rax, strict qword 0         ;   call
> ASM_PFX(CpuSmmDebugEntry)
> +CpuSmmDebugEntryAbsAddr:
> +    call    rax
> 
>      mov     rcx, rbx
> -    call    ASM_PFX(SmiRendezvous)
> +    mov     rax, strict qword 0         ;   call
> ASM_PFX(SmiRendezvous)
> +SmiRendezvousAbsAddr:
> +    call    rax
> 
>      mov     rcx, rbx
> -    call    ASM_PFX(CpuSmmDebugExit)
> +    mov     rax, strict qword 0         ;   call
> ASM_PFX(CpuSmmDebugExit)
> +CpuSmmDebugExitAbsAddr:
> +    call    rax
> 
>      add     rsp, 0x20
> 
> @@ -206,7 +209,8 @@ _SmiHandler:
> 
>      add     rsp, 0x200
> 
> -    lea     rax, [ASM_PFX(mXdSupported)]
> +    mov     rax, strict qword 0         ;       lea     rax,
> [ASM_PFX(mXdSupported)]
> +mXdSupportedAbsAddr:
>      mov     al, [rax]
>      cmp     al, 0
>      jz      .1
> @@ -224,13 +228,33 @@ _SmiHandler:
> 
>  ASM_PFX(gcSmiHandlerSize)    DW      $ - _SmiEntryPoint
> 
> +;
> +; Retrieve the address and fill it into mov opcode.
> +;
> +; It is called in the driver entry point first.
> +; It is used to fix up the real address in mov opcode.
> +; Then, after the code logic is copied to the different location,
> +; the code can also run.
> +;
>  global ASM_PFX(PiSmmCpuSmiEntryFixupAddress)
>  ASM_PFX(PiSmmCpuSmiEntryFixupAddress):
>      lea    rax, [ASM_PFX(gSmiHandlerIdtr)]
>      lea    rcx, [SmiHandlerIdtrAbsAddr]
>      mov    qword [rcx - 8], rax
> 
> -    lea    rax, [_SmiHandler]
> -    lea    rcx, [_SmiHandlerAbsAddr]
> +    lea    rax, [ASM_PFX(CpuSmmDebugEntry)]
> +    lea    rcx, [CpuSmmDebugEntryAbsAddr]
> +    mov    qword [rcx - 8], rax
> +
> +    lea    rax, [ASM_PFX(SmiRendezvous)]
> +    lea    rcx, [SmiRendezvousAbsAddr]
> +    mov    qword [rcx - 8], rax
> +
> +    lea    rax, [ASM_PFX(CpuSmmDebugExit)]
> +    lea    rcx, [CpuSmmDebugExitAbsAddr]
> +    mov    qword [rcx - 8], rax
> +
> +    lea    rax, [ASM_PFX(mXdSupported)]
> +    lea    rcx, [mXdSupportedAbsAddr]
>      mov    qword [rcx - 8], rax
>      ret
> --
> 2.10.0.windows.1