From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web10.34227.1661497500083763450 for ; Fri, 26 Aug 2022 00:05:00 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=SFQsC2Vn; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: zhiguang.liu@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1661497500; x=1693033500; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=dA1DNYwoQhz68xpggLd5KGANhXEVIxnLV1HxB/meG7s=; b=SFQsC2VnYF4z01Jw5yS9I5HJb+fFxA2xnAdzM2d8gXbn5CyCJcbAgut6 1MlWzGSKqGxLeXJ8Qf4JM4e1gBYB1h6x6itvrkg30uuohVjRCgIBikHq0 Flky3G8XBYTT14fgPMVUgE9iW1Il/TUHrUecLA2hVaqET+6dxGpA5B8JL Q1pLT7kJulOMiAhb+swu5TBzL9GBaUy2o847HdOoi8Z1uu+bmTrzDis74 D/EDGZbuvLAThTOrtbmKuOx4ZXY80WFsHlW41/uTxLUFSFwq9yr7o834p Ls1EadYOb/AKYVaUc7F7suGfisk58nyLa3nYn15Dx9QC5JqsCL0nza2Qu g==; X-IronPort-AV: E=McAfee;i="6500,9779,10450"; a="320534723" X-IronPort-AV: E=Sophos;i="5.93,264,1654585200"; d="scan'208";a="320534723" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Aug 2022 00:04:59 -0700 X-IronPort-AV: E=Sophos;i="5.93,264,1654585200"; d="scan'208";a="938651862" Received: from shwdesfp01.ccr.corp.intel.com ([10.239.158.151]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Aug 2022 00:04:58 -0700 From: "Zhiguang Liu" To: devel@edk2.groups.io Cc: Zhiguang Liu , Eric Dong , Ray Ni , Rahul Kumar Subject: [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp Date: Fri, 26 Aug 2022 15:04:46 +0800 Message-Id: <20220826070447.2508-2-zhiguang.liu@intel.com> X-Mailer: git-send-email 2.31.1.windows.1 In-Reply-To: <20220826070447.2508-1-zhiguang.liu@intel.com> References: <20220826070447.2508-1-zhiguang.liu@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit When switch bsp, old bsp and new bsp put CR0/CR4 into stack, and put IDT and GDT register into a structure. After they exchange their stack, they restore these registers. This logic is now implemented by assembly code. This patch aims to reuse (Save/Restore)VolatileRegisters function to replace such assembly code for better code readability. Cc: Eric Dong Cc: Ray Ni Cc: Rahul Kumar Signed-off-by: Zhiguang Liu --- .../Library/MpInitLib/Ia32/MpFuncs.nasm | 20 +-------- UefiCpuPkg/Library/MpInitLib/MpLib.c | 35 ++++++++++++++- UefiCpuPkg/Library/MpInitLib/MpLib.h | 43 +++++++++---------- UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 21 --------- 4 files changed, 56 insertions(+), 63 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 28301bb8f0..1d67f510e9 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -284,15 +284,8 @@ ASM_PFX(AsmExchangeRole): ; edi contains OthersInfo pointer mov edi, [ebp + 28h] - ;Store EFLAGS, GDTR and IDTR register to stack + ;Store EFLAGS to stack pushfd - mov eax, cr4 - push eax ; push cr4 firstly - mov eax, cr0 - push eax - - sgdt [esi + CPU_EXCHANGE_ROLE_INFO.Gdtr] - sidt [esi + CPU_EXCHANGE_ROLE_INFO.Idtr] ; Store the its StackPointer mov [esi + CPU_EXCHANGE_ROLE_INFO.StackPointer],esp @@ -308,13 +301,6 @@ WaitForOtherStored: jmp WaitForOtherStored OtherStored: - ; Since another CPU already stored its state, load them - ; load GDTR value - lgdt [edi + CPU_EXCHANGE_ROLE_INFO.Gdtr] - - ; load IDTR value - lidt [edi + CPU_EXCHANGE_ROLE_INFO.Idtr] - ; load its future StackPointer mov esp, [edi + CPU_EXCHANGE_ROLE_INFO.StackPointer] @@ -331,10 +317,6 @@ WaitForOtherLoaded: OtherLoaded: ; since the other CPU already get the data it want, leave this procedure - pop eax - mov cr0, eax - pop eax - mov cr4, eax popfd popad diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 8d1f24370a..041a32e659 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1,7 +1,7 @@ /** @file CPU MP Initialize Library common functions. - Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.
+ Copyright (c) 2016 - 2022, Intel Corporation. All rights reserved.
Copyright (c) 2020, AMD Inc. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent @@ -15,6 +15,29 @@ EFI_GUID mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID; +/** + Save the volatile registers required to be restored following INIT IPI. + + @param[out] VolatileRegisters Returns buffer saved the volatile resisters +**/ +VOID +SaveVolatileRegisters ( + OUT CPU_VOLATILE_REGISTERS *VolatileRegisters + ); + +/** + Restore the volatile registers following INIT IPI. + + @param[in] VolatileRegisters Pointer to volatile resisters + @param[in] IsRestoreDr TRUE: Restore DRx if supported + FALSE: Do not restore DRx +**/ +VOID +RestoreVolatileRegisters ( + IN CPU_VOLATILE_REGISTERS *VolatileRegisters, + IN BOOLEAN IsRestoreDr + ); + /** The function will check if BSP Execute Disable is enabled. @@ -83,7 +106,12 @@ FutureBSPProc ( CPU_MP_DATA *DataInHob; DataInHob = (CPU_MP_DATA *)Buffer; + // + // Save and restore volatile registers when switch BSP + // + SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegisters); AsmExchangeRole (&DataInHob->APInfo, &DataInHob->BSPInfo); + RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRegisters, FALSE); } /** @@ -2233,7 +2261,12 @@ SwitchBSPWorker ( // WakeUpAP (CpuMpData, FALSE, ProcessorNumber, FutureBSPProc, CpuMpData, TRUE); + // + // Save and restore volatile registers when switch BSP + // + SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters); AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData->APInfo); + RestoreVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters, FALSE); // // Set the BSP bit of MSR_IA32_APIC_BASE on new BSP diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index 974fb76019..47b722cb2f 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -68,14 +68,31 @@ typedef struct { UINTN Size; } MICROCODE_PATCH_INFO; +// +// CPU volatile registers around INIT-SIPI-SIPI +// +typedef struct { + UINTN Cr0; + UINTN Cr3; + UINTN Cr4; + UINTN Dr0; + UINTN Dr1; + UINTN Dr2; + UINTN Dr3; + UINTN Dr6; + UINTN Dr7; + IA32_DESCRIPTOR Gdtr; + IA32_DESCRIPTOR Idtr; + UINT16 Tr; +} CPU_VOLATILE_REGISTERS; + // // CPU exchange information for switch BSP // typedef struct { - UINT8 State; // offset 0 - UINTN StackPointer; // offset 4 / 8 - IA32_DESCRIPTOR Gdtr; // offset 8 / 16 - IA32_DESCRIPTOR Idtr; // offset 14 / 26 + UINT8 State; // offset 0 + UINTN StackPointer; // offset 4 / 8 + CPU_VOLATILE_REGISTERS VolatileRegisters; // offset 8 / 16 } CPU_EXCHANGE_ROLE_INFO; // @@ -112,24 +129,6 @@ typedef enum { CpuStateDisabled } CPU_STATE; -// -// CPU volatile registers around INIT-SIPI-SIPI -// -typedef struct { - UINTN Cr0; - UINTN Cr3; - UINTN Cr4; - UINTN Dr0; - UINTN Dr1; - UINTN Dr2; - UINTN Dr3; - UINTN Dr6; - UINTN Dr7; - IA32_DESCRIPTOR Gdtr; - IA32_DESCRIPTOR Idtr; - UINT16 Tr; -} CPU_VOLATILE_REGISTERS; - // // AP related data // diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm index cd95b03da8..b7f8d48504 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm @@ -482,22 +482,13 @@ ASM_PFX(AsmExchangeRole): push r14 push r15 - mov rax, cr0 - push rax - - mov rax, cr4 - push rax - ; rsi contains MyInfo pointer mov rsi, rcx ; rdi contains OthersInfo pointer mov rdi, rdx - ;Store EFLAGS, GDTR and IDTR regiter to stack pushfq - sgdt [rsi + CPU_EXCHANGE_ROLE_INFO.Gdtr] - sidt [rsi + CPU_EXCHANGE_ROLE_INFO.Idtr] ; Store the its StackPointer mov [rsi + CPU_EXCHANGE_ROLE_INFO.StackPointer], rsp @@ -513,12 +504,6 @@ WaitForOtherStored: jmp WaitForOtherStored OtherStored: - ; Since another CPU already stored its state, load them - ; load GDTR value - lgdt [rdi + CPU_EXCHANGE_ROLE_INFO.Gdtr] - - ; load IDTR value - lidt [rdi + CPU_EXCHANGE_ROLE_INFO.Idtr] ; load its future StackPointer mov rsp, [rdi + CPU_EXCHANGE_ROLE_INFO.StackPointer] @@ -538,12 +523,6 @@ OtherLoaded: ; since the other CPU already get the data it want, leave this procedure popfq - pop rax - mov cr4, rax - - pop rax - mov cr0, rax - pop r15 pop r14 pop r13 -- 2.31.1.windows.1