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 v2] UefiCpuPkg/MpInitLib: Fix MemTest86 failure.
Date: Wed,  3 Apr 2019 10:46:20 +0800	[thread overview]
Message-ID: <20190403024620.15228-1-eric.dong@intel.com> (raw)

V2 changes:
  Update the commit message and comments in the code.

When waking vector buffer allocated by CpuDxe is tested by MemTest86
in MP mode, an error is reported because the same range of memory is
modified by both CpuDxe driver and MemTest86.

The waking vector buffer is not expected to be tested by MemTest86 if
it is allocated out because MemTest86 only tests free memory. But
current CpuDxe driver "borrows" buffer instead of allocate buffer for
waking vector buffer (through allocate & free to get the buffer
pointer, backup the buffer data before using it and restore it after
using). With this implementation, if the buffer borrowed is not used
by any other drivers, MemTest86 tool will treat it as free memory
and test it.

In order to fix the above issue, CpuDxe changes to allocate the
buffer below 1M instead of borrowing it. But directly allocating
memory below 1MB causes LegacyBios driver fails to start. LegacyBios
driver allocates memory range from
"0xA0000 - PcdEbdaReservedMemorySize" to 0xA0000 as Ebda Reserved
Memory. The minimum value for "0xA0000 - PcdEbdaReservedMemorySize"
is 0x88000. If LegacyBios driver allocate this range failed, it
asserts.

LegacyBios also reserves range from 0x60000 to
"0x60000 + PcdOpromReservedMemorySize", it will be used as Oprom
Reserve Memory. The maximum value for "0x60000 +
PcdOpromReservedMemorySize" is 0x88000. LegacyBios driver tries to
allocate these range page(4K size) by page. It just reports warning
message if some pages are already allocated by others.
Base on above investigation, one page in range 0x60000 ~ 0x88000 can
be used as the waking vector buffer.

LegacyBios driver only reports warning when page allocation in range
[0x60000, 0x88000) fails. This library is consumed by CpuDxe driver
to produce CPU Arch protocol. LagacyBios driver depends on CPU Arch
protocol which guarantees below allocation runs earlier than
LegacyBios driver.

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 | 26 ++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index b2307cbb61..cef5b49dde 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,15 @@ GetWakeupBuffer (
   EFI_STATUS              Status;
   EFI_PHYSICAL_ADDRESS    StartAddress;
 
-  StartAddress = BASE_1MB;
+  //
+  // Try to allocate buffer below 1M for waking vector.
+  // LegacyBios driver only reports warning when page allocation in range
+  // [0x60000, 0x88000) fails.
+  // This library is consumed by CpuDxe driver to produce CPU Arch protocol.
+  // LagacyBios driver depends on CPU Arch protocol which guarantees below
+  // allocation runs earlier than LegacyBios driver.
+  //
+  StartAddress = 0x88000;
   Status = gBS->AllocatePages (
                   AllocateMaxAddress,
                   EfiBootServicesData,
@@ -99,17 +107,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.21.0.windows.1



             reply	other threads:[~2019-04-03  2:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-03  2:46 Eric Dong [this message]
2019-04-03  3:00 ` [Patch v2] UefiCpuPkg/MpInitLib: Fix MemTest86 failure Ni, Ray

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=20190403024620.15228-1-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