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
