[edk2-devel] [PATCH edk2-platforms v2 0/4] get rid of DeviceTree from SbsaQemu

[edk2-devel] [PATCH edk2-platforms v2 0/4] get rid of DeviceTree from SbsaQemu

[Marcin Juszkiewicz]

We want to stop parsing DeviceTree to gather hardware information.

Instead we ask TF-A for those details using SMC calls. On real hardware
platform it could be asking on-board Embedded Controller.

If TF-A answer to SMC call would be not success then we assume that it
is old version and go back to parsing DeviceTree data directly. Just now
it is present in one place (SbsaQemuHardwareInfoLib) together with new
code for handling SMC stuff.

I hope that we can drop FdtHandler part as part of 202411 release.

TF-A part: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/25707

After this change the only place where DT data is parsed directly is
checking for memory nodes. Patches for that code will be sent once we
get SMC calls into TF-A.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
Marcin Juszkiewicz (4):
      Platform/SbsaQemu: add SbsaQemuHardwareInfoLib
      Platform/SbsaQemu: read amount of cpus during init
      Platform/SbsaQemu: use PcdCoreCount directly
      Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc                 |   4 +-
 Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf    |   6 +-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf     |   4 +-
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf     |   4 +-
 .../SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf  |  33 ----
 .../SbsaQemuHardwareInfoLib.inf                     |  34 ++++
 .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h |   2 +
 .../Qemu/SbsaQemu/Include/Library/FdtHelperLib.h    |  36 ----
 .../Include/Library/SbsaQemuHardwareInfoLib.h       |  45 +++++
 Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c      |  10 +-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c       |  16 +-
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c       |   9 +-
 .../SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c    |  98 ----------
 .../SbsaQemuHardwareInfoLib.c                       | 204 ++++++++++++++++++++
 14 files changed, 308 insertions(+), 197 deletions(-)
---
base-commit: a1598bbcd167f4a5cf61229156426ef6f0784ab3
change-id: 20240115-no-dt-for-cpu-2c511393df93

Best regards,
-- 
Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113873): https://edk2.groups.io/g/devel/message/113873
Mute This Topic: https://groups.io/mt/103758013/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 1/4] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib

[Marcin Juszkiewicz]

This library provides functions to check for hardware information.
For now it covers CPU ones:

- amount of cpu cores
- MPIDR value for cpu core
- NUMA node id for cpu core

Values are read from TF-A using platform specific SMC calls.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc                 |   3 +-
 .../SbsaQemuHardwareInfoLib.inf                     |  32 +++++++
 .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h |   2 +
 .../Include/Library/SbsaQemuHardwareInfoLib.h       |  45 +++++++++
 .../SbsaQemuHardwareInfoLib.c                       | 100 ++++++++++++++++++++
 5 files changed, 181 insertions(+), 1 deletion(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index 378600050df9..07cb3490f4cf 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -1,6 +1,6 @@
 #
 #  Copyright (c) 2021, NUVIA Inc. All rights reserved.
-#  Copyright (c) 2019, Linaro Limited. All rights reserved.
+#  Copyright (c) 2019-2024, Linaro Ltd. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -128,6 +128,7 @@ [LibraryClasses.common]
 
   FdtHelperLib|Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
   OemMiscLib|Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
+  SbsaQemuHardwareInfoLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
 
   # Debug Support
   PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
new file mode 100644
index 000000000000..8c2def1878e6
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
@@ -0,0 +1,32 @@
+#/* @file
+#
+#  Copyright (c) Linaro Ltd. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#*/
+
+[Defines]
+  INF_VERSION                    = 0x0001001c
+  BASE_NAME                      = SbsaQemuHardwareInfoLib
+  FILE_GUID                      = 6454006f-6502-46e2-9be4-4bba8d4b29fb
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ArmPlatformLib
+
+[Sources]
+  SbsaQemuHardwareInfoLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Qemu/SbsaQemu/SbsaQemu.dec
+
+[LibraryClasses]
+  ArmSmcLib
+  BaseMemoryLib
+  DebugLib
+
+ [Pcd]
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
index 7934875e4aba..e33648ee1462 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
@@ -14,5 +14,7 @@
 #define SIP_SVC_VERSION  SMC_SIP_FUNCTION_ID(1)
 #define SIP_SVC_GET_GIC  SMC_SIP_FUNCTION_ID(100)
 #define SIP_SVC_GET_GIC_ITS  SMC_SIP_FUNCTION_ID(101)
+#define SIP_SVC_GET_CPU_COUNT SMC_SIP_FUNCTION_ID(200)
+#define SIP_SVC_GET_CPU_NODE SMC_SIP_FUNCTION_ID(201)
 
 #endif /* SBSA_QEMU_SMC_H_ */
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
new file mode 100644
index 000000000000..45262baf3511
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
@@ -0,0 +1,45 @@
+/** @file
+*
+*  Copyright (c) Linaro Ltd. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#ifndef SBSA_QEMU_HARDWARE_INFO_
+#define SBSA_QEMU_HARDWARE_INFO_
+
+/**
+  Get CPU count from information passed by Qemu.
+
+**/
+VOID
+SbsaQemuGetCpuCount (
+  VOID
+  );
+
+/**
+  Get MPIDR for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
+
+  @retval                MPIDR value of CPU at index <CpuId>
+**/
+UINT64
+SbsaQemuGetMpidr (
+  IN UINTN   CpuId
+  );
+
+/**
+  Get NUMA node id for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuId    Index of cpu to retrieve NUMA node id for.
+
+  @retval                NUMA node id for CPU at index <CpuId>
+**/
+UINT64
+SbsaQemuGetCpuNumaNode (
+  IN UINTN   CpuId
+  );
+
+#endif /* SBSA_QEMU_HARDWARE_INFO_ */
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
new file mode 100644
index 000000000000..4df973fda75e
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -0,0 +1,100 @@
+/** @file
+*
+*  Copyright (c) 2021, NUVIA Inc. All rights reserved.
+*  Copyright (c) Linaro Ltd. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include <Library/ArmSmcLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/SbsaQemuHardwareInfoLib.h>
+#include <IndustryStandard/SbsaQemuSmc.h>
+
+/**
+  Get CPU count from information passed by Qemu.
+
+**/
+VOID
+SbsaQemuGetCpuCount (
+  VOID
+  )
+{
+  UINTN          Arg0;
+  UINTN          SmcResult;
+  RETURN_STATUS  Result;
+
+  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_COUNT, &Arg0, NULL, NULL);
+  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
+    DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n"));
+    Arg0 = FdtHelperCountCpus();
+  }
+
+  Result = PcdSet32S (PcdCoreCount, Arg0);
+  ASSERT_RETURN_ERROR (Result);
+
+  Arg0 = PcdGet32 (PcdCoreCount);
+
+  DEBUG ((DEBUG_INFO, "We have %d cpus.\n", Arg0));
+}
+
+/**
+  Get MPIDR for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
+
+  @retval                MPIDR value of CPU at index <CpuId>
+**/
+UINT64
+SbsaQemuGetMpidr (
+  IN UINTN   CpuId
+  )
+{
+  UINTN SmcResult;
+  UINTN Arg0;
+  UINTN Arg1;
+
+  Arg0 = CpuId;
+
+  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL);
+  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
+    DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n"));
+    Arg1 = FdtHelperGetMpidr(CpuId);
+}
+
+  DEBUG ((DEBUG_ERROR, "MPIDR for CPU:%d = %d\n", CpuId, Arg1));
+
+  return Arg1;
+}
+
+/**
+  Get NUMA node id for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuId    Index of cpu to retrieve NUMA node id for.
+
+  @retval                NUMA node id for CPU at index <CpuId>
+**/
+UINT64
+SbsaQemuGetCpuNumaNode (
+  IN UINTN   CpuId
+  )
+{
+  UINTN SmcResult;
+  UINTN Arg0;
+  UINTN Arg1;
+
+  Arg0 = CpuId;
+
+  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL);
+  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
+    /* No fallback to DeviceTree as we did not had that info earlier. */
+    DEBUG ((DEBUG_ERROR, "Couldn't find information for CPU:%d\n", CpuId));
+    return 0;
+  }
+
+  DEBUG ((DEBUG_ERROR, "NUMA node for CPU:%d = %d\n", CpuId, Arg0));
+
+  return Arg0;
+}

