public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu
@ 2024-03-06 11:41 Marcin Juszkiewicz
  2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib Marcin Juszkiewicz
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:41 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, 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.

Hardware information (CPU, Memory) is now in SbsaQemuHardwareInfoLib
together with new code for handling SMC stuff. If TF-A answer to SMC
call would be not success then we go back to parsing DeviceTree data
directly. There is no DT parsing elsewhere.

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

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
Changes in v6 (Marcin Juszkiewicz):
- patch 5 now shutdowns system in case of no CPU information

Changes in v5 (Xiong Yining):
- added missing patch
- Link to v5: https://openfw.io/edk2-devel/20240131132400.3022662-1-xiongyining1480@phytium.com.cn/

Changes in v4 (Xiong Yining):
- patch 6 add the support for getting the hardware information of memory via SMC calls.
- patch 7 add the callback of DeviceTree when SMC calls defined on patch 6 failled.
- replace FdtHelperGetMpidr() with SbsaQemuGetMpidr() on patch 4 to compile successfully.
- Link to v4: https://openfw.io/edk2-devel/20240131100027.2538549-1-xiongyining1480@phytium.com.cn/

Changes in v3:
- added SMC_SIP_CALL_SUCCESS
- on SMC call fail tell that SMC call failed instead of blaming TF-A
- hang when there is no cpu information (TODO: shutdown instead)
- Link to v3: https://openfw.io/edk2-devel/20240124-no-dt-for-cpu-v3-0-5375fcf09037@linaro.org/

---
Marcin Juszkiewicz (5):
      Platform/SbsaQemu: add SbsaQemuHardwareInfoLib
      Platform/SbsaQemu: read amount of cpus during init
      Platform/SbsaQemu: use PcdCoreCount directly
      Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib
      Platform/SbsaQemu: hang if there is no cpu information

Xiong Yining (2):
      Platform/SbsaQemu: get the information of memory via SMC calls
      Platform/SbsaQemu: add DeviceTree fallbacks to parse memory information

 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                     |  36 ++
 .../SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf    |   2 +-
 .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h |  17 +-
 .../Qemu/SbsaQemu/Include/Library/FdtHelperLib.h    |  36 --
 .../Include/Library/SbsaQemuHardwareInfoLib.h       |  73 ++++
 Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c      |  10 +-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c       |  15 +-
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c       |   9 +-
 .../SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c    |  98 ------
 .../SbsaQemuHardwareInfoLib.c                       | 352 ++++++++++++++++++++
 .../Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c |  54 +--
 16 files changed, 511 insertions(+), 242 deletions(-)
---
base-commit: fe41713668d42b20a2370dab27de3269e877e454
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 (#116425): https://edk2.groups.io/g/devel/message/116425
Mute This Topic: https://groups.io/mt/104763761/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib
  2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
@ 2024-03-06 11:41 ` Marcin Juszkiewicz
  2024-03-14 15:09   ` Ard Biesheuvel
  2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init Marcin Juszkiewicz
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:41 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, 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  | 15 ++-
 .../Include/Library/SbsaQemuHardwareInfoLib.h        | 45 +++++++++
 .../SbsaQemuHardwareInfoLib.c                        | 98 ++++++++++++++++++++
 5 files changed, 189 insertions(+), 4 deletions(-)

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..d9faee7fa5b2 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
@@ -11,8 +11,17 @@
 
 #include <IndustryStandard/ArmStdSmc.h>
 
-#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_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)
+
+/*
+ *  SMCC does not define return codes for SiP functions.
+ *  We use Architecture ones then.
+ */
+
+#define SMC_SIP_CALL_SUCCESS  SMC_ARCH_CALL_SUCCESS
 
 #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..ca52c6b27093
--- /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..134fe73a5284
--- /dev/null
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -0,0 +1,98 @@
+/** @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_SIP_CALL_SUCCESS) {
+    DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_COUNT call failed. We have to get cpu info from DT.\n"));
+    Arg0 = FdtHelperCountCpus ();
+  }
+
+  Result = PcdSet32S (PcdCoreCount, Arg0);
+  ASSERT_RETURN_ERROR (Result);
+
+  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_SIP_CALL_SUCCESS) {
+    DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_NODE call failed. 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_SIP_CALL_SUCCESS) {
+    /* No fallback to DeviceTree as we did not had that info earlier. */
+    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_CPU_NODE call failed. 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.44.0



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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init
  2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
  2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib Marcin Juszkiewicz
@ 2024-03-06 11:41 ` Marcin Juszkiewicz
  2024-03-14 15:13   ` Ard Biesheuvel
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 3/7] Platform/SbsaQemu: use PcdCoreCount directly Marcin Juszkiewicz
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:41 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, 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..14e1ec7eab29 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.44.0



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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v6 3/7] Platform/SbsaQemu: use PcdCoreCount directly
  2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
  2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib Marcin Juszkiewicz
  2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init Marcin Juszkiewicz
@ 2024-03-06 11:42 ` Marcin Juszkiewicz
  2024-03-14 15:15   ` Ard Biesheuvel
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 4/7] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib Marcin Juszkiewicz
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:42 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, 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   | 11 ++---------
 3 files changed, 8 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..59536ea9575e 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,6 @@ 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
   NumCores = PcdGet32 (PcdCoreCount);
 
   // Calculate the new table size based on the number of cores
