[edk2-devel] [edk2-plaforms PATCH 1/1] SbsaQemu: Fix numerous SSDT generation problems

[Graeme Gregory]

1 - The SBSAQEMU_ACPI_ITOA contained a typo that put bogus characters
in the name if number of CPUs was greater than 10. It is safter to use
the AsciiSPrint function from PrintLib.

2 - The _UID fields were bogus, and indicated as bytes in AML instead of
a word. This caused extra Zero's to appear in disassembly. Fixed by
making them AML_WORD_PREFIX and putting CpuId in little endian.

3 - The table was a number of bytes too long causes bogus Zero in
dissassembly at end of table. Re-adjust code slightly to reduce table
size once we know the size of the length field.

Signed-off-by: Graeme Gregory <graeme at nuviainc.com>
---

This patch supercedes "SbsaQemu: Fix CPUID generation in SSDT"
as while fixing review comments on that I found the other issues.

 .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf       |  1 +
 .../Include/IndustryStandard/SbsaQemuAcpi.h   |  5 +---
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 25 +++++++++++--------
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index cde9d02f7f90..127eef029f3c 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -36,6 +36,7 @@ [LibraryClasses]
   DxeServicesLib
   FdtLib
   PcdLib
+  PrintLib
   UefiDriverEntryPoint
   UefiLib
   UefiRuntimeServicesTableLib
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
index 1a7d9dda2b99..f085765d2677 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
@@ -50,9 +50,6 @@
 #define SBSAQEMU_ACPI_CPU_DEV_LEN        0x1C
 #define SBSAQEMU_ACPI_CPU_DEV_NAME       { 'C', '0', '0', '0' }
 
-// Macro to convert Integer to Character
-#define SBSAQEMU_ACPI_ITOA(Byte)         (0x30 + (Byte > 9 ? (Byte + 1) : Byte))
-
 #define SBSAQEMU_ACPI_CPU_HID           {                                      \
   AML_NAME_OP, AML_NAME_CHAR__, 'H', 'I', 'D',                                 \
   AML_STRING_PREFIX, 'A', 'C', 'P', 'I', '0', '0', '0', '7',                   \
@@ -60,7 +57,7 @@
   }
 
 #define SBSAQEMU_ACPI_CPU_UID            {                                     \
-   AML_NAME_OP, AML_NAME_CHAR__, 'U', 'I', 'D', AML_BYTE_PREFIX,               \
+   AML_NAME_OP, AML_NAME_CHAR__, 'U', 'I', 'D', AML_WORD_PREFIX,               \
    AML_ZERO_OP, AML_ZERO_OP                                                    \
    }
 
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 06552f4b22f3..47a9bd1d423a 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -14,6 +14,7 @@
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/UefiLib.h>
@@ -248,6 +249,7 @@ AddSsdtTable (
   UINT32                TableSize;
   EFI_PHYSICAL_ADDRESS  PageAddress;
   UINT8                 *New;
+  UINT8                 *HeaderAddr;
   UINT32                CpuId;
   UINT32                Offset;
   UINT8                 ScopeOpName[] =  SBSAQEMU_ACPI_SCOPE_NAME;
@@ -283,12 +285,12 @@ AddSsdtTable (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  New = (UINT8 *)(UINTN) PageAddress;
+  HeaderAddr = New = (UINT8 *)(UINTN) PageAddress;
   ZeroMem (New, TableSize);
 
   // Add the ACPI Description table header
   CopyMem (New, &Header, sizeof (EFI_ACPI_DESCRIPTION_HEADER));
-  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
+
   New += sizeof (EFI_ACPI_DESCRIPTION_HEADER);
 
   // Insert the top level ScopeOp
@@ -296,6 +298,11 @@ AddSsdtTable (
   New++;
   Offset = SetPkgLength (New,
              (TableSize - sizeof (EFI_ACPI_DESCRIPTION_HEADER) - 1));
+
+  // Adjust TableSize now we know header length of _SB
+  TableSize -= (SBSAQEMU_ACPI_SCOPE_OP_MAX_LENGTH - Offset);
+  ((EFI_ACPI_DESCRIPTION_HEADER*) HeaderAddr)->Length = TableSize;
+
   New += Offset;
   CopyMem (New, &ScopeOpName, sizeof (ScopeOpName));
   New += sizeof (ScopeOpName);
@@ -303,21 +310,17 @@ AddSsdtTable (
   // Add new Device structures for the Cores
   for (CpuId = 0; CpuId < NumCores; CpuId++) {
     SBSAQEMU_ACPI_CPU_DEVICE *CpuDevicePtr;
-    UINT8 CpuIdByte1, CpuIdByte2, CpuIdByte3;
 
     CopyMem (New, &CpuDevice, sizeof (SBSAQEMU_ACPI_CPU_DEVICE));
     CpuDevicePtr = (SBSAQEMU_ACPI_CPU_DEVICE *) New;
 
-    CpuIdByte1 = CpuId & 0xF;
-    CpuIdByte2 = (CpuId >> 4) & 0xF;
-    CpuIdByte3 = (CpuId >> 8) & 0xF;
+    AsciiSPrint((CHAR8 *)&CpuDevicePtr->dev_name[1], 4, "%03X", CpuId);
 
-    CpuDevicePtr->dev_name[1] = SBSAQEMU_ACPI_ITOA(CpuIdByte3);
-    CpuDevicePtr->dev_name[2] = SBSAQEMU_ACPI_ITOA(CpuIdByte2);
-    CpuDevicePtr->dev_name[3] = SBSAQEMU_ACPI_ITOA(CpuIdByte1);
+    /* replace character lost by above NULL termination */
+    CpuDevicePtr->hid[0] = AML_NAME_OP;
 
-    CpuDevicePtr->uid[6] = CpuIdByte1 | CpuIdByte2;
-    CpuDevicePtr->uid[7] = CpuIdByte3;
+    CpuDevicePtr->uid[6] = CpuId & 0xFF;
+    CpuDevicePtr->uid[7] = (CpuId >> 8) & 0xFF;
     New += sizeof (SBSAQEMU_ACPI_CPU_DEVICE);
   }
 
-- 
2.25.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64692): https://edk2.groups.io/g/devel/message/64692
Mute This Topic: https://groups.io/mt/76453725/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-