Date   

[edk2-platforms] [patch 02/35] Drivers/DisplayLink: Consume RegisterFilterLibNull instance

Dandan Bi
 

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

Add RegisterFilterLibNull in dsc which will be consumed by IoLib and BaseLib.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Andy Hayes <andy.hayes@...>
Signed-off-by: Dandan Bi <dandan.bi@...>
---
Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc | 1 +
1 file changed, 1 insertion(+)

diff --git a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc
index 955331ba60..23a9feb175 100644
--- a/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc
+++ b/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc
@@ -28,10 +28,11 @@ [LibraryClasses]
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
+ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf

[LibraryClasses.common.UEFI_DRIVER]
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf

[LibraryClasses.AARCH64]
--
2.18.0.windows.1


[edk2-platforms] [patch 04/35] Features/Debugging: Consume RegisterFilterLibNull instance

Dandan Bi
 

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

Add RegisterFilterLibNull in dsc which will be consumed by IoLib and BaseLib.

Cc: Eric Dong <eric.dong@...>
Cc: Liming Gao <gaoliming@...>
Signed-off-by: Dandan Bi <dandan.bi@...>
---
.../Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.dsc | 3 ++-
.../Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc | 3 ++-
.../PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc | 3 ++-
.../Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc | 3 ++-
4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/Features/Intel/Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.dsc b/Features/Intel/Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.dsc
index 3e6b1f69c2..ab269dd8df 100644
--- a/Features/Intel/Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.dsc
+++ b/Features/Intel/Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.dsc
@@ -4,11 +4,11 @@
#
# The DEC files are used by the utilities that parse DSC and
# INF files to generate AutoGen.c and AutoGen.h files
# for the build infrastructure.
#
-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2019 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##

@@ -43,10 +43,11 @@ [LibraryClasses]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
+ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf

[LibraryClasses.common.DXE_DRIVER,LibraryClasses.common.DXE_RUNTIME_DRIVER]
#######################################
# Edk2 Packages
#######################################
diff --git a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc
index 65e00b5979..e6e480b734 100644
--- a/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc
+++ b/Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc
@@ -4,11 +4,11 @@
#
# The DEC files are used by the utilities that parse DSC and
# INF files to generate AutoGen.c and AutoGen.h files
# for the build infrastructure.
#
-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##

@@ -57,10 +57,11 @@ [LibraryClasses]
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
+ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf

[LibraryClasses.common.PEIM]
#######################################
# Edk2 Packages
#######################################
diff --git a/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc b/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc
index 2852b9cf7c..539c74c84d 100644
--- a/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc
+++ b/Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc
@@ -4,11 +4,11 @@
#
# The DEC files are used by the utilities that parse DSC and
# INF files to generate AutoGen.c and AutoGen.h files
# for the build infrastructure.
#
-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##

@@ -56,10 +56,11 @@ [LibraryClasses]
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
+ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf

[LibraryClasses.common.PEIM]
#######################################
# Edk2 Packages
#######################################
diff --git a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc
index 95adb01a74..7b1e41d999 100644
--- a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc
+++ b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc
@@ -4,11 +4,11 @@
#
# The DEC files are used by the utilities that parse DSC and
# INF files to generate AutoGen.c and AutoGen.h files
# for the build infrastructure.
#
-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2019 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##

@@ -46,10 +46,11 @@ [LibraryClasses]
TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
+ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf

[LibraryClasses.common.PEIM]
#######################################
# Edk2 Packages
#######################################
--
2.18.0.windows.1


[edk2-platforms] [patch 03/35] Drivers/OptionRomPkg: Consume RegisterFilterLibNull instance

Dandan Bi
 

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

Add RegisterFilterLibNull in dsc which will be consumed by IoLib and BaseLib.

Cc: Ray Ni <ray.ni@...>
Signed-off-by: Dandan Bi <dandan.bi@...>
---
Drivers/OptionRomPkg/OptionRomPkg.dsc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Drivers/OptionRomPkg/OptionRomPkg.dsc b/Drivers/OptionRomPkg/OptionRomPkg.dsc
index 8a13cc54e6..5c3aed6408 100644
--- a/Drivers/OptionRomPkg/OptionRomPkg.dsc
+++ b/Drivers/OptionRomPkg/OptionRomPkg.dsc
@@ -4,11 +4,11 @@
# This package is designed to interoperate with the EDK II open source project
# at http://www.tianocore.org, and this package is required to build PCI compliant
# Option ROM image for all CPU architectures, including EBC target.
# A single driver can support mixes of EFI 1.1, UEFI 2.0 and UEFI 2.1.
#
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
# Copyright (c) 2020, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -53,10 +53,11 @@ [LibraryClasses]
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
+ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf

[LibraryClasses.AARCH64, LibraryClasses.ARM]
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

[LibraryClasses.ARM]
--
2.18.0.windows.1


[edk2-platforms] [patch 00/35] Consume RegisterFilterLibNull instance

Dandan Bi
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246
RFC: https://edk2.groups.io/g/devel/message/72530


Add RegisterFilterLibNull in dsc files in edk2-platforms repo,
which will be consumed by IoLib and BaseLib.

This is the following update in edk2-platforms repo for the change in edk2,
which will add RegisterFilterLib dependency for IoLib and BaseLib to filter/trace
port IO/MMIO/MSR access.
https://edk2.groups.io/g/devel/message/72754

Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>

Dandan Bi (35):
Drivers/ASIX: Consume RegisterFilterLibNull instance
Drivers/DisplayLink: Consume RegisterFilterLibNull instance
Drivers/OptionRomPkg: Consume RegisterFilterLibNull instance
Features/Debugging: Consume RegisterFilterLibNull instance
Features/Network: Consume RegisterFilterLibNull instance
Features/OutOfBandManagement: Consume RegisterFilterLibNull instance
Features/PowerManagement: Consume RegisterFilterLibNull instance
Features/SystemInformation: Consume RegisterFilterLibNull instance
Features/UserInterface: Consume RegisterFilterLibNull instance
Platform/AMD: Consume RegisterFilterLibNull instance
Platform/ARM: Consume RegisterFilterLibNull instance
Platform/BeagleBoard: Consume RegisterFilterLibNull instance
Platform/BoardModulePkg: Consume RegisterFilterLibNull instance
Platform/MinPlatformPkg: Consume RegisterFilterLibNull instance
Platform/QuarkPlatformPkg: Consume RegisterFilterLibNull instance
Platform/Vlv2TbltDevicePkg: Consume RegisterFilterLibNull instance
Platform/LeMaker: Consume RegisterFilterLibNull instance
Platform/Qemu: Consume RegisterFilterLibNull instance
Platform/RaspberryPi: Consume RegisterFilterLibNull instance
Platform/RISC-V: Consume RegisterFilterLibNull instance
Platform/SiFive: Consume RegisterFilterLibNull instance
Platform/Socionext: Consume RegisterFilterLibNull instance
Platform/SoftIron: Consume RegisterFilterLibNull instance
Silicon/Hisilicon: Consume RegisterFilterLibNull instance
Silicon/CoffeelakeSiliconPkg: Consume RegisterFilterLibNull instance
Silicon/IntelSiliconPkg: Consume RegisterFilterLibNull instance
Silicon/KabylakeSiliconPkg: Consume RegisterFilterLibNull instance
Silicon/QuarkSocPkg: Consume RegisterFilterLibNull instance
Silicon/TigerlakeSiliconPkg: Consume RegisterFilterLibNull instance
Silicon/Marvell: Consume RegisterFilterLibNull instance
Silicon/NXP: Consume RegisterFilterLibNull instance
Silicon/Openmoko: Consume RegisterFilterLibNull instance
Silicon/RISC_V: Consume RegisterFilterLibNull instance
Silicon/Synopsys/DesignWare: Consume RegisterFilterLibNull instance
Silicon/TexasInstruments: Consume RegisterFilterLibNull instance

Drivers/ASIX/Asix.dsc | 1 +
Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc | 1 +
Drivers/OptionRomPkg/OptionRomPkg.dsc | 3 ++-
.../Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.dsc | 3 ++-
.../Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc | 3 ++-
.../PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc | 3 ++-
.../Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc | 3 ++-
.../Intel/Network/NetworkFeaturePkg/Include/NetworkFeature.dsc | 3 ++-
.../OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc | 3 ++-
.../OutOfBandManagement/SpcrFeaturePkg/Include/SpcrFeature.dsc | 3 ++-
.../Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc | 3 ++-
.../SmbiosFeaturePkg/Include/SmbiosFeature.dsc | 3 ++-
.../Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc | 3 ++-
.../UserAuthFeaturePkg/Include/UserAuthFeature.dsc | 3 ++-
.../Include/VirtualKeyboardFeature.dsc | 3 ++-
Platform/AMD/OverdriveBoard/OverdriveBoard.dsc | 1 +
Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 1 +
Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 1 +
Platform/BeagleBoard/BeagleBoardPkg/BeagleBoardPkg.dsc | 3 ++-
Platform/Intel/BoardModulePkg/BoardModulePkg.dsc | 3 ++-
Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc | 3 ++-
Platform/Intel/QuarkPlatformPkg/Quark.dsc | 1 +
Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc | 1 +
Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 3 ++-
Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 3 ++-
Platform/LeMaker/CelloBoard/CelloBoard.dsc | 1 +
Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 1 +
Platform/RISC-V/PlatformPkg/RiscVPlatformPkg.dsc | 1 +
Platform/RaspberryPi/RPi3/RPi3.dsc | 3 ++-
Platform/RaspberryPi/RPi4/RPi4.dsc | 3 ++-
Platform/SiFive/U5SeriesPkg/FreedomU500VC707Board/U500.dsc | 1 +
.../U5SeriesPkg/FreedomU540HiFiveUnleashedBoard/U540.dsc | 1 +
Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc | 1 +
Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 1 +
Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc | 1 +
Silicon/Hisilicon/Hisilicon.dsc.inc | 1 +
Silicon/Intel/CoffeelakeSiliconPkg/CoffeelakeSiliconPkg.dsc | 1 +
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 3 ++-
Silicon/Intel/KabylakeSiliconPkg/KabylakeSiliconPkg.dsc | 3 ++-
Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dsc | 3 ++-
Silicon/Intel/TigerlakeSiliconPkg/TigerlakeSiliconPkg.dsc | 1 +
Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 1 +
Silicon/NXP/NxpQoriqLs.dsc.inc | 1 +
Silicon/Openmoko/Openmoko.dsc | 1 +
Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dsc | 1 +
Silicon/Synopsys/DesignWare/DesignWare.dsc | 1 +
Silicon/TexasInstruments/Omap35xxPkg/Omap35xxPkg.dsc | 1 +
47 files changed, 70 insertions(+), 23 deletions(-)

--
2.18.0.windows.1


[edk2-platforms] [patch 01/35] Drivers/ASIX: Consume RegisterFilterLibNull instance

Dandan Bi
 

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

Add RegisterFilterLibNull in dsc which will be consumed by IoLib and BaseLib.

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Dandan Bi <dandan.bi@...>
---
Drivers/ASIX/Asix.dsc | 1 +
1 file changed, 1 insertion(+)

diff --git a/Drivers/ASIX/Asix.dsc b/Drivers/ASIX/Asix.dsc
index 5e02e11760..4a9f8897b0 100644
--- a/Drivers/ASIX/Asix.dsc
+++ b/Drivers/ASIX/Asix.dsc
@@ -40,10 +40,11 @@ [LibraryClasses]
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
+ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf

[LibraryClasses.AARCH64, LibraryClasses.ARM]
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

[LibraryClasses.ARM]
--
2.18.0.windows.1


Re: [PATCH v9 00/10] support CPU hot-unplug

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Hi,

This series adds OVMF support for CPU hot-unplug.

QEMU secureboot hot-unplug logic corresponding to this is in upstream.
Also posted here:
https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imammedo@redhat.com/

Testing (with QEMU 5.2.50):
- Stable with randomized CPU plug/unplug (guest maxcpus=33,128)
- Synthetic tests with simultaneous multi CPU hot-unplug

Also at:
github.com/terminus/edk2/ hot-unplug-v9

Changelog:

v9:
- Rebased on top of edd46cd407ea
- Clarify comments around memory-barriers in patches 7, 8, 9
- Address other review comments from v8
Merged as commit range 4751a48aeb2a..f3bdfc41866e, via
<https://github.com/tianocore/edk2/pull/1494>, with the following R-b's
/ comments / light tweaks added:

1: 4b4466ed7c44 = 1: 0cb242e33693 OvmfPkg/CpuHotplugSmm: refactor hotplug logic
2: 719828efdebe ! 2: a752dd07466c OvmfPkg/CpuHotplugSmm: collect hot-unplug events
@@ -20,6 +20,7 @@
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
Message-Id: <20210312062656.2477515-3-ankur.a.arora@...>
+ Reviewed-by: Laszlo Ersek <lersek@...>

diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
3: 7d732efcb7af = 3: 2d92e052c3af OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
4: 64649eeeee42 = 4: 15e6ae8ea400 OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
5: 7190a5d134a7 ! 5: 8ade9d425a6e OvmfPkg: define CPU_HOT_EJECT_DATA
@@ -15,6 +15,7 @@
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
Message-Id: <20210312062656.2477515-6-ankur.a.arora@...>
+ Reviewed-by: Laszlo Ersek <lersek@...>

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
--- a/OvmfPkg/OvmfPkg.dec
6: 1fd019dac476 ! 6: b6d5996706dd OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
@@ -22,6 +22,7 @@
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
Message-Id: <20210312062656.2477515-7-ankur.a.arora@...>
+ Reviewed-by: Laszlo Ersek <lersek@...>

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
7: c8758d9b9764 ! 7: af9c77e151fa OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler
@@ -19,6 +19,7 @@
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
Message-Id: <20210312062656.2477515-8-ankur.a.arora@...>
+ Reviewed-by: Laszlo Ersek <lersek@...>

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
8: 5ff8e8700678 ! 8: 30c69d2cfa63 OvmfPkg/CpuHotplugSmm: add EjectCpu()
@@ -19,6 +19,7 @@
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
Message-Id: <20210312062656.2477515-9-ankur.a.arora@...>
+ Reviewed-by: Laszlo Ersek <lersek@...>

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
9: d6660f8f2f14 ! 9: f05328886302 OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject
@@ -27,6 +27,9 @@
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
Message-Id: <20210312062656.2477515-10-ankur.a.arora@...>
+ Reviewed-by: Laszlo Ersek <lersek@...>
+ [lersek@...: unneeded inner QemuSelector declaration in EjectCpu()
+ triggers VS warning #4456 (local variable shadowed); remove it]

diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -102,8 +105,6 @@
+ UINT32 Idx;
+
+ for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) {
-+ UINT64 QemuSelector;
-+
+ QemuSelector = mCpuHotEjectData->QemuSelectorMap[Idx];
+
+ if (QemuSelector != CPU_EJECT_QEMU_SELECTOR_INVALID) {
10: 0a9a1cd40b98 ! 10: f3bdfc41866e OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug
@@ -14,6 +14,9 @@
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
Message-Id: <20210312062656.2477515-11-ankur.a.arora@...>
+ [lersek@...: preserve the empty line between the ICH9_LPC_SMI_F_*
+ group of macro definitions and the SCRATCH_BUFFER type definition]
+ Reviewed-by: Laszlo Ersek <lersek@...>

diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -22,16 +25,15 @@
// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files.
//
#define ICH9_LPC_SMI_F_CPU_HOTPLUG BIT1
--
+//
+// The following bit value stands for "enable CPU hot-unplug, and inject an SMI
+// with control value ICH9_APM_CNT_CPU_HOTPLUG upon hot-unplug", in the
+// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files.
+//
+#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG BIT2
+
//
// Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
- // for the S3 boot script fragment to write to and read from.
@@
QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures);
Thanks for the contribution,
Laszlo


Re: [PATCH v2 1/1] UefiCpuPkg/CpuCacheInfoLib: Collect cache associative type

Dong, Eric
 

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

-----Original Message-----
From: Lou, Yun <yun.lou@...>
Sent: Monday, March 15, 2021 9:49 PM
To: devel@edk2.groups.io
Cc: Lou, Yun <yun.lou@...>; Ni, Ray <ray.ni@...>; Dong, Eric <eric.dong@...>; Laszlo Ersek <lersek@...>; Kumar, Rahul1 <rahul1.kumar@...>
Subject: [PATCH v2 1/1] UefiCpuPkg/CpuCacheInfoLib: Collect cache associative type

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

Support collecting cache associative type in CpuCacheInfoLib.
This prevents the user from using additional code to obtain the
same information.

Signed-off-by: Jason Lou <yun.lou@...>
Cc: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c | 49 +++++++++++---------
UefiCpuPkg/Include/Library/CpuCacheInfoLib.h | 15 +++++-
UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h | 15 +++++-
3 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
index d46fb0425851..126ee0da86fc 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
@@ -1,7 +1,7 @@
/** @file

Provides cache info for each package, core type, cache level and cache type.



- Copyright (c) 2020 Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/

@@ -23,18 +23,18 @@ CpuCacheInfoPrintCpuCacheInfoTable (
{

UINTN Index;



- DEBUG ((DEBUG_INFO, "+-------+-------------------------------------------------------------------------------+\n"));

- DEBUG ((DEBUG_INFO, "| Index | Packge CoreType CacheLevel CacheType CacheWays CacheSizeinKB CacheCount |\n"));

- DEBUG ((DEBUG_INFO, "+-------+-------------------------------------------------------------------------------+\n"));

+ DEBUG ((DEBUG_INFO, "+-------+--------------------------------------------------------------------------------------+\n"));

+ DEBUG ((DEBUG_INFO, "| Index | Packge CoreType CacheLevel CacheType CacheWays (FA|DM) CacheSizeinKB CacheCount |\n"));

+ DEBUG ((DEBUG_INFO, "+-------+--------------------------------------------------------------------------------------+\n"));



for (Index = 0; Index < CpuCacheInfoCount; Index++) {

- DEBUG ((DEBUG_INFO, "| %4x | %4x %2x %2x %2x %4x %8x %4x |\n", Index,

- CpuCacheInfo[Index].Package, CpuCacheInfo[Index].CoreType, CpuCacheInfo[Index].CacheLevel,

- CpuCacheInfo[Index].CacheType, CpuCacheInfo[Index].CacheWays, CpuCacheInfo[Index].CacheSizeinKB,

- CpuCacheInfo[Index].CacheCount));

+ DEBUG ((DEBUG_INFO, "| %4x | %4x %2x %2x %2x %4x ( %x| %x) %8x %4x |\n",

+ Index, CpuCacheInfo[Index].Package, CpuCacheInfo[Index].CoreType, CpuCacheInfo[Index].CacheLevel,

+ CpuCacheInfo[Index].CacheType, CpuCacheInfo[Index].CacheWays, CpuCacheInfo[Index].FullyAssociativeCache,

+ CpuCacheInfo[Index].DirectMappedCache, CpuCacheInfo[Index].CacheSizeinKB, CpuCacheInfo[Index].CacheCount));

}



- DEBUG ((DEBUG_INFO, "+-------+-------------------------------------------------------------------------------+\n"));

+ DEBUG ((DEBUG_INFO, "+-------+--------------------------------------------------------------------------------------+\n"));

}



/**

@@ -160,6 +160,7 @@ CpuCacheInfoCollectCoreAndCacheData (
CPUID_CACHE_PARAMS_EAX CacheParamEax;

CPUID_CACHE_PARAMS_EBX CacheParamEbx;

UINT32 CacheParamEcx;

+ CPUID_CACHE_PARAMS_EDX CacheParamEdx;

CPUID_NATIVE_MODEL_ID_AND_CORE_TYPE_EAX NativeModelIdAndCoreTypeEax;

COLLECT_CPUID_CACHE_DATA_CONTEXT *Context;

CPUID_CACHE_DATA *CacheData;

@@ -185,17 +186,19 @@ CpuCacheInfoCollectCoreAndCacheData (
CacheParamLeafIndex = 0;



while (CacheParamLeafIndex < MAX_NUM_OF_CACHE_PARAMS_LEAF) {

- AsmCpuidEx (CPUID_CACHE_PARAMS, CacheParamLeafIndex, &CacheParamEax.Uint32, &CacheParamEbx.Uint32, &CacheParamEcx, NULL);

+ AsmCpuidEx (CPUID_CACHE_PARAMS, CacheParamLeafIndex, &CacheParamEax.Uint32, &CacheParamEbx.Uint32, &CacheParamEcx, &CacheParamEdx.Uint32);



if (CacheParamEax.Bits.CacheType == 0) {

break;

}



- CacheData[CacheParamLeafIndex].CacheLevel = (UINT8)CacheParamEax.Bits.CacheLevel;

- CacheData[CacheParamLeafIndex].CacheType = (UINT8)CacheParamEax.Bits.CacheType;

- CacheData[CacheParamLeafIndex].CacheWays = (UINT16)CacheParamEbx.Bits.Ways;

- CacheData[CacheParamLeafIndex].CacheShareBits = (UINT16)CacheParamEax.Bits.MaximumAddressableIdsForLogicalProcessors;

- CacheData[CacheParamLeafIndex].CacheSizeinKB = (CacheParamEbx.Bits.Ways + 1) *

+ CacheData[CacheParamLeafIndex].CacheLevel = (UINT8)CacheParamEax.Bits.CacheLevel;

+ CacheData[CacheParamLeafIndex].CacheType = (UINT8)CacheParamEax.Bits.CacheType;

+ CacheData[CacheParamLeafIndex].CacheWays = (UINT16)CacheParamEbx.Bits.Ways;

+ CacheData[CacheParamLeafIndex].FullyAssociativeCache = (UINT8)CacheParamEax.Bits.FullyAssociativeCache;

+ CacheData[CacheParamLeafIndex].DirectMappedCache = (UINT8)CacheParamEdx.Bits.ComplexCacheIndexing;

+ CacheData[CacheParamLeafIndex].CacheShareBits = (UINT16)CacheParamEax.Bits.MaximumAddressableIdsForLogicalProcessors;

+ CacheData[CacheParamLeafIndex].CacheSizeinKB = (CacheParamEbx.Bits.Ways + 1) *

(CacheParamEbx.Bits.LinePartitions + 1) * (CacheParamEbx.Bits.LineSize + 1) * (CacheParamEcx + 1) / SIZE_1KB;



CacheParamLeafIndex++;

@@ -305,13 +308,15 @@ CpuCacheInfoCollectCpuCacheInfoData (
if (CacheInfoIndex == LocalCacheInfoCount) {

ASSERT (LocalCacheInfoCount < MaxCacheInfoCount);



- LocalCacheInfo[LocalCacheInfoCount].Package = ProcessorInfo[Index / MAX_NUM_OF_CACHE_PARAMS_LEAF].Package;

- LocalCacheInfo[LocalCacheInfoCount].CoreType = ProcessorInfo[Index / MAX_NUM_OF_CACHE_PARAMS_LEAF].CoreType;

- LocalCacheInfo[LocalCacheInfoCount].CacheLevel = CacheData[Index].CacheLevel;

- LocalCacheInfo[LocalCacheInfoCount].CacheType = CacheData[Index].CacheType;

- LocalCacheInfo[LocalCacheInfoCount].CacheWays = CacheData[Index].CacheWays;

- LocalCacheInfo[LocalCacheInfoCount].CacheSizeinKB = CacheData[Index].CacheSizeinKB;

- LocalCacheInfo[LocalCacheInfoCount].CacheCount = 1;

+ LocalCacheInfo[LocalCacheInfoCount].Package = ProcessorInfo[Index / MAX_NUM_OF_CACHE_PARAMS_LEAF].Package;

+ LocalCacheInfo[LocalCacheInfoCount].CoreType = ProcessorInfo[Index / MAX_NUM_OF_CACHE_PARAMS_LEAF].CoreType;

+ LocalCacheInfo[LocalCacheInfoCount].CacheLevel = CacheData[Index].CacheLevel;

+ LocalCacheInfo[LocalCacheInfoCount].CacheType = CacheData[Index].CacheType;

+ LocalCacheInfo[LocalCacheInfoCount].CacheWays = CacheData[Index].CacheWays;

+ LocalCacheInfo[LocalCacheInfoCount].FullyAssociativeCache = CacheData[Index].FullyAssociativeCache;

+ LocalCacheInfo[LocalCacheInfoCount].DirectMappedCache = CacheData[Index].DirectMappedCache;

+ LocalCacheInfo[LocalCacheInfoCount].CacheSizeinKB = CacheData[Index].CacheSizeinKB;

+ LocalCacheInfo[LocalCacheInfoCount].CacheCount = 1;



LocalCacheInfoCount++;

}

diff --git a/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h b/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h
index a7f29b188775..a66152bce009 100644
--- a/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h
+++ b/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h
@@ -1,7 +1,7 @@
/** @file

Header file for CPU Cache info Library.



- Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/

@@ -33,7 +33,18 @@ typedef struct {
// Ways of associativity.

// Value = CPUID.04h:EBX[31:22]

//

- UINT16 CacheWays;

+ UINT16 CacheWays : 10;

+ //

+ // Fully associative cache.

+ // Value = CPUID.04h:EAX[09]

+ //

+ UINT16 FullyAssociativeCache : 1;

+ //

+ // Direct mapped cache.

+ // Value = CPUID.04h:EDX[02]

+ //

+ UINT16 DirectMappedCache : 1;

+ UINT16 Reserved : 4;

//

// Size of single cache that this package's this type of logical processor corresponds to.

// Value = (CPUID.04h:EBX[31:22] + 1) * (CPUID.04h:EBX[21:12] + 1) *

diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
index de56db9c0cbe..b6e6ae5bc50a 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
@@ -1,7 +1,7 @@
/** @file

Internal header file for CPU Cache info Library.



- Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/

@@ -52,7 +52,18 @@ typedef struct {
// Ways of associativity.

// Value = CPUID.04h:EBX[31:22]

//

- UINT16 CacheWays;

+ UINT16 CacheWays : 10;

+ //

+ // Fully associative cache.

+ // Value = CPUID.04h:EAX[09]

+ //

+ UINT16 FullyAssociativeCache : 1;

+ //

+ // Direct mapped cache.

+ // Value = CPUID.04h:EDX[02]

+ //

+ UINT16 DirectMappedCache : 1;

+ UINT16 Reserved : 4;

//

// Cache share bits.

// Value = CPUID.04h:EAX[25:14]

--
2.28.0.windows.1


Re: [PATCH 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB

Dong, Eric
 

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

-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Tuesday, March 16, 2021 11:34 AM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Laszlo Ersek <lersek@...>; Kumar, Rahul1 <rahul1.kumar@...>
Subject: [PATCH 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB

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

GDT needs to be allocated below 4GB in 64bit environment
because AP needs it for entering to protected mode.
CPU running in big real mode cannot access above 4GB GDT.

But CpuDxe driver contains below code:
gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8);
.....
gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;

The AllocateRuntimePool() may allocate memory above 4GB.
Thus, we cannot use AllocateRuntimePool (), instead,
we should use AllocatePages() to make sure GDT is below 4GB space.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
UefiCpuPkg/CpuDxe/CpuGdt.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c
index 322ce87142..98d5551702 100644
--- a/UefiCpuPkg/CpuDxe/CpuGdt.c
+++ b/UefiCpuPkg/CpuDxe/CpuGdt.c
@@ -124,15 +124,26 @@ InitGlobalDescriptorTable (
VOID

)

{

+ EFI_STATUS Status;

GDT_ENTRIES *Gdt;

IA32_DESCRIPTOR Gdtr;

+ EFI_PHYSICAL_ADDRESS Memory;



//

- // Allocate Runtime Data for the GDT

- //

- Gdt = AllocateRuntimePool (sizeof (gGdtTemplate) + 8);

- ASSERT (Gdt != NULL);

- Gdt = ALIGN_POINTER (Gdt, 8);

+ // Allocate Runtime Data below 4GB for the GDT

+ // AP uses the same GDT when it's waken up from real mode so

+ // the GDT needs to be below 4GB.

+ //

+ Memory = SIZE_4GB - 1;

+ Status = gBS->AllocatePages (

+ AllocateMaxAddress,

+ EfiRuntimeServicesData,

+ EFI_SIZE_TO_PAGES (sizeof (gGdtTemplate)),

+ &Memory

+ );

+ ASSERT_EFI_ERROR (Status);

+ ASSERT ((Memory != 0) && (Memory < SIZE_4GB));

+ Gdt = (GDT_ENTRIES *) (UINTN) Memory;



//

// Initialize all GDT entries

--
2.27.0.windows.1


Re: [PATCH 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard

Dong, Eric
 

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

-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Tuesday, March 16, 2021 11:34 AM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Laszlo Ersek <lersek@...>; Kumar, Rahul1 <rahul1.kumar@...>
Subject: [PATCH 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard

The change doesn't impact any functionality.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
UefiCpuPkg/CpuDxe/CpuGdt.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c
index a1ab543f2d..322ce87142 100644
--- a/UefiCpuPkg/CpuDxe/CpuGdt.c
+++ b/UefiCpuPkg/CpuDxe/CpuGdt.c
@@ -2,7 +2,7 @@
C based implementation of IA32 interrupt handling only

requiring a minimal assembly interrupt entry point.



- Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/

@@ -13,7 +13,7 @@
//

// Global descriptor table (GDT) Template

//

-STATIC GDT_ENTRIES GdtTemplate = {

+STATIC GDT_ENTRIES gGdtTemplate = {

//

// NULL_SEL

//

@@ -124,32 +124,31 @@ InitGlobalDescriptorTable (
VOID

)

{

- GDT_ENTRIES *gdt;

- IA32_DESCRIPTOR gdtPtr;

+ GDT_ENTRIES *Gdt;

+ IA32_DESCRIPTOR Gdtr;



//

// Allocate Runtime Data for the GDT

//

- gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8);

- ASSERT (gdt != NULL);

- gdt = ALIGN_POINTER (gdt, 8);

+ Gdt = AllocateRuntimePool (sizeof (gGdtTemplate) + 8);

+ ASSERT (Gdt != NULL);

+ Gdt = ALIGN_POINTER (Gdt, 8);



//

// Initialize all GDT entries

//

- CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate));

+ CopyMem (Gdt, &gGdtTemplate, sizeof (gGdtTemplate));



//

// Write GDT register

//

- gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;

- gdtPtr.Limit = (UINT16) (sizeof (GdtTemplate) - 1);

- AsmWriteGdtr (&gdtPtr);

+ Gdtr.Base = (UINT32) (UINTN) Gdt;

+ Gdtr.Limit = (UINT16) (sizeof (gGdtTemplate) - 1);

+ AsmWriteGdtr (&Gdtr);



//

// Update selector (segment) registers base on new GDT

//

- SetCodeSelector ((UINT16)CPU_CODE_SEL);

- SetDataSelectors ((UINT16)CPU_DATA_SEL);

+ SetCodeSelector ((UINT16) CPU_CODE_SEL);

+ SetDataSelectors ((UINT16) CPU_DATA_SEL);

}

-

--
2.27.0.windows.1


Re: [edk2-platforms] Intel/MinPlatformPkg: resolve MmUnblockMemoryLib

Zhiguang Liu
 

Hi Kun,
Your patch is ok for me. Please keep this change in your patch series.

Hi All,
Please ignore this patch since the same solution is already included in Kun's patch serials.

Thanks
Zhiguang

-----Original Message-----
From: Kun Qin <kuqin12@...>
Sent: Tuesday, March 16, 2021 5:18 PM
To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@...>
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@...>; Chiu, Chasel
<chasel.chiu@...>; Liming Gao <gaoliming@...>; Dong,
Eric <eric.dong@...>
Subject: Re: [edk2-devel] [edk2-platforms] Intel/MinPlatformPkg: resolve
MmUnblockMemoryLib

Hi Zhiguang,

I have already sent this patch series to resolve dependencies in edk2-
platform (although the change is slightly different):
https://edk2.groups.io/g/devel/message/72645

Specifically:
https://edk2.groups.io/g/devel/message/72646

Could you please let me know if the change above resolves the dependency
issue for you?

The patch series have 2 other patches not being reviewed yet. I plan to send
out another round tomorrow to include reviewed-by tags. But please let me
know if you prefer your change below to check in first, I will just drop my
patch #1 when sending v2.

Thanks,
Kun

On 03/16/2021 02:06, Zhiguang Liu wrote:
The below Edk2 patch makes VariableSmmRuntimeDxe begin to consume
MmUnblockMemoryLib.
It cause multiple platforms build failure.
f463dbadede138dc96a66dae6f361c54f0b3093c
MdeModulePkg: VariableSmmRuntimeDxe: Added request unblock
memory
interface

This change added NULL MmUnblockMemoryLib instance in
MinPlatformPkg
dsc include files

Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>

Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc | 1 +
1 file changed, 1 insertion(+)

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
index fa9098d525..ee91dd8bd6 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
@@ -116,6 +116,7 @@
!endif

BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf


VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic
yLibRuntimeDxe.inf

+
+
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblo
ckMemory
+ LibNull.inf



[LibraryClasses.common.UEFI_DRIVER]


Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.
inf


Re: [PATCH v9 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:

@@ -214,6 +243,83 @@ EjectCpu (
{
UINT64 QemuSelector;

+ if (CheckIfBsp ()) {
+ UINT32 Idx;
+
+ for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) {
+ UINT64 QemuSelector;
Visual Studio warns that the inner QemuSelector declaration shadows the
outer one. I'm going to remove the inner declaration, due to:

+
+ QemuSelector = mCpuHotEjectData->QemuSelectorMap[Idx];
+
+ if (QemuSelector != CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ //
+ // This to-be-ejected-CPU has already received the BSP's SMI exit
+ // signal and will execute SmmCpuFeaturesRendezvousExit()
+ // followed by this callback or is already penned in the
+ // CpuSleep() loop below.
+ //
+ // Tell QEMU to context-switch it out.
+ //
+ QemuCpuhpWriteCpuSelector (mMmCpuIo, (UINT32) QemuSelector);
+ QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECT);
+
+ //
+ // Now that we've ejected the CPU corresponding to QemuSelectorMap[Idx],
+ // clear its eject status to ensure that an invalid future SMI does
+ // not end up trying a spurious eject or a newly hotplugged CPU does
+ // not get penned in the CpuSleep() loop.
+ //
+ // Note that the QemuCpuhpWriteCpuStatus() command above is a write to
+ // a different address space and uses the EFI_MM_CPU_IO_PROTOCOL.
+ //
+ // This means that we are guaranteed that the following assignment
+ // will not be reordered before the eject. And, so we can safely
+ // do this write here.
+ //
+ mCpuHotEjectData->QemuSelectorMap[Idx] =
+ CPU_EJECT_QEMU_SELECTOR_INVALID;
+
+ DEBUG ((DEBUG_INFO, "%a: Unplugged ProcessorNum %u, "
+ "QemuSelector %Lu\n", __FUNCTION__, Idx, QemuSelector));
+ }
+ }
+
+ //
+ // We are done until the next hot-unplug; clear the handler.
+ //
+ // mCpuHotEjectData->Handler is a NOP for any CPU not under ejection.
+ // So, once we are done with all the ejections, we can safely reset it
+ // here since any CPU dereferencing it would only see either the old
+ // or the new value (since it is aligned at a natural boundary.)
+ //
+ mCpuHotEjectData->Handler = NULL;
+ return;
+ }
+
+ //
+ // Reached only on APs
+ //
+
+ //
+ // mCpuHotEjectData->QemuSelectorMap[ProcessorNum] is updated
+ // on the BSP in the ongoing SMI at two places:
+ //
+ // - UnplugCpus() where the BSP determines if a CPU is under ejection
+ // or not. As a comment in UnplugCpus() at set-up, and in
+ // SmmCpuFeaturesRendezvousExit() where it is dereferenced describe,
+ // any such updates are guaranteed to be ordered-before the
+ // dereference below.
+ //
+ // - EjectCpu() on the BSP (above) updates QemuSelectorMap[ProcessorNum]
+ // for a CPU once it's ejected.
+ //
+ // The CPU under ejection: might be executing anywhere between the
+ // AllCpusInSync loop in SmiRendezvous(), to about to dereference
+ // QemuSelectorMap[ProcessorNum].
+ // As described in the comment above where we do the reset, this
+ // is not a problem since the ejected CPU never sees the after value.
+ // CPUs not-under ejection: never see any changes so they are fine.
+ //
QemuSelector = mCpuHotEjectData->QemuSelectorMap[ProcessorNum];
this reference being to the outer one.

Thanks
Laszlo

if (QemuSelector == CPU_EJECT_QEMU_SELECTOR_INVALID) {
return;
@@ -495,11 +601,6 @@ CpuHotplugMmi (
if (EFI_ERROR (Status)) {
goto Fatal;
}
- if (ToUnplugCount > 0) {
- DEBUG ((DEBUG_ERROR, "%a: hot-unplug is not supported yet\n",
- __FUNCTION__));
- goto Fatal;
- }

if (PluggedCount > 0) {
Status = ProcessHotAddedCpus (mPluggedApicIds, PluggedCount);


Re: [PATCH v9 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Advertise OVMF support for CPU hot-unplug and negotiate it
if QEMU requests the feature.

Cc: Laszlo Ersek <lersek@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Igor Mammedov <imammedo@...>
Cc: Boris Ostrovsky <boris.ostrovsky@...>
Cc: Aaron Young <aaron.young@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
---

Notes:
Addresses the following comments from v8:

(1) Remove inconsistent comment style (and stray newline) around the newly
added ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.
(2) Remove spurious empty line.

OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index c9d875543205..3e2e61e4dbd0 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -28,7 +28,12 @@
// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files.
//
#define ICH9_LPC_SMI_F_CPU_HOTPLUG BIT1
-
+//
+// The following bit value stands for "enable CPU hot-unplug, and inject an SMI
+// with control value ICH9_APM_CNT_CPU_HOTPLUG upon hot-unplug", in the
+// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files.
+//
+#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG BIT2
Per my v8 comment, this hunk is not supposed to change the amount of
vertical whitespace, in either direction. v8 added an empty line that
wasn't called for, and this version removes one, which is also unjustified.

But I'll fix that up when I merge this series (= soon).

Reviewed-by: Laszlo Ersek <lersek@...>

Thanks!
Laszlo

//
// Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
// for the S3 boot script fragment to write to and read from.
@@ -112,7 +117,8 @@ NegotiateSmiFeatures (
QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures);

//
- // We want broadcast SMI, SMI on CPU hotplug, and nothing else.
+ // We want broadcast SMI, SMI on CPU hotplug, SMI on CPU hot-unplug
+ // and nothing else.
//
RequestedFeaturesMask = ICH9_LPC_SMI_F_BROADCAST;
if (!MemEncryptSevIsEnabled ()) {
@@ -120,6 +126,7 @@ NegotiateSmiFeatures (
// For now, we only support hotplug with SEV disabled.
//
RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOTPLUG;
+ RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOT_UNPLUG;
}
mSmiFeatures &= RequestedFeaturesMask;
QemuFwCfgSelectItem (mRequestedFeaturesItem);
@@ -166,6 +173,13 @@ NegotiateSmiFeatures (
__FUNCTION__));
}

+ if ((mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOT_UNPLUG) == 0) {
+ DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug not negotiated\n", __FUNCTION__));
+ } else {
+ DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug with SMI negotiated\n",
+ __FUNCTION__));
+ }
+
//
// Negotiation successful (although we may not have gotten the optimal
// feature set).


Re: [PATCH v9 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Add logic in EjectCpu() to do the actual the CPU ejection.

On the BSP, ejection happens by first selecting the CPU via
its QemuSelector and then sending the QEMU "eject" command.
QEMU in-turn signals the remote VCPU thread which context-switches
the CPU out of the SMI handler.

Meanwhile the CPU being ejected, waits around in its holding
area until it is context-switched out. Note that it is possible
that a slow CPU gets ejected before it reaches the wait loop.
However, this would never happen before it has executed the
"AllCpusInSync" loop in SmiRendezvous().
It can mean that an ejected CPU does not execute code after
that point but given that the CPU state will be destroyed by
QEMU, the missed cleanup is no great loss.

Cc: Laszlo Ersek <lersek@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Igor Mammedov <imammedo@...>
Cc: Boris Ostrovsky <boris.ostrovsky@...>
Cc: Aaron Young <aaron.young@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
---

Notes:
Addresses the following comments from v8:

(1a,1b) CheckIfBsp(): get rid of ProcessorNum, document retval.
(2) Line up IsBsp and ApicBaseMsr
(3) s/ongoing SMI iteration/ongoing SMI/
(4) Get rid of the allusions to alignment in the comment in EjectCpu().
() Also reduce some of the repetitive detail in this comment.
(5) EjectCpu(): reorder logic to cleanly separate the AP and the BSP portions.
(6) Get rid of unnecessary MemoryFence() between QemuCpuhpWrite
and clearing of the eject status.
(7) Change type of QemuSelector to %Lu in DEBUG statement
(8) Get rid of the repetitive comment in SmmCpuFeaturesRendezvousExit().
The necessary parts of this got moved to patch-7.

OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 1 +
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 113 ++++++++++++++++++++--
2 files changed, 108 insertions(+), 6 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@...>

Thanks
Laszlo


diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
index 2ec7a107a64d..d0e83102c13f 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -34,6 +34,7 @@
#define QEMU_CPUHP_STAT_ENABLED BIT0
#define QEMU_CPUHP_STAT_INSERT BIT1
#define QEMU_CPUHP_STAT_REMOVE BIT2
+#define QEMU_CPUHP_STAT_EJECT BIT3
#define QEMU_CPUHP_STAT_FW_REMOVE BIT4

#define QEMU_CPUHP_RW_CMD_DATA 0x8
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 2eeb4567a262..ae3abd525900 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -18,6 +18,7 @@
#include <Pcd/CpuHotEjectData.h> // CPU_HOT_EJECT_DATA
#include <Protocol/MmCpuIo.h> // EFI_MM_CPU_IO_PROTOCOL
#include <Protocol/SmmCpuService.h> // EFI_SMM_CPU_SERVICE_PROTOCOL
+#include <Register/Intel/ArchitecturalMsr.h> // MSR_IA32_APIC_BASE_REGISTER
#include <Uefi/UefiBaseType.h> // EFI_STATUS

#include "ApicId.h" // APIC_ID
@@ -193,12 +194,40 @@ RevokeNewSlot:
}

/**
+ EjectCpu needs to know the BSP at SMI exit at a point when
+ some of the EFI_SMM_CPU_SERVICE_PROTOCOL state has been torn
+ down.
+ Reuse the logic from OvmfPkg::PlatformSmmBspElection() to
+ do that.
+
+ @retval TRUE If the CPU executing this function is the BSP.
+
+ @retval FALSE If the CPU executing this function is an AP.
+**/
+STATIC
+BOOLEAN
+CheckIfBsp (
+ VOID
+ )
+{
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;
+ BOOLEAN IsBsp;
+
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ IsBsp = (BOOLEAN)(ApicBaseMsr.Bits.BSP == 1);
+ return IsBsp;
+}
+
+/**
CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
on each CPU at exit from SMM.

- If, the executing CPU is not being ejected, nothing to be done.
+ If, the executing CPU is neither the BSP, nor being ejected, nothing
+ to be done.
If, the executing CPU is being ejected, wait in a halted loop
until ejected.
+ If, the executing CPU is the BSP, set QEMU CPU status to eject
+ for CPUs being ejected.

@param[in] ProcessorNum ProcessorNum denotes the CPU exiting SMM,
and will be used as an index into
@@ -214,6 +243,83 @@ EjectCpu (
{
UINT64 QemuSelector;

+ if (CheckIfBsp ()) {
+ UINT32 Idx;
+
+ for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) {
+ UINT64 QemuSelector;
+
+ QemuSelector = mCpuHotEjectData->QemuSelectorMap[Idx];
+
+ if (QemuSelector != CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ //
+ // This to-be-ejected-CPU has already received the BSP's SMI exit
+ // signal and will execute SmmCpuFeaturesRendezvousExit()
+ // followed by this callback or is already penned in the
+ // CpuSleep() loop below.
+ //
+ // Tell QEMU to context-switch it out.
+ //
+ QemuCpuhpWriteCpuSelector (mMmCpuIo, (UINT32) QemuSelector);
+ QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECT);
+
+ //
+ // Now that we've ejected the CPU corresponding to QemuSelectorMap[Idx],
+ // clear its eject status to ensure that an invalid future SMI does
+ // not end up trying a spurious eject or a newly hotplugged CPU does
+ // not get penned in the CpuSleep() loop.
+ //
+ // Note that the QemuCpuhpWriteCpuStatus() command above is a write to
+ // a different address space and uses the EFI_MM_CPU_IO_PROTOCOL.
+ //
+ // This means that we are guaranteed that the following assignment
+ // will not be reordered before the eject. And, so we can safely
+ // do this write here.
+ //
+ mCpuHotEjectData->QemuSelectorMap[Idx] =
+ CPU_EJECT_QEMU_SELECTOR_INVALID;
+
+ DEBUG ((DEBUG_INFO, "%a: Unplugged ProcessorNum %u, "
+ "QemuSelector %Lu\n", __FUNCTION__, Idx, QemuSelector));
+ }
+ }
+
+ //
+ // We are done until the next hot-unplug; clear the handler.
+ //
+ // mCpuHotEjectData->Handler is a NOP for any CPU not under ejection.
+ // So, once we are done with all the ejections, we can safely reset it
+ // here since any CPU dereferencing it would only see either the old
+ // or the new value (since it is aligned at a natural boundary.)
+ //
+ mCpuHotEjectData->Handler = NULL;
+ return;
+ }
+
+ //
+ // Reached only on APs
+ //
+
+ //
+ // mCpuHotEjectData->QemuSelectorMap[ProcessorNum] is updated
+ // on the BSP in the ongoing SMI at two places:
+ //
+ // - UnplugCpus() where the BSP determines if a CPU is under ejection
+ // or not. As a comment in UnplugCpus() at set-up, and in
+ // SmmCpuFeaturesRendezvousExit() where it is dereferenced describe,
+ // any such updates are guaranteed to be ordered-before the
+ // dereference below.
+ //
+ // - EjectCpu() on the BSP (above) updates QemuSelectorMap[ProcessorNum]
+ // for a CPU once it's ejected.
+ //
+ // The CPU under ejection: might be executing anywhere between the
+ // AllCpusInSync loop in SmiRendezvous(), to about to dereference
+ // QemuSelectorMap[ProcessorNum].
+ // As described in the comment above where we do the reset, this
+ // is not a problem since the ejected CPU never sees the after value.
+ // CPUs not-under ejection: never see any changes so they are fine.
+ //
QemuSelector = mCpuHotEjectData->QemuSelectorMap[ProcessorNum];
if (QemuSelector == CPU_EJECT_QEMU_SELECTOR_INVALID) {
return;
@@ -495,11 +601,6 @@ CpuHotplugMmi (
if (EFI_ERROR (Status)) {
goto Fatal;
}
- if (ToUnplugCount > 0) {
- DEBUG ((DEBUG_ERROR, "%a: hot-unplug is not supported yet\n",
- __FUNCTION__));
- goto Fatal;
- }

if (PluggedCount > 0) {
Status = ProcessHotAddedCpus (mPluggedApicIds, PluggedCount);


Re: [PATCH v9 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu()

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Add EjectCpu(), which handles the CPU ejection, and provides a holding
area for said CPUs. It is called via SmmCpuFeaturesRendezvousExit(),
at the tail end of the SMI handling.

Also UnplugCpus() now stashes QEMU Selectors of CPUs which need to be
ejected in CPU_HOT_EJECT_DATA.QemuSelectorMap. This is used by
EjectCpu() to identify CPUs marked for ejection.

Cc: Laszlo Ersek <lersek@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Igor Mammedov <imammedo@...>
Cc: Boris Ostrovsky <boris.ostrovsky@...>
Cc: Aaron Young <aaron.young@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
---

Notes:
Addresses the following comments from v8:

(1) Fixup the coment about UnplugCpus() to reference stashing QEMU
Cpu Selectors instead of APIC IDs.
(2) s/ToUnplugSelector/ToUnplugSelectors/
(3) Use plural for APIC ID in comment describing retval EFI_ALREADY_STARTED.
(4) Fixup indentation in check against CPU_EJECT_QEMU_SELECTOR_INVALID.
(5) Clarify comment:
- // never match more than one APIC ID and by transitivity, more than one
- // QemuSelector in a single invocation of UnplugCpus().
+ // never match more than one APIC ID -- nor, by transitivity, designate
+ // more than one QemuSelector -- in a single invocation of UnplugCpus().
(6a) Remove unnecessary UINT64 cast for mCpuHotEjectData->QemuSelectorMap[ProcessorNum].
(6b) Switch from 0x%Lx -> %Lu for QemuSelectorMap[ProcessorNum].
(6c) Switch from 0x%Lx -> %u for QemuSelector
(7) Switch to "return EFI_ALREADY_STARTED".
(8a) Replace "QemuSelector 0x%Lx" with "QemuSelector %u".
(8b) Replace the mCpuHotEjectData->QemuSelectorMap[ProcessorNum] argument
with just QemuSelector in DEBUG call.
(9) Clarify comment and make the language complementary to that in patch-7
Explicitly mention release memory fence.

OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf | 2 +
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 154 ++++++++++++++++++++++++++++++--
2 files changed, 148 insertions(+), 8 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@...>

Thanks
Laszlo

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
index 04322b0d7855..ebcc7e2ac63a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
@@ -40,6 +40,7 @@ [Packages]
[LibraryClasses]
BaseLib
BaseMemoryLib
+ CpuLib
DebugLib
LocalApicLib
MmServicesTableLib
@@ -54,6 +55,7 @@ [Protocols]

[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress ## CONSUMES
+ gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress ## CONSUMES
gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase ## CONSUMES

[FeaturePcd]
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 59f000eb7886..2eeb4567a262 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -10,10 +10,12 @@
#include <IndustryStandard/Q35MchIch9.h> // ICH9_APM_CNT
#include <IndustryStandard/QemuCpuHotplug.h> // QEMU_CPUHP_CMD_GET_PENDING
#include <Library/BaseLib.h> // CpuDeadLoop()
+#include <Library/CpuLib.h> // CpuSleep()
#include <Library/DebugLib.h> // ASSERT()
#include <Library/MmServicesTableLib.h> // gMmst
#include <Library/PcdLib.h> // PcdGetBool()
#include <Library/SafeIntLib.h> // SafeUintnSub()
+#include <Pcd/CpuHotEjectData.h> // CPU_HOT_EJECT_DATA
#include <Protocol/MmCpuIo.h> // EFI_MM_CPU_IO_PROTOCOL
#include <Protocol/SmmCpuService.h> // EFI_SMM_CPU_SERVICE_PROTOCOL
#include <Uefi/UefiBaseType.h> // EFI_STATUS
@@ -32,11 +34,12 @@ STATIC EFI_MM_CPU_IO_PROTOCOL *mMmCpuIo;
//
STATIC EFI_SMM_CPU_SERVICE_PROTOCOL *mMmCpuService;
//
-// This structure is a communication side-channel between the
+// These structures serve as communication side-channels between the
// EFI_SMM_CPU_SERVICE_PROTOCOL consumer (i.e., this driver) and provider
// (i.e., PiSmmCpuDxeSmm).
//
STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData;
+STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData;
//
// SMRAM arrays for fetching the APIC IDs of processors with pending events (of
// known event types), for the time of just one MMI.
@@ -190,18 +193,71 @@ RevokeNewSlot:
}

/**
+ CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
+ on each CPU at exit from SMM.
+
+ If, the executing CPU is not being ejected, nothing to be done.
+ If, the executing CPU is being ejected, wait in a halted loop
+ until ejected.
+
+ @param[in] ProcessorNum ProcessorNum denotes the CPU exiting SMM,
+ and will be used as an index into
+ CPU_HOT_EJECT_DATA->QemuSelectorMap. It is
+ identical to the processor handle number in
+ EFI_SMM_CPU_SERVICE_PROTOCOL.
+**/
+VOID
+EFIAPI
+EjectCpu (
+ IN UINTN ProcessorNum
+ )
+{
+ UINT64 QemuSelector;
+
+ QemuSelector = mCpuHotEjectData->QemuSelectorMap[ProcessorNum];
+ if (QemuSelector == CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ return;
+ }
+
+ //
+ // APs being unplugged get here from SmmCpuFeaturesRendezvousExit()
+ // after having been cleared to exit the SMI and so have no SMM
+ // processing remaining.
+ //
+ // Keep them penned here until the BSP tells QEMU to eject them.
+ //
+ for (;;) {
+ DisableInterrupts ();
+ CpuSleep ();
+ }
+}
+
+/**
Process to be hot-unplugged CPUs, per QemuCpuhpCollectApicIds().

For each such CPU, report the CPU to PiSmmCpuDxeSmm via
- EFI_SMM_CPU_SERVICE_PROTOCOL. If the to be hot-unplugged CPU is
- unknown, skip it silently.
+ EFI_SMM_CPU_SERVICE_PROTOCOL and stash the QEMU Cpu Selectors for later
+ ejection. If the to be hot-unplugged CPU is unknown, skip it silently.
+
+ Additonally, if we do stash any Cpu Selectors, also install a CPU eject
+ handler which would handle the ejection.

@param[in] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
hot-unplugged.

+ @param[in] ToUnplugSelectors The QEMU Selectors of the CPUs that are about to
+ be hot-unplugged.
+
@param[in] ToUnplugCount The number of filled-in APIC IDs in
ToUnplugApicIds.

+ @retval EFI_ALREADY_STARTED For the ProcessorNum that
+ EFI_SMM_CPU_SERVICE_PROTOCOL had assigned to
+ one of the APIC IDs in ToUnplugApicIds,
+ mCpuHotEjectData->QemuSelectorMap already has
+ the QemuSelector value stashed. (This should
+ never happen.)
+
@retval EFI_SUCCESS Known APIC IDs have been removed from SMM data
structures.

@@ -212,23 +268,36 @@ STATIC
EFI_STATUS
UnplugCpus (
IN APIC_ID *ToUnplugApicIds,
+ IN UINT32 *ToUnplugSelectors,
IN UINT32 ToUnplugCount
)
{
EFI_STATUS Status;
UINT32 ToUnplugIdx;
+ UINT32 EjectCount;
UINTN ProcessorNum;

ToUnplugIdx = 0;
+ EjectCount = 0;
while (ToUnplugIdx < ToUnplugCount) {
APIC_ID RemoveApicId;
+ UINT32 QemuSelector;

RemoveApicId = ToUnplugApicIds[ToUnplugIdx];
+ QemuSelector = ToUnplugSelectors[ToUnplugIdx];

//
- // mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use it to find
- // the ProcessorNum for the APIC ID to be removed.
+ // mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use RemoveApicId
+ // to find the corresponding ProcessorNum for the CPU to be removed.
//
+ // With this we can establish a 3 way mapping:
+ // APIC_ID -- ProcessorNum -- QemuSelector
+ //
+ // We stash the ProcessorNum -> QemuSelector mapping so it can later be
+ // used for CPU hot-eject in SmmCpuFeaturesRendezvousExit() context (where
+ // we only have ProcessorNum available.)
+ //
+
for (ProcessorNum = 0;
ProcessorNum < mCpuHotPlugData->ArrayLength;
ProcessorNum++) {
@@ -257,11 +326,62 @@ UnplugCpus (
return Status;
}

+ if (mCpuHotEjectData->QemuSelectorMap[ProcessorNum] !=
+ CPU_EJECT_QEMU_SELECTOR_INVALID) {
+ //
+ // mCpuHotEjectData->QemuSelectorMap[ProcessorNum] is set to
+ // CPU_EJECT_QEMU_SELECTOR_INVALID when mCpuHotEjectData->QemuSelectorMap
+ // is allocated, and once the subject processsor is ejected.
+ //
+ // Additionally, mMmCpuService->RemoveProcessor(ProcessorNum) invalidates
+ // mCpuHotPlugData->ApicId[ProcessorNum], so a given ProcessorNum can
+ // never match more than one APIC ID -- nor, by transitivity, designate
+ // more than one QemuSelector -- in a single invocation of UnplugCpus().
+ //
+ DEBUG ((DEBUG_ERROR, "%a: ProcessorNum %Lu maps to QemuSelector %Lu, "
+ "cannot also map to %u\n", __FUNCTION__, (UINT64)ProcessorNum,
+ mCpuHotEjectData->QemuSelectorMap[ProcessorNum], QemuSelector));
+
+ return EFI_ALREADY_STARTED;
+ }
+
+ //
+ // Stash the QemuSelector so we can do the actual ejection later.
+ //
+ mCpuHotEjectData->QemuSelectorMap[ProcessorNum] = (UINT64)QemuSelector;
+
+ DEBUG ((DEBUG_INFO, "%a: Started hot-unplug on ProcessorNum %Lu, APIC ID "
+ FMT_APIC_ID ", QemuSelector %u\n", __FUNCTION__, (UINT64)ProcessorNum,
+ RemoveApicId, QemuSelector));
+
+ EjectCount++;
ToUnplugIdx++;
}

+ if (EjectCount != 0) {
+ //
+ // We have processors to be ejected; install the handler.
+ //
+ mCpuHotEjectData->Handler = EjectCpu;
+
+ //
+ // The BSP and APs load mCpuHotEjectData->Handler, and
+ // mCpuHotEjectData->QemuSelectorMap[] in SmmCpuFeaturesRendezvousExit()
+ // and EjectCpu().
+ //
+ // The comment in SmmCpuFeaturesRendezvousExit() details how we use
+ // the AllCpusInSync control-dependency to ensure that any loads are
+ // ordered-after the stores above.
+ //
+ // Ensure that the stores above are ordered-before the AllCpusInSync store
+ // by using a MemoryFence() with release semantics.
+ //
+ MemoryFence ();
+ }
+
//
- // We've removed this set of APIC IDs from SMM data structures.
+ // We've removed this set of APIC IDs from SMM data structures and
+ // have installed an ejection handler if needed.
//
return EFI_SUCCESS;
}
@@ -389,7 +509,7 @@ CpuHotplugMmi (
}

if (ToUnplugCount > 0) {
- Status = UnplugCpus (mToUnplugApicIds, ToUnplugCount);
+ Status = UnplugCpus (mToUnplugApicIds, mToUnplugSelectors, ToUnplugCount);
if (EFI_ERROR (Status)) {
goto Fatal;
}
@@ -460,9 +580,14 @@ CpuHotplugEntry (

//
// Our DEPEX on EFI_SMM_CPU_SERVICE_PROTOCOL guarantees that PiSmmCpuDxeSmm
- // has pointed PcdCpuHotPlugDataAddress to CPU_HOT_PLUG_DATA in SMRAM.
+ // has pointed:
+ // - PcdCpuHotPlugDataAddress to CPU_HOT_PLUG_DATA in SMRAM,
+ // - PcdCpuHotEjectDataAddress to CPU_HOT_EJECT_DATA in SMRAM, if the
+ // possible CPU count is greater than 1.
//
mCpuHotPlugData = (VOID *)(UINTN)PcdGet64 (PcdCpuHotPlugDataAddress);
+ mCpuHotEjectData = (VOID *)(UINTN)PcdGet64 (PcdCpuHotEjectDataAddress);
+
if (mCpuHotPlugData == NULL) {
Status = EFI_NOT_FOUND;
DEBUG ((DEBUG_ERROR, "%a: CPU_HOT_PLUG_DATA: %r\n", __FUNCTION__, Status));
@@ -474,6 +599,19 @@ CpuHotplugEntry (
if (mCpuHotPlugData->ArrayLength == 1) {
return EFI_UNSUPPORTED;
}
+
+ if (mCpuHotEjectData == NULL) {
+ Status = EFI_NOT_FOUND;
+ } else if (mCpuHotPlugData->ArrayLength != mCpuHotEjectData->ArrayLength) {
+ Status = EFI_INVALID_PARAMETER;
+ } else {
+ Status = EFI_SUCCESS;
+ }
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: CPU_HOT_EJECT_DATA: %r\n", __FUNCTION__, Status));
+ goto Fatal;
+ }
+
//
// Allocate the data structures that depend on the possible CPU count.
//


Re: [PATCH v9 07/10] OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Call the CPU hot-eject handler if one is installed. The condition for
installation is (PcdCpuMaxLogicalProcessorNumber > 1), and there's
a hot-unplug request.

The handler is called from SmmCpuFeaturesRendezvousExit(), which is
in-turn called at the tail-end of SmiRendezvous() after the BSP has
signalled an SMI exit via the "AllCpusInSync" loop.

Cc: Laszlo Ersek <lersek@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Igor Mammedov <imammedo@...>
Cc: Boris Ostrovsky <boris.ostrovsky@...>
Cc: Aaron Young <aaron.young@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
---

Notes:
Addresses the following comments from v8:

(1) Add a MemoryFence() before accessing mCpuHotEjctData->Handler
(and comment to that effect.)

.../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 34 ++++++++++++++++++++++
1 file changed, 34 insertions(+)
Reviewed-by: Laszlo Ersek <lersek@...>

Thanks
Laszlo

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 5c025bc717c3..fdf2380974fa 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -452,6 +452,40 @@ SmmCpuFeaturesRendezvousExit (
IN UINTN CpuIndex
)
{
+ //
+ // We only call the Handler if CPU hot-eject is enabled
+ // (PcdCpuMaxLogicalProcessorNumber > 1), and hot-eject is needed
+ // in this SMI exit (otherwise mCpuHotEjectData->Handler is not armed.)
+ //
+
+ if (mCpuHotEjectData != NULL) {
+ CPU_HOT_EJECT_HANDLER Handler;
+
+ //
+ // As the comment above mentions, mCpuHotEjectData->Handler might be
+ // written to on the BSP as part of handling of the CPU-ejection.
+ //
+ // We know that any initial assignment to mCpuHotEjectData->Handler
+ // (on the BSP, in the CpuHotplugMmi() context) is ordered-before the
+ // load below, since it is guaranteed to happen before the
+ // control-dependency of the BSP's SMI exit signal -- by way of a store
+ // to AllCpusInSync (on the BSP, in BspHandler()) and the corresponding
+ // AllCpusInSync loop (on the APs, in SmiRendezvous()) which depends on
+ // that store.
+ //
+ // This guarantees that these pieces of code can never execute
+ // simultaneously. In addition, we ensure that the following load is
+ // ordered-after the AllCpusInSync loop by using a MemoryFence() with
+ // acquire semantics.
+ //
+ MemoryFence();
+
+ Handler = mCpuHotEjectData->Handler;
+
+ if (Handler != NULL) {
+ Handler (CpuIndex);
+ }
+ }
}

/**


Re: [PATCH v9 06/10] OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Init CPU_HOT_EJECT_DATA, which will be used to share CPU ejection
state between SmmCpuFeaturesLib (via PiSmmCpuDxeSmm) and CpuHotPlugSmm.

The init happens via SmmCpuFeaturesSmmRelocationComplete(), and so it
will run as part of the PiSmmCpuDxeSmm entry point function,
PiCpuSmmEntry(). Once inited, CPU_HOT_EJECT_DATA is exposed via
PcdCpuHotEjectDataAddress.

The CPU hot-eject handler (CPU_HOT_EJECT_DATA->Handler) is setup when
there is an ejection request via CpuHotplugSmm.

Cc: Laszlo Ersek <lersek@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Igor Mammedov <imammedo@...>
Cc: Boris Ostrovsky <boris.ostrovsky@...>
Cc: Aaron Young <aaron.young@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
---

Notes:
Addresses the following comments from v8:

(1) Remove line before the "if (MaxNumberofCpus == 1)" check.
(3) Fixup the space around "||".
(2,6) Simplify the three SafeInt multiplication into the ones suggested
by Laszlo.
(4) Get rid of the mixed sizeof(mCpuHotEjectData->QemuSelectorMap[0]) and
sizeof(UINT64) in favour of UINT64 everywhere. I was planning to use
the first, but describing the alignment needed is easier in terms of the
second.
Also, as Laszlo's comments on v8-patch-9 mention, we don't really need
this alignment for correctness reasons. This patch retains it, so we
don't pay access penalty for unaligned access.
(5) Change alignment from UINT64 to UINT64-1.
(7) Use the more idiomatic ALIGN_POINTER instead of ALIGN_VALUE.
(8) RETURN_ERROR -> ASSERT_RETURN_ERROR.

.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 ++
.../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 77 ++++++++++++++++++++++
2 files changed, 81 insertions(+)
Reviewed-by: Laszlo Ersek <lersek@...>

Thanks!
Laszlo


diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 97a10afb6e27..8a426a4c10fb 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -30,9 +30,13 @@ [LibraryClasses]
BaseMemoryLib
DebugLib
MemEncryptSevLib
+ MemoryAllocationLib
PcdLib
+ SafeIntLib
SmmServicesTableLib
UefiBootServicesTableLib

[Pcd]
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+ gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress
gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 7ef7ed98342e..5c025bc717c3 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -11,10 +11,13 @@
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/MemEncryptSevLib.h>
+#include <Library/MemoryAllocationLib.h>
#include <Library/PcdLib.h>
+#include <Library/SafeIntLib.h>
#include <Library/SmmCpuFeaturesLib.h>
#include <Library/SmmServicesTableLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Pcd/CpuHotEjectData.h>
#include <PiSmm.h>
#include <Register/Intel/SmramSaveStateMap.h>
#include <Register/QemuSmramSaveStateMap.h>
@@ -171,6 +174,77 @@ SmmCpuFeaturesHookReturnFromSmm (
return OriginalInstructionPointer;
}

+STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData = NULL;
+
+/**
+ Initialize mCpuHotEjectData if PcdCpuMaxLogicalProcessorNumber > 1.
+
+ Also setup the corresponding PcdCpuHotEjectDataAddress.
+**/
+STATIC
+VOID
+InitCpuHotEjectData (
+ VOID
+ )
+{
+ UINTN Size;
+ UINT32 Idx;
+ UINT32 MaxNumberOfCpus;
+ RETURN_STATUS PcdStatus;
+
+ MaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+ if (MaxNumberOfCpus == 1) {
+ return;
+ }
+
+ //
+ // We allocate CPU_HOT_EJECT_DATA and CPU_HOT_EJECT_DATA->QemuSelectorMap[]
+ // in a single allocation, and explicitly align the QemuSelectorMap[] (which
+ // is a UINT64 array) at its natural boundary.
+ // Accordingly, allocate:
+ // sizeof(*mCpuHotEjectData) + (MaxNumberOfCpus * sizeof(UINT64))
+ // and, add sizeof(UINT64) - 1 to use as padding if needed.
+ //
+
+ if (RETURN_ERROR (SafeUintnMult (MaxNumberOfCpus, sizeof (UINT64), &Size)) ||
+ RETURN_ERROR (SafeUintnAdd (Size, sizeof (*mCpuHotEjectData), &Size)) ||
+ RETURN_ERROR (SafeUintnAdd (Size, sizeof (UINT64) - 1, &Size))) {
+ DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_EJECT_DATA\n", __FUNCTION__));
+ goto Fatal;
+ }
+
+ mCpuHotEjectData = AllocatePool (Size);
+ if (mCpuHotEjectData == NULL) {
+ ASSERT (mCpuHotEjectData != NULL);
+ goto Fatal;
+ }
+
+ mCpuHotEjectData->Handler = NULL;
+ mCpuHotEjectData->ArrayLength = MaxNumberOfCpus;
+
+ mCpuHotEjectData->QemuSelectorMap = ALIGN_POINTER (mCpuHotEjectData + 1,
+ sizeof (UINT64));
+ //
+ // We use mCpuHotEjectData->QemuSelectorMap to map
+ // ProcessorNum -> QemuSelector. Initialize to invalid values.
+ //
+ for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) {
+ mCpuHotEjectData->QemuSelectorMap[Idx] = CPU_EJECT_QEMU_SELECTOR_INVALID;
+ }
+
+ //
+ // Expose address of CPU Hot eject Data structure
+ //
+ PcdStatus = PcdSet64S (PcdCpuHotEjectDataAddress,
+ (UINTN)(VOID *)mCpuHotEjectData);
+ ASSERT_RETURN_ERROR (PcdStatus);
+
+ return;
+
+Fatal:
+ CpuDeadLoop ();
+}
+
/**
Hook point in normal execution mode that allows the one CPU that was elected
as monarch during System Management Mode initialization to perform additional
@@ -188,6 +262,9 @@ SmmCpuFeaturesSmmRelocationComplete (
UINTN MapPagesBase;
UINTN MapPagesCount;

+
+ InitCpuHotEjectData ();
+
if (!MemEncryptSevIsEnabled ()) {
return;
}


Re: [PATCH v9 05/10] OvmfPkg: define CPU_HOT_EJECT_DATA

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Define CPU_HOT_EJECT_DATA and add PCD PcdCpuHotEjectDataAddress, which
will be used to share CPU ejection state between OvmfPkg/CpuHotPlugSmm
and PiSmmCpuDxeSmm.

Cc: Laszlo Ersek <lersek@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Igor Mammedov <imammedo@...>
Cc: Boris Ostrovsky <boris.ostrovsky@...>
Cc: Aaron Young <aaron.young@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
---

Notes:
Addresses the following comments from v8:

(1) Get rid of the unnecessary commit specifier from the subject.
(2) s/MaxNumberOfCpus/PcdCpuMaxLogicalProcessorNumber/
(3) Shifted the comments to be above each structure field.

OvmfPkg/OvmfPkg.dec | 4 +++
OvmfPkg/Include/Pcd/CpuHotEjectData.h | 60 +++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h
Reviewed-by: Laszlo Ersek <lersek@...>

Thanks,
Laszlo


diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4348bb45c64a..9629707020ba 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -352,6 +352,10 @@ [PcdsDynamic, PcdsDynamicEx]
# This PCD is only accessed if PcdSmmSmramRequire is TRUE (see below).
gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE|BOOLEAN|0x34

+ ## This PCD adds a communication channel between OVMF's SmmCpuFeaturesLib
+ # instance in PiSmmCpuDxeSmm, and CpuHotplugSmm.
+ gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress|0|UINT64|0x46
+
[PcdsFeatureFlag]
gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c
gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d
diff --git a/OvmfPkg/Include/Pcd/CpuHotEjectData.h b/OvmfPkg/Include/Pcd/CpuHotEjectData.h
new file mode 100644
index 000000000000..06714375526c
--- /dev/null
+++ b/OvmfPkg/Include/Pcd/CpuHotEjectData.h
@@ -0,0 +1,60 @@
+/** @file
+ Definition for the CPU_HOT_EJECT_DATA structure, which shares
+ CPU hot-eject state between OVMF's SmmCpuFeaturesLib instance in
+ PiSmmCpuDxeSmm, and CpuHotplugSmm.
+
+ CPU_HOT_EJECT_DATA is allocated in SMRAM, and pointed-to by
+ PcdCpuHotEjectDataAddress.
+
+ PcdCpuHotEjectDataAddress is valid when SMM_REQUIRE is TRUE
+ and PcdCpuMaxLogicalProcessorNumber > 1.
+
+ Copyright (C) 2021, Oracle Corporation.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef CPU_HOT_EJECT_DATA_H_
+#define CPU_HOT_EJECT_DATA_H_
+
+/**
+ CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
+ on each CPU at exit from SMM.
+
+ @param[in] ProcessorNum ProcessorNum denotes the CPU exiting SMM,
+ and will be used as an index into
+ CPU_HOT_EJECT_DATA->QemuSelectorMap. It is
+ identical to the processor handle in
+ EFI_SMM_CPU_SERVICE_PROTOCOL.
+**/
+typedef
+VOID
+(EFIAPI *CPU_HOT_EJECT_HANDLER) (
+ IN UINTN ProcessorNum
+ );
+
+//
+// CPU_EJECT_QEMU_SELECTOR_INVALID marks CPUs not being ejected in
+// CPU_HOT_EJECT_DATA->QemuSelectorMap.
+//
+// QEMU CPU Selector is UINT32, so we choose an invalid value larger
+// than that type.
+//
+#define CPU_EJECT_QEMU_SELECTOR_INVALID (MAX_UINT64)
+
+typedef struct {
+ //
+ // Maps ProcessorNum -> QemuSelector for pending hot-ejects
+ //
+ volatile UINT64 *QemuSelectorMap;
+ //
+ // Handler to do the CPU ejection
+ //
+ volatile CPU_HOT_EJECT_HANDLER Handler;
+ //
+ // Entries in the QemuSelectorMap
+ //
+ UINT32 ArrayLength;
+} CPU_HOT_EJECT_DATA;
+
+#endif // CPU_HOT_EJECT_DATA_H_


