Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpInitLib

["Duran, Leo" <leo.duran@amd.com>]

Laszlo, et al,

I suppose the same can be said about the actual "PlatformId"... it should be a single/global read, correct?
But I'd prefer not tackling that in this patch-set (I'll defer to someone that may want to take that on as an optimization/clean-up.).

Leo.

> -----Original Message-----
> From: Duran, Leo
> Sent: Wednesday, February 26, 2020 11:40 AM
> To: Laszlo Ersek <lersek@redhat.com>; Ni, Ray <ray.ni@intel.com>;
> devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in
> MpInitLib
> 
> 
> 
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Wednesday, February 26, 2020 11:21 AM
> > To: Duran, Leo <leo.duran@amd.com>; Ni, Ray <ray.ni@intel.com>;
> > devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>
> > Cc: Dong, Eric <eric.dong@intel.com>
> > 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 would
> > >look
> > like this:
> > >> > >   //
> > >   // NOTE: PlatformId is not relevant on AMD platforms.
> > >   //
> > >   if (StandardSignatureIsAuthenticAMD ()) {
> > >     CpuMpData->CpuData[ProcessorNumber].IsValidPlatformId = FALSE;
> > >   else {
> > >     PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> > >     CpuMpData->CpuData[ProcessorNumber].PlatformId =
> > (UINT8)PlatformIdMsr.Bits.PlatformId;
> > >     CpuMpData->CpuData[ProcessorNumber].IsValidPlatformId = TRUE;
> > >   }
> > >
> > > This way "IsValidPlatformId" could be checked prior to using "PlatformId".
> > > Anyway, that seemed a bit overkill, so I opted against it... thoughts?
> >
> > I think a global flag is justified; in the above approach, "IsValidPlatformId"
> > would not vary across "ProcessorNumber", so it does look like useless
> > generality.
> [Duran, Leo]
> Great point, Laszlo.
> Indeed, global makes senses in the case!
> I can prepare a v2-set to incorporate that.
> 
> Thanks,
> Leo
> 
> >
> > Thanks
> > Laszlo