From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A132181DA4 for ; Wed, 9 Nov 2016 22:07:16 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP; 09 Nov 2016 22:07:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,617,1473145200"; d="scan'208";a="784712539" Received: from jfan12-desk.ccr.corp.intel.com ([10.239.9.5]) by FMSMGA003.fm.intel.com with ESMTP; 09 Nov 2016 22:07:18 -0800 From: Jeff Fan To: edk2-devel@lists.01.org Cc: Laszlo Ersek , Paolo Bonzini , Jiewen Yao , Michael D Kinney Date: Thu, 10 Nov 2016 14:07:07 +0800 Message-Id: <20161110060708.13932-2-jeff.fan@intel.com> X-Mailer: git-send-email 2.9.3.windows.2 In-Reply-To: <20161110060708.13932-1-jeff.fan@intel.com> References: <20161110060708.13932-1-jeff.fan@intel.com> Subject: [PATCH 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Put AP into safe hlt-loop code on S3 path X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Nov 2016 06:07:16 -0000 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 Reported-by: Laszlo Ersek Cc: Laszlo Ersek Cc: Paolo Bonzini Cc: Jiewen Yao Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jeff Fan --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 31 +++++++++++++++++++++++++++ UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 25 +++++++++++++++++++++ UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 13 +++++++++++ UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 26 ++++++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index 6a798ef..6f290b8 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,13 @@ MPRendezvousProcedure ( // Count down the number with lock mechanism. // InterlockedDecrement (&mNumberToFinish); + + // + // Place AP into the safe code + // + TopOfStack = (UINT32) (UINTN) Stack + sizeof (Stack); + CopyMem ((VOID *) (UINTN) mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); + TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack); } /** @@ -731,6 +747,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 +791,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..3e99aab 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c @@ -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..82fa5a6 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..c05dec7 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c @@ -68,3 +68,29 @@ 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