Re: [PATCH v9 02/10] OvmfPkg/CpuHotplugSmm: collect hot-unplug events

Laszlo Ersek
 

On 03/12/21 07:26, Ankur Arora wrote:
Process fw_remove events in QemuCpuhpCollectApicIds(), and collect APIC IDs
and QEMU CPU Selectors for CPUs being hot-unplugged.

In addition, we now ignore CPUs which only have remove set. These
CPUs haven't been processed by OSPM yet.

This is based on the QEMU hot-unplug protocol documented here:
https://lore.kernel.org/qemu-devel/20201204170939.1815522-3-imammedo@redhat.com/

Cc: Laszlo Ersek <lersek@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Igor Mammedov <imammedo@...>
Cc: Boris Ostrovsky <boris.ostrovsky@...>
Cc: Aaron Young <aaron.young@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora <ankur.a.arora@...>
---

Notes:
Addresses the following comments from v8:
(1) Fix commit message to mention that we collect cpu-selectors as well.
(2,3,6) s/UnplugSelector/UnplugSelectors/ in CpuHotplug.c, QemuCpuhp.c
(4) Fix comment above the declaration of the now renamed mToUnplugSelector.
(5) Fix spacing around "||".
(7) Fix QemuCpuCollectApicIds() comments to line up descriptions for
ToUnplugSelectors and other params.
(8) s/ExtendSel/ExtendSels/.
(9) Add the (ExtendSels => ExtendIds) assert.
(10) Fix the missing CurrentSelector++ bug.

