public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Eric Dong <eric.dong@intel.com>
To: edk2-devel@lists.01.org
Subject: [Patch] UefiCpuPkg/MpInitLib: Direct allocate buffer for Wake up Buffer.
Date: Tue,  5 Mar 2019 10:06:58 +0800	[thread overview]
Message-ID: <20190305020658.23408-2-eric.dong@intel.com> (raw)
In-Reply-To: <20190305020658.23408-1-eric.dong@intel.com>

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

Current CpuDxe driver "borrows" Wakeup Buffer (through Allocate & free
to get the buffer pointer, backup the buffer data before using it and
restore it after using).  Now this logic met a problem described in
the above BZ because the test tool and the CpuDxe both use the same
memory at the same time.

In order to fix the above issue, CpuDxe changed to allocate the buffer
below 1M instead of borrow it. After investigation, we found below
0x88000 is the possible space which can be used. For now, range
0x60000 ~ 0x88000 used for Legacy OPROMs by LegacyBios driver. And it
tries to allocate these range page(4K size) by page. It just reports
warning message if specific page been used by others already.

Also CpuDxe driver will produce CPU arch protocol and LegacyBios driver
has dependency for this protocol. So CpuDxe driver will start before
LegacyBios driver and CpuDxe driver can allocate that space successful.

With this change, CpuDxe driver will use 0x87000 ~ 0x88000 as wakeup
buffer.

Cc: Ray Ni <ray.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index b2307cbb61..5bc9a47efb 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -76,7 +76,7 @@ SaveCpuMpData (
 }
 
 /**
-  Get available system memory below 1MB by specified size.
+  Get available system memory below 0x88000 by specified size.
 
   @param[in] WakeupBufferSize   Wakeup buffer size required
 
@@ -91,7 +91,19 @@ GetWakeupBuffer (
   EFI_STATUS              Status;
   EFI_PHYSICAL_ADDRESS    StartAddress;
 
-  StartAddress = BASE_1MB;
+  //
+  // Current "Borrow" space mechanism caused potential race condition if both
+  // AP and the original owner use the share space.
+  //
+  // LegacyBios driver tries to allocate 4K pages between 0x60000 ~ 0x88000
+  // space. It will just report an waring message if the page has been allocate
+  // by other drivers.
+  // LagacyBios driver depends on CPU Arch protocol, so it will start after
+  // CpuDxe driver which produce Cpu Arch Protocol and use this library.
+  // So below allocate logic will be trigged before LegacyBios driver and it
+  // will always return success.
+  //
+  StartAddress = BASE_512KB + BASE_32KB;
   Status = gBS->AllocatePages (
                   AllocateMaxAddress,
                   EfiBootServicesData,
@@ -99,17 +111,13 @@ GetWakeupBuffer (
                   &StartAddress
                   );
   ASSERT_EFI_ERROR (Status);
-  if (!EFI_ERROR (Status)) {
-    Status = gBS->FreePages(
-               StartAddress,
-               EFI_SIZE_TO_PAGES (WakeupBufferSize)
-               );
-    ASSERT_EFI_ERROR (Status);
-    DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
-                        (UINTN) StartAddress, WakeupBufferSize));
-  } else {
+  if (EFI_ERROR (Status)) {
     StartAddress = (EFI_PHYSICAL_ADDRESS) -1;
   }
+
+  DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
+                      (UINTN) StartAddress, WakeupBufferSize));
+
   return (UINTN) StartAddress;
 }
 
-- 
2.15.0.windows.1



  reply	other threads:[~2019-03-05  2:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-05  2:06 [Patch] MdeModulePkg/PiSmmCore: Control S3 related functionality with flag Eric Dong
2019-03-05  2:06 ` Eric Dong [this message]
2019-03-05  7:33   ` [Patch] UefiCpuPkg/MpInitLib: Direct allocate buffer for Wake up Buffer Zeng, Star
2019-03-07  2:53     ` Dong, Eric
2019-03-07 17:39       ` Laszlo Ersek
2019-03-13  7:44         ` Dong, Eric
2019-03-05  8:33   ` Ni, Ray
2019-03-19  1:51 ` [Patch] MdeModulePkg/PiSmmCore: Control S3 related functionality with flag Wu, Hao A

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=20190305020658.23408-2-eric.dong@intel.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