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

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

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

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

> +    Arg0 = FdtHelperCountCpus();

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

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

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

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

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

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

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

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

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

/
    Leif

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114095): https://edk2.groups.io/g/devel/message/114095
Mute This Topic: https://groups.io/mt/103758014/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to