Re: [edk2-devel] [PATCH 0/4] Sync BSP's APIC mode to APs in MP init flow

["Gerd Hoffmann" <kraxel@redhat.com>]

On Fri, Jul 07, 2023 at 09:25:39AM +0000, Ni, Ray wrote:
> Gerd,
> No. I don't plan to add PCD.
> I thought that initially but in the end I figured out:
> * PCD is a way to let platform configure the common logic behavior.
> * Why not treat "X2 APIC status in BSP" as a "hardware" PCD?

A PCD has the advantage that most configuration knobs use that,
so people are used to use PCDs to configure their platform builds.

Using hardware status as pseudo PCD technically works too, and
it isn't the first case in edk2 either, 5-level paging works the
same way for example.

ovmf test patch (enabling x2mode unconditionally) below.

Tested-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd

------------------------------ cut here -----------------------------

>From 46a2d9128c42f62547f76afbdb8eef9cf5d0a8a1 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Fri, 7 Jul 2023 12:44:21 +0200
Subject: [PATCH 1/1] OvmfPkg/PlatformPei: enable x2apic mode if supported

Switch the BSP local apic into x2apic mode if supported by the CPU.
CpuMpPei will switch the APs into x2apic mode, see commit FIXME.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/PlatformPei/PlatformPei.inf |  1 +
 OvmfPkg/PlatformPei/Platform.c      | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 3934aeed9514..b670e1ba6745 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -52,6 +52,7 @@ [LibraryClasses]
   DebugLib
   HobLib
   IoLib
+  LocalApicLib
   PciLib
   ResourcePublicationLib
   PeiServicesLib
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index f5dc41c3a8c4..fd34fceafa31 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -21,6 +21,7 @@
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
 #include <Library/IoLib.h>
+#include <Library/LocalApicLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/PciLib.h>
@@ -271,6 +272,29 @@ MaxCpuCountInitialization (
   ASSERT_RETURN_ERROR (PcdStatus);
 }
 
+STATIC
+VOID
+PlatformApicInit (
+  IN EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
+  )
+{
+  UINT32   RegEax, RegEbx, RegEcx, RegEdx;
+
+  AsmCpuid (1, &RegEax, &RegEbx, &RegEcx, &RegEdx);
+  if (!(RegEcx & (1 << 21))) {
+    DEBUG ((DEBUG_INFO, "%a: x2apic mode not supported\n", __func__));
+    return;
+  }
+
+  SetApicMode(LOCAL_APIC_MODE_X2APIC);
+  if (!(GetApicMode() == LOCAL_APIC_MODE_X2APIC)) {
+    DEBUG ((DEBUG_WARN, "%a: enabling x2apic mode failed\n", __func__));
+    return;
+  }
+
+  DEBUG ((DEBUG_INFO, "%a: x2apic mode enabled\n", __func__));
+}
+
 /**
  * @brief Builds PlatformInfo Hob
  */
@@ -368,5 +392,7 @@ InitializePlatform (
   IntelTdxInitialize ();
   InstallFeatureControlCallback (PlatformInfoHob);
 
+  PlatformApicInit(PlatformInfoHob);
+
   return EFI_SUCCESS;
 }
-- 
2.41.0