Mike, This patch follows your suggestion to fix the performance issue first, clean up the code next.
Can you check specifically this patch? Thanks, Ray > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Tuesday, February 9, 2021 10:17 PM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.d...@intel.com>; Laszlo Ersek <ler...@redhat.com>; > Kumar, Rahul1 <rahul1.ku...@intel.com> > Subject: [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to > avoid lock acquire/release > > When AP firstly wakes up, MpFuncs.nasm contains below logic to assign > an unique ApIndex to each AP according to who comes first: > ---ASM--- > TestLock: > xchg [edi], eax > cmp eax, NotVacantFlag > jz TestLock > > mov ecx, esi > add ecx, ApIndexLocation > inc dword [ecx] > mov ebx, [ecx] > > Releaselock: > mov eax, VacantFlag > xchg [edi], eax > ---ASM END--- > > "lock inc" cannot be used to increase ApIndex because not only the > global ApIndex should be increased, but also the result should be > stored to a 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...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > --- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 20 ++++++------------- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 18 ++++++----------- > 2 files changed, 12 insertions(+), 26 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > index 7e81d24aa6..2eaddc93bc 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: > > @@ -125,19 +125,11 @@ SkipEnableExecuteDisable: > add edi, LockLocation > > mov eax, NotVacantFlag > > > > -TestLock: > > - xchg [edi], eax > > - cmp eax, NotVacantFlag > > - jz TestLock > > - > > - mov ecx, esi > > - add ecx, ApIndexLocation > > - inc dword [ecx] > > - mov ebx, [ecx] > > - > > -Releaselock: > > - mov eax, VacantFlag > > - xchg [edi], eax > > + mov edi, esi > > + add edi, ApIndexLocation > > + mov ebx, 1 > > + lock xadd dword [edi], ebx ; EBX = ApIndex++ > > + inc ebx ; EBX is CpuNumber > > > > mov edi, esi > > add edi, StackSizeLocation > > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > index aecfd07bc0..5b588f2dcb 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: > > @@ -161,18 +161,12 @@ LongModeStart: > add edi, LockLocation > > mov rax, NotVacantFlag > > > > -TestLock: > > - xchg qword [edi], rax > > - cmp rax, NotVacantFlag > > - jz TestLock > > - > > - lea ecx, [esi + ApIndexLocation] > > - inc dword [ecx] > > - mov ebx, [ecx] > > + mov edi, esi > > + add edi, ApIndexLocation > > + mov ebx, 1 > > + lock xadd dword [edi], ebx ; EBX = ApIndex++ > > + inc ebx ; EBX is CpuNumber > > > > -Releaselock: > > - mov rax, VacantFlag > > - xchg qword [edi], rax > > ; program stack > > mov edi, esi > > add edi, StackSizeLocation > > -- > 2.27.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#71517): https://edk2.groups.io/g/devel/message/71517 > Mute This Topic: https://groups.io/mt/80504936/1712937 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray...@intel.com] > -=-=-=-=-=-= > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72019): https://edk2.groups.io/g/devel/message/72019 Mute This Topic: https://groups.io/mt/80842775/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
