From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web10.5690.1580953175645323918 for ; Wed, 05 Feb 2020 17:39:36 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Feb 2020 17:39:34 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,407,1574150400"; d="scan'208";a="430352948" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 05 Feb 2020 17:39:34 -0800 Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 5 Feb 2020 17:39:34 -0800 Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 5 Feb 2020 17:39:33 -0800 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 5 Feb 2020 17:39:33 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.5]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.141]) with mapi id 14.03.0439.000; Thu, 6 Feb 2020 09:39:31 +0800 From: "Wu, Hao A" To: "Fu, Siyuan" , "Dong, Eric" , "devel@edk2.groups.io" CC: "Ni, Ray" , Laszlo Ersek , "Kinney, Michael D" Subject: Re: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect() Thread-Topic: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect() Thread-Index: AQHV2im939zrOXHAjkSFJHCdl4lEXagKiKAAgALfThA= Date: Thu, 6 Feb 2020 01:39:30 +0000 Message-ID: References: <20200203003438.6724-1-hao.a.wu@intel.com> In-Reply-To: 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: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks Siyuan and Eric, The patch has been pushed via commit a9e3458ba7. Best Regards, Hao Wu > -----Original Message----- > From: Dong, Eric > Sent: Tuesday, February 04, 2020 9:47 PM > To: devel@edk2.groups.io; Wu, Hao A > Cc: Ni, Ray; Laszlo Ersek; Fu, Siyuan; Kinney, Michael D > Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get > CPUID & PlatformID in MicrocodeDetect() >=20 > Reviewed-by: Eric Dong >=20 > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Wu, Hao > A > Sent: Monday, February 3, 2020 8:35 AM > To: devel@edk2.groups.io > Cc: Wu, Hao A ; Dong, Eric ; > Ni, Ray ; Laszlo Ersek ; Fu, Siyuan > ; Kinney, Michael D > Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID = & > PlatformID in MicrocodeDetect() >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2498 >=20 > Commit fd30b00707 updated the logic in function MicrocodeDetect() that > will directly use the CPUID and PlatformID information from the 'CpuData= ' > field in the CPU_MP_DATA structure, instead of collecting these informat= ion > for each processor via AsmCpuid() and AsmReadMsr64() calls respectively. >=20 > At that moment, this approach worked fine for APs. Since: > a) When the APs are waken up for the 1st time (1st MpInitLibInitialize() > entry at PEI phase), the function InitializeApData() will be called f= or > each AP and the CPUID and PlatformID information will be collected. >=20 > b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the > APs are waken up again, the function InitializeApData() will not be > called, which means the CPUID and PlatformID information will not be > collected. However, the below logics in MicrocodeDetect() function: >=20 > CurrentRevision =3D GetCurrentMicrocodeSignature (); > IsBspCallIn =3D (ProcessorNumber =3D=3D (UINTN)CpuMpData->BspNumbe= r) ? > TRUE : FALSE; > if (CurrentRevision !=3D 0 && !IsBspCallIn) { > // > // Skip loading microcode if it has been loaded successfully > // > return; > } >=20 > will ensure that the microcode detection and application will be > skipped due to the fact that such process has already been done in th= e > PEI phase. >=20 > But after commit 396e791059, which removes the above skip loading logic, > the CPUID and PlatformID information on APs will be used upon the 2nd > entry of the MpInitLibInitialize(). But since the CPUID and PlatformID > information has not been collected, it will bring issue to the microcode > detection process. >=20 > This commit will update the logic in MicrocodeDetect() back to always > collecting the CPUID and PlatformID information explicitly. >=20 > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Siyuan Fu > Cc: Michael D Kinney > Signed-off-by: Hao A Wu > --- > UefiCpuPkg/Library/MpInitLib/Microcode.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) >=20 > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c > b/UefiCpuPkg/Library/MpInitLib/Microcode.c > index b6675b9a60..67e214d463 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c > @@ -93,6 +93,7 @@ MicrocodeDetect ( > UINT32 InCompleteCheckSum32; > BOOLEAN CorrectMicrocode; > VOID *MicrocodeData; > + MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr; > UINT32 ThreadId; > BOOLEAN IsBspCallIn; >=20 > @@ -115,8 +116,18 @@ MicrocodeDetect ( > } >=20 > ExtendedTableLength =3D 0; > - Eax.Uint32 =3D CpuMpData- > >CpuData[ProcessorNumber].ProcessorSignature; > - PlatformId =3D CpuMpData->CpuData[ProcessorNumber].PlatformId; > + // > + // Here data of CPUID leafs have not been collected into context > + buffer, so // GetProcessorCpuid() cannot be used here to retrieve CPU= ID > data. > + // > + AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL); > + > + // > + // The index of platform information resides in bits 50:52 of MSR > + IA32_PLATFORM_ID // > + PlatformIdMsr.Uint64 =3D AsmReadMsr64 (MSR_IA32_PLATFORM_ID); > + PlatformId =3D (UINT8) PlatformIdMsr.Bits.PlatformId; > + >=20 > // > // Check whether AP has same processor with BSP. > -- > 2.12.0.windows.1 >=20 >=20 >=20