From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by mx.groups.io with SMTP id smtpd.web09.10104.1614699385278343523 for ; Tue, 02 Mar 2021 07:36:25 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=GdVtSPbf; spf=pass (domain: nuviainc.com, ip: 209.85.221.54, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f54.google.com with SMTP id 7so20357177wrz.0 for ; Tue, 02 Mar 2021 07:36:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=cth41kZCcyYbiXvrifaFD6WDIyQSB0f6zaPJXMzyJ4E=; b=GdVtSPbfyiuhYxjDJ/8CcsS3aGOBjiPpmKBWB+QxhXM677N8WrizmpmVkZJevZyrus K2cS3RMi2zCFk2+AsjAmP6ILqyubmDJqYPfxi27Hsb4M05C50SbdcFP056OEAI39v4k1 vPFlnbb9YnHxQ2iFGWhykkBLt8VBbch9y7HcreXB7GrSho5xmolc11jIv+2dPLN0FrsW 6Xf++9soGm7xkfbVY0kuE+74GGNLJB4bySxOFvQQBtXkPES+aj0dHYhtMpzTCXP9fh3g Dmu2jLvN56iCveDKp+nOTXsxvS20PAZIYICPFc7kH6O/u2RdeW+ijWqsPEgtrepfPycX dHaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=cth41kZCcyYbiXvrifaFD6WDIyQSB0f6zaPJXMzyJ4E=; b=kn8PtnbOrRtT6wQWyzx5uVR8oyH3utTHLIMFRPO37isG0MWXViFR03nB7yjj0H4K2G YWgPASRnXk49QbcJua4SNklpnTv0X6Ij1JAVA7qQjj3Qvd4AHro2yN38tFBS97QauYUn pUtarOTcSlZCFY0ZuExPNRf7vrUE85yVWdkRbP0tntN4mBIvL5Ug3KAjowOwA/+79HTD zoCrJ14IWRJ/dkrCzklTyncXfkEAVbEug5bpkD3cZARNF9MgWHhxj64qauHNpIuImkKB 5kPEhLloP5DAh1LZ7oPlbk2cnmhclC3sNhTtCfvk4p0Hjl600hYIPNSXQ6n+6rA/PfqP 8fBw== X-Gm-Message-State: AOAM533EsNypYkxLnd9cBV2+1JI5bgcoZ3tDSRMeSZi2Y2GBgRvK2crC 8knhvQHFUgTTuS2tdXhTpf2P+g== X-Google-Smtp-Source: ABdhPJyp2y7+r7rthWzHmSCiIThMBQLBFTNz6hzRm9cpvYnPyV2NoW6obqzTpr1kYhZk1REirZvzZw== X-Received: by 2002:a5d:6ca6:: with SMTP id a6mr19851863wra.179.1614699383764; Tue, 02 Mar 2021 07:36:23 -0800 (PST) Return-Path: Received: from vanye (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id n186sm2955759wmn.22.2021.03.02.07.36.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Mar 2021 07:36:23 -0800 (PST) Date: Tue, 2 Mar 2021 15:36:21 +0000 From: "Leif Lindholm" To: Marcin Juszkiewicz Cc: Graeme Gregory , devel@edk2.groups.io, Ard Biesheuvel , Radoslaw Biernacki , Tanmay Jagdale , Rebecca Cran Subject: Re: [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib Message-ID: <20210302153621.GB1664@vanye> References: <20210302133828.19321-1-leif@nuviainc.com> <6ba5503b-9686-3218-f79c-b6c8e1e86fde@nuviainc.com> <924c33ae-e7c1-0145-c38d-1e8c7181516f@linaro.org> MIME-Version: 1.0 In-Reply-To: <924c33ae-e7c1-0145-c38d-1e8c7181516f@linaro.org> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 > > > Cc: Graeme Gregory > > > Cc: Radoslaw Biernacki > > > Cc: Tanmay Jagdale > > > Cc: Rebecca Cran > > > Reported-by: Marcin Juszkiewicz > > > Signed-off-by: Leif Lindholm > > > > Tested-By: Graeme Gregory > > Tested-by: Marcin Juszkiewicz > > 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 > > > +**/ > > > +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; > > >     } > > > > > >