[edk2-devel] [PATCH v2 04/16] OvmfPkg: enable SMM Monarch Election in PiSmmCpuDxeSmm
Laszlo Ersek
lersek at redhat.com
Wed Feb 26 22:11:44 UTC 2020
With "PcdCpuSmmEnableBspElection" set to FALSE, PiSmmCpuDxeSmm always
considers the processor with index 0 to be the SMM Monarch (a.k.a. the SMM
BSP). The SMM Monarch handles the SMI for real, while the other CPUs wait
in their SMM loops.
In a subsequent patch, we want to set "PcdCpuHotPlugSupport" to TRUE. For
that, PiCpuSmmEntry() [UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c] forces
us with an ASSERT() to set "PcdCpuSmmEnableBspElection" to TRUE as well.
To satisfy that expectation, we can simply remove our current
"PcdCpuSmmEnableBspElection|FALSE" setting, and inherit the default TRUE
value from "UefiCpuPkg.dec".
This causes "mSmmMpSyncData->BspIndex" in PiSmmCpuDxeSmm to lose its
static zero value (standing for CPU#0); instead it becomes (-1) in
general, and the SMM Monarch is elected anew on every SMI.
The default SMM Monarch Election is basically a race -- whichever CPU can
flip "mSmmMpSyncData->BspIndex" from (-1) to its own index, becomes king,
for handling that SMI. Refer to SmiRendezvous()
[UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c].
I consider this non-determinism less than ideal on QEMU/KVM; it would be
nice to stick with a "mostly permanent" SMM Monarch even with the Election
enabled. We can do that by implementing the PlatformSmmBspElection() API
in the SmmCpuPlatformHookLibQemu instance:
The IA32 APIC Base MSR can be read on each CPU concurrently, and it will
report the BSP bit as set only on the current Boot Service Processor. QEMU
marks CPU#0 as the BSP, by default.
Elect the current BSP, as reported by QEMU, for the SMM Monarch role.
(Note that the QEMU commit history is not entirely consistent on whether
QEMU/KVM may mark a CPU with nonzero index as the BSP:
- At tag v4.2.0, "target/i386/cpu.c" has a comment saying "We hard-wire
the BSP to the first CPU". This comment goes back to commit 6cb2996cef5e
("x86: Extend validity of bsp_to_cpu", 2010-03-04).
- Compare commit 9cb11fd7539b ("target-i386: clear bsp bit when
designating bsp", 2015-04-02) though, especially considering KVM.
Either way, this OvmfPkg patch is *not* dependent on CPU index 0; it just
takes the race on every SMI out of the game.)
One benefit of using a "mostly permanent" SMM Monarch / BSP is that we can
continue testing the SMM CPU synchronization by deterministically entering
the firmware on the BSP, vs. on an AP, from Linux guests:
$ time taskset -c 0 efibootmgr
$ time taskset -c 1 efibootmgr
(See
<https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#uefi-variable-access-test>.)
Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
Cc: Igor Mammedov <imammedo at redhat.com>
Cc: Jiewen Yao <jiewen.yao at intel.com>
Cc: Jordan Justen <jordan.l.justen at intel.com>
Cc: Michael Kinney <michael.d.kinney at intel.com>
Cc: Philippe Mathieu-Daudé <philmd at redhat.com>
Suggested-by: Igor Mammedov <imammedo at redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512#c5
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
---
Notes:
v2:
- Pick up Ard's Acked-by, which is conditional on approval from Intel
reviewers on Cc. (I'd like to save Ard the churn of re-acking
unmodified patches.)
OvmfPkg/OvmfPkgIa32.dsc | 1 -
OvmfPkg/OvmfPkgIa32X64.dsc | 1 -
OvmfPkg/OvmfPkgX64.dsc | 1 -
OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.inf | 3 +++
OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.c | 9 ++++++++-
5 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 813995fefad8..60d8af185b9c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -414,45 +414,44 @@ [LibraryClasses.common.SMM_CORE]
!endif
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
################################################################################
#
# Pcd Section - list of all EDK II PCD Entries defined by this Platform.
#
################################################################################
[PcdsFeatureFlag]
gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
!ifdef $(CSM_ENABLE)
gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE
!endif
!if $(SMM_REQUIRE) == TRUE
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
- gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE
!endif
[PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
!if $(NETWORK_TLS_ENABLE) == FALSE
# match PcdFlashNvStorageVariableSize purely for convenience
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
!endif
!endif
!if $(FD_SIZE_IN_KB) == 4096
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
!if $(NETWORK_TLS_ENABLE) == FALSE
# match PcdFlashNvStorageVariableSize purely for convenience
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000
!endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a256c7084a7e..be6bc7bd88a7 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -419,45 +419,44 @@ [LibraryClasses.common.SMM_CORE]
!endif
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
################################################################################
#
# Pcd Section - list of all EDK II PCD Entries defined by this Platform.
#
################################################################################
[PcdsFeatureFlag]
gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
!ifdef $(CSM_ENABLE)
gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE
!endif
!if $(SMM_REQUIRE) == TRUE
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
- gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE
!endif
[PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
!if $(NETWORK_TLS_ENABLE) == FALSE
# match PcdFlashNvStorageVariableSize purely for convenience
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
!endif
!endif
!if $(FD_SIZE_IN_KB) == 4096
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
!if $(NETWORK_TLS_ENABLE) == FALSE
# match PcdFlashNvStorageVariableSize purely for convenience
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000
!endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 78079b9f8e13..e258c474b60d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -419,45 +419,44 @@ [LibraryClasses.common.SMM_CORE]
!endif
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
################################################################################
#
# Pcd Section - list of all EDK II PCD Entries defined by this Platform.
#
################################################################################
[PcdsFeatureFlag]
gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
!ifdef $(CSM_ENABLE)
gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable|TRUE
!endif
!if $(SMM_REQUIRE) == TRUE
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
- gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE
!endif
[PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
!if $(NETWORK_TLS_ENABLE) == FALSE
# match PcdFlashNvStorageVariableSize purely for convenience
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
!endif
!endif
!if $(FD_SIZE_IN_KB) == 4096
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
!if $(NETWORK_TLS_ENABLE) == FALSE
# match PcdFlashNvStorageVariableSize purely for convenience
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000
!endif
diff --git a/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.inf b/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.inf
index 82edeca3d12d..413c56fce6e1 100644
--- a/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.inf
+++ b/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.inf
@@ -8,22 +8,25 @@
[Defines]
INF_VERSION = 1.29
BASE_NAME = SmmCpuPlatformHookLibQemu
FILE_GUID = 154D6D26-54B8-45BC-BA3A-CBAA20C02A6A
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0
LIBRARY_CLASS = SmmCpuPlatformHookLib
#
# The following information is for reference only and not required by the build
# tools.
#
# VALID_ARCHITECTURES = IA32 X64
#
[Sources]
SmmCpuPlatformHookLibQemu.c
[Packages]
MdePkg/MdePkg.dec
UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
diff --git a/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.c b/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.c
index 257e1d399cc6..c88a95c6deff 100644
--- a/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.c
+++ b/OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.c
@@ -1,31 +1,34 @@
/** @file
SMM CPU Platform Hook library instance for QEMU.
Copyright (c) 2020, Red Hat, Inc.
Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include <Library/BaseLib.h> // AsmReadMsr64()
#include <PiSmm.h>
+#include <Register/Intel/ArchitecturalMsr.h> // MSR_IA32_APIC_BASE_REGISTER
+
#include <Library/SmmCpuPlatformHookLib.h>
/**
Checks if platform produces a valid SMI.
This function checks if platform produces a valid SMI. This function is
called at SMM entry to detect if this is a spurious SMI. This function
must be implemented in an MP safe way because it is called by multiple CPU
threads.
@retval TRUE There is a valid SMI
@retval FALSE There is no valid SMI
**/
BOOLEAN
EFIAPI
PlatformValidSmi (
VOID
)
{
return TRUE;
}
@@ -56,45 +59,49 @@ ClearTopLevelSmiStatus (
@param IsBsp Output parameter. TRUE: the CPU this function
executes on is elected to be the SMM BSP. FALSE:
the CPU this function executes on is to be SMM AP.
@retval EFI_SUCCESS The function executes successfully.
@retval EFI_NOT_READY The function does not determine whether this CPU
should be BSP or AP. This may occur if hardware
init sequence to enable the determination is yet to
be done, or the function chooses not to do BSP
election and will let SMM CPU driver to use its
default BSP election process.
@retval EFI_DEVICE_ERROR The function cannot determine whether this CPU
should be BSP or AP due to hardware error.
**/
EFI_STATUS
EFIAPI
PlatformSmmBspElection (
OUT BOOLEAN *IsBsp
)
{
- return EFI_NOT_READY;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;
+
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ *IsBsp = (BOOLEAN)(ApicBaseMsr.Bits.BSP == 1);
+ return EFI_SUCCESS;
}
/**
Get platform page table attribute.
This function gets page table attribute of platform.
@param Address Input parameter. Obtain the page table entries
attribute on this address.
@param PageSize Output parameter. The size of the page.
@param NumOfPages Output parameter. Number of page.
@param PageAttribute Output parameter. Paging Attributes (WB, UC, etc).
@retval EFI_SUCCESS The platform page table attribute from the address
is determined.
@retval EFI_UNSUPPORTED The platform does not support getting page table
attribute for the address.
**/
EFI_STATUS
EFIAPI
GetPlatformPageTableAttribute (
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54948): https://edk2.groups.io/g/devel/message/54948
Mute This Topic: https://groups.io/mt/71575177/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list