@@ -291,7 +290,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 +757,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.44.0



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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v6 4/7] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib
  2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
                   ` (2 preceding siblings ...)
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 3/7] Platform/SbsaQemu: use PcdCoreCount directly Marcin Juszkiewicz
@ 2024-03-06 11:42 ` Marcin Juszkiewicz
  2024-03-14 15:16   ` Ard Biesheuvel
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 5/7] Platform/SbsaQemu: hang if there is no cpu information Marcin Juszkiewicz
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:42 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, 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.

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 59536ea9575e..03f7a34977a0 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>
@@ -296,7 +296,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 134fe73a5284..6315cce3fb7f 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 DeviceTree\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 DeviceTree\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.44.0



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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v6 5/7] Platform/SbsaQemu: hang if there is no cpu information
  2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
                   ` (3 preceding siblings ...)
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 4/7] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib Marcin Juszkiewicz
@ 2024-03-06 11:42 ` Marcin Juszkiewicz
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 6/7] Platform/SbsaQemu: get the information of memory via SMC calls Marcin Juszkiewicz
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 7/7] Platform/SbsaQemu: add DeviceTree fallbacks to parse memory information Marcin Juszkiewicz
  6 siblings, 0 replies; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:42 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, Marcin Juszkiewicz

In case we do not have cpu information (SMC call fails,
our minimal DT lacks info) we shutdown system.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 .../Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf        | 2 ++
 .../SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
index 5358dd339eb3..acf91225b4c7 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.inf
@@ -21,6 +21,7 @@ [Packages]
   ArmPkg/ArmPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   Silicon/Qemu/SbsaQemu/SbsaQemu.dec
 
 [LibraryClasses]
@@ -28,6 +29,7 @@ [LibraryClasses]
   BaseMemoryLib
   DebugLib
   FdtLib
+  ResetSystemLib
 
  [Pcd]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
index 6315cce3fb7f..e1f1a9588b19 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -10,6 +10,7 @@
 #include <Library/ArmSmcLib.h>
 #include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
+#include <Library/ResetSystemLib.h>
 #include <Library/SbsaQemuHardwareInfoLib.h>
 #include <libfdt.h>
 #include <IndustryStandard/SbsaQemuSmc.h>
@@ -95,7 +96,7 @@ FdtHelperCountCpus (
   Node = fdt_path_offset (DeviceTreeBase, "/cpus");
   if (Node <= 0) {
     DEBUG ((DEBUG_ERROR, "Unable to locate /cpus in DeviceTree\n"));
-    return 0;
+    ResetShutdown();
   }
 
   CpuCount = 0;

-- 
2.44.0



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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v6 6/7] Platform/SbsaQemu: get the information of memory via SMC calls
  2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
                   ` (4 preceding siblings ...)
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 5/7] Platform/SbsaQemu: hang if there is no cpu information Marcin Juszkiewicz
@ 2024-03-06 11:42 ` Marcin Juszkiewicz
  2024-03-14 15:18   ` Ard Biesheuvel
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 7/7] Platform/SbsaQemu: add DeviceTree fallbacks to parse memory information Marcin Juszkiewicz
  6 siblings, 1 reply; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:42 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, Marcin Juszkiewicz

From: Xiong Yining <xiongyining1480@phytium.com.cn>

Provide functions to check for memory information:

- amount of memory nodes
- memory address
- NUMA node id for memory

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

Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
---
 .../SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf     |  2 +-
 .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h  |  2 +
 .../Include/Library/SbsaQemuHardwareInfoLib.h        | 28 ++++++++++
 .../SbsaQemuHardwareInfoLib.c                        | 47 +++++++++++++++++
 .../Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c  | 54 +++++---------------
 5 files changed, 91 insertions(+), 42 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
index c067a80cc715..fb856efe4c27 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
@@ -32,9 +32,9 @@ [LibraryClasses]
   ArmLib
   BaseMemoryLib
   DebugLib
-  FdtLib
   MemoryAllocationLib
   PcdLib
+  SbsaQemuHardwareInfoLib
 
 [Pcd]
   gArmTokenSpaceGuid.PcdSystemMemoryBase
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
index d9faee7fa5b2..e7bf54978d4e 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
@@ -16,6 +16,8 @@
 #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)
+#define SIP_SVC_GET_MEMORY_NODE_COUNT SMC_SIP_FUNCTION_ID(300)
+#define SIP_SVC_GET_MEMORY_NODE SMC_SIP_FUNCTION_ID(301)
 
 /*
  *  SMCC does not define return codes for SiP functions.
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
index ca52c6b27093..0b71a3f7e6eb 100644
--- a/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
@@ -9,6 +9,12 @@
 #ifndef SBSA_QEMU_HARDWARE_INFO_
 #define SBSA_QEMU_HARDWARE_INFO_
 
+typedef struct{
+  UINT32  NodeId;
+  UINT64  AddressBase;
+  UINT64  AddressSize;
+} MemoryInfo;
+
 /**
   Get CPU count from information passed by Qemu.
 
@@ -42,4 +48,26 @@ SbsaQemuGetCpuNumaNode (
   IN UINTN  CpuId
   );
 
+/**
+  Get the number of memory node from device tree passed by Qemu.
+
+  @retval                   the number of memory nodes.
+**/
+UINT32
+SbsaQemuGetMemNodeCount (
+  VOID
+  );
+
+/**
+  Get memory infomation(node-id, addressbase, addresssize) for a given memory node from device tree passed by Qemu.
+
+  @param [in]   MemoryId    Index of memory to retrieve memory information.
+
+  @retval                   memory infomation for given memory node.
+**/
+MemoryInfo
+SbsaQemuGetMemInfo (
+  IN UINTN   MemoryId
+  );
+
 #endif /* SBSA_QEMU_HARDWARE_INFO_ */
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
index e1f1a9588b19..88b50e46c072 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -201,3 +201,50 @@ SbsaQemuGetCpuNumaNode (
 
   return Arg0;
 }
