[PATCH v3 0/4] Use XADD to avoid lock acquire/release

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [PATCH v3 0/4] Use XADD to avoid lock acquire/release
@ 2021-02-09 14:16 Ni, Ray
  2021-02-09 14:16 ` [PATCH v3 1/4] UefiCpuPkg/MpInitLib: " Ni, Ray
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Ni, Ray @ 2021-02-09 14:16 UTC (permalink / raw)
  To: devel

Patch #1 follows Mike's suggestion to use XADD to avoid lock acquire/release.
Patch #2 follows Laszlo's suggestion to add global NASM macros for NASM struc usage.
Patch #3 simply remves all hardcode offset in NASM without changing any logic.
Patch #4 removes the dead code.

The final code is the same as that of V2.

Ray Ni (4):
  UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
  MdePkg/Nasm.inc: add macros for C types used in structure definition
  UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
  UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO

 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] 14+ messages in thread

* [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
  2021-02-09 14:16 [PATCH v3 0/4] Use XADD to avoid lock acquire/release Ni, Ray
@ 2021-02-09 14:16 ` Ni, Ray
  2021-02-22  9:06   ` Dong, Eric
  2021-02-23 18:11   ` [edk2-devel] " Michael D Kinney
  2021-02-09 14:16 ` [PATCH v3 2/4] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Ni, Ray @ 2021-02-09 14:16 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar

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.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
  2021-02-09 14:16 ` [PATCH v3 1/4] UefiCpuPkg/MpInitLib: " Ni, Ray
@ 2021-02-22  9:06   ` Dong, Eric
  2021-02-23 18:11   ` [edk2-devel] " Michael D Kinney
  1 sibling, 0 replies; 14+ messages in thread
From: Dong, Eric @ 2021-02-22  9:06 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io; +Cc: Laszlo Ersek, Kumar, Rahul1

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, February 9, 2021 10:17 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [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.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@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, StackSizeLocationdiff --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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
  2021-02-09 14:16 ` [PATCH v3 1/4] UefiCpuPkg/MpInitLib: " Ni, Ray
  2021-02-22  9:06   ` Dong, Eric
@ 2021-02-23 18:11   ` Michael D Kinney
  2021-02-25  4:04     ` Ni, Ray
  1 sibling, 1 reply; 14+ messages in thread
From: Michael D Kinney @ 2021-02-23 18:11 UTC (permalink / raw)
  To: devel@edk2.groups.io, Ni, Ray, Kinney, Michael D
  Cc: Dong, Eric, Laszlo Ersek, Kumar, Rahul1

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Tuesday, February 9, 2021 6:17 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@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.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@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/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] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
  2021-02-23 18:11   ` [edk2-devel] " Michael D Kinney
@ 2021-02-25  4:04     ` Ni, Ray
  2021-02-25 19:02       ` Laszlo Ersek
  0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2021-02-25  4:04 UTC (permalink / raw)
  To: devel@edk2.groups.io, Laszlo Ersek
  Cc: Dong, Eric, Kumar, Rahul1, Kinney, Michael D

Laszlo,
Do you think that Mike's R-b to the first patch can be an ack to the V3 patch set?

Can you please review and provide comments?

Thanks,
Ray


> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, February 24, 2021 2:12 AM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: RE: [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD
> to avoid lock acquire/release
> 
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni,
> Ray
> > Sent: Tuesday, February 9, 2021 6:17 AM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Kumar, Rahul1 <rahul1.kumar@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.ni@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Rahul Kumar <rahul1.kumar@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/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] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
  2021-02-25  4:04     ` Ni, Ray
