Re: [Asa-dev] [EXTERNAL] [PATCH 3/4] SbsaQemu: Add new ACPI driver and FDT parser to count CPU - Asa-dev - op-lists.linaro.org

5 Aug 2020

On 04/08/2020 11:33, Tanmay Jagdale via Asa-dev wrote:
...

Add a new ACPI driver for the SbsaQemu platform which would
 handle any modifications needed for the static ACPI tables.

Added a parser function in this driver which parses the FDT
 created by Qemu to determine the number of CPUs and hence
 update the PcdCoreCount variable.


Signed-off-by: Tanmay Jagdale tanmay.jagdale@linaro.org
Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |  9 ++-
  Platform/Qemu/SbsaQemu/SbsaQemu.fdf           |  7 +-
  Silicon/Qemu/SbsaQemu/Acpi.dsc.inc            | 10 +--
  .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |  2 -
  Silicon/Qemu/SbsaQemu/AcpiTables/Madt.aslc    |  4 +-
  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 72 +++++++++++++++++++
  .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf       | 49 +++++++++++++
  .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  6 --
  .../SbsaQemuPlatformDxe.inf                   |  7 --
  Silicon/Qemu/SbsaQemu/SbsaQemu.dec            |  4 ++
  10 files changed, 134 insertions(+), 36 deletions(-)
  create mode 100644 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
  create mode 100644 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index ac1398af8f..d42b9cd4de 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -75,6 +75,7 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
    ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
    FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf

FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
 UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
 OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf

@@ -217,7 +218,6 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
  
    PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf

FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
 ArmPlatformLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf

[LibraryClasses.common.DXE_CORE]
@@ -300,10 +300,6 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
    gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
    gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320

# Core and Cluster Count
gArmPlatformTokenSpaceGuid.PcdCoreCount|4
gArmPlatformTokenSpaceGuid.PcdClusterCount|1
# DEBUG_ASSERT_ENABLED       0x01
  # DEBUG_PRINT_ENABLED        0x02
  # DEBUG_CODE_ENABLED         0x04

@@ -480,6 +476,9 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
  [PcdsDynamicDefault.common]
    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3

# Core and Cluster Count
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|1
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount|1

# System Memory Size -- 128 MB initially, actual size will be fetched from DT
    # TODO as no DT will be used we should pass this by some other method
diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
index 7f1a60e3ee..3bcf0bf004 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf
@@ -232,6 +232,7 @@ READ_LOCK_STATUS   = TRUE
    #
    INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
    INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf

INF Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
 INF RuleOverride = ACPITABLE Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
 INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf

@@ -309,9 +310,3 @@ READ_LOCK_STATUS   = TRUE
    }
  
  !include ArmVirtPkg/ArmVirtRules.fdf.inc



-[Rule.Common.USER_DEFINED.ACPITABLE]

FILE FREEFORM = $(NAMED_GUID) {
RAW ACPI               |.acpi
RAW ASL                |.aml
}

Unrelated change that has snuck in?
...
diff --git a/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
index 76d0fac079..c3e910763c 100644
--- a/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
+++ b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc
@@ -30,13 +30,7 @@
    #
    # ACPI support
    #

MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf {
<LibraryClasses>

 NULL|EmbeddedPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf



<PcdsFixedAtBuild>

 # support ACPI v5.0 or later


 gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20


}


MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
 MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
 Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf

diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index 63d5754e16..9f8ced9db0 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -33,8 +33,6 @@
    MdeModulePkg/MdeModulePkg.dec
  
  [FixedPcd]

gArmPlatformTokenSpaceGuid.PcdClusterCount
gArmPlatformTokenSpaceGuid.PcdCoreCount
 gArmTokenSpaceGuid.PcdGicDistributorBase
 gArmTokenSpaceGuid.PcdGicRedistributorsBase

diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Madt.aslc b/Silicon/Qemu/SbsaQemu/AcpiTables/Madt.aslc
index 7bec2d4ef3..9e39e2e139 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/Madt.aslc
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Madt.aslc
@@ -15,8 +15,8 @@
  
  #include "AcpiTables.h"
  
-#define CORES           (FixedPcdGet32 (PcdClusterCount) * \

                    FixedPcdGet32 (PcdCoreCount))



+#define CORES	4

//
// Multiple APIC Description Table
//

