public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jeff Fan <jeff.fan@intel.com>
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek <lersek@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Put AP into safe hlt-loop code on S3 path
Date: Fri, 11 Nov 2016 13:45:43 +0800	[thread overview]
Message-ID: <20161111054545.19616-2-jeff.fan@intel.com> (raw)
In-Reply-To: <20161111054545.19616-1-jeff.fan@intel.com>

On S3 path, we will wake up APs to restore CPU context in PiSmmCpuDxeSmm
driver. However, we place AP in hlt-loop under 1MB space borrowed after CPU
restoring CPU contexts.
In case, one NMI or SMI happens, APs may exit from hlt state and execute the
instruction after HLT instruction. But the code under 1MB is no longer safe at
that time.

This fix is to allocate one ACPI NVS range to place the AP hlt-loop code. When
CPU finished restoration CPU contexts, AP will execute in this ACPI NVS range.

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

v2:
  1. Make stack alignment per Laszlo's comment.
  2. Trim whitespace at end of end.
  3. Update year mark in file header.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Analyzed-by: Paolo Bonzini <pbonzini@redhat.com>
Analyzed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c             | 32 +++++++++++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 27 +++++++++++++++++++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    | 13 +++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  | 29 +++++++++++++++++++++++-
 4 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 6a798ef..e53e096 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -77,6 +77,13 @@ SMM_S3_RESUME_STATE          *mSmmS3ResumeState = NULL;
 
 BOOLEAN                      mAcpiS3Enable = TRUE;
 
+UINT8                        *mApHltLoopCode = NULL;
+UINT8                        mApHltLoopCodeTemplate[] = {
+                               0xFA,        // cli
+                               0xF4,        // hlt
+                               0xEB, 0xFC   // jmp $-2
+                               };
+
 /**
   Get MSR spin lock by MSR index.
 
@@ -376,6 +383,8 @@ MPRendezvousProcedure (
   CPU_REGISTER_TABLE         *RegisterTableList;
   UINT32                     InitApicId;
   UINTN                      Index;
+  UINT32                     TopOfStack;
+  UINT8                      Stack[128];
 
   ProgramVirtualWireMode ();
   DisableLvtInterrupts ();
@@ -393,6 +402,14 @@ MPRendezvousProcedure (
   // Count down the number with lock mechanism.
   //
   InterlockedDecrement (&mNumberToFinish);
+
+  //
+  // Place AP into the safe code
+  //
+  TopOfStack  = (UINT32) (UINTN) Stack + sizeof (Stack);
+  TopOfStack &= ~(UINT32) (CPU_STACK_ALIGNMENT - 1);
+  CopyMem ((VOID *) (UINTN) mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate));
+  TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack);
 }
 
 /**
@@ -731,6 +748,8 @@ InitSmmS3ResumeState (
   VOID                       *GuidHob;
   EFI_SMRAM_DESCRIPTOR       *SmramDescriptor;
   SMM_S3_RESUME_STATE        *SmmS3ResumeState;
+  EFI_PHYSICAL_ADDRESS       Address;
+  EFI_STATUS                 Status;
 
   if (!mAcpiS3Enable) {
     return;
@@ -773,6 +792,19 @@ InitSmmS3ResumeState (
   // Patch SmmS3ResumeState->SmmS3Cr3
   //
   InitSmmS3Cr3 ();
+
+  //
+  // Allocate safe memory in ACPI NVS for AP to execute hlt loop on S3 path
+  //
+  Address = BASE_4GB - 1;
+  Status  = gBS->AllocatePages (
+                   AllocateMaxAddress,
+                   EfiACPIMemoryNVS,
+                   EFI_SIZE_TO_PAGES (sizeof (mApHltLoopCodeTemplate)),
+                   &Address
+                   );
+  ASSERT_EFI_ERROR (Status);
+  mApHltLoopCode = (UINT8 *) (UINTN) Address;
 }
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
index 545b534..8b880d6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file
   SMM CPU misc functions for Ia32 arch specific.
   
-Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -94,3 +94,28 @@ InitGdt (
   *GdtStepSize = GdtTableStepSize;
   return GdtTssTables;
 }
+
+/**
+  Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
+
+  @param[in] ApHltLoopCode    The 32-bit address of the safe hlt-loop function.
+  @param[in] TopOfStack       A pointer to the new stack to use for the ApHltLoopCode.
+
+**/
+VOID
+TransferApToSafeState (
+  IN UINT32             ApHltLoopCode,
+  IN UINT32             TopOfStack
+  )
+{
+  SwitchStack (
+    (SWITCH_STACK_ENTRY_POINT) (UINTN) ApHltLoopCode,
+    NULL,
+    NULL,
+    (VOID *) (UINTN) TopOfStack
+    );
+  //
+  // It should never reach here
+  //
+  ASSERT (FALSE);
+}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 9b119c8..6c98659 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -825,4 +825,17 @@ GetAcpiS3EnableFlag (
   VOID
   );
 
