From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web10.12972.1660636664990320101 for ; Tue, 16 Aug 2022 00:57:45 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=ScNSpclL; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: yuanhao.xie@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1660636665; x=1692172665; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=w0gRf1nVsuZhcBFTavF7VYqyCUyzCYYM/6fZVipm+/k=; b=ScNSpclLcoW2kchXZJTOow7eWjEjP82+sUFC+59z/Pc0a1IJnbn5ADhy FdGcLKbaTRbzmWUIkBAqKNzxeLX/fQ75qkFjI5P7rKWX+rM5HOGFbU/VZ vqZhXIYHqmAoDblMqTD8ipTvbU06McaYIziY2ZZIyVYCUAON7/Cj+zrTo z+RRdnscUvObCs8Uf7J+U/0RegoekVOLbY3xC9TF6q85opTqVYiFNQXC6 qIX7WrfUlUOUFdI36FSxPdVsAE9WG5L0IAzKP6jak53/jwZTXxEjZhlEh 4KFAlH1k0MzWriEg/WmefRNGUw3wWHG2FIFfBdk1rQdztkDIFz3bCcyZQ w==; X-IronPort-AV: E=McAfee;i="6400,9594,10440"; a="293427015" X-IronPort-AV: E=Sophos;i="5.93,240,1654585200"; d="scan'208";a="293427015" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Aug 2022 00:57:44 -0700 X-IronPort-AV: E=Sophos;i="5.93,240,1654585200"; d="scan'208";a="635792324" Received: from shwdeopenlab705.ccr.corp.intel.com ([10.239.182.56]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Aug 2022 00:57:42 -0700 From: "Yuanhao Xie" To: devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Rahul Kumar Subject: [PATCH] UefiCpuPkg: Use Top of each AP's stack to save CpuMpData Date: Tue, 16 Aug 2022 15:57:15 +0800 Message-Id: <20220816075715.932-1-yuanhao.xie@intel.com> X-Mailer: git-send-email 2.36.1.windows.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To remove the dependency of CPU register, 4/8 byte at the top of the stack is occupied for CpuMpData. BIST information is also taken care here. This modification is only for PEI phase, since in DXE phase CpuMpData is accessed via global variable. Signed-off-by: Yuanhao Cc: Eric Dong Cc: Ray Ni Cc: Rahul Kumar --- .../Library/MpInitLib/Ia32/MpFuncs.nasm | 5 +++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 41 ++++++++++++++----- UefiCpuPkg/Library/MpInitLib/MpLib.h | 8 ++++ UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 10 +++-- UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 6 +++ 5 files changed, 56 insertions(+), 14 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 28301bb8f0..4714f2d527 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -179,6 +179,11 @@ ProgramStack: mov esp, dword [edi + CPU_INFO_IN_HOB.ApTopOfStack] CProcedureInvoke: + ; + ; Reserve 4 bytes for storing CpuMpData. + ; Using sub esp instead of push ebp to avoid overwriting the existed CpuMpData + ; + sub esp, 4 push ebp ; push BIST data at top of AP stack xor ebp, ebp ; clear ebp for call stack trace push ebp diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 8d1f24370a..a9188eb59f 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -571,6 +571,7 @@ InitializeApData ( { CPU_INFO_IN_HOB *CpuInfoInHob; MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr; + AP_STACK_DATA *ApStackData; CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob; CpuInfoInHob[ProcessorNumber].InitialApicId = GetInitialApicId (); @@ -578,6 +579,12 @@ InitializeApData ( CpuInfoInHob[ProcessorNumber].Health = BistData; CpuInfoInHob[ProcessorNumber].ApTopOfStack = ApTopOfStack; + // + // AP_STACK_DATA is stored at the top of AP Stack + // + ApStackData = (AP_STACK_DATA *)((UINTN)ApTopOfStack - sizeof (AP_STACK_DATA)); + ApStackData->MpData = CpuMpData; + CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE; CpuMpData->CpuData[ProcessorNumber].CpuHealthy = (BistData == 0) ? TRUE : FALSE; @@ -623,6 +630,7 @@ ApWakeupFunction ( CPU_INFO_IN_HOB *CpuInfoInHob; UINT64 ApTopOfStack; UINTN CurrentApicMode; + AP_STACK_DATA *ApStackData; // // AP finished assembly code and begin to execute C code @@ -648,7 +656,9 @@ ApWakeupFunction ( // This is first time AP wakeup, get BIST information from AP stack // ApTopOfStack = CpuMpData->Buffer + (ProcessorNumber + 1) * CpuMpData->CpuApStackSize; - BistData = *(UINT32 *)((UINTN)ApTopOfStack - sizeof (UINTN)); + ApStackData = (AP_STACK_DATA *)((UINTN)ApTopOfStack - sizeof (AP_STACK_DATA)); + BistData = (UINT32)ApStackData->Bist; + // // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP environment, // to initialize AP in InitConfig path. @@ -1796,30 +1806,41 @@ MpInitLibInitialize ( AsmGetAddressMap (&AddressMap); GetApResetVectorSize (&AddressMap, &ApResetVectorSizeBelow1Mb, &ApResetVectorSizeAbove1Mb); ApStackSize = PcdGet32 (PcdCpuApStackSize); - ApLoopMode = GetApLoopMode (&MonitorFilterSize); + // + // ApStackSize must be power of 2 + // + ASSERT ((ApStackSize & (ApStackSize - 1)) == 0); + ApLoopMode = GetApLoopMode (&MonitorFilterSize); // // Save BSP's Control registers for APs. // SaveVolatileRegisters (&VolatileRegisters); + // + // Allocate extra ApStackSize to let AP stack align on ApStackSize bounday + // BufferSize = ApStackSize * MaxLogicalProcessorNumber; + BufferSize += ApStackSize; BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber; BufferSize += ApResetVectorSizeBelow1Mb; BufferSize = ALIGN_VALUE (BufferSize, 8); BufferSize += VolatileRegisters.Idtr.Limit + 1; BufferSize += sizeof (CPU_MP_DATA); BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))* MaxLogicalProcessorNumber; - MpBuffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize)); + // + // Allocate extra ApStackSize to let stack align on ApStackSize bounday + // + MpBuffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize)); ASSERT (MpBuffer != NULL); ZeroMem (MpBuffer, BufferSize); - Buffer = (UINTN)MpBuffer; + Buffer = ALIGN_VALUE ((UINTN)MpBuffer, ApStackSize); // - // The layout of the Buffer is as below: + // The layout of the Buffer is as below (lower address on top): // - // +--------------------+ <-- Buffer - // AP Stacks (N) + // +--------------------+ <-- Buffer (Pointer of CpuMpData is stored in the top of each AP's stack.) + // AP Stacks (N) (StackTop = (RSP + ApStackSize) & ~ApStackSize)) // +--------------------+ <-- MonitorBuffer // AP Monitor Filters (N) // +--------------------+ <-- BackupBufferAddr (CpuMpData->BackupBuffer) @@ -1827,7 +1848,7 @@ MpInitLibInitialize ( // +--------------------+ // Padding // +--------------------+ <-- ApIdtBase (8-byte boundary) - // AP IDT All APs share one separate IDT. So AP can get address of CPU_MP_DATA from IDT Base. + // AP IDT All APs share one separate IDT. // +--------------------+ <-- CpuMpData // CPU_MP_DATA // +--------------------+ <-- CpuMpData->CpuData @@ -1866,8 +1887,8 @@ MpInitLibInitialize ( // Make sure no memory usage outside of the allocated buffer. // ASSERT ( - (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) == - Buffer + BufferSize + (CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) <= + (UINTN)MpBuffer + BufferSize ); // diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index 974fb76019..69b621a340 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -302,6 +302,14 @@ struct _CPU_MP_DATA { UINT64 GhcbBase; }; +// +// AP_STACK_DATA is stored at the top of each AP stack. +// +typedef struct { + UINTN Bist; + CPU_MP_DATA *MpData; +} AP_STACK_DATA; + #define AP_SAFE_STACK_SIZE 128 #define AP_RESET_STACK_SIZE AP_SAFE_STACK_SIZE diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c index 65400b95a2..e732371ddd 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c @@ -89,7 +89,7 @@ EnableDebugAgent ( /** Get pointer to CPU MP Data structure. For BSP, the pointer is retrieved from HOB. - For AP, the structure is just after IDT. + For AP, the structure is stored in the top of each AP's stack. @return The pointer to CPU MP Data structure. **/ @@ -100,15 +100,17 @@ GetCpuMpData ( { CPU_MP_DATA *CpuMpData; MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; - IA32_DESCRIPTOR Idtr; + UINTN ApTopOfStack; + AP_STACK_DATA *ApStackData; ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); if (ApicBaseMsr.Bits.BSP == 1) { CpuMpData = GetCpuMpDataFromGuidedHob (); ASSERT (CpuMpData != NULL); } else { - AsmReadIdtr (&Idtr); - CpuMpData = (CPU_MP_DATA *)(Idtr.Base + Idtr.Limit + 1); + ApTopOfStack = ALIGN_VALUE ((UINTN)&ApTopOfStack, (UINTN)PcdGet32 (PcdCpuApStackSize)); + ApStackData = (AP_STACK_DATA *)((UINTN)ApTopOfStack- sizeof (AP_STACK_DATA)); + CpuMpData = (CPU_MP_DATA *)ApStackData->MpData; } return CpuMpData; diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm index 1daaa72b1e..322bdd03e6 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm @@ -237,11 +237,17 @@ ProgramStack: mov rsp, qword [rdi + CPU_INFO_IN_HOB.ApTopOfStack] CProcedureInvoke: + ; + ; Reserve 8 bytes for storing CpuMpData. + ; Using sub rsp instead of push rbp to avoid overwriting existed CpuMpData + ; + sub rsp, 8 push rbp ; Push BIST data at top of AP stack xor rbp, rbp ; Clear ebp for call stack trace push rbp mov rbp, rsp + push qword 0 ; Push 8 bytes for alignment mov rax, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitializeFloatingPointUnits)] sub rsp, 20h call rax ; Call assembly function to initialize FPU per UEFI spec -- 2.36.1.windows.1