@ 2021-02-25 19:02       ` Laszlo Ersek
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-02-25 19:02 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Dong, Eric, Kumar, Rahul1, Kinney, Michael D

On 02/25/21 05:04, Ni, Ray wrote:
> Laszlo,
> Do you think that Mike's R-b to the first patch can be an ack to the V3 patch set?

No, I don't think so. If an R-b is given in response to a specific patch
(not the cover letter), and the reviewer doesn't explicitly say "series
Reviewed-by" or "for the entire series:", then the R-b applies only to
the specific patch.

Now, a different question is whether you want or need Mike's R-b for
*all* of the patches. That's up to you and Mike to decide.

> Can you please review and provide comments?

Yes, I'll comment soon.

Thanks
Laszlo

>> -----Original Message-----
>> From: Kinney, Michael D <michael.d.kinney@intel.com>
>> Sent: Wednesday, February 24, 2021 2:12 AM
>> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>> Kumar, Rahul1 <rahul1.kumar@intel.com>
>> Subject: RE: [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD
>> to avoid lock acquire/release
>>
>> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni,
>> Ray
>>> Sent: Tuesday, February 9, 2021 6:17 AM
>>> To: devel@edk2.groups.io
>>> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>> Kumar, Rahul1 <rahul1.kumar@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.ni@intel.com>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Rahul Kumar <rahul1.kumar@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/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] 14+ messages in thread

* [PATCH v3 2/4] MdePkg/Nasm.inc: add macros for C types used in structure definition
  2021-02-09 14:16 [PATCH v3 0/4] Use XADD to avoid lock acquire/release Ni, Ray
  2021-02-09 14:16 ` [PATCH v3 1/4] UefiCpuPkg/MpInitLib: " Ni, Ray
@ 2021-02-09 14:16 ` Ni, Ray
  2021-02-18  3:24   ` 回复: " gaoliming
  2021-02-09 14:16 ` [PATCH v3 3/4] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2021-02-09 14:16 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] 14+ messages in thread

* 回复: [PATCH v3 2/4] MdePkg/Nasm.inc: add macros for C types used in structure definition
  2021-02-09 14:16 ` [PATCH v3 2/4] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
@ 2021-02-18  3:24   ` gaoliming
  0 siblings, 0 replies; 14+ messages in thread
From: gaoliming @ 2021-02-18  3:24 UTC (permalink / raw)
  To: 'Ray Ni', devel
  Cc: 'Michael D Kinney', 'Zhiguang Liu'

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Ray Ni <ray.ni@intel.com>
> 发送时间: 2021年2月9日 22:17
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
> 主题: [PATCH v3 2/4] MdePkg/Nasm.inc: add macros for C types used in
> structure definition
> 
> 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	[flat|nested] 14+ messages in thread

* [PATCH v3 3/4] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
  2021-02-09 14:16 [PATCH v3 0/4] Use XADD to avoid lock acquire/release Ni, Ray
  2021-02-09 14:16 ` [PATCH v3 1/4] UefiCpuPkg/MpInitLib: " Ni, Ray
  2021-02-09 14:16 ` [PATCH v3 2/4] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
@ 2021-02-09 14:16 ` Ni, Ray
  2021-02-22  9:06   ` Dong, Eric
  2021-02-09 14:16 ` [PATCH v3 4/4] UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO Ni, Ray
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2021-02-09 14:16 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       |  80 +++++++-------
 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 |  92 ++++++++--------
 7 files changed, 193 insertions(+), 180 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 2eaddc93bc..4363ad9a18 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -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,35 +110,35 @@ 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
 
     mov        edi, esi
-    add        edi, ApIndexLocation
+    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, 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
@@ -171,18 +171,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
@@ -195,11 +195,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
@@ -262,17 +262,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
@@ -302,18 +302,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
@@ -321,21 +321,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 5b588f2dcb..db297f5cca 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -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,47 +139,47 @@ 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
 
     mov        edi, esi
-    add        edi, ApIndexLocation
+    add        edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
     mov        ebx, 1
     lock xadd  dword [edi], ebx                 ; EBX = ApIndex++
     inc        ebx                              ; EBX is CpuNumber
 
     ; 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
 
@@ -193,7 +193,7 @@ LongModeStart:
     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
@@ -202,7 +202,7 @@ LongModeStart:
     jmp        CProcedureInvoke
 
 GetApicId:
-    lea        edi, [esi + SevEsIsEnabledLocation]
+    lea        edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
     cmp        byte [edi], 1        ; SevEsIsEnabled
     jne        DoCpuid
 