-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113874): https://edk2.groups.io/g/devel/message/113874
Mute This Topic: https://groups.io/mt/103758014/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 2/4] Platform/SbsaQemu: read amount of cpus during init

[Marcin Juszkiewicz]

We read it once and store in Pcd for future use.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 .../SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf | 4 +++-
 .../SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c   | 9 +++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
index 19534b7a274a..9752694a432b 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
@@ -1,7 +1,7 @@
 ## @file
 #  This driver effectuates SbsaQemu platform configuration settings
 #
-#  Copyright (c) 2019, Linaro Ltd. All rights reserved.
+#  Copyright (c) Linaro Ltd. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -32,6 +32,7 @@ [LibraryClasses]
   PcdLib
   DebugLib
   NonDiscoverableDeviceRegistrationLib
+  SbsaQemuHardwareInfoLib
   UefiDriverEntryPoint
 
 [Pcd]
@@ -46,6 +47,7 @@ [Pcd]
   gArmTokenSpaceGuid.PcdGicDistributorBase
   gArmTokenSpaceGuid.PcdGicRedistributorsBase
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
 
 
 [Depex]
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
index 4ebbe7c93a19..edd72e37f7f1 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
@@ -1,7 +1,7 @@
 /** @file
-*  FDT client protocol driver for qemu,mach-virt-ahci DT node
+*  SbsaQemu Platform Initialization
 *
-*  Copyright (c) 2019, Linaro Ltd. All rights reserved.
+*  Copyright (c) Linaro Ltd. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -12,13 +12,12 @@
 #include <Library/DebugLib.h>
 #include <Library/NonDiscoverableDeviceRegistrationLib.h>
 #include <Library/PcdLib.h>
+#include <Library/SbsaQemuHardwareInfoLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiDriverEntryPoint.h>
 #include <IndustryStandard/SbsaQemuSmc.h>
 #include <IndustryStandard/SbsaQemuPlatformVersion.h>
 
-#include <Protocol/FdtClient.h>
-
 EFI_STATUS
 EFIAPI
 InitializeSbsaQemuPlatformDxe (
@@ -123,5 +122,7 @@ InitializeSbsaQemuPlatformDxe (
     }
   }
 
+  SbsaQemuGetCpuCount();
+
   return EFI_SUCCESS;
 }

-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113875): https://edk2.groups.io/g/devel/message/113875
Mute This Topic: https://groups.io/mt/103758015/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 3/4] Platform/SbsaQemu: use PcdCoreCount directly

[Marcin Juszkiewicz]

During platform initialization we read amount of cpu cores and set
PcdCoreCount so there is no need to call FdtHandler.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf             |  6 ++----
 Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c               | 10 ++++------
 .../Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c  | 12 +++---------
 3 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
index a34f54d431d4..8e2bf8c512f1 100644
--- a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
+++ b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
@@ -3,7 +3,7 @@
 #
 #    Copyright (c) 2021, NUVIA Inc. All rights reserved.
 #    Copyright (c) 2018, Hisilicon Limited. All rights reserved.
-#    Copyright (c) 2018, Linaro Limited. All rights reserved.
+#    Copyright (c) 2023, Linaro Ltd. All rights reserved.
 #
 #    SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -29,8 +29,6 @@ [Packages]
 
 [LibraryClasses]
   BaseMemoryLib
-  FdtLib
-  FdtHelperLib
   IoLib
   PcdLib
 
@@ -40,7 +38,6 @@ [Guids]
 [Pcd]
   gArmTokenSpaceGuid.PcdEmbeddedControllerFirmwareRelease
   gArmTokenSpaceGuid.PcdSystemBiosRelease
-  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
 
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSystemManufacturer
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSystemSerialNumber
@@ -56,3 +53,4 @@ [Pcd]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisManufacturer
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c
index c38f2851904f..ab97768b5ddc 100644
--- a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c
+++ b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c
@@ -2,7 +2,7 @@
 *  OemMiscLib.c
 *
 *  Copyright (c) 2021, NUVIA Inc. All rights reserved.
-*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+*  Copyright (c) Linaro Ltd. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -12,14 +12,12 @@
 #include <Guid/ZeroGuid.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
-#include <Library/FdtHelperLib.h>
 #include <Library/HiiLib.h>
 #include <Library/IoLib.h>
 #include <Library/OemMiscLib.h>
 #include <Library/PcdLib.h>
 #include <Library/SerialPortLib.h>
 #include <Library/TimerLib.h>
-#include <libfdt.h>
 
 /** Returns whether the specified processor is present or not.
 
@@ -33,7 +31,7 @@ OemIsProcessorPresent (
   UINTN ProcessorIndex
   )
 {
-  if (ProcessorIndex < FdtHelperCountCpus ()) {
+  if (ProcessorIndex < PcdGet32 (PcdCoreCount)) {
     return TRUE;
   }
 
@@ -76,7 +74,7 @@ OemGetProcessorInformation (
 {
   UINT16 ProcessorCount;
 
-  ProcessorCount = FdtHelperCountCpus ();
+  ProcessorCount = PcdGet32 (PcdCoreCount);
 
   if (ProcessorIndex < ProcessorCount) {
     ProcessorStatus->Bits.CpuStatus       = 1; // CPU enabled
@@ -121,7 +119,7 @@ OemGetMaxProcessors (
   VOID
   )
 {
-  return FdtHelperCountCpus ();
+  return PcdGet32 (PcdCoreCount);
 }
 
 /** Gets information about the cache at the specified cache level.
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 9fb17151d7b8..7ef314ae9f67 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -1,7 +1,7 @@
 /** @file
 *  This file is an ACPI driver for the Qemu SBSA platform.
 *
-*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+*  Copyright (c) Linaro Ltd. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -255,7 +255,7 @@ AddMadtTable (
  // 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
+  // Get CoreCount which was determined earlier from TF-A
   NumCores = PcdGet32 (PcdCoreCount);
 
   // Calculate the new table size based on the number of cores
@@ -291,7 +291,7 @@ AddMadtTable (
   New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
 
   // Add new GICC structures for the Cores
-  for (CoreIndex = 0; CoreIndex < PcdGet32 (PcdCoreCount); CoreIndex++) {
+  for (CoreIndex = 0; CoreIndex < NumCores; CoreIndex++) {
     EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
 
     CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
@@ -758,12 +758,6 @@ InitializeSbsaQemuAcpiDxe (
 {
   EFI_STATUS                     Status;
   EFI_ACPI_TABLE_PROTOCOL        *AcpiTable;
-  UINT32                         NumCores;
-
-  // Parse the device tree and get the number of CPUs
-  NumCores = FdtHelperCountCpus ();
-  Status = PcdSet32S (PcdCoreCount, NumCores);
-  ASSERT_RETURN_ERROR (Status);
 
   // Check if ACPI Table Protocol has been installed
   Status = gBS->LocateProtocol (

-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113876): https://edk2.groups.io/g/devel/message/113876
Mute This Topic: https://groups.io/mt/103758016/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 4/4] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib

[Marcin Juszkiewicz]

There is no need for EDK2 to know that there is DeviceTree around.
All hardware information is read using functions from
SbsaQemuHardwareInfoLib library.

Library fallbacks to parsing DT if needed (used with too old TF-A).

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc                 |   1 -
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf     |   4 +-
 .../SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf  |  33 -------
 .../SbsaQemuHardwareInfoLib.inf                     |   2 +
 .../Qemu/SbsaQemu/Include/Library/FdtHelperLib.h    |  36 -------
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c       |   4 +-
 .../SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c    |  98 ------------------
 .../SbsaQemuHardwareInfoLib.c                       | 104 ++++++++++++++++++++
 8 files changed, 110 insertions(+), 172 deletions(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index 07cb3490f4cf..bde61651da2e 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -126,7 +126,6 @@ [LibraryClasses.common]
   # ARM PL011 UART Driver
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
 
-  FdtHelperLib|Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
   OemMiscLib|Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
   SbsaQemuHardwareInfoLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
 
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index 291743b19115..9bf0a13de5d1 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -1,7 +1,7 @@
 ## @file
 #  This driver modifies ACPI tables for the Qemu SBSA platform
 #
-#  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+#  Copyright (c) Linaro Ltd. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -35,9 +35,9 @@ [LibraryClasses]
   BaseLib
   DebugLib
   DxeServicesLib
-  FdtHelperLib
   PcdLib
   PrintLib
+  SbsaQemuHardwareInfoLib
   UefiDriverEntryPoint
   UefiLib
   UefiRuntimeServicesTableLib
diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
deleted file mode 100644
index 9c059f3e5851..000000000000
--- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
+++ /dev/null
@@ -1,33 +0,0 @@
-#/** @file
-#
-#  Component description file for FdtHelperLib module
-#
-#  Copyright (c) 2021, NUVIA Inc. All rights reserved.
-#
-#  SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-#**/
-
-[Defines]
-  INF_VERSION                    = 1.29
-  BASE_NAME                      = FdtHelperLib
-  FILE_GUID                      = 34e4396f-c2fc-4f9e-ad58-0f98e99e3875
-  MODULE_TYPE                    = BASE
-  VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = FdtHelperLib
-
-[Sources.common]
-  FdtHelperLib.c
-
-[Packages]
-  EmbeddedPkg/EmbeddedPkg.dec
-  MdePkg/MdePkg.dec
-  Silicon/Qemu/SbsaQemu/SbsaQemu.dec
-
-[LibraryClasses]
-  DebugLib
-  FdtLib
-  PcdLib
-
-[FixedPcd]
-  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
index 8c2def1878e6..5358dd339eb3 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
@@ -27,6 +27,8 @@ [LibraryClasses]
   ArmSmcLib
   BaseMemoryLib
   DebugLib
