From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: ray.ni@intel.com) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by groups.io with SMTP; Tue, 16 Jul 2019 23:59:09 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jul 2019 23:59:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,273,1559545200"; d="scan'208";a="342946753" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga005.jf.intel.com with ESMTP; 16 Jul 2019 23:59:08 -0700 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 16 Jul 2019 23:59:07 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx120.amr.corp.intel.com (10.18.124.208) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 16 Jul 2019 23:59:07 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.110]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.55]) with mapi id 14.03.0439.000; Wed, 17 Jul 2019 14:59:06 +0800 From: "Ni, Ray" To: "Kinney, Michael D" CC: "Dong, Eric" , "Zeng, Star" , "Ni, Ray" , "devel@edk2.groups.io" Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to indicate 1st unit in parent scope. Thread-Topic: [edk2-devel] [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to indicate 1st unit in parent scope. Thread-Index: AQHVMuT4gBcpqQHdwUeERLdZATts6KbOcEhQ Date: Wed, 17 Jul 2019 06:59:06 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C233147@SHSMSX104.ccr.corp.intel.com> References: <15AE6662809251E0.3248@groups.io> In-Reply-To: <15AE6662809251E0.3248@groups.io> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Mike, Per last discussion in TianoCore Community Design Meeting https://edk2.groups.io/g/announce/topic/tianocore_community_design/3243873= 8, "FirstX" is the only proposed option and you would like to see whether a b= etter name can be used. Here I listed some candidates: 1. REGISTER_CPU_FEATURE_FIRST_X FirstX 2. REGISTER_CPU_FEATURE_LOGICAL_FIRST LogicalFirst 3. REGISTER_CPU_FEATURE_BSP Bsp 4. REGISTER_CPU_FEATURE_PROCESSOR_0 Processor0 5. REGISTER_CPU_FEATURE_VIRTUAL_FIRST VirtualFirst Which one do you think is more proper? Or do you have any suggestions? Thanks, Ray > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Ni, Ray > Sent: Friday, July 5, 2019 11:51 AM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Zeng, Star ; > Kinney, Michael D > Subject: [edk2-devel] [PATCH] UefiCpuPkg/CpuFeature: Introduce FirstX to > indicate 1st unit in parent scope. >=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 c= alls > SupportFunc() to detect its own capability; 3. BSP calls InitializeFunc(= ) for > each thead/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, w= hat > InitializeFunc() currently does is to do the initialization operation on= ly CPU > physical location Core# is 0. > But in certain platform, Core#0 might be disabled in hardware level whic= h > results the certain package scope feature isn't initialized at all. >=20 > The patch adds a new field FistX to indicate the CPU's location in its p= arent > scope. > FirstX.Package is set for all APs/threads under first package; FirstX.Co= re is set > for all APs/threads under first core of each package; FirstX.Thread is s= et 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 | 19 +++++ > .../CpuFeaturesInitialize.c | 74 +++++++++++++++++++ > 2 files changed, 93 insertions(+) >=20 > diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > index 6f964027be..7c1e122ed7 100644 > --- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > +++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h > @@ -78,6 +78,20 @@ > #define CPU_FEATURE_END MAX_UINT32 > /// @} >=20 > +/// > +/// The bit field to indicate whether the CPU is the first in its paren= t scope. > +/// > +typedef struct { > + UINT32 Thread : 1; > + UINT32 Core : 1; > + UINT32 Module : 1; > + UINT32 Tile : 1; > + UINT32 Die : 1; > + UINT32 Package : 1; > + UINT32 Reserved : 26; > +} REGISTER_CPU_FEATURE_FIRST_X; > + > + > /// > /// CPU Information passed into the SupportFunc and InitializeFunc of t= he > /// RegisterCpuFeature() library function. This structure contains info= rmation > @@ -88,6 +102,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_X FirstX; > /// > /// 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 aff7ad600c..c7858bc3a8 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > @@ -110,6 +110,9 @@ CpuInitDataInitialize ( > EFI_CPU_PHYSICAL_LOCATION *Location; > BOOLEAN *CoresVisited; > UINTN Index; > + UINT32 PackageIndex; > + UINT32 CoreIndex; > + UINT32 FirstIndex; > ACPI_CPU_DATA *AcpiCpuData; > CPU_STATUS_INFORMATION *CpuStatus; > UINT32 *ValidCoreCountPerPackage; > @@ -239,6 +242,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.FirstX > + // > + > + // > + // Set FirstX.Package for each thread belonging to the first package. > + // > + FirstIndex =3D MAX_UINT32; > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + FirstIndex =3D MIN (Location->Package, FirstIndex); } for > + (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + if (Location->Package =3D=3D FirstIndex) { > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Packag= e > =3D 1; > + } > + } > + > + // > + // Set FirstX.Die/Tile/Module for each thread assuming: > + // single Die under each package, single Tile under each Die, single > + Module under each Tile // for (ProcessorNumber =3D 0; ProcessorNumbe= r > + < NumberOfCpus; ProcessorNumber++) { > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Die =3D = 1; > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Tile =3D= 1; > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Module = =3D > + 1; } > + > + for (PackageIndex =3D 0; PackageIndex < CpuStatus->PackageCount; > PackageIndex++) { > + // > + // Set FirstX.Core for each thread belonging to the first core of e= ach > package. > + // > + FirstIndex =3D MAX_UINT32; > + for (ProcessorNumber =3D 0; ProcessorNumber < NumberOfCpus; > ProcessorNumber++) { > + Location =3D &CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location; > + if (Location->Package =3D=3D PackageIndex) { > + FirstIndex =3D MIN (Location->Core, FirstIndex); > + } > + } > + > + 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 FirstIndex) > { > + CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.FirstX.Core= =3D > 1; > + } > + } > + } > + > + for (PackageIndex =3D 0; PackageIndex < CpuStatus->PackageCount; > PackageIndex++) { > + for (CoreIndex =3D 0; CoreIndex < CpuStatus->MaxCoreCount; > CoreIndex++) { > + // > + // Set FirstX.Thread for the first thread of each core. > + // > + FirstIndex =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) { > + FirstIndex =3D MIN (Location->Thread, FirstIndex); > + } > + } > + > + 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 FirstIndex) { > + CpuFeaturesData- > >InitOrder[ProcessorNumber].CpuInfo.FirstX.Thread =3D 1; > + } > + } > + } > + } > } >=20 > /** > -- > 2.21.0.windows.1 >=20 >=20 >=20