@@ -296,18 +296,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
@@ -315,17 +315,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
@@ -661,18 +661,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
 
 ;-------------------------------------------------------------------------------------
@@ -715,18 +715,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
@@ -734,21 +734,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] 14+ messages in thread

* Re: [PATCH v3 3/4] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
  2021-02-09 14:16 ` [PATCH v3 3/4] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
@ 2021-02-22  9:06   ` Dong, Eric
  0 siblings, 0 replies; 14+ messages in thread
From: Dong, Eric @ 2021-02-22  9:06 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io; +Cc: Laszlo Ersek, Kumar, Rahul1

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, February 9, 2021 10:17 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [PATCH v3 3/4] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset

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       |  80 +++++++-------
 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 |  92 ++++++++--------
 7 files changed, 193 insertions(+), 180 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.hdiff --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 2eaddc93bc..4363ad9a18 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -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,35 +110,35 @@ 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      mov        edi, esi-    add        edi, ApIndexLocation+    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, 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@@ -171,18 +171,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@@ -195,11 +195,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@@ -262,17 +262,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@@ -302,18 +302,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@@ -321,21 +321,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        WaitForOtherLoadeddiff --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.hdiff --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 + 0A2hdiff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 5b588f2dcb..db297f5cca 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -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,47 +139,47 @@ 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      mov        edi, esi-    add        edi, ApIndexLocation+    add        edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)     mov        ebx, 1     lock xadd  dword [edi], ebx                 ; EBX = ApIndex++     inc        ebx                              ; EBX is CpuNumber      ; 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 @@ -193,7 +193,7 @@ LongModeStart:
     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@@ -202,7 +202,7 @@ LongModeStart:
     jmp        CProcedureInvoke  GetApicId:-    lea        edi, [esi + SevEsIsEnabledLocation]+    lea        edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]     cmp        byte [edi], 1        ; SevEsIsEnabled     jne        DoCpuid @@ -296,18 +296,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@@ -315,17 +315,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@@ -661,18 +661,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  ;-------------------------------------------------------------------------------------@@ -715,18 +715,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@@ -734,21 +734,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] 14+ messages in thread

* [PATCH v3 4/4] UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO
  2021-02-09 14:16 [PATCH v3 0/4] Use XADD to avoid lock acquire/release Ni, Ray
                   ` (2 preceding siblings ...)
  2021-02-09 14:16 ` [PATCH v3 3/4] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
@ 2021-02-09 14:16 ` Ni, Ray
  2021-02-22  9:07   ` Dong, Eric
       [not found] ` <166219FF4C25D9C5.16853@groups.io>
  2021-02-25 19:03 ` [edk2-devel] [PATCH v3 0/4] " Laszlo Ersek
  5 siblings, 1 reply; 14+ messages in thread
From: Ni, Ray @ 2021-02-09 14:16 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar

The Lock is no longer needed since "LOCK XADD" was used in
MpFuncs.nasm for ApIndex atomic increment.

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/Ia32/MpFuncs.nasm | 4 ----
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc         | 4 ----
 UefiCpuPkg/Library/MpInitLib/MpLib.c           | 1 -
 UefiCpuPkg/Library/MpInitLib/MpLib.h           | 3 +--
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 4 ----
 5 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 4363ad9a18..7bd2415670 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -121,10 +121,6 @@ SkipEnableExecuteDisable:
     lock inc   dword [edi]
 
     ; AP init
-    mov        edi, esi
-    add        edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
-    mov        eax, NotVacantFlag
-
     mov        edi, esi
     add        edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
     mov        ebx, 1
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 db297f5cca..50df802d1f 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -157,10 +157,6 @@ LongModeStart:
     lock inc   dword [edi]
 
     ; AP init
-    mov        edi, esi
-    add        edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
-    mov        rax, NotVacantFlag
-
     mov        edi, esi
     add        edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
     mov        ebx, 1
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/4] UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO
  2021-02-09 14:16 ` [PATCH v3 4/4] UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO Ni, Ray
@ 2021-02-22  9:07   ` Dong, Eric
  0 siblings, 0 replies; 14+ messages in thread
