From: "Ni, Ray" <ray.ni@intel.com>
To: "Lou, Yun" <yun.lou@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Dong, Eric" <eric.dong@intel.com>,
Laszlo Ersek <lersek@redhat.com>,
"Kumar, Rahul1" <rahul1.kumar@intel.com>
Subject: Re: [PATCH v2 1/2] UefiCpuPkg: Refactor initialization of CPU features during S3 resume
Date: Thu, 16 Sep 2021 13:05:37 +0000 [thread overview]
Message-ID: <CO1PR11MB4930511CFF8B38A13507E3498CDC9@CO1PR11MB4930.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210916092712.4340-1-yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@Intel.com>
I merged the patch with the "#ifndef .." removed.
> -----Original Message-----
> From: Lou, Yun <yun.lou@intel.com>
> Sent: Thursday, September 16, 2021 5:27 PM
> To: devel@edk2.groups.io
> Cc: Lou, Yun <yun.lou@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: [PATCH v2 1/2] UefiCpuPkg: Refactor initialization of CPU features during S3 resume
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3621
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3631
>
> Refactor initialization of CPU features during S3 resume.
>
> In addition, the macro ACPI_CPU_DATA_STRUCTURE_UPDATE is used to fix
> incompatibility issue caused by ACPI_CPU_DATA structure update. It will
> be removed after all the platform code uses new ACPI_CPU_DATA structure.
>
> Signed-off-by: Jason Lou <yun.lou@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> ---
> OvmfPkg/CpuS3DataDxe/CpuS3Data.c | 7 +-
> UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 7 +-
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 12 +-
> UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 18 +--
> UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 160 +++++++++++---------
> UefiCpuPkg/Include/AcpiCpuData.h | 91 ++++++-----
> 6 files changed, 167 insertions(+), 128 deletions(-)
>
> diff --git a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c b/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
> index 5ffe1f3cd7..de20d87567 100644
> --- a/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
> +++ b/OvmfPkg/CpuS3DataDxe/CpuS3Data.c
> @@ -9,7 +9,7 @@ number of CPUs reported by the MP Services Protocol, so this module does not
> support hot plug CPUs. This module can be copied into a CPU specific package
>
> and customized if these additional features are required.
>
>
>
> -Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
>
> +Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved.<BR>
>
> Copyright (c) 2015 - 2020, Red Hat, Inc.
>
>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -252,10 +252,7 @@ CpuS3DataInitialize (
> AcpiCpuDataEx->IdtrProfile.Base = (UINTN)Idt;
>
>
>
> if (OldAcpiCpuData != NULL) {
>
> - AcpiCpuData->RegisterTable = OldAcpiCpuData->RegisterTable;
>
> - AcpiCpuData->PreSmmInitRegisterTable = OldAcpiCpuData->PreSmmInitRegisterTable;
>
> - AcpiCpuData->ApLocation = OldAcpiCpuData->ApLocation;
>
> - CopyMem (&AcpiCpuData->CpuStatus, &OldAcpiCpuData->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
>
> + CopyMem (&AcpiCpuData->CpuFeatureInitData, &OldAcpiCpuData->CpuFeatureInitData, sizeof
> (CPU_FEATURE_INIT_DATA));
>
> }
>
>
>
> //
>
> diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> index 078af36cfb..61ec7c44b2 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> @@ -9,7 +9,7 @@ number of CPUs reported by the MP Services Protocol, so this module does not
> support hot plug CPUs. This module can be copied into a CPU specific package
>
> and customized if these additional features are required.
>
>
>
> -Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
>
> +Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved.<BR>
>
> Copyright (c) 2015, Red Hat, Inc.
>
>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -247,10 +247,7 @@ CpuS3DataInitialize (
> AcpiCpuDataEx->IdtrProfile.Base = (UINTN)Idt;
>
>
>
> if (OldAcpiCpuData != NULL) {
>
> - AcpiCpuData->RegisterTable = OldAcpiCpuData->RegisterTable;
>
> - AcpiCpuData->PreSmmInitRegisterTable = OldAcpiCpuData->PreSmmInitRegisterTable;
>
> - AcpiCpuData->ApLocation = OldAcpiCpuData->ApLocation;
>
> - CopyMem (&AcpiCpuData->CpuStatus, &OldAcpiCpuData->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
>
> + CopyMem (&AcpiCpuData->CpuFeatureInitData, &OldAcpiCpuData->CpuFeatureInitData, sizeof
> (CPU_FEATURE_INIT_DATA));
>
> }
>
>
>
> //
>
> diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 57511c4efa..6e2ab79518 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -1,7 +1,7 @@
> /** @file
>
> CPU Features Initialize functions.
>
>
>
> - Copyright (c) 2017 - 2020, Intel Corporation. All rights reserved.<BR>
>
> + Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> **/
>
> @@ -152,10 +152,10 @@ CpuInitDataInitialize (
> ASSERT (AcpiCpuData != NULL);
>
> CpuFeaturesData->AcpiCpuData= AcpiCpuData;
>
>
>
> - CpuStatus = &AcpiCpuData->CpuStatus;
>
> + CpuStatus = &AcpiCpuData->CpuFeatureInitData.CpuStatus;
>
> Location = AllocateZeroPool (sizeof (EFI_CPU_PHYSICAL_LOCATION) * NumberOfCpus);
>
> ASSERT (Location != NULL);
>
> - AcpiCpuData->ApLocation = (EFI_PHYSICAL_ADDRESS)(UINTN)Location;
>
> + AcpiCpuData->CpuFeatureInitData.ApLocation = (EFI_PHYSICAL_ADDRESS)(UINTN)Location;
>
>
>
> for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {
>
> InitOrder = &CpuFeaturesData->InitOrder[ProcessorNumber];
>
> @@ -1131,7 +1131,7 @@ SetProcessorRegister (
> CpuFeaturesData = (CPU_FEATURES_DATA *) Buffer;
>
> AcpiCpuData = CpuFeaturesData->AcpiCpuData;
>
>
>
> - RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->RegisterTable;
>
> + RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->CpuFeatureInitData.RegisterTable;
>
>
>
> InitApicId = GetInitialApicId ();
>
> RegisterTable = NULL;
>
> @@ -1147,8 +1147,8 @@ SetProcessorRegister (
>
>
> ProgramProcessorRegister (
>
> RegisterTable,
>
> - (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)AcpiCpuData->ApLocation + ProcIndex,
>
> - &AcpiCpuData->CpuStatus,
>
> + (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)AcpiCpuData->CpuFeatureInitData.ApLocation + ProcIndex,
>
> + &AcpiCpuData->CpuFeatureInitData.CpuStatus,
>
> &CpuFeaturesData->CpuFlags
>
> );
>
> }
>
> diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> index 60daa5cc87..e6ef9c602d 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -952,8 +952,8 @@ GetAcpiCpuData (
> AcpiCpuData->NumberOfCpus = (UINT32)NumberOfCpus;
>
> }
>
>
>
> - if (AcpiCpuData->RegisterTable == 0 ||
>
> - AcpiCpuData->PreSmmInitRegisterTable == 0) {
>
> + if (AcpiCpuData->CpuFeatureInitData.RegisterTable == 0 ||
>
> + AcpiCpuData->CpuFeatureInitData.PreSmmInitRegisterTable == 0) {
>
> //
>
> // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs
>
> //
>
> @@ -976,11 +976,11 @@ GetAcpiCpuData (
> RegisterTable[NumberOfCpus + Index].AllocatedSize = 0;
>
> RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0;
>
> }
>
> - if (AcpiCpuData->RegisterTable == 0) {
>
> - AcpiCpuData->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
>
> + if (AcpiCpuData->CpuFeatureInitData.RegisterTable == 0) {
>
> + AcpiCpuData->CpuFeatureInitData.RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
>
> }
>
> - if (AcpiCpuData->PreSmmInitRegisterTable == 0) {
>
> - AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
>
> + if (AcpiCpuData->CpuFeatureInitData.PreSmmInitRegisterTable == 0) {
>
> + AcpiCpuData->CpuFeatureInitData.PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable +
> NumberOfCpus);
>
> }
>
> }
>
>
>
> @@ -1063,9 +1063,9 @@ CpuRegisterTableWriteWorker (
> CpuFeaturesData = GetCpuFeaturesData ();
>
> if (CpuFeaturesData->RegisterTable == NULL) {
>
> AcpiCpuData = GetAcpiCpuData ();
>
> - ASSERT ((AcpiCpuData != NULL) && (AcpiCpuData->RegisterTable != 0));
>
> - CpuFeaturesData->RegisterTable = (CPU_REGISTER_TABLE *) (UINTN) AcpiCpuData->RegisterTable;
>
> - CpuFeaturesData->PreSmmRegisterTable = (CPU_REGISTER_TABLE *) (UINTN) AcpiCpuData->PreSmmInitRegisterTable;
>
> + ASSERT ((AcpiCpuData != NULL) && (AcpiCpuData->CpuFeatureInitData.RegisterTable != 0));
>
> + CpuFeaturesData->RegisterTable = (CPU_REGISTER_TABLE *) (UINTN) AcpiCpuData->CpuFeatureInitData.RegisterTable;
>
> + CpuFeaturesData->PreSmmRegisterTable = (CPU_REGISTER_TABLE *) (UINTN) AcpiCpuData-
> >CpuFeatureInitData.PreSmmInitRegisterTable;
>
> }
>
>
>
> if (PreSmmFlag) {
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index ab7f39aa2b..2873cba083 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> @@ -476,16 +476,19 @@ SetRegister (
> IN BOOLEAN PreSmmRegisterTable
>
> )
>
> {
>
> + CPU_FEATURE_INIT_DATA *FeatureInitData;
>
> CPU_REGISTER_TABLE *RegisterTable;
>
> CPU_REGISTER_TABLE *RegisterTables;
>
> UINT32 InitApicId;
>
> UINTN ProcIndex;
>
> UINTN Index;
>
>
>
> + FeatureInitData = &mAcpiCpuData.CpuFeatureInitData;
>
> +
>
> if (PreSmmRegisterTable) {
>
> - RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)mAcpiCpuData.PreSmmInitRegisterTable;
>
> + RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)FeatureInitData->PreSmmInitRegisterTable;
>
> } else {
>
> - RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)mAcpiCpuData.RegisterTable;
>
> + RegisterTables = (CPU_REGISTER_TABLE *)(UINTN)FeatureInitData->RegisterTable;
>
> }
>
> if (RegisterTables == NULL) {
>
> return;
>
> @@ -503,18 +506,18 @@ SetRegister (
> }
>
> ASSERT (RegisterTable != NULL);
>
>
>
> - if (mAcpiCpuData.ApLocation != 0) {
>
> + if (FeatureInitData->ApLocation != 0) {
>
> ProgramProcessorRegister (
>
> RegisterTable,
>
> - (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)mAcpiCpuData.ApLocation + ProcIndex,
>
> - &mAcpiCpuData.CpuStatus,
>
> + (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)FeatureInitData->ApLocation + ProcIndex,
>
> + &FeatureInitData->CpuStatus,
>
> &mCpuFlags
>
> );
>
> } else {
>
> ProgramProcessorRegister (
>
> RegisterTable,
>
> NULL,
>
> - &mAcpiCpuData.CpuStatus,
>
> + &FeatureInitData->CpuStatus,
>
> &mCpuFlags
>
> );
>
> }
>
> @@ -1010,6 +1013,71 @@ IsRegisterTableEmpty (
> return TRUE;
>
> }
>
>
>
> +/**
>
> + Copy the data used to initialize processor register into SMRAM.
>
> +
>
> + @param[in,out] CpuFeatureInitDataDst Pointer to the destination CPU_FEATURE_INIT_DATA structure.
>
> + @param[in] CpuFeatureInitDataSrc Pointer to the source CPU_FEATURE_INIT_DATA structure.
>
> +
>
> +**/
>
> +VOID
>
> +CopyCpuFeatureInitDatatoSmram (
>
> + IN OUT CPU_FEATURE_INIT_DATA *CpuFeatureInitDataDst,
>
> + IN CPU_FEATURE_INIT_DATA *CpuFeatureInitDataSrc
>
> + )
>
> +{
>
> + CPU_STATUS_INFORMATION *CpuStatus;
>
> +
>
> + if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable,
> mAcpiCpuData.NumberOfCpus)) {
>
> + CpuFeatureInitDataDst->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool
> (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
>
> + ASSERT (CpuFeatureInitDataDst->PreSmmInitRegisterTable != 0);
>
> +
>
> + CopyRegisterTable (
>
> + (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->PreSmmInitRegisterTable,
>
> + (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->PreSmmInitRegisterTable,
>
> + mAcpiCpuData.NumberOfCpus
>
> + );
>
> + }
>
> +
>
> + if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable,
> mAcpiCpuData.NumberOfCpus)) {
>
> + CpuFeatureInitDataDst->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus *
> sizeof (CPU_REGISTER_TABLE));
>
> + ASSERT (CpuFeatureInitDataDst->RegisterTable != 0);
>
> +
>
> + CopyRegisterTable (
>
> + (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataDst->RegisterTable,
>
> + (CPU_REGISTER_TABLE *)(UINTN)CpuFeatureInitDataSrc->RegisterTable,
>
> + mAcpiCpuData.NumberOfCpus
>
> + );
>
> + }
>
> +
>
> + CpuStatus = &CpuFeatureInitDataDst->CpuStatus;
>
> + CopyMem (CpuStatus, &CpuFeatureInitDataSrc->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
>
> +
>
> + if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage != 0) {
>
> + CpuStatus->ThreadCountPerPackage = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
>
> + sizeof (UINT32) * CpuStatus->PackageCount,
>
> + (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerPackage
>
> + );
>
> + ASSERT (CpuStatus->ThreadCountPerPackage != 0);
>
> + }
>
> +
>
> + if (CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore != 0) {
>
> + CpuStatus->ThreadCountPerCore = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
>
> + sizeof (UINT8) * (CpuStatus->PackageCount * CpuStatus->MaxCoreCount),
>
> + (UINT32 *)(UINTN)CpuFeatureInitDataSrc->CpuStatus.ThreadCountPerCore
>
> + );
>
> + ASSERT (CpuStatus->ThreadCountPerCore != 0);
>
> + }
>
> +
>
> + if (CpuFeatureInitDataSrc->ApLocation != 0) {
>
> + CpuFeatureInitDataDst->ApLocation = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
>
> + mAcpiCpuData.NumberOfCpus * sizeof (EFI_CPU_PHYSICAL_LOCATION),
>
> + (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)CpuFeatureInitDataSrc->ApLocation
>
> + );
>
> + ASSERT (CpuFeatureInitDataDst->ApLocation != 0);
>
> + }
>
> +}
>
> +
>
> /**
>
> Get ACPI CPU data.
>
>
>
> @@ -1064,39 +1132,13 @@ GetAcpiCpuData (
>
>
> CopyMem ((VOID *)(UINTN)mAcpiCpuData.IdtrProfile, (VOID *)(UINTN)AcpiCpuData->IdtrProfile, sizeof (IA32_DESCRIPTOR));
>
>
>
> - if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->PreSmmInitRegisterTable,
> mAcpiCpuData.NumberOfCpus)) {
>
> - mAcpiCpuData.PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus *
> sizeof (CPU_REGISTER_TABLE));
>
> - ASSERT (mAcpiCpuData.PreSmmInitRegisterTable != 0);
>
> -
>
> - CopyRegisterTable (
>
> - (CPU_REGISTER_TABLE *)(UINTN)mAcpiCpuData.PreSmmInitRegisterTable,
>
> - (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->PreSmmInitRegisterTable,
>
> - mAcpiCpuData.NumberOfCpus
>
> - );
>
> - } else {
>
> - mAcpiCpuData.PreSmmInitRegisterTable = 0;
>
> - }
>
> -
>
> - if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->RegisterTable, mAcpiCpuData.NumberOfCpus)) {
>
> - mAcpiCpuData.RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (mAcpiCpuData.NumberOfCpus * sizeof
> (CPU_REGISTER_TABLE));
>
> - ASSERT (mAcpiCpuData.RegisterTable != 0);
>
> -
>
> - CopyRegisterTable (
>
> - (CPU_REGISTER_TABLE *)(UINTN)mAcpiCpuData.RegisterTable,
>
> - (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->RegisterTable,
>
> - mAcpiCpuData.NumberOfCpus
>
> - );
>
> - } else {
>
> - mAcpiCpuData.RegisterTable = 0;
>
> - }
>
> -
>
> //
>
> // Copy AP's GDT, IDT and Machine Check handler into SMRAM.
>
> //
>
> Gdtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.GdtrProfile;
>
> Idtr = (IA32_DESCRIPTOR *)(UINTN)mAcpiCpuData.IdtrProfile;
>
>
>
> - GdtForAp = AllocatePool ((Gdtr->Limit + 1) + (Idtr->Limit + 1) + mAcpiCpuData.ApMachineCheckHandlerSize);
>
> + GdtForAp = AllocatePool ((Gdtr->Limit + 1) + (Idtr->Limit + 1) + mAcpiCpuData.ApMachineCheckHandlerSize);
>
> ASSERT (GdtForAp != NULL);
>
> IdtForAp = (VOID *) ((UINTN)GdtForAp + (Gdtr->Limit + 1));
>
> MachineCheckHandlerForAp = (VOID *) ((UINTN)IdtForAp + (Idtr->Limit + 1));
>
> @@ -1109,41 +1151,23 @@ GetAcpiCpuData (
> Idtr->Base = (UINTN)IdtForAp;
>
> mAcpiCpuData.ApMachineCheckHandlerBase = (EFI_PHYSICAL_ADDRESS)(UINTN)MachineCheckHandlerForAp;
>
>
>
> - CpuStatus = &mAcpiCpuData.CpuStatus;
>
> - CopyMem (CpuStatus, &AcpiCpuData->CpuStatus, sizeof (CPU_STATUS_INFORMATION));
>
> - if (AcpiCpuData->CpuStatus.ThreadCountPerPackage != 0) {
>
> - CpuStatus->ThreadCountPerPackage = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
>
> - sizeof (UINT32) * CpuStatus->PackageCount,
>
> - (UINT32 *)(UINTN)AcpiCpuData->CpuStatus.ThreadCountPerPackage
>
> - );
>
> - ASSERT (CpuStatus->ThreadCountPerPackage != 0);
>
> - }
>
> - if (AcpiCpuData->CpuStatus.ThreadCountPerCore != 0) {
>
> - CpuStatus->ThreadCountPerCore = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
>
> - sizeof (UINT8) * (CpuStatus->PackageCount * CpuStatus->MaxCoreCount),
>
> - (UINT32 *)(UINTN)AcpiCpuData->CpuStatus.ThreadCountPerCore
>
> - );
>
> - ASSERT (CpuStatus->ThreadCountPerCore != 0);
>
> - }
>
> - if (AcpiCpuData->ApLocation != 0) {
>
> - mAcpiCpuData.ApLocation = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateCopyPool (
>
> - mAcpiCpuData.NumberOfCpus * sizeof (EFI_CPU_PHYSICAL_LOCATION),
>
> - (EFI_CPU_PHYSICAL_LOCATION *)(UINTN)AcpiCpuData->ApLocation
>
> - );
>
> - ASSERT (mAcpiCpuData.ApLocation != 0);
>
> - }
>
> - if (CpuStatus->PackageCount != 0) {
>
> - mCpuFlags.CoreSemaphoreCount = AllocateZeroPool (
>
> - sizeof (UINT32) * CpuStatus->PackageCount *
>
> - CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
>
> - );
>
> - ASSERT (mCpuFlags.CoreSemaphoreCount != NULL);
>
> - mCpuFlags.PackageSemaphoreCount = AllocateZeroPool (
>
> - sizeof (UINT32) * CpuStatus->PackageCount *
>
> - CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
>
> - );
>
> - ASSERT (mCpuFlags.PackageSemaphoreCount != NULL);
>
> - }
>
> + ZeroMem (&mAcpiCpuData.CpuFeatureInitData, sizeof (CPU_FEATURE_INIT_DATA));
>
> + CopyCpuFeatureInitDatatoSmram (&mAcpiCpuData.CpuFeatureInitData, &AcpiCpuData->CpuFeatureInitData);
>
> +
>
> + CpuStatus = &mAcpiCpuData.CpuFeatureInitData.CpuStatus;
>
> +
>
> + mCpuFlags.CoreSemaphoreCount = AllocateZeroPool (
>
> + sizeof (UINT32) * CpuStatus->PackageCount *
>
> + CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
>
> + );
>
> + ASSERT (mCpuFlags.CoreSemaphoreCount != NULL);
>
> +
>
> + mCpuFlags.PackageSemaphoreCount = AllocateZeroPool (
>
> + sizeof (UINT32) * CpuStatus->PackageCount *
>
> + CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount
>
> + );
>
> + ASSERT (mCpuFlags.PackageSemaphoreCount != NULL);
>
> +
>
> InitializeSpinLock((SPIN_LOCK*) &mCpuFlags.MemoryMappedLock);
>
> }
>
>
>
> diff --git a/UefiCpuPkg/Include/AcpiCpuData.h b/UefiCpuPkg/Include/AcpiCpuData.h
> index 62a01b2c6b..2fa8801d1f 100644
> --- a/UefiCpuPkg/Include/AcpiCpuData.h
> +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> @@ -1,7 +1,7 @@
> /** @file
>
> Definitions for CPU S3 data.
>
>
>
> -Copyright (c) 2013 - 2020, Intel Corporation. All rights reserved.<BR>
>
> +Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> **/
>
> @@ -9,6 +9,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #ifndef _ACPI_CPU_DATA_H_
>
> #define _ACPI_CPU_DATA_H_
>
>
>
> +//
>
> +// This macro definition is used to fix incompatibility issue caused by
>
> +// ACPI_CPU_DATA structure update. It will be removed after all the platform
>
> +// code uses new ACPI_CPU_DATA structure.
>
> +//
>
> +#ifndef ACPI_CPU_DATA_STRUCTURE_UPDATE
>
> +#define ACPI_CPU_DATA_STRUCTURE_UPDATE
>
> +#endif
>
> +
>
> //
>
> // Register types in register table
>
> //
>
> @@ -118,6 +127,49 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS RegisterTableEntry;
>
> } CPU_REGISTER_TABLE;
>
>
>
> +//
>
> +// Data structure that is used for CPU feature initialization during ACPI S3
>
> +// resume.
>
> +//
>
> +typedef struct {
>
> + //
>
> + // Physical address of an array of CPU_REGISTER_TABLE structures, with
>
> + // NumberOfCpus entries. If a register table is not required, then the
>
> + // TableLength and AllocatedSize fields of CPU_REGISTER_TABLE are set to 0.
>
> + // If TableLength is > 0, then elements of RegisterTableEntry are used to
>
> + // initialize the CPU that matches InitialApicId, during an ACPI S3 resume,
>
> + // before SMBASE relocation is performed.
>
> + // If a register table is not required for any one of the CPUs, then
>
> + // PreSmmInitRegisterTable may be set to 0.
>
> + //
>
> + EFI_PHYSICAL_ADDRESS PreSmmInitRegisterTable;
>
> + //
>
> + // Physical address of an array of CPU_REGISTER_TABLE structures, with
>
> + // NumberOfCpus entries. If a register table is not required, then the
>
> + // TableLength and AllocatedSize fields of CPU_REGISTER_TABLE are set to 0.
>
> + // If TableLength is > 0, then elements of RegisterTableEntry are used to
>
> + // initialize the CPU that matches InitialApicId, during an ACPI S3 resume,
>
> + // after SMBASE relocation is performed.
>
> + // If a register table is not required for any one of the CPUs, then
>
> + // RegisterTable may be set to 0.
>
> + //
>
> + EFI_PHYSICAL_ADDRESS RegisterTable;
>
> + //
>
> + // CPU information which is required when set the register table.
>
> + //
>
> + CPU_STATUS_INFORMATION CpuStatus;
>
> + //
>
> + // Location info for each AP.
>
> + // It points to an array which saves all APs location info.
>
> + // The array count is the AP count in this CPU.
>
> + //
>
> + // If the platform does not support MSR setting at S3 resume, and
>
> + // therefore it doesn't need the dependency semaphores, it should set
>
> + // this field to 0.
>
> + //
>
> + EFI_PHYSICAL_ADDRESS ApLocation;
>
> +} CPU_FEATURE_INIT_DATA;
>
> +
>
> //
>
> // Data structure that is required for ACPI S3 resume. The PCD
>
> // PcdCpuS3DataAddress must be set to the physical address where this structure
>
> @@ -172,28 +224,6 @@ typedef struct {
> //
>
> EFI_PHYSICAL_ADDRESS MtrrTable;
>
> //
>
> - // Physical address of an array of CPU_REGISTER_TABLE structures, with
>
> - // NumberOfCpus entries. If a register table is not required, then the
>
> - // TableLength and AllocatedSize fields of CPU_REGISTER_TABLE are set to 0.
>
> - // If TableLength is > 0, then elements of RegisterTableEntry are used to
>
> - // initialize the CPU that matches InitialApicId, during an ACPI S3 resume,
>
> - // before SMBASE relocation is performed.
>
> - // If a register table is not required for any one of the CPUs, then
>
> - // PreSmmInitRegisterTable may be set to 0.
>
> - //
>
> - EFI_PHYSICAL_ADDRESS PreSmmInitRegisterTable;
>
> - //
>
> - // Physical address of an array of CPU_REGISTER_TABLE structures, with
>
> - // NumberOfCpus entries. If a register table is not required, then the
>
> - // TableLength and AllocatedSize fields of CPU_REGISTER_TABLE are set to 0.
>
> - // If TableLength is > 0, then elements of RegisterTableEntry are used to
>
> - // initialize the CPU that matches InitialApicId, during an ACPI S3 resume,
>
> - // after SMBASE relocation is performed.
>
> - // If a register table is not required for any one of the CPUs, then
>
> - // RegisterTable may be set to 0.
>
> - //
>
> - EFI_PHYSICAL_ADDRESS RegisterTable;
>
> - //
>
> // Physical address of a buffer that contains the machine check handler that
>
> // is used during an ACPI S3 Resume. In order for this machine check
>
> // handler to be active on an AP during an ACPI S3 resume, the machine check
>
> @@ -208,19 +238,10 @@ typedef struct {
> //
>
> UINT32 ApMachineCheckHandlerSize;
>
> //
>
> - // CPU information which is required when set the register table.
>
> - //
>
> - CPU_STATUS_INFORMATION CpuStatus;
>
> - //
>
> - // Location info for each AP.
>
> - // It points to an array which saves all APs location info.
>
> - // The array count is the AP count in this CPU.
>
> - //
>
> - // If the platform does not support MSR setting at S3 resume, and
>
> - // therefore it doesn't need the dependency semaphores, it should set
>
> - // this field to 0.
>
> + // Data structure that is used for CPU feature initialization during ACPI S3
>
> + // resume.
>
> //
>
> - EFI_PHYSICAL_ADDRESS ApLocation;
>
> + CPU_FEATURE_INIT_DATA CpuFeatureInitData;
>
> } ACPI_CPU_DATA;
>
>
>
> #endif
>
> --
> 2.28.0.windows.1
prev parent reply other threads:[~2021-09-16 13:05 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-16 9:27 [PATCH v2 1/2] UefiCpuPkg: Refactor initialization of CPU features during S3 resume Jason Lou
2021-09-16 9:27 ` [PATCH v2 2/2] UefiCpuPkg: Prevent from re-initializing " Jason Lou
2021-09-16 13:05 ` Ni, Ray
2021-09-16 13:05 ` Ni, Ray [this message]
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=CO1PR11MB4930511CFF8B38A13507E3498CDC9@CO1PR11MB4930.namprd11.prod.outlook.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