+/**
+  Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
+
+  @param[in] ApHltLoopCode    The 32-bit address of the safe hlt-loop function.
+  @param[in] TopOfStack       A pointer to the new stack to use for the ApHltLoopCode.
+
+**/
+VOID
+TransferApToSafeState (
+  IN UINT32             ApHltLoopCode,
+  IN UINT32             TopOfStack
+  );
+
 #endif
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index b53aa45..bd465c7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file
   SMM CPU misc functions for x64 arch specific.
   
-Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -68,3 +68,30 @@ InitGdt (
   *GdtStepSize = GdtTableStepSize;
   return GdtTssTables;
 }
+
+/**
+  Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.
+
+  @param[in] ApHltLoopCode    The 32-bit address of the safe hlt-loop function.
+  @param[in] TopOfStack       A pointer to the new stack to use for the ApHltLoopCode.
+
+**/
+VOID
+TransferApToSafeState (
+  IN UINT32             ApHltLoopCode,
+  IN UINT32             TopOfStack
+  )
+{
+  SwitchStack (
+    (SWITCH_STACK_ENTRY_POINT) (UINTN) ApHltLoopCode,
+    NULL,
+    NULL,
+    (VOID *) (UINTN) TopOfStack
+    );
+  //
+  // It should never reach here
+  //
+  ASSERT (FALSE);
+}
+
+
-- 
2.9.3.windows.2



  reply	other threads:[~2016-11-11  5:45 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-11  5:45 [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path Jeff Fan
2016-11-11  5:45 ` Jeff Fan [this message]
2016-11-11  5:45 ` [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Place AP to 32bit protected mode " Jeff Fan
2016-11-11  5:45 ` [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Decrease mNumberToFinish in AP safe code Jeff Fan
2016-11-11 10:16   ` Paolo Bonzini
2016-11-11 19:49 ` [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path Laszlo Ersek
2016-11-13 12:51   ` Fan, Jeff
2016-11-14  1:41     ` Yao, Jiewen
2016-11-14  8:17     ` Laszlo Ersek
2016-11-14  8:50       ` Paolo Bonzini
2016-11-14 10:39         ` Laszlo Ersek
2016-11-14 11:09           ` Paolo Bonzini
2016-11-14 11:27             ` Laszlo Ersek
2016-11-14 12:00               ` Paolo Bonzini
2016-11-14 18:07                 ` Laszlo Ersek
2016-11-14 18:13                   ` Paolo Bonzini
2016-11-14 23:56                     ` Laszlo Ersek
2016-11-15  0:47                       ` Fan, Jeff
2016-11-15  1:03                         ` Laszlo Ersek
2016-11-15  1:04                           ` Fan, Jeff
2016-11-15  1:19                       ` Fan, Jeff
2016-11-15  1:30                         ` Laszlo Ersek
2016-11-15  1:27                       ` Laszlo Ersek
2016-11-15  1:38                         ` Fan, Jeff
     [not found] ` <542CF652F8836A4AB8DBFAAD40ED192A4A2DCDE3@shsmsx102.ccr.corp.intel.com>
2016-11-15  1:21   ` Yao, Jiewen
2016-11-15  1:24     ` Fan, Jeff

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=20161111054545.19616-2-jeff.fan@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