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+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>
> >
> > Tested-By: Graeme Gregory <gra...@nuviainc.com>
>
> Tested-by: Marcin Juszkiewicz <marcin.juszkiew...@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;
> > > }
> > >
> >
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72339): https://edk2.groups.io/g/devel/message/72339
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]
-=-=-=-=-=-=-=-=-=-=-=-
