On Tue, 2 Mar 2021 at 14:38, Leif Lindholm <l...@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+tianoc...@kernel.org>
> Cc: Graeme Gregory <gra...@nuviainc.com>
> Cc: Radoslaw Biernacki <r...@semihalf.com>
> Cc: Tanmay Jagdale <tanmay.jagd...@linaro.org>
> Cc: Rebecca Cran <rebe...@nuviainc.com>
> Reported-by: Marcin Juszkiewicz <marcin.juszkiew...@linaro.org>
> Signed-off-by: Leif Lindholm <l...@nuviainc.com>
Acked-by: Ard Biesheuvel <a...@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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72335): https://edk2.groups.io/g/devel/message/72335
Mute This Topic: https://groups.io/mt/81024935/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
