From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by mx.groups.io with SMTP id smtpd.web12.108126.1597924009094955991 for ; Thu, 20 Aug 2020 04:46:49 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=Wi+n3x2s; spf=pass (domain: nuviainc.com, ip: 209.85.128.66, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f66.google.com with SMTP id p14so1312479wmg.1 for ; Thu, 20 Aug 2020 04:46:48 -0700 (PDT) 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:in-reply-to:user-agent; bh=DmOQMBrHj04kddOwiBY6xwUDXSy/yieY+R9hO4GJK6w=; b=Wi+n3x2s4A8EMVy17frghHMSwgVe+9wmg8w8zHPZFRj4DyREngx72zX6nzjzUBbhdb S/SuWrQC/Jsj5hj4h0c1KuwKmG81Vsw63xWbnvJNqwfPKf1o81/bAoWUBflZ/DH9qNb8 Rvs7kGBdchVm9T11KjB6dQfT+WZp9DfhM0wNmdbhjh7+fONMUsSqahAA64B4i754v382 PYW5gHfwjezl/EtOamhtwx14Pt1gd2wkrMnINpvHGpP1zoLbsOZo1a65hEo7sQ10g7dl 17gtWV4C8N8Rrt1dzmi61jKP7oz50fbaMF0857yU56klN0PBvWT8EHh0j6+lioAAah77 eBYw== 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:in-reply-to:user-agent; bh=DmOQMBrHj04kddOwiBY6xwUDXSy/yieY+R9hO4GJK6w=; b=Z1bqHqdm9gxTZaHkPpMJQ9m85QplUwITjyPYDom2ujr4Ij2r67WbojZVowlRow5s1+ oIM9tJfiGKRXhUWutohsEMF8bkehEhcT0zU8XPUTsCXAw2ivxl3bmX3wn4G315Y0Aizx c2slORHdztIusy5IPPe3VGf6sGTJqjELmkol9/6jMhz+8+BLhnjyMCKTNEQXqq0RDU7e BPIK+WeWtTw7jIZKKxZefwIVpKf9uDVSxgoLbX9ag5jzHh/wEepTq4fHint+1malS6j9 CWAcB2wZTVRGhmma5axlnM4AEVCR/yrJD91rbLE6za+cdkPEUC9m1XGtA1UxHWL3LaEy prNw== X-Gm-Message-State: AOAM530aUFqIBJf4gxYUyZEeX2ubb8DTn/eOzwCmCTLjMdpMZf/a28Ql NuZV7l1QUOI8fDw/PkQaTws+FA== X-Google-Smtp-Source: ABdhPJzqkqbXqckOwSKjddiIIrvXiCTsXC2GuWe6eoha7AAyFEzmYjqlqlWZnRe6zGS4we1lz3XdHQ== X-Received: by 2002:a1c:23c4:: with SMTP id j187mr3226667wmj.58.1597924007426; Thu, 20 Aug 2020 04:46:47 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id o125sm3858305wma.27.2020.08.20.04.46.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Aug 2020 04:46:46 -0700 (PDT) Date: Thu, 20 Aug 2020 12:46:44 +0100 From: "Leif Lindholm" To: Tanmay Jagdale Cc: graeme@nuviainc.com, shashi.mallela@linaro.org, devel@edk2.groups.io, paul.isaacs@linaro.org, tanmay@marvell.com Subject: Re: [PATCH edk2-platforms 3/7] SbsaQemu: Add new ACPI driver and FDT parser to count CPUs Message-ID: <20200820114644.GD1191@vanye> References: <20200819143005.13999-1-tanmay.jagdale@linaro.org> <20200819143005.13999-4-tanmay.jagdale@linaro.org> MIME-Version: 1.0 In-Reply-To: <20200819143005.13999-4-tanmay.jagdale@linaro.org> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Aug 19, 2020 at 20:00:01 +0530, Tanmay Jagdale wrote: > - Add a new ACPI driver for the SbsaQemu platform which would > handle any modifications needed for the ACPI tables. > > - Move FdtLib from LibraryClasses.common.PEIM to LibraryClasses.common > so that SbsaQemuAcpiDxe driver can use the device tree APIs > > - Since the core count is controlled by Qemu, move the PcdCoreCount > from PcdsFixedAtBuild to PcdsDynamic. > > - Add a parser function in this driver which parses the FDT created > by Qemu to determine the number of CPUs and hence update the > PcdCoreCount variable. > > Signed-off-by: Tanmay Jagdale > --- > Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 6 +- > Platform/Qemu/SbsaQemu/SbsaQemu.fdf | 1 + > Silicon/Qemu/SbsaQemu/Acpi.dsc.inc | 1 + > .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 76 +++++++++++++++++++ > .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 49 ++++++++++++ > Silicon/Qemu/SbsaQemu/SbsaQemu.dec | 4 + > 6 files changed, 135 insertions(+), 2 deletions(-) > create mode 100644 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > create mode 100644 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > index 4739443cae93..d42b9cd4de49 100644 > --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > @@ -75,6 +75,7 @@ [LibraryClasses.common] Ah, yes, I see here you *have* set up the diff format (probably using SetupGit.py), so the issue is simply an out-of-date edk2 tree. Do try to keep edk2 as up to date as possible - if edk2 changes causes breakage in the platform ports, I'm not going to take any new patches until those have been fixed anyway. > ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf > FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf > > + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf Why are we adding FdtLib? > UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf > > @@ -217,7 +218,6 @@ [LibraryClasses.common.PEIM] > > PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf > > - FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf Why are we removing FdtLib? > ArmPlatformLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf > > [LibraryClasses.common.DXE_CORE] > @@ -376,7 +376,6 @@ [PcdsFixedAtBuild.common] > # > gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE > > - gArmPlatformTokenSpaceGuid.PcdCoreCount|1 Why are we removing PcdCoreCount? > gArmTokenSpaceGuid.PcdVFPEnabled|1 > > # System Memory Base -- fixed > @@ -477,6 +476,9 @@ [PcdsFixedAtBuild.common] > [PcdsDynamicDefault.common] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 > > + # Core and Cluster Count > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|1 Why are we adding PcdCoreCount? (In short - stop moving things around randomly. If you want to reorganise a file that's fine, but that should be a separate commit without functional changes.) > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount|1 > > # System Memory Size -- 128 MB initially, actual size will be fetched from DT > # TODO as no DT will be used we should pass this by some other method > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > index 4526eaaa02c5..3bcf0bf0040a 100644 > --- a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > @@ -232,6 +232,7 @@ [FV.FvMain] > # > INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf > INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf > + INF Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > INF RuleOverride = ACPITABLE Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf > INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf > > diff --git a/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc > index c4a8d7a27b78..593670383750 100644 > --- a/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc > +++ b/Silicon/Qemu/SbsaQemu/Acpi.dsc.inc > @@ -33,3 +33,4 @@ [Components.common] > MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf > MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf > Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf > + Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > new file mode 100644 > index 000000000000..09e5ba432a59 > --- /dev/null > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > @@ -0,0 +1,76 @@ > +/** @file > +* This file is an ACPI driver for the Qemu SBSA platform. > +* > +* Copyright (c) 2020, Linaro Ltd. All rights reserved. > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* > + * A function that walks through the Device Tree created > + * by Qemu and counts the number of CPUs present in it. > + */ > +STATIC > +VOID > +CountCpusFromFdt ( > + VOID > +) > +{ > + VOID *DeviceTreeBase; > + INT32 Node, Prev; > + RETURN_STATUS PcdStatus; > + INT32 CpuNode; > + INT32 CpuCount; > + > + DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress); > + ASSERT (DeviceTreeBase != NULL); > + > + // Make sure we have a valid device tree blob > + ASSERT (fdt_check_header (DeviceTreeBase) == 0); > + > + CpuNode = fdt_path_offset (DeviceTreeBase, "/cpus"); > + if (CpuNode <= 0) { > + DEBUG((EFI_D_ERROR, "Unable to locate /cpus in device tree\n")); > + return; > + } > + > + CpuCount = 0; > + > + // Walk through /cpus node and count the number of subnodes. > + // The count of these subnodes corresponds to the numer of > + // CPUs created by Qemu. > + Prev = fdt_first_subnode (DeviceTreeBase, CpuNode); > + while (1) { > + CpuCount++; > + Node = fdt_next_subnode (DeviceTreeBase, Prev); > + if (Node < 0) { > + break; > + } > + Prev = Node; > + } > + ASSERT (CpuCount > 0); > + > + PcdStatus = PcdSet32S (PcdCoreCount, CpuCount); > + ASSERT_RETURN_ERROR (PcdStatus); > +} > + > +EFI_STATUS > +EFIAPI > +InitializeSbsaQemuAcpiDxe ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + // Parse the device tree and get the number of CPUs > + CountCpusFromFdt (); > + > + return EFI_SUCCESS; > +} > diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > new file mode 100644 > index 000000000000..efc4d295bfb7 > --- /dev/null > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf > @@ -0,0 +1,49 @@ > +## @file > +# This driver modifies ACPI tables for the Qemu SBSA platform > +# > +# Copyright (c) 2020, Linaro Ltd. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x0001001d > + BASE_NAME = SbsaQemuAcpiDxe > + FILE_GUID = 6c592dc9-76c8-474f-93b2-bf1e8f15ae35 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + > + ENTRY_POINT = InitializeSbsaQemuAcpiDxe > + > +[Sources] > + SbsaQemuAcpiDxe.c > + > +[Packages] > + ArmPkg/ArmPkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec > + ArmVirtPkg/ArmVirtPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + Silicon/Qemu/SbsaQemu/SbsaQemu.dec > + > +[LibraryClasses] > + ArmLib > + BaseMemoryLib > + DebugLib > + BaseLib > + FdtLib > + DxeServicesLib Please sort Above LibraryClasses alphabetically. / Leif > + PcdLib > + UefiDriverEntryPoint > + UefiLib > + UefiRuntimeServicesTableLib > + > +[Pcd] > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress > + > +[Depex] > + TRUE > diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec > index 71ba55a082e2..ed87d15de003 100644 > --- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec > +++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec > @@ -35,3 +35,7 @@ [PcdsFixedAtBuild.common] > gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciBase|0|UINT64|0x00000003 > gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformEhciSize|0x10000|UINT32|0x00000004 > gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress|0x10000000000|UINT64|0x00000005 > + > +[PcdsDynamic.common] > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|0x1|UINT32|0x00000006 > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount|0x1|UINT32|0x00000007 > -- > 2.28.0 >