Re: [edk2-platforms PATCH v3 2/3] SbsaQemu: Update SbsaQemuAcpiDxe to use FdtHelperLib

["Leif Lindholm" <leif@nuviainc.com>]

On Thu, Feb 18, 2021 at 20:57:40 -0700, Rebecca Cran wrote:
> Use the copy of the CountCpusFromFdt function from FdtHelperLib.

This patch also needs to invoke CountCpusFromFdt ...
Hmm, come to think of it.
Could we change the name of the function (in 1/3) to
FdtHelperCountCpus? Since it's now a global symbol, some namespace
protection wouldn't go amiss.

Anyway, in this patch, where CountCpusFromFdt was invoked, we now need
to invoke the bit that doesn't get migrated to the helper library:

  Status = PcdSet32S (PcdCoreCount, NumCores);
  ASSERT_RETURN_ERROR (Status);

Urgh...
If you could possibly find it in your heart to add a new temporary
variable called CoreIndex to AddMadtTable() and use that instead of
using NumCores as an index further down, that would improve this code
in general.

/
    Leif

> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
> ---
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 50 +-------------------
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  1 +
>  2 files changed, 2 insertions(+), 49 deletions(-)
> 
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index fb7c1835c3d7..02ba3e452c06 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -12,6 +12,7 @@
>  #include <Library/AcpiLib.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>
> @@ -25,55 +26,6 @@
>  STATIC INT32 FdtFirstCpuOffset;
>  STATIC INT32 FdtCpuNodeSize;
>  
> -/*
> - * A function that walks through the Device Tree created
> - * by Qemu and counts the number of CPUs present in it.
> - */
> -STATIC
> -VOID
> -CountCpusFromFdt (
> -  VOID
> -)
> -{
> -  VOID           *DeviceTreeBase;
> -  INT32          Node, Prev;
> -  RETURN_STATUS  PcdStatus;
> -  INT32          CpuNode;
> -  INT32          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;
> -  }
> -
> -  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);
> -  FdtFirstCpuOffset = Prev;
> -  while (1) {
> -    CpuCount++;
> -    Node = fdt_next_subnode (DeviceTreeBase, Prev);
> -    if (Node < 0) {
> -      break;
> -    }
> -    FdtCpuNodeSize = Node - Prev;
> -    Prev = Node;
> -  }
> -
> -  PcdStatus = PcdSet32S (PcdCoreCount, CpuCount);
> -  ASSERT_RETURN_ERROR (PcdStatus);
> -}
> -
>  /*
>   * Get MPIDR from device tree passed by Qemu
>   */
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index 127eef029f3c..a58ebfaf76d5 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> @@ -34,6 +34,7 @@
>    BaseLib
>    DebugLib
>    DxeServicesLib
> +  FdtHelperLib
>    FdtLib
>    PcdLib
>    PrintLib
> -- 
> 2.26.2
>