+  FdtLib
 
  [Pcd]
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
deleted file mode 100644
index ea9159857215..000000000000
--- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
+++ /dev/null
@@ -1,36 +0,0 @@
-/** @file
-*  FdtHelperLib.h
-*
-*  Copyright (c) 2021, NUVIA Inc. All rights reserved.
-*
-*  SPDX-License-Identifier: BSD-2-Clause-Patent
-*
-**/
-
-#ifndef FDT_HELPER_LIB_
-#define FDT_HELPER_LIB_
-
-/**
-  Get MPIDR for a given cpu from device tree passed by Qemu.
-
-  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
-
-  @retval                MPIDR value of CPU at index <CpuId>
-**/
-UINT64
-FdtHelperGetMpidr (
-  IN UINTN   CpuId
-  );
-
-/** Walks through the Device Tree created by Qemu and counts the number
-    of CPUs present in it.
-
-    @return The number of CPUs present.
-**/
-EFIAPI
-UINT32
-FdtHelperCountCpus (
-  VOID
-  );
-
-#endif /* FDT_HELPER_LIB_ */
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 7ef314ae9f67..c446581b746e 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -15,10 +15,10 @@
 #include <Library/ArmLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
-#include <Library/FdtHelperLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/PrintLib.h>
+#include <Library/SbsaQemuHardwareInfoLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/UefiLib.h>
@@ -297,7 +297,7 @@ AddMadtTable (
     CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
     GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
     GiccPtr->AcpiProcessorUid = CoreIndex;
-    GiccPtr->MPIDR = FdtHelperGetMpidr (CoreIndex);
+    GiccPtr->MPIDR = SbsaQemuGetMpidr (CoreIndex);
     New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
   }
 
diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
deleted file mode 100644
index 7fdfb055db76..000000000000
--- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
+++ /dev/null
@@ -1,98 +0,0 @@
-/** @file
-*  FdtHelperLib.c
-*
-*  Copyright (c) 2021, NUVIA Inc. All rights reserved.
-*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
-*
-*  SPDX-License-Identifier: BSD-2-Clause-Patent
-*
-**/
-
-#include <Uefi.h>
-#include <Library/DebugLib.h>
-#include <Library/FdtHelperLib.h>
-#include <Library/PcdLib.h>
-#include <libfdt.h>
-
-STATIC INT32 mFdtFirstCpuOffset;
-STATIC INT32 mFdtCpuNodeSize;
-
-/**
-  Get MPIDR for a given cpu from device tree passed by Qemu.
-
-  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
-
-  @retval                MPIDR value of CPU at index <CpuId>
-**/
-UINT64
-FdtHelperGetMpidr (
-  IN UINTN   CpuId
-  )
-{
-  VOID           *DeviceTreeBase;
-  CONST UINT64   *RegVal;
-  INT32          Len;
-
-  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
-  ASSERT (DeviceTreeBase != NULL);
-
-  RegVal = fdt_getprop (DeviceTreeBase,
-             mFdtFirstCpuOffset + (CpuId * mFdtCpuNodeSize),
-             "reg",
-             &Len);
-  if (!RegVal) {
-    DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
-    return 0;
-  }
-
-  return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
-}
-
-/** Walks through the Device Tree created by Qemu and counts the number
-    of CPUs present in it.
-
-    @return The number of CPUs present.
-**/
-EFIAPI
-UINT32
-FdtHelperCountCpus (
-  VOID
-  )
-{
-  VOID   *DeviceTreeBase;
-  INT32  Node;
-  INT32  Prev;
-  INT32  CpuNode;
-  UINT32 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 ((DEBUG_ERROR, "Unable to locate /cpus in device tree\n"));
-    return 0;
-  }
-
-  CpuCount = 0;
-
-  // Walk through /cpus node and count the number of subnodes.
-  // The count of these subnodes corresponds to the number of
-  // CPUs created by Qemu.
-  Prev = fdt_first_subnode (DeviceTreeBase, CpuNode);
-  mFdtFirstCpuOffset = Prev;
-  while (1) {
-    CpuCount++;
-    Node = fdt_next_subnode (DeviceTreeBase, Prev);
-    if (Node < 0) {
-      break;
-    }
-    mFdtCpuNodeSize = Node - Prev;
-    Prev = Node;
-  }
-
-  return CpuCount;
-}
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
index 4df973fda75e..900493e02d7f 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -11,8 +11,112 @@
 #include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
 #include <Library/SbsaQemuHardwareInfoLib.h>
+#include <libfdt.h>
 #include <IndustryStandard/SbsaQemuSmc.h>
 
+/**
+  Get MPIDR for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
+
+  @retval                MPIDR value of CPU at index <CpuId>
+**/
+UINT64
+FdtHelperGetMpidr (
+  IN UINTN   CpuId
+  )
+{
+  VOID           *DeviceTreeBase;
+  INT32          Node;
+  INT32          Prev;
+  UINT32         CpuCount;
+  CONST UINT64   *RegVal;
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+
+  // Make sure we have a valid device tree blob
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  Node = fdt_path_offset (DeviceTreeBase, "/cpus");
+  if (Node <= 0) {
+    DEBUG ((DEBUG_ERROR, "Unable to locate /cpus in device tree\n"));
+    return 0;
+  }
+
+  CpuCount = 0;
+
+  Prev = fdt_first_subnode (DeviceTreeBase, Node);
+  while (1) {
+
+    if (CpuCount == CpuId) {
+      RegVal = fdt_getprop (DeviceTreeBase, Prev, "reg", NULL);
+      if (!RegVal) {
+        DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
+        return 0;
+      }
+      return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
+    }
+
+    Node = fdt_next_subnode (DeviceTreeBase, Prev);
+    if (Node < 0) {
+      break;
+    }
+    Prev = Node;
+    CpuCount++;
+  }
+
+  return 0; /* We did not found MPIDR */
+
+}
+
+/** Walks through the Device Tree created by Qemu and counts the number
+    of CPUs present in it.
+
+    @return The number of CPUs present.
+**/
+EFIAPI
+UINT32
+FdtHelperCountCpus (
+  VOID
+  )
+{
+  VOID   *DeviceTreeBase;
+  INT32  Node;
+  INT32  Prev;
+  UINT32 CpuCount;
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+
+  // Make sure we have a valid device tree blob
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  Node = fdt_path_offset (DeviceTreeBase, "/cpus");
+  if (Node <= 0) {
+    DEBUG ((DEBUG_ERROR, "Unable to locate /cpus in device tree\n"));
+    return 0;
+  }
+
+  CpuCount = 0;
+
+  // Walk through /cpus node and count the number of subnodes.
+  // The count of these subnodes corresponds to the number of
+  // CPUs created by Qemu.
+  Prev = fdt_first_subnode (DeviceTreeBase, Node);
+  while (1) {
+    CpuCount++;
+    Node = fdt_next_subnode (DeviceTreeBase, Prev);
+    if (Node < 0) {
+      break;
+    }
+    Prev = Node;
+  }
+
+  return CpuCount;
+}
+
+
 /**
   Get CPU count from information passed by Qemu.
 

-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113877): https://edk2.groups.io/g/devel/message/113877
Mute This Topic: https://groups.io/mt/103758017/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib

[Leif Lindholm]

On Tue, Jan 16, 2024 at 08:48:32 +0100, Marcin Juszkiewicz wrote:
> This library provides functions to check for hardware information.
> For now it covers CPU ones:
> 
> - amount of cpu cores
> - MPIDR value for cpu core
> - NUMA node id for cpu core
> 
> Values are read from TF-A using platform specific SMC calls.
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc                 |   3 +-
>  .../SbsaQemuHardwareInfoLib.inf                     |  32 +++++++
>  .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h |   2 +
>  .../Include/Library/SbsaQemuHardwareInfoLib.h       |  45 +++++++++
>  .../SbsaQemuHardwareInfoLib.c                       | 100 ++++++++++++++++++++
>  5 files changed, 181 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> index 378600050df9..07cb3490f4cf 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> @@ -1,6 +1,6 @@
>  #
>  #  Copyright (c) 2021, NUVIA Inc. All rights reserved.
> -#  Copyright (c) 2019, Linaro Limited. All rights reserved.
> +#  Copyright (c) 2019-2024, Linaro Ltd. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -128,6 +128,7 @@ [LibraryClasses.common]
>  
>    FdtHelperLib|Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
>    OemMiscLib|Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
> +  SbsaQemuHardwareInfoLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
>  
>    # Debug Support
>    PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
> new file mode 100644
> index 000000000000..8c2def1878e6
> --- /dev/null
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
> @@ -0,0 +1,32 @@
> +#/* @file
> +#
> +#  Copyright (c) Linaro Ltd. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#*/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001c
> +  BASE_NAME                      = SbsaQemuHardwareInfoLib
> +  FILE_GUID                      = 6454006f-6502-46e2-9be4-4bba8d4b29fb
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ArmPlatformLib
> +
> +[Sources]
> +  SbsaQemuHardwareInfoLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> +
> +[LibraryClasses]
> +  ArmSmcLib
> +  BaseMemoryLib
> +  DebugLib
> +
> + [Pcd]
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
> diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> index 7934875e4aba..e33648ee1462 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> @@ -14,5 +14,7 @@
>  #define SIP_SVC_VERSION  SMC_SIP_FUNCTION_ID(1)
>  #define SIP_SVC_GET_GIC  SMC_SIP_FUNCTION_ID(100)
>  #define SIP_SVC_GET_GIC_ITS  SMC_SIP_FUNCTION_ID(101)
> +#define SIP_SVC_GET_CPU_COUNT SMC_SIP_FUNCTION_ID(200)
> +#define SIP_SVC_GET_CPU_NODE SMC_SIP_FUNCTION_ID(201)
>  
>  #endif /* SBSA_QEMU_SMC_H_ */
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
> new file mode 100644
> index 000000000000..45262baf3511
> --- /dev/null
> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
> @@ -0,0 +1,45 @@
> +/** @file
> +*
> +*  Copyright (c) Linaro Ltd. All rights reserved.
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#ifndef SBSA_QEMU_HARDWARE_INFO_
> +#define SBSA_QEMU_HARDWARE_INFO_
> +
> +/**
> +  Get CPU count from information passed by Qemu.
> +
> +**/
> +VOID
> +SbsaQemuGetCpuCount (
> +  VOID
> +  );
> +
> +/**
> +  Get MPIDR for a given cpu from device tree passed by Qemu.
> +
> +  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
> +
> +  @retval                MPIDR value of CPU at index <CpuId>
> +**/
> +UINT64
> +SbsaQemuGetMpidr (
> +  IN UINTN   CpuId
> +  );
> +
> +/**
> +  Get NUMA node id for a given cpu from device tree passed by Qemu.
> +
> +  @param [in]   CpuId    Index of cpu to retrieve NUMA node id for.
> +
> +  @retval                NUMA node id for CPU at index <CpuId>
> +**/
> +UINT64
> +SbsaQemuGetCpuNumaNode (
> +  IN UINTN   CpuId
> +  );
> +
> +#endif /* SBSA_QEMU_HARDWARE_INFO_ */
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> new file mode 100644
> index 000000000000..4df973fda75e
> --- /dev/null
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> @@ -0,0 +1,100 @@
> +/** @file
> +*
> +*  Copyright (c) 2021, NUVIA Inc. All rights reserved.
> +*  Copyright (c) Linaro Ltd. All rights reserved.
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <Library/ArmSmcLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/SbsaQemuHardwareInfoLib.h>
> +#include <IndustryStandard/SbsaQemuSmc.h>
> +
> +/**
> +  Get CPU count from information passed by Qemu.
> +
> +**/
> +VOID
> +SbsaQemuGetCpuCount (
> +  VOID
> +  )
> +{
> +  UINTN          Arg0;
> +  UINTN          SmcResult;
> +  RETURN_STATUS  Result;
> +
> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_COUNT, &Arg0, NULL, NULL);
> +  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {

So, this may sound a little bit bonkers, but SMCCC doesn't define
return codes for SiP functions. And the SMC_ARCH_CALL_SUCCESS macro
denotes an Arm Architectural function.
Could you #define an SMC_SIP_CALL_SUCCESS *as* SMC_ARCH_CALL_SUCCESS
in some platform-specific header and then use that?

> +    DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n"));

We don't *know* the TF-A is too old. Rather print which call failed
(and that we need to attempt to get the cpu count from DT).

> +    Arg0 = FdtHelperCountCpus();

We should probably assert this as != 0? Or is that done in the function?

> +  }
> +
> +  Result = PcdSet32S (PcdCoreCount, Arg0);
> +  ASSERT_RETURN_ERROR (Result);
> +
> +  Arg0 = PcdGet32 (PcdCoreCount);

