]> git.proxmox.com Git - mirror_edk2.git/commitdiff
OvmfPkg: enable SMM Monarch Election in PiSmmCpuDxeSmm
authorLaszlo Ersek <lersek@redhat.com>
Wed, 26 Feb 2020 22:11:44 +0000 (23:11 +0100)
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Wed, 4 Mar 2020 12:22:07 +0000 (12:22 +0000)
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@linaro.org>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512#c5
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200226221156.29589-5-lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.c
OvmfPkg/Library/SmmCpuPlatformHookLibQemu/SmmCpuPlatformHookLibQemu.inf
OvmfPkg/OvmfPkgIa32.dsc
OvmfPkg/OvmfPkgIa32X64.dsc
OvmfPkg/OvmfPkgX64.dsc

index 257e1d399cc6d8add51990003e04c3a4733069c0..c88a95c6deffe6757148c3d7631ac1f45e43046e 100644 (file)
@@ -6,7 +6,10 @@ Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent\r
 \r
 **/\r
+#include <Library/BaseLib.h>                 // AsmReadMsr64()\r
 #include <PiSmm.h>\r
+#include <Register/Intel/ArchitecturalMsr.h> // MSR_IA32_APIC_BASE_REGISTER\r
+\r
 #include <Library/SmmCpuPlatformHookLib.h>\r
 \r
 /**\r
@@ -75,7 +78,11 @@ PlatformSmmBspElection (
   OUT BOOLEAN     *IsBsp\r
   )\r
 {\r
-  return EFI_NOT_READY;\r
+  MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;\r
+\r
+  ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);\r
+  *IsBsp = (BOOLEAN)(ApicBaseMsr.Bits.BSP == 1);\r
+  return EFI_SUCCESS;\r
 }\r
 \r
 /**\r
index 82edeca3d12ddac6ce4658778a2da27fc966020c..413c56fce6e12aa52339817ad5570af234f69c7b 100644 (file)
@@ -27,3 +27,6 @@
 [Packages]\r
   MdePkg/MdePkg.dec\r
   UefiCpuPkg/UefiCpuPkg.dec\r
+\r
+[LibraryClasses]\r
+  BaseLib\r
index c2d727730c04fd2dcc85c366cdb42e526c44b0f0..4788f03e9a03819f3fe853db0f6362c9bdb26605 100644 (file)
 !endif\r
 !if $(SMM_REQUIRE) == TRUE\r
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE\r
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE\r
 !endif\r
 \r
index 7d862232b23efdca98d1e42f86695a02035b4581..adbbed8e111252a43c25c924d77ace117cda3feb 100644 (file)
 !endif\r
 !if $(SMM_REQUIRE) == TRUE\r
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE\r
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE\r
 !endif\r
 \r
index c49703c994b3b3c26cc4882e0cf44bf052e2f4b9..02cbc75a12f4dfd7e7020bbc76e839ca2654691e 100644 (file)
 !endif\r
 !if $(SMM_REQUIRE) == TRUE\r
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE\r
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE\r
 !endif\r
 \r