+
+UINT32
+SbsaQemuGetMemNodeCount (
+  VOID
+  )
+{
+  UINTN            SmcResult;
+  UINTN            Arg0;
+
+  SmcResult = ArmCallSmc0 (SIP_SVC_GET_MEMORY_NODE_COUNT, &Arg0, NULL, NULL);
+  if (SmcResult != SMC_SIP_CALL_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE_COUNT call failed.\n"));
+  }
+
+  DEBUG(( DEBUG_INFO, "The number of the memory nodes is %ld\n", Arg0));
+  return (UINT32)Arg0;
+}
+
+MemoryInfo
+SbsaQemuGetMemInfo (
+  IN UINTN   MemoryId
+  )
+{
+  UINTN           SmcResult;
+  UINTN           Arg0;
+  UINTN           Arg1;
+  UINTN           Arg2;
+  MemoryInfo      MemInfo;
+
+  Arg0 = MemoryId;
+
+  SmcResult = ArmCallSmc1 (SIP_SVC_GET_MEMORY_NODE, &Arg0, &Arg1, &Arg2);
+  if (SmcResult != SMC_SIP_CALL_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE call failed.\n"));
+  } else {
+    MemInfo.NodeId = Arg0;
+    MemInfo.AddressBase = Arg1;
+    MemInfo.AddressSize = Arg2;
+  }
+
+  DEBUG(( DEBUG_INFO, "NUMA node for System RAM:%d = 0x%lx - 0x%lx\n",
+      MemInfo.NodeId,
+      MemInfo.AddressBase,
+      MemInfo.AddressBase + MemInfo.AddressSize -1 ));
+
+  return MemInfo;
+}
diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
index 8c2eb0b6a028..5a418a461174 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
@@ -12,7 +12,7 @@
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
-#include <libfdt.h>
+#include <Library/SbsaQemuHardwareInfoLib.h>
 
 // Number of Virtual Memory Map Descriptors
 #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          4
@@ -23,53 +23,25 @@ SbsaQemuLibConstructor (
   VOID
   )
 {
-  VOID          *DeviceTreeBase;
-  INT32         Node, Prev;
   UINT64        NewBase, CurBase;
   UINT64        NewSize, CurSize;
-  CONST CHAR8   *Type;
-  INT32         Len;
-  CONST UINT64  *RegProp;
+  UINT32        NumMemNodes;
+  UINT32        Index;
+  MemoryInfo    MemInfo;
   RETURN_STATUS PcdStatus;
 
   NewBase = 0;
   NewSize = 0;
 
-  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
-  ASSERT (DeviceTreeBase != NULL);
-
-  // Make sure we have a valid device tree blob
-  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
-
-  // Look for the lowest memory node
-  for (Prev = 0;; Prev = Node) {
-    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
-    if (Node < 0) {
-      break;
-    }
-
-    // Check for memory node
-    Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
-    if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
-      // Get the 'reg' property of this node. For now, we will assume
-      // two 8 byte quantities for base and size, respectively.
-      RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
-      if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
-
-        CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
-        CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
-
-        DEBUG ((DEBUG_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
-          __FUNCTION__, CurBase, CurBase + CurSize - 1));
-
-        if (NewBase > CurBase || NewBase == 0) {
-          NewBase = CurBase;
-          NewSize = CurSize;
-        }
-      } else {
-        DEBUG ((DEBUG_ERROR, "%a: Failed to parse FDT memory node\n",
-          __FUNCTION__));
-      }
+  NumMemNodes = SbsaQemuGetMemNodeCount();
+  for(Index = 0; Index < NumMemNodes; Index++){
+    MemInfo = SbsaQemuGetMemInfo(Index);
+    CurBase = MemInfo.AddressBase;
+    CurSize = MemInfo.AddressSize;
+
+    if (NewBase > CurBase || NewBase == 0) {
+      NewBase = CurBase;
+      NewSize = CurSize;
     }
   }
 

-- 
2.44.0



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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v6 7/7] Platform/SbsaQemu: add DeviceTree fallbacks to parse memory information
  2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
                   ` (5 preceding siblings ...)
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 6/7] Platform/SbsaQemu: get the information of memory via SMC calls Marcin Juszkiewicz
@ 2024-03-06 11:42 ` Marcin Juszkiewicz
  6 siblings, 0 replies; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-06 11:42 UTC (permalink / raw)
  To: devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi, Marcin Juszkiewicz

From: Xiong Yining <xiongyining1480@phytium.com.cn>

Add the DeviceTree fallbacks to parsing the information about memory
if the related SMC calls Failed.

Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
---
 .../SbsaQemuHardwareInfoLib.c                       | 106 +++++++++++++++++++-
 1 file changed, 104 insertions(+), 2 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
index 88b50e46c072..d63cfbf7d5ef 100644
--- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
+++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -118,6 +118,106 @@ FdtHelperCountCpus (
   return CpuCount;
 }
 