OvmfPkg/CpuHotplugSmm/QemuCpuhp.h | 1 +
OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h | 1 +
OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 29 +++++--
OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 101 +++++++++++++++-------
4 files changed, 93 insertions(+), 39 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@...>

Thanks
Laszlo


diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
index 8adaa0ad91f0..3e2c2192e1c0 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
@@ -55,6 +55,7 @@ QemuCpuhpCollectApicIds (
OUT APIC_ID *PluggedApicIds,
OUT UINT32 *PluggedCount,
OUT APIC_ID *ToUnplugApicIds,
+ OUT UINT32 *ToUnplugSelectors,
OUT UINT32 *ToUnplugCount
);

diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
index a34a6d3fae61..2ec7a107a64d 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -34,6 +34,7 @@
#define QEMU_CPUHP_STAT_ENABLED BIT0
#define QEMU_CPUHP_STAT_INSERT BIT1
#define QEMU_CPUHP_STAT_REMOVE BIT2
+#define QEMU_CPUHP_STAT_FW_REMOVE BIT4

#define QEMU_CPUHP_RW_CMD_DATA 0x8

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index bf68fcd42914..ee1497b93140 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -45,13 +45,16 @@ STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData;
// don't want to allocate SMRAM at OS runtime, and potentially fail (or
// fragment the SMRAM map).
//
-// These arrays provide room for ("possible CPU count" minus one) APIC IDs
-// each, as we don't expect every possible CPU to appear, or disappear, in a
-// single MMI. The numbers of used (populated) elements in the arrays are
+// The first array stores APIC IDs for hot-plug events, the second and the
+// third store APIC IDs and QEMU CPU Selectors (both indexed similarly) for
+// hot-unplug events. All of these provide room for "possible CPU count" minus
+// one elements as we don't expect every possible CPU to appear, or disappear,
+// in a single MMI. The numbers of used (populated) elements in the arrays are
// determined on every MMI separately.
//
STATIC APIC_ID *mPluggedApicIds;
STATIC APIC_ID *mToUnplugApicIds;
+STATIC UINT32 *mToUnplugSelectors;
//
// Address of the non-SMRAM reserved memory page that contains the Post-SMM Pen
// for hot-added CPUs.
@@ -289,6 +292,7 @@ CpuHotplugMmi (
mPluggedApicIds,
&PluggedCount,
mToUnplugApicIds,
+ mToUnplugSelectors,
&ToUnplugCount
);
if (EFI_ERROR (Status)) {
@@ -333,7 +337,9 @@ CpuHotplugEntry (
)
{
EFI_STATUS Status;
+ UINTN Len;
UINTN Size;
+ UINTN SizeSel;

//
// This module should only be included when SMM support is required.
@@ -387,8 +393,9 @@ CpuHotplugEntry (
//
// Allocate the data structures that depend on the possible CPU count.
//
- if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, &Size)) ||
- RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Size, &Size))) {
+ if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, &Len)) ||
+ RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Len, &Size)) ||
+ RETURN_ERROR (SafeUintnMult (sizeof (UINT32), Len, &SizeSel))) {
Status = EFI_ABORTED;
DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_PLUG_DATA\n", __FUNCTION__));
goto Fatal;
@@ -405,6 +412,12 @@ CpuHotplugEntry (
DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));
goto ReleasePluggedApicIds;
}
+ Status = gMmst->MmAllocatePool (EfiRuntimeServicesData, SizeSel,
+ (VOID **)&mToUnplugSelectors);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));
+ goto ReleaseToUnplugApicIds;
+ }

