Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

[Wu, Jiaxin]

Oh, sorry, I missed the that email. If so, that's great! Thanks Ray & Abner & 
Abdul.


From: Chang, Abner 
Sent: Friday, April 19, 2024 11:47 AM
To: Ni, Ray ; Wu, Jiaxin ; 
devel@edk2.groups.io
Cc: Zeng, Star ; Gerd Hoffmann ; Kumar, 
Rahul R ; Dong, Guo ; Rhodes, Sean 
; Lu, James ; Guo, Gua 
; Ard Biesheuvel ; Yao, Jiewen 
; Attar, AbdulLateef (Abdul Lateef) 
; Lendacky, Thomas 
Subject: Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib


[AMD Official Use Only - General]

Right, Abdul's RB is sufficient!


Thanks
Abner

From: Ni, Ray mailto:ray...@intel.com>>
Sent: Friday, April 19, 2024 11:35:17 AM
To: Wu, Jiaxin mailto:jiaxin...@intel.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>
Cc: Zeng, Star mailto:star.z...@intel.com>>; Gerd Hoffmann 
mailto:kra...@redhat.com>>; Kumar, Rahul R 
mailto:rahul.r.ku...@intel.com>>; Dong, Guo 
mailto:guo.d...@intel.com>>; Rhodes, Sean 
mailto:sean@starlabs.systems>>; Lu, James 
mailto:james...@intel.com>>; Guo, Gua 
mailto:gua@intel.com>>; Ard Biesheuvel 
mailto:ardb+tianoc...@kernel.org>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; Attar, AbdulLateef (Abdul 
Lateef) mailto:abdullateef.at...@amd.com>>; Chang, 
Abner mailto:abner.ch...@amd.com>>; Lendacky, Thomas 
mailto:thomas.lenda...@amd.com>>
Subject: Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.

Jiaxin,
I see Abdul from AMD has given R-B. I assume that's sufficient for merging this 
patch.

Thanks,
Ray

From: Wu, Jiaxin mailto:jiaxin...@intel.com>>
Sent: Thursday, April 18, 2024 16:03
To: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Cc: Ni, Ray mailto:ray...@intel.com>>; Zeng, Star 
mailto:star.z...@intel.com>>; Gerd Hoffmann 
mailto:kra...@redhat.com>>; Kumar, Rahul R 
mailto:rahul.r.ku...@intel.com>>; Dong, Guo 
mailto:guo.d...@intel.com>>; Rhodes, Sean 
mailto:sean@starlabs.systems>>; Lu, James 
mailto:james...@intel.com>>; Guo, Gua 
mailto:gua@intel.com>>; Ard Biesheuvel 
mailto:ardb+tianoc...@kernel.org>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; Abdul Lateef Attar 
mailto:abdullateef.at...@amd.com>>; Abner Chang 
mailto:abner.ch...@amd.com>>; Tom Lendacky 
mailto:thomas.lenda...@amd.com>>
Subject: RE: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

Hi Tom & Abner,

Could you help check & review AMD related patch?
 >   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD

Thanks,
Jiaxin

> -Original Message-
> From: devel@edk2.groups.io 
> mailto:devel@edk2.groups.io>> On Behalf Of Wu,
> Jiaxin
> Sent: Thursday, April 18, 2024 2:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray mailto:ray...@intel.com>>; Zeng, Star 
> mailto:star.z...@intel.com>>; Gerd
> Hoffmann mailto:kra...@redhat.com>>; Kumar, Rahul R 
> mailto:rahul.r.ku...@intel.com>>;
> Dong, Guo mailto:guo.d...@intel.com>>; Rhodes, Sean 
> mailto:sean@starlabs.systems>>;
> Lu, James mailto:james...@intel.com>>; Guo, Gua 
> mailto:gua@intel.com>>; Ard
> Biesheuvel mailto:ardb+tianoc...@kernel.org>>; 
> Yao, Jiewen
> mailto:jiewen@intel.com>>; Abdul Lateef Attar 
> mailto:abdullateef.at...@amd.com>>;
> Abner Chang mailto:abner.ch...@amd.com>>; Tom Lendacky
> mailto:thomas.lenda...@amd.com>>
> Subject: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib
>
> PR: https://github.com/tianocore/edk2/pull/5546
>
> Intel plans to separate the smbase relocation logic from
> PiSmmCpuDxeSmm driver, and the related behavior will be
> moved to the new interface defined by the SmmRelocationLib
> class.
>
> The SmmRelocationLib class provides the SmmRelocationInit()
> interface for platform to do the smbase relocation, which
> shall provide below 2 functionalities:
> 1. Relocate smbases for each processor.
> 2. Create the gSmmBaseHobGuid HOB.
>
> With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
> a later phase) can be simplfied as below for SMM init:
> 1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
> for each Processor.
> 2. Execute the early SMM Init.
>
> Cc: Ray Ni mailto:ray...@intel.com>>
> Cc: Zeng Star mailto:star.z...@intel.com>>
> Cc: Gerd Hoffmann mailto:kra...@redhat.com>>
> Cc: Rahul Kumar mailto:rahul1.ku...@intel.com>>
> Cc: Guo Dong mailto:guo.d...@intel.com>>
> Cc: Sean Rhodes mailto:sean@starlabs.systems>>
> Cc: James Lu mailto:james...@intel.com>>
> Cc: Gua Guo mailto:gua@intel.com>>
> Cc: Ard Biesheuvel 
> mailto:ardb+tianoc...@kernel.org>>
> Cc: Jiewen Yao mailto:jiewen@intel.com>>
> Cc: Abdul Lateef Attar 
> mailto:abdullateef.at...@amd.com>>
> Cc: Abner Chang mailto:abner.ch...@amd.com>>
> Cc: Tom Lendacky mailto:thomas.lenda...@amd.com>>
> Signed-off-by: Jiaxin Wu mailto:jiaxin...@intel.com>>
>

Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Right, Abdul's RB is sufficient!


Thanks
Abner

From: Ni, Ray 
Sent: Friday, April 19, 2024 11:35:17 AM
To: Wu, Jiaxin ; devel@edk2.groups.io 

Cc: Zeng, Star ; Gerd Hoffmann ; Kumar, 
Rahul R ; Dong, Guo ; Rhodes, Sean 
; Lu, James ; Guo, Gua 
; Ard Biesheuvel ; Yao, Jiewen 
; Attar, AbdulLateef (Abdul Lateef) 
; Chang, Abner ; Lendacky, 
Thomas 
Subject: Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.

Jiaxin,
I see Abdul from AMD has given R-B. I assume that's sufficient for merging this 
patch.

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Thursday, April 18, 2024 16:03
To: devel@edk2.groups.io ; Wu, Jiaxin 

Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R ; Dong, Guo 
; Rhodes, Sean ; Lu, James 
; Guo, Gua ; Ard Biesheuvel 
; Yao, Jiewen ; Abdul Lateef 
Attar ; Abner Chang ; Tom 
Lendacky 
Subject: RE: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

Hi Tom & Abner,

Could you help check & review AMD related patch?
 >   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD

Thanks,
Jiaxin

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu,
> Jiaxin
> Sent: Thursday, April 18, 2024 2:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Zeng, Star ; Gerd
> Hoffmann ; Kumar, Rahul R ;
> Dong, Guo ; Rhodes, Sean ;
> Lu, James ; Guo, Gua ; Ard
> Biesheuvel ; Yao, Jiewen
> ; Abdul Lateef Attar ;
> Abner Chang ; Tom Lendacky
> 
> Subject: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib
>
> PR: https://github.com/tianocore/edk2/pull/5546
>
> Intel plans to separate the smbase relocation logic from
> PiSmmCpuDxeSmm driver, and the related behavior will be
> moved to the new interface defined by the SmmRelocationLib
> class.
>
> The SmmRelocationLib class provides the SmmRelocationInit()
> interface for platform to do the smbase relocation, which
> shall provide below 2 functionalities:
> 1. Relocate smbases for each processor.
> 2. Create the gSmmBaseHobGuid HOB.
>
> With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
> a later phase) can be simplfied as below for SMM init:
> 1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
> for each Processor.
> 2. Execute the early SMM Init.
>
> Cc: Ray Ni 
> Cc: Zeng Star 
> Cc: Gerd Hoffmann 
> Cc: Rahul Kumar 
> Cc: Guo Dong 
> Cc: Sean Rhodes 
> Cc: James Lu 
> Cc: Gua Guo 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Abdul Lateef Attar 
> Cc: Abner Chang 
> Cc: Tom Lendacky 
> Signed-off-by: Jiaxin Wu 
>
> Jiaxin Wu (13):
>   UefiCpuPkg: Add SmmRelocationLib class
>   UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
>   UefiCpuPkg/SmmRelocationLib: Rename global variables
>   UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation
>   UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
>   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
>   OvmfPkg/SmmRelocationLib: Add library instance for OVMF
>   OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
>   OvmfPkg: Refine SmmAccess implementation
>   OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
>   OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
>   UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
>   UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic
>
>  OvmfPkg/AmdSev/AmdSevX64.dsc   |   1 +
>  OvmfPkg/CloudHv/CloudHvX64.dsc |   1 +
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c| 104 ++--
>  .../Library/PlatformInitLib/PlatformInitLib.inf|   6 +-
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  33 +-
>  .../Library/SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../Library/SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
>  .../Library/SmmRelocationLib/SmmRelocationLib.c| 549
> +
>  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  60 +++
>  .../SmmRelocationLib/SmramSaveStateConfig.c| 100 
>  .../Library/SmmRelocationLib}/X64/Semaphore.c  |  13 +-
>  .../Library/SmmRelocationLib}/X64/SmmInit.nasm |  85 +++-
>  OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
>  OvmfPkg/OvmfPkgIa32.dsc|   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
>  OvmfPkg/OvmfPkgX64.dsc |   1 +
>  OvmfPkg/PlatformPei/Platform.c |   3 +
>  OvmfPkg/PlatformPei/Platform.h |   5 +
>  OvmfPkg/PlatformPei/PlatformPei.inf|   5 +-
>  OvmfPkg/PlatformPei/SmmRelocation.c|  80 +++
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.c  |   4 +-
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf|   5 +
>  OvmfPkg/SmmAccess/SmmAccessPei.c  

Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

[Ni, Ray]

Jiaxin,
I see Abdul from AMD has given R-B. I assume that's sufficient for merging this 
patch.

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Thursday, April 18, 2024 16:03
To: devel@edk2.groups.io ; Wu, Jiaxin 

Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R ; Dong, Guo 
; Rhodes, Sean ; Lu, James 
; Guo, Gua ; Ard Biesheuvel 
; Yao, Jiewen ; Abdul Lateef 
Attar ; Abner Chang ; Tom 
Lendacky 
Subject: RE: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

Hi Tom & Abner,

Could you help check & review AMD related patch?
 >   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD

Thanks,
Jiaxin

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu,
> Jiaxin
> Sent: Thursday, April 18, 2024 2:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Zeng, Star ; Gerd
> Hoffmann ; Kumar, Rahul R ;
> Dong, Guo ; Rhodes, Sean ;
> Lu, James ; Guo, Gua ; Ard
> Biesheuvel ; Yao, Jiewen
> ; Abdul Lateef Attar ;
> Abner Chang ; Tom Lendacky
> 
> Subject: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib
>
> PR: https://github.com/tianocore/edk2/pull/5546
>
> Intel plans to separate the smbase relocation logic from
> PiSmmCpuDxeSmm driver, and the related behavior will be
> moved to the new interface defined by the SmmRelocationLib
> class.
>
> The SmmRelocationLib class provides the SmmRelocationInit()
> interface for platform to do the smbase relocation, which
> shall provide below 2 functionalities:
> 1. Relocate smbases for each processor.
> 2. Create the gSmmBaseHobGuid HOB.
>
> With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
> a later phase) can be simplfied as below for SMM init:
> 1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
> for each Processor.
> 2. Execute the early SMM Init.
>
> Cc: Ray Ni 
> Cc: Zeng Star 
> Cc: Gerd Hoffmann 
> Cc: Rahul Kumar 
> Cc: Guo Dong 
> Cc: Sean Rhodes 
> Cc: James Lu 
> Cc: Gua Guo 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Abdul Lateef Attar 
> Cc: Abner Chang 
> Cc: Tom Lendacky 
> Signed-off-by: Jiaxin Wu 
>
> Jiaxin Wu (13):
>   UefiCpuPkg: Add SmmRelocationLib class
>   UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
>   UefiCpuPkg/SmmRelocationLib: Rename global variables
>   UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation
>   UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
>   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
>   OvmfPkg/SmmRelocationLib: Add library instance for OVMF
>   OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
>   OvmfPkg: Refine SmmAccess implementation
>   OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
>   OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
>   UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
>   UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic
>
>  OvmfPkg/AmdSev/AmdSevX64.dsc   |   1 +
>  OvmfPkg/CloudHv/CloudHvX64.dsc |   1 +
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c| 104 ++--
>  .../Library/PlatformInitLib/PlatformInitLib.inf|   6 +-
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  33 +-
>  .../Library/SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../Library/SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
>  .../Library/SmmRelocationLib/SmmRelocationLib.c| 549
> +
>  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  60 +++
>  .../SmmRelocationLib/SmramSaveStateConfig.c| 100 
>  .../Library/SmmRelocationLib}/X64/Semaphore.c  |  13 +-
>  .../Library/SmmRelocationLib}/X64/SmmInit.nasm |  85 +++-
>  OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
>  OvmfPkg/OvmfPkgIa32.dsc|   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
>  OvmfPkg/OvmfPkgX64.dsc |   1 +
>  OvmfPkg/PlatformPei/Platform.c |   3 +
>  OvmfPkg/PlatformPei/Platform.h |   5 +
>  OvmfPkg/PlatformPei/PlatformPei.inf|   5 +-
>  OvmfPkg/PlatformPei/SmmRelocation.c|  80 +++
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.c  |   4 +-
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf|   5 +
>  OvmfPkg/SmmAccess/SmmAccessPei.c   |  88 +---
>  OvmfPkg/SmmAccess/SmmAccessPei.inf |   7 +-
>  OvmfPkg/SmmAccess/SmramInternal.c  |  73 +--
>  OvmfPkg/SmmAccess/SmramInternal.h  |  18 +-
>  UefiCpuPkg/Include/Library/SmmRelocationLib.h  |  42 ++
>  .../SmmRelocationLib/AmdSmmRelocationLib.inf   |  60 +++
>  .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 125 +
>  .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
>  

Re: [edk2-devel] [PATCH v3 13/13] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic

[Wu, Jiaxin]

Ok, thanks Ray, I will follow the change in next version.

From: Ni, Ray 
Sent: Thursday, April 18, 2024 4:15 PM
To: Wu, Jiaxin ; devel@edk2.groups.io
Cc: Zeng, Star ; Gerd Hoffmann ; Kumar, 
Rahul R 
Subject: Re: [PATCH v3 13/13] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases 
relocation logic


+  //
+  // Check whether the Required TileSize is enough.
+  //
+  if (TileSize > SIZE_8KB) {
+DEBUG ((DEBUG_ERROR, "The Range of Smbase in SMRAM is not enough -- 
Required TileSize = 0x%08x, Actual TileSize = 0x%08x\n", TileSize, SIZE_8KB));
+FreePool (mCpuHotPlugData.SmBase);
+FreePool (gSmmCpuPrivate->ProcessorInfo);
+CpuDeadLoop ();
+return RETURN_BUFFER_TOO_SMALL;
   }

[Ray] Can you move the "TileSize" check just below the original TileSize 
calculation logic? Others look good to me.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118002): https://edk2.groups.io/g/devel/message/118002
Mute This Topic: https://groups.io/mt/105593584/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH V2 1/1] OvmfPkg/IntelTdx: Update TDVF README

[Min Xu]

From: Min M Xu 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4756

There are below updates in this patch:
1. Rename README to README.md so that it can be show as markdown
   document.
2. Update some information about TDVF.
2. Fix some typo.

Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Signed-off-by: Min Xu 
---
 OvmfPkg/IntelTdx/{README => README.md} | 64 +++---
 1 file changed, 38 insertions(+), 26 deletions(-)
 rename OvmfPkg/IntelTdx/{README => README.md} (52%)

diff --git a/OvmfPkg/IntelTdx/README b/OvmfPkg/IntelTdx/README.md
similarity index 52%
rename from OvmfPkg/IntelTdx/README
rename to OvmfPkg/IntelTdx/README.md
index 7307ede78faf..c168167c1271 100644
--- a/OvmfPkg/IntelTdx/README
+++ b/OvmfPkg/IntelTdx/README.md
@@ -1,13 +1,13 @@
 TDVF Overview
 -
 
-Intel Trust Domain Extension (TDX) is Intel Architecture extension
+**Intel Trust Domain Extension (TDX)** is Intel Architecture extension
 to provide trusted, isolated VM execution by removing CSP software
-(hypervisor etc) from the TCB. TDX Virtual Firmware (TDVF) is an
+(hypervisor etc) from the TCB. **TDX Virtual Firmware (TDVF)** is an
 EDK II based project to enable UEFI support for TDX based Virtual
 Machines. It provides the capability to launch a TD.
 
-The Intel? TDX Virtual Firmware Design Guide is at
+The **Intel TDX Virtual Firmware Design Guide** is at
 
https://www.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.01.pdf.
 
 More information can be found at:
@@ -19,49 +19,61 @@ Configurations and Features
 
 There are 2 configurations for TDVF.
 
-Config-A:
- - Merge the *basic* TDVF feature to existing OvmfX64Pkg.dsc. (Align
+**Config-A:**
+ - Merge the *basic* TDVF feature to existing **OvmfPkgX64.dsc**. (Align
with existing SEV)
- - Threat model: VMM is NOT out of TCB. (We don?t make things worse)
- - The OvmfX64Pkg.dsc includes SEV/TDX/normal OVMF basic boot capability.
+ - Threat model: VMM is **NOT** out of TCB. (We don't make things worse)
+ - The OvmfPkgX64.dsc includes SEV/TDX/normal OVMF basic boot capability.
The final binary can run on SEV/TDX/normal OVMF.
  - No changes to existing OvmfPkgX64 image layout.
  - No need to remove features if they exist today.
- - PEI phase is NOT skipped in either Td or Non-Td.
- - RTMR based measurement is supported.
+ - PEI phase is **NOT** skipped in either TD or Non-TD.
+ - RTMR based measurement (CC_MEASUREMENT) is supported as an optional 
requirement.
  - External inputs from Host VMM are measured, such as TdHob, CFV.
  - Other external inputs are measured, such as FW_CFG data, os loader,
initrd, etc.
 
-Config-B:
- - Add a standalone IntelTdx.dsc to a TDX specific directory for a *full*
-   feature TDVF.(Align with existing SEV)
+**Config-B:**
+ - Add a standalone **IntelTdxX64.dsc** to a TDX specific directory
+   (**OvmfPkg/IntelTdx**) for a *full* feature TDVF.(Align with existing SEV)
  - Threat model: VMM is out of TCB. (We need necessary change to prevent
attack from VMM)
- - IntelTdx.dsc includes TDX/normal OVMF basic boot capability. The final
+ - IntelTdxX64.dsc includes TDX/normal OVMF basic boot capability. The final
binary can run on TDX/normal OVMF.
  - It might eventually merge with AmdSev.dsc, but NOT at this point of
-   time. And we don?t know when it will happen. We need sync with AMD in
+   time. And we don't know when it will happen. We need sync with AMD in
the community after both of us think the solutions are mature to merge.
- - Need to add necessary security feature as mandatory requirement, such
-   as RTMR based Trusted Boot support.
- - Need to measure the external input from Host VMM, such as TdHob, CFV.
- - Need to measure other external input, such as FW_CFG data, os loader,
+ - RTMR based measurement (CC_MEASUREMENT) is supported as a mandatory 
requirement.
+ - External inputs from Host VMM are measured, such as TdHob, CFV.
+ - Other external inputs are measured, such as FW_CFG data, os loader,
initrd, etc.
- - Need to remove unnecessary attack surfaces, such as network stack.
+ - PEI phase is skipped to remove unnecessary attack surface.
+ - DXE FV is split into 2 FVs (DXEFV & NCCFV) to remove the unnecessary attack
+   surface in a TD guest..
+   - When launching a TD guest, only drivers in DXEFV are loaded.
+   - When launching a Non-TD guest, dirvers in both DXEFV and NCCFV are
+ loaded.
 
 Build
 --
 - Build the TDVF (Config-A) target:
-`cd /path/to/edk2`
-`source edksetup.sh`
-`build.sh -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t GCC5`
+```
+cd /path/to/edk2
+source edksetup.sh
+
+## without CC_MEASUREMENT enabled
+build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t GCC5 -b RELEASE
+
+## CC_MEASUREMENT enabled
+build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t GCC5 -D CC_MEASUREMENT_ENABLE=TRUE 
-b RELEASE
+```
 
 - Build the TDVF (Config-B) target:
-`cd /path/to/edk2`
-`set PACKAGES_PATH=/path/to/edk2/OvmfPkg`
-`source edksetup.sh`
-`build.sh -p 

Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/IntelTdx: Update TDVF README

[Min Xu]

On Thursday, April 18, 2024 7:50 PM, Gerd Hoffmann wrote:
> 
> > -The Intel? TDX Virtual Firmware Design Guide is at
> > +The Intel TDX Virtual Firmware Design Guide is at
> 
> '' looks more like HTML than markdown.
Thanks for reminder. It is fixed in v2.

> 
> text updates look fine to me.
> 

Thanks
Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118000): https://edk2.groups.io/g/devel/message/118000
Mute This Topic: https://groups.io/mt/105573458/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in edk2module is not working as expected

[Michael D Kinney]

The use of BitFieldOr64() is really only required when updating
a portion of a 32-bit or 64-bit value that are not aligned on
a 32-bit boundary and do not have a 32-bit aligned width.  Since
this use case is setting the lower 32-bits, simpler logic should
be used.

  data = LShiftU64(vedx, 32) | veax;

Mike

> -Original Message-
> From: Jayaprakash, N 
> Sent: Thursday, April 18, 2024 5:55 AM
> To: devel@edk2.groups.io
> Cc: Jayaprakash, N ; Rebecca Cran
> ; Kinney, Michael D 
> Subject: [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in
> edk2module is not working as expected
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4745
> 
> This commit fixes the issue reported in the BZ4745.
> The wrmsr was always writing 0 to the higher 32 bits of the msr
> register.
> This was due to a logical flaw in the code, where the input variable
> of
> type unsigned int was left shitted by 32 bits without explicitly
> converting it to a 64 bit value.
> 
> The issue is with the below statement.
> data = vedx << 32 | veax;
> Where the vedx which is an unsigned int, after left shifting by 32
> bits
> its value will be set to 0. Because of this the higher 32 bits of the
> MSR
> are always set to 0. This has been fixed by this commit.
> 
> Cc: Rebecca Cran 
> Cc: Michael D Kinney 
> Cc: Jayaprakash N 
> Signed-off-by: Jayaprakash N 
> ---
>  .../Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c   | 3
> ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c b/AppPkg/Applications/Python/Python-
> 3.6.8/PyMod-3.6.8/Modules/edk2module.c
> index 8786df8..06bcf82 100644
> --- a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c
> +++ b/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> 3.6.8/Modules/edk2module.c
> @@ -3886,7 +3886,8 @@ edk2_wrmsr(PyObject *self, PyObject *args)
>UINT64   data = 0;
>if (!PyArg_ParseTuple(args, "III", , , ))
>  return NULL;
> -  data = vedx << 32 | veax;
> +  data = LShiftU64(vedx, 32);
> +  data = BitFieldOr64(data, 0, 31, veax);
>Py_BEGIN_ALLOW_THREADS
>AsmWriteMsr64(vecx, data);
>Py_END_ALLOW_THREADS
> --
> 2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117999): https://edk2.groups.io/g/devel/message/117999
Mute This Topic: https://groups.io/mt/105597214/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 5/6] target/arm: Do memory type alignment check when translation disabled

[Jonathan Cameron via groups.io]

On Thu, 18 Apr 2024 09:15:55 +0100
Jonathan Cameron via  wrote:

> On Wed, 17 Apr 2024 13:07:35 -0700
> Richard Henderson  wrote:
> 
> > On 4/16/24 08:11, Jonathan Cameron wrote:  
> > > On Fri,  1 Mar 2024 10:41:09 -1000
> > > Richard Henderson  wrote:
> > > 
> > >> If translation is disabled, the default memory type is Device, which
> > >> requires alignment checking.  This is more optimally done early via
> > >> the MemOp given to the TCG memory operation.
> > >>
> > >> Reviewed-by: Philippe Mathieu-Daudé 
> > >> Reported-by: Idan Horowitz 
> > >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
> > >> Signed-off-by: Richard Henderson 
> > > 
> > > Hi Richard.
> > > 
> > > I noticed some tests I was running stopped booting with master.
> > > (it's a fun and complex stack of QEMU + kvm on QEMU for vCPU Hotplug 
> > > kernel work,
> > > but this is the host booting)
> > > 
> > > EDK2 build from upstream as of somepoint last week.
> > > 
> > > Bisects to this patch.
> > > 
> > >   qemu-system-aarch64 -M virt,gic-version=3,virtualization=true -m 
> > > 4g,maxmem=8G,slots=8 -cpu cortex-a76 -smp 
> > > cpus=4,threads=2,clusters=2,sockets=1 \
> > >   -kernel Image \
> > >   -drive if=none,file=full.qcow2,format=qcow2,id=hd \
> > >   -device ioh3420,id=root_port1 -device virtio-blk-pci,drive=hd \
> > >   -netdev user,id=mynet,hostfwd=tcp::-:22 -device 
> > > virtio-net-pci,netdev=mynet,id=bob \
> > >   -nographic -no-reboot -append 'earlycon root=/dev/vda2 fsck.mode=skip 
> > > tp_printk' \
> > >   -monitor telnet:127.0.0.1:1235,server,nowait -bios QEMU_EFI.fd \
> > >   -object memory-backend-ram,size=4G,id=mem0 \
> > >   -numa node,nodeid=0,cpus=0-3,memdev=mem0
> > > 
> > > Symptoms: Nothing on console from edk2 which is built in debug mode so is 
> > > normally very noisy.
> > >No sign of anything much happening at all :(
> > 
> > This isn't a fantastic bug report.
> > 
> > (1) If it doesn't boot efi, then none of the -kernel parameters are 
> > necessary.
> > (2) I'd be surprised if the full.qcow2 drive parameters are necessary 
> > either.
> >  But if they are, what contents?  Is a new empty drive sufficient, just
> >  enough to send the bios through the correct device initialization?
> > (3) edk2 build from ...
> >  Well, this is partly edk2's fault, as the build documentation is awful.
> >  I spent an entire afternoon trying to figure it out and gave up.
> > 
> > I will say that the edk2 shipped with qemu does work, so... are you 
> > absolutely
> > certain that it isn't a bug in edk2 since then?  Firmware bugs are exactly 
> > what
> > that patch is supposed to expose, as requested by issue #1204.
> > 
> > I'd say you should boot with "-d int" and see what kind of interrupts 
> > you're getting very 
> > early on.  I suspect that you'll see data aborts with ESR xx/yy where the 
> > last 6 bits of 
> > yy are 0x21 (alignment fault).  
> 
> Hi Richard,
> 
> Sorry for lack of details, I was aware it wasn't great and should have stated 
> I planned
> to come back with more details when I had time to debug.  Snowed under so for 
> now I've
> just dropped back to 8.2 and will get back to this perhaps next week.

+CC EDK2 list and Gerd.

Still not a thorough report but some breadcrumbs.

May be something about my local build setup as the shipped EDK2 succeeds,
but the one I'm building via
uefi-tools/edk2-build.sh armvirtqemu64
(some aged instructions here that are more or less working still)
https://people.kernel.org/jic23/

Indeed starts out with some alignment faults.

Gerd, any ideas?  Maybe I needs something subtly different in my
edk2 build?  I've not looked at this bit of the qemu infrastructure
before - is there a document on how that image is built?
As Richard observed, EDK2 isn't the simplest thing to build - I've
been using uefitools for this for a long time, so maybe I missed some
new requirement?

Build machine is x86_64 ubuntu, gcc 12.2.0.

I need to build it because of some necessary tweaks to debug a
PCI enumeration issue in Linux. (these tests were without those
tweaks)

As Richard observed, most of the command line isn't needed.

qemu-system-aarch64 -M virt,virtualization=true, -m 4g -cpu cortex-a76 \
-bios QEMU_EFI.fd -d int

Jonathan

 


> 
> Jonathan
> 
> > 
> > 
> > r~  
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117998): https://edk2.groups.io/g/devel/message/117998
Mute This Topic: https://groups.io/mt/105602816/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 6/6] OvmfPkg: Use newly defined Unaccepted Memory Type

[Sachin Ganesh via groups.io]

Hi Jiewen,

The other patches are as follows. They are all related to UEFI 2.10 and PI 1.8 
Specification updates:

1)  MdePkg: Add definition for NVMe Over Fabric Device Path - 
https://edk2.groups.io/g/devel/message/117845?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105551420
2) MdePkg: Add new Resource Attributes defined in PI 1.8 Spec - 
https://edk2.groups.io/g/devel/message/117796?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105540404
3) MdePkg: Use newly defined Unaccepted Memory Type - 
https://edk2.groups.io/g/devel/message/117797?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105540405
4) MdePkg: Update Delayed Dispatch PPI as per PI 1.8 Spec - 
https://edk2.groups.io/g/devel/message/117798?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105540406
5) MdePkg: Update to PI 1.8 Revision - 
https://edk2.groups.io/g/devel/message/117799?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105540407

This is the related MR - https://github.com/tianocore/edk2/pull/5569

Thank You,
Sachin.

-Original Message-
From: Yao, Jiewen 
Sent: Thursday, April 18, 2024 5:44 AM
To: Sachin Ganesh ; devel@edk2.groups.io
Cc: gaolim...@byosoft.com.cn; ardb+tianoc...@kernel.org; kra...@redhat.com; 
Aktas, Erdem ; Xu, Min M ; 
thomas.lenda...@amd.com; Felix Polyudov ; Dhanaraj V 

Subject: [EXTERNAL] RE: [PATCH 6/6] OvmfPkg: Use newly defined Unaccepted 
Memory Type


**CAUTION: The e-mail below is from an external source. Please exercise caution 
before opening attachments, clicking links, or following guidance.**

Hi Sachin
I like this clean up. Thanks for doing this.

I saw this patch is 6/6, but I did not see any other such as 1/6 ~ 5/6 in my 
mailbox. Not sure what is happening on my side.

Just double confirm, have you sent those patches?

Thank you
Yao, Jiewen

> -Original Message-
> From: Sachin Ganesh 
> Sent: Thursday, April 18, 2024 3:45 AM
> To: devel@edk2.groups.io
> Cc: gaolim...@byosoft.com.cn; ardb+tianoc...@kernel.org;
> kra...@redhat.com; Yao, Jiewen ; Aktas, Erdem
> ; Xu, Min M ;
> thomas.lenda...@amd.com; POLUDOV, FELIX ; Dhanaraj V
> ; Sachin Ganesh 
> Subject: [PATCH 6/6] OvmfPkg: Use newly defined Unaccepted Memory Type
>
> EFI_RESOURCE_MEMORY_UNACCEPTED has been officially defined in the PI
> 1.8 specification. So all temporary solutions have been replaced with
> the actual definition.
>
> Cc: Felix Polyudov 
> Cc: Dhanaraj V 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Gerd Hoffmann 
> Cc: Erdem Aktas 
> Cc: Min Xu 
> Cc: Tom Lendacky 
> Signed-off-by: Sachin Ganesh 
> ---
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c| 4 ++--
>  OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c | 8 
>  OvmfPkg/Library/PeilessStartupLib/Hob.c  | 4 ++--
>  OvmfPkg/Library/PlatformInitLib/IntelTdx.c   | 8 
>  OvmfPkg/PlatformPei/AmdSev.c | 4 ++--
>  5 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> b/OvmfPkg/AmdSevDxe/AmdSevDxe.c index db3675ae86..d497a343d3 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -20,7 +20,7 @@
>  #include 
>
>  #include 
>
>  #include 
>
> -#include 
>
> +#include 
>
>  #include 
>
>  #include 
>
>  #include 
>
> @@ -119,7 +119,7 @@ AcceptAllMemory (
>  CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *Desc;
>
>
>
>  Desc = [Index];
>
> -if (Desc->GcdMemoryType != EFI_GCD_MEMORY_TYPE_UNACCEPTED) {
>
> +if (Desc->GcdMemoryType != EfiGcdMemoryTypeUnaccepted) {
>
>continue;
>
>  }
>
>
>
> diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> index 3372cee2f7..b6085eab44 100644
> --- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> +++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> @@ -19,7 +19,7 @@
>  #include 
>
>  #include 
>
>  #include 
>
> -#include 
>
> +#include 
>
>  #include 
>
>  #include 
>
>  #include 
>
> @@ -351,7 +351,7 @@ AcceptMemoryForAPsStack (
>  if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
>
>DEBUG ((DEBUG_INFO, "\nResourceType: 0x%x\n",
> Hob.ResourceDescriptor-
> >ResourceType));
>
>
>
> -  if (Hob.ResourceDescriptor->ResourceType ==
> BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED) {
>
> +  if (Hob.ResourceDescriptor->ResourceType ==
> EFI_RESOURCE_MEMORY_UNACCEPTED) {
>
>  ResourceLength = Hob.ResourceDescriptor->ResourceLength;
>
>  PhysicalStart  = Hob.ResourceDescriptor->PhysicalStart;
>
>  PhysicalEnd= PhysicalStart + ResourceLength;
>
> @@ -427,7 +427,7 @@ AcceptMemory (
>//
>
>while (!END_OF_HOB_LIST (Hob)) {
>
>  if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
>
> -  if (Hob.ResourceDescriptor->ResourceType ==
> BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED) {
>
> +  if 

Re: [edk2-devel] [PATCH] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

[Peter Gonda via groups.io]

On Thu, Apr 18, 2024 at 9:39 AM Adam Dunlap  wrote:
>
> On Thu, Apr 18, 2024 at 5:15 AM Gerd Hoffmann  wrote:
> >
> > On Wed, Apr 17, 2024 at 09:54:00AM -0700, Adam Dunlap via groups.io wrote:
> > > +  UINT8  OpCode;
> >
> > The linux kernel patch uses "unsigned int opcode" and apparently
> > checks more than just the first byte for multi-byte opcodes.  Why
> > do it differently here?
>
> Good question. This patch does check for two-byte opcodes with this snippet:
>
> +  OpCode = *(InstructionData->OpCodes);
> +  if (OpCode == TWO_BYTE_OPCODE_ESCAPE) {
> +OpCode = *(InstructionData->OpCodes + 1);
> +  }
>
> This works because the first byte of two-byte opcodes is always 0x0f in the
> cases that we're checking for. I was wary about blindly dereferencing two
> bytes since that could cause a page fault if it was actually a 1 byte opcode
> that was at the very end of an allocated region. This is also what is done in
> the MmioExit function in this file. The linux kernel instruction decoder is 
> much
> more extensive than what is done here and I didn't want to duplicate the
> whole thing.
>
> > On the bigger picture:  I'm wondering why SNP allows external #VC
> > injections in the first place?
>
> Yup, I think it'd be better if it didn't.

I think this is a small mitigation until linux + edk2 guest's support
restricted or alternate interrupt injection. I suggested Adam send
this just to have parity between edk2 and linux.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117996): https://edk2.groups.io/g/devel/message/117996
Mute This Topic: https://groups.io/mt/105581633/21656
Mute #vc:https://edk2.groups.io/g/devel/mutehashtag/vc
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

[Adam Dunlap via groups.io]

On Thu, Apr 18, 2024 at 5:15 AM Gerd Hoffmann  wrote:
>
> On Wed, Apr 17, 2024 at 09:54:00AM -0700, Adam Dunlap via groups.io wrote:
> > +  UINT8  OpCode;
>
> The linux kernel patch uses "unsigned int opcode" and apparently
> checks more than just the first byte for multi-byte opcodes.  Why
> do it differently here?

Good question. This patch does check for two-byte opcodes with this snippet:

+  OpCode = *(InstructionData->OpCodes);
+  if (OpCode == TWO_BYTE_OPCODE_ESCAPE) {
+OpCode = *(InstructionData->OpCodes + 1);
+  }

This works because the first byte of two-byte opcodes is always 0x0f in the
cases that we're checking for. I was wary about blindly dereferencing two
bytes since that could cause a page fault if it was actually a 1 byte opcode
that was at the very end of an allocated region. This is also what is done in
the MmioExit function in this file. The linux kernel instruction decoder is much
more extensive than what is done here and I didn't want to duplicate the
whole thing.

> On the bigger picture:  I'm wondering why SNP allows external #VC
> injections in the first place?

Yup, I think it'd be better if it didn't.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117995): https://edk2.groups.io/g/devel/message/117995
Mute This Topic: https://groups.io/mt/105581633/21656
Mute #vc:https://edk2.groups.io/g/devel/mutehashtag/vc
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce one bug and will cause SUT reset when boot to windows

[Oliver Smith-Denny]

On 4/18/2024 6:56 AM, Huang, Yanbo wrote:

The PCD PcdPlatformEfiRtCodeMemorySize is used in 
https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c
This PCD seems defined the size allocated for run time services code, and the 
similar PCD is PcdPlatformEfiRtDataMemorySize, seems defined the size allocated 
for run time services data.
I guess dandan means if the runtime services code size is small than the PCD defined, 
then the " Extra EfiRuntimeServicesCode regions which aren't part of loaded runtime 
images." size should be FixedPcdGet32 (PcdPlatformEfiRtCodeMemorySize) - the actual 
runtime image code size?



Drawing the lines here, we will lie in the EFI_MEMORY_MAP to say that
all bin memory is allocated, regardless of this PCD:

https://github.com/tianocore/edk2/blob/0afb8743493853e30171f6000de51242e22a1eb8/MdeModulePkg/Core/Dxe/Mem/Page.c#L1973-L1989

So this is just junk unallocated memory that we are reporting as
a type it *could* be if an allocation occurs to minimize failures
of ExitBootServices. Which is questionable. But in terms of
attributes, I would expect we either have this unallocated
memory marked the same as the bin type or better, mark it RP
if we can (Taylor is making a change to set RP on free memory
by default, so we would have this in the page table, but we
would need to decide what we tell the OS).

Going back to the questionableness of this: can we not report
the entire bin as allocated memory to the OS? I understand
what the comment is saying; I don't like that we will lie to
the OS and reserve larger chunks of runtime memory than we
actually need and that will get the wrong permissions set on
it (for example making this garbage data be executable).

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117994): https://edk2.groups.io/g/devel/message/117994
Mute This Topic: https://groups.io/mt/105477564/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Yao, Jiewen]

1) Yes, I highly recommend remove Q35 keyword.
2) Got it. I think we had better add such info in the code as comment as well.

Thank you
Yao, Jiewen

> -Original Message-
> From: kra...@redhat.com 
> Sent: Thursday, April 18, 2024 7:45 PM
> To: Yao, Jiewen 
> Cc: devel@edk2.groups.io; Ard Biesheuvel ; Oliver Steffen
> 
> Subject: Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
> 
> On Wed, Apr 17, 2024 at 01:20:57PM +, Yao, Jiewen wrote:
> > That is good start. The SMRAM lock and Flash lock seem good to me.
> >
> > Comment:
> > 1) Do we really need to add "Q35" for the policy?
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH  BIT1
> >
> > I feel we had better remove it, since SMM_SMRAM_LOCK and
> SMM_SECURE_VARS_FLASH are common features for almost all X86 platforms.
> 
> Well, SMM mode is supported for the qemu 'q35' machine type only, the
> 'pc' machine type doesn't provide enough memory for SMM.  Which why I've
> added 'Q35' to the name.
> 
> The SMM_SMRAM_LOCK test actually is q35-specific because the control
> registers are chipset specific.  But, yes, the concept is not q35
> specific.
> 
> I can drop 'Q35' if you prefer it that way.
> 
> > 2) Would you please let me know what "READONLY_CODE_FLASH" really
> means?
> >
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH  BIT1
> > #define VIRT_HSTI_BYTE0_READONLY_CODE_FLASHBIT2
> >
> > Does READONLY_CODE_FLASH mean NO write to flash even in SMM mode?
> > Or does it just mean NO write in normal operation mode, but still writable 
> > in
> SMM mode?
> 
> With qemu being configured properly flash behavior should be this:
> 
>|  OVMF_CODE.fd  |  OVMF_VARS.fd
> ---++
> SMM_REQUIRE=TRUE, SMM mode |  read-only |  writable
> SMM_REQUIRE=TRUE, normal mode  |  read-only (1) |  read-only (2)
> SMM_REQUIRE=FALSE  |  read-only (3) |  writable
> 
> VIRT_HSTI_BYTE0_READONLY_CODE_FLASH will verify (1) + (3).
> VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH will verify (2).
> 
> (probably a good idea to add that as comment to the patches).
> 
> take care,
>   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117993): https://edk2.groups.io/g/devel/message/117993
Mute This Topic: https://groups.io/mt/105086174/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce one bug and will cause SUT reset when boot to windows

[Huang, Yanbo]

The PCD PcdPlatformEfiRtCodeMemorySize is used in 
https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c
This PCD seems defined the size allocated for run time services code, and the 
similar PCD is PcdPlatformEfiRtDataMemorySize, seems defined the size allocated 
for run time services data.
I guess dandan means if the runtime services code size is small than the PCD 
defined, then the " Extra EfiRuntimeServicesCode regions which aren't part of 
loaded runtime images." size should be FixedPcdGet32 
(PcdPlatformEfiRtCodeMemorySize) - the actual runtime image code size?

Best Regards,
Yanbo Huang
-Original Message-
From: Ard Biesheuvel  
Sent: Thursday, April 18, 2024 9:18 PM
To: devel@edk2.groups.io; Bi, Dandan 
Cc: Taylor Beebe ; Huang, Yanbo 
; Wang, Jian J ; Gao, Liming 
; Zhou, Jianfeng 
Subject: Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce 
one bug and will cause SUT reset when boot to windows

Hello Dandan,

On Thu, 18 Apr 2024 at 15:03, Dandan Bi  wrote:
>
> Hi Taylor,
>
>
>
> >>Extra EfiRuntimeServicesCode regions which aren't part of loaded runtime 
> >>images.
>
> This may be related to the original size of EfiRuntimeServicesCode in memory 
> map, and the size can be configured via PcdPlatformEfiRtCodeMemorySize.
>

That PCD does not exist in EDK2. Can you explain what it does on your platform?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117992): https://edk2.groups.io/g/devel/message/117992
Mute This Topic: https://groups.io/mt/105477564/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 6/6] OvmfPkg: Use newly defined Unaccepted Memory Type

[Yao, Jiewen]

Ah. That is good. I did not realize they are in one set.

For this one, reviewed-by: Jiewen Yao 


> -Original Message-
> From: Sachin Ganesh 
> Sent: Thursday, April 18, 2024 9:32 PM
> To: Yao, Jiewen ; devel@edk2.groups.io
> Cc: gaolim...@byosoft.com.cn; ardb+tianoc...@kernel.org; kra...@redhat.com;
> Aktas, Erdem ; Xu, Min M ;
> thomas.lenda...@amd.com; POLUDOV, FELIX ; Dhanaraj V
> 
> Subject: RE: [EXTERNAL] RE: [PATCH 6/6] OvmfPkg: Use newly defined
> Unaccepted Memory Type
> 
> Hi Jiewen,
> 
> The other patches are as follows. They are all related to UEFI 2.10 and PI 1.8
> Specification updates:
> 
> 1)  MdePkg: Add definition for NVMe Over Fabric Device Path -
> https://edk2.groups.io/g/devel/message/117845?p=%2C%2C%2C20%2C0%2C0
> %2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105
> 551420
> 2) MdePkg: Add new Resource Attributes defined in PI 1.8 Spec -
> https://edk2.groups.io/g/devel/message/117796?p=%2C%2C%2C20%2C0%2C0
> %2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105
> 540404
> 3) MdePkg: Use newly defined Unaccepted Memory Type -
> https://edk2.groups.io/g/devel/message/117797?p=%2C%2C%2C20%2C0%2C0
> %2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105
> 540405
> 4) MdePkg: Update Delayed Dispatch PPI as per PI 1.8 Spec -
> https://edk2.groups.io/g/devel/message/117798?p=%2C%2C%2C20%2C0%2C0
> %2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105
> 540406
> 5) MdePkg: Update to PI 1.8 Revision -
> https://edk2.groups.io/g/devel/message/117799?p=%2C%2C%2C20%2C0%2C0
> %2C0%3A%3Arecentpostdate%2Fsticky%2C%2Csachin%2C20%2C2%2C0%2C105
> 540407
> 
> This is the related MR - https://github.com/tianocore/edk2/pull/5569
> 
> Thank You,
> Sachin.
> 
> -Original Message-
> From: Yao, Jiewen 
> Sent: Thursday, April 18, 2024 5:44 AM
> To: Sachin Ganesh ; devel@edk2.groups.io
> Cc: gaolim...@byosoft.com.cn; ardb+tianoc...@kernel.org; kra...@redhat.com;
> Aktas, Erdem ; Xu, Min M ;
> thomas.lenda...@amd.com; Felix Polyudov ; Dhanaraj V
> 
> Subject: [EXTERNAL] RE: [PATCH 6/6] OvmfPkg: Use newly defined Unaccepted
> Memory Type
> 
> 
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
> 
> Hi Sachin
> I like this clean up. Thanks for doing this.
> 
> I saw this patch is 6/6, but I did not see any other such as 1/6 ~ 5/6 in my
> mailbox. Not sure what is happening on my side.
> 
> Just double confirm, have you sent those patches?
> 
> Thank you
> Yao, Jiewen
> 
> > -Original Message-
> > From: Sachin Ganesh 
> > Sent: Thursday, April 18, 2024 3:45 AM
> > To: devel@edk2.groups.io
> > Cc: gaolim...@byosoft.com.cn; ardb+tianoc...@kernel.org;
> > kra...@redhat.com; Yao, Jiewen ; Aktas, Erdem
> > ; Xu, Min M ;
> > thomas.lenda...@amd.com; POLUDOV, FELIX ; Dhanaraj V
> > ; Sachin Ganesh 
> > Subject: [PATCH 6/6] OvmfPkg: Use newly defined Unaccepted Memory Type
> >
> > EFI_RESOURCE_MEMORY_UNACCEPTED has been officially defined in the PI
> > 1.8 specification. So all temporary solutions have been replaced with
> > the actual definition.
> >
> > Cc: Felix Polyudov 
> > Cc: Dhanaraj V 
> > Cc: Ard Biesheuvel 
> > Cc: Jiewen Yao 
> > Cc: Gerd Hoffmann 
> > Cc: Erdem Aktas 
> > Cc: Min Xu 
> > Cc: Tom Lendacky 
> > Signed-off-by: Sachin Ganesh 
> > ---
> >  OvmfPkg/AmdSevDxe/AmdSevDxe.c| 4 ++--
> >  OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c | 8 
> >  OvmfPkg/Library/PeilessStartupLib/Hob.c  | 4 ++--
> >  OvmfPkg/Library/PlatformInitLib/IntelTdx.c   | 8 
> >  OvmfPkg/PlatformPei/AmdSev.c | 4 ++--
> >  5 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > b/OvmfPkg/AmdSevDxe/AmdSevDxe.c index db3675ae86..d497a343d3 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -20,7 +20,7 @@
> >  #include 
> >
> >  #include 
> >
> >  #include 
> >
> > -#include 
> >
> > +#include 
> >
> >  #include 
> >
> >  #include 
> >
> >  #include 
> >
> > @@ -119,7 +119,7 @@ AcceptAllMemory (
> >  CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *Desc;
> >
> >
> >
> >  Desc = [Index];
> >
> > -if (Desc->GcdMemoryType != EFI_GCD_MEMORY_TYPE_UNACCEPTED) {
> >
> > +if (Desc->GcdMemoryType != EfiGcdMemoryTypeUnaccepted) {
> >
> >continue;
> >
> >  }
> >
> >
> >
> > diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> > b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> > index 3372cee2f7..b6085eab44 100644
> > --- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> > +++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
> > @@ -19,7 +19,7 @@
> >  #include 
> >
> >  #include 
> >
> >  #include 
> >
> > -#include 
> >
> > +#include 
> >
> >  #include 
> >
> >  #include 
> >
> >  #include 
> >
> > @@ -351,7 +351,7 @@ AcceptMemoryForAPsStack (
> >  if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) 

Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce one bug and will cause SUT reset when boot to windows

[Ard Biesheuvel]

Hello Dandan,

On Thu, 18 Apr 2024 at 15:03, Dandan Bi  wrote:
>
> Hi Taylor,
>
>
>
> >>Extra EfiRuntimeServicesCode regions which aren't part of loaded runtime 
> >>images.
>
> This may be related to the original size of EfiRuntimeServicesCode in memory 
> map, and the size can be configured via PcdPlatformEfiRtCodeMemorySize.
>

That PCD does not exist in EDK2. Can you explain what it does on your platform?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117990): https://edk2.groups.io/g/devel/message/117990
Mute This Topic: https://groups.io/mt/105477564/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce one bug and will cause SUT reset when boot to windows

[Dandan Bi]

Hi Taylor,

>>Extra EfiRuntimeServicesCode regions which aren't part of loaded runtime 
>>images.
This may be related to the original size of EfiRuntimeServicesCode in memory 
map, and the size can be configured via PcdPlatformEfiRtCodeMemorySize.

If the size is large enough to hold all the runtime images and still has some 
remaining regions, then these regions are with EfiRuntimeServicesCode type and 
aren't part of loaded runtime images, right?
Pre-change 
https://github.com/tianocore/edk2/commit/e2f2bbe208b4c7ebcedacfc8333df1e52cbf07eb,
 such EfiRuntimeServicesCode regions are set with EFI_MEMORY_XP attribute.
Post-change 
https://github.com/tianocore/edk2/commit/e2f2bbe208b4c7ebcedacfc8333df1e52cbf07eb,
  these regions don’t have any access attribute.
And now this patch [PATCH v1] MdeModulePkg: Fixup MAT Attributes After 
Splitting EFI Memory Map 
(groups.io) set these regions 
with EFI_MEMORY_RO.
I am not sure which attribute should be set for these regions.  And old codes 
with EFI_MEMORY_XP attribute for a long time and seems not cause any issue.


Thanks,
Dandan
From: Taylor Beebe 
Sent: Thursday, April 18, 2024 7:54 AM
To: Huang, Yanbo ; devel@edk2.groups.io; Bi, Dandan 

Cc: Wang, Jian J ; Gao, Liming 
; Zhou, Jianfeng 
Subject: Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce 
one bug and will cause SUT reset when boot to windows


Hi Yanbo,
I didn't do it in the way you suggest for the same reason that the SplitTable() 
logic doesn't set attributes
on descriptors of type EfiRuntimeServicesData or other memory types. The 
purpose of the SplitTable() function
is to use the input image records to split descriptors so each image section 
has it's own descriptor. I think
it's reasonable to set the attributes on descriptors associated with images 
because those new descriptors are the
intended side effect of the function, but I don't think setting attributes on 
other descriptors is a good design pattern.
This pattern matches what's done in PiSmmCore/MemoryAttributesTable.c. Also, 
even if we did
it the way you suggest, we would still need call EnforceMemoryMapAttribute() 
later to set XP on the
EfiRuntimeServicesData descriptors.

Can you or Dandan explain the origin of the extra  EfiRuntimeServicesCode 
regions which aren't
part of loaded runtime images? It would be a good datapoint for our discussion 
on the proposed fix.

-Taylor
On 4/17/2024 7:04 AM, Huang, Yanbo wrote:
Hi Taylor,

Thanks for your update.
After test, issue can be fixed by your patch.
But why we not set the EFI_MEMORY_XP or EFI_MEMORY_RO attribute in SplitRecord 
API?
If we set the attribute in the beginning of the NewRecord created, it seems we 
don’t need to EnforceMemoryMapAttribute later?

Best Regards,
Yanbo Huang


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117989): https://edk2.groups.io/g/devel/message/117989
Mute This Topic: https://groups.io/mt/105477564/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-libc Patch 2 1/1] ek2-libc: wrmsr function available in edk2module is not working as expected

[Jayaprakash, N]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4745

This commit fixes the issue reported in the BZ4745.
The wrmsr was always writing 0 to the higher 32 bits of the msr register.
This was due to a logical flaw in the code, where the input variable of
type unsigned int was left shitted by 32 bits without explicitly
converting it to a 64 bit value.

The issue is with the below statement.
data = vedx << 32 | veax;
Where the vedx which is an unsigned int, after left shifting by 32 bits
its value will be set to 0. Because of this the higher 32 bits of the MSR
are always set to 0. This has been fixed by this commit.

Cc: Rebecca Cran 
Cc: Michael D Kinney 
Cc: Jayaprakash N 
Signed-off-by: Jayaprakash N 
---
 .../Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c 
b/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c
index 8786df8..06bcf82 100644
--- a/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c
+++ b/AppPkg/Applications/Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c
@@ -3886,7 +3886,8 @@ edk2_wrmsr(PyObject *self, PyObject *args)
   UINT64   data = 0;
   if (!PyArg_ParseTuple(args, "III", , , ))
 return NULL;
-  data = vedx << 32 | veax;
+  data = LShiftU64(vedx, 32);
+  data = BitFieldOr64(data, 0, 31, veax);
   Py_BEGIN_ALLOW_THREADS
   AsmWriteMsr64(vecx, data);
   Py_END_ALLOW_THREADS
-- 
2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117988): https://edk2.groups.io/g/devel/message/117988
Mute This Topic: https://groups.io/mt/105597214/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-libc Patch 2 0/1] wrmsr function is not working as expected.

[Jayaprakash, N]

The wrmsr function always writes 0s to the higher 32 bits of the msr register.
This PR fixes the this issue reported through the BZ4745.

Jayaprakash N (1):
  ek2-libc: wrmsr function available in edk2module is not working as
expected

 .../Python/Python-3.6.8/PyMod-3.6.8/Modules/edk2module.c   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117987): https://edk2.groups.io/g/devel/message/117987
Mute This Topic: https://groups.io/mt/105597212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg: introduce RedfishBootstrapAccountDxe

[Nickle Wang via groups.io]

-Introduce RedfishBootstrapAccountDxe to delete bootstrap
account from /redfish/v1/AccountService/Accounts after BIOS
finished all Redfish jobs. The bootstrap account won't be
available to other application. So deleting bootstrap account
helps to release resource at BMC.
- After bootstrap account is deleted at BMC, the Redfish service
instance is no longer usable. Close Redfish service instance to
release the HTTP connection between BIOS and BMC.

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
Cc: Nick Ramirez 
---
 .../RedfishClientComponents.dsc.inc   |   1 +
 .../RedfishBootstrapAccountDxe.inf|  53 +++
 .../RedfishBootstrapAccountDxe.h  |  58 
 .../RedfishBootstrapAccountDxe.c  | 328 ++
 RedfishClientPkg/RedfishClient.fdf.inc|   1 +
 5 files changed, 441 insertions(+)
 create mode 100644 
RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.inf
 create mode 100644 
RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.h
 create mode 100644 
RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.c

diff --git a/RedfishClientPkg/RedfishClientComponents.dsc.inc 
b/RedfishClientPkg/RedfishClientComponents.dsc.inc
index 42fc0c299..fe5248b62 100644
--- a/RedfishClientPkg/RedfishClientComponents.dsc.inc
+++ b/RedfishClientPkg/RedfishClientComponents.dsc.inc
@@ -20,6 +20,7 @@
   RedfishClientPkg/HiiToRedfishMemoryDxe/HiiToRedfishMemoryDxe.inf
   RedfishClientPkg/HiiToRedfishBootDxe/HiiToRedfishBootDxe.inf
   RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosDxe.inf
+  RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.inf
 !endif
   #
   # Below two modules should be pulled in by build tool.
diff --git 
a/RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.inf 
b/RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.inf
new file mode 100644
index 0..4073e95f4
--- /dev/null
+++ b/RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.inf
@@ -0,0 +1,53 @@
+## @file
+#  This driver deletes bootstrap account in BMC after BIOS Redfish finished
+#  all jobs
+#
+#  (C) Copyright 2021 Hewlett Packard Enterprise Development LP
+#  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION   = 0x0001000b
+  BASE_NAME = RedfishBootstrapAccountDxe
+  FILE_GUID = 87555253-2F7E-45FC-B469-FD35B2E51210
+  MODULE_TYPE   = DXE_DRIVER
+  VERSION_STRING= 1.0
+  ENTRY_POINT   = RedfishBootstrapAccountEntryPoint
+  UNLOAD_IMAGE  = RedfishBootstrapAccountUnload
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  RedfishPkg/RedfishPkg.dec
+  RedfishClientPkg/RedfishClientPkg.dec
+
+[Sources]
+  RedfishBootstrapAccountDxe.h
+  RedfishBootstrapAccountDxe.c
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  MemoryAllocationLib
+  PrintLib
+  RedfishEventLib
+  RedfishFeatureUtilityLib
+  RedfishDebugLib
+  RedfishVersionLib
+  RedfishHttpLib
+  UefiLib
+  UefiBootServicesTableLib
+  UefiRuntimeServicesTableLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEdkIIRedfishConfigHandlerProtocolGuid  ## CONSUMES ##
+  gEdkIIRedfishCredentialProtocolGuid ## CONSUMES ##
+  gEfiRestExProtocolGuid  ## CONSUMES ##
+
+[Depex]
+  gEdkIIRedfishCredentialProtocolGuid
diff --git 
a/RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.h 
b/RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.h
new file mode 100644
index 0..5262f1e6b
--- /dev/null
+++ b/RedfishClientPkg/RedfishBootstrapAccountDxe/RedfishBootstrapAccountDxe.h
@@ -0,0 +1,58 @@
+/** @file
+  Common header file for RedfishBootstrapAccountDxe driver.
+
+  (C) Copyright 2021-2022 Hewlett Packard Enterprise Development LP
+  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef REDFISH_BOOTSTRAP_ACCOUNT_DXE_H_
+#define REDFISH_BOOTSTRAP_ACCOUNT_DXE_H_
+
+#include 
+#include 
+
+//
+// Libraries
+//
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define REDFISH_BOOTSTRAP_ACCOUNT_DEBUG DEBUG_VERBOSE
+#define REDFISH_MANAGER_ACCOUNT_COLLECTION_URI  L"AccountService/Accounts"
+#define REDFISH_URI_LENGTH  128
+
+//
+// Definitions of REDFISH_BOOTSTRAP_ACCOUNT_PRIVATE
+//
+typedef struct {
+  EFI_HANDLE   ImageHandle;
+  EFI_HANDLE   RestExHandle;
+  REDFISH_SERVICE  RedfishService;
+  EFI_EVENTRedfishEvent;
+  

Re: [edk2-devel] [PATCH] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

[Gerd Hoffmann]

On Wed, Apr 17, 2024 at 09:54:00AM -0700, Adam Dunlap via groups.io wrote:
> Ensure that when a #VC exception happens, the instruction at the
> instruction pointer matches the instruction that is expected given the
> error code. This is to mitigate the ahoi WeSee attack [1] that could
> allow hypervisors to breach integrity and confidentiality of the
> firmware by maliciously injecting interrupts. This change is a
> translated version of a linux patch e3ef461af35a ("x86/sev: Harden #VC
> instruction emulation somewhat")

> +**/
> +STATIC
> +UINT64
> +VcCheckOpcodeBytes (
> +  IN OUT GHCB*Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN OUT CC_INSTRUCTION_DATA *InstructionData,
> +  IN UINT64  ExitCode
> +  )
> +{
> +  UINT8  OpCode;

The linux kernel patch uses "unsigned int opcode" and apparently
checks more than just the first byte for multi-byte opcodes.  Why
do it differently here?

On the bigger picture:  I'm wondering why SNP allows external #VC
injections in the first place?

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117985): https://edk2.groups.io/g/devel/message/117985
Mute This Topic: https://groups.io/mt/105581633/21656
Mute #vc:https://edk2.groups.io/g/devel/mutehashtag/vc
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/IntelTdx: Update TDVF README

[Gerd Hoffmann]

  Hi,

> -The Intel? TDX Virtual Firmware Design Guide is at
> +The Intel TDX Virtual Firmware Design Guide is at

'' looks more like HTML than markdown.

text updates look fine to me.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117984): https://edk2.groups.io/g/devel/message/117984
Mute This Topic: https://groups.io/mt/105573458/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Gerd Hoffmann]

On Wed, Apr 17, 2024 at 01:20:57PM +, Yao, Jiewen wrote:
> That is good start. The SMRAM lock and Flash lock seem good to me.
> 
> Comment:
> 1) Do we really need to add "Q35" for the policy?
> #define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
> #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH  BIT1
> 
> I feel we had better remove it, since SMM_SMRAM_LOCK and 
> SMM_SECURE_VARS_FLASH are common features for almost all X86 platforms.

Well, SMM mode is supported for the qemu 'q35' machine type only, the
'pc' machine type doesn't provide enough memory for SMM.  Which why I've
added 'Q35' to the name.

The SMM_SMRAM_LOCK test actually is q35-specific because the control
registers are chipset specific.  But, yes, the concept is not q35
specific.

I can drop 'Q35' if you prefer it that way.

> 2) Would you please let me know what "READONLY_CODE_FLASH" really means?
> 
> #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH  BIT1
> #define VIRT_HSTI_BYTE0_READONLY_CODE_FLASHBIT2
> 
> Does READONLY_CODE_FLASH mean NO write to flash even in SMM mode?
> Or does it just mean NO write in normal operation mode, but still writable in 
> SMM mode?

With qemu being configured properly flash behavior should be this:

   |  OVMF_CODE.fd  |  OVMF_VARS.fd
---++
SMM_REQUIRE=TRUE, SMM mode |  read-only |  writable
SMM_REQUIRE=TRUE, normal mode  |  read-only (1) |  read-only (2)
SMM_REQUIRE=FALSE  |  read-only (3) |  writable

VIRT_HSTI_BYTE0_READONLY_CODE_FLASH will verify (1) + (3).
VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH will verify (2).

(probably a good idea to add that as comment to the patches).

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117983): https://edk2.groups.io/g/devel/message/117983
Mute This Topic: https://groups.io/mt/105086174/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Gerd Hoffmann]

On Wed, Apr 17, 2024 at 01:38:20PM +0200, Ard Biesheuvel wrote:
> On Wed, 17 Apr 2024 at 10:18, Gerd Hoffmann  wrote:
> >
> > On Fri, Mar 22, 2024 at 03:27:31PM +0100, Gerd Hoffmann wrote:
> > >
> > >
> > > Gerd Hoffmann (2):
> > >   OvmfPkg/VirtHstiDxe: add varstore flash check
> > >   OvmfPkg/VirtHstiDxe: add code flash check
> > >
> > > Konstantin Kostiuk (2):
> > >   OvmfPkg: Add VirtHstiDxe driver
> > >   OvmfPkg: Add VirtHstiDxe to OVMF firmware build
> >
> > Ping.  Any comments on this series?
> >
> 
> Dunno. What does it do?

It implements 
https://learn.microsoft.com/en-us/windows-hardware/test/hlk/testref/hardware-security-testability-specification
for OVMF.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117982): https://edk2.groups.io/g/devel/message/117982
Mute This Topic: https://groups.io/mt/105086174/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature based on PFP 1.06 spec

[Wenxing Hou]

Dear EDKII reviewers:

Thank you for your previous review of this patch set.
Currently, five patches have been reviewed by. 

But there are five patches need review.
Patch1:  MdePkg: Add SPDM1.2 support.
Patch2:  MdePkg: Add TCG PFP 1.06 support.
Patch4:  MdeModulePkg/Variable: Add TCG SPDM device measurement update
Patch8:  .gitmodule: Add libspdm submodule for EDKII
Patch10: ReadMe.rst: Add libspdm submodule license

Could you please review the PATCH v4?

PS: Jiewen has reviewed all the PATCH. And I have fixed his feedback in PATCH 
v4. Jiewen has no questions about all the patches anymore.

Thanks,
Wenxing


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Wenxing Hou
Sent: Thursday, April 18, 2024 5:28 PM
To: devel@edk2.groups.io
Cc: Andrew Fish ; Leif Lindholm ; 
Kinney, Michael D ; Liming Gao 
; Sean Brogan ; Joey 
Vagedes ; Liu, Zhiguang ; 
Kumar, Rahul R ; Yao, Jiewen 
Subject: [edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature based on PFP 
1.06 spec

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2479

In PFP spec 1.06, platform firmware records the device certificate and device 
measurement for each SPDM responder.
This PATCH set implement the DeviceSecurityLib to support spdm device 
Authentication and Measurement.

Libspdm as submodule is to support DeviceSecurity feature:
https://github.com/DMTF/libspdm

TCG PFP spec 1.06:
https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/

The POC branch:
https://github.com/tianocore/edk2-staging/tree/DeviceSecurity

And the PATCH set has passed the EDKII CI:
https://github.com/tianocore/edk2/pull/5508

v2 changes:
 - Fix typo: PcdEnableSpdmDeviceAuthenticaion -> 
PcdEnableSpdmDeviceAuthentication
v3 changes:
 - Add new patch 10: Update ReadMe.rst for libspdm submodule license
v4 changes:
 - Update submodule libspdm to latest tag

PATCH 3: Reviewed-by: Liming Gao  PATCH 5: 
Reviewed-by: Jiewen Yao  PATCH 6: Reviewed-by: Jiewen Yao 
 PATCH 7: Reviewed-by: Joey Vagedes 
 PATCH 9: Reviewed-by: Jiewen Yao 

Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Joey Vagedes 
Cc: Zhiguang Liu 
Cc: Rahul Kumar 
Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 

Wenxing Hou (10):
  MdePkg: Add SPDM1.2 support.
  MdePkg: Add TCG PFP 1.06 support.
  MdePkg: Add devAuthBoot GlobalVariable
  MdeModulePkg/Variable: Add TCG SPDM device measurement update
  SecurityPkg: Add TCG PFP 1.06 support.
  SecurityPkg: add DeviceSecurity support
  .pytool/CISettings.py: add libspdm submodule.
  .gitmodule: Add libspdm submodule for EDKII
  SecurityPkg: Add libspdm submodule
  ReadMe.rst: Add libspdm submodule license

 .gitmodules   |3 +
 .pytool/CISettings.py |2 +
 MdeModulePkg/MdeModulePkg.dec |5 +
 .../Variable/RuntimeDxe/Measurement.c |   38 +-
 .../RuntimeDxe/VariableRuntimeDxe.inf |3 +
 .../RuntimeDxe/VariableSmmRuntimeDxe.inf  |3 +
 MdePkg/Include/Guid/GlobalVariable.h  |8 +-
 MdePkg/Include/Guid/ImageAuthentication.h |5 +-
 MdePkg/Include/IndustryStandard/Spdm.h| 1112 -
 .../IndustryStandard/UefiTcgPlatform.h|  186 ++-
 ReadMe.rst|1 +
 .../OsStub/CryptlibWrapper/CryptlibWrapper.c  |  970 ++
 .../CryptlibWrapper/CryptlibWrapper.inf   |   38 +
 .../OsStub/MemLibWrapper/MemLibWrapper.c  |  177 +++
 .../OsStub/MemLibWrapper/MemLibWrapper.inf|   33 +
 .../PlatformLibWrapper/PlatformLibWrapper.c   |   85 ++
 .../PlatformLibWrapper/PlatformLibWrapper.inf |   33 +
 .../SpdmLib/Include/Stub/SpdmLibStub.h|  347 +
 .../SpdmLib/Include/hal/LibspdmStdBoolAlt.h   |   23 +
 .../SpdmLib/Include/hal/LibspdmStdDefAlt.h|   16 +
 .../SpdmLib/Include/hal/LibspdmStdIntAlt.h|   25 +
 .../DeviceSecurity/SpdmLib/Include/hal/base.h |   94 ++
 .../SpdmLib/Include/hal/library/debuglib.h|   39 +
 .../SpdmLib/Include/library/spdm_lib_config.h |  394 ++
 .../DeviceSecurity/SpdmLib/SpdmCommonLib.inf  |   47 +
 .../DeviceSecurity/SpdmLib/SpdmCryptLib.inf   |   45 +
 .../SpdmLib/SpdmDeviceSecretLibNull.inf   |   36 +
 .../SpdmLib/SpdmRequesterLib.inf  |   59 +
 .../SpdmLib/SpdmResponderLib.inf  |   61 +
 .../SpdmLib/SpdmSecuredMessageLib.inf |   44 +
 .../SpdmLib/SpdmTransportMctpLib.inf  |   38 +
 .../SpdmLib/SpdmTransportPciDoeLib.inf|   38 +
 SecurityPkg/DeviceSecurity/SpdmLib/libspdm|1 +
 .../SpdmSecurityLib/SpdmAuthentication.c  |  697 +++
 .../SpdmSecurityLib/SpdmConnectionInit.c  |  481 +++
 .../SpdmSecurityLib/SpdmMeasurement.c |  714 +++
 .../SpdmSecurityLib/SpdmSecurityLib.c |  148 +++
 .../SpdmSecurityLib/SpdmSecurityLib.inf   |   54 +
 .../SpdmSecurityLib/SpdmSecurityLibInternal.h |  

[edk2-devel] [PATCH v4 07/10] .pytool/CISettings.py: add libspdm submodule.

[Wenxing Hou]

Add DeviceSecurity submodule libspdm.

Cc: Sean Brogan 
Cc: Joey Vagedes 
Cc: Michael D Kinney 
Cc: Liming Gao 
Signed-off-by: Wenxing Hou 
Reviewed-by: Joey Vagedes 
---
 .pytool/CISettings.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.pytool/CISettings.py b/.pytool/CISettings.py
index ec3beb0dcf..314758da32 100644
--- a/.pytool/CISettings.py
+++ b/.pytool/CISettings.py
@@ -237,6 +237,8 @@ class Settings(CiBuildSettingsManager, 
UpdateSettingsManager, SetupSettingsManag
 "MdePkg/Library/MipiSysTLib/mipisyst", False))
 rs.append(RequiredSubmodule(
 "CryptoPkg/Library/MbedTlsLib/mbedtls", False))
+rs.append(RequiredSubmodule(
+"SecurityPkg/DeviceSecurity/SpdmLib/libspdm", False))
 return rs
 
 def GetName(self):
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117977): https://edk2.groups.io/g/devel/message/117977
Mute This Topic: https://groups.io/mt/105594745/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 10/10] ReadMe.rst: Add libspdm submodule license

[Wenxing Hou]

This patch add libspdm submodule license.

Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Signed-off-by: Wenxing Hou 
---
 ReadMe.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ReadMe.rst b/ReadMe.rst
index 808ccd37af..cfd522fdbd 100644
--- a/ReadMe.rst
+++ b/ReadMe.rst
@@ -99,6 +99,7 @@ that are covered by additional licenses.
 -  `RedfishPkg/Library/JsonLib/jansson 
`__
 -  `MdePkg/Library/BaseFdtLib/libfdt 
`__
 -  `MdePkg/Library/MipiSysTLib/mipisyst 
`__
+-  `SecurityPkg/DeviceSecurity/SpdmLib/libspdm 
`__
 
 The EDK II Project is composed of packages. The maintainers for each package
 are listed in `Maintainers.txt `__.
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117980): https://edk2.groups.io/g/devel/message/117980
Mute This Topic: https://groups.io/mt/105594748/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 09/10] SecurityPkg: Add libspdm submodule

[Wenxing Hou]

libspdm is submodule to support DeviceSecurity feature.

Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 
Reviewed-by: Jiewen Yao 
---
 SecurityPkg/DeviceSecurity/SpdmLib/libspdm | 1 +
 1 file changed, 1 insertion(+)
 create mode 16 SecurityPkg/DeviceSecurity/SpdmLib/libspdm

diff --git a/SecurityPkg/DeviceSecurity/SpdmLib/libspdm 
b/SecurityPkg/DeviceSecurity/SpdmLib/libspdm
new file mode 16
index 00..828ef62524
--- /dev/null
+++ b/SecurityPkg/DeviceSecurity/SpdmLib/libspdm
@@ -0,0 +1 @@
+Subproject commit 828ef62524bcaeca4e90d0c021221e714872e2b5
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117979): https://edk2.groups.io/g/devel/message/117979
Mute This Topic: https://groups.io/mt/105594747/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 08/10] .gitmodule: Add libspdm submodule for EDKII

[Wenxing Hou]

libspdm is submodule, which will be used in DeviceSecurity.

Cc: Andrew Fish 
Cc: Michael D Kinney 
Cc: Michael D Kinney 
Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 
---
 .gitmodules | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.gitmodules b/.gitmodules
index 60d54b45eb..7f069abd3d 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -35,3 +35,6 @@
 [submodule "CryptoPkg/Library/MbedTlsLib/mbedtls"]
path = CryptoPkg/Library/MbedTlsLib/mbedtls
url = https://github.com/ARMmbed/mbedtls
+[submodule "SecurityPkg/DeviceSecurity/SpdmLib/libspdm"]
+   path = SecurityPkg/DeviceSecurity/SpdmLib/libspdm
+   url = https://github.com/DMTF/libspdm.git
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117978): https://edk2.groups.io/g/devel/message/117978
Mute This Topic: https://groups.io/mt/105594746/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 05/10] SecurityPkg: Add TCG PFP 1.06 support.

[Wenxing Hou]

Add new api Tpm2ExtendNvIndex.
It is uesd in HashCompleteAndExtend when PcrIndex > MAX_PCR_INDEX.

Cc: Jiewen Yao 
Cc: Rahul Kumar 
Signed-off-by: Wenxing Hou 
Reviewed-by: Jiewen Yao 
---
 SecurityPkg/Include/Library/Tpm2CommandLib.h  |  23 +++-
 .../HashLibBaseCryptoRouterDxe.c  |  88 +++--
 .../Library/Tpm2CommandLib/Tpm2NVStorage.c| 122 +-
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c |  61 -
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf   |   4 +-
 5 files changed, 278 insertions(+), 20 deletions(-)

diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h 
b/SecurityPkg/Include/Library/Tpm2CommandLib.h
index a2fb97f18d..70eec84c90 100644
--- a/SecurityPkg/Include/Library/Tpm2CommandLib.h
+++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h
@@ -1,7 +1,7 @@
 /** @file
   This library is used by other modules to send TPM2 command.
 
-Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. 
+Copyright (c) 2013 - 2024, Intel Corporation. All rights reserved. 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -467,6 +467,27 @@ Tpm2NvGlobalWriteLock (
   IN  TPMS_AUTH_COMMAND  *AuthSession OPTIONAL
   );
 
+/**
+  This command extends a value to an area in NV memory that was previously 
defined by TPM2_NV_DefineSpace().
+
+  @param[in]  AuthHandle the handle indicating the source of the 
authorization value.
+  @param[in]  NvIndexThe NV Index of the area to extend.
+  @param[in]  AuthSessionAuth Session context
+  @param[in]  InData The data to extend.
+
+  @retval EFI_SUCCESSOperation completed successfully.
+  @retval EFI_DEVICE_ERROR   The command was unsuccessful.
+  @retval EFI_NOT_FOUND  The command was returned successfully, but 
NvIndex is not found.
+**/
+EFI_STATUS
+EFIAPI
+Tpm2NvExtend (
+  IN  TPMI_RH_NV_AUTHAuthHandle,
+  IN  TPMI_RH_NV_INDEX   NvIndex,
+  IN  TPMS_AUTH_COMMAND  *AuthSession  OPTIONAL,
+  IN  TPM2B_MAX_BUFFER   *InData
+  );
+
 /**
   This command is used to cause an update to the indicated PCR.
   The digests parameter contains one or more tagged digest value identified by 
an algorithm ID.
diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
index ee8fe6e06e..2169c5e185 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
@@ -3,7 +3,7 @@
   hash handler registered, such as SHA1, SHA256.
   Platform can use PcdTpm2HashMask to mask some hash engines.
 
-Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. 
+Copyright (c) 2013 - 2024, Intel Corporation. All rights reserved. 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -16,6 +16,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 
 #include "HashLibBaseCryptoRouterCommon.h"
 
@@ -128,6 +129,49 @@ HashUpdate (
   return EFI_SUCCESS;
 }
 
+/**
+  Extend to TPM NvIndex.
+
+  @param[in]  NvIndexThe NV Index of the area to extend.
+  @param[in]  DataSize   The data size to extend.
+  @param[in]  Data   The data to extend.
+
+  @retval EFI_SUCCESSOperation completed successfully.
+  @retval EFI_DEVICE_ERROR   The command was unsuccessful.
+  @retval EFI_NOT_FOUND  The command was returned successfully, but 
NvIndex is not found.
+**/
+EFI_STATUS
+EFIAPI
+Tpm2ExtendNvIndex (
+  TPMI_RH_NV_INDEX  NvIndex,
+  UINT16DataSize,
+  BYTE  *Data
+  )
+{
+  EFI_STATUSStatus;
+  TPMI_RH_NV_AUTH   AuthHandle;
+  TPM2B_MAX_BUFFER  NvExtendData;
+
+  AuthHandle = TPM_RH_PLATFORM;
+  ZeroMem (, sizeof (NvExtendData));
+  CopyMem (NvExtendData.buffer, Data, DataSize);
+  NvExtendData.size = DataSize;
+  Status= Tpm2NvExtend (
+AuthHandle,
+NvIndex,
+NULL,
+
+);
+  if (EFI_ERROR (Status)) {
+DEBUG (
+  (DEBUG_ERROR, "Extend TPM NV index failed, Index: 0x%x Status: %d\n",
+   NvIndex, Status)
+  );
+  }
+
+  return Status;
+}
+
 /**
   Hash sequence complete and extend to PCR.
 
@@ -149,11 +193,16 @@ HashCompleteAndExtend (
   OUT TPML_DIGEST_VALUES  *DigestList
   )
 {
-  TPML_DIGEST_VALUES  Digest;
-  HASH_HANDLE *HashCtx;
-  UINTN   Index;
-  EFI_STATUS  Status;
-  UINT32  HashMask;
+  TPML_DIGEST_VALUES   Digest;
+  HASH_HANDLE  *HashCtx;
+  UINTNIndex;
+  EFI_STATUS   Status;
+  UINT32   HashMask;
+  TPML_DIGEST_VALUES   TcgPcrEvent2Digest;
+  EFI_TCG2_EVENT_ALGORITHM_BITMAP  TpmHashAlgorithmBitmap;
+  UINT32 

[edk2-devel] [PATCH v4 04/10] MdeModulePkg/Variable: Add TCG SPDM device measurement update

[Wenxing Hou]

Add EV_EFI_SPDM_DEVICE_POLICY support for MeasureVariable.

Cc: Liming Gao 
Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 
---
 MdeModulePkg/MdeModulePkg.dec |  5 +++
 .../Variable/RuntimeDxe/Measurement.c | 38 ---
 .../RuntimeDxe/VariableRuntimeDxe.inf |  3 ++
 .../RuntimeDxe/VariableSmmRuntimeDxe.inf  |  3 ++
 4 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a91058e5b5..949babf61f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2146,6 +2146,11 @@
   # @Prompt TCG Platform Firmware Profile revision.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision|0|UINT32|0x00010077
 
+  ## Specify whether to enable the state of SPDM device authentication and 
measurement.
+  #  0: Platform Firmware not supports SPDM device authentication and 
measurement.
+  #  1: Platform Firmware supports SPDM device authentication and measurement.
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableSpdmDeviceAuthentication|0|UINT8|0x00010033
+
   ## Indicates if StatusCode is reported via Serial port.
   #   TRUE  - Reports StatusCode via Serial port.
   #   FALSE - Does not report StatusCode via Serial port.
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
index c15cce9716..a52683a9e3 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
@@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -26,12 +27,13 @@ typedef struct {
 } VARIABLE_TYPE;
 
 VARIABLE_TYPE  mVariableType[] = {
-  { EFI_SECURE_BOOT_MODE_NAME,},
-  { EFI_PLATFORM_KEY_NAME,},
-  { EFI_KEY_EXCHANGE_KEY_NAME,},
-  { EFI_IMAGE_SECURITY_DATABASE,   },
-  { EFI_IMAGE_SECURITY_DATABASE1,  },
-  { EFI_IMAGE_SECURITY_DATABASE2,  },
+  { EFI_SECURE_BOOT_MODE_NAME,  },
+  { EFI_PLATFORM_KEY_NAME,  },
+  { EFI_KEY_EXCHANGE_KEY_NAME,  },
+  { EFI_IMAGE_SECURITY_DATABASE, },
+  { EFI_IMAGE_SECURITY_DATABASE1,},
+  { EFI_IMAGE_SECURITY_DATABASE2,},
+  { EFI_DEVICE_SECURITY_DATABASE,  },
 };
 
 //
@@ -123,6 +125,22 @@ MeasureVariable (
   );
   }
 
+  if (CompareGuid (VendorGuid, )) {
+DEBUG ((DEBUG_INFO, "VariableDxe: MeasureVariable (Pcr - %x, EventType - 
%x, ", PCR_INDEX_FOR_SIGNATURE_DB, (UINTN)EV_EFI_SPDM_DEVICE_POLICY));
+DEBUG ((DEBUG_INFO, "VariableName - %s, VendorGuid - %g)\n", VarName, 
VendorGuid));
+
+Status = TpmMeasureAndLogData (
+   PCR_INDEX_FOR_SIGNATURE_DB,
+   EV_EFI_SPDM_DEVICE_POLICY,
+   VarLog,
+   VarLogSize,
+   VarLog,
+   VarLogSize
+   );
+FreePool (VarLog);
+return Status;
+  }
+
   DEBUG ((DEBUG_INFO, "VariableDxe: MeasureVariable (Pcr - %x, EventType - %x, 
", (UINTN)7, (UINTN)EV_EFI_VARIABLE_DRIVER_CONFIG));
   DEBUG ((DEBUG_INFO, "VariableName - %s, VendorGuid - %g)\n", VarName, 
VendorGuid));
 
@@ -228,6 +246,14 @@ SecureBootHook (
 return;
   }
 
+  if (CompareGuid (VendorGuid, )) {
+if ((PcdGet32 (PcdTcgPfpMeasurementRevision) < 
TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_106) ||
+(PcdGet8 (PcdEnableSpdmDeviceAuthentication) == 0))
+{
+  return;
+}
+  }
+
   //
   // We should NOT use Data and DataSize here,because it may include signature,
   // or is just partial with append attributes, or is deleted.
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index 3858adf673..f90ec70b77 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -125,6 +125,7 @@
   ## SOMETIMES_CONSUMES   ## Variable:L"dbx"
   ## SOMETIMES_CONSUMES   ## Variable:L"dbt"
   gEfiImageSecurityDatabaseGuid
+  gEfiDeviceSignatureDatabaseGuid
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
@@ -138,6 +139,8 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdReclaimVariableSpaceAtEndOfDxe  ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable ## 
SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved  ## 
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision   ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEnableSpdmDeviceAuthentication   ## 
PRODUCES AND CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics  ## CONSUMES # 
statistic the information of variable.
diff --git 
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf 

[edk2-devel] [PATCH v4 03/10] MdePkg: Add devAuthBoot GlobalVariable

[Wenxing Hou]

According to UEFI 2.10 spec 3.3 Globally Defined Variables section,
add devAuthBoot GlobalVariable.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 
Reviewed-by: Liming Gao 
---
 MdePkg/Include/Guid/GlobalVariable.h  | 8 +++-
 MdePkg/Include/Guid/ImageAuthentication.h | 5 -
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Guid/GlobalVariable.h 
b/MdePkg/Include/Guid/GlobalVariable.h
index eb2ce6aaf2..eb6e5a043e 100644
--- a/MdePkg/Include/Guid/GlobalVariable.h
+++ b/MdePkg/Include/Guid/GlobalVariable.h
@@ -1,7 +1,7 @@
 /** @file
   GUID for EFI (NVRAM) Variables.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Revision Reference:
@@ -183,4 +183,10 @@ extern EFI_GUID  gEfiGlobalVariableGuid;
 ///
 #define EFI_VENDOR_KEYS_VARIABLE_NAME  L"VendorKeys"
 
+///
+/// Whether the platform firmware is operating in device authentication boot 
mode (1) or not (0).
+/// The content is UINT8.
+///
+#define EFI_DEVICE_AUTH_BOOT_MODE_NAME  L"devAuthBoot"
+
 #endif
diff --git a/MdePkg/Include/Guid/ImageAuthentication.h 
b/MdePkg/Include/Guid/ImageAuthentication.h
index fe83596571..f95255c0fb 100644
--- a/MdePkg/Include/Guid/ImageAuthentication.h
+++ b/MdePkg/Include/Guid/ImageAuthentication.h
@@ -1,7 +1,7 @@
 /** @file
   Image signature database are defined for the signed image validation.
 
-  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Revision Reference:
@@ -41,6 +41,9 @@
 #define SETUP_MODE  1
 #define USER_MODE   0
 
+#define DEVICE_AUTH_BOOT_MODE_ENABLE   1
+#define DEVICE_AUTH_BOOT_MODE_DISABLE  0
+
 // ***
 // Signature Database
 // ***
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117973): https://edk2.groups.io/g/devel/message/117973
Mute This Topic: https://groups.io/mt/105594741/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v4 02/10] MdePkg: Add TCG PFP 1.06 support.

[Wenxing Hou]

Add support for
TCG PC Client Platform Firmware Profile Specification 1.06.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 

Signed-off-by: Wenxing Hou 
---
 MdePkg/Include/IndustryStandard/Spdm.h|   4 +-
 .../IndustryStandard/UefiTcgPlatform.h| 186 +-
 2 files changed, 187 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Spdm.h 
b/MdePkg/Include/IndustryStandard/Spdm.h
index 7940caa95e..3d511b4768 100644
--- a/MdePkg/Include/IndustryStandard/Spdm.h
+++ b/MdePkg/Include/IndustryStandard/Spdm.h
@@ -1302,7 +1302,9 @@ typedef struct {
 #define SPDM_ST1_VALUE_US  10
 
 ///
-/// id-DMTF 1.3.6.1.4.1.412
+/// id-DMTF 1.3.6.1.4.1.412.
+/// These OID are defiend in ANNEX C (informative) OID reference section from 
the DMTF SPDM spec.
+/// 
https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.2.2.pdf
 ///
 #define SPDM_OID_DMTF \
 {0x2B, 0x06, 0x01, 0x04, 0x01, 0x83, 0x1C }
diff --git a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h 
b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
index e07840c9dd..61bd4e4667 100644
--- a/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+++ b/MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
@@ -1,8 +1,8 @@
 /** @file
   TCG EFI Platform Definition in TCG_EFI_Platform_1_20_Final and
-  TCG PC Client Platform Firmware Profile Specification, Revision 1.05
+  TCG PC Client Platform Firmware Profile Specification, Revision 1.06
 
-  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -53,6 +53,18 @@
 #define EV_EFI_VARIABLE_AUTHORITY (EV_EFI_EVENT_BASE + 0xE0)
 #define EV_EFI_SPDM_FIRMWARE_BLOB (EV_EFI_EVENT_BASE + 0xE1)
 #define EV_EFI_SPDM_FIRMWARE_CONFIG   (EV_EFI_EVENT_BASE + 0xE2)
+#define EV_EFI_SPDM_DEVICE_BLOB   EV_EFI_SPDM_FIRMWARE_BLOB
+#define EV_EFI_SPDM_DEVICE_CONFIG EV_EFI_SPDM_FIRMWARE_CONFIG
+//
+// The SPDM policy database for SPDM verification.
+// It goes to PCR7
+//
+#define EV_EFI_SPDM_DEVICE_POLICY  (EV_EFI_EVENT_BASE + 0xE3)
+//
+// The SPDM policy authority for SPDM verification for the signature
+// of GET_MEASUREMENT or CHALLENGE_AUTH. It goes to PCR7.
+//
+#define EV_EFI_SPDM_DEVICE_AUTHORITY  (EV_EFI_EVENT_BASE + 0xE4)
 
 #define EFI_CALLING_EFI_APPLICATION \
   "Calling EFI Application from Boot Option"
@@ -374,6 +386,7 @@ typedef struct {
 #define TCG_EfiSpecIDEventStruct_SPEC_VERSION_MINOR_TPM2   0
 #define TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2  0
 #define TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105  105
+#define TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_106  106
 
 typedef struct {
   UINT8 signature[16];
@@ -492,4 +505,173 @@ typedef struct tdTCG_EfiStartupLocalityEvent {
 //
 #pragma pack ()
 
+//
+// 
==
+// Event TypePCR  Event Log
   Usage
+// 
==
+// EV_EFI_SPDM_DEVICE_BLOB   2SPDM_MEASUREMENT_BLOCK (subtype) 
   MEASUREMENT from device
+// EV_EFI_SPDM_DEVICE_CONFIG 3SPDM_MEASUREMENT_BLOCK (subtype) 
   MEASUREMENT from device
+// EV_EFI_SPDM_DEVICE_BLOB   2SPDM_MEASUREMENT_SUMMARY_HASH.TCB 
(subtype) SUMMARY_HASH from device
+
+// EV_EFI_SPDM_DEVICE_POLICY 7UEFI_VARIABLE_DATA with 
EFI_SIGNATURE_LIST  Provisioned device public cert.
+// EV_EFI_SPDM_DEVICE_AUTHORITY  7UEFI_VARIABLE_DATA with 
EFI_SIGNATURE_DATA  CHALLENGE_AUTH signature verification
+// 
==
+//
+
+#define PCR_INDEX_FOR_SIGNATURE_DB  7
+
+#pragma pack(1)
+
+#define TCG_DEVICE_SECURITY_EVENT_DATA_VERSION_11
+#define TCG_DEVICE_SECURITY_EVENT_DATA_VERSION_22
+#define TCG_DEVICE_SECURITY_EVENT_DATA_SIGNATURE_2  "SPDM Device Sec2"
+
+typedef struct {
+  UINT8 Signature[16];
+  UINT16Version;
+  UINT8 AuthState;
+  UINT8 Reserved;
+  UINT32Length;  // Length in bytes for all following 
structures.
+  UINT32DeviceType;
+  UINT32SubHeaderType;
+  UINT32SubHeaderLength;  // Length in bytes of the 
sub header followed by.
+  UINT64SubHeaderUID; // Universal identifier 
assigned by the event log creator. It can be used to bind two sub header 
structure together.
+  // UINT64 DevicePathLength;
+  // UINT8  DevicePath[DevicePathLength];
+} TCG_DEVICE_SECURITY_EVENT_DATA_HEADER2;
+
+#define TCG_DEVICE_SECURITY_EVENT_DATA_DEVICE_AUTH_STATE_SUCCESS   0
+#define 

[edk2-devel] [PATCH v4 01/10] MdePkg: Add SPDM1.2 support.

[Wenxing Hou]

Update Spdm.h to support 1.2 new features, such as:
Authentication and measurement. It wil be used in DeviceSecurity.
The DeviceSecurity feature is from
TCG PC Client Platform Firmware Profile Specification 1.06.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 
---
 MdePkg/Include/IndustryStandard/Spdm.h | 1110 ++--
 1 file changed, 1061 insertions(+), 49 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Spdm.h 
b/MdePkg/Include/IndustryStandard/Spdm.h
index 4ec7a5ed1f..7940caa95e 100644
--- a/MdePkg/Include/IndustryStandard/Spdm.h
+++ b/MdePkg/Include/IndustryStandard/Spdm.h
@@ -1,8 +1,8 @@
 /** @file
-  Definitions of Security Protocol & Data Model Specification (SPDM)
-  version 1.0.0 in Distributed Management Task Force (DMTF).
+  Definitions of DSP0274 Security Protocol & Data Model Specification (SPDM)
+  version 1.2.0 in Distributed Management Task Force (DMTF).
 
-Copyright (c) 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2019 - 2024, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -12,29 +12,72 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #pragma pack(1)
 
+#define SPDM_MAX_SLOT_COUNT8
+#define SPDM_MAX_OPAQUE_DATA_SIZE  1024
+#define SPDM_NONCE_SIZE32
+#define SPDM_RANDOM_DATA_SIZE  32
 ///
-/// SPDM response code
+/// SPDM response code (1.0)
 ///
-#define SPDM_DIGESTS0x01
-#define SPDM_CERTIFICATE0x02
-#define SPDM_CHALLENGE_AUTH 0x03
-#define SPDM_VERSION0x04
-#define SPDM_MEASUREMENTS   0x60
-#define SPDM_CAPABILITIES   0x61
-#define SPDM_SET_CERT_RESPONSE  0x62
-#define SPDM_ALGORITHMS 0x63
-#define SPDM_ERROR  0x7F
+#define SPDM_DIGESTS  0x01
+#define SPDM_CERTIFICATE  0x02
+#define SPDM_CHALLENGE_AUTH   0x03
+#define SPDM_VERSION  0x04
+#define SPDM_MEASUREMENTS 0x60
+#define SPDM_CAPABILITIES 0x61
+#define SPDM_ALGORITHMS   0x63
+#define SPDM_VENDOR_DEFINED_RESPONSE  0x7E
+#define SPDM_ERROR0x7F
 ///
-/// SPDM request code
+/// SPDM response code (1.1)
 ///
-#define SPDM_GET_DIGESTS   0x81
-#define SPDM_GET_CERTIFICATE   0x82
-#define SPDM_CHALLENGE 0x83
-#define SPDM_GET_VERSION   0x84
-#define SPDM_GET_MEASUREMENTS  0xE0
-#define SPDM_GET_CAPABILITIES  0xE1
-#define SPDM_NEGOTIATE_ALGORITHMS  0xE3
-#define SPDM_RESPOND_IF_READY  0xFF
+#define SPDM_KEY_EXCHANGE_RSP   0x64
+#define SPDM_FINISH_RSP 0x65
+#define SPDM_PSK_EXCHANGE_RSP   0x66
+#define SPDM_PSK_FINISH_RSP 0x67
+#define SPDM_HEARTBEAT_ACK  0x68
+#define SPDM_KEY_UPDATE_ACK 0x69
+#define SPDM_ENCAPSULATED_REQUEST   0x6A
+#define SPDM_ENCAPSULATED_RESPONSE_ACK  0x6B
+#define SPDM_END_SESSION_ACK0x6C
+///
+/// SPDM response code (1.2)
+///
+#define SPDM_CSR  0x6D
+#define SPDM_SET_CERTIFICATE_RSP  0x6E
+#define SPDM_CHUNK_SEND_ACK   0x05
+#define SPDM_CHUNK_RESPONSE   0x06
+///
+/// SPDM request code (1.0)
+///
+#define SPDM_GET_DIGESTS 0x81
+#define SPDM_GET_CERTIFICATE 0x82
+#define SPDM_CHALLENGE   0x83
+#define SPDM_GET_VERSION 0x84
+#define SPDM_GET_MEASUREMENTS0xE0
+#define SPDM_GET_CAPABILITIES0xE1
+#define SPDM_NEGOTIATE_ALGORITHMS0xE3
+#define SPDM_VENDOR_DEFINED_REQUEST  0xFE
+#define SPDM_RESPOND_IF_READY0xFF
+///
+/// SPDM request code (1.1)
+///
+#define SPDM_KEY_EXCHANGE   0xE4
+#define SPDM_FINISH 0xE5
+#define SPDM_PSK_EXCHANGE   0xE6
+#define SPDM_PSK_FINISH 0xE7
+#define SPDM_HEARTBEAT  0xE8
+#define SPDM_KEY_UPDATE 0xE9
+#define SPDM_GET_ENCAPSULATED_REQUEST   0xEA
+#define SPDM_DELIVER_ENCAPSULATED_RESPONSE  0xEB
+#define SPDM_END_SESSION0xEC
+///
+/// SPDM request code (1.2)
+///
+#define SPDM_GET_CSR  0xED
+#define SPDM_SET_CERTIFICATE  0xEE
+#define SPDM_CHUNK_SEND   0x85
+#define SPDM_CHUNK_GET0x86
 
 ///
 /// SPDM message header
@@ -46,13 +89,18 @@ typedef struct {
   UINT8Param2;
 } SPDM_MESSAGE_HEADER;
 
-#define SPDM_MESSAGE_VERSION  0x10
+#define SPDM_MESSAGE_VERSION_10  0x10
+#define SPDM_MESSAGE_VERSION_11  0x11
+#define SPDM_MESSAGE_VERSION_12  0x12
+#define SPDM_MESSAGE_VERSION SPDM_MESSAGE_VERSION_10
 
 ///
 /// SPDM GET_VERSION request
 ///
 typedef struct {
   SPDM_MESSAGE_HEADERHeader;
+  // Param1 == RSVD
+  // Param2 == RSVD
 } SPDM_GET_VERSION_REQUEST;
 
 ///
@@ -60,6 +108,8 @@ typedef struct {
 ///
 typedef struct {
   SPDM_MESSAGE_HEADERHeader;
+  // Param1 == RSVD
+  // Param2 == RSVD
   UINT8  Reserved;
   UINT8  

[edk2-devel] [PATCH v4 00/10] Add DeviceSecurity feature based on PFP 1.06 spec

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2479

In PFP spec 1.06, platform firmware records the device certificate and device 
measurement for each SPDM responder.
This PATCH set implement the DeviceSecurityLib to support spdm device 
Authentication and Measurement.

Libspdm as submodule is to support DeviceSecurity feature:
https://github.com/DMTF/libspdm

TCG PFP spec 1.06:
https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/

The POC branch:
https://github.com/tianocore/edk2-staging/tree/DeviceSecurity

And the PATCH set has passed the EDKII CI:
https://github.com/tianocore/edk2/pull/5508

v2 changes:
 - Fix typo: PcdEnableSpdmDeviceAuthenticaion -> 
PcdEnableSpdmDeviceAuthentication
v3 changes:
 - Add new patch 10: Update ReadMe.rst for libspdm submodule license
v4 changes:
 - Update submodule libspdm to latest tag

PATCH 3: Reviewed-by: Liming Gao 
PATCH 5: Reviewed-by: Jiewen Yao 
PATCH 6: Reviewed-by: Jiewen Yao 
PATCH 7: Reviewed-by: Joey Vagedes 
PATCH 9: Reviewed-by: Jiewen Yao 

Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Joey Vagedes 
Cc: Zhiguang Liu 
Cc: Rahul Kumar 
Cc: Jiewen Yao 
Signed-off-by: Wenxing Hou 

Wenxing Hou (10):
  MdePkg: Add SPDM1.2 support.
  MdePkg: Add TCG PFP 1.06 support.
  MdePkg: Add devAuthBoot GlobalVariable
  MdeModulePkg/Variable: Add TCG SPDM device measurement update
  SecurityPkg: Add TCG PFP 1.06 support.
  SecurityPkg: add DeviceSecurity support
  .pytool/CISettings.py: add libspdm submodule.
  .gitmodule: Add libspdm submodule for EDKII
  SecurityPkg: Add libspdm submodule
  ReadMe.rst: Add libspdm submodule license

 .gitmodules   |3 +
 .pytool/CISettings.py |2 +
 MdeModulePkg/MdeModulePkg.dec |5 +
 .../Variable/RuntimeDxe/Measurement.c |   38 +-
 .../RuntimeDxe/VariableRuntimeDxe.inf |3 +
 .../RuntimeDxe/VariableSmmRuntimeDxe.inf  |3 +
 MdePkg/Include/Guid/GlobalVariable.h  |8 +-
 MdePkg/Include/Guid/ImageAuthentication.h |5 +-
 MdePkg/Include/IndustryStandard/Spdm.h| 1112 -
 .../IndustryStandard/UefiTcgPlatform.h|  186 ++-
 ReadMe.rst|1 +
 .../OsStub/CryptlibWrapper/CryptlibWrapper.c  |  970 ++
 .../CryptlibWrapper/CryptlibWrapper.inf   |   38 +
 .../OsStub/MemLibWrapper/MemLibWrapper.c  |  177 +++
 .../OsStub/MemLibWrapper/MemLibWrapper.inf|   33 +
 .../PlatformLibWrapper/PlatformLibWrapper.c   |   85 ++
 .../PlatformLibWrapper/PlatformLibWrapper.inf |   33 +
 .../SpdmLib/Include/Stub/SpdmLibStub.h|  347 +
 .../SpdmLib/Include/hal/LibspdmStdBoolAlt.h   |   23 +
 .../SpdmLib/Include/hal/LibspdmStdDefAlt.h|   16 +
 .../SpdmLib/Include/hal/LibspdmStdIntAlt.h|   25 +
 .../DeviceSecurity/SpdmLib/Include/hal/base.h |   94 ++
 .../SpdmLib/Include/hal/library/debuglib.h|   39 +
 .../SpdmLib/Include/library/spdm_lib_config.h |  394 ++
 .../DeviceSecurity/SpdmLib/SpdmCommonLib.inf  |   47 +
 .../DeviceSecurity/SpdmLib/SpdmCryptLib.inf   |   45 +
 .../SpdmLib/SpdmDeviceSecretLibNull.inf   |   36 +
 .../SpdmLib/SpdmRequesterLib.inf  |   59 +
 .../SpdmLib/SpdmResponderLib.inf  |   61 +
 .../SpdmLib/SpdmSecuredMessageLib.inf |   44 +
 .../SpdmLib/SpdmTransportMctpLib.inf  |   38 +
 .../SpdmLib/SpdmTransportPciDoeLib.inf|   38 +
 SecurityPkg/DeviceSecurity/SpdmLib/libspdm|1 +
 .../SpdmSecurityLib/SpdmAuthentication.c  |  697 +++
 .../SpdmSecurityLib/SpdmConnectionInit.c  |  481 +++
 .../SpdmSecurityLib/SpdmMeasurement.c |  714 +++
 .../SpdmSecurityLib/SpdmSecurityLib.c |  148 +++
 .../SpdmSecurityLib/SpdmSecurityLib.inf   |   54 +
 .../SpdmSecurityLib/SpdmSecurityLibInternal.h |  250 
 SecurityPkg/Include/Library/SpdmSecurityLib.h |  437 +++
 SecurityPkg/Include/Library/Tpm2CommandLib.h  |   23 +-
 .../Include/Protocol/DeviceSecurityPolicy.h   |  133 ++
 .../HashLibBaseCryptoRouterDxe.c  |   88 +-
 .../Library/Tpm2CommandLib/Tpm2NVStorage.c|  122 +-
 SecurityPkg/SecurityPkg.ci.yaml   |   17 +-
 SecurityPkg/SecurityPkg.dec   |   13 +-
 SecurityPkg/SecurityPkg.dsc   |   31 +-
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c |   61 +-
 SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf   |4 +-
 49 files changed, 7197 insertions(+), 85 deletions(-)
 create mode 100644 
SecurityPkg/DeviceSecurity/OsStub/CryptlibWrapper/CryptlibWrapper.c
 create mode 100644 
SecurityPkg/DeviceSecurity/OsStub/CryptlibWrapper/CryptlibWrapper.inf
 create mode 100644 
SecurityPkg/DeviceSecurity/OsStub/MemLibWrapper/MemLibWrapper.c
 create mode 100644 
SecurityPkg/DeviceSecurity/OsStub/MemLibWrapper/MemLibWrapper.inf
 create mode 100644 

Re: [edk2-devel] [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST

[Xianglei Cai]

Hi Liming,

Could you help pick the change to master branch?

Thanks,
Xianglei

-Original Message-
From: gaoliming  
Sent: Monday, April 15, 2024 2:36 PM
To: Cai, Xianglei ; devel@edk2.groups.io
Cc: Ni, Ray ; Lewandowski, Krzysztof 
; Huang, Jenny ; Shih, 
More ; Chiu, Ian 
Subject: 回复: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of HCRST

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Xianglei Cai 
> 发送时间: 2024年4月15日 14:34
> 收件人: devel@edk2.groups.io
> 抄送: Xianglei Cai ; Ray Ni ; 
> Liming Gao ; Krzysztof Lewandowski 
> ; Jenny Huang 
> ; More Shih ; Ian Chiu 
> 
> 主题: [PATCH V3 1/1] MdeModulePkg/XhciDxe: Add PCD for the delay of 
> HCRST
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4727
> 
> Recently some of XHCI host controllers require to have extra 1ms delay 
> before accessing any MMIO register during reset. PHY transition from 
> P3 to P0 can take around 1.3ms and the xHCI reset can take around 
> 1.5ms.
> Add PCD to control the delay, the default is 2 ms.
> 
> Cc: Ray Ni   
> Cc: Liming Gao   
> Cc: Krzysztof Lewandowski
> Cc: Jenny Huang  
> Cc: More Shih
> Cc: Ian Chiu 
> Signed-off-by: Xianglei Cai 
> Reviewed-by: Krzysztof Lewandowski 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 1 +
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf | 4 
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 2 +-
>  MdeModulePkg/MdeModulePkg.dec| 5 +
>  4 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> index 44016758724c..c9a12095c29e 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  
> #include   #include 
>   #include 
> +#include 
> 
>  #include 
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> index 18ef87916ae4..e6c1ac8a6346 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
> @@ -56,6 +56,7 @@
>DebugLib
>ReportStatusCodeLib
>TimerLib
> +  PcdLib
> 
>  [Guids]
>gEfiEventExitBootServicesGuid ##
> SOMETIMES_CONSUMES ## Event
> @@ -64,6 +65,9 @@
>gEfiPciIoProtocolGuid ## TO_START
>gEfiUsb2HcProtocolGuid## BY_START
> 
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdDelayXhciHCReset  ##
> CONSUMES
> +
>  # [Event]
>  # EVENT_TYPE_PERIODIC_TIMER   ## CONSUMES
>  #
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> index 40f2f1f22766..525942a167b0 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> @@ -864,7 +864,7 @@ XhcResetHC (
>  // Otherwise there may have the timeout case happened.
>  // The below is a workaround to solve such problem.
>  //
> -gBS->Stall (XHC_1_MILLISECOND);
> +gBS->Stall (PcdGet16 (PcdDelayXhciHCReset));
>  Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, 
> XHC_USBCMD_RESET, FALSE, Timeout);
> 
>  if (!EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/MdeModulePkg.dec 
> b/MdeModulePkg/MdeModulePkg.dec index a91058e5b5df..d9e2e724df9e 
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1148,6 +1148,11 @@
># @Prompt Enable large address image loading.
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageLargeAddressLoad|TRUE|BOO
> LEAN|0x30001059
> 
> +  ## Indicates time delay for XHCI registers access after it issues
HCRST.
> +  # Default is 2000, it represent delay is 2 ms.
> +  # @Prompt Delay access XHCI register after it issues HCRST (us)
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdDelayXhciHCReset|2000|UINT16|0x3
> 0001060
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>## Dynamic type PCD can be registered callback function for Pcd 
> setting action.
>#  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum 
> number of callback function
> --
> 2.42.0.windows.2





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117969): https://edk2.groups.io/g/devel/message/117969
Mute This Topic: https://groups.io/mt/105594498/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint while USB Transaction error

[Xianglei Cai]

Hi Liming,

Could you help pick the change to the master branch?

Thanks,
Xianglei

-Original Message-
From: gaoliming  
Sent: Monday, April 15, 2024 5:52 PM
To: devel@edk2.groups.io; Cai, Xianglei 
Cc: Ni, Ray ; Lewandowski, Krzysztof 
; Huang, Jenny ; Shih, 
More 
Subject: 回复: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint 
while USB Transaction error

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Xianglei Cai
> 发送时间: 2024年4月15日 14:55
> 收件人: devel@edk2.groups.io
> 抄送: Xianglei Cai ; Ray Ni ; 
> Liming Gao ; Krzysztof Lewandowski 
> ; Jenny Huang 
> ; More Shih 
> 主题: [edk2-devel] [PATCH V2 1/1] MdeModulePkg/XhciDxe: Reset endpoint 
> while USB Transaction error
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4556
> 
> Based on XHCI spec 4.8.3, software should do the reset endpoint while 
> USB Transaction occur.
> Add the error code for USB Transaction error since UEFI spec don't 
> have the related definition.
> 
> Cc: Ray Ni  
> Cc: Liming Gao  
> Cc: Krzysztof Lewandowski   
> Cc: Jenny Huang 
> Cc: More Shih   
> Signed-off-by: Xianglei Cai 
> Reviewed-by: Krzysztof Lewandowski 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 5 -
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 5 -  
> MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 7 +++
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index f4e61d223c1b..cf6b32959e68 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -825,7 +825,10 @@ XhcTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if ((*TransferResult == EFI_USB_ERR_STALL) || (*TransferResult ==
> EFI_USB_ERR_BABBLE)) {
> +  //
> +  // Based on XHCI spec 4.8.3, software should do the reset endpoint
while
> USB Transaction occur.
> +  //
> +  if ((*TransferResult == EFI_USB_ERR_STALL) || (*TransferResult ==
> EFI_USB_ERR_BABBLE) || (*TransferResult ==
> EDKII_USB_ERR_TRANSACTION)) {
>  ASSERT (Status == EFI_DEVICE_ERROR);
>  RecoveryStatus = XhcRecoverHaltedEndpoint (Xhc, Urb);
>  if (EFI_ERROR (RecoveryStatus)) { diff --git 
> a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 5d735008ba31..a97ed44dbfc3 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -1192,8 +1192,11 @@ XhcCheckUrbResult (
>  DEBUG ((DEBUG_ERROR, "XhcCheckUrbResult: ERR_BUFFER!
> Completecode = %x\n", EvtTrb->Completecode));
>  goto EXIT;
> 
> +  //
> +  // Based on XHCI spec 4.8.3, software should do the reset 
> + endpoint
> while USB Transaction occur.
> +  //
>case TRB_COMPLETION_USB_TRANSACTION_ERROR:
> -CheckedUrb->Result  |= EFI_USB_ERR_TIMEOUT;
> +CheckedUrb->Result  |= EDKII_USB_ERR_TRANSACTION;
>  CheckedUrb->Finished = TRUE;
>  DEBUG ((DEBUG_ERROR, "XhcCheckUrbResult:
> TRANSACTION_ERROR! Completecode = %x\n", EvtTrb->Completecode));
>  goto EXIT;
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> index 7c85f7993b5c..e606e212a1d3 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
> @@ -78,6 +78,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define TRB_COMPLETION_STOPPED 26
>  #define TRB_COMPLETION_STOPPED_LENGTH_INVALID  27
> 
> +//
> +// USB Transfer Results Internal Definition // Based on XHCI spec 
> +4.8.3, software should do the reset endpoint while
> USB Transaction occur.
> +// Add the error code for USB Transaction error since UEFI spec don't
have
> the related definition.
> +//
> +#define EDKII_USB_ERR_TRANSACTION  0x200
> +
>  //
>  // The topology string used to present usb device location  //
> --
> 2.42.0.windows.2
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117968): https://edk2.groups.io/g/devel/message/117968
Mute This Topic: https://groups.io/mt/105594486/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 13/13] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic

[Ni, Ray]

+  //
+  // Check whether the Required TileSize is enough.
+  //
+  if (TileSize > SIZE_8KB) {
+DEBUG ((DEBUG_ERROR, "The Range of Smbase in SMRAM is not enough -- 
Required TileSize = 0x%08x, Actual TileSize = 0x%08x\n", TileSize, SIZE_8KB));
+FreePool (mCpuHotPlugData.SmBase);
+FreePool (gSmmCpuPrivate->ProcessorInfo);
+CpuDeadLoop ();
+return RETURN_BUFFER_TOO_SMALL;
   }

[Ray] Can you move the "TileSize" check just below the original TileSize 
calculation logic? Others look good to me.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117967): https://edk2.groups.io/g/devel/message/117967
Mute This Topic: https://groups.io/mt/105593584/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] OvmfPkg: Harden #VC instruction emulation somewhat (CVE-2024-25742)

[Yao, Jiewen]

Thanks Adam and Ard.

Since this #VC specific hardening, I would rely on AMD people's expertise to 
fix it.
I have no objection for the patch.

Thank you
Yao, Jiewen

> -Original Message-
> From: Adam Dunlap 
> Sent: Thursday, April 18, 2024 1:45 AM
> To: Ard Biesheuvel 
> Cc: devel@edk2.groups.io; Yao, Jiewen ; Borislav Petkov
> ; Peter Gonda ; Tom Lendacky
> ; Aktas, Erdem ; Gerd
> Hoffmann ; Michael Roth ; Xu,
> Min M 
> Subject: Re: [edk2-devel] [PATCH] OvmfPkg: Harden #VC instruction emulation
> somewhat (CVE-2024-25742)
> 
> On Wed, Apr 17, 2024 at 10:08 AM Ard Biesheuvel  wrote:
> >
> > (cc Jiewen)
> >
> > Please cc the OVMF maintainers when you send edk2 patches. (There is a
> > Maintainers file in the root of the repo)
> 
> Thanks, I added everyone returned from the GetMaintainer.py script.
> 
> > On Wed, 17 Apr 2024 at 18:54, Adam Dunlap via groups.io
> >  wrote:
> > >
> > > Ensure that when a #VC exception happens, the instruction at the
> > > instruction pointer matches the instruction that is expected given the
> > > error code. This is to mitigate the ahoi WeSee attack [1] that could
> > > allow hypervisors to breach integrity and confidentiality of the
> > > firmware by maliciously injecting interrupts. This change is a
> > > translated version of a linux patch e3ef461af35a ("x86/sev: Harden #VC
> > > instruction emulation somewhat")
> > >
> > > [1] https://ahoi-attacks.github.io/wesee/
> > >
> > > Cc: Borislav Petkov (AMD) 
> > > Cc: Tom Lendacky 
> > > Signed-off-by: Adam Dunlap 
> > > ---
> > >  OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 171 ++--
> > >  1 file changed, 160 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> > > index 0fc30f7bc4..bd3e9f304a 100644
> > > --- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> > > +++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> > > @@ -532,8 +532,6 @@ MwaitExit (
> > >IN CC_INSTRUCTION_DATA *InstructionData
> > >)
> > >  {
> > > -  CcDecodeModRm (Regs, InstructionData);
> > > -
> > >Ghcb->SaveArea.Rax = Regs->Rax;
> > >CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
> > >Ghcb->SaveArea.Rcx = Regs->Rcx;
> > > @@ -564,8 +562,6 @@ MonitorExit (
> > >IN CC_INSTRUCTION_DATA *InstructionData
> > >)
> > >  {
> > > -  CcDecodeModRm (Regs, InstructionData);
> > > -
> > >Ghcb->SaveArea.Rax = Regs->Rax;  // Identity mapped, so VA = PA
> > >CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
> > >Ghcb->SaveArea.Rcx = Regs->Rcx;
> > > @@ -670,8 +666,6 @@ VmmCallExit (
> > >  {
> > >UINT64  Status;
> > >
> > > -  CcDecodeModRm (Regs, InstructionData);
> > > -
> > >Ghcb->SaveArea.Rax = Regs->Rax;
> > >CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
> > >Ghcb->SaveArea.Cpl = (UINT8)(Regs->Cs & 0x3);
> > > @@ -1603,8 +1597,6 @@ Dr7WriteExit (
> > >Ext   = >Ext;
> > >SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
> > >
> > > -  CcDecodeModRm (Regs, InstructionData);
> > > -
> > >//
> > >// MOV DRn always treats MOD == 3 no matter how encoded
> > >//
> > > @@ -1655,8 +1647,6 @@ Dr7ReadExit (
> > >Ext   = >Ext;
> > >SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
> > >
> > > -  CcDecodeModRm (Regs, InstructionData);
> > > -
> > >//
> > >// MOV DRn always treats MOD == 3 no matter how encoded
> > >//
> > > @@ -1671,6 +1661,160 @@ Dr7ReadExit (
> > >return 0;
> > >  }
> > >
> > > +/**
> > > +  Check that the opcode matches the exit code for a #VC.
> > > +
> > > +  Each exit code should only be raised while executing certain 
> > > instructions.
> > > +  Verify that rIP points to a correct instruction based on the exit code 
> > > to
> > > +  protect against maliciously injected interrupts via the hypervisor. If 
> > > it does
> > > +  not, report an unsupported event to the hypervisor.
> > > +
> > > +  Decodes the ModRm byte into InstructionData if necessary.
> > > +
> > > +  @param[in, out] Ghcb Pointer to the Guest-Hypervisor
> Communication
> > > +   Block
> > > +  @param[in, out] Regs x64 processor context
> > > +  @param[in, out] InstructionData  Instruction parsing context
> > > +  @param[in]  ExitCode Exit code given by #VC.
> > > +
> > > +  @retval 0No problems detected.
> > > +  @return  New exception value to propagate
> > > +
> > > +
> > > +**/
> > > +STATIC
> > > +UINT64
> > > +VcCheckOpcodeBytes (
> > > +  IN OUT GHCB*Ghcb,
> > > +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> > > +  IN OUT CC_INSTRUCTION_DATA *InstructionData,
> > > +  IN UINT64  ExitCode
> > > +  )
> > > +{
> > > +  UINT8  OpCode;
> > > +
> > > +  //
> > > +  // Expected opcodes are either 1 or 2 bytes. If they are 2 bytes, they 
> > > always
> > > +  // start with TWO_BYTE_OPCODE_ESCAPE (0x0f), so skip over 

Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

[Wu, Jiaxin]

Hi Tom & Abner,

Could you help check & review AMD related patch?
 >   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD

Thanks,
Jiaxin

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu,
> Jiaxin
> Sent: Thursday, April 18, 2024 2:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Zeng, Star ; Gerd
> Hoffmann ; Kumar, Rahul R ;
> Dong, Guo ; Rhodes, Sean ;
> Lu, James ; Guo, Gua ; Ard
> Biesheuvel ; Yao, Jiewen
> ; Abdul Lateef Attar ;
> Abner Chang ; Tom Lendacky
> 
> Subject: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib
> 
> PR: https://github.com/tianocore/edk2/pull/5546
> 
> Intel plans to separate the smbase relocation logic from
> PiSmmCpuDxeSmm driver, and the related behavior will be
> moved to the new interface defined by the SmmRelocationLib
> class.
> 
> The SmmRelocationLib class provides the SmmRelocationInit()
> interface for platform to do the smbase relocation, which
> shall provide below 2 functionalities:
> 1. Relocate smbases for each processor.
> 2. Create the gSmmBaseHobGuid HOB.
> 
> With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
> a later phase) can be simplfied as below for SMM init:
> 1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
> for each Processor.
> 2. Execute the early SMM Init.
> 
> Cc: Ray Ni 
> Cc: Zeng Star 
> Cc: Gerd Hoffmann 
> Cc: Rahul Kumar 
> Cc: Guo Dong 
> Cc: Sean Rhodes 
> Cc: James Lu 
> Cc: Gua Guo 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Abdul Lateef Attar 
> Cc: Abner Chang 
> Cc: Tom Lendacky 
> Signed-off-by: Jiaxin Wu 
> 
> Jiaxin Wu (13):
>   UefiCpuPkg: Add SmmRelocationLib class
>   UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
>   UefiCpuPkg/SmmRelocationLib: Rename global variables
>   UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation
>   UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
>   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
>   OvmfPkg/SmmRelocationLib: Add library instance for OVMF
>   OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
>   OvmfPkg: Refine SmmAccess implementation
>   OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
>   OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
>   UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
>   UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic
> 
>  OvmfPkg/AmdSev/AmdSevX64.dsc   |   1 +
>  OvmfPkg/CloudHv/CloudHvX64.dsc |   1 +
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c| 104 ++--
>  .../Library/PlatformInitLib/PlatformInitLib.inf|   6 +-
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  33 +-
>  .../Library/SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../Library/SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
>  .../Library/SmmRelocationLib/SmmRelocationLib.c| 549
> +
>  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  60 +++
>  .../SmmRelocationLib/SmramSaveStateConfig.c| 100 
>  .../Library/SmmRelocationLib}/X64/Semaphore.c  |  13 +-
>  .../Library/SmmRelocationLib}/X64/SmmInit.nasm |  85 +++-
>  OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
>  OvmfPkg/OvmfPkgIa32.dsc|   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
>  OvmfPkg/OvmfPkgX64.dsc |   1 +
>  OvmfPkg/PlatformPei/Platform.c |   3 +
>  OvmfPkg/PlatformPei/Platform.h |   5 +
>  OvmfPkg/PlatformPei/PlatformPei.inf|   5 +-
>  OvmfPkg/PlatformPei/SmmRelocation.c|  80 +++
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.c  |   4 +-
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf|   5 +
>  OvmfPkg/SmmAccess/SmmAccessPei.c   |  88 +---
>  OvmfPkg/SmmAccess/SmmAccessPei.inf |   7 +-
>  OvmfPkg/SmmAccess/SmramInternal.c  |  73 +--
>  OvmfPkg/SmmAccess/SmramInternal.h  |  18 +-
>  UefiCpuPkg/Include/Library/SmmRelocationLib.h  |  42 ++
>  .../SmmRelocationLib/AmdSmmRelocationLib.inf   |  60 +++
>  .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 125 +
>  .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
>  .../Library/SmmRelocationLib/SmmRelocationLib.c| 549
> +
>  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 +++
>  .../SmmRelocationLib/SmramSaveStateConfig.c| 136 +
>  .../SmmRelocationLib}/X64/Semaphore.c  |  13 +-
>  .../SmmRelocationLib}/X64/SmmInit.nasm |  85 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c  |  21 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  10 +-
>  

Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

[Wu, Jiaxin]

Hi Gerd,

Could you help review & check below OVMF related patches?

>   OvmfPkg/SmmRelocationLib: Add library instance for OVMF
>   OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
>   OvmfPkg: Refine SmmAccess implementation
>   OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
>   OvmfPkg/PlatformPei: Relocate SmBases in PEI phase

Thanks,
Jiaxin 


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu,
> Jiaxin
> Sent: Thursday, April 18, 2024 2:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Zeng, Star ; Gerd
> Hoffmann ; Kumar, Rahul R ;
> Dong, Guo ; Rhodes, Sean ;
> Lu, James ; Guo, Gua ; Ard
> Biesheuvel ; Yao, Jiewen
> ; Abdul Lateef Attar ;
> Abner Chang ; Tom Lendacky
> 
> Subject: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib
> 
> PR: https://github.com/tianocore/edk2/pull/5546
> 
> Intel plans to separate the smbase relocation logic from
> PiSmmCpuDxeSmm driver, and the related behavior will be
> moved to the new interface defined by the SmmRelocationLib
> class.
> 
> The SmmRelocationLib class provides the SmmRelocationInit()
> interface for platform to do the smbase relocation, which
> shall provide below 2 functionalities:
> 1. Relocate smbases for each processor.
> 2. Create the gSmmBaseHobGuid HOB.
> 
> With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
> a later phase) can be simplfied as below for SMM init:
> 1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
> for each Processor.
> 2. Execute the early SMM Init.
> 
> Cc: Ray Ni 
> Cc: Zeng Star 
> Cc: Gerd Hoffmann 
> Cc: Rahul Kumar 
> Cc: Guo Dong 
> Cc: Sean Rhodes 
> Cc: James Lu 
> Cc: Gua Guo 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Abdul Lateef Attar 
> Cc: Abner Chang 
> Cc: Tom Lendacky 
> Signed-off-by: Jiaxin Wu 
> 
> Jiaxin Wu (13):
>   UefiCpuPkg: Add SmmRelocationLib class
>   UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
>   UefiCpuPkg/SmmRelocationLib: Rename global variables
>   UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation
>   UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
>   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
>   OvmfPkg/SmmRelocationLib: Add library instance for OVMF
>   OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
>   OvmfPkg: Refine SmmAccess implementation
>   OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
>   OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
>   UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
>   UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic
> 
>  OvmfPkg/AmdSev/AmdSevX64.dsc   |   1 +
>  OvmfPkg/CloudHv/CloudHvX64.dsc |   1 +
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c| 104 ++--
>  .../Library/PlatformInitLib/PlatformInitLib.inf|   6 +-
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  33 +-
>  .../Library/SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../Library/SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
>  .../Library/SmmRelocationLib/SmmRelocationLib.c| 549
> +
>  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  60 +++
>  .../SmmRelocationLib/SmramSaveStateConfig.c| 100 
>  .../Library/SmmRelocationLib}/X64/Semaphore.c  |  13 +-
>  .../Library/SmmRelocationLib}/X64/SmmInit.nasm |  85 +++-
>  OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
>  OvmfPkg/OvmfPkgIa32.dsc|   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
>  OvmfPkg/OvmfPkgX64.dsc |   1 +
>  OvmfPkg/PlatformPei/Platform.c |   3 +
>  OvmfPkg/PlatformPei/Platform.h |   5 +
>  OvmfPkg/PlatformPei/PlatformPei.inf|   5 +-
>  OvmfPkg/PlatformPei/SmmRelocation.c|  80 +++
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.c  |   4 +-
>  OvmfPkg/SmmAccess/SmmAccess2Dxe.inf|   5 +
>  OvmfPkg/SmmAccess/SmmAccessPei.c   |  88 +---
>  OvmfPkg/SmmAccess/SmmAccessPei.inf |   7 +-
>  OvmfPkg/SmmAccess/SmramInternal.c  |  73 +--
>  OvmfPkg/SmmAccess/SmramInternal.h  |  18 +-
>  UefiCpuPkg/Include/Library/SmmRelocationLib.h  |  42 ++
>  .../SmmRelocationLib/AmdSmmRelocationLib.inf   |  60 +++
>  .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 125 +
>  .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
>  .../Library/SmmRelocationLib/SmmRelocationLib.c| 549
> +
>  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 +++
>  .../SmmRelocationLib/SmramSaveStateConfig.c| 136 +
>  .../SmmRelocationLib}/X64/Semaphore.c  |  13 

Re: [edk2-devel] [PATCH v3 05/13] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable

[Wu, Jiaxin]

Sure, I will you split the removal of CpuIndex parameter in a new patch.

Thanks,
Jiaxin

From: Ni, Ray 
Sent: Thursday, April 18, 2024 3:48 PM
To: Wu, Jiaxin ; devel@edk2.groups.io
Cc: Zeng, Star ; Gerd Hoffmann ; Kumar, 
Rahul R 
Subject: Re: [PATCH v3 05/13] UefiCpuPkg/SmmRelocationLib: Remove unnecessary 
global variable

 UINT64
 EFIAPI
 HookReturnFromSmm (
-  IN UINTN CpuIndex,
   IN OUT SMRAM_SAVE_STATE_MAP  *CpuState,
   IN UINT64NewInstructionPointer32,
   IN UINT64NewInstructionPointer
   )
 {
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c 
b/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
index 53f3084363..cd6778e3fc 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
@@ -26,18 +26,16 @@ SmmRelocationSemaphoreComplete32 (
 /**
   Hook return address of SMM Save State so that semaphore code
   can be executed immediately after AP exits SMM to indicate to
   the BSP that an AP has exited SMM after SMBASE relocation.

-  @param[in] CpuIndex The processor index.
   @param[in] RebasedFlag  A pointer to a flag that is set to TRUE
   immediately after AP exits SMM.

 **/
 VOID
 SemaphoreHook (
-  IN UINTN CpuIndex,
   IN volatile BOOLEAN  *RebasedFlag
   )
 {
   SMRAM_SAVE_STATE_MAP  *CpuState;
   UINTN TempValue;
@@ -49,11 +47,10 @@ SemaphoreHook (
 4
 );

   CpuState  = (SMRAM_SAVE_STATE_MAP 
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
   mSmmRelocationOriginalAddress = HookReturnFromSmm (
-CpuIndex,
 CpuState,
 
(UINT64)(UINTN),
 
(UINT64)(UINTN)
 );


[Ray] Can you split the removal of CpuIndex parameter in a new patch? Others 
look good to me.

--
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117963): https://edk2.groups.io/g/devel/message/117963
Mute This Topic: https://groups.io/mt/105593573/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 05/13] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable

[Ni, Ray]

 UINT64
 EFIAPI
 HookReturnFromSmm (
-  IN UINTN CpuIndex,
   IN OUT SMRAM_SAVE_STATE_MAP  *CpuState,
   IN UINT64NewInstructionPointer32,
   IN UINT64NewInstructionPointer
   )
 {
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c 
b/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
index 53f3084363..cd6778e3fc 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
@@ -26,18 +26,16 @@ SmmRelocationSemaphoreComplete32 (
 /**
   Hook return address of SMM Save State so that semaphore code
   can be executed immediately after AP exits SMM to indicate to
   the BSP that an AP has exited SMM after SMBASE relocation.

-  @param[in] CpuIndex The processor index.
   @param[in] RebasedFlag  A pointer to a flag that is set to TRUE
   immediately after AP exits SMM.

 **/
 VOID
 SemaphoreHook (
-  IN UINTN CpuIndex,
   IN volatile BOOLEAN  *RebasedFlag
   )
 {
   SMRAM_SAVE_STATE_MAP  *CpuState;
   UINTN TempValue;
@@ -49,11 +47,10 @@ SemaphoreHook (
 4
 );

   CpuState  = (SMRAM_SAVE_STATE_MAP 
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
   mSmmRelocationOriginalAddress = HookReturnFromSmm (
-CpuIndex,
 CpuState,
 
(UINT64)(UINTN),
 
(UINT64)(UINTN)
 );


[Ray] Can you split the removal of CpuIndex parameter in a new patch? Others 
look good to me.

--
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117962): https://edk2.groups.io/g/devel/message/117962
Mute This Topic: https://groups.io/mt/105593573/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 04/13] UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation

[Ni, Ray]

Reviewed-by: Ray Ni 

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Thursday, April 18, 2024 14:55
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R 
Subject: [PATCH v3 04/13] UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory 
allocation

Since SMM relocation is performed serially for each CPU, there is
no need to allocate buffers for all CPUs to store the SmBase
address in mSmBase and the Rebased flag in mRebased. A defined
global variable is sufficient.

This patch focuses on the mSmBase and mRebased global variables
to prevent unnecessary memory allocation for these variables.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 201 +
 1 file changed, 90 insertions(+), 111 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c 
b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index ca98f06a05..3694a07cbb 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -25,31 +25,57 @@ EFI_PROCESSOR_INFORMATION  *mProcessorInfo = NULL;
 // IDT used during SMM Init
 //
 IA32_DESCRIPTOR  gcSmmInitIdtr;

 //
-// Smbase for all CPUs
+// Smbase for current CPU
 //
-UINT64  *mSmBase = NULL;
+UINT64  mSmBase;

 //
-// SmBase Rebased flag for all CPUs
+// SmBase Rebased flag for current CPU
 //
-volatile BOOLEAN  *mRebased;
+volatile BOOLEAN  mRebased;
+
+/**
+  This function will get the SmBase for CpuIndex.
+
+  @param[in]   CpuIndexThe processor index.
+  @param[in]   SmmRelocationStart  The start address of Smm relocated memory 
in SMRAM.
+  @param[in]   TileSizeThe total size required for a CPU save 
state, any
+   additional CPU-specific context and the 
size of code
+   for the SMI entry point.
+
+  @retval The value of SmBase for CpuIndex.
+
+**/
+UINTN
+GetSmBase (
+  IN UINTN CpuIndex,
+  IN EFI_PHYSICAL_ADDRESS  SmmRelocationStart,
+  IN UINTN TileSize
+  )
+{
+  return (UINTN)(SmmRelocationStart) + CpuIndex * TileSize - 
SMM_HANDLER_OFFSET;
+}

 /**
   This function will create SmBase for all CPUs.

-  @param[in] SmBasePointer to SmBase for all CPUs.
+  @param[in]   SmmRelocationStart  The start address of Smm relocated memory 
in SMRAM.
+  @param[in]   TileSizeThe total size required for a CPU save 
state, any
+   additional CPU-specific context and the 
size of code
+   for the SMI entry point.

   @retval EFI_SUCCESS   Create SmBase for all CPUs successfully.
   @retval OthersFailed to create SmBase for all CPUs.

 **/
 EFI_STATUS
 CreateSmmBaseHob (
-  IN UINT64  *SmBase
+  IN EFI_PHYSICAL_ADDRESS  SmmRelocationStart,
+  IN UINTN TileSize
   )
 {
   UINTN  Index;
   SMM_BASE_HOB_DATA  *SmmBaseHobData;
   UINT32 CpuCount;
@@ -90,11 +116,11 @@ CreateSmmBaseHob (
 DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - 
SmmBaseHobData[%d]->NumberOfProcessors: %d\n", HobCount, 
SmmBaseHobData->NumberOfProcessors));
 for (Index = 0; Index < SmmBaseHobData->NumberOfProcessors; Index++) {
   //
   // Calculate the new SMBASE address
   //
-  SmmBaseHobData->SmBase[Index] = SmBase[Index + CpuCount];
+  SmmBaseHobData->SmBase[Index] = GetSmBase (Index + CpuCount, 
SmmRelocationStart, TileSize);
   DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - SmmBaseHobData[%d]->SmBase[%d]: 
0x%08x\n", HobCount, Index, SmmBaseHobData->SmBase[Index]));
 }

 CpuCount += NumberOfProcessorsInHob;
 HobCount++;
@@ -126,18 +152,18 @@ SmmInitHandler (
   for (Index = 0; Index < mNumberOfCpus; Index++) {
 if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
   //
   // Configure SmBase.
   //
-  ConfigureSmBase (mSmBase[Index]);
+  ConfigureSmBase (mSmBase);

   //
   // Hook return after RSM to set SMM re-based flag
   // SMM re-based flag can't be set before RSM, because SMM save state 
context might be override
   // by next AP flow before it take effect.
   //
-  SemaphoreHook (Index, [Index]);
+  SemaphoreHook (Index, );
   return;
 }
   }

   ASSERT (FALSE);
@@ -145,14 +171,22 @@ SmmInitHandler (

 /**
   Relocate SmmBases for each processor.
   Execute on first boot and all S3 resumes

+  @param[in]   MpServices2 Pointer to this instance of the MpServices.
+  @param[in]   SmmRelocationStart  The start address of Smm relocated memory 
in SMRAM.
+  @param[in]   TileSizeThe total size required for a CPU save 
state, any
+   additional CPU-specific context and the 
size of code
+   for the SMI entry point.
+
 **/
 VOID
 

Re: [edk2-devel] [PATCH v3 03/13] UefiCpuPkg/SmmRelocationLib: Rename global variables

[Ni, Ray]

Reviewed-by: Ray Ni 

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Thursday, April 18, 2024 14:55
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R 
Subject: [PATCH v3 03/13] UefiCpuPkg/SmmRelocationLib: Rename global variables

This patch aims to rename global variables for clearer
association with Smm Init, ensuring their names are
distinct from those used in the PiSmmCpuDxeSmm Driver.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 18 +++
 .../SmmRelocationLib/InternalSmmRelocationLib.h|  8 +++
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 26 +++---
 .../Library/SmmRelocationLib/X64/SmmInit.nasm  | 18 +++
 4 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
index 3d845e9e16..8916cb7d06 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
@@ -16,15 +16,15 @@

 extern ASM_PFX(SmmInitHandler)
 extern ASM_PFX(mRebasedFlag)
 extern ASM_PFX(mSmmRelocationOriginalAddress)

-global ASM_PFX(gPatchSmmCr3)
-global ASM_PFX(gPatchSmmCr4)
-global ASM_PFX(gPatchSmmCr0)
+global ASM_PFX(gPatchSmmInitCr3)
+global ASM_PFX(gPatchSmmInitCr4)
+global ASM_PFX(gPatchSmmInitCr0)
 global ASM_PFX(gPatchSmmInitStack)
-global ASM_PFX(gcSmiInitGdtr)
+global ASM_PFX(gcSmmInitGdtr)
 global ASM_PFX(gcSmmInitSize)
 global ASM_PFX(gcSmmInitTemplate)

 %define PROTECT_MODE_CS 0x8
 %define PROTECT_MODE_DS 0x20
@@ -81,11 +81,11 @@ CodeSeg64:
 DB  0x9b
 DB  0xaf; LimitHigh
 DB  0   ; BaseHigh
 GDT_SIZE equ $ - NullSeg

-ASM_PFX(gcSmiInitGdtr):
+ASM_PFX(gcSmmInitGdtr):
 DW  GDT_SIZE - 1
 DD  NullSeg


 SECTION .text
@@ -98,22 +98,22 @@ ASM_PFX(SmmStartup):
 cpuid
 mov ebx, edx; rdmsr will change edx. keep it in 
ebx.
 and ebx, BIT20  ; extract NX capability bit
 shr ebx, 9  ; shift bit to IA32_EFER.NXE[BIT11] 
position
 mov eax, strict dword 0 ; source operand will be patched
-ASM_PFX(gPatchSmmCr3):
+ASM_PFX(gPatchSmmInitCr3):
 mov cr3, eax
-o32 lgdt[cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
+o32 lgdt[cs:ebp + (ASM_PFX(gcSmmInitGdtr) - ASM_PFX(SmmStartup))]
 mov eax, strict dword 0 ; source operand will be patched
-ASM_PFX(gPatchSmmCr4):
+ASM_PFX(gPatchSmmInitCr4):
 mov cr4, eax
 mov ecx, 0xc080 ; IA32_EFER MSR
 rdmsr
 or  eax, ebx; set NXE bit if NX is available
 wrmsr
 mov eax, strict dword 0 ; source operand will be patched
-ASM_PFX(gPatchSmmCr0):
+ASM_PFX(gPatchSmmInitCr0):
 mov di, PROTECT_MODE_DS
 mov cr0, eax
 jmp PROTECT_MODE_CS : dword @32bit

 BITS 32
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h 
b/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
index a9d3f271a9..ede61b956f 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
+++ b/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
@@ -30,17 +30,17 @@
 #include 
 #include 
 #include 
 #include 

-extern IA32_DESCRIPTOR  gcSmiInitGdtr;
+extern IA32_DESCRIPTOR  gcSmmInitGdtr;
 extern CONST UINT16 gcSmmInitSize;
 extern CONST UINT8  gcSmmInitTemplate[];

-X86_ASSEMBLY_PATCH_LABEL  gPatchSmmCr0;
-X86_ASSEMBLY_PATCH_LABEL  gPatchSmmCr3;
-X86_ASSEMBLY_PATCH_LABEL  gPatchSmmCr4;
+X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitCr0;
+X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitCr3;
+X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitCr4;
 X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitStack;

 //
 // The size 0x20 must be bigger than
 // the size of template code of SmmInit. Currently,
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c 
b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index 13e62b662d..ca98f06a05 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -22,11 +22,11 @@ UINTN  mNumberOfCpus= 1;
 EFI_PROCESSOR_INFORMATION  *mProcessorInfo = NULL;

 //
 // IDT used during SMM Init
 //
-IA32_DESCRIPTOR  gcSmiIdtr;
+IA32_DESCRIPTOR  gcSmmInitIdtr;

 //
 // Smbase for all CPUs
 //
 UINT64  *mSmBase = NULL;
@@ -118,11 +118,11 @@ SmmInitHandler (
   UINTN   Index;

   //
   // Update SMM IDT entries' code segment and load IDT
   //
-  AsmWriteIdtr ();
+  AsmWriteIdtr ();
   ApicId = GetApicId ();

   for (Index = 0; Index < mNumberOfCpus; Index++) {
 if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
   //
@@ -167,13 +167,13 @@ SmmRelocateBases (
   

Re: [edk2-devel] [PATCH v3 02/13] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance

[Ni, Ray]

Reviewed-by: Ray Ni 

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Thursday, April 18, 2024 14:55
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R 
Subject: [PATCH v3 02/13] 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   ; BaseHigh
+ProtModeCodeSeg32:
+DW  -1  ; LimitLow
+

[edk2-devel] [PATCH v3 13/13] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic

[Wu, Jiaxin]

This patch is to remove legacy SmBase relocation in
PiSmmCpuDxeSmm Driver. The responsibility for SmBase
relocation has been transferred to the SmmRelocationInit
interface, which now handles the following tasks:
1. Relocates the SmBase for each processor.
2. Generates the gSmmBaseHobGuid HOB.

As a result of this change, the PiSmmCpuDxeSmm driver's
role in SMM environment setup is simplified to:
1. Utilize the gSmmBaseHobGuid to determine the SmBase.
2. Perform the ExecuteFirstSmiInit() to do early SMM
initialization.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c|  21 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c   |  42 
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm  |  96 
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c|  10 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 331 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   | 103 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   4 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h|   2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c   |  69 --
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c|  69 --
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm   | 146 
 11 files changed, 39 insertions(+), 854 deletions(-)
 delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
 delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
 delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c
 delete mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index b14c289a27..d67fb49890 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -639,27 +639,14 @@ InitializeCpuProcedure (
 //
 InitializeCpuBeforeRebase (IsBsp);
   }
 
   if (IsBsp) {
-DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n", 
mSmmRelocated));
-
 //
-// Check whether Smm Relocation is done or not.
-// If not, will do the SmmBases Relocation here!!!
+// Issue SMI IPI (All Excluding  Self SMM IPI + BSP SMM IPI) to execute 
first SMI init.
 //
-if (!mSmmRelocated) {
-  //
-  // Restore SMBASE for BSP and all APs
-  //
-  SmmRelocateBases ();
-} else {
-  //
-  // Issue SMI IPI (All Excluding  Self SMM IPI + BSP SMM IPI) to execute 
first SMI init.
-  //
-  ExecuteFirstSmiInit ();
-}
+ExecuteFirstSmiInit ();
   }
 
   //
   // Skip initialization if mAcpiCpuData is not valid
   //
@@ -978,13 +965,13 @@ InitSmmS3ResumeState (
 SmmS3ResumeState->SmmS3StackBase = 
(EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES 
((UINTN)SmmS3ResumeState->SmmS3StackSize));
 if (SmmS3ResumeState->SmmS3StackBase == 0) {
   SmmS3ResumeState->SmmS3StackSize = 0;
 }
 
-SmmS3ResumeState->SmmS3Cr0 = mSmmCr0;
+SmmS3ResumeState->SmmS3Cr0 = (UINT32)AsmReadCr0 ();
 SmmS3ResumeState->SmmS3Cr3 = Cr3;
-SmmS3ResumeState->SmmS3Cr4 = mSmmCr4;
+SmmS3ResumeState->SmmS3Cr4 = (UINT32)AsmReadCr4 ();
 
 if (sizeof (UINTN) == sizeof (UINT64)) {
   SmmS3ResumeState->Signature = SMM_S3_RESUME_SMM_64;
 }
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
deleted file mode 100644
index a9fcc89dda..00
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ /dev/null
@@ -1,42 +0,0 @@
-/** @file
-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
-
-**/
-
-#include "PiSmmCpuDxeSmm.h"
-
-UINTN mSmmRelocationOriginalAddress;
-volatile BOOLEAN  *mRebasedFlag;
-
-/**
-  Hook return address of SMM Save State so that semaphore code
-  can be executed immediately after AP exits SMM to indicate to
-  the BSP that an AP has exited SMM after SMBASE relocation.
-
-  @param[in] CpuIndex The processor index.
-  @param[in] RebasedFlag  A pointer to a flag that is set to TRUE
-  immediately after AP exits SMM.
-
-**/
-VOID
-SemaphoreHook (
-  IN UINTN CpuIndex,
-  IN volatile BOOLEAN  *RebasedFlag
-  )
-{
-  SMRAM_SAVE_STATE_MAP  *CpuState;
-
-  mRebasedFlag = RebasedFlag;
-
-  CpuState  = (SMRAM_SAVE_STATE_MAP 
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
-  mSmmRelocationOriginalAddress = (UINTN)HookReturnFromSmm (
-   CpuIndex,
-   CpuState,
-   
(UINT64)(UINTN),
-   
(UINT64)(UINTN)
-   );
-}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
deleted file mode 100644
index 

[edk2-devel] [PATCH v3 11/13] OvmfPkg/PlatformPei: Relocate SmBases in PEI phase

[Wu, Jiaxin]

This patch is to consume SmmRelocationInit for SmBase
Relocation.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
Signed-off-by: Jiaxin Wu 
---
 OvmfPkg/PlatformPei/Platform.c |  3 +
 OvmfPkg/PlatformPei/Platform.h |  5 ++
 OvmfPkg/PlatformPei/PlatformPei.inf|  5 +-
 .../ClearCache.c => PlatformPei/SmmRelocation.c}   | 79 ++
 4 files changed, 34 insertions(+), 58 deletions(-)
 copy OvmfPkg/{Bhyve/PlatformPei/ClearCache.c => PlatformPei/SmmRelocation.c} 
(32%)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index f5dc41c3a8..df35726ff6 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -365,8 +365,11 @@ InitializePlatform (
 MiscInitialization (PlatformInfoHob);
   }
 
   IntelTdxInitialize ();
   InstallFeatureControlCallback (PlatformInfoHob);
+  if (PlatformInfoHob->SmmSmramRequire) {
+RelocateSmBase ();
+  }
 
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 1cf44844a7..0a59547cfc 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -76,10 +76,15 @@ InstallFeatureControlCallback (
 VOID
 InstallClearCacheCallback (
   VOID
   );
 
+VOID
+RelocateSmBase (
+  VOID
+  );
+
 VOID
 AmdSevInitialize (
   IN EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
   );
 
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 20b1b98292..e036018eab 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -1,10 +1,10 @@
 ## @file
 #  Platform PEI driver
 #
 #  This module provides platform specific function to detect boot mode.
-#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
 
@@ -30,10 +30,11 @@
   MemDetect.c
   MemTypeInfo.c
   Platform.c
   Platform.h
   IntelTdx.c
+  SmmRelocation.c
 
 [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
@@ -64,10 +65,11 @@
   MtrrLib
   MemEncryptSevLib
   PcdLib
   CcExitLib
   PlatformInitLib
+  SmmRelocationLib
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
@@ -140,9 +142,10 @@
 
 [Ppis]
   gEfiPeiMasterBootModePpiGuid
   gEfiPeiMpServicesPpiGuid
   gEfiPeiReadOnlyVariable2PpiGuid
+  gEdkiiPeiMpServices2PpiGuid
 
 [Depex]
   TRUE
 
diff --git a/OvmfPkg/Bhyve/PlatformPei/ClearCache.c 
b/OvmfPkg/PlatformPei/SmmRelocation.c
similarity index 32%
copy from OvmfPkg/Bhyve/PlatformPei/ClearCache.c
copy to OvmfPkg/PlatformPei/SmmRelocation.c
index 9e59f31df7..1151fc9220 100644
--- a/OvmfPkg/Bhyve/PlatformPei/ClearCache.c
+++ b/OvmfPkg/PlatformPei/SmmRelocation.c
@@ -1,45 +1,21 @@
 /**@file
-  Install a callback to clear cache on all processors.
-  This is for conformance with the TCG "Platform Reset Attack Mitigation
-  Specification". Because clearing the CPU caches at boot doesn't impact
-  performance significantly, do it unconditionally, for simplicity's
-  sake.
+  Install a callback to do smm relocation.
 
-  Copyright (C) 2018, Red Hat, Inc.
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
-#include 
 #include 
 #include 
-#include 
-
+#include 
+#include 
 #include "Platform.h"
 
 /**
-  Invalidate data & instruction caches.
-  All APs execute this function in parallel. The BSP executes the function
-  separately.
-
-  @param[in,out] WorkSpace  Pointer to the input/output argument workspace
-shared by all processors.
-**/
-STATIC
-VOID
-EFIAPI
-ClearCache (
-  IN OUT VOID  *WorkSpace
-  )
-{
-  WriteBackInvalidateDataCache ();
-  InvalidateInstructionCache ();
-}
-
-/**
-  Notification function called when EFI_PEI_MP_SERVICES_PPI becomes available.
+  Notification function called when EDKII_PEI_MP_SERVICES2_PPI becomes 
available.
 
   @param[in] PeiServices  Indirect reference to the PEI Services Table.
   @param[in] NotifyDescriptor Address of the notification descriptor data
   structure.
   @param[in] Ppi  Address of the PPI that was installed.
@@ -48,68 +24,57 @@ ClearCache (
function is ignored.
 **/
 STATIC
 EFI_STATUS
 EFIAPI
-ClearCacheOnMpServicesAvailable (
+OnMpServices2Available (
   IN EFI_PEI_SERVICES   **PeiServices,
   IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
   IN VOID   *Ppi
   )
 {
-  EFI_PEI_MP_SERVICES_PPI  *MpServices;
-  EFI_STATUS   Status;
+  EDKII_PEI_MP_SERVICES2_PPI  *MpServices2;
+  EFI_STATUS  Status;
 
   DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName, __func__));
 
+  MpServices2 = Ppi;
+
   //
-  // 

[edk2-devel] [PATCH v3 12/13] UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib

[Wu, Jiaxin]

This patch just includes SmmRelocationLib in UefiPayloadPkg.

Cc: Guo Dong 
Cc: Sean Rhodes 
Cc: James Lu 
Cc: Gua Guo 
Cc: Ray Ni 
Signed-off-by: Jiaxin Wu 
Reviewed-by: Gua Guo 
Reviewed-by: Guo Dong 
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc 
b/UefiPayloadPkg/UefiPayloadPkg.dsc
index e1b9d5ecf1..2860a659f6 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -314,10 +314,12 @@
   
VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
   CcExitLib|UefiCpuPkg/Library/CcExitLibNull/CcExitLibNull.inf
   AmdSvsmLib|UefiCpuPkg/Library/AmdSvsmLibNull/AmdSvsmLibNull.inf
   
ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
   FdtLib|MdePkg/Library/BaseFdtLib/BaseFdtLib.inf
+  SmmRelocationLib|UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
+
 [LibraryClasses.common]
 !if $(BOOTSPLASH_IMAGE)
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
 !endif
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117957): https://edk2.groups.io/g/devel/message/117957
Mute This Topic: https://groups.io/mt/105593583/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 10/13] OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not

[Wu, Jiaxin]

Based on gSmmBaseHobGuid:
If gSmmBaseHobGuid found, means SmBase info has been relocated
and recorded in the SmBase array.
So, this patch check smbase relocation is done or not in
SmmCpuFeaturesInitializeProcessor().

With SmmRelocationLib, gSmmBaseHobGuid will be always created.
Here this patch just makes the function/logic correct. The SMM
Relocation logic can be totally cleaned from the
SmmCpuFeaturesLib. But it will happen in the future patch set,
this patch does not target to the cleanup work.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
Signed-off-by: Jiaxin Wu 
Reviewed-by: Ray Ni 
---
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  | 33 ++
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c 
b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 63822b126e..0a6f33c2b1 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -26,10 +26,16 @@
 //
 // EFER register LMA bit
 //
 #define LMA  BIT10
 
+//
+// Indicate SmBase for each Processors has been relocated or not. If TRUE,
+// means no need to do the relocation in SmmCpuFeaturesInitializeProcessor().
+//
+BOOLEAN  mSmmCpuFeaturesSmmRelocated;
+
 /**
   The constructor function
 
   @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
   @param[in]  SystemTable  A pointer to the EFI System Table.
@@ -44,13 +50,13 @@ SmmCpuFeaturesLibConstructor (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
   //
   // If gSmmBaseHobGuid found, means SmBase info has been relocated and 
recorded
-  // in the SmBase array. ASSERT it's not supported in OVMF.
+  // in the SmBase array.
   //
-  ASSERT (GetFirstGuidHob () == NULL);
+  mSmmCpuFeaturesSmmRelocated = (BOOLEAN)(GetFirstGuidHob () 
!= NULL);
 
   //
   // No need to program SMRRs on our virtual platform.
   //
   return EFI_SUCCESS;
@@ -90,20 +96,25 @@ SmmCpuFeaturesInitializeProcessor (
   )
 {
   AMD_SMRAM_SAVE_STATE_MAP  *CpuState;
 
   //
-  // Configure SMBASE.
+  // No need to configure SMBASE if SmBase relocation has been done.
   //
-  CpuState = (AMD_SMRAM_SAVE_STATE_MAP *)(UINTN)(
- SMM_DEFAULT_SMBASE +
- SMRAM_SAVE_STATE_MAP_OFFSET
- );
-  if ((CpuState->x86.SMMRevId & 0x) == 0) {
-CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
-  } else {
-CpuState->x64.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+  if (!mSmmCpuFeaturesSmmRelocated) {
+//
+// Configure SMBASE.
+//
+CpuState = (AMD_SMRAM_SAVE_STATE_MAP *)(UINTN)(
+   SMM_DEFAULT_SMBASE +
+   SMRAM_SAVE_STATE_MAP_OFFSET
+   );
+if ((CpuState->x86.SMMRevId & 0x) == 0) {
+  CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+} else {
+  CpuState->x64.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+}
   }
 
   //
   // No need to program SMRRs on our virtual platform.
   //
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117956): https://edk2.groups.io/g/devel/message/117956
Mute This Topic: https://groups.io/mt/105593582/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 09/13] OvmfPkg: Refine SmmAccess implementation

[Wu, Jiaxin]

This patch refines the SmmAccess implementation:
1. SmramMap will be retrieved from the
gEfiSmmSmramMemoryGuid instead of original from
the TSEG Memory Base register.
2. Remove the gEfiAcpiVariableGuid creation, thus
the DESCRIPTOR_INDEX definition can be also cleaned.
3. The gEfiAcpiVariableGuid HOB is moved to the
OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
Signed-off-by: Jiaxin Wu 
---
 OvmfPkg/Library/PlatformInitLib/MemDetect.c|  9 +++
 .../Library/PlatformInitLib/PlatformInitLib.inf|  1 +
 OvmfPkg/SmmAccess/SmmAccess2Dxe.c  |  4 +-
 OvmfPkg/SmmAccess/SmmAccess2Dxe.inf|  5 ++
 OvmfPkg/SmmAccess/SmmAccessPei.c   | 88 ++
 OvmfPkg/SmmAccess/SmmAccessPei.inf |  7 +-
 OvmfPkg/SmmAccess/SmramInternal.c  | 73 ++
 OvmfPkg/SmmAccess/SmramInternal.h  | 18 +
 8 files changed, 51 insertions(+), 154 deletions(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c 
b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 8b98256225..f451c9d80c 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -41,10 +41,11 @@ Module Name:
 #include 
 #include 
 
 #include 
 
+#include 
 #include 
 
 #define MEGABYTE_SHIFT  20
 
 VOID
@@ -1050,10 +1051,11 @@ PlatformQemuInitializeRam (
 UINT32  TsegSize;
 UINTN   BufferSize;
 UINT8   SmramRanges;
 EFI_PEI_HOB_POINTERSHob;
 EFI_SMRAM_HOB_DESCRIPTOR_BLOCK  *SmramHobDescriptorBlock;
+VOID*GuidHob;
 
 TsegSize = PlatformInfoHob->Q35TsegMbytes * SIZE_1MB;
 PlatformAddMemoryRangeHob (BASE_1MB, PlatformInfoHob->LowMemory - 
TsegSize);
 PlatformAddReservedMemoryBaseSizeHob (
   PlatformInfoHob->LowMemory - TsegSize,
@@ -1080,10 +1082,17 @@ PlatformQemuInitializeRam (
 SmramHobDescriptorBlock->Descriptor[0].PhysicalStart = 
PlatformInfoHob->LowMemory - TsegSize;
 SmramHobDescriptorBlock->Descriptor[0].CpuStart  = 
PlatformInfoHob->LowMemory - TsegSize;
 SmramHobDescriptorBlock->Descriptor[0].PhysicalSize  = EFI_PAGE_SIZE;
 SmramHobDescriptorBlock->Descriptor[0].RegionState   = EFI_SMRAM_CLOSED | 
EFI_CACHEABLE | EFI_ALLOCATED;
 
+//
+// Create gEfiAcpiVariableGuid
+//
+GuidHob = BuildGuidHob (, sizeof 
(EFI_SMRAM_DESCRIPTOR));
+ASSERT (GuidHob != NULL);
+CopyMem (GuidHob, >Descriptor[0], sizeof 
(EFI_SMRAM_DESCRIPTOR));
+
 //
 // Create second SMRAM descriptor, which is free and will be used by SMM 
foundation.
 //
 SmramHobDescriptorBlock->Descriptor[1].PhysicalStart = 
SmramHobDescriptorBlock->Descriptor[0].PhysicalStart + EFI_PAGE_SIZE;
 SmramHobDescriptorBlock->Descriptor[1].CpuStart  = 
SmramHobDescriptorBlock->Descriptor[0].CpuStart + EFI_PAGE_SIZE;
diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf 
b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
index 2bb1c0296f..21e6efa5e0 100644
--- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
+++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
@@ -56,10 +56,11 @@
 [LibraryClasses.X64]
   TdxLib
 
 [Guids]
   gEfiSmmSmramMemoryGuid
+  gEfiAcpiVariableGuid
 
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
 
diff --git a/OvmfPkg/SmmAccess/SmmAccess2Dxe.c 
b/OvmfPkg/SmmAccess/SmmAccess2Dxe.c
index 4b9e6df37f..3371592de7 100644
--- a/OvmfPkg/SmmAccess/SmmAccess2Dxe.c
+++ b/OvmfPkg/SmmAccess/SmmAccess2Dxe.c
@@ -4,11 +4,11 @@
 
   Q35 TSEG is expected to have been verified and set up by the SmmAccessPei
   driver.
 
   Copyright (C) 2013, 2015, Red Hat, Inc.
-  Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2024, Intel Corporation. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
@@ -113,12 +113,10 @@ SmmAccess2DxeGetCapabilities (
   IN OUT UINTN   *SmramMapSize,
   IN OUT EFI_SMRAM_DESCRIPTOR*SmramMap
   )
 {
   return SmramAccessGetCapabilities (
-   This->LockState,
-   This->OpenState,
SmramMapSize,
SmramMap
);
 }
 
diff --git a/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf 
b/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
index d86381d0fb..d9f01a13c4 100644
--- a/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
+++ b/OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
@@ -3,10 +3,11 @@
 #
 # Q35 TSEG is expected to have been verified and set up by the SmmAccessPei
 # driver.
 #
 # Copyright (C) 2013, 2015, Red Hat, Inc.
+# Copyright (c) 2024 Intel Corporation.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
 
@@ -39,17 +40,21 @@
   DebugLib
   PcdLib
   PciLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
+  HobLib
 
 [Protocols]
   

[edk2-devel] [PATCH v3 08/13] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid

[Wu, Jiaxin]

Background:
In the PiSmmCpuDxeSmm driver, SMRAM allocation for SMI
handlers and processor Save State areas was traditionally
performed using the Smst->AllocatePages() function during
the DXE phase. The introduction of SmmRelocationLib
changes this process by moving the allocation to the PEI
phase, where Smst->AllocatePages() is not accessible.
Instead, the allocation is now handled by partitioning
the SMRAM based on the information provided by a GUID HOB
(identified by gEfiSmmSMramMemoryGuid).

This patch is to ensure that OVMF produces the
gEfiSmmSMramMemoryGuid HOB, allowing SmmRelocationLib to
reserve the necessary memory for SMBASE relocation.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
Signed-off-by: Jiaxin Wu 
---
 OvmfPkg/Library/PlatformInitLib/MemDetect.c| 97 +-
 .../Library/PlatformInitLib/PlatformInitLib.inf|  5 +-
 2 files changed, 64 insertions(+), 38 deletions(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c 
b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index 7b6e5102ad..8b98256225 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -1,9 +1,9 @@
 /**@file
   Memory Detection for Virtual Machines.
 
-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2024, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 Module Name:
 
   MemDetect.c
@@ -41,10 +41,12 @@ Module Name:
 #include 
 #include 
 
 #include 
 
+#include 
+
 #define MEGABYTE_SHIFT  20
 
 VOID
 EFIAPI
 PlatformQemuUc32BaseInitialization (
@@ -1027,52 +1029,73 @@ PlatformQemuInitializeRam (
   //
   // Determine total memory size available
   //
   PlatformGetSystemMemorySizeBelow4gb (PlatformInfoHob);
 
-  if (PlatformInfoHob->BootMode == BOOT_ON_S3_RESUME) {
-//
-// Create the following memory HOB as an exception on the S3 boot path.
+  //
+  // CpuMpPei saves the original contents of the borrowed area in permanent
+  // PEI RAM, in a backup buffer allocated with the normal PEI services.
+  // CpuMpPei restores the original contents ("returns" the borrowed area) at
+  // End-of-PEI. End-of-PEI in turn is emitted by S3Resume2Pei before
+  // transferring control to the OS's wakeup vector in the FACS.
+  //
+  // We expect any other PEIMs that "borrow" memory similarly to CpuMpPei to
+  // restore the original contents. Furthermore, we expect all such PEIMs
+  // (CpuMpPei included) to claim the borrowed areas by producing memory
+  // allocation HOBs, and to honor preexistent memory allocation HOBs when
+  // looking for an area to borrow.
+  //
+  QemuInitializeRamBelow1gb (PlatformInfoHob);
+
+  if (PlatformInfoHob->SmmSmramRequire) {
+UINT32  TsegSize;
+UINTN   BufferSize;
+UINT8   SmramRanges;
+EFI_PEI_HOB_POINTERSHob;
+EFI_SMRAM_HOB_DESCRIPTOR_BLOCK  *SmramHobDescriptorBlock;
+
+TsegSize = PlatformInfoHob->Q35TsegMbytes * SIZE_1MB;
+PlatformAddMemoryRangeHob (BASE_1MB, PlatformInfoHob->LowMemory - 
TsegSize);
+PlatformAddReservedMemoryBaseSizeHob (
+  PlatformInfoHob->LowMemory - TsegSize,
+  TsegSize,
+  TRUE
+  );
+
+SmramRanges = 2;
+BufferSize  = sizeof (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK) + (SmramRanges - 1) 
* sizeof (EFI_SMRAM_DESCRIPTOR);
+
+Hob.Raw = BuildGuidHob (
+,
+BufferSize
+);
+ASSERT (Hob.Raw);
+
+SmramHobDescriptorBlock = 
(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *)(Hob.Raw);
+SmramHobDescriptorBlock->NumberOfSmmReservedRegions = SmramRanges;
+
 //
-// Normally we'd create memory HOBs only on the normal boot path. However,
-// CpuMpPei specifically needs such a low-memory HOB on the S3 path as
-// well, for "borrowing" a subset of it temporarily, for the AP startup
-// vector.
+// Create first SMRAM descriptor, which contains data structures used in 
S3 resume.
+// One page is enough for the data structure
 //
-// CpuMpPei saves the original contents of the borrowed area in permanent
-// PEI RAM, in a backup buffer allocated with the normal PEI services.
-// CpuMpPei restores the original contents ("returns" the borrowed area) at
-// End-of-PEI. End-of-PEI in turn is emitted by S3Resume2Pei before
-// transferring control to the OS's wakeup vector in the FACS.
+SmramHobDescriptorBlock->Descriptor[0].PhysicalStart = 
PlatformInfoHob->LowMemory - TsegSize;
+SmramHobDescriptorBlock->Descriptor[0].CpuStart  = 
PlatformInfoHob->LowMemory - TsegSize;
+SmramHobDescriptorBlock->Descriptor[0].PhysicalSize  = EFI_PAGE_SIZE;
+SmramHobDescriptorBlock->Descriptor[0].RegionState   = EFI_SMRAM_CLOSED | 
EFI_CACHEABLE | EFI_ALLOCATED;
+
 //
-// We expect any other PEIMs that "borrow" memory similarly to CpuMpPei to
-

[edk2-devel] [PATCH v3 07/13] OvmfPkg/SmmRelocationLib: Add library instance for OVMF

[Wu, Jiaxin]

There are below 2 differences between AMD & OVMF according
existing implementation:
1.The mode of the CPU check is different between the AMD & OVMF.
OVMF:
CpuSaveState->x86.SMMRevId & 0X

AMD:
 LMAValue = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA

2.Existing SmBase configuration is different between the
AMD & OVMF.
OVMF:
 if ((CpuSaveState->x86.SMMRevId & 0x) == 0) {
   CpuSaveState->x86.SMBASE = mSmBaseForAllCpus[CpuIndex];
 } else {
   CpuSaveState->x64.SMBASE = mSmBaseForAllCpus[CpuIndex];
 }

AMD:
 AmdCpuState->x64.SMBASE = mSmBaseForAllCpus[CpuIndex];

This patch provides the SmmRelocationLib library instance
for OVMF to handle the logic difference, and it won't change
the existing implementation code logic.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 OvmfPkg/AmdSev/AmdSevX64.dsc   |  1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc |  1 +
 .../Library/SmmRelocationLib/Ia32/Semaphore.c  |  0
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm |  0
 .../SmmRelocationLib/InternalSmmRelocationLib.h|  0
 .../Library/SmmRelocationLib/SmmRelocationLib.c|  0
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  3 +-
 .../SmmRelocationLib/SmramSaveStateConfig.c| 65 +++---
 .../Library/SmmRelocationLib/X64/Semaphore.c   |  0
 .../Library/SmmRelocationLib/X64/SmmInit.nasm  |  0
 OvmfPkg/Microvm/MicrovmX64.dsc |  1 +
 OvmfPkg/OvmfPkgIa32.dsc|  1 +
 OvmfPkg/OvmfPkgIa32X64.dsc |  1 +
 OvmfPkg/OvmfPkgX64.dsc |  1 +
 14 files changed, 27 insertions(+), 47 deletions(-)
 copy {UefiCpuPkg => OvmfPkg}/Library/SmmRelocationLib/Ia32/Semaphore.c (100%)
 copy {UefiCpuPkg => OvmfPkg}/Library/SmmRelocationLib/Ia32/SmmInit.nasm (100%)
 copy {UefiCpuPkg => 
OvmfPkg}/Library/SmmRelocationLib/InternalSmmRelocationLib.h (100%)
 copy {UefiCpuPkg => OvmfPkg}/Library/SmmRelocationLib/SmmRelocationLib.c (100%)
 copy {UefiCpuPkg => OvmfPkg}/Library/SmmRelocationLib/SmmRelocationLib.inf 
(90%)
 copy UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c => 
OvmfPkg/Library/SmmRelocationLib/SmramSaveStateConfig.c (60%)
 copy {UefiCpuPkg => OvmfPkg}/Library/SmmRelocationLib/X64/Semaphore.c (100%)
 copy {UefiCpuPkg => OvmfPkg}/Library/SmmRelocationLib/X64/SmmInit.nasm (100%)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index a7540bb636..8eb6f4f24f 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -108,10 +108,11 @@
 

 
 !include MdePkg/MdeLibs.dsc.inc
 
 [LibraryClasses]
+  SmmRelocationLib|OvmfPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
   ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index b1911d6ab4..4996885301 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -124,10 +124,11 @@
 

 
 !include MdePkg/MdeLibs.dsc.inc
 
 [LibraryClasses]
+  SmmRelocationLib|OvmfPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
   ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c 
b/OvmfPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
similarity index 100%
copy from UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
copy to OvmfPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm 
b/OvmfPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
similarity index 100%
copy from UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
copy to OvmfPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h 
b/OvmfPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
similarity index 100%
copy from UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
copy to OvmfPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c 
b/OvmfPkg/Library/SmmRelocationLib/SmmRelocationLib.c
similarity index 100%
copy from UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
copy to 

[edk2-devel] [PATCH v3 06/13] UefiCpuPkg/SmmRelocationLib: Add library instance for AMD

[Wu, Jiaxin]

Due to the definition difference of SMRAM Save State,
SmmBase config in SMRAM Save State for AMD is also different.

This patch provides the AmdSmmRelocationLib library instance
to handle the SMRAM Save State difference.

Cc: Abdul Lateef Attar 
Cc: Abner Chang 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
Acked-by: Ray Ni 
---
 ...mmRelocationLib.inf => AmdSmmRelocationLib.inf} |  5 +-
 ...SaveStateConfig.c => AmdSmramSaveStateConfig.c} | 91 ++
 UefiCpuPkg/UefiCpuPkg.dsc  |  1 +
 3 files changed, 43 insertions(+), 54 deletions(-)
 copy UefiCpuPkg/Library/SmmRelocationLib/{SmmRelocationLib.inf => 
AmdSmmRelocationLib.inf} (89%)
 copy UefiCpuPkg/Library/SmmRelocationLib/{SmramSaveStateConfig.c => 
AmdSmramSaveStateConfig.c} (47%)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf 
b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
similarity index 89%
copy from UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
copy to UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
index 45fdaf35bc..e69079fa02 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
+++ b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
@@ -13,18 +13,18 @@
 ##
 
 [Defines]
   INF_VERSION= 0x00010005
   BASE_NAME  = SmmRelocationLib
-  FILE_GUID  = 853E97B3-790C-4EA3-945C-8F622FC47FE8
+  FILE_GUID  = 65C74DCD-0D09-494A-8BFF-A64226EB8054
   MODULE_TYPE= PEIM
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = SmmRelocationLib
 
 [Sources]
   InternalSmmRelocationLib.h
-  SmramSaveStateConfig.c
+  AmdSmramSaveStateConfig.c
   SmmRelocationLib.c
 
 [Sources.Ia32]
   Ia32/Semaphore.c
   Ia32/SmmInit.nasm
@@ -40,11 +40,10 @@
 
 [LibraryClasses]
   BaseLib
   BaseMemoryLib
   CpuExceptionHandlerLib
-  CpuLib
   DebugLib
   HobLib
   LocalApicLib
   MemoryAllocationLib
   PcdLib
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmramSaveStateConfig.c 
b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
similarity index 47%
copy from UefiCpuPkg/Library/SmmRelocationLib/SmramSaveStateConfig.c
copy to UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
index 76d798aba5..068ae60887 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmramSaveStateConfig.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
@@ -1,14 +1,17 @@
 /** @file
   Config SMRAM Save State for SmmBases Relocation.
 
+  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
   Copyright (c) 2024, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 #include "InternalSmmRelocationLib.h"
-#include 
+#include 
+
+#define EFER_ADDRESS  0XC080ul
 
 /**
   Get the mode of the CPU at the time an SMI occurs
 
   @retval EFI_MM_SAVE_STATE_REGISTER_LMA_32BIT   32 bit.
@@ -18,44 +21,18 @@
 UINT8
 GetMmSaveStateRegisterLma (
   VOID
   )
 {
-  CPUID_VERSION_INFO_EAX  RegEax;
-  CPUID_EXTENDED_CPU_SIG_EDX  RegEdx;
-  UINTN   FamilyId;
-  UINTN   ModelId;
-  UINT32  Eax;
-  UINT8   SmmSaveStateRegisterLma;
-
-  //
-  // Determine the mode of the CPU at the time an SMI occurs
-  //   Intel(R) 64 and IA-32 Architectures Software Developer's Manual
-  //   Volume 3C, Section 34.4.1.1
-  //
-  RegEax.Uint32 = GetCpuFamilyModel ();
-  FamilyId  = RegEax.Bits.FamilyId;
-  ModelId   = RegEax.Bits.Model;
-  if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
-ModelId = ModelId | RegEax.Bits.ExtendedModelId << 4;
-  }
-
-  RegEdx.Uint32 = 0;
-  AsmCpuid (CPUID_EXTENDED_FUNCTION, , NULL, NULL, NULL);
-  if (Eax >= CPUID_EXTENDED_CPU_SIG) {
-AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &(RegEdx.Uint32));
-  }
+  UINT8   SmmSaveStateRegisterLma;
+  UINT32  LMAValue;
 
   SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_32BIT;
-  if (RegEdx.Bits.LM) {
-SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_64BIT;
-  }
 
-  if (FamilyId == 0x06) {
-if ((ModelId == 0x17) || (ModelId == 0x0f) || (ModelId == 0x1c)) {
-  SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_64BIT;
-}
+  LMAValue = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
+  if (LMAValue) {
+SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_64BIT;
   }
 
   return SmmSaveStateRegisterLma;
 }
 
@@ -69,22 +46,32 @@ VOID
 EFIAPI
 ConfigureSmBase (
   IN UINT64  SmBase
   )
 {
-  SMRAM_SAVE_STATE_MAP  *CpuState;
+  AMD_SMRAM_SAVE_STATE_MAP  *AmdCpuState;
 
-  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
+  AmdCpuState = (AMD_SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
 
-  CpuState->x86.SMBASE = (UINT32)SmBase;
+  

[edk2-devel] [PATCH v3 05/13] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable

[Wu, Jiaxin]

This patch aims on mProcessorInfo global variable, which can be
defined as local variable in SmmRelocateBases(). With this patch,
no need to allocate the memory for all CPUs to store the
Processor Info.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../Library/SmmRelocationLib/Ia32/Semaphore.c  |  3 -
 .../SmmRelocationLib/InternalSmmRelocationLib.h|  5 --
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 78 +++---
 .../SmmRelocationLib/SmramSaveStateConfig.c|  3 -
 .../Library/SmmRelocationLib/X64/Semaphore.c   |  3 -
 5 files changed, 24 insertions(+), 68 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
index ba329d6ba2..5d9eea3de9 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
@@ -15,28 +15,25 @@ volatile BOOLEAN  *mRebasedFlag;
 /**
   Hook return address of SMM Save State so that semaphore code
   can be executed immediately after AP exits SMM to indicate to
   the BSP that an AP has exited SMM after SMBASE relocation.
 
-  @param[in] CpuIndex The processor index.
   @param[in] RebasedFlag  A pointer to a flag that is set to TRUE
   immediately after AP exits SMM.
 
 **/
 VOID
 SemaphoreHook (
-  IN UINTN CpuIndex,
   IN volatile BOOLEAN  *RebasedFlag
   )
 {
   SMRAM_SAVE_STATE_MAP  *CpuState;
 
   mRebasedFlag = RebasedFlag;
 
   CpuState  = (SMRAM_SAVE_STATE_MAP 
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
   mSmmRelocationOriginalAddress = (UINTN)HookReturnFromSmm (
-   CpuIndex,
CpuState,

(UINT64)(UINTN),

(UINT64)(UINTN)
);
 }
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h 
b/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
index ede61b956f..d1387f2dfb 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
+++ b/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
@@ -81,12 +81,10 @@ SmmRelocationSemaphoreComplete (
   Hook the code executed immediately after an RSM instruction on the currently
   executing CPU.  The mode of code executed immediately after RSM must be
   detected, and the appropriate hook must be selected.  Always clear the auto
   HALT restart flag if it is set.
 
-  @param[in] CpuIndex The processor index for the currently
-  executing CPU.
   @param[in,out] CpuState Pointer to SMRAM Save State Map for 
the
   currently executing CPU.
   @param[in] NewInstructionPointer32  Instruction pointer to use if 
resuming to
   32-bit mode from 64-bit SMM.
   @param[in] NewInstructionPointerInstruction pointer to use if 
resuming to
@@ -96,29 +94,26 @@ SmmRelocationSemaphoreComplete (
 
 **/
 UINT64
 EFIAPI
 HookReturnFromSmm (
-  IN UINTN CpuIndex,
   IN OUT SMRAM_SAVE_STATE_MAP  *CpuState,
   IN UINT64NewInstructionPointer32,
   IN UINT64NewInstructionPointer
   );
 
 /**
   Hook return address of SMM Save State so that semaphore code
   can be executed immediately after AP exits SMM to indicate to
   the BSP that an AP has exited SMM after SMBASE relocation.
 
-  @param[in] CpuIndex The processor index.
   @param[in] RebasedFlag  A pointer to a flag that is set to TRUE
   immediately after AP exits SMM.
 
 **/
 VOID
 SemaphoreHook (
-  IN UINTN CpuIndex,
   IN volatile BOOLEAN  *RebasedFlag
   );
 
 /**
   This function fixes up the address of the global variable or function
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c 
b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index 3694a07cbb..7e65bbf929 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -14,15 +14,10 @@
 #include "InternalSmmRelocationLib.h"
 
 UINTN  mMaxNumberOfCpus = 1;
 UINTN  mNumberOfCpus= 1;
 
-//
-// Record all Processors Info
-//
-EFI_PROCESSOR_INFORMATION  *mProcessorInfo = NULL;
-
 //
 // IDT used during SMM Init
 //
 IA32_DESCRIPTOR  gcSmmInitIdtr;
 
@@ -138,37 +133,26 @@ VOID
 EFIAPI
 SmmInitHandler (
   VOID
   )
 {
-  UINT32  ApicId;
-  UINTN   Index;
-
   //
   // Update SMM IDT entries' code segment and load IDT
   //
   AsmWriteIdtr ();
-  ApicId = GetApicId ();
-
-  for (Index = 0; Index < mNumberOfCpus; Index++) {
-if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
-  //
-  // Configure SmBase.
-  

[edk2-devel] [PATCH v3 04/13] UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation

[Wu, Jiaxin]

Since SMM relocation is performed serially for each CPU, there is
no need to allocate buffers for all CPUs to store the SmBase
address in mSmBase and the Rebased flag in mRebased. A defined
global variable is sufficient.

This patch focuses on the mSmBase and mRebased global variables
to prevent unnecessary memory allocation for these variables.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 201 +
 1 file changed, 90 insertions(+), 111 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c 
b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index ca98f06a05..3694a07cbb 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -25,31 +25,57 @@ EFI_PROCESSOR_INFORMATION  *mProcessorInfo = NULL;
 // IDT used during SMM Init
 //
 IA32_DESCRIPTOR  gcSmmInitIdtr;
 
 //
-// Smbase for all CPUs
+// Smbase for current CPU
 //
-UINT64  *mSmBase = NULL;
+UINT64  mSmBase;
 
 //
-// SmBase Rebased flag for all CPUs
+// SmBase Rebased flag for current CPU
 //
-volatile BOOLEAN  *mRebased;
+volatile BOOLEAN  mRebased;
+
+/**
+  This function will get the SmBase for CpuIndex.
+
+  @param[in]   CpuIndexThe processor index.
+  @param[in]   SmmRelocationStart  The start address of Smm relocated memory 
in SMRAM.
+  @param[in]   TileSizeThe total size required for a CPU save 
state, any
+   additional CPU-specific context and the 
size of code
+   for the SMI entry point.
+
+  @retval The value of SmBase for CpuIndex.
+
+**/
+UINTN
+GetSmBase (
+  IN UINTN CpuIndex,
+  IN EFI_PHYSICAL_ADDRESS  SmmRelocationStart,
+  IN UINTN TileSize
+  )
+{
+  return (UINTN)(SmmRelocationStart) + CpuIndex * TileSize - 
SMM_HANDLER_OFFSET;
+}
 
 /**
   This function will create SmBase for all CPUs.
 
-  @param[in] SmBasePointer to SmBase for all CPUs.
+  @param[in]   SmmRelocationStart  The start address of Smm relocated memory 
in SMRAM.
+  @param[in]   TileSizeThe total size required for a CPU save 
state, any
+   additional CPU-specific context and the 
size of code
+   for the SMI entry point.
 
   @retval EFI_SUCCESS   Create SmBase for all CPUs successfully.
   @retval OthersFailed to create SmBase for all CPUs.
 
 **/
 EFI_STATUS
 CreateSmmBaseHob (
-  IN UINT64  *SmBase
+  IN EFI_PHYSICAL_ADDRESS  SmmRelocationStart,
+  IN UINTN TileSize
   )
 {
   UINTN  Index;
   SMM_BASE_HOB_DATA  *SmmBaseHobData;
   UINT32 CpuCount;
@@ -90,11 +116,11 @@ CreateSmmBaseHob (
 DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - 
SmmBaseHobData[%d]->NumberOfProcessors: %d\n", HobCount, 
SmmBaseHobData->NumberOfProcessors));
 for (Index = 0; Index < SmmBaseHobData->NumberOfProcessors; Index++) {
   //
   // Calculate the new SMBASE address
   //
-  SmmBaseHobData->SmBase[Index] = SmBase[Index + CpuCount];
+  SmmBaseHobData->SmBase[Index] = GetSmBase (Index + CpuCount, 
SmmRelocationStart, TileSize);
   DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - SmmBaseHobData[%d]->SmBase[%d]: 
0x%08x\n", HobCount, Index, SmmBaseHobData->SmBase[Index]));
 }
 
 CpuCount += NumberOfProcessorsInHob;
 HobCount++;
@@ -126,18 +152,18 @@ SmmInitHandler (
   for (Index = 0; Index < mNumberOfCpus; Index++) {
 if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
   //
   // Configure SmBase.
   //
-  ConfigureSmBase (mSmBase[Index]);
+  ConfigureSmBase (mSmBase);
 
   //
   // Hook return after RSM to set SMM re-based flag
   // SMM re-based flag can't be set before RSM, because SMM save state 
context might be override
   // by next AP flow before it take effect.
   //
-  SemaphoreHook (Index, [Index]);
+  SemaphoreHook (Index, );
   return;
 }
   }
 
   ASSERT (FALSE);
@@ -145,14 +171,22 @@ SmmInitHandler (
 
 /**
   Relocate SmmBases for each processor.
   Execute on first boot and all S3 resumes
 
+  @param[in]   MpServices2 Pointer to this instance of the MpServices.
+  @param[in]   SmmRelocationStart  The start address of Smm relocated memory 
in SMRAM.
+  @param[in]   TileSizeThe total size required for a CPU save 
state, any
+   additional CPU-specific context and the 
size of code
+   for the SMI entry point.
+
 **/
 VOID
 SmmRelocateBases (
-  VOID
+  IN EDKII_PEI_MP_SERVICES2_PPI  *MpServices2,
+  IN EFI_PHYSICAL_ADDRESSSmmRelocationStart,
+  IN UINTN   TileSize
   )
 {
   UINT8 BakBuf[BACK_BUF_SIZE];
   SMRAM_SAVE_STATE_MAP  BakBuf2;
   SMRAM_SAVE_STATE_MAP  

[edk2-devel] [PATCH v3 03/13] UefiCpuPkg/SmmRelocationLib: Rename global variables

[Wu, Jiaxin]

This patch aims to rename global variables for clearer
association with Smm Init, ensuring their names are
distinct from those used in the PiSmmCpuDxeSmm Driver.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 18 +++
 .../SmmRelocationLib/InternalSmmRelocationLib.h|  8 +++
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 26 +++---
 .../Library/SmmRelocationLib/X64/SmmInit.nasm  | 18 +++
 4 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
index 3d845e9e16..8916cb7d06 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
@@ -16,15 +16,15 @@
 
 extern ASM_PFX(SmmInitHandler)
 extern ASM_PFX(mRebasedFlag)
 extern ASM_PFX(mSmmRelocationOriginalAddress)
 
-global ASM_PFX(gPatchSmmCr3)
-global ASM_PFX(gPatchSmmCr4)
-global ASM_PFX(gPatchSmmCr0)
+global ASM_PFX(gPatchSmmInitCr3)
+global ASM_PFX(gPatchSmmInitCr4)
+global ASM_PFX(gPatchSmmInitCr0)
 global ASM_PFX(gPatchSmmInitStack)
-global ASM_PFX(gcSmiInitGdtr)
+global ASM_PFX(gcSmmInitGdtr)
 global ASM_PFX(gcSmmInitSize)
 global ASM_PFX(gcSmmInitTemplate)
 
 %define PROTECT_MODE_CS 0x8
 %define PROTECT_MODE_DS 0x20
@@ -81,11 +81,11 @@ CodeSeg64:
 DB  0x9b
 DB  0xaf; LimitHigh
 DB  0   ; BaseHigh
 GDT_SIZE equ $ - NullSeg
 
-ASM_PFX(gcSmiInitGdtr):
+ASM_PFX(gcSmmInitGdtr):
 DW  GDT_SIZE - 1
 DD  NullSeg
 
 
 SECTION .text
@@ -98,22 +98,22 @@ ASM_PFX(SmmStartup):
 cpuid
 mov ebx, edx; rdmsr will change edx. keep it in 
ebx.
 and ebx, BIT20  ; extract NX capability bit
 shr ebx, 9  ; shift bit to IA32_EFER.NXE[BIT11] 
position
 mov eax, strict dword 0 ; source operand will be patched
-ASM_PFX(gPatchSmmCr3):
+ASM_PFX(gPatchSmmInitCr3):
 mov cr3, eax
-o32 lgdt[cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
+o32 lgdt[cs:ebp + (ASM_PFX(gcSmmInitGdtr) - ASM_PFX(SmmStartup))]
 mov eax, strict dword 0 ; source operand will be patched
-ASM_PFX(gPatchSmmCr4):
+ASM_PFX(gPatchSmmInitCr4):
 mov cr4, eax
 mov ecx, 0xc080 ; IA32_EFER MSR
 rdmsr
 or  eax, ebx; set NXE bit if NX is available
 wrmsr
 mov eax, strict dword 0 ; source operand will be patched
-ASM_PFX(gPatchSmmCr0):
+ASM_PFX(gPatchSmmInitCr0):
 mov di, PROTECT_MODE_DS
 mov cr0, eax
 jmp PROTECT_MODE_CS : dword @32bit
 
 BITS 32
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h 
b/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
index a9d3f271a9..ede61b956f 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
+++ b/UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
@@ -30,17 +30,17 @@
 #include 
 #include 
 #include 
 #include 
 
-extern IA32_DESCRIPTOR  gcSmiInitGdtr;
+extern IA32_DESCRIPTOR  gcSmmInitGdtr;
 extern CONST UINT16 gcSmmInitSize;
 extern CONST UINT8  gcSmmInitTemplate[];
 
-X86_ASSEMBLY_PATCH_LABEL  gPatchSmmCr0;
-X86_ASSEMBLY_PATCH_LABEL  gPatchSmmCr3;
-X86_ASSEMBLY_PATCH_LABEL  gPatchSmmCr4;
+X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitCr0;
+X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitCr3;
+X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitCr4;
 X86_ASSEMBLY_PATCH_LABEL  gPatchSmmInitStack;
 
 //
 // The size 0x20 must be bigger than
 // the size of template code of SmmInit. Currently,
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c 
b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index 13e62b662d..ca98f06a05 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -22,11 +22,11 @@ UINTN  mNumberOfCpus= 1;
 EFI_PROCESSOR_INFORMATION  *mProcessorInfo = NULL;
 
 //
 // IDT used during SMM Init
 //
-IA32_DESCRIPTOR  gcSmiIdtr;
+IA32_DESCRIPTOR  gcSmmInitIdtr;
 
 //
 // Smbase for all CPUs
 //
 UINT64  *mSmBase = NULL;
@@ -118,11 +118,11 @@ SmmInitHandler (
   UINTN   Index;
 
   //
   // Update SMM IDT entries' code segment and load IDT
   //
-  AsmWriteIdtr ();
+  AsmWriteIdtr ();
   ApicId = GetApicId ();
 
   for (Index = 0; Index < mNumberOfCpus; Index++) {
 if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
   //
@@ -167,13 +167,13 @@ SmmRelocateBases (
   ASSERT (sizeof (BakBuf) >= gcSmmInitSize);
 
   //
   // Patch ASM code template with current CR0, CR3, and CR4 values
   //
-  PatchInstructionX86 (gPatchSmmCr0, AsmReadCr0 (), 4);
-  PatchInstructionX86 (gPatchSmmCr3, AsmReadCr3 (), 4);
-  PatchInstructionX86 (gPatchSmmCr4, 

[edk2-devel] [PATCH v3 02/13] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance

[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
+  

[edk2-devel] [PATCH v3 01/13] UefiCpuPkg: Add SmmRelocationLib class

[Wu, Jiaxin]

Intel plans to separate the smbase relocation logic from
PiSmmCpuDxeSmm driver, and the related behavior will be
moved to the new interface defined by the SmmRelocationLib
class.

The SmmRelocationLib class provides the SmmRelocationInit()
interface for platform to do the smbase relocation, which
shall provide below 2 functionalities:
1. Relocate smbases for each processor.
2. Create the gSmmBaseHobGuid HOB.

With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
a later phase) shall:
1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
for each Processor.
2. Execute the early SMM Init.

This patch just provides the SmmRelocationLib class.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
Reviewed-by: Ray Ni 
---
 UefiCpuPkg/Include/Library/SmmRelocationLib.h | 42 +++
 UefiCpuPkg/UefiCpuPkg.dec |  3 ++
 2 files changed, 45 insertions(+)
 create mode 100644 UefiCpuPkg/Include/Library/SmmRelocationLib.h

diff --git a/UefiCpuPkg/Include/Library/SmmRelocationLib.h 
b/UefiCpuPkg/Include/Library/SmmRelocationLib.h
new file mode 100644
index 00..999a5b46d1
--- /dev/null
+++ b/UefiCpuPkg/Include/Library/SmmRelocationLib.h
@@ -0,0 +1,42 @@
+/** @file
+  Header file for SMM Relocation Library.
+
+  The SmmRelocationLib class provides the SmmRelocationInit()
+  interface for platform to do the smbase relocation, which
+  shall provide below 2 functionalities:
+  1. Relocate SmBases for each processor.
+  2. Create the SmBase HOB (gSmmBaseHobGuid).
+
+  With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at a later phase)
+  shall:
+  1. Consume the gSmmBaseHobGuid for the relocated smbase for each Processor.
+  2. Execute early SMM init.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SMM_RELOCATION_LIB_H_
+#define SMM_RELOCATION_LIB_H_
+
+#include 
+
+/**
+  CPU SmmBase Relocation Init.
+
+  This function is to relocate CPU SmmBase.
+
+  @param[in] MpServices2Pointer to this instance of the MpServices.
+
+  @retval EFI_SUCCESS   CPU SmmBase Relocated successfully.
+  @retval OthersCPU SmmBase Relocation failed.
+
+**/
+EFI_STATUS
+EFIAPI
+SmmRelocationInit (
+  IN EDKII_PEI_MP_SERVICES2_PPI  *MpServices2
+  );
+
+#endif
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index f412ac0ce2..f86a6d2bcb 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -69,10 +69,13 @@
   MmSaveStateLib|Include/Library/MmSaveStateLib.h
 
   ## @libraryclass   Provides functions for SMM CPU Sync Operation.
   SmmCpuSyncLib|Include/Library/SmmCpuSyncLib.h
 
+  ## @libraryclass   Provides functions for SMM Relocation Operation.
+  SmmRelocationLib|Include/Library/SmmRelocationLib.h
+
 [LibraryClasses.RISCV64]
   ##  @libraryclass  Provides functions to manage MMU features on RISCV64 CPUs.
   ##
   RiscVMmuLib|Include/Library/BaseRiscVMmuLib.h
 
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117946): https://edk2.groups.io/g/devel/message/117946
Mute This Topic: https://groups.io/mt/105593569/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

[Wu, Jiaxin]

PR: https://github.com/tianocore/edk2/pull/5546

Intel plans to separate the smbase relocation logic from
PiSmmCpuDxeSmm driver, and the related behavior will be
moved to the new interface defined by the SmmRelocationLib
class.

The SmmRelocationLib class provides the SmmRelocationInit()
interface for platform to do the smbase relocation, which
shall provide below 2 functionalities:
1. Relocate smbases for each processor.
2. Create the gSmmBaseHobGuid HOB.

With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
a later phase) can be simplfied as below for SMM init:
1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
for each Processor.
2. Execute the early SMM Init.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Cc: Guo Dong 
Cc: Sean Rhodes 
Cc: James Lu 
Cc: Gua Guo 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Abdul Lateef Attar 
Cc: Abner Chang 
Cc: Tom Lendacky 
Signed-off-by: Jiaxin Wu 

Jiaxin Wu (13):
  UefiCpuPkg: Add SmmRelocationLib class
  UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
  UefiCpuPkg/SmmRelocationLib: Rename global variables
  UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation
  UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
  UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
  OvmfPkg/SmmRelocationLib: Add library instance for OVMF
  OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
  OvmfPkg: Refine SmmAccess implementation
  OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
  OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
  UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
  UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic

 OvmfPkg/AmdSev/AmdSevX64.dsc   |   1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc |   1 +
 OvmfPkg/Library/PlatformInitLib/MemDetect.c| 104 ++--
 .../Library/PlatformInitLib/PlatformInitLib.inf|   6 +-
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  33 +-
 .../Library/SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
 .../Library/SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 549 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  60 +++
 .../SmmRelocationLib/SmramSaveStateConfig.c| 100 
 .../Library/SmmRelocationLib}/X64/Semaphore.c  |  13 +-
 .../Library/SmmRelocationLib}/X64/SmmInit.nasm |  85 +++-
 OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
 OvmfPkg/OvmfPkgIa32.dsc|   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
 OvmfPkg/OvmfPkgX64.dsc |   1 +
 OvmfPkg/PlatformPei/Platform.c |   3 +
 OvmfPkg/PlatformPei/Platform.h |   5 +
 OvmfPkg/PlatformPei/PlatformPei.inf|   5 +-
 OvmfPkg/PlatformPei/SmmRelocation.c|  80 +++
 OvmfPkg/SmmAccess/SmmAccess2Dxe.c  |   4 +-
 OvmfPkg/SmmAccess/SmmAccess2Dxe.inf|   5 +
 OvmfPkg/SmmAccess/SmmAccessPei.c   |  88 +---
 OvmfPkg/SmmAccess/SmmAccessPei.inf |   7 +-
 OvmfPkg/SmmAccess/SmramInternal.c  |  73 +--
 OvmfPkg/SmmAccess/SmramInternal.h  |  18 +-
 UefiCpuPkg/Include/Library/SmmRelocationLib.h  |  42 ++
 .../SmmRelocationLib/AmdSmmRelocationLib.inf   |  60 +++
 .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 125 +
 .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
 .../SmmRelocationLib}/Ia32/SmmInit.nasm|  83 +++-
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 127 +
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 549 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 +++
 .../SmmRelocationLib/SmramSaveStateConfig.c| 136 +
 .../SmmRelocationLib}/X64/Semaphore.c  |  13 +-
 .../SmmRelocationLib}/X64/SmmInit.nasm |  85 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c  |  21 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  10 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 331 ++---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 103 +---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   4 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h  |   2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c |  69 ---
 UefiCpuPkg/UefiCpuPkg.dec  |   3 +
 UefiCpuPkg/UefiCpuPkg.dsc  |   2 +
 UefiPayloadPkg/UefiPayloadPkg.dsc  |   2 +
 48 files changed, 2514 insertions(+), 794 deletions(-)
 copy {UefiCpuPkg/PiSmmCpuDxeSmm => 
OvmfPkg/Library/SmmRelocationLib}/Ia32/Semaphore.c (71%)
 copy {UefiCpuPkg/PiSmmCpuDxeSmm => 
OvmfPkg/Library/SmmRelocationLib}/Ia32/SmmInit.nasm (47%)
 create mode 100644 

Re: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex & wrmsr_ex functions to read/write cpu specific msrs

[Jayaprakash, N]

Thanks Mike. I shall make necessary changes and submit the PR again for review.

Regards,
JP

-Original Message-
From: Kinney, Michael D  
Sent: Thursday, April 18, 2024 10:46 AM
To: Jayaprakash, N ; devel@edk2.groups.io
Cc: Rebecca Cran ; Kinney, Michael D 

Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex & 
wrmsr_ex functions to read/write cpu specific msrs

Please use MP Services Protocol APIs StartupAllAPs() or StarupThisAP() to 
read/write MSRs on other logical processors.

There and many examples of this in the UefiCpuPkg to programming MSRs on all 
the logical processors.

Mike

> -Original Message-
> From: Jayaprakash, N 
> Sent: Wednesday, April 17, 2024 8:16 PM
> To: Kinney, Michael D ; 
> devel@edk2.groups.io
> Cc: Rebecca Cran 
> Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex 
> & wrmsr_ex functions to read/write cpu specific msrs
> 
> In the validation and debug scenarios, engineers tend to read MSRs and 
> write to MSRs of different CPUs.
> So we are providing a simple mechanism through these APIs to enable 
> them to do these operations.
> 
> These APIs will be part of the edk2 module of the Python interpreter 
> and will be used during the validation and debug scenarios only.
> 
> This is not for switching the BSP for OS boot. This is only used 
> during the validation and debug use cases.
> 
> Regards,
> JP
> -Original Message-
> From: Kinney, Michael D 
> Sent: Thursday, April 18, 2024 12:08 AM
> To: Jayaprakash, N ; devel@edk2.groups.io
> Cc: Rebecca Cran ; Kinney, Michael D 
> 
> Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex 
> & wrmsr_ex functions to read/write cpu specific msrs
> 
> Hi JP,
> 
> Is there a reason switch BSP is being used.  That is not a common 
> operation and is typically used if the current BSP is not stable or 
> there is a good reason to switch the BSP for OS boot.
> 
> The MP Services can be used to execute a C function on APs to execute 
> MSR related instructions.
> 
> Mike
> 
> > -Original Message-
> > From: Jayaprakash, N 
> > Sent: Sunday, April 14, 2024 10:33 PM
> > To: devel@edk2.groups.io; Jayaprakash, N 
> > Cc: Rebecca Cran ; Kinney, Michael D 
> > 
> > Subject: RE: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add
> rdmsr_ex
> > & wrmsr_ex functions to read/write cpu specific msrs
> >
> > + Rebecca and Mike,
> >
> > Would you be able to review this PR?
> >
> > Regards,
> > JP
> >
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of 
> > Jayaprakash, N
> > Sent: Wednesday, April 10, 2024 11:49 AM
> > To: devel@edk2.groups.io
> > Cc: Jayaprakash, N ; Rebecca Cran 
> > ; Kinney, Michael D 
> > Subject: [edk2-devel] [edk2-libc Patch 1/1] edk2-libc:add rdmsr_ex & 
> > wrmsr_ex functions to read/write cpu specific msrs
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4746
> >
> > The rdmsr_ex and wrmsr_ex are extension APIs to the rdmsr and wrmsr 
> > APIs supported in edk2 module. These extension APIs makes it
> possible
> > to read / write MSRs from specific processors and fills an existing 
> > gap in this area.
> >
> > Cc: Rebecca Cran 
> > Cc: Michael D Kinney 
> > Cc: Jayaprakash N 
> > Signed-off-by: Jayaprakash N 
> > ---
> >  .../PyMod-3.6.8/Modules/edk2module.c  | 159
> > +-
> >  .../Python/Python-3.6.8/Python368.inf |   3 +
> >  2 files changed, 158 insertions(+), 4 deletions(-)
> >
> > diff --git a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> > 3.6.8/Modules/edk2module.c b/AppPkg/Applications/Python/Python-
> > 3.6.8/PyMod-3.6.8/Modules/edk2module.c
> > index d6af8da..f1b13a6 100644
> > --- a/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> > 3.6.8/Modules/edk2module.c
> > +++ b/AppPkg/Applications/Python/Python-3.6.8/PyMod-
> > 3.6.8/Modules/edk2mo
> > +++ dule.c
> > @@ -3,7 +3,7 @@
> >  Derived from posixmodule.c in Python 2.7.2.
> >
> >  Copyright (c) 2015, Daryl McDaniel. All rights reserved.
> > -Copyright (c) 2011 - 2023, Intel Corporation. All rights
> > reserved.
> > +Copyright (c) 2011 - 2024, Intel Corporation. All rights 
> > + reserved.
> >  This program and the accompanying materials are licensed and
> made
> > available under
> >  the terms and conditions of the BSD License that accompanies
> this
> > distribution.
> >  The full text of the license may be found at @@ -22,16 +22,23
> @@
> > #include#include#include  
> > +#include// Needed for the definition of
> > EFI_AP_PROCEDURE
> >  #include  
> >  #include  
> >  #include  
> >  #include  
> > +#include  
> > +#include  
> >
> >  #ifdef __cplusplus
> >  extern "C" {
> >  #endif
> >
> >  PyTypeObject EfiGuidType;
> > +EFI_MP_SERVICES_PROTOCOL   *gpMpService = NULL;
> > +UINTN   gBSPProcessorNumber = 0;
> > +UINTN   gNumberOfProcessors = 0;
> > +UINTN   gNumberOfEnabledProcessors = 0;
> >
> >  extern void