+/**
+  Walks through the Device Tree created by Qemu and counts the number of Memory node present in it.
+
+  @retval                   the number of memory nodes.
+**/
+UINT32
+FdtHelperMemNodeCount (
+  VOID
+  )
+{
+  VOID          *DeviceTreeBase;
+  CONST CHAR8   *Type;
+  UINT32        MemNodeCount;
+  INT32         Node;
+  INT32         Prev;
+  INT32         Len;
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+
+  // Make sure we have a valid device tree blob
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  MemNodeCount = 0;
+  for (Prev = 0;; Prev = Node) {
+    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
+    if (Node < 0){
+      break;
+    }
+
+    Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
+    if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
+      MemNodeCount++;
+    }
+  }
+
+  return MemNodeCount;
+}
+
+/** Walks through the Device Tree created by Qemu and counts the adress、size and node-id of the Memory node present in it.
+
+  @param [in]   MemoryId    Index of memory to retrieve memory information.
+
+  @retval                   memory infomation for given memory node.
+**/
+MemoryInfo
+FdtHelperGetMemInfo (
+  IN UINTN   MemoryId
+)
+{
+  VOID          *DeviceTreeBase;
+  CONST UINT64  *RegProp;
+  CONST UINT32  *NodeProp;
+  CONST CHAR8   *Type;
+  INT64         MemNodeCount;
+  INT32         Len;
+  INT32         Node;
+  INT32         Prev;
+  MemoryInfo    MemInfo = {0};
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+
+  // Make sure we have a valid device tree blob
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  MemNodeCount = 0;
+
+  for (Prev = 0;; Prev = Node){
+    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
+    if (Node < 0){
+      break;
+    }
+
+    Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
+    if (Type && AsciiStrnCmp (Type, "memory", Len) == 0){
+      if (MemoryId == MemNodeCount){
+        RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
+        if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
+            MemInfo.AddressBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
+            MemInfo.AddressSize= fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
+        } else {
+          DEBUG ((DEBUG_ERROR, "Failed to find the address and size of the memory"));
+        }
+
+        NodeProp= fdt_getprop (DeviceTreeBase, Node, "numa-node-id", &Len);
+        if (NodeProp){
+          MemInfo.NodeId= fdt32_to_cpu (ReadUnaligned32 (NodeProp));
+        } else {
+          DEBUG ((DEBUG_ERROR, "Failed to find the node-id of the memory"));
+        }
+      }
+
+      MemNodeCount ++;
+    }
+  }
+
+  return MemInfo;
+}
+
 /**
   Get CPU count from information passed by Qemu.
 
@@ -212,7 +312,8 @@ SbsaQemuGetMemNodeCount (
 
   SmcResult = ArmCallSmc0 (SIP_SVC_GET_MEMORY_NODE_COUNT, &Arg0, NULL, NULL);
   if (SmcResult != SMC_SIP_CALL_SUCCESS) {
-    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE_COUNT call failed.\n"));
+    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE_COUNT call failed. We have to get the number of memory nodes from DT.\n"));
+    Arg0 = FdtHelperMemNodeCount();
   }
 
   DEBUG(( DEBUG_INFO, "The number of the memory nodes is %ld\n", Arg0));
@@ -234,7 +335,8 @@ SbsaQemuGetMemInfo (
 
   SmcResult = ArmCallSmc1 (SIP_SVC_GET_MEMORY_NODE, &Arg0, &Arg1, &Arg2);
   if (SmcResult != SMC_SIP_CALL_SUCCESS) {
-    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE call failed.\n"));
+    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE call failed. We have to get memory info from DT.\n"));
+    MemInfo = FdtHelperGetMemInfo(MemoryId);
   } else {
     MemInfo.NodeId = Arg0;
     MemInfo.AddressBase = Arg1;

-- 
2.44.0



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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib
  2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib Marcin Juszkiewicz
@ 2024-03-14 15:09   ` Ard Biesheuvel
  2024-03-14 15:14     ` Ard Biesheuvel
  0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2024-03-14 15:09 UTC (permalink / raw)
  To: devel, marcin.juszkiewicz
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi

On Wed, 6 Mar 2024 at 12:42, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> 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  | 15 ++-
>  .../Include/Library/SbsaQemuHardwareInfoLib.h        | 45 +++++++++
>  .../SbsaQemuHardwareInfoLib.c                        | 98 ++++++++++++++++++++
>  5 files changed, 189 insertions(+), 4 deletions(-)
>
> 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
> +

This is a new library class, so you should name it and define it in
the associated .DEC file (which does not exist yet)

You can look at Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dec
for inspiration (but please use 1.27 and not 0x00010005 for the
version)


> +[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..d9faee7fa5b2 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> @@ -11,8 +11,17 @@
>
>  #include <IndustryStandard/ArmStdSmc.h>
>
> -#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_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)
> +
> +/*
> + *  SMCC does not define return codes for SiP functions.
> + *  We use Architecture ones then.
> + */
> +
> +#define SMC_SIP_CALL_SUCCESS  SMC_ARCH_CALL_SUCCESS
>
>  #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..ca52c6b27093
> --- /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..134fe73a5284
> --- /dev/null
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> @@ -0,0 +1,98 @@
> +/** @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_SIP_CALL_SUCCESS) {
> +    DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_COUNT call failed. We have to get cpu info from DT.\n"));
> +    Arg0 = FdtHelperCountCpus ();

I don't think the fallbacks are needed here. SbsaQemu is tightly
coupled between secure and non-secure in terms of firmware, and this
is supposed to be a reference, so having legacy cruft in here does not
make sense.

> +  }
> +
> +  Result = PcdSet32S (PcdCoreCount, Arg0);
> +  ASSERT_RETURN_ERROR (Result);
> +
> +  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_SIP_CALL_SUCCESS) {
> +    DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_NODE call failed. We have to get cpu info from DT.\n"));
> +    Arg1 = FdtHelperGetMpidr (CpuId);
> +  }
> +
> +  DEBUG ((DEBUG_ERROR, "MPIDR for CPU:%d = %d\n", CpuId, Arg1));
> +

Don't use DEBUG_ERROR for something that is not an error.

> +  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_SIP_CALL_SUCCESS) {
> +    /* No fallback to DeviceTree as we did not had that info earlier. */
> +    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_CPU_NODE call failed. 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.44.0
>
>
>
> 
>
>


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init
  2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init Marcin Juszkiewicz