//
// Allocate the Post-SMM Pen for hot-added CPUs.
@@ -412,7 +425,7 @@ CpuHotplugEntry (
Status = SmbaseAllocatePostSmmPen (&mPostSmmPenAddress,
SystemTable->BootServices);
if (EFI_ERROR (Status)) {
- goto ReleaseToUnplugApicIds;
+ goto ReleaseToUnplugSelectors;
}

//
@@ -472,6 +485,10 @@ ReleasePostSmmPen:
SmbaseReleasePostSmmPen (mPostSmmPenAddress, SystemTable->BootServices);
mPostSmmPenAddress = 0;

+ReleaseToUnplugSelectors:
+ gMmst->MmFreePool (mToUnplugSelectors);
+ mToUnplugSelectors = NULL;
+
ReleaseToUnplugApicIds:
gMmst->MmFreePool (mToUnplugApicIds);
mToUnplugApicIds = NULL;
diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
index 8d4a6693c8d6..8434dd446b96 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
@@ -145,27 +145,30 @@ QemuCpuhpWriteCommand (

On error, the contents of the output parameters are undefined.

- @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for
- accessing IO Ports.
+ @param[in] MmCpuIo The EFI_MM_CPU_IO_PROTOCOL instance for
+ accessing IO Ports.

- @param[in] PossibleCpuCount The number of possible CPUs in the system. Must
- be positive.
+ @param[in] PossibleCpuCount The number of possible CPUs in the system. Must
+ be positive.

- @param[in] ApicIdCount The number of elements each one of the
- PluggedApicIds and ToUnplugApicIds arrays can
- accommodate. Must be positive.
+ @param[in] ApicIdCount The number of elements each one of the
+ PluggedApicIds and ToUnplugApicIds arrays can
+ accommodate. Must be positive.

- @param[out] PluggedApicIds The APIC IDs of the CPUs that have been
- hot-plugged.
+ @param[out] PluggedApicIds The APIC IDs of the CPUs that have been
+ hot-plugged.

- @param[out] PluggedCount The number of filled-in APIC IDs in
- PluggedApicIds.
+ @param[out] PluggedCount The number of filled-in APIC IDs in
+ PluggedApicIds.

- @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
- hot-unplugged.
+ @param[out] ToUnplugApicIds The APIC IDs of the CPUs that are about to be
+ hot-unplugged.

- @param[out] ToUnplugCount The number of filled-in APIC IDs in
- ToUnplugApicIds.
+ @param[out] ToUnplugSelectors The QEMU Selectors of the CPUs that are about
+ to be hot-unplugged.
+
+ @param[out] ToUnplugCount The number of filled-in APIC IDs in
+ ToUnplugApicIds.

@retval EFI_INVALID_PARAMETER PossibleCpuCount is zero, or ApicIdCount is
zero.
@@ -187,6 +190,7 @@ QemuCpuhpCollectApicIds (
OUT APIC_ID *PluggedApicIds,
OUT UINT32 *PluggedCount,
OUT APIC_ID *ToUnplugApicIds,
+ OUT UINT32 *ToUnplugSelectors,
OUT UINT32 *ToUnplugCount
)
{
@@ -204,6 +208,7 @@ QemuCpuhpCollectApicIds (
UINT32 PendingSelector;
UINT8 CpuStatus;
APIC_ID *ExtendIds;
+ UINT32 *ExtendSels;
UINT32 *ExtendCount;
APIC_ID NewApicId;

@@ -245,10 +250,10 @@ QemuCpuhpCollectApicIds (
if ((CpuStatus & QEMU_CPUHP_STAT_INSERT) != 0) {
//
// The "insert" event guarantees the "enabled" status; plus it excludes
- // the "remove" event.
+ // the "fw_remove" event.
//
if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0 ||
- (CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
+ (CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {
DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "
"inconsistent CPU status\n", __FUNCTION__, CurrentSelector,
CpuStatus));
@@ -259,33 +264,63 @@ QemuCpuhpCollectApicIds (
CurrentSelector));

ExtendIds = PluggedApicIds;
+ ExtendSels = NULL;
ExtendCount = PluggedCount;
- } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
- DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove\n", __FUNCTION__,
- CurrentSelector));
+ } else if ((CpuStatus & QEMU_CPUHP_STAT_FW_REMOVE) != 0) {
+ //
+ // "fw_remove" event guarantees "enabled".
+ //
+ if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) == 0) {
+ DEBUG ((DEBUG_ERROR, "%a: CurrentSelector=%u CpuStatus=0x%x: "
+ "inconsistent CPU status\n", __FUNCTION__, CurrentSelector,
+ CpuStatus));
+ return EFI_PROTOCOL_ERROR;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: fw_remove\n",
+ __FUNCTION__, CurrentSelector));

