From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web09.2942.1580720974051342830 for ; Mon, 03 Feb 2020 01:09:34 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=aWzDWVTS; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580720973; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Gz05GcZUAHmWhoV6UYBmPfgZUTRhgr+lmMfLxLuZfe8=; b=aWzDWVTSojV9Soi/TkOnetU50pBKewW+5IClZlpPGTFNsUdJ+Teqe1KItmH/ltuMrA3MoI h3puktsC2oav/EPevEErT9uAeKo26Vvx2Xu71lJ5kzCewQanuPnT4GGh2HvpQlDAIIOvtM 47w5p6jEmjYjzal5ilefzej2C+P/Awg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-37-dS-KCZmnOB2JWC2ztNa-yA-1; Mon, 03 Feb 2020 04:09:12 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9A4F8800D48; Mon, 3 Feb 2020 09:09:10 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-136.ams2.redhat.com [10.36.116.136]) by smtp.corp.redhat.com (Postfix) with ESMTP id 00B1D8CCC2; Mon, 3 Feb 2020 09:09:08 +0000 (UTC) Subject: Re: [PATCH v1] UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect() To: Hao A Wu , devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Siyuan Fu , Michael D Kinney References: <20200203003438.6724-1-hao.a.wu@intel.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 3 Feb 2020 10:09:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200203003438.6724-1-hao.a.wu@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-MC-Unique: dS-KCZmnOB2JWC2ztNa-yA-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Hello Hao, On 02/03/20 01:34, Hao A Wu wrote: > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498 > > 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 > information for each processor via AsmCpuid() and AsmReadMsr64() calls > respectively. > > 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 for > each AP and the CPUID and PlatformID information will be collected. > > 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: > > CurrentRevision = GetCurrentMicrocodeSignature (); > IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ? TRUE : FALSE; > if (CurrentRevision != 0 && !IsBspCallIn) { > // > // Skip loading microcode if it has been loaded successfully > // > return; > } > > will ensure that the microcode detection and application will be > skipped due to the fact that such process has already been done in the > PEI phase. > > 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. > > This commit will update the logic in MicrocodeDetect() back to always > collecting the CPUID and PlatformID information explicitly. > > 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(-) It seems like I haven't been involved in reviewing either commit fd30b0070773 ("UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA", 2020-01-02) or commit 396e791059f3 ("UefiCpuPkg: Always load microcode patch on AP processor.", 2020-01-08), so I'd like to defer on this patch to the other UefiCpuPkg reviewers. Thanks! Laszlo > 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; > > @@ -115,8 +116,18 @@ MicrocodeDetect ( > } > > ExtendedTableLength = 0; > - Eax.Uint32 = CpuMpData->CpuData[ProcessorNumber].ProcessorSignature; > - PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId; > + // > + // Here data of CPUID leafs have not been collected into context buffer, 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 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID); > + PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId; > + > > // > // Check whether AP has same processor with BSP. >