Leo and all, I replied in https://bugzilla.tianocore.org/show_bug.cgi?id=2556 for a more 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 <leo.du...@amd.com> > Sent: Thursday, February 27, 2020 1:52 AM > To: Laszlo Ersek <ler...@redhat.com>; Ni, Ray <ray...@intel.com>; > devel@edk2.groups.io; Wu, Hao A > <hao.a...@intel.com>; Fu, Siyuan <siyuan...@intel.com> > Cc: Dong, Eric <eric.d...@intel.com> > Subject: RE: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpInitLib > > > > > -----Original Message----- > > From: Laszlo Ersek [mailto:ler...@redhat.com] > > Sent: Wednesday, February 26, 2020 12:45 PM > > To: Duran, Leo <leo.du...@amd.com>; Ni, Ray <ray...@intel.com>; > > devel@edk2.groups.io; Wu, Hao A <hao.a...@intel.com>; Fu, Siyuan > > <siyuan...@intel.com> > > Cc: Dong, Eric <eric.d...@intel.com> > > 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:ler...@redhat.com] > > >> Sent: Wednesday, February 26, 2020 11:21 AM > > >> To: Duran, Leo <leo.du...@amd.com>; Ni, Ray <ray...@intel.com>; > > >> devel@edk2.groups.io; Wu, Hao A <hao.a...@intel.com>; Fu, Siyuan > > >> <siyuan...@intel.com> > > >> Cc: Dong, Eric <eric.d...@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. > > > > No, sorry, that wasn't what I meant. I didn't try to suggest a global > > variable. > > 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 current > > patches > > are good enough. > [Duran, Leo] > Great... I hear you. > Then, I'd prefer not refactoring further at this point.... I hope Ray & Eric > agree. > > Thanks for your feedback! > Leo. > > > > > Thanks > > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54977): https://edk2.groups.io/g/devel/message/54977 Mute This Topic: https://groups.io/mt/71541516/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