Seems redundant to re-set it to the value we know it already holds.

> +
> +  DEBUG ((DEBUG_INFO, "We have %d cpus.\n", Arg0));
> +}
> +
> +/**
> +  Get MPIDR for a given cpu from device tree passed by Qemu.
> +
> +  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
> +
> +  @retval                MPIDR value of CPU at index <CpuId>
> +**/
> +UINT64
> +SbsaQemuGetMpidr (
> +  IN UINTN   CpuId
> +  )
> +{
> +  UINTN SmcResult;
> +  UINTN Arg0;
> +  UINTN Arg1;
> +
> +  Arg0 = CpuId;
> +
> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL);
> +  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
> +    DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n"));

We don't *know* the TF-A is too old. Rather print which call failed
(and that we need to attempt to get the cpu date from DT).

> +    Arg1 = FdtHelperGetMpidr(CpuId);
> +}
> +
> +  DEBUG ((DEBUG_ERROR, "MPIDR for CPU:%d = %d\n", CpuId, Arg1));
> +
> +  return Arg1;
> +}
> +
> +/**
> +  Get NUMA node id for a given cpu from device tree passed by Qemu.
> +
> +  @param [in]   CpuId    Index of cpu to retrieve NUMA node id for.
> +
> +  @retval                NUMA node id for CPU at index <CpuId>
> +**/
> +UINT64
> +SbsaQemuGetCpuNumaNode (
> +  IN UINTN   CpuId
> +  )
> +{
> +  UINTN SmcResult;
> +  UINTN Arg0;
> +  UINTN Arg1;
> +
> +  Arg0 = CpuId;
> +
> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL);

It does seem a bit wasteful we're making the same call twice per core,
discarding one of the results.
Could we have an init function that allocates an array and
prepopulates it, with the Get-functions just returning values from the array?

> +  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
> +    /* No fallback to DeviceTree as we did not had that info earlier. */

We don't *know* the TF-A is too old. Rather print which call failed
(and that we need to attempt to get the cpu count from DT).

/
    Leif

> +    DEBUG ((DEBUG_ERROR, "Couldn't find information for CPU:%d\n", CpuId));
> +    return 0;
> +  }
> +
> +  DEBUG ((DEBUG_ERROR, "NUMA node for CPU:%d = %d\n", CpuId, Arg0));
> +
> +  return Arg0;
> +}
> 
> -- 
> 2.43.0
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114095): https://edk2.groups.io/g/devel/message/114095
Mute This Topic: https://groups.io/mt/103758014/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 3/4] Platform/SbsaQemu: use PcdCoreCount directly

[Leif Lindholm]

