From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, ray.ni@intel.com
Cc: Eric Dong <eric.dong@intel.com>, Rahul Kumar <rahul1.kumar@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
Date: Fri, 5 Feb 2021 16:09:55 +0100 [thread overview]
Message-ID: <6da20851-2614-8ee6-ff32-479d9603305b@redhat.com> (raw)
In-Reply-To: <20210205075810.981-3-ray.ni@intel.com>
On 02/05/21 08:58, Ni, Ray wrote:
> In Windows environment, "dumpbin /disasm" is used to verify the
> disassembly before and after using NASM struc doesn't change.
>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 --------
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 82 +++++++-------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 103 ++++++++++++++++++
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 94 ++++++++--------
> 7 files changed, 195 insertions(+), 182 deletions(-)
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> index 1771575c69..860a9750e2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> @@ -1,7 +1,7 @@
> ## @file
> # MP Initialize Library instance for DXE driver.
> #
> -# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -22,14 +22,13 @@ [Defines]
> #
>
> [Sources.IA32]
> - Ia32/MpEqu.inc
> Ia32/MpFuncs.nasm
>
> [Sources.X64]
> - X64/MpEqu.inc
> X64/MpFuncs.nasm
>
> [Sources.common]
> + MpEqu.inc
> DxeMpLib.c
> MpLib.c
> MpLib.h
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> deleted file mode 100644
> index 4f5a7c859a..0000000000
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> -; SPDX-License-Identifier: BSD-2-Clause-Patent
> -;
> -; Module Name:
> -;
> -; MpEqu.inc
> -;
> -; Abstract:
> -;
> -; This is the equates file for Multiple Processor support
> -;
> -;-------------------------------------------------------------------------------
> -
> -VacantFlag equ 00h
> -NotVacantFlag equ 0ffh
> -
> -CPU_SWITCH_STATE_IDLE equ 0
> -CPU_SWITCH_STATE_STORED equ 1
> -CPU_SWITCH_STATE_LOADED equ 2
> -
> -LockLocation equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
> -StackStartAddressLocation equ LockLocation + 04h
> -StackSizeLocation equ LockLocation + 08h
> -ApProcedureLocation equ LockLocation + 0Ch
> -GdtrLocation equ LockLocation + 10h
> -IdtrLocation equ LockLocation + 16h
> -BufferStartLocation equ LockLocation + 1Ch
> -ModeOffsetLocation equ LockLocation + 20h
> -ApIndexLocation equ LockLocation + 24h
> -CodeSegmentLocation equ LockLocation + 28h
> -DataSegmentLocation equ LockLocation + 2Ch
> -EnableExecuteDisableLocation equ LockLocation + 30h
> -Cr3Location equ LockLocation + 34h
> -InitFlagLocation equ LockLocation + 38h
> -CpuInfoLocation equ LockLocation + 3Ch
> -NumApsExecutingLocation equ LockLocation + 40h
> -InitializeFloatingPointUnitsAddress equ LockLocation + 48h
> -ModeTransitionMemoryLocation equ LockLocation + 4Ch
> -ModeTransitionSegmentLocation equ LockLocation + 50h
> -ModeHighMemoryLocation equ LockLocation + 52h
> -ModeHighSegmentLocation equ LockLocation + 56h
> -
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index 7e81d24aa6..2f1b102717 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -1,5 +1,5 @@
> ;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> ; SPDX-License-Identifier: BSD-2-Clause-Patent
> ;
> ; Module Name:
> @@ -39,21 +39,21 @@ BITS 16
> mov fs, ax
> mov gs, ax
>
> - mov si, BufferStartLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (BufferStart)
> mov ebx, [si]
>
> - mov si, DataSegmentLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (DataSegment)
> mov edx, [si]
>
> ;
> ; Get start address of 32-bit code in low memory (<1MB)
> ;
> - mov edi, ModeTransitionMemoryLocation
> + mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeTransitionMemory)
>
> - mov si, GdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
> o32 lgdt [cs:si]
>
> - mov si, IdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
> o32 lidt [cs:si]
>
> ;
> @@ -82,7 +82,7 @@ Flat32Start: ; protected mode entry point
> mov esi, ebx
>
> mov edi, esi
> - add edi, EnableExecuteDisableLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (EnableExecuteDisable)
> cmp byte [edi], 0
> jz SkipEnableExecuteDisable
>
> @@ -96,7 +96,7 @@ Flat32Start: ; protected mode entry point
> wrmsr
>
> mov edi, esi
> - add edi, Cr3Location
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (Cr3)
> mov eax, dword [edi]
> mov cr3, eax
>
> @@ -110,19 +110,19 @@ Flat32Start: ; protected mode entry point
>
> SkipEnableExecuteDisable:
> mov edi, esi
> - add edi, InitFlagLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)
> cmp dword [edi], 1 ; 1 == ApInitConfig
> jnz GetApicId
>
> ; Increment the number of APs executing here as early as possible
> ; This is decremented in C code when AP is finished executing
> mov edi, esi
> - add edi, NumApsExecutingLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (NumApsExecuting)
> lock inc dword [edi]
>
> ; AP init
> mov edi, esi
> - add edi, LockLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
> mov eax, NotVacantFlag
>
> TestLock:
> @@ -131,7 +131,7 @@ TestLock:
> jz TestLock
>
> mov ecx, esi
> - add ecx, ApIndexLocation
> + add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
> inc dword [ecx]
> mov ebx, [ecx]
>
> @@ -140,13 +140,13 @@ Releaselock:
> xchg [edi], eax
>
> mov edi, esi
> - add edi, StackSizeLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
> mov eax, [edi]
> mov ecx, ebx
> inc ecx
> mul ecx ; EAX = StackSize * (CpuNumber + 1)
> mov edi, esi
> - add edi, StackStartAddressLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackStart)
> add eax, [edi]
> mov esp, eax
> jmp CProcedureInvoke
> @@ -179,18 +179,18 @@ GetProcessorNumber:
> ; Note that BSP may become an AP due to SwitchBsp()
> ;
> xor ebx, ebx
> - lea eax, [esi + CpuInfoLocation]
> + lea eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuInfo)]
> mov edi, [eax]
>
> GetNextProcNumber:
> - cmp [edi], edx ; APIC ID match?
> + cmp dword [edi + CPU_INFO_IN_HOB.InitialApicId], edx ; APIC ID match?
> jz ProgramStack
> - add edi, 20
> + add edi, CPU_INFO_IN_HOB_size
> inc ebx
> jmp GetNextProcNumber
>
> ProgramStack:
> - mov esp, [edi + 12]
> + mov esp, dword [edi + CPU_INFO_IN_HOB.ApTopOfStack]
>
> CProcedureInvoke:
> push ebp ; push BIST data at top of AP stack
> @@ -203,11 +203,11 @@ CProcedureInvoke:
>
> push ebx ; Push ApIndex
> mov eax, esi
> - add eax, LockLocation
> + add eax, MP_CPU_EXCHANGE_INFO_OFFSET
> push eax ; push address of exchange info data buffer
>
> mov edi, esi
> - add edi, ApProcedureLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
> mov eax, [edi]
>
> call eax ; Invoke C function
> @@ -270,17 +270,17 @@ ASM_PFX(AsmGetAddressMap):
> mov ebp,esp
>
> mov ebx, [ebp + 24h]
> - mov dword [ebx], RendezvousFunnelProcStart
> - mov dword [ebx + 4h], Flat32Start - RendezvousFunnelProcStart
> - mov dword [ebx + 8h], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> - mov dword [ebx + 0Ch], AsmRelocateApLoopStart
> - mov dword [ebx + 10h], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> - mov dword [ebx + 14h], Flat32Start - RendezvousFunnelProcStart
> - mov dword [ebx + 18h], SwitchToRealProcEnd - SwitchToRealProcStart ; SwitchToRealSize
> - mov dword [ebx + 1Ch], SwitchToRealProcStart - RendezvousFunnelProcStart ; SwitchToRealOffset
> - mov dword [ebx + 20h], SwitchToRealProcStart - Flat32Start ; SwitchToRealNoNxOffset
> - mov dword [ebx + 24h], 0 ; SwitchToRealPM16ModeOffset
> - mov dword [ebx + 28h], 0 ; SwitchToRealPM16ModeSize
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelAddress], RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.ModeEntryOffset], Flat32Start - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelSize], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncAddress], AsmRelocateApLoopStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncSize], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.ModeTransitionOffset], Flat32Start - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealSize], SwitchToRealProcEnd - SwitchToRealProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealOffset], SwitchToRealProcStart - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealNoNxOffset], SwitchToRealProcStart - Flat32Start
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeOffset], 0
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeSize], 0
>
> popad
> ret
> @@ -310,18 +310,18 @@ ASM_PFX(AsmExchangeRole):
> mov eax, cr0
> push eax
>
> - sgdt [esi + 8]
> - sidt [esi + 14]
> + sgdt [esi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
> + sidt [esi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; Store the its StackPointer
> - mov [esi + 4],esp
> + mov [esi + CPU_EXCHANGE_ROLE_INFO.StackPointer],esp
>
> ; update its switch state to STORED
> - mov byte [esi], CPU_SWITCH_STATE_STORED
> + mov byte [esi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
>
> WaitForOtherStored:
> ; wait until the other CPU finish storing its state
> - cmp byte [edi], CPU_SWITCH_STATE_STORED
> + cmp byte [edi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
> jz OtherStored
> pause
> jmp WaitForOtherStored
> @@ -329,21 +329,21 @@ WaitForOtherStored:
> OtherStored:
> ; Since another CPU already stored its state, load them
> ; load GDTR value
> - lgdt [edi + 8]
> + lgdt [edi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
>
> ; load IDTR value
> - lidt [edi + 14]
> + lidt [edi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; load its future StackPointer
> - mov esp, [edi + 4]
> + mov esp, [edi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
>
> ; update the other CPU's switch state to LOADED
> - mov byte [edi], CPU_SWITCH_STATE_LOADED
> + mov byte [edi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
>
> WaitForOtherLoaded:
> ; wait until the other CPU finish loading new state,
> ; otherwise the data in stack may corrupt
> - cmp byte [esi], CPU_SWITCH_STATE_LOADED
> + cmp byte [esi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
> jz OtherLoaded
> pause
> jmp WaitForOtherLoaded
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> new file mode 100644
> index 0000000000..46c2b5c116
> --- /dev/null
> +++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> @@ -0,0 +1,103 @@
> +;------------------------------------------------------------------------------ ;
> +; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +; Module Name:
> +;
> +; MpEqu.inc
> +;
> +; Abstract:
> +;
> +; This is the equates file for Multiple Processor support
> +;
> +;-------------------------------------------------------------------------------
> +%include "Nasm.inc"
> +
> +VacantFlag equ 00h
> +NotVacantFlag equ 0ffh
> +
> +CPU_SWITCH_STATE_IDLE equ 0
> +CPU_SWITCH_STATE_STORED equ 1
> +CPU_SWITCH_STATE_LOADED equ 2
> +
> +;
> +; Equivalent NASM structure of MP_ASSEMBLY_ADDRESS_MAP
> +;
> +struc MP_ASSEMBLY_ADDRESS_MAP
> + .RendezvousFunnelAddress CTYPE_UINTN 1
> + .ModeEntryOffset CTYPE_UINTN 1
> + .RendezvousFunnelSize CTYPE_UINTN 1
> + .RelocateApLoopFuncAddress CTYPE_UINTN 1
> + .RelocateApLoopFuncSize CTYPE_UINTN 1
> + .ModeTransitionOffset CTYPE_UINTN 1
> + .SwitchToRealSize CTYPE_UINTN 1
> + .SwitchToRealOffset CTYPE_UINTN 1
> + .SwitchToRealNoNxOffset CTYPE_UINTN 1
> + .SwitchToRealPM16ModeOffset CTYPE_UINTN 1
> + .SwitchToRealPM16ModeSize CTYPE_UINTN 1
> +endstruc
> +
> +;
> +; Equivalent NASM structure of IA32_DESCRIPTOR
> +;
> +struc IA32_DESCRIPTOR
> + .Limit CTYPE_UINT16 1
> + .Base CTYPE_UINTN 1
> +endstruc
> +
> +;
> +; Equivalent NASM structure of CPU_EXCHANGE_ROLE_INFO
> +;
> +struc CPU_EXCHANGE_ROLE_INFO
> + ; State is defined as UINT8 in C header file
> + ; Define it as UINTN here to guarantee the fields that follow State
> + ; is naturally aligned. The structure layout doesn't change.
> + .State CTYPE_UINTN 1
> + .StackPointer CTYPE_UINTN 1
> + .Gdtr CTYPE_UINT8 IA32_DESCRIPTOR_size
> + .Idtr CTYPE_UINT8 IA32_DESCRIPTOR_size
> +endstruc
> +
> +;
> +; Equivalent NASM structure of CPU_INFO_IN_HOB
> +;
> +struc CPU_INFO_IN_HOB
> + .InitialApicId CTYPE_UINT32 1
> + .ApicId CTYPE_UINT32 1
> + .Health CTYPE_UINT32 1
> + .ApTopOfStack CTYPE_UINT64 1
> +endstruc
> +
> +;
> +; Equivalent NASM structure of MP_CPU_EXCHANGE_INFO
> +;
> +struc MP_CPU_EXCHANGE_INFO
> + .Lock: CTYPE_UINTN 1
> + .StackStart: CTYPE_UINTN 1
> + .StackSize: CTYPE_UINTN 1
> + .CFunction: CTYPE_UINTN 1
> + .GdtrProfile: CTYPE_UINT8 IA32_DESCRIPTOR_size
> + .IdtrProfile: CTYPE_UINT8 IA32_DESCRIPTOR_size
> + .BufferStart: CTYPE_UINTN 1
> + .ModeOffset: CTYPE_UINTN 1
> + .ApIndex: CTYPE_UINTN 1
> + .CodeSegment: CTYPE_UINTN 1
> + .DataSegment: CTYPE_UINTN 1
> + .EnableExecuteDisable: CTYPE_UINTN 1
> + .Cr3: CTYPE_UINTN 1
> + .InitFlag: CTYPE_UINTN 1
> + .CpuInfo: CTYPE_UINTN 1
> + .NumApsExecuting: CTYPE_UINTN 1
> + .CpuMpData: CTYPE_UINTN 1
> + .InitializeFloatingPointUnits: CTYPE_UINTN 1
> + .ModeTransitionMemory: CTYPE_UINT32 1
> + .ModeTransitionSegment: CTYPE_UINT16 1
> + .ModeHighMemory: CTYPE_UINT32 1
> + .ModeHighSegment: CTYPE_UINT16 1
> + .Enable5LevelPaging: CTYPE_BOOLEAN 1
> + .SevEsIsEnabled: CTYPE_BOOLEAN 1
> + .GhcbBase: CTYPE_UINTN 1
> +endstruc
> +
> +MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
> +%define MP_CPU_EXCHANGE_INFO_FIELD(Field) (MP_CPU_EXCHANGE_INFO_OFFSET + MP_CPU_EXCHANGE_INFO. %+ Field)
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> index 34abf25d43..49b0ffe8be 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> @@ -1,7 +1,7 @@
> ## @file
> # MP Initialize Library instance for PEI driver.
> #
> -# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -22,14 +22,13 @@ [Defines]
> #
>
> [Sources.IA32]
> - Ia32/MpEqu.inc
> Ia32/MpFuncs.nasm
>
> [Sources.X64]
> - X64/MpEqu.inc
> X64/MpFuncs.nasm
>
> [Sources.common]
> + MpEqu.inc
> PeiMpLib.c
> MpLib.c
> MpLib.h
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> deleted file mode 100644
> index c92daaaffd..0000000000
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> -; SPDX-License-Identifier: BSD-2-Clause-Patent
> -;
> -; Module Name:
> -;
> -; MpEqu.inc
> -;
> -; Abstract:
> -;
> -; This is the equates file for Multiple Processor support
> -;
> -;-------------------------------------------------------------------------------
> -
> -VacantFlag equ 00h
> -NotVacantFlag equ 0ffh
> -
> -CPU_SWITCH_STATE_IDLE equ 0
> -CPU_SWITCH_STATE_STORED equ 1
> -CPU_SWITCH_STATE_LOADED equ 2
> -
> -LockLocation equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
> -StackStartAddressLocation equ LockLocation + 08h
> -StackSizeLocation equ LockLocation + 10h
> -ApProcedureLocation equ LockLocation + 18h
> -GdtrLocation equ LockLocation + 20h
> -IdtrLocation equ LockLocation + 2Ah
> -BufferStartLocation equ LockLocation + 34h
> -ModeOffsetLocation equ LockLocation + 3Ch
> -ApIndexLocation equ LockLocation + 44h
> -CodeSegmentLocation equ LockLocation + 4Ch
> -DataSegmentLocation equ LockLocation + 54h
> -EnableExecuteDisableLocation equ LockLocation + 5Ch
> -Cr3Location equ LockLocation + 64h
> -InitFlagLocation equ LockLocation + 6Ch
> -CpuInfoLocation equ LockLocation + 74h
> -NumApsExecutingLocation equ LockLocation + 7Ch
> -InitializeFloatingPointUnitsAddress equ LockLocation + 8Ch
> -ModeTransitionMemoryLocation equ LockLocation + 94h
> -ModeTransitionSegmentLocation equ LockLocation + 98h
> -ModeHighMemoryLocation equ LockLocation + 9Ah
> -ModeHighSegmentLocation equ LockLocation + 9Eh
> -Enable5LevelPagingLocation equ LockLocation + 0A0h
> -SevEsIsEnabledLocation equ LockLocation + 0A1h
> -GhcbBaseLocation equ LockLocation + 0A2h
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index aecfd07bc0..bf7faaf60b 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -1,5 +1,5 @@
> ;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> ; SPDX-License-Identifier: BSD-2-Clause-Patent
> ;
> ; Module Name:
> @@ -43,21 +43,21 @@ BITS 16
> mov fs, ax
> mov gs, ax
>
> - mov si, BufferStartLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (BufferStart)
> mov ebx, [si]
>
> - mov si, DataSegmentLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (DataSegment)
> mov edx, [si]
>
> ;
> ; Get start address of 32-bit code in low memory (<1MB)
> ;
> - mov edi, ModeTransitionMemoryLocation
> + mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeTransitionMemory)
>
> - mov si, GdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
> o32 lgdt [cs:si]
>
> - mov si, IdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
> o32 lidt [cs:si]
>
> ;
> @@ -85,7 +85,7 @@ Flat32Start: ; protected mode entry point
> ;
> ; Enable execute disable bit
> ;
> - mov esi, EnableExecuteDisableLocation
> + mov esi, MP_CPU_EXCHANGE_INFO_FIELD (EnableExecuteDisable)
> cmp byte [ebx + esi], 0
> jz SkipEnableExecuteDisableBit
>
> @@ -101,7 +101,7 @@ SkipEnableExecuteDisableBit:
> mov eax, cr4
> bts eax, 5
>
> - mov esi, Enable5LevelPagingLocation
> + mov esi, MP_CPU_EXCHANGE_INFO_FIELD (Enable5LevelPaging)
> cmp byte [ebx + esi], 0
> jz SkipEnable5LevelPaging
>
> @@ -117,7 +117,7 @@ SkipEnable5LevelPaging:
> ;
> ; Load page table
> ;
> - mov esi, Cr3Location ; Save CR3 in ecx
> + mov esi, MP_CPU_EXCHANGE_INFO_FIELD (Cr3) ; Save CR3 in ecx
> mov ecx, [ebx + esi]
> mov cr3, ecx ; Load CR3
>
> @@ -139,26 +139,26 @@ SkipEnable5LevelPaging:
> ;
> ; Far jump to 64-bit code
> ;
> - mov edi, ModeHighMemoryLocation
> + mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeHighMemory)
> add edi, ebx
> jmp far [edi]
>
> BITS 64
> LongModeStart:
> mov esi, ebx
> - lea edi, [esi + InitFlagLocation]
> + lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
> cmp qword [edi], 1 ; ApInitConfig
> jnz GetApicId
>
> ; Increment the number of APs executing here as early as possible
> ; This is decremented in C code when AP is finished executing
> mov edi, esi
> - add edi, NumApsExecutingLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (NumApsExecuting)
> lock inc dword [edi]
>
> ; AP init
> mov edi, esi
> - add edi, LockLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
> mov rax, NotVacantFlag
>
> TestLock:
> @@ -166,7 +166,7 @@ TestLock:
> cmp rax, NotVacantFlag
> jz TestLock
>
> - lea ecx, [esi + ApIndexLocation]
> + lea ecx, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)]
> inc dword [ecx]
> mov ebx, [ecx]
>
> @@ -175,17 +175,17 @@ Releaselock:
> xchg qword [edi], rax
> ; program stack
> mov edi, esi
> - add edi, StackSizeLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
> mov eax, dword [edi]
> mov ecx, ebx
> inc ecx
> mul ecx ; EAX = StackSize * (CpuNumber + 1)
> mov edi, esi
> - add edi, StackStartAddressLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackStart)
> add rax, qword [edi]
> mov rsp, rax
>
> - lea edi, [esi + SevEsIsEnabledLocation]
> + lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
> cmp byte [edi], 1 ; SevEsIsEnabled
> jne CProcedureInvoke
>
> @@ -199,7 +199,7 @@ Releaselock:
> mov ecx, ebx
> mul ecx ; EAX = SIZE_4K * 2 * CpuNumber
> mov edi, esi
> - add edi, GhcbBaseLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (GhcbBase)
> add rax, qword [edi]
> mov rdx, rax
> shr rdx, 32
> @@ -208,7 +208,7 @@ Releaselock:
> jmp CProcedureInvoke
>
> GetApicId:
> - lea edi, [esi + SevEsIsEnabledLocation]
> + lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
> cmp byte [edi], 1 ; SevEsIsEnabled
> jne DoCpuid
>
> @@ -302,18 +302,18 @@ GetProcessorNumber:
> ; Note that BSP may become an AP due to SwitchBsp()
> ;
> xor ebx, ebx
> - lea eax, [esi + CpuInfoLocation]
> + lea eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuInfo)]
> mov rdi, [eax]
>
> GetNextProcNumber:
> - cmp dword [rdi], edx ; APIC ID match?
> + cmp dword [rdi + CPU_INFO_IN_HOB.InitialApicId], edx ; APIC ID match?
> jz ProgramStack
> - add rdi, 20
> + add rdi, CPU_INFO_IN_HOB_size
> inc ebx
> jmp GetNextProcNumber
>
> ProgramStack:
> - mov rsp, qword [rdi + 12]
> + mov rsp, qword [rdi + CPU_INFO_IN_HOB.ApTopOfStack]
>
> CProcedureInvoke:
> push rbp ; Push BIST data at top of AP stack
> @@ -321,17 +321,17 @@ CProcedureInvoke:
> push rbp
> mov rbp, rsp
>
> - mov rax, qword [esi + InitializeFloatingPointUnitsAddress]
> + mov rax, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitializeFloatingPointUnits)]
> sub rsp, 20h
> call rax ; Call assembly function to initialize FPU per UEFI spec
> add rsp, 20h
>
> mov edx, ebx ; edx is ApIndex
> mov ecx, esi
> - add ecx, LockLocation ; rcx is address of exchange info data buffer
> + add ecx, MP_CPU_EXCHANGE_INFO_OFFSET ; rcx is address of exchange info data buffer
>
> mov edi, esi
> - add edi, ApProcedureLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
> mov rax, qword [edi]
>
> sub rsp, 20h
> @@ -667,18 +667,18 @@ AsmRelocateApLoopEnd:
> global ASM_PFX(AsmGetAddressMap)
> ASM_PFX(AsmGetAddressMap):
> lea rax, [ASM_PFX(RendezvousFunnelProc)]
> - mov qword [rcx], rax
> - mov qword [rcx + 8h], LongModeStart - RendezvousFunnelProcStart
> - mov qword [rcx + 10h], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelAddress], rax
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeEntryOffset], LongModeStart - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelSize], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> lea rax, [ASM_PFX(AsmRelocateApLoop)]
> - mov qword [rcx + 18h], rax
> - mov qword [rcx + 20h], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> - mov qword [rcx + 28h], Flat32Start - RendezvousFunnelProcStart
> - mov qword [rcx + 30h], SwitchToRealProcEnd - SwitchToRealProcStart ; SwitchToRealSize
> - mov qword [rcx + 38h], SwitchToRealProcStart - RendezvousFunnelProcStart ; SwitchToRealOffset
> - mov qword [rcx + 40h], SwitchToRealProcStart - Flat32Start ; SwitchToRealNoNxOffset
> - mov qword [rcx + 48h], PM16Mode - RendezvousFunnelProcStart ; SwitchToRealPM16ModeOffset
> - mov qword [rcx + 50h], SwitchToRealProcEnd - PM16Mode ; SwitchToRealPM16ModeSize
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncAddress], rax
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncSize], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeTransitionOffset], Flat32Start - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealSize], SwitchToRealProcEnd - SwitchToRealProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealOffset], SwitchToRealProcStart - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealNoNxOffset], SwitchToRealProcStart - Flat32Start
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeOffset], PM16Mode - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeSize], SwitchToRealProcEnd - PM16Mode
> ret
>
> ;-------------------------------------------------------------------------------------
> @@ -721,18 +721,18 @@ ASM_PFX(AsmExchangeRole):
>
> ;Store EFLAGS, GDTR and IDTR regiter to stack
> pushfq
> - sgdt [rsi + 16]
> - sidt [rsi + 26]
> + sgdt [rsi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
> + sidt [rsi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; Store the its StackPointer
> - mov [rsi + 8], rsp
> + mov [rsi + CPU_EXCHANGE_ROLE_INFO.StackPointer], rsp
>
> ; update its switch state to STORED
> - mov byte [rsi], CPU_SWITCH_STATE_STORED
> + mov byte [rsi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
>
> WaitForOtherStored:
> ; wait until the other CPU finish storing its state
> - cmp byte [rdi], CPU_SWITCH_STATE_STORED
> + cmp byte [rdi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
> jz OtherStored
> pause
> jmp WaitForOtherStored
> @@ -740,21 +740,21 @@ WaitForOtherStored:
> OtherStored:
> ; Since another CPU already stored its state, load them
> ; load GDTR value
> - lgdt [rdi + 16]
> + lgdt [rdi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
>
> ; load IDTR value
> - lidt [rdi + 26]
> + lidt [rdi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; load its future StackPointer
> - mov rsp, [rdi + 8]
> + mov rsp, [rdi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
>
> ; update the other CPU's switch state to LOADED
> - mov byte [rdi], CPU_SWITCH_STATE_LOADED
> + mov byte [rdi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
>
> WaitForOtherLoaded:
> ; wait until the other CPU finish loading new state,
> ; otherwise the data in stack may corrupt
> - cmp byte [rsi], CPU_SWITCH_STATE_LOADED
> + cmp byte [rsi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
> jz OtherLoaded
> pause
> jmp WaitForOtherLoaded
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
next prev parent reply other threads:[~2021-02-05 15:10 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray
2021-02-05 7:58 ` [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
2021-02-05 15:02 ` [edk2-devel] " Laszlo Ersek
2021-02-05 7:58 ` [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
2021-02-05 15:09 ` Laszlo Ersek [this message]
2021-02-05 7:58 ` [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray
2021-02-05 15:11 ` [edk2-devel] " Laszlo Ersek
2021-02-05 17:11 ` [edk2-devel] [PATCH v2 0/3] " Michael D Kinney
2021-02-05 18:38 ` Ni, Ray
2021-02-05 19:54 ` Michael D Kinney
2021-02-06 4:32 ` Ni, Ray
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6da20851-2614-8ee6-ff32-479d9603305b@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox