Re: [edk2-devel] [PATCH v3 1/4] OvmfPkg/Sec: Setup MTRR early in the boot process.

[Corvin Köhne]

On Tue, 2024-02-20 at 09:15 +0100, Gerd Hoffmann wrote:
> On Tue, Feb 20, 2024 at 06:27:21AM +, Min Xu wrote:
> > On Monday, February 12, 2024 11:22 PM, Gerd Hoffmann wrote:
> > > On Thu, Feb 01, 2024 at 10:38:43AM +0100, Gerd Hoffmann wrote:
> > > >   Hi,
> > > > 
> > > > > > Can you confirm (a) this patch is OK for
> > > > > > "OvmfPkg/IntelTdx/Sec/SecMain.c", and (b) this series fixes
> > > > > > the slowdown
> > > you had encountered?
> > > > > > 
> > > > > > (that's what's left before we can merge this series)
> > > > > > 
> > > > > We test the patch in TDX and find EXIT_REASON_CR_ACCESS is
> > > > > triggered in
> > > DXE phase.
> > > > 
> > > > Hmm.  Sure this caused by this patch series?  For the PEI-less
> > > > TDX
> > > > build this series moves the MTRR setup to a different place in
> > > > SEC.
> > > > Once the DXE phase started the MTRR configuration should be
> > > > identical
> > > > with and without this patch series, and the series also doesn't
> > > > touch
> > > > any control register.
> > > 
> > > Ping.  Can you double-check please?  Our QE ran a test build with
> > > this series
> > > applied through regression testing (including TDX) and has not
> > > found any
> > > issues.
> > 
> > We double check the patch-set (v3) for both OvmfPkgX64 and
> > IntelTdx.
> > It triggered EXIT_REASON_CR_ACCESS in DXE phase when launching a
> > td-guest.
> 
> Have you been able to figure which control register access caused the
> EXIT_REASON_CR_ACCESS?
> 
> > @Gerd, what's the qemu command and test environment your QE
> > run the case? We'd like run it in our side.
> 
> 
> 
> Tested edk2-ovmf-20231122-1.el9.rhel21704.20240202.1130.noarch with
> TDX guest, no issue found
> 
> Version:
> 
> edk2-ovmf-20231122-1.el9.rhel21704.20240202.1130.noarch
> 
> guest kernel: 5.14.0-415.el9.x86_64
> 
> qemu-kvm-8.0.0-15.el9s.x86_64
> host kernel-5.14.0-411.test.el9s.x86_64
> 
> Steps:
> 
> $ sudo /usr/libexec/qemu-kvm  -accel kvm   -drive
> file=/home/zixchen/rhel94_tdx.qcow2,if=none,id=virtio-disk0   -device
> virtio-blk-pci,drive=virtio-disk0   -cpu host -smp 16 -m 10240 -
> object tdx-guest,id=tdx,debug=on   -machine
> q35,hpet=off,kernel_irqchip=split,memory-encryption=tdx,confidential-
> guest-support=tdx,memory-backend=ram1   -object memory-backend-
> ram,id=ram1,size=10240M,private=on  -nographic -vga none   -
> nodefaults -bios /usr/share/edk2/ovmf/OVMF.inteltdx.secboot.fd  -
> serial stdio  -netdev user,id=user.0 -device e1000,netdev=user.0
> 
> $ dmesg|grep -i tdx
> [    0.00] tdx: Guest detected
> [    0.719122] TECH PREVIEW: Intel Trusted Domain Extensions (TDX)
> may not be fully supported.
> [    0.719122]  Intel TDX
> [    0.719122] process: using TDX aware idle routine
> 
> 
> 
> Host configuration with the tdx test packages:
> https://sigs.centos.org/virt/tdx/host/
> 
> Latest edk2 build (stable202311 + patches) has the patch series
> included:
> 
> https://kojihub.stream.centos.org/koji/buildinfo?buildID=56985
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 
> 

Hi,

any progress on that patch?

I'm currently trying to passthrough the integrated GPU of an Intel
CPUs. When I add the GPU to the qemu command, I'm faced with the
descripted issue. This patch solves the issue.


-- 
Kind regards,
Corvin


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




signature.asc
Description: This is a digitally signed message part

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Ard Biesheuvel]

Hello all,

On Thu, 11 Apr 2024 at 03:20, Yao, Jiewen  wrote:
>
> Hi Dionna/Qinkun
> I am not sure if systemd is the last software in guest we need to patch to 
> support coexistence to extend the measurement.
> Are you aware of any other Linux guest software needs to be updated? Such as 
> Linux IMA (Integrity Measurement Architecture)?
>
> To move this forward.
>
> In Intel, we had discussed and we did see the potential security risk. As I 
> mentioned in the first email, "In case that any the guest component only 
> knows one of vTPM or RTMR, and only extends one of vTPM or RTMR, but the 
> other one only verifies the other, then the chain of trust is broken."
>
> At same time, we also respect that it might be a valid use case for Google.
> I would like to ask the opinion in the EDKII community, especially the OVMF 
> and CC maintainer and reviewer.
>
>
> Hi Ard Biesheuvel
> Do you think Kernel is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>

I think it is a bad idea to go and apply changes all across the boot
software ecosystem to measure the same assets into different
measurement protocols. I'mm afraid it creates technical debt that will
come and bite us in the future.

Given that RTMR is a proper subset of vTPM (modulo the PCR/RTMR index
conversion), I feel that it should be the CoCo firmware's
responsibility to either:
- expose RTMR and not vTPM
- expose vTPM, and duplicate each measurement into RTMR as they are taken

However, I understand that this is only viable for execution under the
UEFI boot services, and after that, the vTPM and RTMR are exposed in
different ways to the OS.

Could someone explain how that piece of the puzzle is supposed to
work? Do we measure into RTMR after ExitBootServices()?


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

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[qinkun Bao via groups.io]

Hi Jiewen,

Thank you!

On Wed, Apr 10, 2024 at 3:20 PM Yao, Jiewen  wrote:
>
> Hi Dionna/Qinkun
> I am not sure if systemd is the last software in guest we need to patch to 
> support coexistence to extend the measurement.

The direct boot patch needs to be patched as well. Here is the link.
efi/libstub: Add Confidential Computing (CC) measurement support -
Kuppuswamy Sathyanarayanan (kernel.org)
https://lore.kernel.org/lkml/20240215030002.281456-2-sathyanarayanan.kuppusw...@linux.intel.com/
Ard is the maintainer for EFI Stub.

> Are you aware of any other Linux guest software needs to be updated? Such as 
> Linux IMA (Integrity Measurement Architecture)?

You are right that Linux IMA needs to support coexistence. However,
the TDX RTMR IMA support has not been merged into the kernel source
code yet. I have never seen the TDX IMA patch in the LKML as well.

I find that Intel's TDX MVP kernel has the TDX RTMR IMA support patch.
Here is the link
https://github.com/intel/tdx-tools/tree/tdx-1.5/build/common
For what I see, the TDX RTMR IMA patches ([PATCH 672/731] ima: support
for boot aggregate and runtime
measurements in TDX RTMR) from TDX MVP kernel support the coexistence.
The patch author is Ruoyu Ying .


>
> To move this forward.
>
> In Intel, we had discussed and we did see the potential security risk. As I 
> mentioned in the first email, "In case that any the guest component only 
> knows one of vTPM or RTMR, and only extends one of vTPM or RTMR, but the 
> other one only verifies the other, then the chain of trust is broken."
>
> At same time, we also respect that it might be a valid use case for Google.
> I would like to ask the opinion in the EDKII community, especially the OVMF 
> and CC maintainer and reviewer.
>
>
> Hi Ard Biesheuvel
> Do you think Kernel is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
> Hi Gerd Hoffman
> Do you think RedHat is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
> Hi James Bottomley
> Do you think IBM is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
> Hi Tom Lendacky/Michael Roth
> Do you think AMD is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
>
> Thank you
> Yao, Jiewen
>
>
> > -Original Message-
> > From: Dionna Amalie Glaze 
> > Sent: Monday, March 25, 2024 11:29 PM
> > To: Mikko Ylinen 
> > Cc: Gerd Hoffmann ; Yao, Jiewen ;
> > qinkun Bao ; devel@edk2.groups.io; linux-
> > c...@lists.linux.dev; Aktas, Erdem ; Ard Biesheuvel
> > ; Peter Gonda ; James Bottomley
> > ; Tom Lendacky ; Michael
> > Roth 
> > Subject: Re: [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for 
> > coexistance
> > of vTPM and RTMR.
> >
> > On Mon, Mar 25, 2024 at 6:07 AM Mikko Ylinen
> >  wrote:
> > >
> > > > >
> > > > > Looking at systemd-boot I see it will likewise not measure to both 
> > > > > RTMR
> > > > > and vTPM, but with reversed priority (use vTPM not RTMR in case both 
> > > > > are
> > > > > present).
> > > > >
> > > >
> > > > Interesting. Thanks for this report. We'll push for the changed
> > > > semantics here if the spec is indeed changed, and request partner
> > > > distros in the CCC to include the updated systemd-boot.
> > >
> > > FWIW, my RTMRs patch to systemd was merged quite recently so it's not
> > > included in any systemd release yet. (It was mainly implemented for the
> > > UKI case that allows TDVF to boot a UKI image directly and then have the
> > > image sections measured separately.)
> > >
> >
> > Thank you, I've proposed a change in
> > https://github.com/systemd/systemd/pull/31939
> >
> >
> > --
> > -Dionna Glaze, PhD (she/her)


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

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

[Wu, Jiaxin]

Share the PR here: https://github.com/tianocore/edk2/pull/5546

From: Ni, Ray 
Sent: Thursday, April 11, 2024 11:15 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 

Subject: Re: [PATCH v1 00/13] Add SmmRelocationLib

Jiaxin, can you kindly share a PR in GitHub so that it will be easier to review?

Thanks,
Ray

From: Wu, Jiaxin mailto:jiaxin...@intel.com>>
Sent: Wednesday, April 10, 2024 21:57
To: devel@edk2.groups.io 
mailto: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>>
Subject: [PATCH v1 00/13] Add SmmRelocationLib

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>>
Signed-off-by: Jiaxin Wu mailto:jiaxin...@intel.com>>

Jiaxin Wu (13):
  UefiCpuPkg: Add SmmRelocationLib class
  UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
  UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF
  UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
  UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib in UefiCpuPkg
  UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
  OvmfPkg: Include SmmRelocationLib in OvmfPkg
  OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
  OvmfPkg/SmmAccess: Consume gEfiSmmSmramMemoryGuid
  OvmfPkg/PlatformInitLib: Create gEfiAcpiVariableGuid
  OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
  OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
  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 +-
 OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
 OvmfPkg/OvmfPkgIa32.dsc|   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
 OvmfPkg/OvmfPkgX64.dsc |   1 +
 OvmfPkg/PlatformPei/Platform.c |   1 +
 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   |  61 ++
 .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 109 
 .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 157 +
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 141 +
 .../SmmRelocationLib/OvmfSmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/OvmfSmramSaveStateConfig.c| 107 
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 659 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/SmramSaveStateConfig.c|  91 +++
 .../SmmRelocationLib}/X64/Semaphore.c  |  13 +-
 .../SmmRelocationLib}/X64/SmmInit.nasm |  

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

[Ni, Ray]

I think the commit message can have more details to help OVMF developers 
understand the needs of this patch.
E.g.:
  Before using SmmRelocationLib, the PiSmmCpuDxeSmm driver allocates the SMRAM 
to be used for SMI handler + 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 when the Smst->AllocatePages() service is not 
available. So, the allocation is done by
  splitting the SMRAM out of the SMRAM regions reported from Guid HOB 
(gEfiSmmSMramMemoryGuid).
  However, OVMF platform does not produce the GUID HOB. This patch produces the 
HOB..

Thanks,
Ray


From: Wu, Jiaxin 
Sent: Wednesday, April 10, 2024 21:57
To: devel@edk2.groups.io 
Cc: Ard Biesheuvel ; Yao, Jiewen 
; Gerd Hoffmann ; Ni, Ray 

Subject: [PATCH v1 08/13] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid

SmmRelocationLib instance will reserve the memory from
gEfiSmmSmramMemoryGuid for the smbase relocation.

So, system must produce the gEfiSmmSmramMemoryGuid.

This patch is for the OVMF to produce the
gEfiSmmSmramMemoryGuid HOB.

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 (
+&gEfiSmmSmramMemoryGuid,
+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
-// tra

Re: [edk2-devel] [PATCH v1 05/13] UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib in UefiCpuPkg

[Ni, Ray]

Please change the DSC in the earlier patch that adds the lib instance and avoid 
this patch.


Thanks,
Ray

From: Wu, Jiaxin 
Sent: Wednesday, April 10, 2024 21:57
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R 
Subject: [PATCH v1 05/13] UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib 
in UefiCpuPkg

This patch just includes SmmRelocationLib in UefiCpuPkg.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 UefiCpuPkg/UefiCpuPkg.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index 10b33594e5..631a850c78 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -194,10 +194,13 @@
   
UnitTestResultReportLib|UnitTestFrameworkPkg/Library/UnitTestResultReportLib/UnitTestResultReportLibConOut.inf
   }
   UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
   UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf
   UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
+  UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
+  UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
+  UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf

 [Components.X64]
   
UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf

 [Components.RISCV64]
--
2.16.2.windows.1



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

Re: [edk2-devel] [PATCH v1 03/13] UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF

[Ni, Ray]

No. Please do not create a OVMF specific lib instance in UefiCpuPkg.


Thanks,
Ray

From: Wu, Jiaxin 
Sent: Wednesday, April 10, 2024 21:57
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Zeng, Star ; Ard 
Biesheuvel ; Yao, Jiewen ; 
Gerd Hoffmann ; Kumar, Rahul R 
Subject: [PATCH v1 03/13] UefiCpuPkg/SmmRelocationLib: Add library instance for 
OVMF

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

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

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../SmmRelocationLib/OvmfSmmRelocationLib.inf  |  61 
 .../SmmRelocationLib/OvmfSmramSaveStateConfig.c| 107 +
 2 files changed, 168 insertions(+)
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf 
b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
new file mode 100644
index 00..eba1129ac2
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
@@ -0,0 +1,61 @@
+## @file
+# SMM Relocation Lib for each processor.
+#
+# This Lib produces the SMM_BASE_HOB in HOB database which tells
+# the PiSmmCpuDxeSmm driver (runs at a later phase) about the new
+# SMBASE for each processor. PiSmmCpuDxeSmm driver installs the
+# SMI handler at the SMM_BASE_HOB.SmBase[Index]+0x8000 for processor
+# Index.
+#
+# Copyright (c) 2024, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = SmmRelocationLib
+  FILE_GUID  = 51834F51-CCE0-4743-B553-935D0C8A53FF
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = SmmRelocationLib
+
+[Sources]
+  InternalSmmRelocationLib.h
+  OvmfSmramSaveStateConfig.c
+  SmmRelocationLib.c
+
+[Sources.Ia32]
+  Ia32/Semaphore.c
+  Ia32/SmmInit.nasm
+
+[Sources.X64]
+  X64/Semaphore.c
+  X64/SmmInit.nasm
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  CpuExceptionHandlerLib
+  DebugLib
+  HobLib
+  LocalApicLib
+  MemoryAllocationLib
+  PcdLib
+  PeiServicesLib
+
+[Guids]
+  gSmmBaseHobGuid   ## HOB ALWAYS_PRODUCED
+  gEfiSmmSmramMemoryGuid## CONSUMES
+
+[Pcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize ## CONSUMES
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport## 
CONSUMES
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c 
b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c
new file mode 100644
index 00..505b1d694a
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c
@@ -0,0 +1,107 @@
+/** @file
+  Config SMRAM Save State for SmmBases Relocation.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include "InternalSmmRelocationLib.h"
+#include 
+
+/**
+  This function configures the SmBase on the currently executing CPU.
+
+  @param[in] CpuIndex The index of the CPU.
+  @param[in,out] CpuState Pointer to SMRAM Save State Map for the
+  currently executing CPU. On out, SmBase 
is
+  updated to the new value.
+
+**/
+VOID
+EFIAPI
+ConfigureSmBase (
+  IN UINTN CpuIndex,
+  IN OUT SMRAM_SAVE_STATE_MAP  *CpuState
+  )
+{
+  AMD_SMRAM_SAVE_STATE_MAP  *CpuSaveState;
+
+  CpuSaveState = (AMD_SMRAM_SAVE_STATE_MAP *)CpuState;
+
+  if ((CpuSaveState->x86.SMMRevId & 0x) == 0) {
+CpuSaveState->x86.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  } else {
+CpuSaveState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  }
+}
+
+/**
+  This function updates the SMRAM save state on the currently executing CPU
+  to resume execution at a specific address after an RSM instruction.  This
+  function must evaluate the SMRAM save state to determine the execution mode
+  the RSM instruction resumes and update the resume execution address with
+  either NewInstructionPointer32 or NewInstructionPoint.  The auto HALT restart
+  flag in the SMRAM save state must always be cleared.  This function returns
+  the value of the instruction pointer from the SMRAM save state that was
+  replaced.  If this function returns 0, then the SMRAM save state was not
+  modified.
+
+  This

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

[Ni, Ray]

Not sure if "--find-copies-harder" helps to detect that the new files are 
copies of existing files.
Can you try in next version if some comments request a new version of patch?

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Wednesday, April 10, 2024 21:57
To: devel@edk2.groups.io 
Cc: Ni, Ray ; Zeng, Star ; Gerd Hoffmann 
; Kumar, Rahul R 
Subject: [PATCH v1 02/13] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib 
library instance

This patch separates the smbase relocation logic from
PiSmmCpuDxeSmm driver, and moves to the
SmmRelocationInit interface.

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

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../Library/SmmRelocationLib/Ia32/Semaphore.c  |  43 ++
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 157 +
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 141 +
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 659 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/SmramSaveStateConfig.c|  91 +++
 .../Library/SmmRelocationLib/X64/Semaphore.c   |  70 +++
 .../Library/SmmRelocationLib/X64/SmmInit.nasm  | 207 +++
 8 files changed, 1429 insertions(+)
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
 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
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/X64/SmmInit.nasm

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
new file mode 100644
index 00..ace3221cfc
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
@@ -0,0 +1,43 @@
+/** @file
+  Semaphore mechanism to indicate to the BSP that an AP has exited SMM
+  after SMBASE relocation.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalSmmRelocationLib.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)&SmmRelocationSemaphoreComplete,
+   
(UINT64)(UINTN)&SmmRelocationSemaphoreComplete
+   );
+}
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
new file mode 100644
index 00..cb8b030693
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
@@ -0,0 +1,157 @@
+;--
 ;
+; Copyright (c) 2024, Intel Corporation. All rights reserved.
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+;   SmmInit.nasm
+;
+; Abstract:
+;
+;   Functions for relocating SMBASE's for all processors
+;
+;---
+
+%include "StuffRsbNasm.inc"
+
+global  ASM_PFX(gcSmiIdtr)
+global  ASM_PFX(gcSmiGdtr)
+
+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(gPatchSmmInitStack)
+global ASM_PFX(gcSmmInitSize)
+global ASM_PFX(gcSmmInitTemplate)
+
+%define PROTECT_MODE_CS 0x8
+%define PROTECT_MODE_DS 0x20
+
+SECTION .data
+
+NullSeg: DQ 0   ; reserved by architecture
+CodeSeg32:
+DW  -1  ; LimitLow
+DW  0   ; BaseLow
+DB  0   ; BaseMid
+DB  0x9b

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

[Ni, Ray]

Jiaxin, can you kindly share a PR in GitHub so that it will be easier to review?

Thanks,
Ray

From: Wu, Jiaxin 
Sent: Wednesday, April 10, 2024 21:57
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 
Subject: [PATCH v1 00/13] Add SmmRelocationLib

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 
Signed-off-by: Jiaxin Wu 

Jiaxin Wu (13):
  UefiCpuPkg: Add SmmRelocationLib class
  UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
  UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF
  UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
  UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib in UefiCpuPkg
  UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
  OvmfPkg: Include SmmRelocationLib in OvmfPkg
  OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
  OvmfPkg/SmmAccess: Consume gEfiSmmSmramMemoryGuid
  OvmfPkg/PlatformInitLib: Create gEfiAcpiVariableGuid
  OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
  OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
  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 +-
 OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
 OvmfPkg/OvmfPkgIa32.dsc|   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
 OvmfPkg/OvmfPkgX64.dsc |   1 +
 OvmfPkg/PlatformPei/Platform.c |   1 +
 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   |  61 ++
 .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 109 
 .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 157 +
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 141 +
 .../SmmRelocationLib/OvmfSmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/OvmfSmramSaveStateConfig.c| 107 
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 659 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/SmramSaveStateConfig.c|  91 +++
 .../SmmRelocationLib}/X64/Semaphore.c  |  13 +-
 .../SmmRelocationLib}/X64/SmmInit.nasm |  93 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c  |  21 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm|  96 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |   6 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 322 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  98 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   4 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c |  69 ---
 UefiCpuPkg/UefiCpuPkg.dec  |   3 +
 UefiCpuPkg/UefiCpuPkg.dsc  |   3 +
 UefiPayloadPkg/UefiPayloadPkg.dsc  |   2 +
 42 files changed, 1850 insertions(+), 817 deletions(-)
 create mode 100644 OvmfPkg/PlatformPei/SmmRelocation.c
 create mode 100644 UefiCpuPkg/Include/Library/SmmRelocationLib.h
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
 rename UefiCpuPkg/{PiSmmCpuDxeSmm => 
Library/SmmRelocationLib}/

[edk2-devel] [PATCH v3] SecurityPkg/OpalPasswordDxe: Update UI according to UEFI spec

[Cindy Kuo]

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

Should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN
call back function.
Those APIs are called within OpalHiiSetBrowserData/OpalHiiGetBrowserData
which have been used by OpalHii.c.

1. Change callback action from FORM_OPEN to RETRIEVE.
2. Create dummy label with suppressif statement in VFR for form update
usage.
3. Add HiiUpdateForm() to force reparsing the IFR binary.

Cc: Jiewen Yao 
Cc: Rahul Kumar 
Cc: Dandan Bi 
Cc: Ming Tan 
Cc: Arthur Chen 
Cc: Xiao X Chen 
Cc: Tina Chen 
Signed-off-by: CindyX Kuo 
---
 .../Tcg/Opal/OpalPassword/OpalDriver.h|  1 +
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   | 84 ---
 .../Tcg/Opal/OpalPassword/OpalHiiFormValues.h |  6 ++
 .../Tcg/Opal/OpalPassword/OpalPasswordDxe.inf |  1 +
 .../Opal/OpalPassword/OpalPasswordForm.vfr|  8 +-
 5 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h 
b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
index 2089bd81b6..1a4671c602 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
@@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c 
b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
index 8035f44ebe..47af4fee40 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
@@ -40,6 +40,7 @@ EFI_HII_HANDLE  gHiiPackageListHandle = NULL;
 //
 const EFI_GUID  gHiiPackageListGuid   = PACKAGE_LIST_GUID;
 const EFI_GUID  gHiiSetupVariableGuid = SETUP_VARIABLE_GUID;
+const EFI_GUID  gOpalSetupFormSetGuid = SETUP_FORMSET_GUID;
 
 //
 // Structure that contains state of the HII
@@ -611,10 +612,15 @@ DriverCallback (
   EFI_BROWSER_ACTION_REQUEST*ActionRequest
   )
 {
-  HII_KEYHiiKey;
-  UINT8  HiiKeyId;
-  UINT32 PpRequest;
-  OPAL_DISK  *OpalDisk;
+  HII_KEY HiiKey;
+  UINT8   HiiKeyId;
+  UINT32  PpRequest;
+  OPAL_DISK   *OpalDisk;
+  EFI_STATUS  Status;
+  VOID*StartOpCodeHandle;
+  VOID*EndOpCodeHandle;
+  EFI_IFR_GUID_LABEL  *StartLabel;
+  EFI_IFR_GUID_LABEL  *EndLabel;
 
   if (ActionRequest != NULL) {
 *ActionRequest = EFI_BROWSER_ACTION_REQUEST_NONE;
@@ -632,15 +638,69 @@ DriverCallback (
   HiiKey.Raw = QuestionId;
   HiiKeyId   = (UINT8)HiiKey.KeyBits.Id;
 
-  if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {
-switch (HiiKeyId) {
-  case HII_KEY_ID_VAR_SUPPORTED_DISKS:
-DEBUG ((DEBUG_INFO, "HII_KEY_ID_VAR_SUPPORTED_DISKS\n"));
-return HiiPopulateMainMenuForm ();
+  if (Action == EFI_BROWSER_ACTION_RETRIEVE) {
+if ((HiiKeyId == HII_KEY_ID_VAR_SUPPORTED_DISKS) || (HiiKeyId == 
HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS)) {
+  //
+  // Allocate space for creation of UpdateData Buffer
+  //
+  StartOpCodeHandle = HiiAllocateOpCodeHandle ();
+  if (StartOpCodeHandle == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  EndOpCodeHandle = HiiAllocateOpCodeHandle ();
+  if (EndOpCodeHandle == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Create Hii Extend Label OpCode as the start opcode
+  //
+  StartLabel   = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode 
(StartOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL));
+  StartLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
+
+  //
+  // Create Hii Extend Label OpCode as the end opcode
+  //
+  EndLabel   = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode 
(EndOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL));
+  EndLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL;
+
+  switch (HiiKeyId) {
+case HII_KEY_ID_VAR_SUPPORTED_DISKS:
+  DEBUG ((DEBUG_INFO, "HII_KEY_ID_VAR_SUPPORTED_DISKS\n"));
+  Status = HiiPopulateMainMenuForm ();
+
+  StartLabel->Number = OPAL_MAIN_MENU_LABEL_START;
+  EndLabel->Number   = OPAL_MAIN_MENU_LABEL_END;
+  HiiUpdateForm (
+gHiiPackageListHandle,
+(EFI_GUID *)&gOpalSetupFormSetGuid,
+FORMID_VALUE_MAIN_MENU,
+StartOpCodeHandle,
+EndOpCodeHandle
+);
+  break;
+
+case HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS:
+  DEBUG ((DEBUG_INFO, 
"HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS\n"));
+  Status = HiiPopulateDiskInfoForm ();
+
+  StartLabel->Number = OPAL_DISK_INFO_LABEL_START;
+  EndLabel->Number   = OPAL_DISK_INFO_LABEL_END;
+  HiiUpdateForm (
+gHiiPackageListHandle,
+(EFI_GUID *)&gOpalSetupFormSetGuid,
+FORMID_VALUE_DISK_INFO_FORM_MAIN,
+StartOpCodeHandle,
+EndOpCodeHand

Re: [edk2-devel] [PATCH V3 6/6] Maintainers.txt: Update maintainer for SPI modules

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Hi maintainers,
Please help to check this update as we added an entry for Bus\Spi under 
MdeModulePkg.

Thanks
Abner

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chang,
> Abner via groups.io
> Sent: Sunday, August 13, 2023 12:37 PM
> To: devel@edk2.groups.io
> Cc: Michael D Kinney ; Andrew Fish
> ; Leif Lindholm ; Attar,
> AbdulLateef (Abdul Lateef) ; Chesley, Brit
> ; Jian J Wang ; Liming Gao
> ; Hao A Wu ; Ray Ni
> ; Zhiguang Liu 
> Subject: [edk2-devel] [PATCH V3 6/6] Maintainers.txt: Update maintainer for
> SPI modules
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> From: Abner Chang 
>
> Add maintainer entry of MdeModulePkg/Bus/Spi.
>
> Cc: Michael D Kinney 
> Cc: Andrew Fish 
> Cc: Leif Lindholm 
> Cc: Abdul Lateef Attar 
> Cc: Brit Chesley 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> Cc: Zhiguang Liu 
> Signed-off-by: Abner Chang 
> ---
>  Maintainers.txt | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 5a95929a14b..a6a5a9b4e03 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -443,6 +443,11 @@ F:
> MdeModulePkg/Include/Protocol/UsbEthernetProtocol.h
>  M: Richard Ho  [richardho]
>  R: Rebecca Cran  [bcran]
>
> +MdeModulePkg: SPI Bus modules
> +F: MdeModulePkg/Bus/Spi
> +M: Abner Chang  [changab]
> +R: Brit Chesley  [BritChesley]
> +
>  MdePkg
>  F: MdePkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/MdePkg
> --
> 2.37.1.windows.1
>
>
>
> 
>



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

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Yao, Jiewen]

Hi Dionna/Qinkun
I am not sure if systemd is the last software in guest we need to patch to 
support coexistence to extend the measurement.
Are you aware of any other Linux guest software needs to be updated? Such as 
Linux IMA (Integrity Measurement Architecture)?

To move this forward.

In Intel, we had discussed and we did see the potential security risk. As I 
mentioned in the first email, "In case that any the guest component only knows 
one of vTPM or RTMR, and only extends one of vTPM or RTMR, but the other one 
only verifies the other, then the chain of trust is broken."

At same time, we also respect that it might be a valid use case for Google.
I would like to ask the opinion in the EDKII community, especially the OVMF and 
CC maintainer and reviewer.


Hi Ard Biesheuvel
Do you think Kernel is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?

Hi Gerd Hoffman
Do you think RedHat is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?

Hi James Bottomley
Do you think IBM is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?

Hi Tom Lendacky/Michael Roth
Do you think AMD is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?


Thank you
Yao, Jiewen


> -Original Message-
> From: Dionna Amalie Glaze 
> Sent: Monday, March 25, 2024 11:29 PM
> To: Mikko Ylinen 
> Cc: Gerd Hoffmann ; Yao, Jiewen ;
> qinkun Bao ; devel@edk2.groups.io; linux-
> c...@lists.linux.dev; Aktas, Erdem ; Ard Biesheuvel
> ; Peter Gonda ; James Bottomley
> ; Tom Lendacky ; Michael
> Roth 
> Subject: Re: [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance
> of vTPM and RTMR.
> 
> On Mon, Mar 25, 2024 at 6:07 AM Mikko Ylinen
>  wrote:
> >
> > > >
> > > > Looking at systemd-boot I see it will likewise not measure to both RTMR
> > > > and vTPM, but with reversed priority (use vTPM not RTMR in case both are
> > > > present).
> > > >
> > >
> > > Interesting. Thanks for this report. We'll push for the changed
> > > semantics here if the spec is indeed changed, and request partner
> > > distros in the CCC to include the updated systemd-boot.
> >
> > FWIW, my RTMRs patch to systemd was merged quite recently so it's not
> > included in any systemd release yet. (It was mainly implemented for the
> > UKI case that allows TDVF to boot a UKI image directly and then have the
> > image sections measured separately.)
> >
> 
> Thank you, I've proposed a change in
> https://github.com/systemd/systemd/pull/31939
> 
> 
> --
> -Dionna Glaze, PhD (she/her)


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

Re: [edk2-devel] [PATCH] NetworkPkg/WifiConnectionManagerDxe: Update UI according to UEFI spec

[Clark-williams, Zachary]

Reviewed-by: Zachary Clark-Williams 

-Original Message-
From: Tan, Ming  
Sent: Thursday, March 21, 2024 6:16 PM
To: devel@edk2.groups.io
Cc: Kasbekar, Saloni ; Clark-williams, Zachary 
; Bi, Dandan ; POLUDOV, 
FELIX ; Liu, Liqi 
Subject: [PATCH] NetworkPkg/WifiConnectionManagerDxe: Update UI according to 
UEFI spec

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

In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
EFI_BROWSER_ACTION_FORM_OPEN:
NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used with this 
browser action because question values have not been retrieved yet.

So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN 
call back function.

Now use wifi list key and enroll cert key instead of the connect action key, 
move wifi info display from open action to close action.

Cc: Saloni Kasbekar 
Cc: Zachary Clark-williams 
Cc: Dandan Bi 
Cc: Felix Polyudov 
Signed-off-by: Liqi Liu 
Signed-off-by: Ming Tan 
---
  PR: https://github.com/tianocore/edk2/pull/5487

 .../WifiConnectionMgrHiiConfigAccess.c| 113 --
 1 file changed, 74 insertions(+), 39 deletions(-)

diff --git 
a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess.c 
b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess.c
index 431fcbb33a..f242bdf056 100644
--- a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess.c
+++ b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAcce
+++ ss.c
@@ -1412,7 +1412,9 @@ WifiMgrDxeHiiConfigAccessCallback (
 return EFI_OUT_OF_RESOURCES;   } -  HiiGetBrowserData 
(&gWifiConfigFormSetGuid, mVendorStorageName, BufferSize, (UINT8 *)IfrNvData);+ 
 if (Action != EFI_BROWSER_ACTION_FORM_OPEN) {+HiiGetBrowserData 
(&gWifiConfigFormSetGuid, mVendorStorageName, BufferSize, (UINT8 *)IfrNvData);+ 
 }if (Action == EFI_BROWSER_ACTION_FORM_OPEN) { switch (QuestionId) {@@ 
-1438,43 +1440,6 @@ WifiMgrDxeHiiConfigAccessCallback (
  break; -  case KEY_CONNECT_ACTION:--if 
(Private->CurrentNic->UserSelectedProfile == NULL) {-  break;-
}--Profile = Private->CurrentNic->UserSelectedProfile;--//- 
   // Enter the network connection configuration page-// Recovery from 
restored data-//-if (HiiSetString (Private->RegisteredHandle, 
STRING_TOKEN (STR_SSID), Profile->SSId, NULL) == 0) {-  return 
EFI_OUT_OF_RESOURCES;-}--IfrNvData->SecurityType = 
Profile->SecurityType;-if (HiiSetString (-  
Private->RegisteredHandle,-  STRING_TOKEN (STR_SECURITY_TYPE),- 
 mSecurityType[IfrNvData->SecurityType],-  NULL-
  ) == 0)-{-  return EFI_OUT_OF_RESOURCES;-}--
if ((IfrNvData->SecurityType == SECURITY_TYPE_WPA2_ENTERPRISE) ||-
(IfrNvData->SecurityType == SECURITY_TYPE_WPA3_ENTERPRISE))-{-  
IfrNvData->EapAuthMethod   = Profile->EapAuthMethod;-  
IfrNvData->EapSecondAuthMethod = Profile->EapSecondAuthMethod;-  
StrCpyS (IfrNvData->EapIdentity, EAP_IDENTITY_SIZE, Profile->EapIdentity);- 
   }--break;-   case KEY_ENROLLED_CERT_NAME:  if 
(Private->CurrentNic->UserSelectedProfile == NULL) {@@ -1524,6 +1489,43 @@ 
WifiMgrDxeHiiConfigAccessCallback (
 }   } else if (Action == EFI_BROWSER_ACTION_FORM_CLOSE) { switch 
(QuestionId) {+  case KEY_EAP_ENROLL_CERT_FROM_FILE:++if 
(Private->CurrentNic->UserSelectedProfile == NULL) {+  break;+
}++Profile = Private->CurrentNic->UserSelectedProfile;++//+ 
   // Enter the network connection configuration page+// Recovery from 
restored data+//+if (HiiSetString (Private->RegisteredHandle, 
STRING_TOKEN (STR_SSID), Profile->SSId, NULL) == 0) {+  return 
EFI_OUT_OF_RESOURCES;+}++IfrNvData->SecurityType = 
Profile->SecurityType;+if (HiiSetString (+  
Private->RegisteredHandle,+  STRING_TOKEN (STR_SECURITY_TYPE),+ 
 mSecurityType[IfrNvData->SecurityType],+  NULL+
  ) == 0)+{+  return EFI_OUT_OF_RESOURCES;+}++
if (  (IfrNvData->SecurityType == SECURITY_TYPE_WPA2_ENTERPRISE)+   || 
(IfrNvData->SecurityType == SECURITY_TYPE_WPA3_ENTERPRISE))+{+  
IfrNvData->EapAuthMethod   = Profile->EapAuthMethod;+  
IfrNvData->EapSecondAuthMethod = Profile->EapSecondAuthMethod;+  
StrCpyS (IfrNvData->EapIdentity, EAP_IDENTITY_SIZE, Profile->EapIdentity);+ 
   }++break;+   case KEY_CONNECT_ACTION:  if 
(Private->CurrentNic->UserSelectedProfile == NULL) {@@ -1909,6 +1911,39 @@ 
WifiMgrDxeHiiConfigAccessCallback (
   NULL   );   }++  if 
(Private->CurrentNic->UserSelectedProfile == NULL) {+   

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

[Wu, Jiaxin]

Sure, Jiewen, Yes, I have tested on OVMF:

1) Checked the OVMF can normal boot with SMM enabled. 
2) Checked the multiple processor support on OVMF with -smp 4 command.
3) Test the OVMF can response soft SMI, the smbase has been rebased before smm 
cpu driver dispatch.
4) Pass SMM test on intel QSP platform.

Thanks,
Jiaxin


> -Original Message-
> From: Yao, Jiewen 
> Sent: Wednesday, April 10, 2024 10:02 PM
> To: Wu, Jiaxin ; devel@edk2.groups.io
> Cc: Ni, Ray ; Zeng, Star ; Gerd
> Hoffmann ; Kumar, Rahul R ;
> Dong, Guo ; Rhodes, Sean ;
> Lu, James ; Guo, Gua ; Ard
> Biesheuvel 
> Subject: RE: [PATCH v1 00/13] Add SmmRelocationLib
> 
> Hello
> Would you please describe what test has been done for OvmfPkg?
> For example, have you validated OVMF with SMM enabled?
> 
> 
> > -Original Message-
> > From: Wu, Jiaxin 
> > Sent: Wednesday, April 10, 2024 9:57 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 
> > Subject: [PATCH v1 00/13] Add SmmRelocationLib
> >
> > 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 
> > Signed-off-by: Jiaxin Wu 
> >
> > Jiaxin Wu (13):
> >   UefiCpuPkg: Add SmmRelocationLib class
> >   UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
> >   UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF
> >   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
> >   UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib in UefiCpuPkg
> >   UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
> >   OvmfPkg: Include SmmRelocationLib in OvmfPkg
> >   OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
> >   OvmfPkg/SmmAccess: Consume gEfiSmmSmramMemoryGuid
> >   OvmfPkg/PlatformInitLib: Create gEfiAcpiVariableGuid
> >   OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
> >   OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
> >   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 +-
> >  OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
> >  OvmfPkg/OvmfPkgIa32.dsc|   1 +
> >  OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
> >  OvmfPkg/OvmfPkgX64.dsc |   1 +
> >  OvmfPkg/PlatformPei/Platform.c |   1 +
> >  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   |  61 ++
> >  .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 109 
> >  .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
> >  .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 157 +
> >  .../SmmRelocationLib/InternalSmmRelocationLib.h| 141 +
> >  .../SmmRelocationLib/OvmfSmmRelocationLib.inf  |  61 ++
> >  .../SmmRelocationLib/OvmfSmramSaveStateConfig.c| 107 
> >  .../Library/SmmRelocationLib/SmmRelocationLib.c| 659
> > +
> >  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 ++
> >  .../SmmRelocationLib/SmramSaveStateConfig.c|  91 +++
> >  .../SmmRelocationLib}/X64/Semaphore.c  |  13 +-
> >  .../SmmRelocationLib}/X64/SmmInit.nasm

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

[Yao, Jiewen]

Hello
Would you please describe what test has been done for OvmfPkg?
For example, have you validated OVMF with SMM enabled?


> -Original Message-
> From: Wu, Jiaxin 
> Sent: Wednesday, April 10, 2024 9:57 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 
> Subject: [PATCH v1 00/13] Add SmmRelocationLib
> 
> 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 
> Signed-off-by: Jiaxin Wu 
> 
> Jiaxin Wu (13):
>   UefiCpuPkg: Add SmmRelocationLib class
>   UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
>   UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF
>   UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
>   UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib in UefiCpuPkg
>   UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
>   OvmfPkg: Include SmmRelocationLib in OvmfPkg
>   OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
>   OvmfPkg/SmmAccess: Consume gEfiSmmSmramMemoryGuid
>   OvmfPkg/PlatformInitLib: Create gEfiAcpiVariableGuid
>   OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
>   OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
>   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 +-
>  OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
>  OvmfPkg/OvmfPkgIa32.dsc|   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
>  OvmfPkg/OvmfPkgX64.dsc |   1 +
>  OvmfPkg/PlatformPei/Platform.c |   1 +
>  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   |  61 ++
>  .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 109 
>  .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
>  .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 157 +
>  .../SmmRelocationLib/InternalSmmRelocationLib.h| 141 +
>  .../SmmRelocationLib/OvmfSmmRelocationLib.inf  |  61 ++
>  .../SmmRelocationLib/OvmfSmramSaveStateConfig.c| 107 
>  .../Library/SmmRelocationLib/SmmRelocationLib.c| 659
> +
>  .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 ++
>  .../SmmRelocationLib/SmramSaveStateConfig.c|  91 +++
>  .../SmmRelocationLib}/X64/Semaphore.c  |  13 +-
>  .../SmmRelocationLib}/X64/SmmInit.nasm |  93 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c  |  21 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm|  96 ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |   6 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 322 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  98 ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   4 -
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c |  69 ---
>  UefiCpuPkg/UefiCpuPkg.dec  |   3 +
>  UefiCpuPkg/UefiCpuPkg.dsc  |   3 +
>  UefiPayloadPkg/UefiPayloadPkg.dsc  |   2 +
>  42 files changed, 1850 insertions(+), 817 deletions(-)
>  create mode 100644 OvmfPkg/PlatformPei/SmmRelocation.c
>  create mode 100644 UefiCpuPkg/Include/Library/SmmRelocationLib.h
>  

Re: [edk2-devel] [PATCH v1 06/13] UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib

[Guo, Gua]

Reviewed-by: Gua Guo 

From: Wu, Jiaxin 
Sent: Wednesday, April 10, 2024 9:57:17 PM
To: devel@edk2.groups.io 
Cc: Dong, Guo ; Rhodes, Sean ; Lu, 
James ; Guo, Gua ; Ni, Ray 

Subject: [PATCH v1 06/13] UefiPayloadPkg/UefiPayloadPkg.dsc: Include 
SmmRelocationLib

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 
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc 
b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 433fb51a56..328b7fd4fe 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -313,10 +313,12 @@
   
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
   
VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
   CcExitLib|UefiCpuPkg/Library/CcExitLibNull/CcExitLibNull.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 (#117602): https://edk2.groups.io/g/devel/message/117602
Mute This Topic: https://groups.io/mt/105441995/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Wu, Jiaxin]

This patch is to remove legacy SmBase relocation in
PiSmmCpuDxeSmm Driver, and the SmBase relocation
behavior will be in the SmmRelocationInit interface:
1. Relocate smbases for each processor.
2. Create the gSmmBaseHobGuid HOB.

Then, PiSmmCpuDxeSmm driver can be simplified to:
1. Consume the gSmmBaseHobGuid for the smbase.
2. ExecuteFirstSmiInit for early SMM Init.

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|   6 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 322 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |  98 
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   4 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c   |  69 --
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c|  69 --
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm   | 146 
 10 files changed, 30 insertions(+), 843 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)&SmmRelocationSemaphoreComplete,
-   
(UINT64)(UINTN)&SmmRelocationSemaphoreComplete
-   );
-}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
deleted file mode 100644
index b5e77a1a5b..00
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
+++ /dev/null
@@ -1,96 +0,0 @@
-;---

[edk2-devel] [PATCH v1 12/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  |  1 +
 OvmfPkg/PlatformPei/Platform.h  |  5 +++
 OvmfPkg/PlatformPei/PlatformPei.inf |  5 ++-
 OvmfPkg/PlatformPei/SmmRelocation.c | 80 +
 4 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/PlatformPei/SmmRelocation.c

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index f5dc41c3a8..47ccf9e2b9 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -365,8 +365,9 @@ InitializePlatform (
 MiscInitialization (PlatformInfoHob);
   }
 
   IntelTdxInitialize ();
   InstallFeatureControlCallback (PlatformInfoHob);
+  InstallSmmRelocationCallback ();
 
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 1cf44844a7..e2057623b7 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -76,10 +76,15 @@ InstallFeatureControlCallback (
 VOID
 InstallClearCacheCallback (
   VOID
   );
 
+VOID
+InstallSmmRelocationCallback (
+  VOID
+  );
+
 VOID
 AmdSevInitialize (
   IN EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
   );
 
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index ad52be3065..d7a4fd5ffe 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
@@ -63,10 +64,11 @@
   MtrrLib
   MemEncryptSevLib
   PcdLib
   CcExitLib
   PlatformInitLib
+  SmmRelocationLib
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
@@ -137,9 +139,10 @@
 
 [Ppis]
   gEfiPeiMasterBootModePpiGuid
   gEfiPeiMpServicesPpiGuid
   gEfiPeiReadOnlyVariable2PpiGuid
+  gEdkiiPeiMpServices2PpiGuid
 
 [Depex]
   TRUE
 
diff --git a/OvmfPkg/PlatformPei/SmmRelocation.c 
b/OvmfPkg/PlatformPei/SmmRelocation.c
new file mode 100644
index 00..63db7101b5
--- /dev/null
+++ b/OvmfPkg/PlatformPei/SmmRelocation.c
@@ -0,0 +1,80 @@
+/**@file
+  Install a callback to do smm relocation.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include "Platform.h"
+
+/**
+  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.
+
+  @return  Status of the notification. The status code returned from this
+   function is ignored.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+OnMpServices2Available (
+  IN EFI_PEI_SERVICES   **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID   *Ppi
+  )
+{
+  EDKII_PEI_MP_SERVICES2_PPI  *MpServices2;
+  EFI_STATUS  Status;
+
+  DEBUG ((DEBUG_INFO, "%a: %a\n", gEfiCallerBaseName, __func__));
+
+  MpServices2 = Ppi;
+
+  //
+  // Smm Relocation Initialize.
+  //
+  Status = SmmRelocationInit (MpServices2);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_WARN, "OnMpServices2Available: Not able to execute Smm 
Relocation Init.  Status: %r\n", Status));
+  }
+
+  return EFI_SUCCESS;
+}
+
+//
+// Notification object for registering the callback, for when
+// EDKII_PEI_MP_SERVICES2_PPI becomes available.
+//
+STATIC CONST EFI_PEI_NOTIFY_DESCRIPTOR  mMpServices2Notify = {
+  EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK |   // Flags
+  EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+  &gEdkiiPeiMpServices2PpiGuid,  // Guid
+  OnMpServices2Available // Notify
+};
+
+VOID
+InstallSmmRelocationCallback (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = PeiServicesNotifyPpi (&mMpServices2Notify);
+  if (EFI_ERROR (Status)) {
+DEBUG ((
+  DEBUG_ERROR,
+  "%a: failed to set up MP Services2 callback: %r\n",
+  __func__,
+  Status
+  ));
+  }
+}
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117600): https://edk2.groups.io/g/devel/message/117600

[edk2-devel] [PATCH v1 11/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 
---
 .../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 (&gSmmBaseHobGuid) == NULL);
+  mSmmCpuFeaturesSmmRelocated = (BOOLEAN)(GetFirstGuidHob (&gSmmBaseHobGuid) 
!= 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 (#117599): https://edk2.groups.io/g/devel/message/117599
Mute This Topic: https://groups.io/mt/105442004/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 09/13] OvmfPkg/SmmAccess: Consume gEfiSmmSmramMemoryGuid

[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.
The gEfiAcpiVariableGuid HOB will be created in the
OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
Signed-off-by: Jiaxin Wu 
---
 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 +---
 6 files changed, 41 insertions(+), 154 deletions(-)

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]
   gEfiSmmAccess2ProtocolGuid   ## PRODUCES
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
 
+[Guids]
+  gEfiSmmSmramMemoryGuid # ALWAYS_CONSUMED
+
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
 
 [Depex]
diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.c b/OvmfPkg/SmmAccess/SmmAccessPei.c
index 0e57b7804c..9459bcc989 100644
--- a/OvmfPkg/SmmAccess/SmmAccessPei.c
+++ b/OvmfPkg/SmmAccess/SmmAccessPei.c
@@ -9,21 +9,19 @@
 
   This PEIM runs from RAM, so we can write to variables with static storage
   duration.
 
   Copyright (C) 2013, 2015, Red Hat, Inc.
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
 #include 
@@ -62,14 +60,10 @@ SmmAccessPeiOpen (
   IN EFI_PEI_SERVICES**PeiServices,
   IN PEI_SMM_ACCESS_PPI  *This,
   IN UINTN   DescriptorIndex
   )
 {
-  if (DescriptorIndex >= DescIdxCount) {
-return EFI_INVALID_PARAMETER;
-  }
-
   //
   // According to current practice, DescriptorIndex is not considered at all,
   // beyond validating it.
   //
   return SmramAccessOpen (&This->LockState, &This->OpenState);
@@ -100,14 +94,10 @@ SmmAccessPeiClose (
   IN EFI_PEI_SERVICES**PeiServices,
   IN PEI_SMM_ACCESS_PPI  *This,
   IN UINTN   DescriptorIndex
   )
 {
-  if (DescriptorIndex >= DescIdxCount) {
-return EFI_INVALID_PARAMETER;
-  }
-
   //
   // According to current practice, DescriptorIndex is not considered at all,
   // beyond validating it.
   //
   return SmramAccessClose (&This->LockState, &This->OpenState);
@@ -137,14 +127,10 @@ SmmAccessPeiLock (
   IN EFI_PEI_SERVICES**PeiServices,
   IN PEI_SMM_ACCESS_PPI  *This,
   IN UINTN   DescriptorIndex
   )
 {
-  if (DescriptorIndex >= DescIdxCount) {
-return EFI_INVALID_PARAMETER;
-  }
-
   //
   // According to current practice, DescriptorIndex is not considered at all,
   // beyond validating it.
   //
   return SmramAccessLock (&This->LockState, &This->OpenState);
@@ -176,12 +162,10 @@ SmmAccessPeiGetCapabilities (
   IN OUT UINTN *SmramMapSize,
   IN OUT EFI_SMRAM_DESCRIPTOR  *SmramMap
   )
 {
   return SmramAccessGetCapabilities (
-   This->LockState,
-   This->OpenState,
SmramMapSize,
SmramMap
);
 }
 
@@ -238,18 +222,14 @@ EFIAPI
 SmmAccessPeiEntryPoint (
   IN   EFI_PEI_FILE_HANDLE  FileHandle,

[edk2-devel] [PATCH v1 10/13] OvmfPkg/PlatformInitLib: Create gEfiAcpiVariableGuid

[Wu, Jiaxin]

This patch is for OVMF to create the gEfiAcpiVariableGuid
since it has been removed from the SmmAccess driver.

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

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 (&gEfiAcpiVariableGuid, sizeof 
(EFI_SMRAM_DESCRIPTOR));
+ASSERT (GuidHob != NULL);
+CopyMem (GuidHob, &SmramHobDescriptorBlock->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
 
-- 
2.16.2.windows.1



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

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

[Wu, Jiaxin]

SmmRelocationLib instance will reserve the memory from
gEfiSmmSmramMemoryGuid for the smbase relocation.

So, system must produce the gEfiSmmSmramMemoryGuid.

This patch is for the OVMF to produce the
gEfiSmmSmramMemoryGuid HOB.

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 (
+&gEfiSmmSmramMemoryGuid,
+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
-// 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.
+// Create second SMRAM descriptor, which is free and will be used by SMM 
foundation.
 //
-QemuInitializeRamBelow1gb (PlatformI

[edk2-devel] [PATCH v1 07/13] OvmfPkg: Include SmmRelocationLib in OvmfPkg

[Wu, Jiaxin]

This patch just includes SmmRelocationLib in OvmfPkg.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
Signed-off-by: Jiaxin Wu 
---
 OvmfPkg/AmdSev/AmdSevX64.dsc   | 1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
 OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
 OvmfPkg/OvmfPkgIa32.dsc| 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgX64.dsc | 1 +
 6 files changed, 6 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 60e916b4fd..e171f4e89f 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -146,10 +146,11 @@
   
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
   MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
   MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
   CpuPageTableLib|UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
+  SmmRelocationLib|UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
   UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
   
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
   
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
   
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index b522fa1059..5d7ec9bbfa 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -163,10 +163,11 @@
   
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
   MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
   MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
   CpuPageTableLib|UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
+  SmmRelocationLib|UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
   UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
   
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
   
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
   
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 2c6bb83beb..a8b12a49a3 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -166,10 +166,11 @@
   
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
   MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
   MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
   CpuPageTableLib|UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
+  SmmRelocationLib|UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
   UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
   
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
   
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
   
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 713f08764b..03c35a1a72 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -166,10 +166,11 @@
   
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
   MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
   MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
   CpuPageTableLib|UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
+  SmmRelocationLib|UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
   UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
   
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
   
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
   
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 90b15dc270..90a0c08d6d 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -171,10 +171,11 @@
   
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCod

[edk2-devel] [PATCH v1 06/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 
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc 
b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 433fb51a56..328b7fd4fe 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -313,10 +313,12 @@
   
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
   
VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
   CcExitLib|UefiCpuPkg/Library/CcExitLibNull/CcExitLibNull.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 (#117594): https://edk2.groups.io/g/devel/message/117594
Mute This Topic: https://groups.io/mt/105441995/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 05/13] UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib in UefiCpuPkg

[Wu, Jiaxin]

This patch just includes SmmRelocationLib in UefiCpuPkg.

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 UefiCpuPkg/UefiCpuPkg.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index 10b33594e5..631a850c78 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -194,10 +194,13 @@
   
UnitTestResultReportLib|UnitTestFrameworkPkg/Library/UnitTestResultReportLib/UnitTestResultReportLibConOut.inf
   }
   UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
   UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf
   UefiCpuPkg/Library/SmmCpuFeaturesLib/AmdSmmCpuFeaturesLib.inf
+  UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.inf
+  UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
+  UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
 
 [Components.X64]
   
UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHandlerLibUnitTest.inf
 
 [Components.RISCV64]
-- 
2.16.2.windows.1



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

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

[Wu, Jiaxin]

This patch separates the smbase relocation logic from
PiSmmCpuDxeSmm driver, and moves to the
SmmRelocationInit interface.

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

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../Library/SmmRelocationLib/Ia32/Semaphore.c  |  43 ++
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 157 +
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 141 +
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 659 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/SmramSaveStateConfig.c|  91 +++
 .../Library/SmmRelocationLib/X64/Semaphore.c   |  70 +++
 .../Library/SmmRelocationLib/X64/SmmInit.nasm  | 207 +++
 8 files changed, 1429 insertions(+)
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
 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
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/X64/Semaphore.c
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/X64/SmmInit.nasm

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
new file mode 100644
index 00..ace3221cfc
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/Semaphore.c
@@ -0,0 +1,43 @@
+/** @file
+  Semaphore mechanism to indicate to the BSP that an AP has exited SMM
+  after SMBASE relocation.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalSmmRelocationLib.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)&SmmRelocationSemaphoreComplete,
+   
(UINT64)(UINTN)&SmmRelocationSemaphoreComplete
+   );
+}
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm 
b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
new file mode 100644
index 00..cb8b030693
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
@@ -0,0 +1,157 @@
+;--
 ;
+; Copyright (c) 2024, Intel Corporation. All rights reserved.
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+;   SmmInit.nasm
+;
+; Abstract:
+;
+;   Functions for relocating SMBASE's for all processors
+;
+;---
+
+%include "StuffRsbNasm.inc"
+
+global  ASM_PFX(gcSmiIdtr)
+global  ASM_PFX(gcSmiGdtr)
+
+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(gPatchSmmInitStack)
+global ASM_PFX(gcSmmInitSize)
+global ASM_PFX(gcSmmInitTemplate)
+
+%define PROTECT_MODE_CS 0x8
+%define PROTECT_MODE_DS 0x20
+
+SECTION .data
+
+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 

[edk2-devel] [PATCH v1 04/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 
---
 .../SmmRelocationLib/AmdSmmRelocationLib.inf   |  61 
 .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 109 +
 2 files changed, 170 insertions(+)
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf 
b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
new file mode 100644
index 00..710cd1948b
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
@@ -0,0 +1,61 @@
+## @file
+# SMM Relocation Lib for each processor.
+#
+# This Lib produces the SMM_BASE_HOB in HOB database which tells
+# the PiSmmCpuDxeSmm driver (runs at a later phase) about the new
+# SMBASE for each processor. PiSmmCpuDxeSmm driver installs the
+# SMI handler at the SMM_BASE_HOB.SmBase[Index]+0x8000 for processor
+# Index.
+#
+# Copyright (c) 2024, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = SmmRelocationLib
+  FILE_GUID  = 65C74DCD-0D09-494A-8BFF-A64226EB8054
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = SmmRelocationLib
+
+[Sources]
+  InternalSmmRelocationLib.h
+  AmdSmramSaveStateConfig.c
+  SmmRelocationLib.c
+
+[Sources.Ia32]
+  Ia32/Semaphore.c
+  Ia32/SmmInit.nasm
+
+[Sources.X64]
+  X64/Semaphore.c
+  X64/SmmInit.nasm
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  CpuExceptionHandlerLib
+  DebugLib
+  HobLib
+  LocalApicLib
+  MemoryAllocationLib
+  PcdLib
+  PeiServicesLib
+
+[Guids]
+  gSmmBaseHobGuid   ## HOB ALWAYS_PRODUCED
+  gEfiSmmSmramMemoryGuid## CONSUMES
+
+[Pcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize ## CONSUMES
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport## 
CONSUMES
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c 
b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
new file mode 100644
index 00..fbcf347f9b
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
@@ -0,0 +1,109 @@
+/** @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 
+
+/**
+  This function configures the SmBase on the currently executing CPU.
+
+  @param[in] CpuIndex The index of the CPU.
+  @param[in,out] CpuState Pointer to SMRAM Save State Map for the
+  currently executing CPU. On out, SmBase 
is
+  updated to the new value.
+
+**/
+VOID
+EFIAPI
+ConfigureSmBase (
+  IN UINTN CpuIndex,
+  IN OUT SMRAM_SAVE_STATE_MAP  *CpuState
+  )
+{
+  AMD_SMRAM_SAVE_STATE_MAP  *AmdCpuState;
+
+  AmdCpuState = (AMD_SMRAM_SAVE_STATE_MAP *)CpuState;
+
+  if (mSmmSaveStateRegisterLma == EFI_MM_SAVE_STATE_REGISTER_LMA_32BIT) {
+AmdCpuState->x86.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  } else {
+AmdCpuState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  }
+}
+
+/**
+  This function updates the SMRAM save state on the currently executing CPU
+  to resume execution at a specific address after an RSM instruction.  This
+  function must evaluate the SMRAM save state to determine the execution mode
+  the RSM instruction resumes and update the resume execution address with
+  either NewInstructionPointer32 or NewInstructionPoint.  The auto HALT restart
+  flag in the SMRAM save state must always be cleared.  This function returns
+  the value of the instruction pointer from the SMRAM save state that was
+  replaced.  If this function returns 0, then the SMRAM save state was not
+  modified.
+
+  This function is called during the very first SMI on each CPU after
+  SmmCpuFeaturesInitializeProcessor() to set a flag in normal execution mode
+  to signal that the SMBASE of each CPU has been updated before the default
+  SMBASE address is used for the first SMI to the next CPU.
+
+  @param[i

[edk2-devel] [PATCH v1 03/13] UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF

[Wu, Jiaxin]

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

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

Cc: Ray Ni 
Cc: Zeng Star 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 .../SmmRelocationLib/OvmfSmmRelocationLib.inf  |  61 
 .../SmmRelocationLib/OvmfSmramSaveStateConfig.c| 107 +
 2 files changed, 168 insertions(+)
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf 
b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
new file mode 100644
index 00..eba1129ac2
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
@@ -0,0 +1,61 @@
+## @file
+# SMM Relocation Lib for each processor.
+#
+# This Lib produces the SMM_BASE_HOB in HOB database which tells
+# the PiSmmCpuDxeSmm driver (runs at a later phase) about the new
+# SMBASE for each processor. PiSmmCpuDxeSmm driver installs the
+# SMI handler at the SMM_BASE_HOB.SmBase[Index]+0x8000 for processor
+# Index.
+#
+# Copyright (c) 2024, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = SmmRelocationLib
+  FILE_GUID  = 51834F51-CCE0-4743-B553-935D0C8A53FF
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = SmmRelocationLib
+
+[Sources]
+  InternalSmmRelocationLib.h
+  OvmfSmramSaveStateConfig.c
+  SmmRelocationLib.c
+
+[Sources.Ia32]
+  Ia32/Semaphore.c
+  Ia32/SmmInit.nasm
+
+[Sources.X64]
+  X64/Semaphore.c
+  X64/SmmInit.nasm
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  CpuExceptionHandlerLib
+  DebugLib
+  HobLib
+  LocalApicLib
+  MemoryAllocationLib
+  PcdLib
+  PeiServicesLib
+
+[Guids]
+  gSmmBaseHobGuid   ## HOB ALWAYS_PRODUCED
+  gEfiSmmSmramMemoryGuid## CONSUMES
+
+[Pcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize ## CONSUMES
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport## 
CONSUMES
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c 
b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c
new file mode 100644
index 00..505b1d694a
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c
@@ -0,0 +1,107 @@
+/** @file
+  Config SMRAM Save State for SmmBases Relocation.
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include "InternalSmmRelocationLib.h"
+#include 
+
+/**
+  This function configures the SmBase on the currently executing CPU.
+
+  @param[in] CpuIndex The index of the CPU.
+  @param[in,out] CpuState Pointer to SMRAM Save State Map for the
+  currently executing CPU. On out, SmBase 
is
+  updated to the new value.
+
+**/
+VOID
+EFIAPI
+ConfigureSmBase (
+  IN UINTN CpuIndex,
+  IN OUT SMRAM_SAVE_STATE_MAP  *CpuState
+  )
+{
+  AMD_SMRAM_SAVE_STATE_MAP  *CpuSaveState;
+
+  CpuSaveState = (AMD_SMRAM_SAVE_STATE_MAP *)CpuState;
+
+  if ((CpuSaveState->x86.SMMRevId & 0x) == 0) {
+CpuSaveState->x86.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  } else {
+CpuSaveState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  }
+}
+
+/**
+  This function updates the SMRAM save state on the currently executing CPU
+  to resume execution at a specific address after an RSM instruction.  This
+  function must evaluate the SMRAM save state to determine the execution mode
+  the RSM instruction resumes and update the resume execution address with
+  either NewInstructionPointer32 or NewInstructionPoint.  The auto HALT restart
+  flag in the SMRAM save state must always be cleared.  This function returns
+  the value of the instruction pointer from the SMRAM save state that was
+  replaced.  If this function returns 0, then the SMRAM save state was not
+  modified.
+
+  This function is called during the very first SMI on each CPU after
+  SmmCpuFeaturesInitializeProcessor() to set a flag in normal execution mode
+  to signal that the SMBASE of each CPU has been updated before the default
+  SMBASE address is used for the first SMI to the next CPU.
+
+  @param[in] CpuIndex The processor index for the currently
+ 

[edk2-devel] [PATCH v1 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 
---
 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..13043641a2
--- /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) 2023, 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 571b59b36f..1d7890f19e 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -65,10 +65,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 (#117589): https://edk2.groups.io/g/devel/message/117589
Mute This Topic: https://groups.io/mt/105441988/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

[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) 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 
Signed-off-by: Jiaxin Wu 

Jiaxin Wu (13):
  UefiCpuPkg: Add SmmRelocationLib class
  UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance
  UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF
  UefiCpuPkg/SmmRelocationLib: Add library instance for AMD
  UefiCpuPkg/UefiCpuPkg.dsc: Include SmmRelocationLib in UefiCpuPkg
  UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib
  OvmfPkg: Include SmmRelocationLib in OvmfPkg
  OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid
  OvmfPkg/SmmAccess: Consume gEfiSmmSmramMemoryGuid
  OvmfPkg/PlatformInitLib: Create gEfiAcpiVariableGuid
  OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not
  OvmfPkg/PlatformPei: Relocate SmBases in PEI phase
  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 +-
 OvmfPkg/Microvm/MicrovmX64.dsc |   1 +
 OvmfPkg/OvmfPkgIa32.dsc|   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
 OvmfPkg/OvmfPkgX64.dsc |   1 +
 OvmfPkg/PlatformPei/Platform.c |   1 +
 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   |  61 ++
 .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 109 
 .../SmmRelocationLib}/Ia32/Semaphore.c |  13 +-
 .../Library/SmmRelocationLib/Ia32/SmmInit.nasm | 157 +
 .../SmmRelocationLib/InternalSmmRelocationLib.h| 141 +
 .../SmmRelocationLib/OvmfSmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/OvmfSmramSaveStateConfig.c| 107 
 .../Library/SmmRelocationLib/SmmRelocationLib.c| 659 +
 .../Library/SmmRelocationLib/SmmRelocationLib.inf  |  61 ++
 .../SmmRelocationLib/SmramSaveStateConfig.c|  91 +++
 .../SmmRelocationLib}/X64/Semaphore.c  |  13 +-
 .../SmmRelocationLib}/X64/SmmInit.nasm |  93 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c  |  21 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm|  96 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |   6 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 322 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  98 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   4 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c |  69 ---
 UefiCpuPkg/UefiCpuPkg.dec  |   3 +
 UefiCpuPkg/UefiCpuPkg.dsc  |   3 +
 UefiPayloadPkg/UefiPayloadPkg.dsc  |   2 +
 42 files changed, 1850 insertions(+), 817 deletions(-)
 create mode 100644 OvmfPkg/PlatformPei/SmmRelocation.c
 create mode 100644 UefiCpuPkg/Include/Library/SmmRelocationLib.h
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
 rename UefiCpuPkg/{PiSmmCpuDxeSmm => 
Library/SmmRelocationLib}/Ia32/Semaphore.c (72%)
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/Ia32/SmmInit.nasm
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/InternalSmmRelocationLib.h
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/OvmfSmmRelocationLib.inf
 create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/OvmfSmramSaveStateConfig.c
 create mode 100644 UefiCpuPkg/Library/SmmRelocationLib

Re: [edk2-devel] [PATCH] CryptoPkg: BaseCryptLib: ASN1_get_object() function return value is not checked properly in CryptX509.c.

[Sountharya N via groups.io]

Hi Yi,

I Removed 'Reviewed-by: @yi1...@intel.com' and Sent V2 patch to 
devel@edk2.groups.io.

Thanks,
Sountharya

-Original Message-
From: Li, Yi1 
Sent: Tuesday, April 2, 2024 6:42 AM
To: Sountharya N ; devel@edk2.groups.io
Cc: Srinivasan Mani ; Prarthana Sagar V 
; Yao, Jiewen ; Hou, Wenxing 

Subject: RE: [EXTERNAL] RE: [PATCH] CryptoPkg: BaseCryptLib: ASN1_get_object() 
function return value is not checked properly in CryptX509.c.

Hi,

Please remove 'Reviewed-by: @yi1...@intel.com' and send V2 patch to 
devel@edk2.groups.io by send-email.

Generate V2 patch:
Add '--subject-prefix="PATCH V2" ' when you format patch:
git format-patch --subject-prefix="PATCH V2" ...

The content of patch looks good to me, will give you my Reviewed-by and push it 
after receiving your V2 patch.

Thanks,
Yi

-Original Message-
From: Sountharya N 
Sent: Monday, April 1, 2024 7:23 PM
To: Li, Yi1 ; devel@edk2.groups.io
Cc: MANI, SRINIVASAN ; Prarthana Sagar V 
; Yao, Jiewen ; Hou, Wenxing 

Subject: RE: [EXTERNAL] RE: [PATCH] CryptoPkg: BaseCryptLib: ASN1_get_object() 
function return value is not checked properly in CryptX509.c.

Hi Yi,

As you suggested, modified the changes and gave PR. Here I have attached the 
Link for reference.
https://github.com/tianocore/edk2/pull/5507

Thanks,
Sountharya

-Original Message-
From: Li, Yi1 
Sent: Thursday, March 28, 2024 7:17 AM
To: devel@edk2.groups.io; Sountharya N 
Cc: Srinivasan Mani ; Prarthana Sagar V 
; Yao, Jiewen ; Hou, Wenxing 

Subject: [EXTERNAL] RE: [PATCH] CryptoPkg: BaseCryptLib: ASN1_get_object() 
function return value is not checked properly in CryptX509.c.


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

Hi,

Please follow the correct upstream process to avoid Github CI errors.
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers#contributor-workflow
You are missing your Sign-off and Cc maintainers in this patch, And please add 
BZ link to commit message: ''REF: 
https://bugzilla.tianocore.org/show_bug.cgi?id=4509''.

For your code change, '''ASN1_get_object() return error''' or  
''ASN1_get_object() success but Asn1Tag != V_ASN1_SEQUENCE ''' both need to 
be treated as errors and returned.
So you should use logic or instead of and.

Before you send V2 patch, please create a PR in EDK2 github to test CI: 
https://github.com/tianocore/edk2/pulls

Regards,
Yi

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Sountharya N via 
groups.io
Sent: Tuesday, March 19, 2024 1:44 PM
To: devel@edk2.groups.io; Sountharya N 
Cc: MANI, SRINIVASAN ; Prarthana Sagar V 

Subject: [edk2-devel] [PATCH] CryptoPkg: BaseCryptLib: ASN1_get_object() 
function return value is not checked properly in CryptX509.c.

Added Inf variable, and the error case returned value was checked properly.
---
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 1182323b63..ac05441383 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -839,17 +839,17 @@ X509GetTBSCert (
   Length = 0;

   Inf= ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int 
*)&ObjClass, (long)CertSize);



-  if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {

+  if (((Inf & 0x80) == 0x80) && (Asn1Tag != V_ASN1_SEQUENCE)) {

 return FALSE;

   }



   *TBSCert = (UINT8 *)Temp;



-  ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int *)&ObjClass, 
(long)Length);

+  Inf= ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int 
*)&ObjClass, (long)Length);

   //

   // Verify the parsed TBSCertificate is one correct SEQUENCE data.

   //

-  if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {

+  if (((Inf & 0x80) == 0x80) && (Asn1Tag != V_ASN1_SEQUENCE)) {

 return FALSE;

   }



--
2.35.1.windows.2
-The information contained in this message may be confidential and proprietary 
to American Megatrends (AMI). This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited. Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.





-The information contained in this message may be confidential and proprietary 
to American Megatrends (AMI). This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on noti

[edk2-devel] [PATCH] PATCH_V2

[Sountharya N via groups.io]

---
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 1182323b63..021cc328f8 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -839,17 +839,17 @@ X509GetTBSCert (
   Length = 0;

   Inf= ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int 
*)&ObjClass, (long)CertSize);



-  if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {

+  if (((Inf & 0x80) == 0x80) || (Asn1Tag != V_ASN1_SEQUENCE)) {

 return FALSE;

   }



   *TBSCert = (UINT8 *)Temp;



-  ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int *)&ObjClass, 
(long)Length);

+  Inf = ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int 
*)&ObjClass, (long)Length);

   //

   // Verify the parsed TBSCertificate is one correct SEQUENCE data.

   //

-  if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {

+  if (((Inf & 0x80) == 0x80) || (Asn1Tag != V_ASN1_SEQUENCE)) {

 return FALSE;

   }



--
2.35.1.windows.2
-The information contained in this message may be confidential and proprietary 
to American Megatrends (AMI). This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited. Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.


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

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

[Lewandowski, Krzysztof]

Looks good to me.

Reviewed-by: Krzysztof Lewandowski 

From: Cai, Xianglei 
Sent: Wednesday, April 10, 2024 09:01
To: devel@edk2.groups.io 
Cc: Cai, Xianglei ; Ni, Ray ; Liming 
Gao ; Lewandowski, Krzysztof 
; Huang, Jenny ; Shih, 
More ; Chiu, Ian 
Subject: [PATCH 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 
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 1 +
 MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf | 4 
 MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 2 +-
 MdePkg/MdePkg.dec| 4 
 MdePkg/MdePkg.dsc| 1 +
 5 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..2ee4c22d1e32 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]
+  gEfiMdePkgTokenSpaceGuid.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/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 0459418906f8..a5b3ce94072c 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2510,5 +2510,9 @@
   # @Prompt Memory encryption attribute
   
gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0|UINT64|0x002e

+  ## This dynamic PCD indicates time delay for XHCI registers access after it 
issues HCRST.
+  # @Prompt Delay access XHCI register after it issues HCRST (us)
+  gEfiMdePkgTokenSpaceGuid.PcdDelayXhciHCReset|2000|UINT16|0x0043
+
 [UserExtensions.TianoCore."ExtraFiles"]
   MdePkgExtra.uni
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 109224c527f7..23aea04fea2e 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -31,6 +31,7 @@
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x0f
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE000
+  gEfiMdePkgTokenSpaceGuid.PcdDelayXhciHCReset|2000

 [LibraryClasses]
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
--
2.42.0.windows.2

-
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z 
dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach 
handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by others 
is strictly prohibited.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messag

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg: OVMF supports USB mouses

[Gerd Hoffmann]

On Tue, Apr 09, 2024 at 04:51:20PM +0100, Pedro Falcato wrote:
> On Tue, Apr 9, 2024 at 12:56 PM Gerd Hoffmann  wrote:
> >
> > On Mon, Apr 08, 2024 at 08:53:10AM +0100, Phillip Tennen wrote:
> > > Hi, thank you for taking a look at the patch!
> > >
> > > This patch can be verified to be working with this app (which was the
> > > motivation for submitting this):
> > > https://github.com/codyd51/uefirc/releases/tag/1.0.1.
> >
> > Quoting https://github.com/codyd51/uefirc:
> >
> > Q: Should I use this?
> > A: This should not exist.
> >
> > Well.  This certainly one of the more interesting ways to have some fun
> > and improve your rust coding skills.  But a justification to include a
> > mouse driver by default which is not used by anything else?  IMHO it
> > isn't.
> 
> Maybe some better reasons:
> 
> 1) It has been conspicuously missing from OVMF. I've heard N questions
> over the years (on the #osdev IRC, etc) regarding their mouse code not
> working on OVMF, whereas you'd see that protocol in other normal
> platforms

Those other platforms often have a funky setup application which (unlike
UiApp) actually support using the mouse.

So, I'm still wondering what applications people want use the mouse for?

> For sure, UsbMouseDxe isn't #1 on my most desired EFI modules list
> (e.g I'd love to eventually be able to consume Ext4Dxe from OVMF,
> where it'd actually be useful, if I can ever ditch edk2-platforms),
> but I don't really see the harm in doing it.

UsbMouseDxe is IMHO the least useful driver.

We have:
  - Ps2MouseDxe, exposing EFI_SIMPLE_POINTER_PROTOCOL, and
  - UsbMouseDxe, exposing EFI_SIMPLE_POINTER_PROTOCOL too, and
  - UsbMouseAbsolutePointerDxe, exposing EFI_ABSOLUTE_POINTER_PROTOCOL.

On the qemu side a ps/2 mouse is always present.  Working with a
relative pointer device in a virtual machine isn't very smooth though,
which why qemu offers absolute pointer devices as alternative
(usb-tablet, virtio-tablet).

So a typical configuration (on x64, aa64 is a different story) is to
have a ps/2 mouse and an usb tablet.  qemu will start in relative
pointer mode and send events to the mouse.  As soon as the guest
activates the tablet (typically when the linux kernel loads the usb hid
drivers) qemu switches into absolute pointer mode and sends events to
the tablet.

Linux applications don't have to worry about relative vs. absolute
pointer mode.  The display server (x11/wayland) will deal with that for
them, they get the pointer position already translated to screen
coordinates.

That is not the case for efi applications though.  If they want work
with both relative and absolute pointing devices they have to implement
both protocols.

Now the tricky part here is that efi applications implementing only
EFI_SIMPLE_POINTER_PROTOCOL will *not* work in case
UsbMouseAbsolutePointerDxe included in the firmware image.  Loading the
driver will activate absolute pointer mode and qemu will route events
to the tablet not the mouse ...

> There's an argument in giving people a full-fledged UEFI
> implementation of most protocols. OVMF is *the* platform in mainline
> edk2 after all :)

If we do that we IMHO should
 (a) add a config option for that (similar to the rarely used scsi
 drivers),
 (b) update all ovmf variants consistently, and
 (c) include both ps/2 and usb mouse drivers.

Not sure whenever it is a good idea to include
UsbMouseAbsolutePointerDxe.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117584): https://edk2.groups.io/g/devel/message/117584
Mute This Topic: https://groups.io/mt/105365480/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] IntelFsp2Pkg: Optional Plugin for FSP SecCore/PeiCore Rebasing

[Ashraf Ali S]

Reviewed-by: S, Ashraf Ali 
mailto:ashraf.al...@intel.com>>

Thanks.,
S, Ashraf Ali

From: Ni, Ray 
Sent: Wednesday, April 10, 2024 1:27 PM
To: Liu, Zhiguang ; devel@edk2.groups.io
Cc: Chiu, Chasel ; Desimone, Nathaniel L 
; Duggapu, Chinni B 
; Zeng, Star ; Kuo, Ted 
; S, Ashraf Ali ; Susovan Mohapatra 

Subject: Re: [PATCH v2] IntelFsp2Pkg: Optional Plugin for FSP SecCore/PeiCore 
Rebasing

Reviewed-by: Ray Ni mailto:ray...@intel.com>>

Thanks,
Ray

From: Liu, Zhiguang mailto:zhiguang@intel.com>>
Sent: Wednesday, April 10, 2024 15:08
To: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>
Cc: Liu, Zhiguang mailto:zhiguang@intel.com>>; 
Chiu, Chasel mailto:chasel.c...@intel.com>>; Desimone, 
Nathaniel L 
mailto:nathaniel.l.desim...@intel.com>>; 
Duggapu, Chinni B 
mailto:chinni.b.dugg...@intel.com>>; Zeng, Star 
mailto:star.z...@intel.com>>; Kuo, Ted 
mailto:ted@intel.com>>; S, Ashraf Ali 
mailto:ashraf.al...@intel.com>>; Susovan Mohapatra 
mailto:susovan.mohapa...@intel.com>>; Ni, Ray 
mailto:ray...@intel.com>>
Subject: [PATCH v2] IntelFsp2Pkg: Optional Plugin for FSP SecCore/PeiCore 
Rebasing

Note this plugin only applies to 64-bit PSP
This optional plugin is designed to execute before the FSP SecCore to
rebase SecCore and PeiCore during runtime. If the FSP binary requires
rebasing at runtime, this module should be included within the FSP
binary. Additionally, specific patches must be applied to ensure proper
functionality. In the absence of this module, manual patching of API
offsets within the FSP header is necessary. To illustrate, let's
consider a scenario within FSP-S where 'FspSiliconInitEntry' is the
initial API to be executed post-rebase. Rather than directly inputting
the 'FspSiliconInit' offset into the 'FspSiliconInitEntryOffset' field
of the FSP header, the entry point of this module should be used.
Furthermore, the 'FspSiliconInit' offset should be placed into
'AsmGetFspOriginalEntry', which signifies the address to which this
module will jump.
It is also essential to patch the image bases of SecCore and PeiCore
to enable the rebasing functionality of this module.
The following is an example of how to apply the necessary patches:
Patch Address   Patch Value
 PreFspSec:_ModuleEntryPoint - [0x]
PreFspSec:SecCoreRelativeOffPreFspSec:AsmGetFspSecCoreImageBase
 - Fsp24SecCoreS:BASE
PreFspSec:PeiCoreRelativeOffPreFspSec:AsmGetFspPeiCoreImageBase
 - PeiCore:BASE
PreFspSec:SecEntryRelativeOff   PreFspSec:AsmGetFspOriginalEntry
 - Fsp24SecCoreS:FspSiliconInitApi

Cc: Chasel Chiu mailto:chasel.c...@intel.com>>
Cc: Nate DeSimone 
mailto:nathaniel.l.desim...@intel.com>>
Cc: Duggapu Chinni B 
mailto:chinni.b.dugg...@intel.com>>
Cc: Star Zeng mailto:star.z...@intel.com>>
Cc: Ted Kuo mailto:ted@intel.com>>
Cc: Ashraf Ali S mailto:ashraf.al...@intel.com>>
Cc: Susovan Mohapatra 
mailto:susovan.mohapa...@intel.com>>
Cc: Ray Ni mailto:ray...@intel.com>>
Signed-off-by: Zhiguang Liu 
mailto:zhiguang@intel.com>>
---
 IntelFsp2Pkg/IntelFsp2Pkg.dsc |   5 +
 IntelFsp2Pkg/PreFspSec/PreFspSec.c| 115 ++
 IntelFsp2Pkg/PreFspSec/PreFspSec.inf  |  62 ++
 .../PreFspSec/X64/PreFspSecCommon.nasm|  94 ++
 4 files changed, 276 insertions(+)
 create mode 100644 IntelFsp2Pkg/PreFspSec/PreFspSec.c
 create mode 100644 IntelFsp2Pkg/PreFspSec/PreFspSec.inf
 create mode 100644 IntelFsp2Pkg/PreFspSec/X64/PreFspSecCommon.nasm

diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
index f236a7010b..a2cc29c940 100644
--- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
@@ -33,6 +33,8 @@
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
   
DebugDeviceLib|IntelFsp2Pkg/Library/BaseDebugDeviceLibNull/BaseDebugDeviceLibNull.inf
+  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+  
PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf

   # FSP override
   
DebugLib|IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/BaseFspDebugLibSerialPort.inf
@@ -75,6 +77,9 @@
   IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf
   IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf

+[Components.X64]
+  IntelFsp2Pkg/PreFspSec/PreFspSec.inf
+
 [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x1f
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80080046
diff --git a/IntelFsp2Pkg/PreFspSec/PreFspSec.c 
b/IntelFsp2Pkg/PreFspSec/PreFspSec.c
new file mode 100644
index 00..b3b52b8064
--- /dev/null
+++ b/IntelFsp2Pkg/PreFspSec/PreFspSec.c
@@ -0,0 +1,115 @@
+/** @file
+
+  Copyright (c) 2024, Intel Corporation. All rights

Re: [edk2-devel] [edk2-redfish-client][PATCH v2 1/2] RedfishClientPkg/Features: support Redfish Secure Boot

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Ok, then you can just merge this patch set.

thanks
Abner

> -Original Message-
> From: Nickle Wang 
> Sent: Wednesday, April 10, 2024 4:17 PM
> To: devel@edk2.groups.io; Chang, Abner 
> Cc: Igor Kulchytskyy ; Nick Ramirez 
> Subject: RE: [edk2-devel] [edk2-redfish-client][PATCH v2 1/2]
> RedfishClientPkg/Features: support Redfish Secure Boot
>
> [AMD Official Use Only - General]
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Thanks for your review, Abner!
>
> The reason why I keep HPE copyright statement is because I create this feature
> driver by copying existing one and modifying it. Since I still leverage some
> functions contributed by HPE, I think I am supposed to keep HPE copyright
> statement.
>
> Regards,
> Nickle
>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Chang,
> Abner
> > via groups.io
> > Sent: Wednesday, April 10, 2024 9:43 AM
> > To: Nickle Wang ; devel@edk2.groups.io
> > Cc: Igor Kulchytskyy ; Nick Ramirez 
> > Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH v2 1/2]
> > RedfishClientPkg/Features: support Redfish Secure Boot
> >
> > External email: Use caution opening links or attachments
> >
> >
> > [AMD Official Use Only - General]
> >
> > Hi Nickle,
> > I don't see the obvious coding error in this patch. However, I think we can
> remove
> > HPE copyright from Redfish secure boot feature driver, as this is a new 
> > feature
> > driver and HPE doesn't have contributions to these files, right? If so, then
> please
> > remove it.
> >
> > Thanks
> >
> > Reviewed-by: Abner Chang 
> >
> > > -Original Message-
> > > From: Nickle Wang 
> > > Sent: Monday, April 1, 2024 10:18 PM
> > > To: devel@edk2.groups.io
> > > Cc: Chang, Abner ; Igor Kulchytskyy
> > > ; Nick Ramirez 
> > > Subject: [edk2-redfish-client][PATCH v2 1/2] RedfishClientPkg/Features:
> > > support Redfish Secure Boot
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > Introduce SecureBoot driver to support
> > > /redfish/v1/Systems/SYS/SecureBoot resource.
> > >
> > > Signed-off-by: Nickle Wang 
> > > Cc: Abner Chang 
> > > Cc: Igor Kulchytskyy 
> > > Cc: Nick Ramirez 
> > > ---
> > >  .../RedfishClientComponents.dsc.inc   |   2 +
> > >  RedfishClientPkg/RedfishClientLibs.dsc.inc|   4 +
> > >  .../SecureBoot/v1_1_0/Dxe/SecureBootDxe.inf   |  60 ++
> > >  .../v1_1_0/Common/SecureBootCommon.h  |  40 +
> > >  .../v1_1_0/Common/SecureBootCommon.c  | 765
> +
> > >  .../SecureBoot/v1_1_0/Dxe/SecureBootDxe.c | 809
> ++
> > >  RedfishClientPkg/RedfishClient.fdf.inc|   1 +
> > >  7 files changed, 1681 insertions(+)
> > >  create mode 100644
> > > RedfishClientPkg/Features/SecureBoot/v1_1_0/Dxe/SecureBootDxe.inf
> > >  create mode 100644
> > > RedfishClientPkg/Features/SecureBoot/v1_1_0/Common/SecureBootCommo
> > > n.h
> > >  create mode 100644
> > > RedfishClientPkg/Features/SecureBoot/v1_1_0/Common/SecureBootCommo
> > > n.c
> > >  create mode 100644
> > > RedfishClientPkg/Features/SecureBoot/v1_1_0/Dxe/SecureBootDxe.c
> > >
> > > diff --git a/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > > b/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > > index ae2a4b025..42fc0c299 100644
> > > --- a/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > > +++ b/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > > @@ -34,6 +34,7 @@
> > >RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.inf
> > >
> > >
> RedfishClientPkg/Features/BootOptionCollection/BootOptionCollectionDxe.in
> > > f
> > >RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.inf
> > > +  RedfishClientPkg/Features/SecureBoot/v1_1_0/Dxe/SecureBootDxe.inf
> > >
> > >!include RedfishClientPkg/RedfishJsonStructureDxe.dsc.inc
> > >
> > > @@ -47,3 +48,4 @@
> > >RedfishClientPkg/Converter/Bios/v1_0_9/RedfishBios_V1_0_9_Dxe.inf
> > >
> > >
> RedfishClientPkg/Converter/BootOptionCollection/RedfishBootOptionCollecti
> > > on_Dxe.inf
> > >
> > >
> RedfishClientPkg/Converter/BootOption/v1_0_4/RedfishBootOption_V1_0_4_
> > > Dxe.inf
> > > +
> > >
> RedfishClientPkg/Converter/SecureBoot/v1_1_0/RedfishSecureBoot_V1_1_0_
> > > Dxe.inf
> > > diff --git a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > > b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > > index 6599926ab..9126465df 100644
> > > --- a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > > +++ b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > > @@ -25,6 +25,8 @@
> > >
> > >
> BiosV1_0_9Lib|RedfishClientPkg/ConverterLib/edk2library/Bios/v1_0_9/Lib.in
> > > f
> > >
> > >
> BootOptionCollectionLib|RedfishClientPkg/ConverterLib/edk2library/BootOp
> > > tionCollection/Lib.inf
> > >
> > >
> BootOptionV1_0_4Lib|RedfishClientPkg/ConverterLib/edk2lib

Re: [edk2-devel] [edk2-redfish-client][PATCH v2 1/2] RedfishClientPkg/Features: support Redfish Secure Boot

[Nickle Wang via groups.io]

Thanks for your review, Abner!

The reason why I keep HPE copyright statement is because I create this feature 
driver by copying existing one and modifying it. Since I still leverage some 
functions contributed by HPE, I think I am supposed to keep HPE copyright 
statement.

Regards,
Nickle

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chang, Abner
> via groups.io
> Sent: Wednesday, April 10, 2024 9:43 AM
> To: Nickle Wang ; devel@edk2.groups.io
> Cc: Igor Kulchytskyy ; Nick Ramirez 
> Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH v2 1/2]
> RedfishClientPkg/Features: support Redfish Secure Boot
> 
> External email: Use caution opening links or attachments
> 
> 
> [AMD Official Use Only - General]
> 
> Hi Nickle,
> I don't see the obvious coding error in this patch. However, I think we can 
> remove
> HPE copyright from Redfish secure boot feature driver, as this is a new 
> feature
> driver and HPE doesn't have contributions to these files, right? If so, then 
> please
> remove it.
> 
> Thanks
> 
> Reviewed-by: Abner Chang 
> 
> > -Original Message-
> > From: Nickle Wang 
> > Sent: Monday, April 1, 2024 10:18 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner ; Igor Kulchytskyy
> > ; Nick Ramirez 
> > Subject: [edk2-redfish-client][PATCH v2 1/2] RedfishClientPkg/Features:
> > support Redfish Secure Boot
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Introduce SecureBoot driver to support
> > /redfish/v1/Systems/SYS/SecureBoot resource.
> >
> > Signed-off-by: Nickle Wang 
> > Cc: Abner Chang 
> > Cc: Igor Kulchytskyy 
> > Cc: Nick Ramirez 
> > ---
> >  .../RedfishClientComponents.dsc.inc   |   2 +
> >  RedfishClientPkg/RedfishClientLibs.dsc.inc|   4 +
> >  .../SecureBoot/v1_1_0/Dxe/SecureBootDxe.inf   |  60 ++
> >  .../v1_1_0/Common/SecureBootCommon.h  |  40 +
> >  .../v1_1_0/Common/SecureBootCommon.c  | 765 +
> >  .../SecureBoot/v1_1_0/Dxe/SecureBootDxe.c | 809 ++
> >  RedfishClientPkg/RedfishClient.fdf.inc|   1 +
> >  7 files changed, 1681 insertions(+)
> >  create mode 100644
> > RedfishClientPkg/Features/SecureBoot/v1_1_0/Dxe/SecureBootDxe.inf
> >  create mode 100644
> > RedfishClientPkg/Features/SecureBoot/v1_1_0/Common/SecureBootCommo
> > n.h
> >  create mode 100644
> > RedfishClientPkg/Features/SecureBoot/v1_1_0/Common/SecureBootCommo
> > n.c
> >  create mode 100644
> > RedfishClientPkg/Features/SecureBoot/v1_1_0/Dxe/SecureBootDxe.c
> >
> > diff --git a/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > b/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > index ae2a4b025..42fc0c299 100644
> > --- a/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > +++ b/RedfishClientPkg/RedfishClientComponents.dsc.inc
> > @@ -34,6 +34,7 @@
> >RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.inf
> >
> > RedfishClientPkg/Features/BootOptionCollection/BootOptionCollectionDxe.in
> > f
> >RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.inf
> > +  RedfishClientPkg/Features/SecureBoot/v1_1_0/Dxe/SecureBootDxe.inf
> >
> >!include RedfishClientPkg/RedfishJsonStructureDxe.dsc.inc
> >
> > @@ -47,3 +48,4 @@
> >RedfishClientPkg/Converter/Bios/v1_0_9/RedfishBios_V1_0_9_Dxe.inf
> >
> > RedfishClientPkg/Converter/BootOptionCollection/RedfishBootOptionCollecti
> > on_Dxe.inf
> >
> > RedfishClientPkg/Converter/BootOption/v1_0_4/RedfishBootOption_V1_0_4_
> > Dxe.inf
> > +
> > RedfishClientPkg/Converter/SecureBoot/v1_1_0/RedfishSecureBoot_V1_1_0_
> > Dxe.inf
> > diff --git a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > index 6599926ab..9126465df 100644
> > --- a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > +++ b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > @@ -25,6 +25,8 @@
> >
> > BiosV1_0_9Lib|RedfishClientPkg/ConverterLib/edk2library/Bios/v1_0_9/Lib.in
> > f
> >
> > BootOptionCollectionLib|RedfishClientPkg/ConverterLib/edk2library/BootOp
> > tionCollection/Lib.inf
> >
> > BootOptionV1_0_4Lib|RedfishClientPkg/ConverterLib/edk2library/BootOptio
> > n/v1_0_4/Lib.inf
> > +
> > SecureBootV1_1_0Lib|RedfishClientPkg/ConverterLib/edk2library/SecureBoot
> > /v1_1_0/Lib.inf
> > +
> >#
> ># Above modules should be pulled in by build tool.
> >#
> > @@ -42,3 +44,5 @@
> >
> > RedfishAddendumLib|RedfishClientPkg/Library/RedfishAddendumLib/Redfis
> > hAddendumLib.inf
> >RedfishDebugLib|RedfishPkg/Library/RedfishDebugLib/RedfishDebugLib.inf
> >RedfishHttpLib|RedfishPkg/Library/RedfishHttpLib/RedfishHttpLib.inf
> > +
> > SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBo
> > otVariableLib.inf
> > +
> > PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolic
> > y/PlatformPKProtectionLibVarPolicy.inf
> > diff --git
> > a/RedfishClientPkg/Features/SecureBoot/v1_1_0/Dxe/SecureB

Re: [edk2-devel] [PATCH v3 0/6] Support to unregister SMI handler in SMI handlers

[Ni, Ray]

Reviewed-by: Ray Ni 

Thanks,
Ray

From: devel@edk2.groups.io  on behalf of Zhiguang Liu 

Sent: Wednesday, April 10, 2024 15:49
To: devel@edk2.groups.io 
Cc: Liu, Zhiguang 
Subject: [edk2-devel] [PATCH v3 0/6] Support to unregister SMI handler in SMI 
handlers

Months ago, I sent patch set to fix potential issues in the usage of SMI 
handler unregistering SMI handler. Discussion can be found in below
link:
https://edk2.groups.io/g/devel/topic/103925794#114251

The conclusion was to only support SMI handler unregistering itself, and not 
allow  SMI handler unregistering other handlers, because we thought there would 
be no such usage.
However, recently, I find in some platform, there is kind of usage.
To also support SMI handler unregistering other handlers, this patch set use a 
totally different design. So I revert the former ones first to make the code 
more readable.

Thank Laszlo for bring up the initial idea for the new design.

V2&V3:
Code refine based on Ray's comments

Zhiguang Liu (6):
  Revert 2ec8f0c6407f062441b205b900038933865c7b3c
  Revert 049ff6c39c73edd3709c05bd0e46184320471358
  Revert 17b28722008eab745ce186b72cd325944cbe6bf0
  Revert ae1079b386a597108a8070652bf7cdaa4ec3dda3
  MdeModulePkg/SMM: Support to unregister SMI handler in SMI handlers
  StandaloneMmPkg: Support to unregister MMI handler in MMI handlers

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h |   1 +
 MdeModulePkg/Core/PiSmmCore/Smi.c   | 198 ---
 StandaloneMmPkg/Core/Mmi.c  | 200 +---
 3 files changed, 292 insertions(+), 107 deletions(-)

--
2.31.1.windows.1








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117580): https://edk2.groups.io/g/devel/message/117580
Mute This Topic: https://groups.io/mt/105437953/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] IntelFsp2Pkg: Optional Plugin for FSP SecCore/PeiCore Rebasing

[Ni, Ray]

Reviewed-by: Ray Ni 

Thanks,
Ray

From: Liu, Zhiguang 
Sent: Wednesday, April 10, 2024 15:08
To: devel@edk2.groups.io 
Cc: Liu, Zhiguang ; Chiu, Chasel 
; Desimone, Nathaniel L 
; Duggapu, Chinni B 
; Zeng, Star ; Kuo, Ted 
; S, Ashraf Ali ; Susovan Mohapatra 
; Ni, Ray 
Subject: [PATCH v2] IntelFsp2Pkg: Optional Plugin for FSP SecCore/PeiCore 
Rebasing

Note this plugin only applies to 64-bit PSP
This optional plugin is designed to execute before the FSP SecCore to
rebase SecCore and PeiCore during runtime. If the FSP binary requires
rebasing at runtime, this module should be included within the FSP
binary. Additionally, specific patches must be applied to ensure proper
functionality. In the absence of this module, manual patching of API
offsets within the FSP header is necessary. To illustrate, let's
consider a scenario within FSP-S where 'FspSiliconInitEntry' is the
initial API to be executed post-rebase. Rather than directly inputting
the 'FspSiliconInit' offset into the 'FspSiliconInitEntryOffset' field
of the FSP header, the entry point of this module should be used.
Furthermore, the 'FspSiliconInit' offset should be placed into
'AsmGetFspOriginalEntry', which signifies the address to which this
module will jump.
It is also essential to patch the image bases of SecCore and PeiCore
to enable the rebasing functionality of this module.
The following is an example of how to apply the necessary patches:
Patch Address   Patch Value
 PreFspSec:_ModuleEntryPoint - [0x]
PreFspSec:SecCoreRelativeOffPreFspSec:AsmGetFspSecCoreImageBase
 - Fsp24SecCoreS:BASE
PreFspSec:PeiCoreRelativeOffPreFspSec:AsmGetFspPeiCoreImageBase
 - PeiCore:BASE
PreFspSec:SecEntryRelativeOff   PreFspSec:AsmGetFspOriginalEntry
 - Fsp24SecCoreS:FspSiliconInitApi

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Duggapu Chinni B 
Cc: Star Zeng 
Cc: Ted Kuo 
Cc: Ashraf Ali S 
Cc: Susovan Mohapatra 
Cc: Ray Ni 
Signed-off-by: Zhiguang Liu 
---
 IntelFsp2Pkg/IntelFsp2Pkg.dsc |   5 +
 IntelFsp2Pkg/PreFspSec/PreFspSec.c| 115 ++
 IntelFsp2Pkg/PreFspSec/PreFspSec.inf  |  62 ++
 .../PreFspSec/X64/PreFspSecCommon.nasm|  94 ++
 4 files changed, 276 insertions(+)
 create mode 100644 IntelFsp2Pkg/PreFspSec/PreFspSec.c
 create mode 100644 IntelFsp2Pkg/PreFspSec/PreFspSec.inf
 create mode 100644 IntelFsp2Pkg/PreFspSec/X64/PreFspSecCommon.nasm

diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
index f236a7010b..a2cc29c940 100644
--- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
@@ -33,6 +33,8 @@
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
   
DebugDeviceLib|IntelFsp2Pkg/Library/BaseDebugDeviceLibNull/BaseDebugDeviceLibNull.inf
+  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+  
PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf

   # FSP override
   
DebugLib|IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/BaseFspDebugLibSerialPort.inf
@@ -75,6 +77,9 @@
   IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf
   IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf

+[Components.X64]
+  IntelFsp2Pkg/PreFspSec/PreFspSec.inf
+
 [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x1f
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80080046
diff --git a/IntelFsp2Pkg/PreFspSec/PreFspSec.c 
b/IntelFsp2Pkg/PreFspSec/PreFspSec.c
new file mode 100644
index 00..b3b52b8064
--- /dev/null
+++ b/IntelFsp2Pkg/PreFspSec/PreFspSec.c
@@ -0,0 +1,115 @@
+/** @file
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include "Guid/FspHeaderFile.h"
+#include 
+#include 
+#include 
+#include 
+
+/**
+  This fuction gets SecCore image base
+
+  @return   SecCore image base, or zero if no patch in nasm code
+
+**/
+UINTN
+EFIAPI
+AsmGetFspSecCoreImageBase  (
+  VOID
+  );
+
+/**
+  This fuction gets PeiCore image base
+
+  @return   PeiCore image base, or zero if no patch in nasm code
+
+**/
+UINTN
+EFIAPI
+AsmGetFspPeiCoreImageBase (
+  VOID
+  );
+
+/**
+  Relocate Pe/Te Image
+
+  @param[in] ImageBaseAddress   Image base address
+
+  @retval EFI_SUCCESS   Image is relocated successfully
+  @retval OthersImage is not relocated successfully
+**/
+EFI_STATUS
+RelocatePeTeImage (
+  UINT64  ImageBaseAddress
+  )
+{
+  RETURN_STATUS Status;
+  PE_COFF_LOADER_IMAGE_CONTEXT  ImageContext;
+
+  ZeroMem (&ImageContext, sizeof (ImageContext));
+
+  ImageContext.Handle= (VOID *)ImageBaseAddress;
+  ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
+
+  Status 

[edk2-devel] [PATCH v3 6/6] StandaloneMmPkg: Support to unregister MMI handler in MMI handlers

[Zhiguang Liu]

Unregistering MMI handler will free the MMI_HANDLER. However, the
MmiManage() may be using the link node from MMI_HANDLER for loop if
the unregistering happens in MMI handlers.
To avoid that, the idea is to inform MmiHandlerUnRegister() whether
it's running or not running on the stack of MmiManage(), and to
postpone MMI_HANDLER deletion until after the loop finishes.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Signed-off-by: Zhiguang Liu 
---
 StandaloneMmPkg/Core/Mmi.c | 161 +++--
 1 file changed, 136 insertions(+), 25 deletions(-)

diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
index 0de6fd17fc..e035245c87 100644
--- a/StandaloneMmPkg/Core/Mmi.c
+++ b/StandaloneMmPkg/Core/Mmi.c
@@ -34,11 +34,51 @@ typedef struct {
   LIST_ENTRYLink;// Link on MMI_ENTRY.MmiHandlers
   EFI_MM_HANDLER_ENTRY_POINTHandler; // The mm handler's entry point
   MMI_ENTRY *MmiEntry;
+  BOOLEAN   ToRemove;// To remove this MMI_HANDLER 
later
 } MMI_HANDLER;
 
+//
+// mMmiManageCallingDepth is used to track the depth of recursive calls of 
MmiManage.
+//
+UINTN  mMmiManageCallingDepth = 0;
+
 LIST_ENTRY  mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE 
(mRootMmiHandlerList);
 LIST_ENTRY  mMmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
(mMmiEntryList);
 
+/**
+  Remove MmiHandler and free the memory it used.
+  If MmiEntry is empty, remove MmiEntry and free the memory it used.
+
+  @param  MmiHandler  Points to MMI handler.
+  @param  MmiEntryPoints to MMI Entry or NULL for root MMI handlers.
+
+  @retval TRUEMmiEntry is removed.
+  @retval FALSE   MmiEntry is not removed.
+**/
+BOOLEAN
+RemoveMmiHandler (
+  IN MMI_HANDLER  *MmiHandler,
+  IN MMI_ENTRY*MmiEntry
+  )
+{
+  ASSERT (MmiHandler->ToRemove);
+  RemoveEntryList (&MmiHandler->Link);
+  FreePool (MmiHandler);
+
+  //
+  // Remove the MMI_ENTRY if all handlers have been removed.
+  //
+  if (MmiEntry != NULL) {
+if (IsListEmpty (&MmiEntry->MmiHandlers)) {
+  RemoveEntryList (&MmiEntry->AllEntries);
+  FreePool (MmiEntry);
+  return TRUE;
+}
+  }
+
+  return FALSE;
+}
+
 /**
   Finds the MMI entry for the requested handler type.
 
@@ -126,13 +166,16 @@ MmiManage (
 {
   LIST_ENTRY   *Link;
   LIST_ENTRY   *Head;
+  LIST_ENTRY   *EntryLink;
   MMI_ENTRY*MmiEntry;
   MMI_HANDLER  *MmiHandler;
-  BOOLEAN  SuccessReturn;
+  EFI_STATUS   ReturnStatus;
+  BOOLEAN  WillReturn;
   EFI_STATUS   Status;
 
-  Status= EFI_NOT_FOUND;
-  SuccessReturn = FALSE;
+  mMmiManageCallingDepth++;
+  Status   = EFI_NOT_FOUND;
+  ReturnStatus = Status;
   if (HandlerType == NULL) {
 //
 // Root MMI handler
@@ -171,7 +214,16 @@ MmiManage (
 // no additional handlers will be processed and EFI_INTERRUPT_PENDING 
will be returned.
 //
 if (HandlerType != NULL) {
-  return EFI_INTERRUPT_PENDING;
+  ReturnStatus = EFI_INTERRUPT_PENDING;
+  WillReturn   = TRUE;
+} else {
+  //
+  // If any other handler's result sets ReturnStatus as EFI_SUCCESS, 
the return status
+  // will be EFI_SUCCESS.
+  //
+  if (ReturnStatus != EFI_SUCCESS) {
+ReturnStatus = Status;
+  }
 }
 
 break;
@@ -183,10 +235,10 @@ MmiManage (
 // additional handlers will be processed.
 //
 if (HandlerType != NULL) {
-  return EFI_SUCCESS;
+  WillReturn = TRUE;
 }
 
-SuccessReturn = TRUE;
+ReturnStatus = EFI_SUCCESS;
 break;
 
   case EFI_WARN_INTERRUPT_SOURCE_QUIESCED:
@@ -194,7 +246,7 @@ MmiManage (
 // If at least one of the handlers returns 
EFI_WARN_INTERRUPT_SOURCE_QUIESCED
 // then the function will return EFI_SUCCESS.
 //
-SuccessReturn = TRUE;
+ReturnStatus = EFI_SUCCESS;
 break;
 
   case EFI_WARN_INTERRUPT_SOURCE_PENDING:
@@ -202,6 +254,10 @@ MmiManage (
 // If all the handlers returned EFI_WARN_INTERRUPT_SOURCE_PENDING
 // then EFI_WARN_INTERRUPT_SOURCE_PENDING will be returned.
 //
+if (ReturnStatus != EFI_SUCCESS) {
+  ReturnStatus = Status;
+}
+
 break;
 
   default:
@@ -211,13 +267,76 @@ MmiManage (
 ASSERT_EFI_ERROR (Status);
 break;
 }
+
+if (WillReturn) {
+  break;
+}
   }
 
-  if (SuccessReturn) {
-Status = EFI_SUCCESS;
+  ASSERT (mMmiManageCallingDepth > 0);
+  mMmiManageCallingDepth--;
+
+  //
+  // MmiHandlerUnRegister() calls from MMI handlers are deferred till this 
point.
+  // Before returned from MmiManage, delete the MmiHandler which is
+  // marked as ToRemove.
+  // Note that MmiManage can be called recursively.
+  //
+  if (mMmiManageCallingDepth == 0) {
+//
+   

[edk2-devel] [PATCH v3 5/6] MdeModulePkg/SMM: Support to unregister SMI handler in SMI handlers

[Zhiguang Liu]

Unregistering SMI handler will free the SMI_HANDLER. However, the
SmiManage() may be using the link node from SMI_HANDLER for loop if
the unregistering happens in SMI handlers.
To avoid that, the idea is to inform SmiHandlerUnRegister() whether
it's running or not running on the stack of SmiManage(), and to
postpone SMI_HANDLER deletion before SmiManage() returns.
Noted SmiManage() may be called recursively, the SmiHandlerUnRegister()
won't take effect until the root SmiManage() returns.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Signed-off-by: Zhiguang Liu 
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h |   1 +
 MdeModulePkg/Core/PiSmmCore/Smi.c   | 161 
 2 files changed, 137 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h 
b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index b8a490a8c3..60073c78b4 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -93,6 +93,7 @@ typedef struct {
   SMI_ENTRY   *SmiEntry;
   VOID*Context;// for profile
   UINTN   ContextSize; // for profile
+  BOOLEAN ToRemove;// To remove this SMI_HANDLER 
later
 } SMI_HANDLER;
 
 //
diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c 
b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 2985f989c3..53bec7600c 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -8,6 +8,11 @@
 
 #include "PiSmmCore.h"
 
+//
+// mSmiManageCallingDepth is used to track the depth of recursive calls of 
SmiManage.
+//
+UINTN  mSmiManageCallingDepth = 0;
+
 LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
 
 SMI_ENTRY  mRootSmiEntry = {
@@ -79,6 +84,40 @@ SmmCoreFindSmiEntry (
   return SmiEntry;
 }
 
+/**
+  Remove SmiHandler and free the memory it used.
+  If SmiEntry is empty, remove SmiEntry and free the memory it used.
+
+  @param  SmiHandler Points to SMI handler.
+  @param  SmiEntry   Points to SMI Entry or NULL for root SMI handlers.
+
+  @retval TRUESmiEntry is removed.
+  @retval FALSE   SmiEntry is not removed.
+**/
+BOOLEAN
+RemoveSmiHandler (
+  IN SMI_HANDLER  *SmiHandler,
+  IN SMI_ENTRY*SmiEntry
+  )
+{
+  ASSERT (SmiHandler->ToRemove);
+  RemoveEntryList (&SmiHandler->Link);
+  FreePool (SmiHandler);
+
+  //
+  // Remove the SMI_ENTRY if all handlers have been removed.
+  //
+  if (SmiEntry != NULL) {
+if (IsListEmpty (&SmiEntry->SmiHandlers)) {
+  RemoveEntryList (&SmiEntry->AllEntries);
+  FreePool (SmiEntry);
+  return TRUE;
+}
+  }
+
+  return FALSE;
+}
+
 /**
   Manage SMI of a particular type.
 
@@ -104,15 +143,17 @@ SmiManage (
 {
   LIST_ENTRY   *Link;
   LIST_ENTRY   *Head;
+  LIST_ENTRY   *EntryLink;
   SMI_ENTRY*SmiEntry;
   SMI_HANDLER  *SmiHandler;
-  BOOLEAN  SuccessReturn;
+  EFI_STATUS   ReturnStatus;
+  BOOLEAN  WillReturn;
   EFI_STATUS   Status;
 
   PERF_FUNCTION_BEGIN ();
-
-  Status= EFI_NOT_FOUND;
-  SuccessReturn = FALSE;
+  mSmiManageCallingDepth++;
+  Status   = EFI_NOT_FOUND;
+  ReturnStatus = Status;
   if (HandlerType == NULL) {
 //
 // Root SMI handler
@@ -152,7 +193,16 @@ SmiManage (
 //
 if (HandlerType != NULL) {
   PERF_FUNCTION_END ();
-  return EFI_INTERRUPT_PENDING;
+  ReturnStatus = EFI_INTERRUPT_PENDING;
+  WillReturn   = TRUE;
+} else {
+  //
+  // If any other handler's result sets ReturnStatus as EFI_SUCCESS, 
the return status
+  // will be EFI_SUCCESS.
+  //
+  if (ReturnStatus != EFI_SUCCESS) {
+ReturnStatus = Status;
+  }
 }
 
 break;
@@ -165,10 +215,10 @@ SmiManage (
 //
 if (HandlerType != NULL) {
   PERF_FUNCTION_END ();
-  return EFI_SUCCESS;
+  WillReturn = TRUE;
 }
 
-SuccessReturn = TRUE;
+ReturnStatus = EFI_SUCCESS;
 break;
 
   case EFI_WARN_INTERRUPT_SOURCE_QUIESCED:
@@ -176,7 +226,7 @@ SmiManage (
 // If at least one of the handlers returns 
EFI_WARN_INTERRUPT_SOURCE_QUIESCED
 // then the function will return EFI_SUCCESS.
 //
-SuccessReturn = TRUE;
+ReturnStatus = EFI_SUCCESS;
 break;
 
   case EFI_WARN_INTERRUPT_SOURCE_PENDING:
@@ -184,6 +234,10 @@ SmiManage (
 // If all the handlers returned EFI_WARN_INTERRUPT_SOURCE_PENDING
 // then EFI_WARN_INTERRUPT_SOURCE_PENDING will be returned.
 //
+if (ReturnStatus != EFI_SUCCESS) {
+  ReturnStatus = Status;
+}
+
 break;
 
   default:
@@ -193,14 +247,74 @@ SmiManage (
 ASSERT (FALSE);
 break;
 }
+
+if (WillReturn) {
+  break;
+}
   }
 
-  if (SuccessReturn) {
-Status = EFI_SUCCESS;
+  ASSERT (mSmiManageCallingDepth > 0);
+  

[edk2-devel] [PATCH v3 4/6] Revert ae1079b386a597108a8070652bf7cdaa4ec3dda3

[Zhiguang Liu]

This reverts "MdeModulePkg/SMM: Support to unregister
SMI handler inside SMI handler" for better design later.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Signed-off-by: Zhiguang Liu 
---
 MdeModulePkg/Core/PiSmmCore/Smi.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c 
b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 3489c130fd..2985f989c3 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -134,14 +134,8 @@ SmiManage (
 
   Head = &SmiEntry->SmiHandlers;
 
-  for (Link = Head->ForwardLink; Link != Head;) {
+  for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
 SmiHandler = CR (Link, SMI_HANDLER, Link, SMI_HANDLER_SIGNATURE);
-//
-// To support unregister SMI handler inside SMI handler itself,
-// get next node before handler is executed, since LIST_ENTRY that
-// Link points to may be freed if unregister SMI handler.
-//
-Link = Link->ForwardLink;
 
 Status = SmiHandler->Handler (
(EFI_HANDLE)SmiHandler,
-- 
2.31.1.windows.1



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

[edk2-devel] [PATCH v3 3/6] Revert 17b28722008eab745ce186b72cd325944cbe6bf0

[Zhiguang Liu]

This reverts commit "MdeModulePkg/SMM: Disallow unregister
 SMI handler in other SMI handler" for better design later.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Signed-off-by: Zhiguang Liu 
---
 MdeModulePkg/Core/PiSmmCore/Smi.c | 29 -
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c 
b/MdeModulePkg/Core/PiSmmCore/Smi.c
index b3a81ac877..3489c130fd 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -8,8 +8,7 @@
 
 #include "PiSmmCore.h"
 
-SMI_HANDLER  *mCurrentSmiHandler = NULL;
-LIST_ENTRY   mSmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
(mSmiEntryList);
+LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
 
 SMI_ENTRY  mRootSmiEntry = {
   SMI_ENTRY_SIGNATURE,
@@ -143,18 +142,13 @@ SmiManage (
 // Link points to may be freed if unregister SMI handler.
 //
 Link = Link->ForwardLink;
-//
-// Assign gCurrentSmiHandle before calling the SMI handler and
-// set to NULL when it returns.
-//
-mCurrentSmiHandler = SmiHandler;
-Status = SmiHandler->Handler (
-   (EFI_HANDLE)SmiHandler,
-   Context,
-   CommBuffer,
-   CommBufferSize
-   );
-mCurrentSmiHandler = NULL;
+
+Status = SmiHandler->Handler (
+   (EFI_HANDLE)SmiHandler,
+   Context,
+   CommBuffer,
+   CommBufferSize
+   );
 
 switch (Status) {
   case EFI_INTERRUPT_PENDING:
@@ -334,13 +328,6 @@ SmiHandlerUnRegister (
 return EFI_INVALID_PARAMETER;
   }
 
-  //
-  // Do not allow to unregister SMI Handler inside other SMI Handler
-  //
-  if ((mCurrentSmiHandler != NULL) && (mCurrentSmiHandler != SmiHandler)) {
-return EFI_INVALID_PARAMETER;
-  }
-
   SmiEntry = SmiHandler->SmiEntry;
 
   RemoveEntryList (&SmiHandler->Link);
-- 
2.31.1.windows.1



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

[edk2-devel] [PATCH v3 2/6] Revert 049ff6c39c73edd3709c05bd0e46184320471358

[Zhiguang Liu]

This reverts commit "StandaloneMmPkg: Support to unregister
MMI handler inside MMI handler" for better design later.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Signed-off-by: Zhiguang Liu 
---
 StandaloneMmPkg/Core/Mmi.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
index c1a1d76e85..0de6fd17fc 100644
--- a/StandaloneMmPkg/Core/Mmi.c
+++ b/StandaloneMmPkg/Core/Mmi.c
@@ -154,14 +154,9 @@ MmiManage (
 Head = &MmiEntry->MmiHandlers;
   }
 
-  for (Link = Head->ForwardLink; Link != Head;) {
+  for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
 MmiHandler = CR (Link, MMI_HANDLER, Link, MMI_HANDLER_SIGNATURE);
-//
-// To support unregister MMI handler inside MMI handler itself,
-// get next node before handler is executed, since LIST_ENTRY that
-// Link points to may be freed if unregister MMI handler.
-//
-Link   = Link->ForwardLink;
+
 Status = MmiHandler->Handler (
(EFI_HANDLE)MmiHandler,
Context,
-- 
2.31.1.windows.1



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

[edk2-devel] [PATCH v3 1/6] Revert 2ec8f0c6407f062441b205b900038933865c7b3c

[Zhiguang Liu]

This reverts commit "StandaloneMmPkg: Disallow unregister MMI
handler in other MMI handler" for better design later.

Cc: Liming Gao 
Cc: Jiaxin Wu 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Signed-off-by: Zhiguang Liu 
---
 StandaloneMmPkg/Core/Mmi.c | 32 +---
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
index 9e52072bf7..c1a1d76e85 100644
--- a/StandaloneMmPkg/Core/Mmi.c
+++ b/StandaloneMmPkg/Core/Mmi.c
@@ -36,9 +36,8 @@ typedef struct {
   MMI_ENTRY *MmiEntry;
 } MMI_HANDLER;
 
-LIST_ENTRY   mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE 
(mRootMmiHandlerList);
-LIST_ENTRY   mMmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
(mMmiEntryList);
-MMI_HANDLER  *mCurrentMmiHandler = NULL;
+LIST_ENTRY  mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE 
(mRootMmiHandlerList);
+LIST_ENTRY  mMmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
(mMmiEntryList);
 
 /**
   Finds the MMI entry for the requested handler type.
@@ -162,19 +161,13 @@ MmiManage (
 // get next node before handler is executed, since LIST_ENTRY that
 // Link points to may be freed if unregister MMI handler.
 //
-Link = Link->ForwardLink;
-//
-// Assign gCurrentMmiHandle before calling the MMI handler and
-// set to NULL when it returns.
-//
-mCurrentMmiHandler = MmiHandler;
-Status = MmiHandler->Handler (
-   (EFI_HANDLE)MmiHandler,
-   Context,
-   CommBuffer,
-   CommBufferSize
-   );
-mCurrentMmiHandler = NULL;
+Link   = Link->ForwardLink;
+Status = MmiHandler->Handler (
+   (EFI_HANDLE)MmiHandler,
+   Context,
+   CommBuffer,
+   CommBufferSize
+   );
 
 switch (Status) {
   case EFI_INTERRUPT_PENDING:
@@ -321,13 +314,6 @@ MmiHandlerUnRegister (
 return EFI_INVALID_PARAMETER;
   }
 
-  //
-  // Do not allow to unregister MMI Handler inside other MMI Handler
-  //
-  if ((mCurrentMmiHandler != NULL) && (mCurrentMmiHandler != MmiHandler)) {
-return EFI_INVALID_PARAMETER;
-  }
-
   MmiEntry = MmiHandler->MmiEntry;
 
   RemoveEntryList (&MmiHandler->Link);
-- 
2.31.1.windows.1



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

[edk2-devel] [PATCH v3 0/6] Support to unregister SMI handler in SMI handlers

[Zhiguang Liu]

Months ago, I sent patch set to fix potential issues in the usage of SMI 
handler unregistering SMI handler. Discussion can be found in below
link:
https://edk2.groups.io/g/devel/topic/103925794#114251

The conclusion was to only support SMI handler unregistering itself, and not 
allow  SMI handler unregistering other handlers, because we thought there would 
be no such usage.
However, recently, I find in some platform, there is kind of usage.
To also support SMI handler unregistering other handlers, this patch set use a 
totally different design. So I revert the former ones first to make the code 
more readable.

Thank Laszlo for bring up the initial idea for the new design.

V2&V3:
Code refine based on Ray's comments

Zhiguang Liu (6):
  Revert 2ec8f0c6407f062441b205b900038933865c7b3c
  Revert 049ff6c39c73edd3709c05bd0e46184320471358
  Revert 17b28722008eab745ce186b72cd325944cbe6bf0
  Revert ae1079b386a597108a8070652bf7cdaa4ec3dda3
  MdeModulePkg/SMM: Support to unregister SMI handler in SMI handlers
  StandaloneMmPkg: Support to unregister MMI handler in MMI handlers

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h |   1 +
 MdeModulePkg/Core/PiSmmCore/Smi.c   | 198 ---
 StandaloneMmPkg/Core/Mmi.c  | 200 +---
 3 files changed, 292 insertions(+), 107 deletions(-)

-- 
2.31.1.windows.1



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

[edk2-devel] [PATCH v2] IntelFsp2Pkg: Optional Plugin for FSP SecCore/PeiCore Rebasing

[Zhiguang Liu]

Note this plugin only applies to 64-bit PSP
This optional plugin is designed to execute before the FSP SecCore to
rebase SecCore and PeiCore during runtime. If the FSP binary requires
rebasing at runtime, this module should be included within the FSP
binary. Additionally, specific patches must be applied to ensure proper
functionality. In the absence of this module, manual patching of API
offsets within the FSP header is necessary. To illustrate, let's
consider a scenario within FSP-S where 'FspSiliconInitEntry' is the
initial API to be executed post-rebase. Rather than directly inputting
the 'FspSiliconInit' offset into the 'FspSiliconInitEntryOffset' field
of the FSP header, the entry point of this module should be used.
Furthermore, the 'FspSiliconInit' offset should be placed into
'AsmGetFspOriginalEntry', which signifies the address to which this
module will jump.
It is also essential to patch the image bases of SecCore and PeiCore
to enable the rebasing functionality of this module.
The following is an example of how to apply the necessary patches:
Patch Address   Patch Value
 PreFspSec:_ModuleEntryPoint - [0x]
PreFspSec:SecCoreRelativeOffPreFspSec:AsmGetFspSecCoreImageBase
 - Fsp24SecCoreS:BASE
PreFspSec:PeiCoreRelativeOffPreFspSec:AsmGetFspPeiCoreImageBase
 - PeiCore:BASE
PreFspSec:SecEntryRelativeOff   PreFspSec:AsmGetFspOriginalEntry
 - Fsp24SecCoreS:FspSiliconInitApi

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Duggapu Chinni B 
Cc: Star Zeng 
Cc: Ted Kuo 
Cc: Ashraf Ali S 
Cc: Susovan Mohapatra 
Cc: Ray Ni 
Signed-off-by: Zhiguang Liu 
---
 IntelFsp2Pkg/IntelFsp2Pkg.dsc |   5 +
 IntelFsp2Pkg/PreFspSec/PreFspSec.c| 115 ++
 IntelFsp2Pkg/PreFspSec/PreFspSec.inf  |  62 ++
 .../PreFspSec/X64/PreFspSecCommon.nasm|  94 ++
 4 files changed, 276 insertions(+)
 create mode 100644 IntelFsp2Pkg/PreFspSec/PreFspSec.c
 create mode 100644 IntelFsp2Pkg/PreFspSec/PreFspSec.inf
 create mode 100644 IntelFsp2Pkg/PreFspSec/X64/PreFspSecCommon.nasm

diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
index f236a7010b..a2cc29c940 100644
--- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
@@ -33,6 +33,8 @@
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
   
DebugDeviceLib|IntelFsp2Pkg/Library/BaseDebugDeviceLibNull/BaseDebugDeviceLibNull.inf
+  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+  
PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf
 
   # FSP override
   
DebugLib|IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/BaseFspDebugLibSerialPort.inf
@@ -75,6 +77,9 @@
   IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf
   IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
 
+[Components.X64]
+  IntelFsp2Pkg/PreFspSec/PreFspSec.inf
+
 [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x1f
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80080046
diff --git a/IntelFsp2Pkg/PreFspSec/PreFspSec.c 
b/IntelFsp2Pkg/PreFspSec/PreFspSec.c
new file mode 100644
index 00..b3b52b8064
--- /dev/null
+++ b/IntelFsp2Pkg/PreFspSec/PreFspSec.c
@@ -0,0 +1,115 @@
+/** @file
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include "Guid/FspHeaderFile.h"
+#include 
+#include 
+#include 
+#include 
+
+/**
+  This fuction gets SecCore image base
+
+  @return   SecCore image base, or zero if no patch in nasm code
+
+**/
+UINTN
+EFIAPI
+AsmGetFspSecCoreImageBase  (
+  VOID
+  );
+
+/**
+  This fuction gets PeiCore image base
+
+  @return   PeiCore image base, or zero if no patch in nasm code
+
+**/
+UINTN
+EFIAPI
+AsmGetFspPeiCoreImageBase (
+  VOID
+  );
+
+/**
+  Relocate Pe/Te Image
+
+  @param[in] ImageBaseAddress   Image base address
+
+  @retval EFI_SUCCESS   Image is relocated successfully
+  @retval OthersImage is not relocated successfully
+**/
+EFI_STATUS
+RelocatePeTeImage (
+  UINT64  ImageBaseAddress
+  )
+{
+  RETURN_STATUS Status;
+  PE_COFF_LOADER_IMAGE_CONTEXT  ImageContext;
+
+  ZeroMem (&ImageContext, sizeof (ImageContext));
+
+  ImageContext.Handle= (VOID *)ImageBaseAddress;
+  ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
+
+  Status = PeCoffLoaderGetImageInfo (&ImageContext);
+  if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+
+  ImageContext.ImageAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)ImageBaseAddress;
+
+  //
+  // rebase the image
+  //
+  Status = PeCoffLoaderRelocateImage (&ImageContext);
+
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
+
+/**
+  This function will patc