From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web12.290.1577686771934306243 for ; Sun, 29 Dec 2019 22:19:32 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: eric.dong@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Dec 2019 22:19:31 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,374,1571727600"; d="scan'208";a="220611685" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga006.jf.intel.com with ESMTP; 29 Dec 2019 22:19:31 -0800 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 29 Dec 2019 22:19:30 -0800 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by FMSMSX155.amr.corp.intel.com (10.18.116.71) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 29 Dec 2019 22:19:30 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.202]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.89]) with mapi id 14.03.0439.000; Mon, 30 Dec 2019 14:19:28 +0800 From: "Dong, Eric" To: "Wu, Hao A" , "devel@edk2.groups.io" CC: "Ni, Ray" , Laszlo Ersek , "Zeng, Star" , "Fu, Siyuan" , "Kinney, Michael D" Subject: Re: [PATCH v4 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA Thread-Topic: [PATCH v4 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA Thread-Index: AQHVvIfa5MjbJMNwiUelBDili7mPiafSORdQ Date: Mon, 30 Dec 2019 06:19:27 +0000 Message-ID: References: <20191227073229.9416-1-hao.a.wu@intel.com> <20191227073229.9416-7-hao.a.wu@intel.com> In-Reply-To: <20191227073229.9416-7-hao.a.wu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong > -----Original Message----- > From: Wu, Hao A > Sent: Friday, December 27, 2019 3:32 PM > To: devel@edk2.groups.io > Cc: Wu, Hao A ; Dong, Eric ; Ni, > Ray ; Laszlo Ersek ; Zeng, Star > ; Fu, Siyuan ; Kinney, Michael > D > Subject: [PATCH v4 6/6] UefiCpuPkg/MpInitLib: Remove redundant > microcode fields in CPU_MP_DATA >=20 > Previous commits have introduced below fields in structure CPU_AP_DATA: >=20 > UINT32 ProcessorSignature; > UINT8 PlatformId; > UINT64 MicrocodeEntryAddr; >=20 > which store the information of: >=20 > A. CPUID > B. Platform ID > C. Detected microcode patch entry address (including the microcode patch > header) >=20 > for each processor within system. >=20 > Therefore, the below fields in structure CPU_MP_DATA: >=20 > UINT32 ProcessorSignature; > UINT32 ProcessorFlags; > UINT64 MicrocodeDataAddress; > UINT32 MicrocodeRevision; >=20 > which store the BSP's information of: >=20 > A. CPUID > B. Platform ID > C. The address and revision of detected microcode patch >=20 > are redundant and can be removed. >=20 > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Star Zeng > Cc: Siyuan Fu > Cc: Michael D Kinney > Signed-off-by: Hao A Wu > --- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 5 -- > UefiCpuPkg/Library/MpInitLib/Microcode.c | 51 ++++++-------------- > 2 files changed, 14 insertions(+), 42 deletions(-) >=20 > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h > b/UefiCpuPkg/Library/MpInitLib/MpLib.h > index 5f50e79744..6609c958ce 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h > @@ -263,11 +263,6 @@ struct _CPU_MP_DATA { > BOOLEAN PeriodicMode; > BOOLEAN TimerInterruptState; >=20 > - UINT32 ProcessorSignature; > - UINT32 ProcessorFlags; > - UINT64 MicrocodeDataAddress; > - UINT32 MicrocodeRevision; > - > // > // Whether need to use Init-Sipi-Sipi to wake up the APs. > // Two cases need to set this value to TRUE. One is in HLT diff --git > a/UefiCpuPkg/Library/MpInitLib/Microcode.c > b/UefiCpuPkg/Library/MpInitLib/Microcode.c > index 74a34c48fa..4ec54b6220 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c > @@ -85,6 +85,7 @@ MicrocodeDetect ( > UINTN Index; > UINT8 PlatformId; > CPUID_VERSION_INFO_EAX Eax; > + CPU_AP_DATA *CpuData; > UINT32 CurrentRevision; > UINT32 LatestRevision; > UINTN TotalSize; > @@ -92,16 +93,9 @@ MicrocodeDetect ( > UINT32 InCompleteCheckSum32; > BOOLEAN CorrectMicrocode; > VOID *MicrocodeData; > - MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr; > - UINT32 ProcessorFlags; > UINT32 ThreadId; > BOOLEAN IsBspCallIn; >=20 > - // > - // set ProcessorFlags to suppress incorrect compiler/analyzer warnings > - // > - ProcessorFlags =3D 0; > - > if (CpuMpData->MicrocodePatchRegionSize =3D=3D 0) { > // > // There is no microcode patches > @@ -127,28 +121,25 @@ MicrocodeDetect ( > } >=20 > ExtendedTableLength =3D 0; > - // > - // Here data of CPUID leafs have not been collected into context buffe= r, so > - // GetProcessorCpuid() cannot be used here to retrieve CPUID 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; > + Eax.Uint32 =3D CpuMpData- > >CpuData[ProcessorNumber].ProcessorSignature; > + PlatformId =3D CpuMpData->CpuData[ProcessorNumber].PlatformId; >=20 > // > // Check whether AP has same processor with BSP. > // If yes, direct use microcode info saved by BSP. > // > if (!IsBspCallIn) { > - if ((CpuMpData->ProcessorSignature =3D=3D Eax.Uint32) && > - (CpuMpData->ProcessorFlags & (1 << PlatformId)) !=3D 0) { > - MicrocodeData =3D (VOID *)(UINTN) CpuMpData- > >MicrocodeDataAddress; > - LatestRevision =3D CpuMpData->MicrocodeRevision; > - goto Done; > + // > + // Get the CPU data for BSP > + // > + CpuData =3D &(CpuMpData->CpuData[CpuMpData->BspNumber]); > + if ((CpuData->ProcessorSignature =3D=3D Eax.Uint32) && > + (CpuData->PlatformId =3D=3D PlatformId) && > + (CpuData->MicrocodeEntryAddr !=3D 0)) { > + MicrocodeEntryPoint =3D (CPU_MICROCODE_HEADER *)(UINTN) CpuData- > >MicrocodeEntryAddr; > + MicrocodeData =3D (VOID *) (MicrocodeEntryPoint + 1); > + LatestRevision =3D MicrocodeEntryPoint->UpdateRevision; > + goto Done; > } > } >=20 > @@ -216,7 +207,6 @@ MicrocodeDetect ( > CheckSum32 +=3D MicrocodeEntryPoint->Checksum; > if (CheckSum32 =3D=3D 0) { > CorrectMicrocode =3D TRUE; > - ProcessorFlags =3D MicrocodeEntryPoint->ProcessorFlags; > } > } else if ((MicrocodeEntryPoint->DataSize !=3D 0) && > (MicrocodeEntryPoint->UpdateRevision > LatestRevision))= { @@ - > 260,7 +250,6 @@ MicrocodeDetect ( > // Find one > // > CorrectMicrocode =3D TRUE; > - ProcessorFlags =3D ExtendedTable->ProcessorFlag; > break; > } > } > @@ -332,18 +321,6 @@ Done: > CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =3D > (UINTN) MicrocodeData - sizeof (CPU_MICROCODE_HEADER); > } > - > - if (IsBspCallIn && (LatestRevision !=3D 0)) { > - // > - // Save BSP processor info and microcode info for later AP use. > - // > - CpuMpData->ProcessorSignature =3D Eax.Uint32; > - CpuMpData->ProcessorFlags =3D ProcessorFlags; > - CpuMpData->MicrocodeDataAddress =3D (UINTN) MicrocodeData; > - CpuMpData->MicrocodeRevision =3D LatestRevision; > - DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], > ProcessorFlags [0x%08x], \ > - MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFl= ags, > (UINTN) MicrocodeData, LatestRevision)); > - } > } >=20 > /** > -- > 2.12.0.windows.1