From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.6147.1585650843786046347 for ; Tue, 31 Mar 2020 03:34:04 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0FAAF30E for ; Tue, 31 Mar 2020 03:34:03 -0700 (PDT) Received: from mail-wr1-f45.google.com (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DB62A3F52E for ; Tue, 31 Mar 2020 03:34:02 -0700 (PDT) Received: by mail-wr1-f45.google.com with SMTP id u10so25241216wro.7 for ; Tue, 31 Mar 2020 03:34:02 -0700 (PDT) X-Gm-Message-State: ANhLgQ3Py0CQoiGQ7qKEYC8ILTXyOZ0eWBznJEclg1tg3fkL8DFpopZe z122GJa/BpeB5vf9FZVl8+sNW/tkkf/zT0f/8EiltA== X-Google-Smtp-Source: ADFU+vusrIOIDuK4aQZF5sHtuoqWZvHHOyHIBg7FTwqBcixbuLhzz+5CwZUwwfXvmG0eLNKpnvBc3auteQSD16eu3VI= X-Received: by 2002:a5d:604a:: with SMTP id j10mr19333346wrt.126.1585650837606; Tue, 31 Mar 2020 03:33:57 -0700 (PDT) MIME-Version: 1.0 References: <20200325105252.13905-1-aditya.angadi@arm.com> <20200325105252.13905-2-aditya.angadi@arm.com> In-Reply-To: <20200325105252.13905-2-aditya.angadi@arm.com> From: "Ard Biesheuvel" Date: Tue, 31 Mar 2020 12:33:46 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-platforms][PATCH v3 1/9] Platform/ARM/SgiPkg: create individual To: Aditya Angadi Cc: edk2-devel-groups-io , Vijayenthiran Subramaniam , Leif Lindholm Content-Type: text/plain; charset="UTF-8" Hello Aditya, Could you please use a meaningful subject line for this patch? On Wed, 25 Mar 2020 at 11:53, Aditya Angadi wrote: > > From: Vijayenthiran Subramaniam > > In preparation for adding support for Reference Design (RD) platforms > that have different base addresses for GIC distributor or redistributor, > create individual platform description files for all SGI/RD platforms > and move GIC related base addresses from the common SGI/RD platform > description file to individual platform description files. > The existing platform description is then included by individual > platform description files. > > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Signed-off-by: Aditya Angadi > --- > Platform/ARM/SgiPkg/Include/SgiPlatform.h | 7 +--- > Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +- > Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c | 8 ++--- > Platform/ARM/SgiPkg/RdE1Edge.dsc | 37 ++++++++++++++++++++ > Platform/ARM/SgiPkg/RdN1Edge.dsc | 37 ++++++++++++++++++++ > Platform/ARM/SgiPkg/Sgi575.dsc | 37 ++++++++++++++++++++ > Platform/ARM/SgiPkg/SgiPlatform.dec | 5 ++- > Platform/ARM/SgiPkg/SgiPlatform.dsc | 25 ++----------- > 8 files changed, 124 insertions(+), 35 deletions(-) > > diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h > index e36a412155ff..d87fb2b5409f 100644 > --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h > +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h > @@ -1,6 +1,6 @@ > /** @file > * > -* Copyright (c) 2018, ARM Limited. All rights reserved. > +* Copyright (c) 2018-2020, ARM Limited. All rights reserved. > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > * > @@ -45,11 +45,6 @@ > #define SGI_SUBSYS_GENERIC_WDOG_BASE 0x2A440000 > #define SGI_SUBSYS_GENERIC_WDOG_SZ SIZE_128KB > > -// Sub System Peripherals - GIC > -#define SGI_SUBSYS_GENERIC_GIC_BASE 0x30000000 > -#define SGI_SUBSYS_GENERIC_GICR_BASE 0x300C0000 > -#define SGI_SUBSYS_GENERIC_GIC_SZ SIZE_1MB > - > // Expansion AXI - Platform Peripherals - HDLCD1 > #define SGI_EXP_PLAT_PERIPH_HDLCD1_BASE 0x7FF60000 > #define SGI_EXP_PLAT_PERIPH_HDLCD1_SZ SIZE_64KB > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf > index 3db70e900d61..a918afef5fba 100644 > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf > @@ -1,5 +1,5 @@ > # > -# Copyright (c) 2018, ARM Limited. All rights reserved. > +# Copyright (c) 2018-2020, ARM Limited. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -42,6 +42,7 @@ [FixedPcd] > > gArmSgiTokenSpaceGuid.PcdDramBlock2Base > gArmSgiTokenSpaceGuid.PcdDramBlock2Size > + gArmSgiTokenSpaceGuid.PcdGicSize > > gArmTokenSpaceGuid.PcdSystemMemoryBase > gArmTokenSpaceGuid.PcdSystemMemorySize > diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c > index 845aeaf4dd49..8d0ad4ec9c84 100644 > --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c > +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c > @@ -1,6 +1,6 @@ > /** @file > * > -* Copyright (c) 2018, ARM Limited. All rights reserved. > +* Copyright (c) 2018-2020, ARM Limited. All rights reserved. > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > * > @@ -93,9 +93,9 @@ ArmPlatformGetVirtualMemoryMap ( > VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > > // Sub System Peripherals - GIC-600 > - VirtualMemoryTable[++Index].PhysicalBase = SGI_SUBSYS_GENERIC_GIC_BASE; > - VirtualMemoryTable[Index].VirtualBase = SGI_SUBSYS_GENERIC_GIC_BASE; > - VirtualMemoryTable[Index].Length = SGI_SUBSYS_GENERIC_GIC_SZ; > + VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64(PcdGicDistributorBase); > + VirtualMemoryTable[Index].VirtualBase = FixedPcdGet64(PcdGicDistributorBase); > + VirtualMemoryTable[Index].Length = FixedPcdGet64(PcdGicSize); Please use a space before '(' > VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > > // Expansion AXI - Platform Peripherals - HDLCD1 > diff --git a/Platform/ARM/SgiPkg/RdE1Edge.dsc b/Platform/ARM/SgiPkg/RdE1Edge.dsc > new file mode 100644 > index 000000000000..082cbb0157f7 > --- /dev/null > +++ b/Platform/ARM/SgiPkg/RdE1Edge.dsc > @@ -0,0 +1,37 @@ > +# > +# Copyright (c) 2020, ARM Limited. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > + > +################################################################################ > +# > +# Defines Section - statements that will be processed to create a Makefile. > +# > +################################################################################ > +[Defines] > + PLATFORM_NAME = ArmSgi > + PLATFORM_GUID = c834de39-c5b0-458b-8ea3-882427179b8a > + PLATFORM_VERSION = 0.1 > + DSC_SPECIFICATION = 0x0001001B > + OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME) > + SUPPORTED_ARCHITECTURES = AARCH64|ARM > + BUILD_TARGETS = NOOPT|DEBUG|RELEASE > + SKUID_IDENTIFIER = DEFAULT > + FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf If you are going to split these into separate DSCs, please do this first in a separate patch, and update the PLATFORM_NAME accordingly. If there is no need to use separate FDFs than you can keep using a shared one. If there is, please do the split in the same patch. > + BUILD_NUMBER = 1 > + > +# include common definitions from SgiPlatform.dsc > +!include Platform/ARM/SgiPkg/SgiPlatform.dsc > + Please rename that file to use a .dsc.inc extension to make it clear this is an include file > +################################################################################ > +# > +# Pcd Section - list of all EDK II PCD Entries defined by this Platform > +# > +################################################################################ > + > +[PcdsFixedAtBuild.common] > + # GIC Base Addresses > + gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000 > + gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000 > + gArmSgiTokenSpaceGuid.PcdGicSize|0x100000 > diff --git a/Platform/ARM/SgiPkg/RdN1Edge.dsc b/Platform/ARM/SgiPkg/RdN1Edge.dsc > new file mode 100644 > index 000000000000..6774990ad6f6 > --- /dev/null > +++ b/Platform/ARM/SgiPkg/RdN1Edge.dsc > @@ -0,0 +1,37 @@ > +# > +# Copyright (c) 2020, ARM Limited. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > + > +################################################################################ > +# > +# Defines Section - statements that will be processed to create a Makefile. > +# > +################################################################################ > +[Defines] > + PLATFORM_NAME = ArmSgi > + PLATFORM_GUID = dbc75915-03df-4640-8f3d-3d3abf7c119b > + PLATFORM_VERSION = 0.1 > + DSC_SPECIFICATION = 0x0001001B > + OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME) > + SUPPORTED_ARCHITECTURES = AARCH64|ARM > + BUILD_TARGETS = NOOPT|DEBUG|RELEASE > + SKUID_IDENTIFIER = DEFAULT > + FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf > + BUILD_NUMBER = 1 > + > +# include common definitions from SgiPlatform.dsc > +!include Platform/ARM/SgiPkg/SgiPlatform.dsc > + > +################################################################################ > +# > +# Pcd Section - list of all EDK II PCD Entries defined by this Platform > +# > +################################################################################ > + > +[PcdsFixedAtBuild.common] > + # GIC Base Addresses > + gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000 > + gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000 > + gArmSgiTokenSpaceGuid.PcdGicSize|0x100000 > diff --git a/Platform/ARM/SgiPkg/Sgi575.dsc b/Platform/ARM/SgiPkg/Sgi575.dsc > new file mode 100644 > index 000000000000..3c1904c2da91 > --- /dev/null > +++ b/Platform/ARM/SgiPkg/Sgi575.dsc > @@ -0,0 +1,37 @@ > +# > +# Copyright (c) 2020, ARM Limited. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > + > +################################################################################ > +# > +# Defines Section - statements that will be processed to create a Makefile. > +# > +################################################################################ > +[Defines] > + PLATFORM_NAME = ArmSgi > + PLATFORM_GUID = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae > + PLATFORM_VERSION = 0.1 > + DSC_SPECIFICATION = 0x0001001B > + OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME) > + SUPPORTED_ARCHITECTURES = AARCH64|ARM > + BUILD_TARGETS = NOOPT|DEBUG|RELEASE > + SKUID_IDENTIFIER = DEFAULT > + FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf > + BUILD_NUMBER = 1 > + > +# include common definitions from SgiPlatform.dsc > +!include Platform/ARM/SgiPkg/SgiPlatform.dsc > + > +################################################################################ > +# > +# Pcd Section - list of all EDK II PCD Entries defined by this Platform > +# > +################################################################################ > + > +[PcdsFixedAtBuild.common] > + # GIC Base Addresses > + gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000 > + gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000 > + gArmSgiTokenSpaceGuid.PcdGicSize|0x100000 > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec > index 9d70ec677776..4ac3dec91e3d 100644 > --- a/Platform/ARM/SgiPkg/SgiPlatform.dec > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec > @@ -1,5 +1,5 @@ > # > -# Copyright (c) 2018, ARM Limited. All rights reserved. > +# Copyright (c) 2018-2020, ARM Limited. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -49,5 +49,8 @@ [PcdsFixedAtBuild] > gArmSgiTokenSpaceGuid.PcdVirtioNetSize|0x00000000|UINT32|0x00000008 > gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt|0x00000000|UINT32|0x00000009 > > + # GIC > + gArmSgiTokenSpaceGuid.PcdGicSize|0|UINT64|0x0000000A Wouldn't a UINT32 be sufficient to describe the size of a MMIO block? > + > [Ppis] > gNtFwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } } > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc > index 5226c5751e98..4e1fcefb1442 100644 > --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc > @@ -1,26 +1,9 @@ > # > -# Copyright (c) 2018, ARM Limited. All rights reserved. > +# Copyright (c) 2018-2020, ARM Limited. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > > -################################################################################ > -# > -# Defines Section - statements that will be processed to create a Makefile. > -# > -################################################################################ > -[Defines] > - PLATFORM_NAME = ArmSgi > - PLATFORM_GUID = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae > - PLATFORM_VERSION = 0.1 > - DSC_SPECIFICATION = 0x0001001B > - OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME) > - SUPPORTED_ARCHITECTURES = AARCH64|ARM > - BUILD_TARGETS = NOOPT|DEBUG|RELEASE > - SKUID_IDENTIFIER = DEFAULT > - FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf > - BUILD_NUMBER = 1 > - > !include Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc > > [BuildOptions] > @@ -93,7 +76,7 @@ [LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION, Libr > > ################################################################################ > # > -# Pcd Section - list of all EDK II PCD Entries defined by this Platform > +# Pcd Section - list of all EDK II PCD Entries common to all SGI/RD platforms > # > ################################################################################ > > @@ -126,10 +109,6 @@ [PcdsFixedAtBuild.common] > gArmTokenSpaceGuid.PcdSystemMemoryBase|0x80000000 > gArmTokenSpaceGuid.PcdSystemMemorySize|0x7F000000 > > - # GIC Base Addresses > - gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000 > - gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000 > - > # > # PCIe > # > -- > 2.17.1 >