This seems to be purely needed because patch 1 created this file and as 
I commented in patch 1 I don't think we need it.
Graeme
...
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
new file mode 100644
index 0000000000..87f2c1aaec
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -0,0 +1,72 @@
+/** @file
+* This file is an ACPI driver for the Qemu SBSA platform.
+*
+*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/UefiLib.h>
+#include <Protocol/FdtClient.h>
+#include <libfdt.h>



+/*


A function that walks through the Device Tree created



by Qemu and counts the number of CPUs present in it.


*/

+VOID
+SbsaQemuCountCpusFromFdt(

VOID

+)
+{

VOID           *DeviceTreeBase;
INT32          Node, Prev;
RETURN_STATUS  PcdStatus;
INT32          CpuNode;
INT32          CpuCount;

DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
ASSERT (DeviceTreeBase != NULL);

// Make sure we have a valid device tree blob
ASSERT (fdt_check_header (DeviceTreeBase) == 0);

CpuNode = fdt_path_offset (DeviceTreeBase, "/cpus");
if (CpuNode <= 0) {
DEBUG((EFI_D_ERROR, "Unable to locate /cpus in device tree\n"));
return;
}

CpuCount = 0;

// Walk through /cpus node and count the number of subnodes.
// The count of these subnodes corresponds to the numer of
// CPUs created by Qemu.
for (Prev = fdt_first_subnode(DeviceTreeBase, CpuNode);; Prev = Node) {
CpuCount++;
Node = fdt_next_subnode (DeviceTreeBase, Prev);
if (Node < 0) {
 break;


}
}

PcdStatus = PcdSet32S (PcdCoreCount, CpuCount);
ASSERT_RETURN_ERROR (PcdStatus);

+}



+EFI_STATUS
+EFIAPI
+InitializeSbsaQemuAcpiDxe (

IN EFI_HANDLE           ImageHandle,
IN EFI_SYSTEM_TABLE     *SystemTable
)

+{

// Parse the device tree and get the number of CPUs
SbsaQemuCountCpusFromFdt();

return EFI_SUCCESS;

+}
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
new file mode 100644
index 0000000000..efc4d295bf
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -0,0 +1,49 @@
+## @file
+#  This driver modifies ACPI tables for the Qemu SBSA platform
+#
+#  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##



+[Defines]

INF_VERSION                    = 0x0001001d
BASE_NAME                      = SbsaQemuAcpiDxe
FILE_GUID                      = 6c592dc9-76c8-474f-93b2-bf1e8f15ae35
MODULE_TYPE                    = DXE_DRIVER
VERSION_STRING                 = 1.0

ENTRY_POINT                    = InitializeSbsaQemuAcpiDxe


+[Sources]

SbsaQemuAcpiDxe.c


+[Packages]

ArmPkg/ArmPkg.dec
ArmPlatformPkg/ArmPlatformPkg.dec
ArmVirtPkg/ArmVirtPkg.dec
EmbeddedPkg/EmbeddedPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
Silicon/Qemu/SbsaQemu/SbsaQemu.dec


+[LibraryClasses]

ArmLib
BaseMemoryLib
DebugLib
BaseLib
FdtLib
DxeServicesLib
PcdLib
UefiDriverEntryPoint
UefiLib
UefiRuntimeServicesTableLib


+[Pcd]

gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress


+[Depex]

TRUE

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
index f55dbdb453..b7270a07ab 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
@@ -51,11 +51,5 @@ InitializeSbsaQemuPlatformDxe (
      return Status;
    }

Status = gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
             &gEdkiiPlatformHasAcpiGuid, NULL, NULL);


if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "FAILED TO INSTALL gEDKIIPlatformHasAcpiGuid protocol\n"));
}
return EFI_SUCCESS;
}

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
index a50d563782..6c9c28946e 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
@@ -39,13 +39,6 @@
  [Depex]
    TRUE
  
-[Guids]

gEdkiiPlatformHasAcpiGuid
[Protocols]
  gEdkiiNonDiscoverableDeviceProtocolGuid         ## PRODUCES
gEfiAcpiTableProtocolGuid                       ## CONSUMES
 gEfiPciIoProtocolGuid                           ## CONSUMES


-[FixedPcd]

gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision

diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
index cd879f4dbd..6aca632996 100644
--- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
+++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
@@ -35,3 +35,7 @@
    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase|0|UINT64|0x00000003
    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize|0x10000|UINT32|0x00000004
    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress|0x10000000000|UINT64|0x00000005



+[PcdsDynamic.common]

gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|0x1|UINT32|0x00000006
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount|0x1|UINT32|0x00000007


    