From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web12.11204.1577267941703447881 for ; Wed, 25 Dec 2019 01:59:01 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: star.zeng@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Dec 2019 01:59:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,354,1571727600"; d="scan'208";a="214721440" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga008.fm.intel.com with ESMTP; 25 Dec 2019 01:59:00 -0800 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 25 Dec 2019 01:59:00 -0800 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by fmsmsx118.amr.corp.intel.com (10.18.116.18) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 25 Dec 2019 01:59:00 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.109]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.236]) with mapi id 14.03.0439.000; Wed, 25 Dec 2019 17:58:58 +0800 From: "Zeng, Star" To: Ray Ni , "devel@edk2.groups.io" CC: "Ni, Ray" , "Dong, Eric" , "Kinney, Michael D" Subject: Re: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit. Thread-Topic: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit. Thread-Index: AQHVpCEqq2O5kWjKlE2gLmpXzSLbhafKyWGQ Date: Wed, 25 Dec 2019 09:58:58 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB048310404960B6@shsmsx102.ccr.corp.intel.com> References: <20191126061550.494828-1-niruiyu@users.noreply.github.com> <20191126061550.494828-4-niruiyu@users.noreply.github.com> In-Reply-To: <20191126061550.494828-4-niruiyu@users.noreply.github.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: star.zeng@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Some feedback added. > -----Original Message----- > From: Ray Ni [mailto:niruiyu@users.noreply.github.com] > Sent: Tuesday, November 26, 2019 2:16 PM > To: devel@edk2.groups.io > Cc: Ni, Ray ; Dong, Eric ; Zeng, S= tar > ; Kinney, Michael D > Subject: [PATCH v2 3/3] UefiCpuPkg/CpuFeature: Introduce First to indicat= e > 1st unit. >=20 > From: Ray Ni >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1584 >=20 > The flow of CPU feature initialization logic is: > 1. BSP calls GetConfigDataFunc() for each thread/AP; > 2. Each thread/AP calls SupportFunc() to detect its own capability; > 3. BSP calls InitializeFunc() for each thread/AP. >=20 > There is a design gap in step #3. For a package scope feature that only > requires one thread of each package does the initialization operation, > what InitializeFunc() currently does is to do the initialization > operation only CPU physical location Core# is 0. > But in certain platform, Core#0 might be disabled in hardware level > which results the certain package scope feature isn't initialized at > all. Need some patches to update individual InitializeFunc() for features. These patches can be a separated patch series. >=20 > The patch adds a new field Fist to indicate the CPU's location in "Firt" should be "First". > its parent scope. > First.Package is set for all APs/threads under first package; > First.Core is set for all APs/threads under first core of each > package; > First.Thread is set for the AP/thread of each core. >=20 > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Star Zeng > Cc: Michael D Kinney > --- > .../Include/Library/RegisterCpuFeaturesLib.h | 36 +++++++++ > .../CpuFeaturesInitialize.c | 74 +++++++++++++++++++ > 2 files changed, 110 insertions(+) >=20 > diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > index d075606cdb..7114c8ce89 100644 > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > @@ -78,6 +78,37 @@ > #define CPU_FEATURE_END MAX_UINT32 > /// @} >=20 > +/// > +/// The bit field to indicate whether the processor is the first in its = parent > scope. > +/// > +typedef struct { > + // > + // Set to 1 when current processor is the first thread in the core it = resides > in. > + // > + UINT32 Thread : 1; > + // > + // Set to 1 when current processor is a thread of the first core in th= e > module it resides in. > + // > + UINT32 Core : 1; > + // > + // Set to 1 when current processor is a thread of the first module in = the tile > it resides in. > + // > + UINT32 Module : 1; > + // > + // Set to 1 when current processor is a thread of the first tile in th= e die it > resides in. > + // > + UINT32 Tile : 1; > + // > + // Set to 1 when current processor is a thread of the first die in the= package > it resides in. > + // > + UINT32 Die : 1; > + // > + // Set to 1 when current processor is a thread of the first package in= the > system. > + // > + UINT32 Package : 1; > + UINT32 Reserved : 26; > +} REGISTER_CPU_FEATURE_FIRST_PROCESSOR; > + > /// > /// CPU Information passed into the SupportFunc and InitializeFunc of th= e > /// RegisterCpuFeature() library function. This structure contains > information > @@ -88,6 +119,11 @@ typedef struct { > /// The package that the CPU resides > /// > EFI_PROCESSOR_INFORMATION ProcessorInfo; > + > + /// > + /// The bit flag indicating whether the CPU is the first > Thread/Core/Module/Tile/Die/Package in its parent scope. > + /// > + REGISTER_CPU_FEATURE_FIRST_PROCESSOR First; > /// > /// The Display Family of the CPU computed from CPUID leaf > CPUID_VERSION_INFO > /// > diff --git > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > index 0a4fcff033..23076fd453 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > @@ -105,6 +105,9 @@ CpuInitDataInitialize ( > EFI_CPU_PHYSICAL_LOCATION *Location; > BOOLEAN *CoresVisited; > UINTN Index; > + UINT32 PackageIndex; > + UINT32 CoreIndex; > + UINT32 First; > ACPI_CPU_DATA *AcpiCpuData; > CPU_STATUS_INFORMATION *CpuStatus; > UINT32 *ValidCoreCountPerPackage; > @@ -234,6 +237,77 @@ CpuInitDataInitialize ( > ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount !=3D NULL); > CpuFeaturesData->CpuFlags.PackageSemaphoreCount =3D AllocateZeroPool > (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount * > CpuStatus->MaxThreadCount); > ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount !=3D NULL); > + > + // > + // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First > + // > + > + // > + // Set First.Package for each thread belonging to the first package. > + // > + First =3D MAX_UINT32; > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + First =3D MIN (Location->Package, First); > + } > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + if (Location->Package =3D=3D First) { > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package = =3D > 1; > + } > + } > + > + // > + // Set First.Die/Tile/Module for each thread assuming: > + // single Die under each package, single Tile under each Die, single = Module > under each Tile This assumption needs to be addressed in this or a separated patch series. > + // > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die =3D 1; > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile =3D 1= ; > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module =3D > 1; > + } > + > + for (PackageIndex =3D 0; PackageIndex < CpuStatus->PackageCount; > PackageIndex++) { > + // > + // Set First.Core for each thread in the first core of each package. > + // > + First =3D MAX_UINT32; > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + if (Location->Package =3D=3D PackageIndex) { Here the code is assuming Location->Package starts from 0 and consecutive. > + First =3D MIN (Location->Core, First); > + } > + } > + > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + if (Location->Package =3D=3D PackageIndex && Location->Core =3D=3D= First) { > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = =3D 1; > + } > + } > + } > + > + for (PackageIndex =3D 0; PackageIndex < CpuStatus->PackageCount; > PackageIndex++) { > + for (CoreIndex =3D 0; CoreIndex < CpuStatus->MaxCoreCount; > CoreIndex++) { > + // > + // Set First.Thread for the first thread of each core. > + // > + First =3D MAX_UINT32; > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + if (Location->Package =3D=3D PackageIndex && Location->Core =3D= =3D > CoreIndex) { Here the code is assuming Location->Package and Location->Core start from 0= and consecutive. We could not have this assumption, this patch is to resolve this assumption= . Thanks, Star > + First =3D MIN (Location->Thread, First); > + } > + } > + > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + if (Location->Package =3D=3D PackageIndex && Location->Core =3D= =3D > CoreIndex && Location->Thread =3D=3D First) { > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thre= ad > =3D 1; > + } > + } > + } > + } > } >=20 > /** > -- > 2.21.0.windows.1