[edk2-devel] [edk2-platforms: PATCH V8] Platform/Intel: Correct CPU APIC IDs

[Ni, Ray]

> +      CpuIdMapPtr->AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
> (UINT32)ProcessorInfoBuffer.ProcessorId;

1. I don't understand this assignment. Since you will re-assign this later, why not remove this assignment?


> +
> +  //make sure 1st entry is BSP
> +  if (mX2ApicEnabled) {
> +    BspApicId = (UINT32)AsmReadMsr64 (0x802);
> +  } else {
> +    BspApicId = (*(volatile UINT32 *)(UINTN)0xFEE00020) >> 24;
> +  }
> +  DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));

2. you can use GetApicId() API from LocalApicLib library in UefiCpuPkg.


> -    if(mCpuApicIdOrderTable[0].ApicId != BspApicId) {
> -      //check to see if 1st entry is BSP, if not swap it
> -      Index = ApicId2SwProcApicId(BspApicId);
> +    CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
> +    CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
> +    CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
> +    TempCpuApicIdOrderTable[0].Flags = 1;

3. Why is " TempCpuApicIdOrderTable[0].Flags = 1;" needed? 

> +  for (CurrProcessor = 1; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> +    for (Index = CurrProcessor+1; Index < mNumberOfCpus; Index++) {
> +      if (TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId > TempCpuApicIdOrderTable[Index].AcpiProcessorId) {
> +        CopyMem (&TempVal, &TempCpuApicIdOrderTable[CurrProcessor], sizeof (EFI_CPU_ID_ORDER_MAP));
> +        CopyMem (&TempCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof
> (EFI_CPU_ID_ORDER_MAP));
> +        CopyMem (&TempCpuApicIdOrderTable[Index], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP));
>        }
> +    }
> +  }

4. Can you use PerformQuickSort() API from SortLib in MdeModulePkg?



> +
> +  //
> +  // 5. Re-assigen AcpiProcessorId for AcpiProcessorUId uses purpose.
> +  //
> +  for (Socket = 0; Socket < MAX_SOCKET; Socket++) {
> +    for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) {
> +      if (mCpuApicIdOrderTable[CurrProcessor].Flags && (mCpuApicIdOrderTable[CurrProcessor].SocketNum == Socket)) {
> +        mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) +
> Index;
> +        Index++;

5. I think you need a AcpiProcessorId[MAX_SOCKET] array to remember the "AcpiProcessorId" for each socket/package.
For example:
AcpiProcessorId[MAX_SOCKET];
ZeroMem (AcpiProcessorId);
for (Socket = 0; ...) {
  for (CurrProcessor = 0; ...) {
    if (...Flags == 1 && ...SocketNum == Socket) {
      mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + AcpiProcessorId[Socket];
      AcpiProcessorId[Socket]++;




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79572): https://edk2.groups.io/g/devel/message/79572
Mute This Topic: https://groups.io/mt/84921613/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-