@ 2024-03-14 15:13   ` Ard Biesheuvel
  2024-03-15 11:49     ` Marcin Juszkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2024-03-14 15:13 UTC (permalink / raw)
  To: Marcin Juszkiewicz
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

On Wed, 6 Mar 2024 at 12:42, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> 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]

How is it guaranteed that other components will only see the correct
core count? DXE dispatch is ordered using a dependency graph, so all
users of this PCD should never execute before this driver.

This is why PCDs suck for dynamic information, to be honest. Much
better to use a protocol (DEPEXes declare dependencies on protocols,
so a driver will never run before the protocols it depends on have
been made available)

Given that this is intended as reference code, I think it is very
important to get this right.


> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> index 4ebbe7c93a19..14e1ec7eab29 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.44.0
>


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib
  2024-03-14 15:09   ` Ard Biesheuvel
@ 2024-03-14 15:14     ` Ard Biesheuvel
  2024-03-15 11:34       ` Marcin Juszkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2024-03-14 15:14 UTC (permalink / raw)
  To: devel, marcin.juszkiewicz
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi

On Thu, 14 Mar 2024 at 16:09, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 6 Mar 2024 at 12:42, Marcin Juszkiewicz
> <marcin.juszkiewicz@linaro.org> 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  | 15 ++-
> >  .../Include/Library/SbsaQemuHardwareInfoLib.h        | 45 +++++++++
> >  .../SbsaQemuHardwareInfoLib.c                        | 98 ++++++++++++++++++++
> >  5 files changed, 189 insertions(+), 4 deletions(-)
> >
> > 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
> > +
>
> This is a new library class, so you should name it and define it in
> the associated .DEC file (which does not exist yet)
>
> You can look at Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dec
> for inspiration (but please use 1.27 and not 0x00010005 for the
> version)
>

Just noticed there is already a .DEC in Silicon/Qemu/SbsaQemu/ so
better to put it there.

>
> > +[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..d9faee7fa5b2 100644
> > --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> > +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> > @@ -11,8 +11,17 @@
> >
> >  #include <IndustryStandard/ArmStdSmc.h>
> >
> > -#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_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)
> > +
> > +/*
> > + *  SMCC does not define return codes for SiP functions.
> > + *  We use Architecture ones then.
> > + */
> > +
> > +#define SMC_SIP_CALL_SUCCESS  SMC_ARCH_CALL_SUCCESS
> >
> >  #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..ca52c6b27093
> > --- /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..134fe73a5284
> > --- /dev/null
> > +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> > @@ -0,0 +1,98 @@
> > +/** @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_SIP_CALL_SUCCESS) {
> > +    DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_COUNT call failed. We have to get cpu info from DT.\n"));
> > +    Arg0 = FdtHelperCountCpus ();
>
> I don't think the fallbacks are needed here. SbsaQemu is tightly
> coupled between secure and non-secure in terms of firmware, and this
> is supposed to be a reference, so having legacy cruft in here does not
> make sense.
>
> > +  }
> > +
> > +  Result = PcdSet32S (PcdCoreCount, Arg0);
> > +  ASSERT_RETURN_ERROR (Result);
> > +
> > +  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_SIP_CALL_SUCCESS) {
> > +    DEBUG ((DEBUG_INFO, "SIP_SVC_GET_CPU_NODE call failed. We have to get cpu info from DT.\n"));
> > +    Arg1 = FdtHelperGetMpidr (CpuId);
> > +  }
> > +
> > +  DEBUG ((DEBUG_ERROR, "MPIDR for CPU:%d = %d\n", CpuId, Arg1));
> > +
>
> Don't use DEBUG_ERROR for something that is not an error.
>
> > +  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_SIP_CALL_SUCCESS) {
> > +    /* No fallback to DeviceTree as we did not had that info earlier. */
> > +    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_CPU_NODE call failed. 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.44.0
> >
> >
> >
> > 
> >
> >


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 3/7] Platform/SbsaQemu: use PcdCoreCount directly
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 3/7] Platform/SbsaQemu: use PcdCoreCount directly Marcin Juszkiewicz
@ 2024-03-14 15:15   ` Ard Biesheuvel
  0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2024-03-14 15:15 UTC (permalink / raw)
  To: Marcin Juszkiewicz
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

On Wed, 6 Mar 2024 at 12:42, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> During platform initialization we read amount of cpu cores and set
> PcdCoreCount so there is no need to call FdtHandler.
>

How can you be sure that that code executes first?

> 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   | 11 ++---------
>  3 files changed, 8 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..59536ea9575e 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,6 @@ 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
>    NumCores = PcdGet32 (PcdCoreCount);
>
>    // Calculate the new table size based on the number of cores
> @@ -291,7 +290,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 +757,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.44.0
>


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 4/7] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 4/7] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib Marcin Juszkiewicz
@ 2024-03-14 15:16   ` Ard Biesheuvel
  0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2024-03-14 15:16 UTC (permalink / raw)
  To: Marcin Juszkiewicz
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

On Wed, 6 Mar 2024 at 12:42, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> 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.
>

I'd prefer to have only a single method of obtaining this information.

> 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 59536ea9575e..03f7a34977a0 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>
> @@ -296,7 +296,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 134fe73a5284..6315cce3fb7f 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 DeviceTree\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 DeviceTree\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.44.0
>


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 6/7] Platform/SbsaQemu: get the information of memory via SMC calls
  2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 6/7] Platform/SbsaQemu: get the information of memory via SMC calls Marcin Juszkiewicz
@ 2024-03-14 15:18   ` Ard Biesheuvel
  0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2024-03-14 15:18 UTC (permalink / raw)
  To: Marcin Juszkiewicz
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

