From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web12.172.1582782934015570525 for ; Wed, 26 Feb 2020 21:55:34 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Feb 2020 21:55:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,490,1574150400"; d="scan'208";a="238292922" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga003.jf.intel.com with ESMTP; 26 Feb 2020 21:55:31 -0800 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 26 Feb 2020 21:55:30 -0800 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 26 Feb 2020 21:55:30 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.206]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.86]) with mapi id 14.03.0439.000; Thu, 27 Feb 2020 13:55:28 +0800 From: "Ni, Ray" To: "Duran, Leo" , Laszlo Ersek , "devel@edk2.groups.io" , "Wu, Hao A" , "Fu, Siyuan" CC: "Dong, Eric" Subject: Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpInitLib Thread-Topic: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpInitLib Thread-Index: AQHV7B0pGPIUTxhG5EqTSXQjAjWqRagsIFiAgAD60nD///hQAIAABf+AgAAJk4CAAAVYgIAAEk2AgAABuACAAU/CYA== Date: Thu, 27 Feb 2020 05:55:28 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C463F32@SHSMSX104.ccr.corp.intel.com> References: <1582659566-9893-1-git-send-email-leo.duran@amd.com> <734D49CCEBEEF84792F5B80ED585239D5C4542DA@SHSMSX104.ccr.corp.intel.com> <444c59ea-70dc-0edd-d680-add054dad2c5@redhat.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action 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 Leo and all, I replied in https://bugzilla.tianocore.org/show_bug.cgi?id=3D2556 for a mo= re general question about how uCode is used in AMD processors. Because this package recently exposed a new interface ShadowMicrocodePpi, I= try to involve Leo in the review from AMD uCode's needs. Thanks, Ray > -----Original Message----- > From: Duran, Leo > Sent: Thursday, February 27, 2020 1:52 AM > To: Laszlo Ersek ; Ni, Ray ; devel@e= dk2.groups.io; Wu, Hao A > ; Fu, Siyuan > Cc: Dong, Eric > Subject: RE: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpIn= itLib >=20 >=20 >=20 > > -----Original Message----- > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > Sent: Wednesday, February 26, 2020 12:45 PM > > To: Duran, Leo ; Ni, Ray ; > > devel@edk2.groups.io; Wu, Hao A ; Fu, Siyuan > > > > Cc: Dong, Eric > > Subject: Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in > > MpInitLib > > > > On 02/26/20 17:39, Duran, Leo wrote: > > > > > > > > >> -----Original Message----- > > >> From: Laszlo Ersek [mailto:lersek@redhat.com] > > >> Sent: Wednesday, February 26, 2020 11:21 AM > > >> To: Duran, Leo ; Ni, Ray ; > > >> devel@edk2.groups.io; Wu, Hao A ; Fu, Siyuan > > >> > > >> Cc: Dong, Eric > > >> Subject: Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in > > >> MpInitLib > > >> > > >> On 02/26/20 16:46, Duran, Leo wrote: > > >>> BTW, > > >>> > > >>> I also considered adding a flag to CPU_MP_DATA to make the usage of > > >> PlatformId a bit more explicit. > > >>> E.g., something like CpuMpData- > > >>> CpuData[ProcessorNumber].IsValidPlatformId... So the init code woul= d > > >>> look > > >> like this: > > >>> > > >>> // > > >>> // NOTE: PlatformId is not relevant on AMD platforms. > > >>> // > > >>> if (StandardSignatureIsAuthenticAMD ()) { > > >>> CpuMpData->CpuData[ProcessorNumber].IsValidPlatformId =3D FALSE= ; > > >>> else { > > >>> PlatformIdMsr.Uint64 =3D AsmReadMsr64 (MSR_IA32_PLATFORM_ID); > > >>> CpuMpData->CpuData[ProcessorNumber].PlatformId =3D > > >> (UINT8)PlatformIdMsr.Bits.PlatformId; > > >>> CpuMpData->CpuData[ProcessorNumber].IsValidPlatformId =3D TRUE; > > >>> } > > >>> > > >>> This way "IsValidPlatformId" could be checked prior to using "Platf= ormId". > > >>> Anyway, that seemed a bit overkill, so I opted against it... though= ts? > > >> > > >> I think a global flag is justified; in the above approach, "IsValidP= latformId" > > >> would not vary across "ProcessorNumber", so it does look like useles= s > > >> generality. > > > [Duran, Leo] > > > Great point, Laszlo. > > > Indeed, global makes senses in the case! > > > I can prepare a v2-set to incorporate that. > > > > No, sorry, that wasn't what I meant. I didn't try to suggest a global v= ariable. > > Instead, I meant that a "global check" (conceptually, i.e. > > regardless of particular processor number) made sense. > > > > I'm also not particularly *against* a global variable. In other words, = I didn't try > > to comment on using a global variable *at all*. > > > > Using a global variable might as well work, I just feel that your curre= nt patches > > are good enough. > [Duran, Leo] > Great... I hear you. > Then, I'd prefer not refactoring further at this point.... I hope Ray & E= ric agree. >=20 > Thanks for your feedback! > Leo. >=20 > > > > Thanks > > Laszlo