public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib
@ 2021-03-02 13:38 Leif Lindholm
  2021-03-02 14:05 ` Ard Biesheuvel
  2021-03-02 14:14 ` Graeme Gregory
  0 siblings, 2 replies; 5+ messages in thread
From: Leif Lindholm @ 2021-03-02 13:38 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Graeme Gregory, Radoslaw Biernacki,
	Tanmay Jagdale, Rebecca Cran, Marcin Juszkiewicz

Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use FdtHelperLib")
replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a call to
FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static variables
FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the GetMpidr()
function kept returning the value for cpu 0.

Resolve this by moving the GetMpidr() function over to FdtHelperLib, where
it can again share these variables with FdtHelperCountCpus().

Fix up coding style issues as part of copy:
- Add m prefix to module-global variables.
- Add doxygen function comment header.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Graeme Gregory <graeme@nuviainc.com>
Cc: Radoslaw Biernacki <rad@semihalf.com>
Cc: Tanmay Jagdale <tanmay.jagdale@linaro.org>
Cc: Rebecca Cran <rebecca@nuviainc.com>
Reported-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  2 --
 Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf       |  5 +++
 Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h              | 12 +++++++
 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 35 +------------------
 Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c         | 36 ++++++++++++++++++++
 5 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index a58ebfaf76d5..c6de685bd2c4 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -35,7 +35,6 @@ [LibraryClasses]
   DebugLib
   DxeServicesLib
   FdtHelperLib
-  FdtLib
   PcdLib
   PrintLib
   UefiDriverEntryPoint
@@ -46,7 +45,6 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
-  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
 
 [Depex]
   gEfiAcpiTableProtocolGuid                       ## CONSUMES
diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
index d84c16f888d1..9c059f3e5851 100644
--- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
@@ -24,5 +24,10 @@ [Packages]
   MdePkg/MdePkg.dec
   Silicon/Qemu/SbsaQemu/SbsaQemu.dec
 
+[LibraryClasses]
+  DebugLib
+  FdtLib
+  PcdLib
+
 [FixedPcd]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
index f9045fd5df45..ea9159857215 100644
--- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
@@ -10,6 +10,18 @@
 #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.
 
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 84120f1c1b51..b8901030ecd0 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -20,39 +20,6 @@
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/UefiLib.h>
 #include <Protocol/AcpiTable.h>
-#include <Protocol/FdtClient.h>
-#include <libfdt.h>
-
-STATIC INT32 FdtFirstCpuOffset;
-STATIC INT32 FdtCpuNodeSize;
-
-/*
- * Get MPIDR from device tree passed by Qemu
- */
-STATIC
-UINT64
-GetMpidr (
-  IN UINTN   CpuId
-  )
-{
-  VOID           *DeviceTreeBase;
-  CONST UINT64   *RegVal;
-  INT32          Len;
-
-  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
-  ASSERT (DeviceTreeBase != NULL);
-
-  RegVal = fdt_getprop (DeviceTreeBase,
-             FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
-             "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)));
-}
 
 /*
  * A Function to Compute the ACPI Table Checksum
@@ -159,7 +126,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 = GetMpidr (CoreIndex);
+    GiccPtr->MPIDR = FdtHelperGetMpidr (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
index bbd756f01a21..7fdfb055db76 100644
--- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
+++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
@@ -14,6 +14,40 @@
 #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.
 
@@ -49,12 +83,14 @@ FdtHelperCountCpus (
   // 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;
   }
 
-- 
2.20.1


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

* Re: [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib
  2021-03-02 13:38 [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib Leif Lindholm
@ 2021-03-02 14:05 ` Ard Biesheuvel
  2021-03-02 14:14 ` Graeme Gregory
  1 sibling, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2021-03-02 14:05 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: devel, Ard Biesheuvel, Graeme Gregory, Radoslaw Biernacki,
	Tanmay Jagdale, Rebecca Cran, Marcin Juszkiewicz

On Tue, 2 Mar 2021 at 14:38, Leif Lindholm <leif@nuviainc.com> wrote:
>
> Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use FdtHelperLib")
> replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a call to
> FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static variables
> FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the GetMpidr()
> function kept returning the value for cpu 0.
>
> Resolve this by moving the GetMpidr() function over to FdtHelperLib, where
> it can again share these variables with FdtHelperCountCpus().
>
> Fix up coding style issues as part of copy:
> - Add m prefix to module-global variables.
> - Add doxygen function comment header.
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Graeme Gregory <graeme@nuviainc.com>
> Cc: Radoslaw Biernacki <rad@semihalf.com>
> Cc: Tanmay Jagdale <tanmay.jagdale@linaro.org>
> Cc: Rebecca Cran <rebecca@nuviainc.com>
> Reported-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  2 --
>  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf       |  5 +++
>  Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h              | 12 +++++++
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 35 +------------------
>  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c         | 36 ++++++++++++++++++++
>  5 files changed, 54 insertions(+), 36 deletions(-)
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index a58ebfaf76d5..c6de685bd2c4 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> @@ -35,7 +35,6 @@ [LibraryClasses]
>    DebugLib
>    DxeServicesLib
>    FdtHelperLib
> -  FdtLib
>    PcdLib
>    PrintLib
>    UefiDriverEntryPoint
> @@ -46,7 +45,6 @@ [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>
>  [Depex]
>    gEfiAcpiTableProtocolGuid                       ## CONSUMES
> diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> index d84c16f888d1..9c059f3e5851 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> @@ -24,5 +24,10 @@ [Packages]
>    MdePkg/MdePkg.dec
>    Silicon/Qemu/SbsaQemu/SbsaQemu.dec
>
> +[LibraryClasses]
> +  DebugLib
> +  FdtLib
> +  PcdLib
> +
>  [FixedPcd]
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> index f9045fd5df45..ea9159857215 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> @@ -10,6 +10,18 @@
>  #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.
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 84120f1c1b51..b8901030ecd0 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -20,39 +20,6 @@
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <Library/UefiLib.h>
>  #include <Protocol/AcpiTable.h>
> -#include <Protocol/FdtClient.h>
> -#include <libfdt.h>
> -
> -STATIC INT32 FdtFirstCpuOffset;
> -STATIC INT32 FdtCpuNodeSize;
> -
> -/*
> - * Get MPIDR from device tree passed by Qemu
> - */
> -STATIC
> -UINT64
> -GetMpidr (
> -  IN UINTN   CpuId
> -  )
> -{
> -  VOID           *DeviceTreeBase;
> -  CONST UINT64   *RegVal;
> -  INT32          Len;
> -
> -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> -  ASSERT (DeviceTreeBase != NULL);
> -
> -  RegVal = fdt_getprop (DeviceTreeBase,
> -             FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
> -             "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)));
> -}
>
>  /*
>   * A Function to Compute the ACPI Table Checksum
> @@ -159,7 +126,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 = GetMpidr (CoreIndex);
> +    GiccPtr->MPIDR = FdtHelperGetMpidr (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
> index bbd756f01a21..7fdfb055db76 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> @@ -14,6 +14,40 @@
>  #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.
>
> @@ -49,12 +83,14 @@ FdtHelperCountCpus (
>    // 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;
>    }
>
> --
> 2.20.1
>

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

* Re: [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib
  2021-03-02 13:38 [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib Leif Lindholm
  2021-03-02 14:05 ` Ard Biesheuvel
@ 2021-03-02 14:14 ` Graeme Gregory
  2021-03-02 15:01   ` Marcin Juszkiewicz
  1 sibling, 1 reply; 5+ messages in thread
From: Graeme Gregory @ 2021-03-02 14:14 UTC (permalink / raw)
  To: Leif Lindholm, devel
  Cc: Ard Biesheuvel, Radoslaw Biernacki, Tanmay Jagdale, Rebecca Cran,
	Marcin Juszkiewicz

On 02/03/2021 13:38, Leif Lindholm wrote:
> Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use FdtHelperLib")
> replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a call to
> FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static variables
> FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the GetMpidr()
> function kept returning the value for cpu 0.
> 
> Resolve this by moving the GetMpidr() function over to FdtHelperLib, where
> it can again share these variables with FdtHelperCountCpus().
> 
> Fix up coding style issues as part of copy:
> - Add m prefix to module-global variables.
> - Add doxygen function comment header.
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Graeme Gregory <graeme@nuviainc.com>
> Cc: Radoslaw Biernacki <rad@semihalf.com>
> Cc: Tanmay Jagdale <tanmay.jagdale@linaro.org>
> Cc: Rebecca Cran <rebecca@nuviainc.com>
> Reported-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>

Tested-By: Graeme Gregory <graeme@nuviainc.com>

This fixes the issue from inspection of APIC table with acpiview!

> ---
>   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  2 --
>   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf       |  5 +++
>   Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h              | 12 +++++++
>   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 35 +------------------
>   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c         | 36 ++++++++++++++++++++
>   5 files changed, 54 insertions(+), 36 deletions(-)
> 
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index a58ebfaf76d5..c6de685bd2c4 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> @@ -35,7 +35,6 @@ [LibraryClasses]
>     DebugLib
>     DxeServicesLib
>     FdtHelperLib
> -  FdtLib
>     PcdLib
>     PrintLib
>     UefiDriverEntryPoint
> @@ -46,7 +45,6 @@ [Pcd]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
>     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>   
>   [Depex]
>     gEfiAcpiTableProtocolGuid                       ## CONSUMES
> diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> index d84c16f888d1..9c059f3e5851 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> @@ -24,5 +24,10 @@ [Packages]
>     MdePkg/MdePkg.dec
>     Silicon/Qemu/SbsaQemu/SbsaQemu.dec
>   
> +[LibraryClasses]
> +  DebugLib
> +  FdtLib
> +  PcdLib
> +
>   [FixedPcd]
>     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> index f9045fd5df45..ea9159857215 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> @@ -10,6 +10,18 @@
>   #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.
>   
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 84120f1c1b51..b8901030ecd0 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -20,39 +20,6 @@
>   #include <Library/UefiDriverEntryPoint.h>
>   #include <Library/UefiLib.h>
>   #include <Protocol/AcpiTable.h>
> -#include <Protocol/FdtClient.h>
> -#include <libfdt.h>
> -
> -STATIC INT32 FdtFirstCpuOffset;
> -STATIC INT32 FdtCpuNodeSize;
> -
> -/*
> - * Get MPIDR from device tree passed by Qemu
> - */
> -STATIC
> -UINT64
> -GetMpidr (
> -  IN UINTN   CpuId
> -  )
> -{
> -  VOID           *DeviceTreeBase;
> -  CONST UINT64   *RegVal;
> -  INT32          Len;
> -
> -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> -  ASSERT (DeviceTreeBase != NULL);
> -
> -  RegVal = fdt_getprop (DeviceTreeBase,
> -             FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
> -             "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)));
> -}
>   
>   /*
>    * A Function to Compute the ACPI Table Checksum
> @@ -159,7 +126,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 = GetMpidr (CoreIndex);
> +    GiccPtr->MPIDR = FdtHelperGetMpidr (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
> index bbd756f01a21..7fdfb055db76 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> @@ -14,6 +14,40 @@
>   #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.
>   
> @@ -49,12 +83,14 @@ FdtHelperCountCpus (
>     // 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;
>     }
>   
> 


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

* Re: [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib
  2021-03-02 14:14 ` Graeme Gregory
@ 2021-03-02 15:01   ` Marcin Juszkiewicz
  2021-03-02 15:36     ` Leif Lindholm
  0 siblings, 1 reply; 5+ messages in thread
From: Marcin Juszkiewicz @ 2021-03-02 15:01 UTC (permalink / raw)
  To: Graeme Gregory, Leif Lindholm, devel
  Cc: Ard Biesheuvel, Radoslaw Biernacki, Tanmay Jagdale, Rebecca Cran

W dniu 02.03.2021 o 15:14, Graeme Gregory pisze:
> On 02/03/2021 13:38, Leif Lindholm wrote:
>> Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use 
>> FdtHelperLib")
>> replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a 
>> call to
>> FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static 
>> variables
>> FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the 
>> GetMpidr()
>> function kept returning the value for cpu 0.
>>
>> Resolve this by moving the GetMpidr() function over to FdtHelperLib, 
>> where
>> it can again share these variables with FdtHelperCountCpus().
>>
>> Fix up coding style issues as part of copy:
>> - Add m prefix to module-global variables.
>> - Add doxygen function comment header.
>>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Graeme Gregory <graeme@nuviainc.com>
>> Cc: Radoslaw Biernacki <rad@semihalf.com>
>> Cc: Tanmay Jagdale <tanmay.jagdale@linaro.org>
>> Cc: Rebecca Cran <rebecca@nuviainc.com>
>> Reported-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
>> Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> 
> Tested-By: Graeme Gregory <graeme@nuviainc.com>

Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

sbsa-acs now finish in seconds like before.

> 
>> ---
>>   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  
>> 2 --
>>   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf       |  
>> 5 +++
>>   Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h              | 
>> 12 +++++++
>>   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 
>> 35 +------------------
>>   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c         | 
>> 36 ++++++++++++++++++++
>>   5 files changed, 54 insertions(+), 36 deletions(-)
>>
>> diff --git 
>> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf 
>> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
>> index a58ebfaf76d5..c6de685bd2c4 100644
>> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
>> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
>> @@ -35,7 +35,6 @@ [LibraryClasses]
>>     DebugLib
>>     DxeServicesLib
>>     FdtHelperLib
>> -  FdtLib
>>     PcdLib
>>     PrintLib
>>     UefiDriverEntryPoint
>> @@ -46,7 +45,6 @@ [Pcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
>>     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>>     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
>> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>>   [Depex]
>>     gEfiAcpiTableProtocolGuid                       ## CONSUMES
>> diff --git 
>> a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf 
>> b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
>> index d84c16f888d1..9c059f3e5851 100644
>> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
>> +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
>> @@ -24,5 +24,10 @@ [Packages]
>>     MdePkg/MdePkg.dec
>>     Silicon/Qemu/SbsaQemu/SbsaQemu.dec
>> +[LibraryClasses]
>> +  DebugLib
>> +  FdtLib
>> +  PcdLib
>> +
>>   [FixedPcd]
>>     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h 
>> b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
>> index f9045fd5df45..ea9159857215 100644
>> --- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
>> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
>> @@ -10,6 +10,18 @@
>>   #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.
>> diff --git 
>> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
>> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> index 84120f1c1b51..b8901030ecd0 100644
>> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
>> @@ -20,39 +20,6 @@
>>   #include <Library/UefiDriverEntryPoint.h>
>>   #include <Library/UefiLib.h>
>>   #include <Protocol/AcpiTable.h>
>> -#include <Protocol/FdtClient.h>
>> -#include <libfdt.h>
>> -
>> -STATIC INT32 FdtFirstCpuOffset;
>> -STATIC INT32 FdtCpuNodeSize;
>> -
>> -/*
>> - * Get MPIDR from device tree passed by Qemu
>> - */
>> -STATIC
>> -UINT64
>> -GetMpidr (
>> -  IN UINTN   CpuId
>> -  )
>> -{
>> -  VOID           *DeviceTreeBase;
>> -  CONST UINT64   *RegVal;
>> -  INT32          Len;
>> -
>> -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
>> -  ASSERT (DeviceTreeBase != NULL);
>> -
>> -  RegVal = fdt_getprop (DeviceTreeBase,
>> -             FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
>> -             "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)));
>> -}
>>   /*
>>    * A Function to Compute the ACPI Table Checksum
>> @@ -159,7 +126,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 = GetMpidr (CoreIndex);
>> +    GiccPtr->MPIDR = FdtHelperGetMpidr (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
>> index bbd756f01a21..7fdfb055db76 100644
>> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
>> +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
>> @@ -14,6 +14,40 @@
>>   #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.
>> @@ -49,12 +83,14 @@ FdtHelperCountCpus (
>>     // 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;
>>     }
>>
> 


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

* Re: [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib
  2021-03-02 15:01   ` Marcin Juszkiewicz
@ 2021-03-02 15:36     ` Leif Lindholm
  0 siblings, 0 replies; 5+ messages in thread
From: Leif Lindholm @ 2021-03-02 15:36 UTC (permalink / raw)
  To: Marcin Juszkiewicz
  Cc: Graeme Gregory, devel, Ard Biesheuvel, Radoslaw Biernacki,
	Tanmay Jagdale, Rebecca Cran

On Tue, Mar 02, 2021 at 16:01:56 +0100, Marcin Juszkiewicz wrote:
> W dniu 02.03.2021 o 15:14, Graeme Gregory pisze:
> > On 02/03/2021 13:38, Leif Lindholm wrote:
> > > Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use
> > > FdtHelperLib")
> > > replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a
> > > call to
> > > FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static
> > > variables
> > > FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the
> > > GetMpidr()
> > > function kept returning the value for cpu 0.
> > > 
> > > Resolve this by moving the GetMpidr() function over to FdtHelperLib,
> > > where
> > > it can again share these variables with FdtHelperCountCpus().
> > > 
> > > Fix up coding style issues as part of copy:
> > > - Add m prefix to module-global variables.
> > > - Add doxygen function comment header.
> > > 
> > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > Cc: Graeme Gregory <graeme@nuviainc.com>
> > > Cc: Radoslaw Biernacki <rad@semihalf.com>
> > > Cc: Tanmay Jagdale <tanmay.jagdale@linaro.org>
> > > Cc: Rebecca Cran <rebecca@nuviainc.com>
> > > Reported-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> > > Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> > 
> > Tested-By: Graeme Gregory <graeme@nuviainc.com>
> 
> Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> 
> sbsa-acs now finish in seconds like before.

Thanks all.
Pushed as a3ce6f8df2b6.

> > 
> > > ---
> > >   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > |  2 --
> > >   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf      
> > > |  5 +++
> > >   Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h             
> > > | 12 +++++++
> > >   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c  
> > > | 35 +------------------
> > >   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c        
> > > | 36 ++++++++++++++++++++
> > >   5 files changed, 54 insertions(+), 36 deletions(-)
> > > 
> > > diff --git
> > > a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > index a58ebfaf76d5..c6de685bd2c4 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > @@ -35,7 +35,6 @@ [LibraryClasses]
> > >     DebugLib
> > >     DxeServicesLib
> > >     FdtHelperLib
> > > -  FdtLib
> > >     PcdLib
> > >     PrintLib
> > >     UefiDriverEntryPoint
> > > @@ -46,7 +45,6 @@ [Pcd]
> > >     gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
> > >     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
> > >     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
> > > -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> > >   [Depex]
> > >     gEfiAcpiTableProtocolGuid                       ## CONSUMES
> > > diff --git
> > > a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > index d84c16f888d1..9c059f3e5851 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > @@ -24,5 +24,10 @@ [Packages]
> > >     MdePkg/MdePkg.dec
> > >     Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> > > +[LibraryClasses]
> > > +  DebugLib
> > > +  FdtLib
> > > +  PcdLib
> > > +
> > >   [FixedPcd]
> > >     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> > > diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > index f9045fd5df45..ea9159857215 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > +++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > @@ -10,6 +10,18 @@
> > >   #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.
> > > diff --git
> > > a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > index 84120f1c1b51..b8901030ecd0 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > @@ -20,39 +20,6 @@
> > >   #include <Library/UefiDriverEntryPoint.h>
> > >   #include <Library/UefiLib.h>
> > >   #include <Protocol/AcpiTable.h>
> > > -#include <Protocol/FdtClient.h>
> > > -#include <libfdt.h>
> > > -
> > > -STATIC INT32 FdtFirstCpuOffset;
> > > -STATIC INT32 FdtCpuNodeSize;
> > > -
> > > -/*
> > > - * Get MPIDR from device tree passed by Qemu
> > > - */
> > > -STATIC
> > > -UINT64
> > > -GetMpidr (
> > > -  IN UINTN   CpuId
> > > -  )
> > > -{
> > > -  VOID           *DeviceTreeBase;
> > > -  CONST UINT64   *RegVal;
> > > -  INT32          Len;
> > > -
> > > -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> > > -  ASSERT (DeviceTreeBase != NULL);
> > > -
> > > -  RegVal = fdt_getprop (DeviceTreeBase,
> > > -             FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
> > > -             "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)));
> > > -}
> > >   /*
> > >    * A Function to Compute the ACPI Table Checksum
> > > @@ -159,7 +126,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 = GetMpidr (CoreIndex);
> > > +    GiccPtr->MPIDR = FdtHelperGetMpidr (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
> > > index bbd756f01a21..7fdfb055db76 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> > > +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> > > @@ -14,6 +14,40 @@
> > >   #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.
> > > @@ -49,12 +83,14 @@ FdtHelperCountCpus (
> > >     // 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;
> > >     }
> > > 
> > 
> 

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

end of thread, other threads:[~2021-03-02 15:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-02 13:38 [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib Leif Lindholm
2021-03-02 14:05 ` Ard Biesheuvel
2021-03-02 14:14 ` Graeme Gregory
2021-03-02 15:01   ` Marcin Juszkiewicz
2021-03-02 15:36     ` Leif Lindholm

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