On Wed, 6 Mar 2024 at 12:42, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> From: Xiong Yining <xiongyining1480@phytium.com.cn>
>
> Provide functions to check for memory information:
>
> - amount of memory nodes
> - memory address
> - NUMA node id for memory
>
> Values are read from TF-A using platform specific SMC calls.
>
> Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
> ---
>  .../SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf     |  2 +-
>  .../SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h  |  2 +
>  .../Include/Library/SbsaQemuHardwareInfoLib.h        | 28 ++++++++++
>  .../SbsaQemuHardwareInfoLib.c                        | 47 +++++++++++++++++
>  .../Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c  | 54 +++++---------------
>  5 files changed, 91 insertions(+), 42 deletions(-)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> index c067a80cc715..fb856efe4c27 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> @@ -32,9 +32,9 @@ [LibraryClasses]
>    ArmLib
>    BaseMemoryLib
>    DebugLib
> -  FdtLib
>    MemoryAllocationLib
>    PcdLib
> +  SbsaQemuHardwareInfoLib
>
>  [Pcd]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
> diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> index d9faee7fa5b2..e7bf54978d4e 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
> @@ -16,6 +16,8 @@
>  #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)
> +#define SIP_SVC_GET_MEMORY_NODE_COUNT SMC_SIP_FUNCTION_ID(300)
> +#define SIP_SVC_GET_MEMORY_NODE SMC_SIP_FUNCTION_ID(301)
>
>  /*
>   *  SMCC does not define return codes for SiP functions.
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
> index ca52c6b27093..0b71a3f7e6eb 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/SbsaQemuHardwareInfoLib.h
> @@ -9,6 +9,12 @@
>  #ifndef SBSA_QEMU_HARDWARE_INFO_
>  #define SBSA_QEMU_HARDWARE_INFO_
>
> +typedef struct{
> +  UINT32  NodeId;
> +  UINT64  AddressBase;
> +  UINT64  AddressSize;
> +} MemoryInfo;
> +
>  /**
>    Get CPU count from information passed by Qemu.
>
> @@ -42,4 +48,26 @@ SbsaQemuGetCpuNumaNode (
>    IN UINTN  CpuId
>    );
>
> +/**
> +  Get the number of memory node from device tree passed by Qemu.
> +
> +  @retval                   the number of memory nodes.
> +**/
> +UINT32
> +SbsaQemuGetMemNodeCount (
> +  VOID
> +  );
> +
> +/**
> +  Get memory infomation(node-id, addressbase, addresssize) for a given memory node from device tree passed by Qemu.
> +
> +  @param [in]   MemoryId    Index of memory to retrieve memory information.
> +
> +  @retval                   memory infomation for given memory node.
> +**/
> +MemoryInfo

Let's avoid aggregates as return types. Pass/return by reference
instead using an OUT pointer.


