* [PATCH v2 0/3] Use XADD to avoid lock acquire/release
@ 2021-02-05 7:58 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
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw)
To: devel
Patch #1 follows Laszlo's suggestion to add global NASM macros
for NASM struc usage.
Patch #2 changes all hardcode offset to use struc.
Patch #3 doesn't have any change comparing to V1 except
1). dword/qword prefix is added.
2). the comments "program AP stack" is removed.
Ray Ni (3):
MdePkg/Nasm.inc: add macros for C types used in structure definition
UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
MdePkg/Include/Ia32/Nasm.inc | 38 ++++++
MdePkg/Include/X64/Nasm.inc | 38 ++++++
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 -------
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++---------
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
11 files changed, 272 insertions(+), 211 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
--
2.27.0.windows.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition 2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray @ 2021-02-05 7:58 ` 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 ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw) To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> --- MdePkg/Include/Ia32/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++ MdePkg/Include/X64/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/MdePkg/Include/Ia32/Nasm.inc b/MdePkg/Include/Ia32/Nasm.inc index 31ce861f1e..017fe5ffd8 100644 --- a/MdePkg/Include/Ia32/Nasm.inc +++ b/MdePkg/Include/Ia32/Nasm.inc @@ -20,3 +20,41 @@ %macro INCSSP_EAX 0 DB 0xF3, 0x0F, 0xAE, 0xE8 %endmacro + +; NASM provides built-in macros STRUC and ENDSTRUC for structure definition. +; For example, to define a structure called mytype containing a longword, +; a word, a byte and a string of bytes, you might code +; +; struc mytype +; +; mt_long: resd 1 +; mt_word: resw 1 +; mt_byte: resb 1 +; mt_str: resb 32 +; +; endstruc +; +; Below macros are help to map the C types and the RESB family of pseudo-instructions. +; So that the above structure definition can be coded as +; +; struc mytype +; +; mt_long: CTYPE_UINT32 1 +; mt_word: CTYPE_UINT16 1 +; mt_byte: CTYPE_UINT8 1 +; mt_str: CTYPE_CHAR8 32 +; +; endstruc +%define CTYPE_UINT64 resq +%define CTYPE_INT64 resq +%define CTYPE_UINT32 resd +%define CTYPE_INT32 resd +%define CTYPE_UINT16 resw +%define CTYPE_INT16 resw +%define CTYPE_BOOLEAN resb +%define CTYPE_UINT8 resb +%define CTYPE_CHAR8 resb +%define CTYPE_INT8 resb + +%define CTYPE_UINTN resd +%define CTYPE_INTN resd diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc index 42412735ea..b48d8680bb 100644 --- a/MdePkg/Include/X64/Nasm.inc +++ b/MdePkg/Include/X64/Nasm.inc @@ -20,3 +20,41 @@ %macro INCSSP_RAX 0 DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8 %endmacro + +; NASM provides built-in macros STRUC and ENDSTRUC for structure definition. +; For example, to define a structure called mytype containing a longword, +; a word, a byte and a string of bytes, you might code +; +; struc mytype +; +; mt_long: resd 1 +; mt_word: resw 1 +; mt_byte: resb 1 +; mt_str: resb 32 +; +; endstruc +; +; Below macros are help to map the C types and the RESB family of pseudo-instructions. +; So that the above structure definition can be coded as +; +; struc mytype +; +; mt_long: CTYPE_UINT32 1 +; mt_word: CTYPE_UINT16 1 +; mt_byte: CTYPE_UINT8 1 +; mt_str: CTYPE_CHAR8 32 +; +; endstruc +%define CTYPE_UINT64 resq +%define CTYPE_INT64 resq +%define CTYPE_UINT32 resd +%define CTYPE_INT32 resd +%define CTYPE_UINT16 resw +%define CTYPE_INT16 resw +%define CTYPE_BOOLEAN resb +%define CTYPE_UINT8 resb +%define CTYPE_CHAR8 resb +%define CTYPE_INT8 resb + +%define CTYPE_UINTN resq +%define CTYPE_INTN resq -- 2.27.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition 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 ` Laszlo Ersek 0 siblings, 0 replies; 11+ messages in thread From: Laszlo Ersek @ 2021-02-05 15:02 UTC (permalink / raw) To: devel, ray.ni; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu On 02/05/21 08:58, Ni, Ray wrote: > Signed-off-by: Ray Ni <ray.ni@intel.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > --- > MdePkg/Include/Ia32/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++ > MdePkg/Include/X64/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 76 insertions(+) > > diff --git a/MdePkg/Include/Ia32/Nasm.inc b/MdePkg/Include/Ia32/Nasm.inc > index 31ce861f1e..017fe5ffd8 100644 > --- a/MdePkg/Include/Ia32/Nasm.inc > +++ b/MdePkg/Include/Ia32/Nasm.inc > @@ -20,3 +20,41 @@ > %macro INCSSP_EAX 0 > DB 0xF3, 0x0F, 0xAE, 0xE8 > %endmacro > + > +; NASM provides built-in macros STRUC and ENDSTRUC for structure definition. > +; For example, to define a structure called mytype containing a longword, > +; a word, a byte and a string of bytes, you might code > +; > +; struc mytype > +; > +; mt_long: resd 1 > +; mt_word: resw 1 > +; mt_byte: resb 1 > +; mt_str: resb 32 > +; > +; endstruc > +; > +; Below macros are help to map the C types and the RESB family of pseudo-instructions. > +; So that the above structure definition can be coded as > +; > +; struc mytype > +; > +; mt_long: CTYPE_UINT32 1 > +; mt_word: CTYPE_UINT16 1 > +; mt_byte: CTYPE_UINT8 1 > +; mt_str: CTYPE_CHAR8 32 > +; > +; endstruc > +%define CTYPE_UINT64 resq > +%define CTYPE_INT64 resq > +%define CTYPE_UINT32 resd > +%define CTYPE_INT32 resd > +%define CTYPE_UINT16 resw > +%define CTYPE_INT16 resw > +%define CTYPE_BOOLEAN resb > +%define CTYPE_UINT8 resb > +%define CTYPE_CHAR8 resb > +%define CTYPE_INT8 resb > + > +%define CTYPE_UINTN resd > +%define CTYPE_INTN resd > diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc > index 42412735ea..b48d8680bb 100644 > --- a/MdePkg/Include/X64/Nasm.inc > +++ b/MdePkg/Include/X64/Nasm.inc > @@ -20,3 +20,41 @@ > %macro INCSSP_RAX 0 > DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8 > %endmacro > + > +; NASM provides built-in macros STRUC and ENDSTRUC for structure definition. > +; For example, to define a structure called mytype containing a longword, > +; a word, a byte and a string of bytes, you might code > +; > +; struc mytype > +; > +; mt_long: resd 1 > +; mt_word: resw 1 > +; mt_byte: resb 1 > +; mt_str: resb 32 > +; > +; endstruc > +; > +; Below macros are help to map the C types and the RESB family of pseudo-instructions. > +; So that the above structure definition can be coded as > +; > +; struc mytype > +; > +; mt_long: CTYPE_UINT32 1 > +; mt_word: CTYPE_UINT16 1 > +; mt_byte: CTYPE_UINT8 1 > +; mt_str: CTYPE_CHAR8 32 > +; > +; endstruc > +%define CTYPE_UINT64 resq > +%define CTYPE_INT64 resq > +%define CTYPE_UINT32 resd > +%define CTYPE_INT32 resd > +%define CTYPE_UINT16 resw > +%define CTYPE_INT16 resw > +%define CTYPE_BOOLEAN resb > +%define CTYPE_UINT8 resb > +%define CTYPE_CHAR8 resb > +%define CTYPE_INT8 resb > + > +%define CTYPE_UINTN resq > +%define CTYPE_INTN resq > Reviewed-by: Laszlo Ersek <lersek@redhat.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset 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 7:58 ` Ni, Ray 2021-02-05 15:09 ` [edk2-devel] " Laszlo Ersek 2021-02-05 7:58 ` [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray 2021-02-05 17:11 ` [edk2-devel] [PATCH v2 0/3] " Michael D Kinney 3 siblings, 1 reply; 11+ messages in thread From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar 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 -- 2.27.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset 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 0 siblings, 0 replies; 11+ messages in thread From: Laszlo Ersek @ 2021-02-05 15:09 UTC (permalink / raw) To: devel, ray.ni; +Cc: Eric Dong, Rahul Kumar 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> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release 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 7:58 ` [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray @ 2021-02-05 7:58 ` 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 3 siblings, 1 reply; 11+ messages in thread From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw) To: devel; +Cc: Laszlo Ersek, Eric Dong, Rahul1 Kumar When AP firstly wakes up, MpFuncs.nasm contains below logic to assign an unique ApIndex to each AP according to who comes first: ---NASM--- mov edi, esi add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock) mov eax, NotVacantFlag TestLock: xchg [edi], eax cmp eax, NotVacantFlag jz TestLock mov ecx, esi add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) inc dword [ecx] mov ebx, [ecx] Releaselock: mov eax, VacantFlag xchg [edi], eax ---NASM END--- "LOCK INC" cannot be used to increase MP_CPU_EXCHANGE_INFO.ApIndex because not only the MP_CPU_EXCHANGE_INFO.ApIndex should be increased, but also the result should be stored to a thread local general purpose register EBX. This patch learns from the NASM implementation of InternalSyncIncrement() to use "XADD" instruction which can increase the global ApIndex and store the original ApIndex to EBX in one instruction. With this patch, OVMF when running in a 255 threads QEMU spends about one second to wakeup all APs. Original implementation needs more than 10 seconds. Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Rahul1 Kumar <rahul1.kumar@intel.com> --- .../Library/MpInitLib/Ia32/MpFuncs.nasm | 20 ++++--------------- UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 4 ---- UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 - UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-- UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 18 ++++------------- 5 files changed, 9 insertions(+), 37 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 2f1b102717..7bd2415670 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -122,22 +122,10 @@ SkipEnableExecuteDisable: ; AP init mov edi, esi - add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock) - mov eax, NotVacantFlag - -TestLock: - xchg [edi], eax - cmp eax, NotVacantFlag - jz TestLock - - mov ecx, esi - add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) - inc dword [ecx] - mov ebx, [ecx] - -Releaselock: - mov eax, VacantFlag - xchg [edi], eax + add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) + mov ebx, 1 + lock xadd dword [edi], ebx ; EBX = ApIndex++ + inc ebx ; EBX is CpuNumber mov edi, esi add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize) diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc index 46c2b5c116..2e9368a374 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc +++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc @@ -13,9 +13,6 @@ ;------------------------------------------------------------------------------- %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 @@ -72,7 +69,6 @@ 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 diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 2568986d8c..5040053dad 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1006,7 +1006,6 @@ FillExchangeInfoData ( IA32_CR4 Cr4; ExchangeInfo = CpuMpData->MpCpuExchangeInfo; - ExchangeInfo->Lock = 0; ExchangeInfo->StackStart = CpuMpData->Buffer; ExchangeInfo->StackSize = CpuMpData->CpuApStackSize; ExchangeInfo->BufferStart = CpuMpData->WakeupBuffer; diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index 02652eaae1..0bd60388b1 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -1,7 +1,7 @@ /** @file Common header file for MP Initialize Library. - Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR> + Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR> Copyright (c) 2020, AMD Inc. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -190,7 +190,6 @@ typedef struct _CPU_MP_DATA CPU_MP_DATA; // into this structure are used in assembly code in this module // typedef struct { - UINTN Lock; UINTN StackStart; UINTN StackSize; UINTN CFunction; diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm index bf7faaf60b..50df802d1f 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm @@ -158,21 +158,11 @@ LongModeStart: ; AP init mov edi, esi - add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock) - mov rax, NotVacantFlag + add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) + mov ebx, 1 + lock xadd dword [edi], ebx ; EBX = ApIndex++ + inc ebx ; EBX is CpuNumber -TestLock: - xchg qword [edi], rax - cmp rax, NotVacantFlag - jz TestLock - - lea ecx, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)] - inc dword [ecx] - mov ebx, [ecx] - -Releaselock: - mov rax, VacantFlag - xchg qword [edi], rax ; program stack mov edi, esi add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize) -- 2.27.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release 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 ` Laszlo Ersek 0 siblings, 0 replies; 11+ messages in thread From: Laszlo Ersek @ 2021-02-05 15:11 UTC (permalink / raw) To: devel, ray.ni; +Cc: Eric Dong, Rahul1 Kumar On 02/05/21 08:58, Ni, Ray wrote: > When AP firstly wakes up, MpFuncs.nasm contains below logic to assign > an unique ApIndex to each AP according to who comes first: > ---NASM--- > mov edi, esi > add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock) > mov eax, NotVacantFlag > > TestLock: > xchg [edi], eax > cmp eax, NotVacantFlag > jz TestLock > > mov ecx, esi > add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) > inc dword [ecx] > mov ebx, [ecx] > > Releaselock: > mov eax, VacantFlag > xchg [edi], eax > ---NASM END--- > > "LOCK INC" cannot be used to increase MP_CPU_EXCHANGE_INFO.ApIndex > because not only the MP_CPU_EXCHANGE_INFO.ApIndex should be > increased, but also the result should be stored to a thread local > general purpose register EBX. > > This patch learns from the NASM implementation of > InternalSyncIncrement() to use "XADD" instruction which can increase > the global ApIndex and store the original ApIndex to EBX in one > instruction. > > With this patch, OVMF when running in a 255 threads QEMU spends about > one second to wakeup all APs. Original implementation needs more than > 10 seconds. > > Signed-off-by: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Rahul1 Kumar <rahul1.kumar@intel.com> > --- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 20 ++++--------------- > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 4 ---- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 - > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 18 ++++------------- > 5 files changed, 9 insertions(+), 37 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > index 2f1b102717..7bd2415670 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > @@ -122,22 +122,10 @@ SkipEnableExecuteDisable: > > ; AP init > mov edi, esi > - add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock) > - mov eax, NotVacantFlag > - > -TestLock: > - xchg [edi], eax > - cmp eax, NotVacantFlag > - jz TestLock > - > - mov ecx, esi > - add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) > - inc dword [ecx] > - mov ebx, [ecx] > - > -Releaselock: > - mov eax, VacantFlag > - xchg [edi], eax > + add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) > + mov ebx, 1 > + lock xadd dword [edi], ebx ; EBX = ApIndex++ > + inc ebx ; EBX is CpuNumber > > mov edi, esi > add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize) > diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc > index 46c2b5c116..2e9368a374 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc > +++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc > @@ -13,9 +13,6 @@ > ;------------------------------------------------------------------------------- > %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 > @@ -72,7 +69,6 @@ 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 > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 2568986d8c..5040053dad 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1006,7 +1006,6 @@ FillExchangeInfoData ( > IA32_CR4 Cr4; > > ExchangeInfo = CpuMpData->MpCpuExchangeInfo; > - ExchangeInfo->Lock = 0; > ExchangeInfo->StackStart = CpuMpData->Buffer; > ExchangeInfo->StackSize = CpuMpData->CpuApStackSize; > ExchangeInfo->BufferStart = CpuMpData->WakeupBuffer; > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h > index 02652eaae1..0bd60388b1 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h > @@ -1,7 +1,7 @@ > /** @file > Common header file for MP Initialize Library. > > - Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2020, AMD Inc. All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -190,7 +190,6 @@ typedef struct _CPU_MP_DATA CPU_MP_DATA; > // into this structure are used in assembly code in this module > // > typedef struct { > - UINTN Lock; > UINTN StackStart; > UINTN StackSize; > UINTN CFunction; > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > index bf7faaf60b..50df802d1f 100644 > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > @@ -158,21 +158,11 @@ LongModeStart: > > ; AP init > mov edi, esi > - add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock) > - mov rax, NotVacantFlag > + add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex) > + mov ebx, 1 > + lock xadd dword [edi], ebx ; EBX = ApIndex++ > + inc ebx ; EBX is CpuNumber > > -TestLock: > - xchg qword [edi], rax > - cmp rax, NotVacantFlag > - jz TestLock > - > - lea ecx, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)] > - inc dword [ecx] > - mov ebx, [ecx] > - > -Releaselock: > - mov rax, VacantFlag > - xchg qword [edi], rax > ; program stack > mov edi, esi > add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize) > Reviewed-by: Laszlo Ersek <lersek@redhat.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release 2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray ` (2 preceding siblings ...) 2021-02-05 7:58 ` [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray @ 2021-02-05 17:11 ` Michael D Kinney 2021-02-05 18:38 ` Ni, Ray 3 siblings, 1 reply; 11+ messages in thread From: Michael D Kinney @ 2021-02-05 17:11 UTC (permalink / raw) To: devel@edk2.groups.io, Ni, Ray, Kinney, Michael D Hi Ray, I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series. Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct? Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field? Thanks, Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Thursday, February 4, 2021 11:58 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release > > Patch #1 follows Laszlo's suggestion to add global NASM macros > > for NASM struc usage. > > Patch #2 changes all hardcode offset to use struc. > > Patch #3 doesn't have any change comparing to V1 except > > 1). dword/qword prefix is added. > > 2). the comments "program AP stack" is removed. > > Ray Ni (3): > MdePkg/Nasm.inc: add macros for C types used in structure definition > UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset > UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release > > MdePkg/Include/Ia32/Nasm.inc | 38 ++++++ > MdePkg/Include/X64/Nasm.inc | 38 ++++++ > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 ------- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++--------- > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 - > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +- > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 -------- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++---------- > 11 files changed, 272 insertions(+), 211 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 > > -- > 2.27.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344 > Mute This Topic: https://groups.io/mt/80401290/1643496 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com] > -=-=-=-=-=-= > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release 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 0 siblings, 1 reply; 11+ messages in thread From: Ni, Ray @ 2021-02-05 18:38 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io [-- Attachment #1: Type: text/plain, Size: 3246 bytes --] Mike, The clean up doesn’t cause any final instruction change and I have verified that. The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets. What potential issue do you see? Thanks, Ray thanks, ray ________________________________ 发件人: Kinney, Michael D <michael.d.kinney@intel.com> 发送时间: Saturday, February 6, 2021 1:11:19 AM 收件人: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com> 主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release Hi Ray, I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series. Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct? Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field? Thanks, Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Thursday, February 4, 2021 11:58 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release > > Patch #1 follows Laszlo's suggestion to add global NASM macros > > for NASM struc usage. > > Patch #2 changes all hardcode offset to use struc. > > Patch #3 doesn't have any change comparing to V1 except > > 1). dword/qword prefix is added. > > 2). the comments "program AP stack" is removed. > > Ray Ni (3): > MdePkg/Nasm.inc: add macros for C types used in structure definition > UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset > UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release > > MdePkg/Include/Ia32/Nasm.inc | 38 ++++++ > MdePkg/Include/X64/Nasm.inc | 38 ++++++ > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 ------- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++--------- > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 - > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +- > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 -------- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++---------- > 11 files changed, 272 insertions(+), 211 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 > > -- > 2.27.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344 > Mute This Topic: https://groups.io/mt/80401290/1643496 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com] > -=-=-=-=-=-= > [-- Attachment #2: Type: text/html, Size: 5882 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release 2021-02-05 18:38 ` Ni, Ray @ 2021-02-05 19:54 ` Michael D Kinney 2021-02-06 4:32 ` Ni, Ray 0 siblings, 1 reply; 11+ messages in thread From: Michael D Kinney @ 2021-02-05 19:54 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io, Kinney, Michael D [-- Attachment #1: Type: text/plain, Size: 3903 bytes --] My comment is only to make the history of changes easier to understand by separating the functional and non-functional changes. Mike From: Ni, Ray <ray.ni@intel.com> Sent: Friday, February 5, 2021 10:38 AM To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io Subject: Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release Mike, The clean up doesn’t cause any final instruction change and I have verified that. The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets. What potential issue do you see? Thanks, Ray thanks, ray ________________________________ 发件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> 发送时间: Saturday, February 6, 2021 1:11:19 AM 收件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> 主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release Hi Ray, I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series. Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct? Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field? Thanks, Mike > -----Original Message----- > From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray > Sent: Thursday, February 4, 2021 11:58 PM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release > > Patch #1 follows Laszlo's suggestion to add global NASM macros > > for NASM struc usage. > > Patch #2 changes all hardcode offset to use struc. > > Patch #3 doesn't have any change comparing to V1 except > > 1). dword/qword prefix is added. > > 2). the comments "program AP stack" is removed. > > Ray Ni (3): > MdePkg/Nasm.inc: add macros for C types used in structure definition > UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset > UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release > > MdePkg/Include/Ia32/Nasm.inc | 38 ++++++ > MdePkg/Include/X64/Nasm.inc | 38 ++++++ > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 ------- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++--------- > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 - > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +- > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 -------- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++---------- > 11 files changed, 272 insertions(+), 211 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 > > -- > 2.27.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344 > Mute This Topic: https://groups.io/mt/80401290/1643496 > Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com] > -=-=-=-=-=-= > [-- Attachment #2: Type: text/html, Size: 47382 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release 2021-02-05 19:54 ` Michael D Kinney @ 2021-02-06 4:32 ` Ni, Ray 0 siblings, 0 replies; 11+ messages in thread From: Ni, Ray @ 2021-02-06 4:32 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io [-- Attachment #1: Type: text/plain, Size: 4299 bytes --] I see. Will send a V3. From: Kinney, Michael D <michael.d.kinney@intel.com> Sent: Saturday, February 6, 2021 3:54 AM To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com> Subject: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release My comment is only to make the history of changes easier to understand by separating the functional and non-functional changes. Mike From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>> Sent: Friday, February 5, 2021 10:38 AM To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Subject: Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release Mike, The clean up doesn’t cause any final instruction change and I have verified that. The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets. What potential issue do you see? Thanks, Ray thanks, ray ________________________________ 发件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> 发送时间: Saturday, February 6, 2021 1:11:19 AM 收件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> 主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release Hi Ray, I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series. Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct? Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field? Thanks, Mike > -----Original Message----- > From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray > Sent: Thursday, February 4, 2021 11:58 PM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release > > Patch #1 follows Laszlo's suggestion to add global NASM macros > > for NASM struc usage. > > Patch #2 changes all hardcode offset to use struc. > > Patch #3 doesn't have any change comparing to V1 except > > 1). dword/qword prefix is added. > > 2). the comments "program AP stack" is removed. > > Ray Ni (3): > MdePkg/Nasm.inc: add macros for C types used in structure definition > UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset > UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release > > MdePkg/Include/Ia32/Nasm.inc | 38 ++++++ > MdePkg/Include/X64/Nasm.inc | 38 ++++++ > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 ------- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++--------- > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 - > UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +- > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +- > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 -------- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++---------- > 11 files changed, 272 insertions(+), 211 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 > > -- > 2.27.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344 > Mute This Topic: https://groups.io/mt/80401290/1643496 > Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com] > -=-=-=-=-=-= > [-- Attachment #2: Type: text/html, Size: 11926 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-02-06 4:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 ` [edk2-devel] " Laszlo Ersek 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox