[edk2-devel] [edk2-plaforms PATCH 1/1] SbsaQemu: Fix numerous SSDT generation problems
Leif Lindholm
leif at nuviainc.com
Thu Aug 27 17:10:25 UTC 2020
A few minor typos in the commit message:
On Thu, Aug 27, 2020 at 16:21:33 +0100, Graeme Gregory wrote:
> 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
safter -> safer
> the AsciiSPrint function from PrintLib.
>
> 2 - The _UID fields were bogus, and indicated as bytes in AML instead of
> a word. This caused extra Zeros to appear in disassembly. Fixed by
Zero's -> Zeros
> 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
missing ", which"
> 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>
Reviewed-by: Leif Lindholm <leif at nuviainc.com>
I took the liberty to address the typos locally before pushing as
b54d65a4a671.
Thanks!
/
Leif
> ---
>
> 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 (#64701): https://edk2.groups.io/g/devel/message/64701
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list