Re: [edk2-devel] [PATCH edk2-platforms v6 1/7] Platform/SbsaQemu: add SbsaQemuHardwareInfoLib

["Ard Biesheuvel" <ardb@kernel.org>]

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]
-=-=-=-=-=-=-=-=-=-=-=-