On Tue, Jan 16, 2024 at 08:48:34 +0100, Marcin Juszkiewicz wrote:
> During platform initialization we read amount of cpu cores and set
> PcdCoreCount so there is no need to call FdtHandler.
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>  Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf             |  6 ++----
>  Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c               | 10 ++++------
>  .../Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c  | 12 +++---------
>  3 files changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
> index a34f54d431d4..8e2bf8c512f1 100644
> --- a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
> +++ b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #    Copyright (c) 2021, NUVIA Inc. All rights reserved.
>  #    Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> -#    Copyright (c) 2018, Linaro Limited. All rights reserved.
> +#    Copyright (c) 2023, Linaro Ltd. All rights reserved.
>  #
>  #    SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -29,8 +29,6 @@ [Packages]
>  
>  [LibraryClasses]
>    BaseMemoryLib
> -  FdtLib
> -  FdtHelperLib
>    IoLib
>    PcdLib
>  
> @@ -40,7 +38,6 @@ [Guids]
>  [Pcd]
>    gArmTokenSpaceGuid.PcdEmbeddedControllerFirmwareRelease
>    gArmTokenSpaceGuid.PcdSystemBiosRelease
> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>  
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSystemManufacturer
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSystemSerialNumber
> @@ -56,3 +53,4 @@ [Pcd]
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisManufacturer
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
> diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c
> index c38f2851904f..ab97768b5ddc 100644
> --- a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c
> +++ b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c
> @@ -2,7 +2,7 @@
>  *  OemMiscLib.c
>  *
>  *  Copyright (c) 2021, NUVIA Inc. All rights reserved.
> -*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
> +*  Copyright (c) Linaro Ltd. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -12,14 +12,12 @@
>  #include <Guid/ZeroGuid.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
> -#include <Library/FdtHelperLib.h>
>  #include <Library/HiiLib.h>
>  #include <Library/IoLib.h>
>  #include <Library/OemMiscLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/SerialPortLib.h>
>  #include <Library/TimerLib.h>
> -#include <libfdt.h>
>  
>  /** Returns whether the specified processor is present or not.
>  
> @@ -33,7 +31,7 @@ OemIsProcessorPresent (
>    UINTN ProcessorIndex
>    )
>  {
> -  if (ProcessorIndex < FdtHelperCountCpus ()) {
> +  if (ProcessorIndex < PcdGet32 (PcdCoreCount)) {
>      return TRUE;
>    }
>  
> @@ -76,7 +74,7 @@ OemGetProcessorInformation (
>  {
>    UINT16 ProcessorCount;
>  
> -  ProcessorCount = FdtHelperCountCpus ();
> +  ProcessorCount = PcdGet32 (PcdCoreCount);
>  
>    if (ProcessorIndex < ProcessorCount) {
>      ProcessorStatus->Bits.CpuStatus       = 1; // CPU enabled
> @@ -121,7 +119,7 @@ OemGetMaxProcessors (
>    VOID
>    )
>  {
> -  return FdtHelperCountCpus ();
> +  return PcdGet32 (PcdCoreCount);
>  }
>  
>  /** Gets information about the cache at the specified cache level.
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 9fb17151d7b8..7ef314ae9f67 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -1,7 +1,7 @@
>  /** @file
>  *  This file is an ACPI driver for the Qemu SBSA platform.
>  *
> -*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
> +*  Copyright (c) Linaro Ltd. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -255,7 +255,7 @@ AddMadtTable (
>   // 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
> +  // Get CoreCount which was determined earlier from TF-A

Where we got the information from no longer matters, since we've
abstracted that away.

/
    Leif

>    NumCores = PcdGet32 (PcdCoreCount);
>  
>    // Calculate the new table size based on the number of cores
> @@ -291,7 +291,7 @@ AddMadtTable (
>    New += sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
>  
>    // Add new GICC structures for the Cores
> -  for (CoreIndex = 0; CoreIndex < PcdGet32 (PcdCoreCount); CoreIndex++) {
> +  for (CoreIndex = 0; CoreIndex < NumCores; CoreIndex++) {
>      EFI_ACPI_6_0_GIC_STRUCTURE *GiccPtr;
>  
>      CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
> @@ -758,12 +758,6 @@ InitializeSbsaQemuAcpiDxe (
>  {
>    EFI_STATUS                     Status;
>    EFI_ACPI_TABLE_PROTOCOL        *AcpiTable;
> -  UINT32                         NumCores;
> -
> -  // Parse the device tree and get the number of CPUs
> -  NumCores = FdtHelperCountCpus ();
> -  Status = PcdSet32S (PcdCoreCount, NumCores);
> -  ASSERT_RETURN_ERROR (Status);
>  
>    // Check if ACPI Table Protocol has been installed
>    Status = gBS->LocateProtocol (
> 
> -- 
> 2.43.0
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114096): https://edk2.groups.io/g/devel/message/114096
Mute This Topic: https://groups.io/mt/103758016/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 4/4] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib

[Leif Lindholm]

On Tue, Jan 16, 2024 at 08:48:35 +0100, Marcin Juszkiewicz wrote:
> There is no need for EDK2 to know that there is DeviceTree around.
> All hardware information is read using functions from
> SbsaQemuHardwareInfoLib library.
> 
> Library fallbacks to parsing DT if needed (used with too old TF-A).
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>


> ---
>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc                 |   1 -
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf     |   4 +-
>  .../SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf  |  33 -------
>  .../SbsaQemuHardwareInfoLib.inf                     |   2 +
>  .../Qemu/SbsaQemu/Include/Library/FdtHelperLib.h    |  36 -------
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c       |   4 +-
>  .../SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c    |  98 ------------------
>  .../SbsaQemuHardwareInfoLib.c                       | 104 ++++++++++++++++++++
>  8 files changed, 110 insertions(+), 172 deletions(-)
> 
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> index 07cb3490f4cf..bde61651da2e 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> @@ -126,7 +126,6 @@ [LibraryClasses.common]
>    # ARM PL011 UART Driver
>    PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>  
> -  FdtHelperLib|Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
>    OemMiscLib|Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
>    SbsaQemuHardwareInfoLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
>  
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index 291743b19115..9bf0a13de5d1 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  This driver modifies ACPI tables for the Qemu SBSA platform
>  #
> -#  Copyright (c) 2020, Linaro Ltd. All rights reserved.
> +#  Copyright (c) Linaro Ltd. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -35,9 +35,9 @@ [LibraryClasses]
>    BaseLib
>    DebugLib
>    DxeServicesLib
> -  FdtHelperLib
>    PcdLib
>    PrintLib
> +  SbsaQemuHardwareInfoLib
>    UefiDriverEntryPoint
>    UefiLib
>    UefiRuntimeServicesTableLib
> diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> deleted file mode 100644
> index 9c059f3e5851..000000000000
> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -#/** @file
> -#
> -#  Component description file for FdtHelperLib module
> -#
> -#  Copyright (c) 2021, NUVIA Inc. All rights reserved.
> -#
> -#  SPDX-License-Identifier: BSD-2-Clause-Patent
> -#
> -#**/
> -
> -[Defines]
> -  INF_VERSION                    = 1.29
> -  BASE_NAME                      = FdtHelperLib
> -  FILE_GUID                      = 34e4396f-c2fc-4f9e-ad58-0f98e99e3875
> -  MODULE_TYPE                    = BASE
> -  VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = FdtHelperLib
> -
> -[Sources.common]
> -  FdtHelperLib.c
> -
> -[Packages]
> -  EmbeddedPkg/EmbeddedPkg.dec
> -  MdePkg/MdePkg.dec
> -  Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> -
> -[LibraryClasses]
> -  DebugLib
> -  FdtLib
> -  PcdLib
> -
> -[FixedPcd]
> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
> index 8c2def1878e6..5358dd339eb3 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
> @@ -27,6 +27,8 @@ [LibraryClasses]
>    ArmSmcLib
>    BaseMemoryLib
>    DebugLib
> +  FdtLib
>  
>   [Pcd]
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> deleted file mode 100644
> index ea9159857215..000000000000
> --- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> +++ /dev/null
> @@ -1,36 +0,0 @@
> -/** @file
> -*  FdtHelperLib.h
> -*
> -*  Copyright (c) 2021, NUVIA Inc. All rights reserved.
> -*
> -*  SPDX-License-Identifier: BSD-2-Clause-Patent
> -*
> -**/
> -
> -#ifndef FDT_HELPER_LIB_
> -#define FDT_HELPER_LIB_
> -
> -/**
> -  Get MPIDR for a given cpu from device tree passed by Qemu.
> -
> -  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
> -
> -  @retval                MPIDR value of CPU at index <CpuId>
> -**/
> -UINT64
> -FdtHelperGetMpidr (
> -  IN UINTN   CpuId
> -  );
> -
> -/** Walks through the Device Tree created by Qemu and counts the number
> -    of CPUs present in it.
> -
> -    @return The number of CPUs present.
> -**/
> -EFIAPI
> -UINT32
> -FdtHelperCountCpus (
> -  VOID
> -  );
> -
> -#endif /* FDT_HELPER_LIB_ */
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 7ef314ae9f67..c446581b746e 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -15,10 +15,10 @@
>  #include <Library/ArmLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
> -#include <Library/FdtHelperLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/PrintLib.h>
> +#include <Library/SbsaQemuHardwareInfoLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <Library/UefiLib.h>
> @@ -297,7 +297,7 @@ AddMadtTable (
>      CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
>      GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
>      GiccPtr->AcpiProcessorUid = CoreIndex;
> -    GiccPtr->MPIDR = FdtHelperGetMpidr (CoreIndex);
> +    GiccPtr->MPIDR = SbsaQemuGetMpidr (CoreIndex);
>      New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
>    }
>  
> diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> deleted file mode 100644
> index 7fdfb055db76..000000000000
> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> +++ /dev/null
> @@ -1,98 +0,0 @@
> -/** @file
> -*  FdtHelperLib.c
> -*
> -*  Copyright (c) 2021, NUVIA Inc. All rights reserved.
> -*  Copyright (c) 2020, Linaro Ltd. All rights reserved.
> -*
> -*  SPDX-License-Identifier: BSD-2-Clause-Patent
> -*
> -**/
> -
> -#include <Uefi.h>
> -#include <Library/DebugLib.h>
> -#include <Library/FdtHelperLib.h>
> -#include <Library/PcdLib.h>
> -#include <libfdt.h>
> -
> -STATIC INT32 mFdtFirstCpuOffset;
> -STATIC INT32 mFdtCpuNodeSize;
> -
> -/**
> -  Get MPIDR for a given cpu from device tree passed by Qemu.
> -
> -  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
> -
> -  @retval                MPIDR value of CPU at index <CpuId>
> -**/
> -UINT64
> -FdtHelperGetMpidr (
> -  IN UINTN   CpuId
> -  )
> -{
> -  VOID           *DeviceTreeBase;
> -  CONST UINT64   *RegVal;
> -  INT32          Len;
> -
> -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> -  ASSERT (DeviceTreeBase != NULL);
> -
> -  RegVal = fdt_getprop (DeviceTreeBase,
> -             mFdtFirstCpuOffset + (CpuId * mFdtCpuNodeSize),
> -             "reg",
> -             &Len);
> -  if (!RegVal) {
> -    DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
> -    return 0;
> -  }
> -
> -  return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
> -}
> -
> -/** Walks through the Device Tree created by Qemu and counts the number
> -    of CPUs present in it.
> -
> -    @return The number of CPUs present.
> -**/
> -EFIAPI
> -UINT32
> -FdtHelperCountCpus (
> -  VOID
> -  )
> -{
> -  VOID   *DeviceTreeBase;
> -  INT32  Node;
> -  INT32  Prev;
> -  INT32  CpuNode;
> -  UINT32 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 ((DEBUG_ERROR, "Unable to locate /cpus in device tree\n"));
> -    return 0;
> -  }
> -
> -  CpuCount = 0;
> -
> -  // Walk through /cpus node and count the number of subnodes.
> -  // The count of these subnodes corresponds to the number of
> -  // CPUs created by Qemu.
> -  Prev = fdt_first_subnode (DeviceTreeBase, CpuNode);
> -  mFdtFirstCpuOffset = Prev;
> -  while (1) {
> -    CpuCount++;
> -    Node = fdt_next_subnode (DeviceTreeBase, Prev);
> -    if (Node < 0) {
> -      break;
> -    }
> -    mFdtCpuNodeSize = Node - Prev;
> -    Prev = Node;
> -  }
> -
> -  return CpuCount;
> -}
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> index 4df973fda75e..900493e02d7f 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> @@ -11,8 +11,112 @@
>  #include <Library/DebugLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/SbsaQemuHardwareInfoLib.h>
> +#include <libfdt.h>
>  #include <IndustryStandard/SbsaQemuSmc.h>
>  
> +/**
> +  Get MPIDR for a given cpu from device tree passed by Qemu.
> +
> +  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
> +
> +  @retval                MPIDR value of CPU at index <CpuId>
> +**/
> +UINT64
> +FdtHelperGetMpidr (
> +  IN UINTN   CpuId
> +  )
> +{
> +  VOID           *DeviceTreeBase;
> +  INT32          Node;
> +  INT32          Prev;
> +  UINT32         CpuCount;
> +  CONST UINT64   *RegVal;
> +
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> +  ASSERT (DeviceTreeBase != NULL);
> +
> +  // Make sure we have a valid device tree blob
> +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> +  Node = fdt_path_offset (DeviceTreeBase, "/cpus");
> +  if (Node <= 0) {
> +    DEBUG ((DEBUG_ERROR, "Unable to locate /cpus in device tree\n"));
> +    return 0;
> +  }
> +
> +  CpuCount = 0;
> +
> +  Prev = fdt_first_subnode (DeviceTreeBase, Node);
> +  while (1) {
> +
> +    if (CpuCount == CpuId) {
> +      RegVal = fdt_getprop (DeviceTreeBase, Prev, "reg", NULL);
> +      if (!RegVal) {
> +        DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
> +        return 0;
> +      }
> +      return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
> +    }
> +
> +    Node = fdt_next_subnode (DeviceTreeBase, Prev);
> +    if (Node < 0) {
> +      break;
> +    }
> +    Prev = Node;
> +    CpuCount++;
> +  }
> +
> +  return 0; /* We did not found MPIDR */
> +
> +}
> +
> +/** Walks through the Device Tree created by Qemu and counts the number
> +    of CPUs present in it.
> +
> +    @return The number of CPUs present.
> +**/
> +EFIAPI
> +UINT32
> +FdtHelperCountCpus (
> +  VOID
> +  )
> +{
> +  VOID   *DeviceTreeBase;
> +  INT32  Node;
> +  INT32  Prev;
> +  UINT32 CpuCount;
> +
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> +  ASSERT (DeviceTreeBase != NULL);
> +
> +  // Make sure we have a valid device tree blob
> +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> +  Node = fdt_path_offset (DeviceTreeBase, "/cpus");
> +  if (Node <= 0) {
> +    DEBUG ((DEBUG_ERROR, "Unable to locate /cpus in device tree\n"));
> +    return 0;
> +  }
> +
> +  CpuCount = 0;
> +
> +  // Walk through /cpus node and count the number of subnodes.
> +  // The count of these subnodes corresponds to the number of
> +  // CPUs created by Qemu.
> +  Prev = fdt_first_subnode (DeviceTreeBase, Node);
> +  while (1) {
> +    CpuCount++;
> +    Node = fdt_next_subnode (DeviceTreeBase, Prev);
> +    if (Node < 0) {
> +      break;
> +    }
> +    Prev = Node;
> +  }
> +
> +  return CpuCount;
> +}
> +
> +
>  /**
>    Get CPU count from information passed by Qemu.
>  
> 
> -- 
> 2.43.0
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114097): https://edk2.groups.io/g/devel/message/114097
Mute This Topic: https://groups.io/mt/103758017/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib

[Marcin Juszkiewicz]

W dniu 19.01.2024 o 20:18, Leif Lindholm pisze:
> On Tue, Jan 16, 2024 at 08:48:32 +0100, Marcin Juszkiewicz wrote:
>> This library provides functions to check for hardware information.
>> For now it covers CPU ones:
>>
>> - amount of cpu cores
>> - MPIDR value for cpu core
>> - NUMA node id for cpu core
>>
>> Values are read from TF-A using platform specific SMC calls.
>>
>> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
>> ---
>>   Platform/Qemu/SbsaQemu/SbsaQemu.dsc                 |   3 +-
>>   .../SbsaQemuHardwareInfoLib.inf                     |  32 +++++++
>>   .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h |   2 +
>>   .../Include/Library/SbsaQemuHardwareInfoLib.h       |  45 +++++++++
>>   .../SbsaQemuHardwareInfoLib.c                       | 100 ++++++++++++++++++++
>>   5 files changed, 181 insertions(+), 1 deletion(-)
>>


>> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
>> new file mode 100644
>> index 000000000000..4df973fda75e
>> --- /dev/null
>> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
>> @@ -0,0 +1,100 @@
>> +/** @file
>> +*
>> +*  Copyright (c) 2021, NUVIA Inc. All rights reserved.
>> +*  Copyright (c) Linaro Ltd. All rights reserved.
>> +*
>> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +*
>> +**/
>> +
>> +#include <Library/ArmSmcLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/SbsaQemuHardwareInfoLib.h>
>> +#include <IndustryStandard/SbsaQemuSmc.h>
>> +
>> +/**
>> +  Get CPU count from information passed by Qemu.
>> +
>> +**/
>> +VOID
>> +SbsaQemuGetCpuCount (
>> +  VOID
>> +  )
>> +{
>> +  UINTN          Arg0;
>> +  UINTN          SmcResult;
>> +  RETURN_STATUS  Result;
>> +
>> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_COUNT, &Arg0, NULL, NULL);
>> +  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
> 
> So, this may sound a little bit bonkers, but SMCCC doesn't define
> return codes for SiP functions. And the SMC_ARCH_CALL_SUCCESS macro
> denotes an Arm Architectural function.
> Could you #define an SMC_SIP_CALL_SUCCESS *as* SMC_ARCH_CALL_SUCCESS
> in some platform-specific header and then use that?

