From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web12.8990.1614693937557426448 for ; Tue, 02 Mar 2021 06:05:37 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=aPc/gm9e; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id C6D8764F11 for ; Tue, 2 Mar 2021 14:05:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1614693936; bh=PC4aHmrmkFWmYNsIDJ7OG6XrxafmuxaMW/D0iYfR3vY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=aPc/gm9eqk1TUXwchEoDzQI3At4dmQ5lJb+BWBTlVO1rzAwRTDEx4eMXail9vkRis 0NC7T/ADwRn/1J6KgKdYTVIedlmdUvmt5BwEfy+4Vn9TGtznaF7BISDX9jrpdohEnX /061cbs1SUCl0L4iTDtQZpm5DQ3xZ9N9Fk0DdrkGPNY8BLuzf1d/66zNWIPlUImxzM RocK/gsADrx9cvvcWZF1jEfUIQ7vQ4P23bvoAQnV84fGR0TxeR82bgn0wDGKB6uIjS a6wI9uxrkpWHX8PH6kOuoO4eikc877vrPj7zqYuXY/guv4HVg1t3b9oQkqR0sb2FlY JOKeIx7eKfvMw== Received: by mail-oi1-f177.google.com with SMTP id w69so22072194oif.1 for ; Tue, 02 Mar 2021 06:05:36 -0800 (PST) X-Gm-Message-State: AOAM533rNbb4cSxgFXMLRqbiU7WJD0vnRZXvq9An7PxJYWM5jYSffZCM D5xUR6oYk1JwPLQuaRfvFmv0FuMvdGRequ6IXV4= X-Google-Smtp-Source: ABdhPJxgfd67M6YxIK8jpxGLBVWCPKOLvhK41Qwzdb0bykAXGhtpyglX5bdiw4b7bm86tjFJ7cGpwEmZJXJNClSm5jU= X-Received: by 2002:aca:b6c1:: with SMTP id g184mr3301115oif.47.1614693935931; Tue, 02 Mar 2021 06:05:35 -0800 (PST) MIME-Version: 1.0 References: <20210302133828.19321-1-leif@nuviainc.com> In-Reply-To: <20210302133828.19321-1-leif@nuviainc.com> From: "Ard Biesheuvel" Date: Tue, 2 Mar 2021 15:05:24 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib To: Leif Lindholm Cc: devel@edk2.groups.io, Ard Biesheuvel , Graeme Gregory , Radoslaw Biernacki , Tanmay Jagdale , Rebecca Cran , Marcin Juszkiewicz Content-Type: text/plain; charset="UTF-8" On Tue, 2 Mar 2021 at 14: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 > Cc: Graeme Gregory > Cc: Radoslaw Biernacki > Cc: Tanmay Jagdale > Cc: Rebecca Cran > Reported-by: Marcin Juszkiewicz > Signed-off-by: Leif Lindholm Acked-by: Ard Biesheuvel > --- > 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 > +**/ > +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 > #include > #include > -#include > -#include > - > -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 > #include > > +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 > +**/ > +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 >