From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 484A52117AE4D for ; Wed, 17 Oct 2018 23:00:43 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Oct 2018 23:00:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,395,1534834800"; d="scan'208";a="98427741" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.11]) ([10.239.9.11]) by fmsmga004.fm.intel.com with ESMTP; 17 Oct 2018 23:00:42 -0700 To: Eric Dong , edk2-devel@lists.01.org Cc: Laszlo Ersek References: <20181017021635.14972-1-eric.dong@intel.com> <20181017021635.14972-7-eric.dong@intel.com> From: "Ni, Ruiyu" Message-ID: Date: Thu, 18 Oct 2018 14:01:55 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181017021635.14972-7-eric.dong@intel.com> Subject: Re: [Patch v2 6/6] UefiCpuPkg/CpuCommonFeaturesLib: Register MSR base on scope Info. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Oct 2018 06:00:44 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/17/2018 10:16 AM, Eric Dong wrote: > Because MSR has scope attribute, driver has no needs to set > MSR for all APs if MSR scope is core or package type. This patch > updates code to base on the MSR scope value to add MSR to the register > table. > > Cc: Ruiyu Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c | 8 +++++ > UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c | 12 +++++++ > .../Library/CpuCommonFeaturesLib/ExecuteDisable.c | 10 ++++++ > .../Library/CpuCommonFeaturesLib/FastStrings.c | 12 +++++++ > .../Library/CpuCommonFeaturesLib/FeatureControl.c | 38 ++++++++++++++++++++++ > .../CpuCommonFeaturesLib/LimitCpuIdMaxval.c | 14 ++++++++ > .../Library/CpuCommonFeaturesLib/MachineCheck.c | 38 ++++++++++++++++++++++ > .../Library/CpuCommonFeaturesLib/MonitorMwait.c | 15 +++++++++ > .../Library/CpuCommonFeaturesLib/PendingBreak.c | 11 +++++++ > UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c | 11 +++++++ > .../Library/CpuCommonFeaturesLib/ProcTrace.c | 11 +++++++ > UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c | 10 ++++++ > 12 files changed, 190 insertions(+) > > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c > index 47116355a8..1beaebe69c 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/C1e.c > @@ -67,6 +67,14 @@ C1eInitialize ( > IN BOOLEAN State > ) > { > + // > + // The scope of C1EEnable bit in the MSR_NEHALEM_POWER_CTL is Package, only program > + // MSR_FEATURE_CONFIG for thread 0 core 0 in each package. > + // > + if ((CpuInfo->ProcessorInfo.Location.Thread != 0) || (CpuInfo->ProcessorInfo.Location.Core != 0)) { > + return RETURN_SUCCESS; > + } > + > CPU_REGISTER_TABLE_WRITE_FIELD ( > ProcessorNumber, > Msr, > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c > index 2038171a14..f30117d2c5 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Eist.c > @@ -69,6 +69,18 @@ EistInitialize ( > IN BOOLEAN State > ) > { > + // > + // The scope of the MSR_IA32_MISC_ENABLE is core for below processor type, only program > + // MSR_IA32_MISC_ENABLE for thread 0 in each core. > + // > + if (IS_ATOM_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_CORE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_CORE2_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > CPU_REGISTER_TABLE_WRITE_FIELD ( > ProcessorNumber, > Msr, > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c > index 921656a1e8..ff06cb9b60 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c > @@ -79,6 +79,16 @@ ExecuteDisableInitialize ( > IN BOOLEAN State > ) > { > + // > + // The scope of the MSR_IA32_EFER is core for below processor type, only program > + // MSR_IA32_EFER for thread 0 in each core. > + // > + if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > CPU_REGISTER_TABLE_WRITE_FIELD ( > ProcessorNumber, > Msr, > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c > index 029bcf87b3..2682093c23 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FastStrings.c > @@ -40,6 +40,18 @@ FastStringsInitialize ( > IN BOOLEAN State > ) > { > + // > + // The scope of FastStrings bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program > + // MSR_IA32_MISC_ENABLE for thread 0 in each core. > + // > + if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_PENTIUM_4_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > CPU_REGISTER_TABLE_WRITE_FIELD ( > ProcessorNumber, > Msr, > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c > index d28c4ec51a..8c1eb5eb4f 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c > @@ -96,6 +96,19 @@ VmxInitialize ( > { > MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister; > > + // > + // The scope of EnableVmxOutsideSmx bit in the MSR_IA32_FEATURE_CONTROL is core for > + // below processor type, only program MSR_IA32_FEATURE_CONTROL for thread 0 in each > + // core. > + // > + if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PLUS_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > ASSERT (ConfigData != NULL); > MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData; > if (MsrRegister[ProcessorNumber].Bits.Lock == 0) { > @@ -171,6 +184,19 @@ LockFeatureControlRegisterInitialize ( > { > MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister; > > + // > + // The scope of Lock bit in the MSR_IA32_FEATURE_CONTROL is core for > + // below processor type, only program MSR_IA32_FEATURE_CONTROL for thread 0 in each > + // core. > + // > + if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PLUS_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > ASSERT (ConfigData != NULL); > MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData; > if (MsrRegister[ProcessorNumber].Bits.Lock == 0) { > @@ -248,6 +274,18 @@ SmxInitialize ( > MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister; > RETURN_STATUS Status; > > + // > + // The scope of Lock bit in the MSR_IA32_FEATURE_CONTROL is core for > + // below processor type, only program MSR_IA32_FEATURE_CONTROL for thread 0 in each > + // core. > + // > + if (IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PLUS_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > Status = RETURN_SUCCESS; > > if (State && (!IsCpuFeatureInSetting (CPU_FEATURE_VMX))) { > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/LimitCpuIdMaxval.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/LimitCpuIdMaxval.c > index 3d41efe9e9..eab1fb538c 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/LimitCpuIdMaxval.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/LimitCpuIdMaxval.c > @@ -70,6 +70,20 @@ LimitCpuidMaxvalInitialize ( > IN BOOLEAN State > ) > { > + // > + // The scope of LimitCpuidMaxval bit in the MSR_IA32_MISC_ENABLE is core for below > + // processor type, only program MSR_IA32_MISC_ENABLE for thread 0 in each core. > + // > + if (IS_PENTIUM_4_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > > + IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_CORE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_CORE2_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > CPU_REGISTER_TABLE_WRITE_FIELD ( > ProcessorNumber, > Msr, > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c > index c4eca062fd..f8bee53819 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c > @@ -140,6 +140,32 @@ McaInitialize ( > MSR_IA32_MCG_CAP_REGISTER McgCap; > UINT32 BankIndex; > > + // > + // The scope of MSR_IA32_MC*_CTL/MSR_IA32_MC*_STATUS is core for below processor type, only program > + // MSR_IA32_MC*_CTL/MSR_IA32_MC*_STATUS for thread 0 in each core. > + // > + if (IS_ATOM_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_SANDY_BRIDGE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_SKYLAKE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_XEON_PHI_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_PENTIUM_4_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_CORE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > + // > + // The scope of MSR_IA32_MC*_CTL/MSR_IA32_MC*_STATUS is package for below processor type, only program > + // MSR_IA32_MC*_CTL/MSR_IA32_MC*_STATUS for thread 0 core 0 in each package. > + // > + if (IS_NEHALEM_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if ((CpuInfo->ProcessorInfo.Location.Thread != 0) || (CpuInfo->ProcessorInfo.Location.Core != 0)) { > + return RETURN_SUCCESS; > + } > + } > + > if (State) { > McgCap.Uint64 = AsmReadMsr64 (MSR_IA32_MCG_CAP); > for (BankIndex = 0; BankIndex < (UINT32) McgCap.Bits.Count; BankIndex++) { > @@ -301,6 +327,18 @@ LmceInitialize ( > { > MSR_IA32_FEATURE_CONTROL_REGISTER *MsrRegister; > > + // > + // The scope of FastStrings bit in the MSR_IA32_MISC_ENABLE is core for below processor type, only program > + // MSR_IA32_MISC_ENABLE for thread 0 in each core. > + // > + if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_PENTIUM_4_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > ASSERT (ConfigData != NULL); > MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData; > if (MsrRegister[ProcessorNumber].Bits.Lock == 0) { > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MonitorMwait.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MonitorMwait.c > index 1d43bd128a..530748bf46 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MonitorMwait.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MonitorMwait.c > @@ -67,6 +67,21 @@ MonitorMwaitInitialize ( > IN BOOLEAN State > ) > { > + // > + // The scope of the MSR_IA32_MISC_ENABLE is core for below processor type, only program > + // MSR_IA32_MISC_ENABLE for thread 0 in each core. > + // > + if (IS_CORE2_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_ATOM_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_PENTIUM_4_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_CORE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > CPU_REGISTER_TABLE_WRITE_FIELD ( > ProcessorNumber, > Msr, > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/PendingBreak.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/PendingBreak.c > index 8cafba4f4a..2e0d2bdeca 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/PendingBreak.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/PendingBreak.c > @@ -74,6 +74,17 @@ PendingBreakInitialize ( > IN BOOLEAN State > ) > { > + // > + // The scope of the MSR_ATOM_IA32_MISC_ENABLE is core for below processor type, only program > + // MSR_ATOM_IA32_MISC_ENABLE for thread 0 in each core. > + // > + // Support function has check the processer type for this feature, no need to check again > + // here. > + // > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + > // > // ATOM, CORE2, CORE, PENTIUM_4 and IS_PENTIUM_M_PROCESSOR have the same MSR index, > // Simply use MSR_ATOM_IA32_MISC_ENABLE here > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c > index 721470cdfe..d6219f4f3f 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Ppin.c > @@ -101,6 +101,17 @@ PpinInitialize ( > return MsrPpinCtrl.Bits.Enable_PPIN == State ? RETURN_SUCCESS : RETURN_DEVICE_ERROR; > } > > + // > + // Support function already check the processor which support PPIN feature, so this function not need > + // to check the processor again. > + // > + // The scope of the MSR_IVY_BRIDGE_PPIN_CTL is package level, only program MSR_IVY_BRIDGE_PPIN_CTL for > + // thread 0 core 0 in each package. > + // > + if ((CpuInfo->ProcessorInfo.Location.Thread != 0) || (CpuInfo->ProcessorInfo.Location.Core != 0)) { > + return RETURN_SUCCESS; > + } > + > CPU_REGISTER_TABLE_WRITE_FIELD ( > ProcessorNumber, > Msr, > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c > index 98490c6777..cf34ad4d1f 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c > @@ -191,6 +191,17 @@ ProcTraceInitialize ( > MSR_IA32_RTIT_OUTPUT_MASK_PTRS_REGISTER OutputMaskPtrsReg; > RTIT_TOPA_TABLE_ENTRY *TopaEntryPtr; > > + // > + // The scope of the MSR_IA32_RTIT_* is core for below processor type, only program > + // MSR_IA32_RTIT_* for thread 0 in each core. > + // > + if (IS_GOLDMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) || > + IS_GOLDMONT_PLUS_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > ProcTraceData = (PROC_TRACE_DATA *) ConfigData; > ASSERT (ProcTraceData != NULL); > > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c > index b4a453c352..342b45f25b 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/X2Apic.c > @@ -102,6 +102,16 @@ X2ApicInitialize ( > { > BOOLEAN *X2ApicEnabled; > > + // > + // The scope of the MSR_IA32_APIC_BASE is core for below processor type, only program > + // MSR_IA32_APIC_BASE for thread 0 in each core. > + // > + if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) { > + if (CpuInfo->ProcessorInfo.Location.Thread != 0) { > + return RETURN_SUCCESS; > + } > + } > + > ASSERT (ConfigData != NULL); > X2ApicEnabled = (BOOLEAN *) ConfigData; > if (X2ApicEnabled[ProcessorNumber]) { > Reviewed-by: Ruiyu Ni -- Thanks, Ray