public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Lendacky, Thomas" <thomas.lendacky@amd.com>
To: devel@edk2.groups.io
Cc: Brijesh Singh <brijesh.singh@amd.com>,
	Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	Rahul Kumar <rahul1.kumar@intel.com>
Subject: [PATCH] UefiCpuPkg/MpInitLib: Allocate a separate SEV-ES AP reset stack area
Date: Tue, 11 May 2021 15:50:24 -0500	[thread overview]
Message-ID: <1162d1f4fe09048aaafba6f6ea3046bebd34fc2b.1620766224.git.thomas.lendacky@amd.com> (raw)

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3324

The SEV-ES stacks currently share a page with the reset code and data.
Separate the SEV-ES stacks from the reset vector code and data to avoid
possible stack overflows from overwriting the code and/or data.

When SEV-ES is enabled, invoke the GetWakeupBuffer() routine a second time
to allocate a new area, below the reset vector and data.

Both the PEI and DXE versions of GetWakeupBuffer() are changed to track
the previous reset buffer allocation in order to ensure that the new
buffer allocation is below the previous allocation.

Fixes: 7b7508ad784d16a5208c8d12dff43aef6df0835b
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 12 ++++-
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 48 +++++++++++++-------
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 14 ++++--
 3 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 7839c249760e..fdfa0755d37a 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -29,6 +29,11 @@ VOID             *mReservedApLoopFunc = NULL;
 UINTN            mReservedTopOfApStack;
 volatile UINT32  mNumberToFinish = 0;
 
+//
+// Begin wakeup buffer allocation below 0x88000
+//
+STATIC EFI_PHYSICAL_ADDRESS mWakeupBuffer = 0x88000;
+
 /**
   Enable Debug Agent to support source debugging on AP function.
 
@@ -102,7 +107,7 @@ GetWakeupBuffer (
   // LagacyBios driver depends on CPU Arch protocol which guarantees below
   // allocation runs earlier than LegacyBios driver.
   //
-  StartAddress = 0x88000;
+  StartAddress = mWakeupBuffer;
   Status = gBS->AllocatePages (
                   AllocateMaxAddress,
                   MemoryType,
@@ -112,6 +117,11 @@ GetWakeupBuffer (
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
     StartAddress = (EFI_PHYSICAL_ADDRESS) -1;
+  } else {
+    //
+    // Next wakeup buffer allocation must be below this allocation
+    //
+    mWakeupBuffer = StartAddress;
   }
 
   DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index dc2a54aa31e8..a76dae437606 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1164,20 +1164,6 @@ GetApResetVectorSize (
            AddressMap->SwitchToRealSize +
            sizeof (MP_CPU_EXCHANGE_INFO);
 
-  //
-  // The AP reset stack is only used by SEV-ES guests. Do not add to the
-  // allocation if SEV-ES is not enabled.
-  //
-  if (PcdGetBool (PcdSevEsIsEnabled)) {
-    //
-    // Stack location is based on APIC ID, so use the total number of
-    // processors for calculating the total stack area.
-    //
-    Size += AP_RESET_STACK_SIZE * PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
-
-    Size = ALIGN_VALUE (Size, CPU_STACK_ALIGNMENT);
-  }
-
   return Size;
 }
 
@@ -1207,9 +1193,39 @@ AllocateResetVector (
                                     CpuMpData->AddressMap.ModeTransitionOffset
                                     );
     //
-    // The reset stack starts at the end of the buffer.
+    // The AP reset stack is only used by SEV-ES guests. Do not allocate it
+    // if SEV-ES is not enabled.
     //
-    CpuMpData->SevEsAPResetStackStart = CpuMpData->WakeupBuffer + ApResetVectorSize;
+    if (PcdGetBool (PcdSevEsIsEnabled)) {
+      UINTN  ApResetStackSize;
+
+      //
+      // Stack location is based on ProcessorNumber, so use the total number
+      // of processors for calculating the total stack area.
+      //
+      ApResetStackSize = AP_RESET_STACK_SIZE *
+                           PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+
+      //
+      // Invoke GetWakeupBuffer a second time to allocate the stack area
+      // below 1MB. The returned buffer will be page aligned and sized and
+      // below the previously allocated buffer.
+      //
+      CpuMpData->SevEsAPResetStackStart = GetWakeupBuffer (ApResetStackSize);
+
+      //
+      // Check to be sure that the "allocate below" behavior hasn't changed.
+      // This will also catch a failed allocation, as "-1" is returned on
+      // failure.
+      //
+      if (CpuMpData->SevEsAPResetStackStart >= CpuMpData->WakeupBuffer) {
+        DEBUG ((DEBUG_ERROR,
+          "SEV-ES AP reset stack is not below wakeup buffer\n"));
+
+        ASSERT (FALSE);
+        CpuDeadLoop ();
+      }
+    }
   }
   BackupAndPrepareWakeupBuffer (CpuMpData);
 }
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 3989bd6a7a9f..4d09e89b4128 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -11,6 +11,8 @@
 #include <Guid/S3SmmInitDone.h>
 #include <Ppi/ShadowMicrocode.h>
 
+STATIC UINT64 mWakeupBuffer = BASE_1MB;
+
 /**
   S3 SMM Init Done notification function.
 
@@ -220,11 +222,11 @@ GetWakeupBuffer (
         // Need memory under 1MB to be collected here
         //
         WakeupBufferEnd = Hob.ResourceDescriptor->PhysicalStart + Hob.ResourceDescriptor->ResourceLength;
-        if (WakeupBufferEnd > BASE_1MB) {
+        if (WakeupBufferEnd > mWakeupBuffer) {
           //
-          // Wakeup buffer should be under 1MB
+          // Wakeup buffer should be under 1MB and under the previous one
           //
-          WakeupBufferEnd = BASE_1MB;
+          WakeupBufferEnd = mWakeupBuffer;
         }
         while (WakeupBufferEnd > WakeupBufferSize) {
           //
@@ -244,6 +246,12 @@ GetWakeupBuffer (
           }
           DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
                                WakeupBufferStart, WakeupBufferSize));
+
+          //
+          // Next wakeup buffer allocation must be below this allocation
+          //
+          mWakeupBuffer = WakeupBufferStart;
+
           return (UINTN)WakeupBufferStart;
         }
       }
-- 
2.31.0


             reply	other threads:[~2021-05-11 20:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-11 20:50 Lendacky, Thomas [this message]
2021-05-14  9:14 ` [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Allocate a separate SEV-ES AP reset stack area Laszlo Ersek
2021-05-14 13:33   ` Lendacky, Thomas
2021-05-14 14:54     ` Lendacky, Thomas
2021-05-14 15:04 ` Marvin Häuser
2021-05-14 15:23   ` Lendacky, Thomas
2021-05-14 15:44     ` Marvin Häuser
2021-05-16  1:17       ` Laszlo Ersek
2021-05-16 22:15         ` Marvin Häuser
2021-05-18 15:29           ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1162d1f4fe09048aaafba6f6ea3046bebd34fc2b.1620766224.git.thomas.lendacky@amd.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox