[edk2-devel] [PATCH v3 edk2-platforms 5/8] SbsaQemu: AcpiDxe: Create MADT table at runtime

Tanmay Jagdale posted 8 patches 5 years, 5 months ago
[edk2-devel] [PATCH v3 edk2-platforms 5/8] SbsaQemu: AcpiDxe: Create MADT table at runtime
Posted by Tanmay Jagdale 5 years, 5 months ago
- Add support to create MADT table at runtime.
- Included a macro for GIC Redistributor structure initialisation.

Signed-off-by: Tanmay Jagdale <tanmay.jagdale@linaro.org>
---
 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  20 ++-
 Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h     |  15 ++
 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 156 ++++++++++++++++++++
 3 files changed, 190 insertions(+), 1 deletion(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index 3795a7e11639..8125e8ba7553 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -41,9 +41,27 @@ [LibraryClasses]
   UefiRuntimeServicesTableLib
 
 [Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
 
 [Depex]
-  TRUE
+  gEfiAcpiTableProtocolGuid                       ## CONSUMES
+
+[Guids]
+  gEdkiiPlatformHasAcpiGuid
+
+[Protocols]
+  gEfiAcpiTableProtocolGuid                       ## CONSUMES
+
+[FixedPcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
+  gArmTokenSpaceGuid.PcdGicDistributorBase
+  gArmTokenSpaceGuid.PcdGicRedistributorsBase
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
index eac195b0585c..7a9a0061675f 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
@@ -22,6 +22,21 @@
     FixedPcdGet32 (PcdAcpiDefaultCreatorRevision)/* UINT32  CreatorRevision */ \
   }
 
+// Defines for MADT
+#define SBSAQEMU_MADT_GIC_VBASE          0x2c020000
+#define SBSAQEMU_MADT_GIC_HBASE          0x2c010000
+#define SBSAQEMU_MADT_GIC_PMU_IRQ        23
+#define SBSAQEMU_MADT_GICR_SIZE          0x4000000
+
+// Macro for MADT GIC Redistributor Structure
+#define SBSAQEMU_MADT_GICR_INIT() {                                            \
+   EFI_ACPI_6_0_GICR,                        /* Type */                        \
+   sizeof (EFI_ACPI_6_0_GICR_STRUCTURE),     /* Length */                      \
+   EFI_ACPI_RESERVED_WORD,                   /* Reserved */                    \
+   FixedPcdGet32 (PcdGicRedistributorsBase), /* DiscoveryRangeBaseAddress */   \
+   SBSAQEMU_MADT_GICR_SIZE                   /* DiscoveryRangeLength */        \
+   }
+
 #define SBSAQEMU_UART0_BASE              0x60000000
 
 #define SBSAQEMU_PCI_SEG0_CONFIG_BASE    0xf0000000
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 75abdae3b8ce..16cb4e904e6f 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -6,11 +6,17 @@
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
 **/
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/SbsaQemuAcpi.h>
+#include <Library/AcpiLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/UefiLib.h>
+#include <Protocol/AcpiTable.h>
 #include <Protocol/FdtClient.h>
 #include <libfdt.h>
 
@@ -61,6 +67,137 @@ CountCpusFromFdt (
   ASSERT_RETURN_ERROR (PcdStatus);
 }
 
+/*
+ * A Function to Compute the ACPI Table Checksum
+ */
+VOID
+AcpiPlatformChecksum (
+  IN UINT8      *Buffer,
+  IN UINTN      Size
+  )
+{
+  UINTN ChecksumOffset;
+
+  ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
+
+  // Set checksum field to 0 since it is used as part of the calculation
+  Buffer[ChecksumOffset] = 0;
+
+  Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);
+}
+
+/*
+ * A function that add the MADT ACPI table.
+  IN EFI_ACPI_COMMON_HEADER    *CurrentTable
+ */
+EFI_STATUS
+AddMadtTable (
+  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
+  )
+{
+  EFI_STATUS            Status;
+  UINTN                 TableHandle;
+  UINT32                TableSize;
+  EFI_PHYSICAL_ADDRESS  PageAddress;
+  UINT8                 *New;
+  UINT32                NumCores;
+
+  // Initialize MADT ACPI Header
+  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header = {
+     SBSAQEMU_ACPI_HEADER (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+                           EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER,
+                           EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION),
+      0, 0 };
+
+  // Initialize GICC Structure
+  EFI_ACPI_6_0_GIC_STRUCTURE Gicc = EFI_ACPI_6_0_GICC_STRUCTURE_INIT (
+    0,                                     /* GicID */
+    0,                                     /* AcpiCpuUid */
+    0,                                     /* Mpidr */
+    EFI_ACPI_6_0_GIC_ENABLED,              /* Flags */
+    SBSAQEMU_MADT_GIC_PMU_IRQ,             /* PMU Irq */
+    FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */
+    SBSAQEMU_MADT_GIC_VBASE,               /* GicVBase */
+    SBSAQEMU_MADT_GIC_HBASE,               /* GicHBase */
+    25,                                    /* GsivId */
+    0,                                     /* GicRBase */
+    0                                      /* Efficiency */
+    );
+
+  // Initialize GIC Distributor Structure
+  EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE Gicd =
+    EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT (
+      0,
+      FixedPcdGet32 (PcdGicDistributorBase),
+      0,
+      3 /* GicVersion */
+    );
+
+ // Initialize GIC Redistributor Structure
+  EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();
+
+  // Get CoreCount which was determined eariler after parsing device tree
+  NumCores = PcdGet32 (PcdCoreCount);
+
+  // Calculate the new table size based on the number of cores
+  TableSize = sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
+               (sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) +
+               sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) +
+               sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
+
+  Status = gBS->AllocatePages (
+                  AllocateAnyPages,
+                  EfiACPIReclaimMemory,
+                  EFI_SIZE_TO_PAGES (TableSize),
+                  &PageAddress
+                  );
+  if (EFI_ERROR(Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to allocate pages for MADT table\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  New = (UINT8 *)(UINTN) PageAddress;
+  ZeroMem (New, TableSize);
+
+  // Add the  ACPI Description table header
+  CopyMem (New, &Header, sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER));
+  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
+  New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
+
+  // Add new GICC structures for the Cores
+  for (NumCores = 0; NumCores < PcdGet32 (PcdCoreCount); NumCores++) {
+    EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
+
+    CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
+    GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
+    GiccPtr->AcpiProcessorUid = NumCores;
+    GiccPtr->MPIDR = NumCores;
+    New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
+  }
+
+  // GIC Distributor Structure
+  CopyMem (New, &Gicd, sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE));
+  New += sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE);
+
+  // GIC ReDistributor Structure
+  CopyMem (New, &Gicr, sizeof (EFI_ACPI_6_0_GICR_STRUCTURE));
+  New += sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
+
+  AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);
+
+  Status = AcpiTable->InstallAcpiTable (
+                        AcpiTable,
+                        (EFI_ACPI_COMMON_HEADER *)PageAddress,
+                        TableSize,
+                        &TableHandle
+                        );
+  if (EFI_ERROR(Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to install MADT table\n"));
+  }
+
+  return Status;
+}
+
 EFI_STATUS
 EFIAPI
 InitializeSbsaQemuAcpiDxe (
@@ -68,8 +205,27 @@ InitializeSbsaQemuAcpiDxe (
   IN EFI_SYSTEM_TABLE     *SystemTable
   )
 {
+  EFI_STATUS                     Status;
+  EFI_ACPI_TABLE_PROTOCOL        *AcpiTable;
+
   // Parse the device tree and get the number of CPUs
   CountCpusFromFdt ();
 
+  // Check if ACPI Table Protocol has been installed
+  Status = gBS->LocateProtocol (
+                  &gEfiAcpiTableProtocolGuid,
+                  NULL,
+                  (VOID **)&AcpiTable
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to locate ACPI Table Protocol\n"));
+    return Status;
+  }
+
+  Status = AddMadtTable (AcpiTable);
+  if (EFI_ERROR(Status)) {
+     DEBUG ((DEBUG_ERROR, "Failed to add MADT table\n"));
+  }
+
   return EFI_SUCCESS;
 }
-- 
2.28.0


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

View/Reply Online (#64616): https://edk2.groups.io/g/devel/message/64616
Mute This Topic: https://groups.io/mt/76406685/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 edk2-platforms 5/8] SbsaQemu: AcpiDxe: Create MADT table at runtime
Posted by Graeme Gregory 5 years, 5 months ago
On Tue, Aug 25, 2020 at 07:09:55PM +0530, Tanmay Jagdale wrote:
> - Add support to create MADT table at runtime.
> - Included a macro for GIC Redistributor structure initialisation.
> 
> Signed-off-by: Tanmay Jagdale <tanmay.jagdale@linaro.org>
> ---
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  20 ++-
>  Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h     |  15 ++
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 156 ++++++++++++++++++++
>  3 files changed, 190 insertions(+), 1 deletion(-)
> 
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index 3795a7e11639..8125e8ba7553 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> @@ -41,9 +41,27 @@ [LibraryClasses]
>    UefiRuntimeServicesTableLib
>  
>  [Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>  
>  [Depex]
> -  TRUE
> +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
> +
> +[Guids]
> +  gEdkiiPlatformHasAcpiGuid
> +
> +[Protocols]
> +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
> +
> +[FixedPcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
> +  gArmTokenSpaceGuid.PcdGicDistributorBase
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
> diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> index eac195b0585c..7a9a0061675f 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> @@ -22,6 +22,21 @@
>      FixedPcdGet32 (PcdAcpiDefaultCreatorRevision)/* UINT32  CreatorRevision */ \
>    }
>  
> +// Defines for MADT
> +#define SBSAQEMU_MADT_GIC_VBASE          0x2c020000
> +#define SBSAQEMU_MADT_GIC_HBASE          0x2c010000
> +#define SBSAQEMU_MADT_GIC_PMU_IRQ        23
> +#define SBSAQEMU_MADT_GICR_SIZE          0x4000000
> +
> +// Macro for MADT GIC Redistributor Structure
> +#define SBSAQEMU_MADT_GICR_INIT() {                                            \
> +   EFI_ACPI_6_0_GICR,                        /* Type */                        \
> +   sizeof (EFI_ACPI_6_0_GICR_STRUCTURE),     /* Length */                      \
> +   EFI_ACPI_RESERVED_WORD,                   /* Reserved */                    \
> +   FixedPcdGet32 (PcdGicRedistributorsBase), /* DiscoveryRangeBaseAddress */   \
> +   SBSAQEMU_MADT_GICR_SIZE                   /* DiscoveryRangeLength */        \
> +   }
> +
>  #define SBSAQEMU_UART0_BASE              0x60000000
>  
>  #define SBSAQEMU_PCI_SEG0_CONFIG_BASE    0xf0000000
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 75abdae3b8ce..16cb4e904e6f 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -6,11 +6,17 @@
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
>  **/
> +#include <IndustryStandard/Acpi.h>
> +#include <IndustryStandard/SbsaQemuAcpi.h>
> +#include <Library/AcpiLib.h>
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <Library/UefiLib.h>
> +#include <Protocol/AcpiTable.h>
>  #include <Protocol/FdtClient.h>
>  #include <libfdt.h>
>  
> @@ -61,6 +67,137 @@ CountCpusFromFdt (
>    ASSERT_RETURN_ERROR (PcdStatus);
>  }
>  
> +/*
> + * A Function to Compute the ACPI Table Checksum
> + */
> +VOID
> +AcpiPlatformChecksum (
> +  IN UINT8      *Buffer,
> +  IN UINTN      Size
> +  )
> +{
> +  UINTN ChecksumOffset;
> +
> +  ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
> +
> +  // Set checksum field to 0 since it is used as part of the calculation
> +  Buffer[ChecksumOffset] = 0;
> +
> +  Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);
> +}
> +
> +/*
> + * A function that add the MADT ACPI table.
> +  IN EFI_ACPI_COMMON_HEADER    *CurrentTable
> + */
> +EFI_STATUS
> +AddMadtTable (
> +  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
> +  )
> +{
> +  EFI_STATUS            Status;
> +  UINTN                 TableHandle;
> +  UINT32                TableSize;
> +  EFI_PHYSICAL_ADDRESS  PageAddress;
> +  UINT8                 *New;
> +  UINT32                NumCores;
> +
> +  // Initialize MADT ACPI Header
> +  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header = {
> +     SBSAQEMU_ACPI_HEADER (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> +                           EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER,
> +                           EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION),
> +      0, 0 };
> +
> +  // Initialize GICC Structure
> +  EFI_ACPI_6_0_GIC_STRUCTURE Gicc = EFI_ACPI_6_0_GICC_STRUCTURE_INIT (
> +    0,                                     /* GicID */
> +    0,                                     /* AcpiCpuUid */
> +    0,                                     /* Mpidr */
> +    EFI_ACPI_6_0_GIC_ENABLED,              /* Flags */
> +    SBSAQEMU_MADT_GIC_PMU_IRQ,             /* PMU Irq */
> +    FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */
> +    SBSAQEMU_MADT_GIC_VBASE,               /* GicVBase */
> +    SBSAQEMU_MADT_GIC_HBASE,               /* GicHBase */
> +    25,                                    /* GsivId */
> +    0,                                     /* GicRBase */
> +    0                                      /* Efficiency */
> +    );
> +
> +  // Initialize GIC Distributor Structure
> +  EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE Gicd =
> +    EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT (
> +      0,
> +      FixedPcdGet32 (PcdGicDistributorBase),
> +      0,
> +      3 /* GicVersion */
> +    );
> +
> + // Initialize GIC Redistributor Structure
> +  EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();
> +
> +  // Get CoreCount which was determined eariler after parsing device tree
> +  NumCores = PcdGet32 (PcdCoreCount);
> +
> +  // Calculate the new table size based on the number of cores
> +  TableSize = sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
> +               (sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) +
> +               sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) +
> +               sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
> +
> +  Status = gBS->AllocatePages (
> +                  AllocateAnyPages,
> +                  EfiACPIReclaimMemory,
> +                  EFI_SIZE_TO_PAGES (TableSize),
> +                  &PageAddress
> +                  );
> +  if (EFI_ERROR(Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to allocate pages for MADT table\n"));
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  New = (UINT8 *)(UINTN) PageAddress;
> +  ZeroMem (New, TableSize);
> +
> +  // Add the  ACPI Description table header
> +  CopyMem (New, &Header, sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER));
> +  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
> +  New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
> +
> +  // Add new GICC structures for the Cores
> +  for (NumCores = 0; NumCores < PcdGet32 (PcdCoreCount); NumCores++) {
> +    EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
> +
> +    CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
> +    GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
> +    GiccPtr->AcpiProcessorUid = NumCores;
> +    GiccPtr->MPIDR = NumCores;

This does not seem to be quite correct, if I dump the MPIDRs from ARM-TF when
booting with 12 cpus this is what I get.

NOTICE:  MPIDR 0
NOTICE:  MPIDR 1
NOTICE:  MPIDR 2
NOTICE:  MPIDR 3
NOTICE:  MPIDR 4
NOTICE:  MPIDR 5
NOTICE:  MPIDR 6
NOTICE:  MPIDR 7
NOTICE:  MPIDR 100
NOTICE:  MPIDR 101
NOTICE:  MPIDR 102
NOTICE:  MPIDR 103

I think this will make PSCI operations from CPU8 onwards fail.

Thanks

Graeme


> +    New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
> +  }
> +
> +  // GIC Distributor Structure
> +  CopyMem (New, &Gicd, sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE));
> +  New += sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE);
> +
> +  // GIC ReDistributor Structure
> +  CopyMem (New, &Gicr, sizeof (EFI_ACPI_6_0_GICR_STRUCTURE));
> +  New += sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
> +
> +  AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);
> +
> +  Status = AcpiTable->InstallAcpiTable (
> +                        AcpiTable,
> +                        (EFI_ACPI_COMMON_HEADER *)PageAddress,
> +                        TableSize,
> +                        &TableHandle
> +                        );
> +  if (EFI_ERROR(Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to install MADT table\n"));
> +  }
> +
> +  return Status;
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  InitializeSbsaQemuAcpiDxe (
> @@ -68,8 +205,27 @@ InitializeSbsaQemuAcpiDxe (
>    IN EFI_SYSTEM_TABLE     *SystemTable
>    )
>  {
> +  EFI_STATUS                     Status;
> +  EFI_ACPI_TABLE_PROTOCOL        *AcpiTable;
> +
>    // Parse the device tree and get the number of CPUs
>    CountCpusFromFdt ();
>  
> +  // Check if ACPI Table Protocol has been installed
> +  Status = gBS->LocateProtocol (
> +                  &gEfiAcpiTableProtocolGuid,
> +                  NULL,
> +                  (VOID **)&AcpiTable
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to locate ACPI Table Protocol\n"));
> +    return Status;
> +  }
> +
> +  Status = AddMadtTable (AcpiTable);
> +  if (EFI_ERROR(Status)) {
> +     DEBUG ((DEBUG_ERROR, "Failed to add MADT table\n"));
> +  }
> +
>    return EFI_SUCCESS;
>  }
> -- 
> 2.28.0
> 

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

View/Reply Online (#64647): https://edk2.groups.io/g/devel/message/64647
Mute This Topic: https://groups.io/mt/76406685/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 edk2-platforms 5/8] SbsaQemu: AcpiDxe: Create MADT table at runtime
Posted by Graeme Gregory 5 years, 5 months ago
On Wed, Aug 26, 2020 at 04:35:22PM +0100, Graeme Gregory wrote:
> On Tue, Aug 25, 2020 at 07:09:55PM +0530, Tanmay Jagdale wrote:
> > - Add support to create MADT table at runtime.
> > - Included a macro for GIC Redistributor structure initialisation.
> > 
> > Signed-off-by: Tanmay Jagdale <tanmay.jagdale@linaro.org>
> > ---
> >  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  20 ++-
> >  Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h     |  15 ++
> >  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 156 ++++++++++++++++++++
> >  3 files changed, 190 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > index 3795a7e11639..8125e8ba7553 100644
> > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > @@ -41,9 +41,27 @@ [LibraryClasses]
> >    UefiRuntimeServicesTableLib
> >  
> >  [Pcd]
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
> >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
> >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
> >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> >  
> >  [Depex]
> > -  TRUE
> > +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
> > +
> > +[Guids]
> > +  gEdkiiPlatformHasAcpiGuid
> > +
> > +[Protocols]
> > +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
> > +
> > +[FixedPcd]
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
> > +  gArmTokenSpaceGuid.PcdGicDistributorBase
> > +  gArmTokenSpaceGuid.PcdGicRedistributorsBase
> > +
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
> > diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> > index eac195b0585c..7a9a0061675f 100644
> > --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> > +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> > @@ -22,6 +22,21 @@
> >      FixedPcdGet32 (PcdAcpiDefaultCreatorRevision)/* UINT32  CreatorRevision */ \
> >    }
> >  
> > +// Defines for MADT
> > +#define SBSAQEMU_MADT_GIC_VBASE          0x2c020000
> > +#define SBSAQEMU_MADT_GIC_HBASE          0x2c010000
> > +#define SBSAQEMU_MADT_GIC_PMU_IRQ        23
> > +#define SBSAQEMU_MADT_GICR_SIZE          0x4000000
> > +
> > +// Macro for MADT GIC Redistributor Structure
> > +#define SBSAQEMU_MADT_GICR_INIT() {                                            \
> > +   EFI_ACPI_6_0_GICR,                        /* Type */                        \
> > +   sizeof (EFI_ACPI_6_0_GICR_STRUCTURE),     /* Length */                      \
> > +   EFI_ACPI_RESERVED_WORD,                   /* Reserved */                    \
> > +   FixedPcdGet32 (PcdGicRedistributorsBase), /* DiscoveryRangeBaseAddress */   \
> > +   SBSAQEMU_MADT_GICR_SIZE                   /* DiscoveryRangeLength */        \
> > +   }
> > +
> >  #define SBSAQEMU_UART0_BASE              0x60000000
> >  
> >  #define SBSAQEMU_PCI_SEG0_CONFIG_BASE    0xf0000000
> > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > index 75abdae3b8ce..16cb4e904e6f 100644
> > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > @@ -6,11 +6,17 @@
> >  *  SPDX-License-Identifier: BSD-2-Clause-Patent
> >  *
> >  **/
> > +#include <IndustryStandard/Acpi.h>
> > +#include <IndustryStandard/SbsaQemuAcpi.h>
> > +#include <Library/AcpiLib.h>
> > +#include <Library/BaseMemoryLib.h>
> >  #include <Library/DebugLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> >  #include <Library/PcdLib.h>
> >  #include <Library/UefiBootServicesTableLib.h>
> >  #include <Library/UefiDriverEntryPoint.h>
> >  #include <Library/UefiLib.h>
> > +#include <Protocol/AcpiTable.h>
> >  #include <Protocol/FdtClient.h>
> >  #include <libfdt.h>
> >  
> > @@ -61,6 +67,137 @@ CountCpusFromFdt (
> >    ASSERT_RETURN_ERROR (PcdStatus);
> >  }
> >  
> > +/*
> > + * A Function to Compute the ACPI Table Checksum
> > + */
> > +VOID
> > +AcpiPlatformChecksum (
> > +  IN UINT8      *Buffer,
> > +  IN UINTN      Size
> > +  )
> > +{
> > +  UINTN ChecksumOffset;
> > +
> > +  ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
> > +
> > +  // Set checksum field to 0 since it is used as part of the calculation
> > +  Buffer[ChecksumOffset] = 0;
> > +
> > +  Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);
> > +}
> > +
> > +/*
> > + * A function that add the MADT ACPI table.
> > +  IN EFI_ACPI_COMMON_HEADER    *CurrentTable
> > + */
> > +EFI_STATUS
> > +AddMadtTable (
> > +  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
> > +  )
> > +{
> > +  EFI_STATUS            Status;
> > +  UINTN                 TableHandle;
> > +  UINT32                TableSize;
> > +  EFI_PHYSICAL_ADDRESS  PageAddress;
> > +  UINT8                 *New;
> > +  UINT32                NumCores;
> > +
> > +  // Initialize MADT ACPI Header
> > +  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header = {
> > +     SBSAQEMU_ACPI_HEADER (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> > +                           EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER,
> > +                           EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION),
> > +      0, 0 };
> > +
> > +  // Initialize GICC Structure
> > +  EFI_ACPI_6_0_GIC_STRUCTURE Gicc = EFI_ACPI_6_0_GICC_STRUCTURE_INIT (
> > +    0,                                     /* GicID */
> > +    0,                                     /* AcpiCpuUid */
> > +    0,                                     /* Mpidr */
> > +    EFI_ACPI_6_0_GIC_ENABLED,              /* Flags */
> > +    SBSAQEMU_MADT_GIC_PMU_IRQ,             /* PMU Irq */
> > +    FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */
> > +    SBSAQEMU_MADT_GIC_VBASE,               /* GicVBase */
> > +    SBSAQEMU_MADT_GIC_HBASE,               /* GicHBase */
> > +    25,                                    /* GsivId */
> > +    0,                                     /* GicRBase */
> > +    0                                      /* Efficiency */
> > +    );
> > +
> > +  // Initialize GIC Distributor Structure
> > +  EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE Gicd =
> > +    EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT (
> > +      0,
> > +      FixedPcdGet32 (PcdGicDistributorBase),
> > +      0,
> > +      3 /* GicVersion */
> > +    );
> > +
> > + // Initialize GIC Redistributor Structure
> > +  EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();
> > +
> > +  // Get CoreCount which was determined eariler after parsing device tree
> > +  NumCores = PcdGet32 (PcdCoreCount);
> > +
> > +  // Calculate the new table size based on the number of cores
> > +  TableSize = sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
> > +               (sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) +
> > +               sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) +
> > +               sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
> > +
> > +  Status = gBS->AllocatePages (
> > +                  AllocateAnyPages,
> > +                  EfiACPIReclaimMemory,
> > +                  EFI_SIZE_TO_PAGES (TableSize),
> > +                  &PageAddress
> > +                  );
> > +  if (EFI_ERROR(Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Failed to allocate pages for MADT table\n"));
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  New = (UINT8 *)(UINTN) PageAddress;
> > +  ZeroMem (New, TableSize);
> > +
> > +  // Add the  ACPI Description table header
> > +  CopyMem (New, &Header, sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER));
> > +  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
> > +  New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
> > +
> > +  // Add new GICC structures for the Cores
> > +  for (NumCores = 0; NumCores < PcdGet32 (PcdCoreCount); NumCores++) {
> > +    EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
> > +
> > +    CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
> > +    GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
> > +    GiccPtr->AcpiProcessorUid = NumCores;
> > +    GiccPtr->MPIDR = NumCores;
> 
> This does not seem to be quite correct, if I dump the MPIDRs from ARM-TF when
> booting with 12 cpus this is what I get.
> 
> NOTICE:  MPIDR 0
> NOTICE:  MPIDR 1
> NOTICE:  MPIDR 2
> NOTICE:  MPIDR 3
> NOTICE:  MPIDR 4
> NOTICE:  MPIDR 5
> NOTICE:  MPIDR 6
> NOTICE:  MPIDR 7
> NOTICE:  MPIDR 100
> NOTICE:  MPIDR 101
> NOTICE:  MPIDR 102
> NOTICE:  MPIDR 103
> 
> I think this will make PSCI operations from CPU8 onwards fail.
> 

I can confirm this is wrong

I did a quick hack

GiccPtr->MPIDR = ((NumCores / 8) << 8) | (NumCores % 8);

and I can boot 128 cores (MPIDR also needs fixed in SSDT I think)

[   12.637579] GICv3: CPU127: found redistributor f07 region
0:0x0000000041060000
[   12.640185] CPU127: Booted secondary processor 0x0000000f07
[0x411fd070]
[   12.676961] smp: Brought up 1 node, 128 CPUs

Considering this patch is in the works

https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg06223.html

which add MPIDR to the DT, i think you will when that is accepted be
able to use the MPIDR from there and then you are protected for the
CPU topology changing.

Thanks

Graeme


> Thanks
> 
> Graeme
> 
> 
> > +    New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
> > +  }
> > +
> > +  // GIC Distributor Structure
> > +  CopyMem (New, &Gicd, sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE));
> > +  New += sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE);
> > +
> > +  // GIC ReDistributor Structure
> > +  CopyMem (New, &Gicr, sizeof (EFI_ACPI_6_0_GICR_STRUCTURE));
> > +  New += sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
> > +
> > +  AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);
> > +
> > +  Status = AcpiTable->InstallAcpiTable (
> > +                        AcpiTable,
> > +                        (EFI_ACPI_COMMON_HEADER *)PageAddress,
> > +                        TableSize,
> > +                        &TableHandle
> > +                        );
> > +  if (EFI_ERROR(Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Failed to install MADT table\n"));
> > +  }
> > +
> > +  return Status;
> > +}
> > +
> >  EFI_STATUS
> >  EFIAPI
> >  InitializeSbsaQemuAcpiDxe (
> > @@ -68,8 +205,27 @@ InitializeSbsaQemuAcpiDxe (
> >    IN EFI_SYSTEM_TABLE     *SystemTable
> >    )
> >  {
> > +  EFI_STATUS                     Status;
> > +  EFI_ACPI_TABLE_PROTOCOL        *AcpiTable;
> > +
> >    // Parse the device tree and get the number of CPUs
> >    CountCpusFromFdt ();
> >  
> > +  // Check if ACPI Table Protocol has been installed
> > +  Status = gBS->LocateProtocol (
> > +                  &gEfiAcpiTableProtocolGuid,
> > +                  NULL,
> > +                  (VOID **)&AcpiTable
> > +                  );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Failed to locate ACPI Table Protocol\n"));
> > +    return Status;
> > +  }
> > +
> > +  Status = AddMadtTable (AcpiTable);
> > +  if (EFI_ERROR(Status)) {
> > +     DEBUG ((DEBUG_ERROR, "Failed to add MADT table\n"));
> > +  }
> > +
> >    return EFI_SUCCESS;
> >  }
> > -- 
> > 2.28.0
> > 

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

View/Reply Online (#64662): https://edk2.groups.io/g/devel/message/64662
Mute This Topic: https://groups.io/mt/76406685/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 edk2-platforms 5/8] SbsaQemu: AcpiDxe: Create MADT table at runtime
Posted by Tanmay Jagdale 5 years, 5 months ago
Hi Graeme,


On Thu, 27 Aug 2020 at 04:18, Graeme Gregory <graeme@nuviainc.com> wrote:

> On Wed, Aug 26, 2020 at 04:35:22PM +0100, Graeme Gregory wrote:
> > On Tue, Aug 25, 2020 at 07:09:55PM +0530, Tanmay Jagdale wrote:
> > > - Add support to create MADT table at runtime.
> > > - Included a macro for GIC Redistributor structure initialisation.
> > >
> > > Signed-off-by: Tanmay Jagdale <tanmay.jagdale@linaro.org>
> > > ---
> > >  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |
> 20 ++-
> > >  Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h     |
> 15 ++
> > >  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   |
> 156 ++++++++++++++++++++
> > >  3 files changed, 190 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > index 3795a7e11639..8125e8ba7553 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > @@ -41,9 +41,27 @@ [LibraryClasses]
> > >    UefiRuntimeServicesTableLib
> > >
> > >  [Pcd]
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
> > >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
> > >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
> > >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> > >
> > >  [Depex]
> > > -  TRUE
> > > +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
> > > +
> > > +[Guids]
> > > +  gEdkiiPlatformHasAcpiGuid
> > > +
> > > +[Protocols]
> > > +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
> > > +
> > > +[FixedPcd]
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
> > > +  gArmTokenSpaceGuid.PcdGicDistributorBase
> > > +  gArmTokenSpaceGuid.PcdGicRedistributorsBase
> > > +
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
> > > diff --git
> a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> > > index eac195b0585c..7a9a0061675f 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> > > +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
> > > @@ -22,6 +22,21 @@
> > >      FixedPcdGet32 (PcdAcpiDefaultCreatorRevision)/* UINT32
> CreatorRevision */ \
> > >    }
> > >
> > > +// Defines for MADT
> > > +#define SBSAQEMU_MADT_GIC_VBASE          0x2c020000
> > > +#define SBSAQEMU_MADT_GIC_HBASE          0x2c010000
> > > +#define SBSAQEMU_MADT_GIC_PMU_IRQ        23
> > > +#define SBSAQEMU_MADT_GICR_SIZE          0x4000000
> > > +
> > > +// Macro for MADT GIC Redistributor Structure
> > > +#define SBSAQEMU_MADT_GICR_INIT() {
>           \
> > > +   EFI_ACPI_6_0_GICR,                        /* Type */
>           \
> > > +   sizeof (EFI_ACPI_6_0_GICR_STRUCTURE),     /* Length */
>           \
> > > +   EFI_ACPI_RESERVED_WORD,                   /* Reserved */
>           \
> > > +   FixedPcdGet32 (PcdGicRedistributorsBase), /*
> DiscoveryRangeBaseAddress */   \
> > > +   SBSAQEMU_MADT_GICR_SIZE                   /* DiscoveryRangeLength
> */        \
> > > +   }
> > > +
> > >  #define SBSAQEMU_UART0_BASE              0x60000000
> > >
> > >  #define SBSAQEMU_PCI_SEG0_CONFIG_BASE    0xf0000000
> > > diff --git
> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > index 75abdae3b8ce..16cb4e904e6f 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > @@ -6,11 +6,17 @@
> > >  *  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >  *
> > >  **/
> > > +#include <IndustryStandard/Acpi.h>
> > > +#include <IndustryStandard/SbsaQemuAcpi.h>
> > > +#include <Library/AcpiLib.h>
> > > +#include <Library/BaseMemoryLib.h>
> > >  #include <Library/DebugLib.h>
> > > +#include <Library/MemoryAllocationLib.h>
> > >  #include <Library/PcdLib.h>
> > >  #include <Library/UefiBootServicesTableLib.h>
> > >  #include <Library/UefiDriverEntryPoint.h>
> > >  #include <Library/UefiLib.h>
> > > +#include <Protocol/AcpiTable.h>
> > >  #include <Protocol/FdtClient.h>
> > >  #include <libfdt.h>
> > >
> > > @@ -61,6 +67,137 @@ CountCpusFromFdt (
> > >    ASSERT_RETURN_ERROR (PcdStatus);
> > >  }
> > >
> > > +/*
> > > + * A Function to Compute the ACPI Table Checksum
> > > + */
> > > +VOID
> > > +AcpiPlatformChecksum (
> > > +  IN UINT8      *Buffer,
> > > +  IN UINTN      Size
> > > +  )
> > > +{
> > > +  UINTN ChecksumOffset;
> > > +
> > > +  ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
> > > +
> > > +  // Set checksum field to 0 since it is used as part of the
> calculation
> > > +  Buffer[ChecksumOffset] = 0;
> > > +
> > > +  Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);
> > > +}
> > > +
> > > +/*
> > > + * A function that add the MADT ACPI table.
> > > +  IN EFI_ACPI_COMMON_HEADER    *CurrentTable
> > > + */
> > > +EFI_STATUS
> > > +AddMadtTable (
> > > +  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
> > > +  )
> > > +{
> > > +  EFI_STATUS            Status;
> > > +  UINTN                 TableHandle;
> > > +  UINT32                TableSize;
> > > +  EFI_PHYSICAL_ADDRESS  PageAddress;
> > > +  UINT8                 *New;
> > > +  UINT32                NumCores;
> > > +
> > > +  // Initialize MADT ACPI Header
> > > +  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header = {
> > > +     SBSAQEMU_ACPI_HEADER
> (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> > > +
>  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER,
> > > +
>  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION),
> > > +      0, 0 };
> > > +
> > > +  // Initialize GICC Structure
> > > +  EFI_ACPI_6_0_GIC_STRUCTURE Gicc = EFI_ACPI_6_0_GICC_STRUCTURE_INIT (
> > > +    0,                                     /* GicID */
> > > +    0,                                     /* AcpiCpuUid */
> > > +    0,                                     /* Mpidr */
> > > +    EFI_ACPI_6_0_GIC_ENABLED,              /* Flags */
> > > +    SBSAQEMU_MADT_GIC_PMU_IRQ,             /* PMU Irq */
> > > +    FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */
> > > +    SBSAQEMU_MADT_GIC_VBASE,               /* GicVBase */
> > > +    SBSAQEMU_MADT_GIC_HBASE,               /* GicHBase */
> > > +    25,                                    /* GsivId */
> > > +    0,                                     /* GicRBase */
> > > +    0                                      /* Efficiency */
> > > +    );
> > > +
> > > +  // Initialize GIC Distributor Structure
> > > +  EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE Gicd =
> > > +    EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT (
> > > +      0,
> > > +      FixedPcdGet32 (PcdGicDistributorBase),
> > > +      0,
> > > +      3 /* GicVersion */
> > > +    );
> > > +
> > > + // Initialize GIC Redistributor Structure
> > > +  EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();
> > > +
> > > +  // Get CoreCount which was determined eariler after parsing device
> tree
> > > +  NumCores = PcdGet32 (PcdCoreCount);
> > > +
> > > +  // Calculate the new table size based on the number of cores
> > > +  TableSize = sizeof
> (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
> > > +               (sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) +
> > > +               sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) +
> > > +               sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
> > > +
> > > +  Status = gBS->AllocatePages (
> > > +                  AllocateAnyPages,
> > > +                  EfiACPIReclaimMemory,
> > > +                  EFI_SIZE_TO_PAGES (TableSize),
> > > +                  &PageAddress
> > > +                  );
> > > +  if (EFI_ERROR(Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "Failed to allocate pages for MADT
> table\n"));
> > > +    return EFI_OUT_OF_RESOURCES;
> > > +  }
> > > +
> > > +  New = (UINT8 *)(UINTN) PageAddress;
> > > +  ZeroMem (New, TableSize);
> > > +
> > > +  // Add the  ACPI Description table header
> > > +  CopyMem (New, &Header, sizeof
> (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER));
> > > +  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
> > > +  New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
> > > +
> > > +  // Add new GICC structures for the Cores
> > > +  for (NumCores = 0; NumCores < PcdGet32 (PcdCoreCount); NumCores++) {
> > > +    EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
> > > +
> > > +    CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
> > > +    GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
> > > +    GiccPtr->AcpiProcessorUid = NumCores;
> > > +    GiccPtr->MPIDR = NumCores;
> >
> > This does not seem to be quite correct, if I dump the MPIDRs from ARM-TF
> when
> > booting with 12 cpus this is what I get.
> >
> > NOTICE:  MPIDR 0
> > NOTICE:  MPIDR 1
> > NOTICE:  MPIDR 2
> > NOTICE:  MPIDR 3
> > NOTICE:  MPIDR 4
> > NOTICE:  MPIDR 5
> > NOTICE:  MPIDR 6
> > NOTICE:  MPIDR 7
> > NOTICE:  MPIDR 100
> > NOTICE:  MPIDR 101
> > NOTICE:  MPIDR 102
> > NOTICE:  MPIDR 103
> >
> > I think this will make PSCI operations from CPU8 onwards fail.
> >
>
> I can confirm this is wrong
>
> I did a quick hack
>
> GiccPtr->MPIDR = ((NumCores / 8) << 8) | (NumCores % 8);
>
> Thanks for the fix.


> and I can boot 128 cores (MPIDR also needs fixed in SSDT I think)
>
> [   12.637579] GICv3: CPU127: found redistributor f07 region
> 0:0x0000000041060000
> [   12.640185] CPU127: Booted secondary processor 0x0000000f07
> [0x411fd070]
> [   12.676961] smp: Brought up 1 node, 128 CPUs
>
> Considering this patch is in the works
>
> https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg06223.html
>
> which add MPIDR to the DT, i think you will when that is accepted be
> able to use the MPIDR from there and then you are protected for the
> CPU topology changing.
>
> So shall I push an interim patch that uses the aforementioned formula
to derive the MPIDR ? Or wait for the qemu patch to get merged and
then read MPIDR directly from there ?

Thanks,
Tanmay

Thanks
>
> Graeme
>
>
> > Thanks
> >
> > Graeme
> >
> >
> > > +    New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
> > > +  }
> > > +
> > > +  // GIC Distributor Structure
> > > +  CopyMem (New, &Gicd, sizeof
> (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE));
> > > +  New += sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE);
> > > +
> > > +  // GIC ReDistributor Structure
> > > +  CopyMem (New, &Gicr, sizeof (EFI_ACPI_6_0_GICR_STRUCTURE));
> > > +  New += sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
> > > +
> > > +  AcpiPlatformChecksum ((UINT8*) PageAddress, TableSize);
> > > +
> > > +  Status = AcpiTable->InstallAcpiTable (
> > > +                        AcpiTable,
> > > +                        (EFI_ACPI_COMMON_HEADER *)PageAddress,
> > > +                        TableSize,
> > > +                        &TableHandle
> > > +                        );
> > > +  if (EFI_ERROR(Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "Failed to install MADT table\n"));
> > > +  }
> > > +
> > > +  return Status;
> > > +}
> > > +
> > >  EFI_STATUS
> > >  EFIAPI
> > >  InitializeSbsaQemuAcpiDxe (
> > > @@ -68,8 +205,27 @@ InitializeSbsaQemuAcpiDxe (
> > >    IN EFI_SYSTEM_TABLE     *SystemTable
> > >    )
> > >  {
> > > +  EFI_STATUS                     Status;
> > > +  EFI_ACPI_TABLE_PROTOCOL        *AcpiTable;
> > > +
> > >    // Parse the device tree and get the number of CPUs
> > >    CountCpusFromFdt ();
> > >
> > > +  // Check if ACPI Table Protocol has been installed
> > > +  Status = gBS->LocateProtocol (
> > > +                  &gEfiAcpiTableProtocolGuid,
> > > +                  NULL,
> > > +                  (VOID **)&AcpiTable
> > > +                  );
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "Failed to locate ACPI Table Protocol\n"));
> > > +    return Status;
> > > +  }
> > > +
> > > +  Status = AddMadtTable (AcpiTable);
> > > +  if (EFI_ERROR(Status)) {
> > > +     DEBUG ((DEBUG_ERROR, "Failed to add MADT table\n"));
> > > +  }
> > > +
> > >    return EFI_SUCCESS;
> > >  }
> > > --
> > > 2.28.0
> > >
>

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

View/Reply Online (#64671): https://edk2.groups.io/g/devel/message/64671
Mute This Topic: https://groups.io/mt/76406685/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 edk2-platforms 5/8] SbsaQemu: AcpiDxe: Create MADT table at runtime
Posted by Graeme Gregory 5 years, 5 months ago
On Thu, Aug 27, 2020 at 08:26:21AM +0530, Tanmay Jagdale wrote:
> Hi Graeme,
> 
> 
> On Thu, 27 Aug 2020 at 04:18, Graeme Gregory <[1]graeme@nuviainc.com> wrote:
> 
>     On Wed, Aug 26, 2020 at 04:35:22PM +0100, Graeme Gregory wrote:
>     > On Tue, Aug 25, 2020 at 07:09:55PM +0530, Tanmay Jagdale wrote:
>     > > - Add support to create MADT table at runtime.
>     > > - Included a macro for GIC Redistributor structure initialisation.
>     > >
>     > > Signed-off-by: Tanmay Jagdale <[2]tanmay.jagdale@linaro.org>
>     > > ---
>     > >  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 
>     20 ++-
>     > >  Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h     | 
>     15 ++
>     > >  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   |
>     156 ++++++++++++++++++++
>     > >  3 files changed, 190 insertions(+), 1 deletion(-)
>     > >
>     > > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/
>     SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/
>     SbsaQemuAcpiDxe.inf
>     > > index 3795a7e11639..8125e8ba7553 100644
>     > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
>     > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
>     > > @@ -41,9 +41,27 @@ [LibraryClasses]
>     > >    UefiRuntimeServicesTableLib
>     > > 
>     > >  [Pcd]
>     > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
>     > >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>     > >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
>     > >    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>     > > 
>     > >  [Depex]
>     > > -  TRUE
>     > > +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
>     > > +
>     > > +[Guids]
>     > > +  gEdkiiPlatformHasAcpiGuid
>     > > +
>     > > +[Protocols]
>     > > +  gEfiAcpiTableProtocolGuid                       ## CONSUMES
>     > > +
>     > > +[FixedPcd]
>     > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
>     > > +  gArmTokenSpaceGuid.PcdGicDistributorBase
>     > > +  gArmTokenSpaceGuid.PcdGicRedistributorsBase
>     > > +
>     > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
>     > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
>     > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
>     > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
>     > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
>     > > diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/
>     SbsaQemuAcpi.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/
>     SbsaQemuAcpi.h
>     > > index eac195b0585c..7a9a0061675f 100644
>     > > --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
>     > > +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
>     > > @@ -22,6 +22,21 @@
>     > >      FixedPcdGet32 (PcdAcpiDefaultCreatorRevision)/* UINT32 
>     CreatorRevision */ \
>     > >    }
>     > > 
>     > > +// Defines for MADT
>     > > +#define SBSAQEMU_MADT_GIC_VBASE          0x2c020000
>     > > +#define SBSAQEMU_MADT_GIC_HBASE          0x2c010000
>     > > +#define SBSAQEMU_MADT_GIC_PMU_IRQ        23
>     > > +#define SBSAQEMU_MADT_GICR_SIZE          0x4000000
>     > > +
>     > > +// Macro for MADT GIC Redistributor Structure
>     > > +#define SBSAQEMU_MADT_GICR_INIT() {                                   
>             \
>     > > +   EFI_ACPI_6_0_GICR,                        /* Type */               
>             \
>     > > +   sizeof (EFI_ACPI_6_0_GICR_STRUCTURE),     /* Length */             
>             \
>     > > +   EFI_ACPI_RESERVED_WORD,                   /* Reserved */           
>             \
>     > > +   FixedPcdGet32 (PcdGicRedistributorsBase), /*
>     DiscoveryRangeBaseAddress */   \
>     > > +   SBSAQEMU_MADT_GICR_SIZE                   /* DiscoveryRangeLength *
>     /        \
>     > > +   }
>     > > +
>     > >  #define SBSAQEMU_UART0_BASE              0x60000000
>     > > 
>     > >  #define SBSAQEMU_PCI_SEG0_CONFIG_BASE    0xf0000000
>     > > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/
>     SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/
>     SbsaQemuAcpiDxe.c
>     > > index 75abdae3b8ce..16cb4e904e6f 100644
>     > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>     > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>     > > @@ -6,11 +6,17 @@
>     > >  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>     > >  *
>     > >  **/
>     > > +#include <IndustryStandard/Acpi.h>
>     > > +#include <IndustryStandard/SbsaQemuAcpi.h>
>     > > +#include <Library/AcpiLib.h>
>     > > +#include <Library/BaseMemoryLib.h>
>     > >  #include <Library/DebugLib.h>
>     > > +#include <Library/MemoryAllocationLib.h>
>     > >  #include <Library/PcdLib.h>
>     > >  #include <Library/UefiBootServicesTableLib.h>
>     > >  #include <Library/UefiDriverEntryPoint.h>
>     > >  #include <Library/UefiLib.h>
>     > > +#include <Protocol/AcpiTable.h>
>     > >  #include <Protocol/FdtClient.h>
>     > >  #include <libfdt.h>
>     > > 
>     > > @@ -61,6 +67,137 @@ CountCpusFromFdt (
>     > >    ASSERT_RETURN_ERROR (PcdStatus);
>     > >  }
>     > > 
>     > > +/*
>     > > + * A Function to Compute the ACPI Table Checksum
>     > > + */
>     > > +VOID
>     > > +AcpiPlatformChecksum (
>     > > +  IN UINT8      *Buffer,
>     > > +  IN UINTN      Size
>     > > +  )
>     > > +{
>     > > +  UINTN ChecksumOffset;
>     > > +
>     > > +  ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
>     > > +
>     > > +  // Set checksum field to 0 since it is used as part of the
>     calculation
>     > > +  Buffer[ChecksumOffset] = 0;
>     > > +
>     > > +  Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);
>     > > +}
>     > > +
>     > > +/*
>     > > + * A function that add the MADT ACPI table.
>     > > +  IN EFI_ACPI_COMMON_HEADER    *CurrentTable
>     > > + */
>     > > +EFI_STATUS
>     > > +AddMadtTable (
>     > > +  IN EFI_ACPI_TABLE_PROTOCOL   *AcpiTable
>     > > +  )
>     > > +{
>     > > +  EFI_STATUS            Status;
>     > > +  UINTN                 TableHandle;
>     > > +  UINT32                TableSize;
>     > > +  EFI_PHYSICAL_ADDRESS  PageAddress;
>     > > +  UINT8                 *New;
>     > > +  UINT32                NumCores;
>     > > +
>     > > +  // Initialize MADT ACPI Header
>     > > +  EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header = {
>     > > +     SBSAQEMU_ACPI_HEADER
>     (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
>     > > +                         
>      EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER,
>     > > +                         
>      EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION),
>     > > +      0, 0 };
>     > > +
>     > > +  // Initialize GICC Structure
>     > > +  EFI_ACPI_6_0_GIC_STRUCTURE Gicc = EFI_ACPI_6_0_GICC_STRUCTURE_INIT (
>     > > +    0,                                     /* GicID */
>     > > +    0,                                     /* AcpiCpuUid */
>     > > +    0,                                     /* Mpidr */
>     > > +    EFI_ACPI_6_0_GIC_ENABLED,              /* Flags */
>     > > +    SBSAQEMU_MADT_GIC_PMU_IRQ,             /* PMU Irq */
>     > > +    FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */
>     > > +    SBSAQEMU_MADT_GIC_VBASE,               /* GicVBase */
>     > > +    SBSAQEMU_MADT_GIC_HBASE,               /* GicHBase */
>     > > +    25,                                    /* GsivId */
>     > > +    0,                                     /* GicRBase */
>     > > +    0                                      /* Efficiency */
>     > > +    );
>     > > +
>     > > +  // Initialize GIC Distributor Structure
>     > > +  EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE Gicd =
>     > > +    EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT (
>     > > +      0,
>     > > +      FixedPcdGet32 (PcdGicDistributorBase),
>     > > +      0,
>     > > +      3 /* GicVersion */
>     > > +    );
>     > > +
>     > > + // Initialize GIC Redistributor Structure
>     > > +  EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();
>     > > +
>     > > +  // Get CoreCount which was determined eariler after parsing device
>     tree
>     > > +  NumCores = PcdGet32 (PcdCoreCount);
>     > > +
>     > > +  // Calculate the new table size based on the number of cores
>     > > +  TableSize = sizeof
>     (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
>     > > +               (sizeof (EFI_ACPI_6_0_GIC_STRUCTURE) * NumCores) +
>     > > +               sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE) +
>     > > +               sizeof (EFI_ACPI_6_0_GICR_STRUCTURE);
>     > > +
>     > > +  Status = gBS->AllocatePages (
>     > > +                  AllocateAnyPages,
>     > > +                  EfiACPIReclaimMemory,
>     > > +                  EFI_SIZE_TO_PAGES (TableSize),
>     > > +                  &PageAddress
>     > > +                  );
>     > > +  if (EFI_ERROR(Status)) {
>     > > +    DEBUG ((DEBUG_ERROR, "Failed to allocate pages for MADT table\
>     n"));
>     > > +    return EFI_OUT_OF_RESOURCES;
>     > > +  }
>     > > +
>     > > +  New = (UINT8 *)(UINTN) PageAddress;
>     > > +  ZeroMem (New, TableSize);
>     > > +
>     > > +  // Add the  ACPI Description table header
>     > > +  CopyMem (New, &Header, sizeof
>     (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER));
>     > > +  ((EFI_ACPI_DESCRIPTION_HEADER*) New)->Length = TableSize;
>     > > +  New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
>     > > +
>     > > +  // Add new GICC structures for the Cores
>     > > +  for (NumCores = 0; NumCores < PcdGet32 (PcdCoreCount); NumCores++) {
>     > > +    EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
>     > > +
>     > > +    CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
>     > > +    GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
>     > > +    GiccPtr->AcpiProcessorUid = NumCores;
>     > > +    GiccPtr->MPIDR = NumCores;
>     >
>     > This does not seem to be quite correct, if I dump the MPIDRs from ARM-TF
>     when
>     > booting with 12 cpus this is what I get.
>     >
>     > NOTICE:  MPIDR 0
>     > NOTICE:  MPIDR 1
>     > NOTICE:  MPIDR 2
>     > NOTICE:  MPIDR 3
>     > NOTICE:  MPIDR 4
>     > NOTICE:  MPIDR 5
>     > NOTICE:  MPIDR 6
>     > NOTICE:  MPIDR 7
>     > NOTICE:  MPIDR 100
>     > NOTICE:  MPIDR 101
>     > NOTICE:  MPIDR 102
>     > NOTICE:  MPIDR 103
>     >
>     > I think this will make PSCI operations from CPU8 onwards fail.
>     >
> 
>     I can confirm this is wrong
> 
>     I did a quick hack
> 
>     GiccPtr->MPIDR = ((NumCores / 8) << 8) | (NumCores % 8);
> 
> 
> Thanks for the fix.
>  
> 
>     and I can boot 128 cores (MPIDR also needs fixed in SSDT I think)
> 
>     [   12.637579] GICv3: CPU127: found redistributor f07 region
>     0:0x0000000041060000
>     [   12.640185] CPU127: Booted secondary processor 0x0000000f07
>     [0x411fd070]
>     [   12.676961] smp: Brought up 1 node, 128 CPUs
> 
>     Considering this patch is in the works
> 
>     [3]https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg06223.html
> 
>     which add MPIDR to the DT, i think you will when that is accepted be
>     able to use the MPIDR from there and then you are protected for the
>     CPU topology changing.
> 
> 
> So shall I push an interim patch that uses the aforementioned formula
> to derive the MPIDR ? Or wait for the qemu patch to get merged and
> then read MPIDR directly from there ?
> 
IMHO wait for to see progress on Leif's patch, then issue an update to
read the MPIDR from the FDT. Obviously you can start the
development/review process for that before patch lands.

The formula only works because I happen to know what qemu generates today.
This might change with a qemu change in future though.

Graeme


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

View/Reply Online (#64677): https://edk2.groups.io/g/devel/message/64677
Mute This Topic: https://groups.io/mt/76406685/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-