ExtendIds = ToUnplugApicIds;
+ ExtendSels = ToUnplugSelectors;
ExtendCount = ToUnplugCount;
+ } else if ((CpuStatus & QEMU_CPUHP_STAT_REMOVE) != 0) {
+ //
+ // Let the OSPM deal with the "remove" event.
+ //
+ DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: remove (ignored)\n",
+ __FUNCTION__, CurrentSelector));
+
+ ExtendIds = NULL;
+ ExtendSels = NULL;
+ ExtendCount = NULL;
} else {
DEBUG ((DEBUG_VERBOSE, "%a: CurrentSelector=%u: no event\n",
__FUNCTION__, CurrentSelector));
break;
}

- //
- // Save the APIC ID of the CPU with the pending event, to the corresponding
- // APIC ID array.
- //
- if (*ExtendCount == ApicIdCount) {
- DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));
- return EFI_BUFFER_TOO_SMALL;
- }
- QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);
- NewApicId = QemuCpuhpReadCommandData (MmCpuIo);
- DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,
- NewApicId));
- ExtendIds[(*ExtendCount)++] = NewApicId;
+ ASSERT ((ExtendIds == NULL) == (ExtendCount == NULL));
+ ASSERT ((ExtendSels == NULL) || (ExtendIds != NULL));

+ if (ExtendIds != NULL) {
+ //
+ // Save the APIC ID of the CPU with the pending event, to the
+ // corresponding APIC ID array.
+ // For unplug events, also save the CurrentSelector.
+ //
+ if (*ExtendCount == ApicIdCount) {
+ DEBUG ((DEBUG_ERROR, "%a: APIC ID array too small\n", __FUNCTION__));
+ return EFI_BUFFER_TOO_SMALL;
+ }
+ QemuCpuhpWriteCommand (MmCpuIo, QEMU_CPUHP_CMD_GET_ARCH_ID);
+ NewApicId = QemuCpuhpReadCommandData (MmCpuIo);
+ DEBUG ((DEBUG_VERBOSE, "%a: ApicId=" FMT_APIC_ID "\n", __FUNCTION__,
+ NewApicId));
+ if (ExtendSels != NULL) {
+ ExtendSels[(*ExtendCount)] = CurrentSelector;
+ }
+ ExtendIds[(*ExtendCount)++] = NewApicId;
+ }
//
// We've processed the CPU with (known) pending events, but we must never
// clear events. Therefore we need to advance past this CPU manually;


[PATCH] ShellPkg/Library: Fix bug in Pci.c

IanX Kuo
 

From: VincentX Ke <vincentx.ke@...>

Bugzilla: 3262 (https://bugzilla.tianocore.org/show_bug.cgi?id=3D3262)

No need to print PCIe details while CapabilityId is 0xFFFF.
Limit the NextCapabilityOffset value to AllocatePool() memory.

Signed-off-by: VincentX Ke <vincentx.ke@...>
---
ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c b/ShellPkg/L=
ibrary/UefiShellDebug1CommandsLib/Pci.c
index a2f04d8db5..9ebf1c26ef 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
@@ -2038,12 +2038,14 @@ LocatePciCapability (
=0D
@param[in] PciExpressCap PCI Express capability buffer.=0D
@param[in] ExtendedConfigSpace PCI Express extended configuration space.=
=0D
+ @param[in] ExtendedConfigSize PCI Express extended configuration size.=
=0D
@param[in] ExtendedCapability PCI Express extended capability ID to exp=
lain.=0D
**/=0D
VOID=0D
PciExplainPciExpress (=0D
IN PCI_CAPABILITY_PCIEXP *PciExpressCap,=0D
IN UINT8 *ExtendedConfigSpace,=0D
+ IN UINTN ExtendedConfigSize,=0D
IN CONST UINT16 ExtendedCapability=0D
);=0D
=0D
@@ -2921,6 +2923,7 @@ ShellCommandRunPci (
PciExplainPciExpress (=0D
(PCI_CAPABILITY_PCIEXP *) ((UINT8 *) &ConfigSpace + PcieCapabili=
tyPtr),=0D
ExtendedConfigSpace,=0D
+ ExtendedConfigSize,=0D
ExtendedCapability=0D
);=0D
}=0D
@@ -5698,12 +5701,14 @@ PrintPciExtendedCapabilityDetails(
=0D
@param[in] PciExpressCap PCI Express capability buffer.=0D
@param[in] ExtendedConfigSpace PCI Express extended configuration space.=
=0D
+ @param[in] ExtendedConfigSize PCI Express extended configuration size.=
=0D
@param[in] ExtendedCapability PCI Express extended capability ID to exp=
lain.=0D
**/=0D
VOID=0D
PciExplainPciExpress (=0D
IN PCI_CAPABILITY_PCIEXP *PciExpressCap,=0D
IN UINT8 *ExtendedConfigSpace,=0D
+ IN UINTN ExtendedConfigSize,=0D
IN CONST UINT16 ExtendedCapability=0D
)=0D
{=0D
@@ -5786,7 +5791,7 @@ PciExplainPciExpress (
}=0D
=0D
ExtHdr =3D (PCI_EXP_EXT_HDR*)ExtendedConfigSpace;=0D
- while (ExtHdr->CapabilityId !=3D 0 && ExtHdr->CapabilityVersion !=3D 0) =
{=0D
+ while (ExtHdr->CapabilityId !=3D 0 && ExtHdr->CapabilityVersion !=3D 0 &=
& ExtHdr->CapabilityId !=3D 0xFFFF) {=0D
//=0D
// Process this item=0D
//=0D
@@ -5800,7 +5805,7 @@ PciExplainPciExpress (
//=0D
// Advance to the next item if it exists=0D
//=0D
- if (ExtHdr->NextCapabilityOffset !=3D 0) {=0D
+ if (ExtHdr->NextCapabilityOffset !=3D 0 && (ExtHdr->NextCapabilityOffs=
et <=3D (UINT32)(ExtendedConfigSize - sizeof (PCI_EXP_EXT_HDR)))) {=0D
ExtHdr =3D (PCI_EXP_EXT_HDR*)(ExtendedConfigSpace + ExtHdr->NextCapa=
bilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET);=0D
} else {=0D
break;=0D
--=20
2.18.0.windows.1


Re: [edk2-platforms] Intel/MinPlatformPkg: resolve MmUnblockMemoryLib

Kun Qin
 

Hi Zhiguang,

I have already sent this patch series to resolve dependencies in edk2-platform (although the change is slightly different):
https://edk2.groups.io/g/devel/message/72645

Specifically:
https://edk2.groups.io/g/devel/message/72646

Could you please let me know if the change above resolves the dependency issue for you?

The patch series have 2 other patches not being reviewed yet. I plan to send out another round tomorrow to include reviewed-by tags. But please let me know if you prefer your change below to check in first, I will just drop my patch #1 when sending v2.

Thanks,
Kun

On 03/16/2021 02:06, Zhiguang Liu wrote:
The below Edk2 patch makes VariableSmmRuntimeDxe begin to consume MmUnblockMemoryLib.
It cause multiple platforms build failure.
f463dbadede138dc96a66dae6f361c54f0b3093c
MdeModulePkg: VariableSmmRuntimeDxe: Added request unblock memory interface
This change added NULL MmUnblockMemoryLib instance in MinPlatformPkg dsc include files
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc | 1 +
1 file changed, 1 insertion(+)
diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
index fa9098d525..ee91dd8bd6 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
@@ -116,6 +116,7 @@
!endif
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
+ MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
[LibraryClasses.common.UEFI_DRIVER]
Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf

19621 - 19640 of 92426