Re: [edk2-devel] [PATCH v4 02/14] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance

2024-04-29 Thread Ni, Ray
I've given Reviewed-by for the v3 version.
What extra changes did you make so that you need me to review it again?

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Friday, April 26, 2024 20:17
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R 
Subject: [PATCH v4 02/14] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib 
library instance

This patch just separates the smbase relocation logic from
PiSmmCpuDxeSmm driver, and moves to the SmmRelocationInit
interface. It maintains the original implementation of most
functions and leaves the definitions of global variables
intact. Further refinements to the code are planned for
subsequent patches.

Platform shall consume the interface for the smbase
relocation if need SMM support.

Note:
Before using SmmRelocationLib, the PiSmmCpuDxeSmm driver
allocates the SMRAM to be used for SMI handler and Save
state area of each processor from Smst->AllocatePages().
With SmmRelocationLib, the SMRAM allocation for SMI
handlers and Save state areas is moved to early PEI
phase (Smst->AllocatePages() service is not available).
So, the allocation is done by splitting the SMRAM out of
the SMRAM regions reported from gEfiSmmSMramMemoryGuid.

So, Platform must produce the gEfiSmmSMramMemoryGuid HOB
for SmmRelocationLib usage.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../SmmRelocationLib}/Ia32/Semaphore.c |  10 +-
 .../SmmRelocationLib}/Ia32/SmmInit.nasm|  67 ++-
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 132 +
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 600 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 +++
 .../SmmRelocationLib/SmramSaveStateConfig.c| 139 +
 .../SmmRelocationLib}/X64/Semaphore.c  |  10 +-
 .../SmmRelocationLib}/X64/SmmInit.nasm |  69 ++-
 UefiCpuPkg/UefiCpuPkg.dsc  |   1 +
 9 files changed, 1066 insertions(+), 23 deletions(-)
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/Ia32/Semaphore.c 
(79%)
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/Ia32/SmmInit.nasm 
(53%)
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/SmramSaveStateConfig.c
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/X64/Semaphore.c 
(84%)
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/X64/SmmInit.nasm 
(64%)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
similarity index 79%
copy from UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
copy to UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
index a9fcc89dda..ba329d6ba2 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
@@ -1,15 +1,15 @@
 /** @file
-Semaphore mechanism to indicate to the BSP that an AP has exited SMM
-after SMBASE relocation.
+  Semaphore mechanism to indicate to the BSP that an AP has exited SMM
+  after SMBASE relocation.

-Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent

 **/

-#include "PiSmmCpuDxeSmm.h"
+#include "InternalSmmRelocationLib.h"

 UINTN mSmmRelocationOriginalAddress;
 volatile BOOLEAN  *mRebasedFlag;

 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
similarity index 53%
copy from UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
copy to UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
index b5e77a1a5b..3d845e9e16 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
@@ -1,7 +1,7 @@
 
;-- 
;
-; Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
+; Copyright (c) 2024, Intel Corporation. All rights reserved.
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Module Name:
 ;
 ;   SmmInit.nasm
@@ -27,15 +27,70 @@ global ASM_PFX(gcSmmInitSize)
 global ASM_PFX(gcSmmInitTemplate)

 %define PROTECT_MODE_CS 0x8
 %define PROTECT_MODE_DS 0x20

-SECTION .text
+SECTION .data

-ASM_PFX(gcSmiInitGdtr):
+NullSeg: DQ 0   ; reserved by architecture
+CodeSeg32:
+DW  -1  ; LimitLow
+DW  0   ; BaseLow
+DB  0   ; BaseMid
+DB  0x9b
+DB  0xcf; LimitHigh
+DB  0   ; 

[edk2-devel] [PATCH v4 02/14] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance

2024-04-26 Thread Wu, Jiaxin
This patch just separates the smbase relocation logic from
PiSmmCpuDxeSmm driver, and moves to the SmmRelocationInit
interface. It maintains the original implementation of most
functions and leaves the definitions of global variables
intact. Further refinements to the code are planned for
subsequent patches.

Platform shall consume the interface for the smbase
relocation if need SMM support.

Note:
Before using SmmRelocationLib, the PiSmmCpuDxeSmm driver
allocates the SMRAM to be used for SMI handler and Save
state area of each processor from Smst->AllocatePages().
With SmmRelocationLib, the SMRAM allocation for SMI
handlers and Save state areas is moved to early PEI
phase (Smst->AllocatePages() service is not available).
So, the allocation is done by splitting the SMRAM out of
the SMRAM regions reported from gEfiSmmSMramMemoryGuid.

So, Platform must produce the gEfiSmmSMramMemoryGuid HOB
for SmmRelocationLib usage.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../SmmRelocationLib}/Ia32/Semaphore.c |  10 +-
 .../SmmRelocationLib}/Ia32/SmmInit.nasm|  67 ++-
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 132 +
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 600 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 +++
 .../SmmRelocationLib/SmramSaveStateConfig.c| 139 +
 .../SmmRelocationLib}/X64/Semaphore.c  |  10 +-
 .../SmmRelocationLib}/X64/SmmInit.nasm |  69 ++-
 UefiCpuPkg/UefiCpuPkg.dsc  |   1 +
 9 files changed, 1066 insertions(+), 23 deletions(-)
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/Ia32/Semaphore.c 
(79%)
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/Ia32/SmmInit.nasm 
(53%)
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/SmramSaveStateConfig.c
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/X64/Semaphore.c 
(84%)
 copy UefiCpuPkg/{PiSmmCpuDxeSmm => Library/SmmRelocationLib}/X64/SmmInit.nasm 
(64%)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
similarity index 79%
copy from UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
copy to UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
index a9fcc89dda..ba329d6ba2 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
@@ -1,15 +1,15 @@
 /** @file
-Semaphore mechanism to indicate to the BSP that an AP has exited SMM
-after SMBASE relocation.
+  Semaphore mechanism to indicate to the BSP that an AP has exited SMM
+  after SMBASE relocation.
 
-Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include "PiSmmCpuDxeSmm.h"
+#include "InternalSmmRelocationLib.h"
 
 UINTN mSmmRelocationOriginalAddress;
 volatile BOOLEAN  *mRebasedFlag;
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
similarity index 53%
copy from UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
copy to UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
index b5e77a1a5b..3d845e9e16 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
@@ -1,7 +1,7 @@
 
;-- 
;
-; Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
+; Copyright (c) 2024, Intel Corporation. All rights reserved.
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Module Name:
 ;
 ;   SmmInit.nasm
@@ -27,15 +27,70 @@ global ASM_PFX(gcSmmInitSize)
 global ASM_PFX(gcSmmInitTemplate)
 
 %define PROTECT_MODE_CS 0x8
 %define PROTECT_MODE_DS 0x20
 
-SECTION .text
+SECTION .data
 
-ASM_PFX(gcSmiInitGdtr):
+NullSeg: DQ 0   ; reserved by architecture
+CodeSeg32:
+DW  -1  ; LimitLow
+DW  0   ; BaseLow
+DB  0   ; BaseMid
+DB  0x9b
+DB  0xcf; LimitHigh
+DB  0   ; BaseHigh
+ProtModeCodeSeg32:
+DW  -1  ; LimitLow
+DW  0   ; BaseLow
+DB  0   ; BaseMid
+DB  0x9b
+DB  0xcf; LimitHigh
+DB  0   ; BaseHigh
+ProtModeSsSeg32:
+DW  -1  ; LimitLow
+