Added it in Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h 
and used.


>> +    DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n"));
> 
> We don't *know* the TF-A is too old. Rather print which call failed
> (and that we need to attempt to get the cpu count from DT).

Changed:

     DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_COUNT call failed. We have to 
get cpu info from DT.\n"));


>> +    Arg0 = FdtHelperCountCpus();
> 
> We should probably assert this as != 0? Or is that done in the function?

Added code to stop going if we do not have cpu information. In such case 
we will not be back here with wrong info.

> 
>> +  }
>> +
>> +  Result = PcdSet32S (PcdCoreCount, Arg0);
>> +  ASSERT_RETURN_ERROR (Result);
>> +
>> +  Arg0 = PcdGet32 (PcdCoreCount);
> 
> Seems redundant to re-set it to the value we know it already holds.

Done

>> +
>> +  DEBUG ((DEBUG_INFO, "We have %d cpus.\n", Arg0));
>> +}
>> +
>> +/**
>> +  Get MPIDR for a given cpu from device tree passed by Qemu.
>> +
>> +  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
>> +
>> +  @retval                MPIDR value of CPU at index <CpuId>
>> +**/
>> +UINT64
>> +SbsaQemuGetMpidr (
>> +  IN UINTN   CpuId
>> +  )
>> +{
>> +  UINTN SmcResult;
>> +  UINTN Arg0;
>> +  UINTN Arg1;
>> +
>> +  Arg0 = CpuId;
>> +
>> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL);
>> +  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
>> +    DEBUG ((DEBUG_INFO, "Too old TF-A. We have to get cpu info from DT.\n"));
> 
> We don't *know* the TF-A is too old. Rather print which call failed
> (and that we need to attempt to get the cpu date from DT).