From: Dong, Eric @ 2021-02-22  9:07 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io; +Cc: Laszlo Ersek, Kumar, Rahul1

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, February 9, 2021 10:17 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [PATCH v3 4/4] UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO

The Lock is no longer needed since "LOCK XADD" was used in MpFuncs.nasm for ApIndex atomic increment.

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/Ia32/MpFuncs.nasm | 4 ----
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc         | 4 ----
 UefiCpuPkg/Library/MpInitLib/MpLib.c           | 1 -
 UefiCpuPkg/Library/MpInitLib/MpLib.h           | 3 +--
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 4 ----
 5 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 4363ad9a18..7bd2415670 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -121,10 +121,6 @@ SkipEnableExecuteDisable:
     lock inc   dword [edi]      ; AP init-    mov        edi, esi-    add        edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)-    mov        eax, NotVacantFlag-     mov        edi, esi     add        edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)     mov        ebx, 1diff --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 1diff --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 db297f5cca..50df802d1f 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -157,10 +157,6 @@ LongModeStart:
     lock inc   dword [edi]      ; AP init-    mov        edi, esi-    add        edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)-    mov        rax, NotVacantFlag-     mov        edi, esi     add        edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)     mov        ebx, 1-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

[parent not found: <166219FF4C25D9C5.16853@groups.io>]

* Re: [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
       [not found] ` <166219FF4C25D9C5.16853@groups.io>
@ 2021-02-23  2:22   ` Ni, Ray
  0 siblings, 0 replies; 14+ messages in thread
From: Ni, Ray @ 2021-02-23  2:22 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: Dong, Eric, Laszlo Ersek, Kumar, Rahul1, devel@edk2.groups.io,
	Ni, Ray

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.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Kumar, Rahul1 <rahul1.kumar@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.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@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+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/4] Use XADD to avoid lock acquire/release
  2021-02-09 14:16 [PATCH v3 0/4] Use XADD to avoid lock acquire/release Ni, Ray
                   ` (4 preceding siblings ...)
       [not found] ` <166219FF4C25D9C5.16853@groups.io>
@ 2021-02-25 19:03 ` Laszlo Ersek
  5 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2021-02-25 19:03 UTC (permalink / raw)
  To: devel, ray.ni

On 02/09/21 15:16, Ni, Ray wrote:
> Patch #1 follows Mike's suggestion to use XADD to avoid lock acquire/release.
> Patch #2 follows Laszlo's suggestion to add global NASM macros for NASM struc usage.
> Patch #3 simply remves all hardcode offset in NASM without changing any logic.
> Patch #4 removes the dead code.
> 
> The final code is the same as that of V2.

Given that I was OK with v2:

series
Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> Ray Ni (4):
>   UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>   MdePkg/Nasm.inc: add macros for C types used in structure definition
>   UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
>   UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO
> 
>  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
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-02-25 19:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-09 14:16 [PATCH v3 0/4] Use XADD to avoid lock acquire/release Ni, Ray
2021-02-09 14:16 ` [PATCH v3 1/4] UefiCpuPkg/MpInitLib: " Ni, Ray
2021-02-22  9:06   ` Dong, Eric
2021-02-23 18:11   ` [edk2-devel] " Michael D Kinney
2021-02-25  4:04     ` Ni, Ray
2021-02-25 19:02       ` Laszlo Ersek
2021-02-09 14:16 ` [PATCH v3 2/4] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
2021-02-18  3:24   ` 回复: " gaoliming
2021-02-09 14:16 ` [PATCH v3 3/4] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
2021-02-22  9:06   ` Dong, Eric
2021-02-09 14:16 ` [PATCH v3 4/4] UefiCpuPkg/MpInitLib: Remove unused Lock from MP_CPU_EXCHANGE_INFO Ni, Ray
2021-02-22  9:07   ` Dong, Eric
     [not found] ` <166219FF4C25D9C5.16853@groups.io>
2021-02-23  2:22   ` [edk2-devel] [PATCH v3 1/4] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray
2021-02-25 19:03 ` [edk2-devel] [PATCH v3 0/4] " Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox