Re: [Patch v3 3/3] UefiCpuPkg/MpInitLib: Load uCode once for each core.

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Thanks/Ray

> -----Original Message-----
> From: Dong, Eric
> Sent: Wednesday, July 18, 2018 8:12 AM
> To: Laszlo Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [Patch v3 3/3] UefiCpuPkg/MpInitLib: Load uCode once
> for each core.
> 
> Hi Laszlo & Ray,
> 
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Tuesday, July 17, 2018 10:07 PM
> > To: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [Patch v3 3/3] UefiCpuPkg/MpInitLib: Load uCode
> > once for each core.
> >
> > On 07/17/18 12:02, Ni, Ruiyu wrote:
> > > On 7/16/2018 11:08 AM, Eric Dong wrote:
> > >> GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL,
> > >> &ThreadId);
> > >> +  if (ThreadId != 0) {
> > >> +    //
> > >> +    // Skip loading microcode if it is not the first thread in one core.
> > >> +    //
> > >> +    return;
> > >> +  }
> > >> +
> > >
> > > Eric,
> > > Is it possible that Thread#0 is disabled while Thread#1 is enabled?
> > > It may cause the certain core with no uCode applied.
> > >
> >
> > I thought of this (superficially) but I figured the code would run
> > anyway at CpuMpPei / CpuDxe startup only, at which point no logical
> > processors could have been disabled yet -- programmatically, through
> > the MP PPI / protocol. Is there any other reason why ThreadId=0 could be
> missing?
> >
> 
> Yes, I think at this point, all the threads should at the ready state.  The only
> case I can image is that the hardware has broken and the core can't be wake
> up.  But I think in this case, the core should can't work, not only thread 0 can't
> work.  What do you think?
I am not sure. We need to check the IA32 SDM carefully for any potential unexpected cases.

> 
> > (I plan to check the rest of the series later.)
> >
> > Thanks
> > Laszlo