Re: [edk2-devel] [PATCH edk2-platforms v9 2/4] Platform/SbsaQemu: use SbsaQemuHardwareInfoLib for cpu information

["Leif Lindholm" <quic_llindhol@quicinc.com>]

On Fri, Mar 22, 2024 at 17:08:48 +0100, Marcin Juszkiewicz wrote:
> We have SbsaQemuHardwareInfoLib to ask for hardware details. No need to
> parse DeviceTree anymore.
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

Nitpicks or changing function calls based on what I commented on in
previous patch does not retract from:
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

> ---
>  Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf     |  6 ++----
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf      |  5 ++---
>  .../SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf     |  4 ++--
>  Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c       | 11 +++++-----
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c        | 21 +++++++-------------
>  5 files changed, 18 insertions(+), 29 deletions(-)
> 
> diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf
> index a34f54d431d4..f959d8e0e4ee 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) 2018-2024, Linaro Ltd. All rights reserved.
>  #
>  #    SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -29,8 +29,7 @@ [Packages]
>  
>  [LibraryClasses]
>    BaseMemoryLib
> -  FdtLib
> -  FdtHelperLib
> +  HardwareInfoLib
>    IoLib
>    PcdLib
>  
> @@ -40,7 +39,6 @@ [Guids]
>  [Pcd]
>    gArmTokenSpaceGuid.PcdEmbeddedControllerFirmwareRelease
>    gArmTokenSpaceGuid.PcdSystemBiosRelease
> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>  
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSystemManufacturer
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSystemSerialNumber
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index 291743b19115..727c8e82d16e 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) 2020-2024, Linaro Ltd. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -35,7 +35,7 @@ [LibraryClasses]
>    BaseLib
>    DebugLib
>    DxeServicesLib
> -  FdtHelperLib
> +  HardwareInfoLib
>    PcdLib
>    PrintLib
>    UefiDriverEntryPoint
> @@ -44,7 +44,6 @@ [LibraryClasses]
>  
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
>  
>    gArmTokenSpaceGuid.PcdGicDistributorBase
> diff --git a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> index c067a80cc715..07e6bc4e9b11 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> +++ b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf
> @@ -1,6 +1,6 @@
>  #/* @file
>  #
> -#  Copyright (c) 2019, Linaro Limited. All rights reserved.
> +#  Copyright (c) 2019-2024, Linaro Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -32,9 +32,9 @@ [LibraryClasses]
>    ArmLib
>    BaseMemoryLib
>    DebugLib
> -  FdtLib
>    MemoryAllocationLib
>    PcdLib
> +  SbsaQemuHardwareInfoLib

LibraryClass uses different name here?

>  
>  [Pcd]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
> diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c
> index c38f2851904f..854f6f4072d5 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) 2020-2024, Linaro Ltd. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -12,14 +12,13 @@
>  #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/HardwareInfoLib.h>

Move above HiiLib.h.

>  #include <Library/SerialPortLib.h>
>  #include <Library/TimerLib.h>
> -#include <libfdt.h>
>  
>  /** Returns whether the specified processor is present or not.
>  
> @@ -33,7 +32,7 @@ OemIsProcessorPresent (
>    UINTN ProcessorIndex
>    )
>  {
> -  if (ProcessorIndex < FdtHelperCountCpus ()) {
> +  if (ProcessorIndex < GetCpuCount ()) {
>      return TRUE;
>    }
>  
> @@ -76,7 +75,7 @@ OemGetProcessorInformation (
>  {
>    UINT16 ProcessorCount;
>  
> -  ProcessorCount = FdtHelperCountCpus ();
> +  ProcessorCount = GetCpuCount ();
>  
>    if (ProcessorIndex < ProcessorCount) {
>      ProcessorStatus->Bits.CpuStatus       = 1; // CPU enabled
> @@ -121,7 +120,7 @@ OemGetMaxProcessors (
>    VOID
>    )
>  {
> -  return FdtHelperCountCpus ();
> +  return GetCpuCount ();
>  }
>  
>  /** 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..4ebe2a445344 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) 2020-2024, Linaro Ltd. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -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/HardwareInfoLib.h>

Move above MemoryAllocationLib.h.

/
    Leif

>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <Library/UefiLib.h>
> @@ -255,8 +255,7 @@ AddMadtTable (
>   // Initialize GIC Redistributor Structure
>    EFI_ACPI_6_0_GICR_STRUCTURE Gicr = SBSAQEMU_MADT_GICR_INIT();
>  
> -  // Get CoreCount which was determined eariler after parsing device tree
> -  NumCores = PcdGet32 (PcdCoreCount);
> +  NumCores = GetCpuCount ();
>  
>    // Calculate the new table size based on the number of cores
>    TableSize = sizeof (EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER) +
> @@ -291,13 +290,13 @@ 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));
>      GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
>      GiccPtr->AcpiProcessorUid = CoreIndex;
> -    GiccPtr->MPIDR = FdtHelperGetMpidr (CoreIndex);
> +    GiccPtr->MPIDR = GetMpidr (CoreIndex);
>      New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
>    }
>  
> @@ -396,7 +395,7 @@ AddSsdtTable (
>    UINT32                CpuId;
>    UINT32                Offset;
>    UINT8                 ScopeOpName[] =  SBSAQEMU_ACPI_SCOPE_NAME;
> -  UINT32                NumCores = PcdGet32 (PcdCoreCount);
> +  UINT32                NumCores = GetCpuCount ();
>  
>    EFI_ACPI_DESCRIPTION_HEADER Header =
>      SBSAQEMU_ACPI_HEADER (
> @@ -497,7 +496,7 @@ AddPpttTable (
>    EFI_PHYSICAL_ADDRESS  PageAddress;
>    UINT8                 *New;
>    UINT32                CpuId;
> -  UINT32                NumCores = PcdGet32 (PcdCoreCount);
> +  UINT32                NumCores = GetCpuCount ();
>  
>    EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE L1DCache = SBSAQEMU_ACPI_PPTT_L1_D_CACHE_STRUCT;
>    EFI_ACPI_6_3_PPTT_STRUCTURE_CACHE L1ICache = SBSAQEMU_ACPI_PPTT_L1_I_CACHE_STRUCT;
> @@ -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 (#117089): https://edk2.groups.io/g/devel/message/117089
Mute This Topic: https://groups.io/mt/105088440/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-