> +SbsaQemuGetMemInfo (
> +  IN UINTN   MemoryId
> +  );
> +
>  #endif /* SBSA_QEMU_HARDWARE_INFO_ */
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> index e1f1a9588b19..88b50e46c072 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
> @@ -201,3 +201,50 @@ SbsaQemuGetCpuNumaNode (
>
>    return Arg0;
>  }
> +
> +UINT32
> +SbsaQemuGetMemNodeCount (
> +  VOID
> +  )
> +{
> +  UINTN            SmcResult;
> +  UINTN            Arg0;
> +
> +  SmcResult = ArmCallSmc0 (SIP_SVC_GET_MEMORY_NODE_COUNT, &Arg0, NULL, NULL);
> +  if (SmcResult != SMC_SIP_CALL_SUCCESS) {
> +    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE_COUNT call failed.\n"));
> +  }
> +
> +  DEBUG(( DEBUG_INFO, "The number of the memory nodes is %ld\n", Arg0));
> +  return (UINT32)Arg0;
> +}
> +
> +MemoryInfo
> +SbsaQemuGetMemInfo (
> +  IN UINTN   MemoryId
> +  )
> +{
> +  UINTN           SmcResult;
> +  UINTN           Arg0;
> +  UINTN           Arg1;
> +  UINTN           Arg2;
> +  MemoryInfo      MemInfo;
> +
> +  Arg0 = MemoryId;
> +
> +  SmcResult = ArmCallSmc1 (SIP_SVC_GET_MEMORY_NODE, &Arg0, &Arg1, &Arg2);
> +  if (SmcResult != SMC_SIP_CALL_SUCCESS) {
> +    DEBUG ((DEBUG_ERROR, "SIP_SVC_GET_MEMORY_NODE call failed.\n"));
> +  } else {
> +    MemInfo.NodeId = Arg0;
> +    MemInfo.AddressBase = Arg1;
> +    MemInfo.AddressSize = Arg2;
> +  }
> +
> +  DEBUG(( DEBUG_INFO, "NUMA node for System RAM:%d = 0x%lx - 0x%lx\n",
> +      MemInfo.NodeId,
> +      MemInfo.AddressBase,
> +      MemInfo.AddressBase + MemInfo.AddressSize -1 ));
> +
> +  return MemInfo;
> +}
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
> index 8c2eb0b6a028..5a418a461174 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c
> @@ -12,7 +12,7 @@
>  #include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
> -#include <libfdt.h>
> +#include <Library/SbsaQemuHardwareInfoLib.h>
>
>  // Number of Virtual Memory Map Descriptors
>  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          4
> @@ -23,53 +23,25 @@ SbsaQemuLibConstructor (
>    VOID
>    )
>  {
> -  VOID          *DeviceTreeBase;
> -  INT32         Node, Prev;
>    UINT64        NewBase, CurBase;
>    UINT64        NewSize, CurSize;
> -  CONST CHAR8   *Type;
> -  INT32         Len;
> -  CONST UINT64  *RegProp;
> +  UINT32        NumMemNodes;
> +  UINT32        Index;
> +  MemoryInfo    MemInfo;
>    RETURN_STATUS PcdStatus;
>
>    NewBase = 0;
>    NewSize = 0;
>
> -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> -  ASSERT (DeviceTreeBase != NULL);
> -
> -  // Make sure we have a valid device tree blob
> -  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> -
> -  // Look for the lowest memory node
> -  for (Prev = 0;; Prev = Node) {
> -    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
> -    if (Node < 0) {
> -      break;
> -    }
> -
> -    // Check for memory node
> -    Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
> -    if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
> -      // Get the 'reg' property of this node. For now, we will assume
> -      // two 8 byte quantities for base and size, respectively.
> -      RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
> -      if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
> -
> -        CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
> -        CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
> -
> -        DEBUG ((DEBUG_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
> -          __FUNCTION__, CurBase, CurBase + CurSize - 1));
> -
> -        if (NewBase > CurBase || NewBase == 0) {
> -          NewBase = CurBase;
> -          NewSize = CurSize;
> -        }
> -      } else {
> -        DEBUG ((DEBUG_ERROR, "%a: Failed to parse FDT memory node\n",
> -          __FUNCTION__));
> -      }
> +  NumMemNodes = SbsaQemuGetMemNodeCount();
> +  for(Index = 0; Index < NumMemNodes; Index++){
> +    MemInfo = SbsaQemuGetMemInfo(Index);
> +    CurBase = MemInfo.AddressBase;
> +    CurSize = MemInfo.AddressSize;
> +
> +    if (NewBase > CurBase || NewBase == 0) {
> +      NewBase = CurBase;
> +      NewSize = CurSize;
>      }
>    }
>
>
> --
> 2.44.0
>


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib
  2024-03-14 15:14     ` Ard Biesheuvel
@ 2024-03-15 11:34       ` Marcin Juszkiewicz
  0 siblings, 0 replies; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-15 11:34 UTC (permalink / raw)
  To: Ard Biesheuvel, devel
  Cc: Leif Lindholm, Ard Biesheuvel, Graeme Gregory, Xiong Yining,
	Chen Baozi

W dniu 14.03.2024 o 16:14, Ard Biesheuvel pisze:
>>> +++ 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
>>> +
>> This is a new library class, so you should name it and define it in
>> the associated .DEC file (which does not exist yet)
>>
>> You can look at Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dec
>> for inspiration (but please use 1.27 and not 0x00010005 for the
>> version)
>>
> Just noticed there is already a .DEC in Silicon/Qemu/SbsaQemu/ so
> better to put it there.

I based on other libraries in Silicon/Qemu/SbsaQemu/Library/ directory.
One C source file, one INF and then a line in 
Platform/Qemu/SbsaQemu/SbsaQemu.dsc file to tell where it is.



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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init
  2024-03-14 15:13   ` Ard Biesheuvel
@ 2024-03-15 11:49     ` Marcin Juszkiewicz
  2024-03-19 10:25       ` Marcin Juszkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-15 11:49 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

W dniu 14.03.2024 o 16:13, Ard Biesheuvel pisze:
>> +++ 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]

> How is it guaranteed that other components will only see the correct
> core count? DXE dispatch is ordered using a dependency graph, so all
> users of this PCD should never execute before this driver.

SbsaQemuPlatformDxe is a DXE, right? So it is called on platform init.

At the end of initialization it calls SbsaQemuGetCpuCount() from 
SbsaQemuHardwareInfoLib to SET this PCD. It does not use it during 
platform init cause it does not require this information. But calls 
function to make sure that amount of cpus is known to whatever will be 
called later.

Sure, maybe SbsaQemuHardwareInfoLib should be something else (DXE, 
Protocol or other EDK2 magic thing) but it is set of functions to be 
called from other places of EDK2.

> This is why PCDs suck for dynamic information, to be honest. Much
> better to use a protocol (DEPEXes declare dependencies on protocols,
> so a driver will never run before the protocols it depends on have
> been made available)

I am still learning. Will look at other platforms.

> Given that this is intended as reference code, I think it is very
> important to get this right.

Fully agree.


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init
  2024-03-15 11:49     ` Marcin Juszkiewicz
@ 2024-03-19 10:25       ` Marcin Juszkiewicz
  2024-03-19 11:02         ` Ard Biesheuvel
  0 siblings, 1 reply; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-19 10:25 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

W dniu 15.03.2024 o 12:49, Marcin Juszkiewicz pisze:
> W dniu 14.03.2024 o 16:13, Ard Biesheuvel pisze:

>> How is it guaranteed that other components will only see the correct
>> core count? DXE dispatch is ordered using a dependency graph, so all
>> users of this PCD should never execute before this driver.
> 
> SbsaQemuPlatformDxe is a DXE, right? So it is called on platform init.
> 
> At the end of initialization it calls SbsaQemuGetCpuCount() from 
> SbsaQemuHardwareInfoLib to SET this PCD. It does not use it during 
> platform init cause it does not require this information. But calls 
> function to make sure that amount of cpus is known to whatever will be 
> called later.
> 
> Sure, maybe SbsaQemuHardwareInfoLib should be something else (DXE, 
> Protocol or other EDK2 magic thing) but it is set of functions to be 
> called from other places of EDK2.

EDK2 starts and one of the first DXE called is SbsaQemuPlatformDxe one:

InitializeSbsaQemuPlatformDxe: InitializeSbsaQemuPlatformDxe called

InitializeSbsaQemuPlatformDxe: Got platform AHCI 60100000 65536

INFO:    SMC call: (0xc2000001) (function id: 1)

INFO:    Platform version requested

Platform version: 0.3

INFO:    SMC call: (0xc2000064) (function id: 100)

GICD base: 0x40060000

GICR base: 0x40080000

INFO:    SMC call: (0xc2000065) (function id: 101)

GICI base: 0x44081000

InitializeSbsaQemuPlatformDxe: Got platform XHCI 60110000 65536

INFO:    SMC call: (0xc20000c8) (function id: 200)

We have 4 cpus.


It does:

- AHCI init
- Platform version
- GIC addresses
- GIC ITS address
- XHCI init
- CPU count

Then system boot continues with 
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount being set to proper 
value.

If replacing use of Pcd with calls to SbsaQemuGetCpuCount() look better 
then I can change code to make it happen.


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init
  2024-03-19 10:25       ` Marcin Juszkiewicz
@ 2024-03-19 11:02         ` Ard Biesheuvel
  2024-03-19 11:27           ` Marcin Juszkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2024-03-19 11:02 UTC (permalink / raw)
  To: Marcin Juszkiewicz
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

On Tue, 19 Mar 2024 at 11:25, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> W dniu 15.03.2024 o 12:49, Marcin Juszkiewicz pisze:
> > W dniu 14.03.2024 o 16:13, Ard Biesheuvel pisze:
>
> >> How is it guaranteed that other components will only see the correct
> >> core count? DXE dispatch is ordered using a dependency graph, so all
> >> users of this PCD should never execute before this driver.
> >
> > SbsaQemuPlatformDxe is a DXE, right? So it is called on platform init.
> >
> > At the end of initialization it calls SbsaQemuGetCpuCount() from
> > SbsaQemuHardwareInfoLib to SET this PCD. It does not use it during
> > platform init cause it does not require this information. But calls
> > function to make sure that amount of cpus is known to whatever will be
> > called later.
> >
> > Sure, maybe SbsaQemuHardwareInfoLib should be something else (DXE,
> > Protocol or other EDK2 magic thing) but it is set of functions to be
> > called from other places of EDK2.
>
> EDK2 starts and one of the first DXE called is SbsaQemuPlatformDxe one:
>

How is this guaranteed? DXE are generally dispatched in the order in
which they appear in the FDF, but only if all DEPEX dependencies are
satisfied. DEPEXes are compiled from the DXE .inf along with all its
[recursive] library dependencies, so upstream changes could affect the
DEPEX of SbsaQemuPlatformDxe, and therefore where it appears in the
dispatch order.

None of the below is relevant, really. If you want to rely on dynamic
PCDs in DXE, the only safe way to set them is from the PEI phase.


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init
  2024-03-19 11:02         ` Ard Biesheuvel
@ 2024-03-19 11:27           ` Marcin Juszkiewicz
  0 siblings, 0 replies; 19+ messages in thread
From: Marcin Juszkiewicz @ 2024-03-19 11:27 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: devel, Leif Lindholm, Ard Biesheuvel, Graeme Gregory,
	Xiong Yining, Chen Baozi

W dniu 19.03.2024 o 12:02, Ard Biesheuvel pisze:
>> EDK2 starts and one of the first DXE called is SbsaQemuPlatformDxe one:
>>
> How is this guaranteed? DXE are generally dispatched in the order in
> which they appear in the FDF, but only if all DEPEX dependencies are
> satisfied. DEPEXes are compiled from the DXE .inf along with all its
> [recursive] library dependencies, so upstream changes could affect the
> DEPEX of SbsaQemuPlatformDxe, and therefore where it appears in the
> dispatch order.

OK. Than it will be safer to call SbsaQemuGetCpuCount() in each place 
which needs CoreCount. Will adapt code. Thanks.


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



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2024-03-19 11:27 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-06 11:41 [edk2-devel] [PATCH edk2-platforms v6 0/7] get rid of DeviceTree from SbsaQemu Marcin Juszkiewicz
2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib Marcin Juszkiewicz
2024-03-14 15:09   ` Ard Biesheuvel
2024-03-14 15:14     ` Ard Biesheuvel
2024-03-15 11:34       ` Marcin Juszkiewicz
2024-03-06 11:41 ` [edk2-devel] [PATCH edk2-platforms v6 2/7] Platform/SbsaQemu: read amount of cpus during init Marcin Juszkiewicz
2024-03-14 15:13   ` Ard Biesheuvel
2024-03-15 11:49     ` Marcin Juszkiewicz
2024-03-19 10:25       ` Marcin Juszkiewicz
2024-03-19 11:02         ` Ard Biesheuvel
2024-03-19 11:27           ` Marcin Juszkiewicz
2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 3/7] Platform/SbsaQemu: use PcdCoreCount directly Marcin Juszkiewicz
2024-03-14 15:15   ` Ard Biesheuvel
2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 4/7] Platform/SbsaQemu: move FdtHandlerLib to SbsaQemuHardwareInfoLib Marcin Juszkiewicz
2024-03-14 15:16   ` Ard Biesheuvel
2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 5/7] Platform/SbsaQemu: hang if there is no cpu information Marcin Juszkiewicz
2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 6/7] Platform/SbsaQemu: get the information of memory via SMC calls Marcin Juszkiewicz
2024-03-14 15:18   ` Ard Biesheuvel
2024-03-06 11:42 ` [edk2-devel] [PATCH edk2-platforms v6 7/7] Platform/SbsaQemu: add DeviceTree fallbacks to parse memory information Marcin Juszkiewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox