Yes. I think so.

If x2APIC is supported by CPUID.(EAX=00H).ECX.BIT21, CPUID leaf 0x0B should be 
supported because x2APIC ID could be gotten from CPUID.(EAX=0BH, ECX=0H):EDX.
Otherwise, we cannot get x2APIC ID before we enabled x2APIC mode.

Jeff
-----Original Message-----
From: Kinney, Michael D 
Sent: Tuesday, November 24, 2015 3:58 PM
To: Fan, Jeff; [email protected]; Kinney, Michael D
Cc: Tian, Feng
Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/CpuMpPei: Get APIC ID from CPUID if 
x2APIC supported

Jeff,

Yes. That is ASSERT() location I was suggesting.

Does is also makes sense to fall back to GetInitialApicId() if CPUID leaf is 
not supported?  Or is this really a  condition that should never occur?

Mike

> -----Original Message-----
> From: Fan, Jeff
> Sent: Monday, November 23, 2015 11:38 PM
> To: Kinney, Michael D <[email protected]>; 
> [email protected]
> Cc: Tian, Feng <[email protected]>
> Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/CpuMpPei: Get APIC ID from 
> CPUID if x2APIC supported
> 
> Mike,
> 
> OK.  So,  if 0xB leaf is not supported, then ASSERT() in 
> GetX2ApicIdFromCpuId();
> 
>   //
>   // Get the maximum index of basic CPUID
>   //
>   AsmCpuid (CPUID_SIGNATURE, &MaxCpuIdIndex, NULL, NULL, NULL);
>   ASSERT (MaxCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY);
> 
> Jeff
> 
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Tuesday, November 24, 2015 3:00 PM
> To: Fan, Jeff; [email protected]; Kinney, Michael D
> Cc: Tian, Feng
> Subject: RE: [edk2] [Patch 1/3] UefiCpuPkg/CpuMpPei: Get APIC ID from 
> CPUID if x2APIC supported
> 
> Jeff,
> 
> Can you move the ASSERT() for no APIC ID available into 
> GetX2ApicIdFromCpuId().  Then you do not need special APIC ID value of 
> 0xFFFFFFFF.
> 
> Mike
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:[email protected]] On Behalf 
> > Of Jeff Fan
> > Sent: Monday, November 23, 2015 12:06 AM
> > To: [email protected]
> > Cc: Kinney, Michael D <[email protected]>; Tian, Feng 
> > <[email protected]>
> > Subject: [edk2] [Patch 1/3] UefiCpuPkg/CpuMpPei: Get APIC ID from 
> > CPUID if x2APIC supported
> >
> > If x2APIC is supported by processor, get the APIC ID from 
> > CPUID.(EAX=0BH, ECX=0H):EDX instead of legacy APIC ID. It is used to check 
> > if need to enable x2APIC mode.
> >
> > Cc: Feng Tian <[email protected]>
> > Cc: Michael Kinney <[email protected]>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Jeff Fan <[email protected]>
> > ---
> >  UefiCpuPkg/CpuMpPei/CpuMpPei.c | 65
> > ++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 63 insertions(+), 2 deletions(-)
> >
> > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c 
> > b/UefiCpuPkg/CpuMpPei/CpuMpPei.c index 8e35f28..a301fe5 100644
> > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> > @@ -102,6 +102,62 @@ SortApicId (
> >  }
> >
> >  /**
> > +  Check if x2APIC is supported by processor.
> > +
> > +  @retval  TRUE    x2APIC is supported.
> > +  @retval  FALSE   x2APIC isn't supported.
> > +**/
> > +BOOLEAN
> > +IsX2ApicModeSupport (
> > +  VOID
> > +  )
> > +{
> > +  UINT32                        RegEcx;
> > +  //
> > +  // Check if processor supports x2APIC feature
> > +  //
> > +  AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, &RegEcx, NULL);
> > +  if ((RegEcx & BIT21) != 0) {
> > +    return TRUE;
> > +  } else {
> > +    return FALSE;
> > +  }
> > +}
> > +
> > +/**
> > +  Return x2APIC ID from CPUID.
> > +
> > +  @retval -1      Invalid x2APIC ID
> > +  @retval others  x2APIC ID returned **/
> > +UINT32
> > +GetX2ApicIdFromCpuId (
> > +  VOID
> > +  )
> > +{
> > +  UINT32                        RegEdx;
> > +  UINT32                        MaxCpuIdIndex;
> > +
> > +  //
> > +  // The APIC ID value of FFFF_FFFFH is reserved and cannot be 
> > +assigned
> > +  // to any logical processor.
> > +  //
> > +  RegEdx = 0xFFFFFFFF;
> > +  //
> > +  // Get the maximum index of basic CPUID
> > +  //
> > +  AsmCpuid (CPUID_SIGNATURE, &MaxCpuIdIndex, NULL, NULL, NULL);
> > +  //
> > +  // If the extended topology enumeration leaf is available,
> > +  // get x2APIC ID from CPUID.(EAX=0BH, ECX=0H):EDX
> > +  //
> > +  if (MaxCpuIdIndex >= CPUID_EXTENDED_TOPOLOGY) {
> > +    AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, NULL, NULL, 
> > +&RegEdx);
> > +  }
> > +  return RegEdx;
> > +}
> > +
> > +/**
> >    Get CPU MP Data pointer from the Guided HOB.
> >
> >    @return  Pointer to Pointer to PEI CPU MP Data @@ -146,11 +202,16 @@ 
> > ApCFunction (
> >    PeiCpuMpData = ExchangeInfo->PeiCpuMpData;
> >    if (PeiCpuMpData->InitFlag) {
> >      //
> > -    // This is first time AP wakeup, get BIST inforamtion from AP stack
> > +    // This is first time AP wakeup, get BIST information from AP 
> > + stack
> >      //
> >      BistData = *(UINTN *) (PeiCpuMpData->Buffer + NumApsExecuting * 
> > PeiCpuMpData->CpuApStackSize - sizeof (UINTN));
> > -    PeiCpuMpData->CpuData[NumApsExecuting].ApicId        = 
> > GetInitialApicId ();
> >      PeiCpuMpData->CpuData[NumApsExecuting].Health.Uint32 = (UINT32) 
> > BistData;
> > +    if (!IsX2ApicModeSupport()) {
> > +      PeiCpuMpData->CpuData[NumApsExecuting].ApicId = GetInitialApicId ();
> > +    } else {
> > +      PeiCpuMpData->CpuData[NumApsExecuting].ApicId = GetX2ApicIdFromCpuId 
> > ();
> > +      ASSERT (PeiCpuMpData->CpuData[NumApsExecuting].ApicId != 0xFFFFFFFF);
> > +    }
> >      //
> >      // Sync BSP's Mtrr table to all wakeup APs and load microcode on APs.
> >      //
> > --
> > 1.9.5.msysgit.0
> >
> > _______________________________________________
> > edk2-devel mailing list
> > [email protected]
> > https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to