[edk2-devel][edk2-platforms][PATCH V2 1/1] MinPlatformPkg: AcpiTables: Add additional thread mapping in MADT

[Oram, Isaac W]

Generally, convention for "Pkg-Module" is "Pkg/Module: " not "Pkg: Module: ".  Kind of a nit, but using : as separator will make parsing tools harder to write.

Convention for comments is "no newline between comment block and code block" is that the comment applies to that block.  A blank line indicates the comment applies to multiple code blocks.

Please fix the comments to match the new code.  Prior comments made sense; first, middle, last.  Second, third, and fourth threads "in the middle" makes less sense.  I think explaining why we are grouping into the order we are is perhaps important.  There is nothing explaining why this ordering and why third and fourth are being added when they appear to have been previously ignored.  That info may belong in the commit message if not appropriate for the comments.

Also note that your author name format settings need to be modified such that the author field in the log is: Ankit Sinha <ankit.sinha at intel.com>

Regards,
Isaac

-----Original Message-----
From: Sinha, Ankit <ankit.sinha at intel.com> 
Sent: Wednesday, July 27, 2022 12:13 PM
To: devel at edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Oram, Isaac W <isaac.w.oram at intel.com>; Dong, Eric <eric.dong at intel.com>
Subject: [edk2-devel][edk2-platforms][PATCH V2 1/1] MinPlatformPkg: AcpiTables: Add additional thread mapping in MADT

Adding second, third and fourth thread mapping to MADT

Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
Cc: Isaac Oram <isaac.w.oram at intel.com>
Cc: Eric Dong <eric.dong at intel.com>
Signed-off-by: Ankit Sinha <ankit.sinha at intel.com>
---
 Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index c7e87cbd7d9d..524f9914b0b1 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -261,7 +261,7 @@ SortCpuLocalApicInTable (
   }
 
   //
-  // 3. Sort and map the second threads to the middle of the CpuApicIdOrderTable
+  // 3. Sort and map the second, third and fourth threads to the middle 
+ of the CpuApicIdOrderTable
   //
   for (Index = 0; Index < mNumberOfCpus; Index++) {
     if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread @@ -270,6 +270,20 @@ SortCpuLocalApicInTable (
     }
   }
 
+  for (Index = 0; Index < mNumberOfCpus; Index++) {
+    if ((TempCpuApicIdOrderTable[Index].Thread) == 2) { // third thread
+      CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
+      CurrProcessor++;
+    }
+  }
+
+  for (Index = 0; Index < mNumberOfCpus; Index++) {
+    if ((TempCpuApicIdOrderTable[Index].Thread) == 3) { // fourth thread
+      CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
+      CurrProcessor++;
+    }
+  }
+
   //
   // 4. Sort and map the not enabled threads to the bottom of the CpuApicIdOrderTable
   //
--
2.27.0.windows.1



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