done

>> +UINT64
>> +SbsaQemuGetCpuNumaNode (
>> +  IN UINTN   CpuId
>> +  )
>> +{
>> +  UINTN SmcResult;
>> +  UINTN Arg0;
>> +  UINTN Arg1;
>> +
>> +  Arg0 = CpuId;
>> +
>> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL);
> 
> It does seem a bit wasteful we're making the same call twice per core,
> discarding one of the results.
> Could we have an init function that allocates an array and
> prepopulates it, with the Get-functions just returning values from the array?

good idea, will look into

>> +  if (SmcResult != SMC_ARCH_CALL_SUCCESS) {
>> +    /* No fallback to DeviceTree as we did not had that info earlier. */
> 
> We don't *know* the TF-A is too old. Rather print which call failed
> (and that we need to attempt to get the cpu count from DT).

done



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114287): https://edk2.groups.io/g/devel/message/114287
Mute This Topic: https://groups.io/mt/103758014/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 3/4] Platform/SbsaQemu: use PcdCoreCount directly

[Marcin Juszkiewicz]

W dniu 19.01.2024 o 20:20, Leif Lindholm pisze:
> On Tue, Jan 16, 2024 at 08:48:34 +0100, Marcin Juszkiewicz wrote:
>> During platform initialization we read amount of cpu cores and set
>> PcdCoreCount so there is no need to call FdtHandler.
>>
>> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
>> ---
>>   Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf             |  6 ++----
>>   Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c               | 10 ++++------
>>   .../Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c  | 12 +++---------
>>   3 files changed, 9 insertions(+), 19 deletions(-)

>> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> index 9fb17151d7b8..7ef314ae9f67 100644
>> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> @@ -255,7 +255,7 @@ AddMadtTable (
>>    // 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
>> +  // Get CoreCount which was determined earlier from TF-A
> 
> Where we got the information from no longer matters, since we've
> abstracted that away.

dropped comment



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114288): https://edk2.groups.io/g/devel/message/114288
Mute This Topic: https://groups.io/mt/103758016/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib

[Marcin Juszkiewicz]

W dniu 19.01.2024 o 20:18, Leif Lindholm pisze:
>> +UINT64
>> +SbsaQemuGetCpuNumaNode (
>> +  IN UINTN   CpuId
>> +  )
>> +{
>> +  UINTN SmcResult;
>> +  UINTN Arg0;
>> +  UINTN Arg1;
>> +
>> +  Arg0 = CpuId;
>> +
>> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_CPU_NODE, &Arg0, &Arg1, NULL);

> It does seem a bit wasteful we're making the same call twice per core,
> discarding one of the results.
> Could we have an init function that allocates an array and
> prepopulates it, with the Get-functions just returning values from the array?

We have discussed it over IRC and decided to merge working version first.

EDK2 statically links libraries into modules so "static struct" solution 
I tried to use does not work. Turning SbsaHardwareInfoLib into protocol 
would probably solve the problem but that's out of my knowledge of EDK2 
internals.

Also do not like idea of memory buffer with some DynamicPcd pointing to it.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115350): https://edk2.groups.io/g/devel/message/115350
Mute This Topic: https://groups.io/mt/103758014/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-