From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x234.google.com (mail-io0-x234.google.com [IPv6:2607:f8b0:4001:c06::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 53F6921C9127B for ; Tue, 29 Aug 2017 10:44:07 -0700 (PDT) Received: by mail-io0-x234.google.com with SMTP id n71so24153387iod.1 for ; Tue, 29 Aug 2017 10:46:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=fP29yhqvwQ8jUfISK1Afbn7S8XsXssFLR30N8pH2Qto=; b=J0Z1591iLEJxoxxo2PI7TjXmbmem9yi2fDSUm/XHwJmOoadaByiyzMZPyHURR2G6q3 hZZeRlWjc6VhiR+Nj4oid4PvSLUQHsM9PebCmNnXuxVefngpbJJymeyuZbtcr0MshYii RsDqaNNXclszKfTNwWff9XG1PuqQT2hGn7oN4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fP29yhqvwQ8jUfISK1Afbn7S8XsXssFLR30N8pH2Qto=; b=g2bUBXCjEk/XYum1WZPBXqtw9iOHHV2RPhUZFjCP03WC/WPHMH2NdhcLc6lKUqgt5f kezUyEPogxs8+GSNv41hdKHylkQsPSAGDIbj6OLGzV/+ir5G0mRjgxVLIN0hN1+ZQaKp /wFbEPQhN+VxYW0WE1tLvMFebxM3zPhZMwqUTHpW2+NjfZ0AhiEyyYLqc5JKm8aqY6X5 NjHkhj2tNAb1E5eWZVsFkYF2WYj+LhGXx0VeERelmW9DGp/pMbBxrFRWpubn0eFwp8N2 AwdaqBdI97CsagmOXm9hj4vTHpMYcVSg+QP90VglJ4O25PNTepEzUZECVInS1mtZUB41 anmQ== X-Gm-Message-State: AHYfb5iHy1iZULsaF23nSwQukAalN6RGNAdr1Thl1QE0ZQkffbGojeHk 6Z4i0AaSpNx0+oEQ3tX8Hh9oWOtPw08b X-Received: by 10.36.107.145 with SMTP id v139mr5037849itc.41.1504028807183; Tue, 29 Aug 2017 10:46:47 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Tue, 29 Aug 2017 10:46:46 -0700 (PDT) In-Reply-To: References: <20170829133651.30909-1-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Tue, 29 Aug 2017 18:46:46 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm , "Zhu, Yonghong" , "Gao, Liming" Subject: Re: [PATCH] BaseTools/Gcc ARM AARCH64: add support for building device tree binaries X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Aug 2017 17:44:07 -0000 Content-Type: text/plain; charset="UTF-8" On 29 August 2017 at 18:18, Laszlo Ersek wrote: > On 08/29/17 15:36, Ard Biesheuvel wrote: >> While modern AARCH64 server systems use ACPI for describing the platform >> topology to the OS, ARM systems and AARCH64 outside of the server space >> mostly use device tree binaries, which are compiled from device tree >> source files using the device tree compiler. >> >> Currently, such source files and binaries may be kept in the EDK2 platform >> trees, but are not integrated with the build, which means they need to be >> kept in sync and recompiled manually, which is cumbersome. >> >> So let's wire up BaseTools support for them: add tool definitions for the >> DTC compiler and preprocessor flags that allow these source files to use >> FixedPcd expressions and other macros defined by AutoGen.h >> >> This way, a device tree binary can be built from source and emitted into >> a FFS file automatically using something like: >> >> DeviceTree.inf: >> [Defines] >> INF_VERSION = 0x00010019 >> BASE_NAME = SomePlatformDeviceTree >> FILE_GUID = 25462CDA-221F-47DF-AC1D-259CFAA4E326 # gDtPlatformDefaultDtbFileGuid >> MODULE_TYPE = USER_DEFINED >> VERSION_STRING = 1.0 >> >> [Sources] >> SomePlatform.dts >> >> [Packages] >> MdePkg/MdePkg.dec >> >> SomePlatform.fdf: >> INF RuleOverride = DTB xxx/yyy/DeviceTree.inf >> >> [Rule.Common.USER_DEFINED.DTB] >> FILE FREEFORM = $(NAMED_GUID) { >> RAW BIN |.dtb >> } >> >> where it can be picked at runtime by the DTB loader that may refer to it >> using gDtPlatformDefaultDtbFileGuid. >> >> Note that this is very similar to how ACPI tables may be emitted into a >> FFS file with a known GUID and picked up by AcpiTableDxe at runtime. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> BaseTools/Conf/build_rule.template | 14 ++++++++++++++ >> BaseTools/Conf/tools_def.template | 19 +++++++++++++++++++ >> 2 files changed, 33 insertions(+) > > Any time I have to review a tools_def.template / build_rule.template > patch, I'm in trouble :) > > Some comments, good or bad: > >> diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template >> index 1db94b696f89..2684c8c990a1 100755 >> --- a/BaseTools/Conf/build_rule.template >> +++ b/BaseTools/Conf/build_rule.template >> @@ -238,6 +238,20 @@ >> # For RVCTCYGWIN ASM_FLAGS must be first to work around pathing issues >> "$(ASM)" $(ASM_FLAGS) -o ${dst} ${d_path}(+)${s_base}.iii >> >> +[Device-Tree-Source-File] >> + >> + ?.dts > > Do we care about "?.DTS" files as well? Some other rules spell out both > lower and upper case. > I'm happy to add them if it could be useful. >> + >> + >> + $(MAKE_FILE) >> + >> + >> + $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dtb >> + >> + > > Is there a strong reason to make this GCC toolchain family specific? > > The example I'm looking at is NASM. Is it possible that someone wants to > (cross-build) for ARM without a GCC toolchain? (Of course we can add > that later then.) > The reason is a practical one: while ARM is supported by RVCT as well, nobody actually uses that, and so GCC is the only toolchain I bothered to add. If anyone wants to contribute other toolchain support (and is able to test it), that's fine with me. >> + "$(PP)" $(DTCPP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i > > Seems to match other rules, yes. > >> + "$(DTC)" -I dts -O dtb -o ${dst} ${d_path}(+)${s_base}.i >> + > > Seems good. > >> [Visual-Form-Representation-File] >> >> ?.vfr >> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template >> index 1fa3ca3ceae9..3102488a03d7 100755 >> --- a/BaseTools/Conf/tools_def.template >> +++ b/BaseTools/Conf/tools_def.template >> @@ -227,6 +227,8 @@ DEFINE IPHONE_TOOLS = /Developer/Platforms/iPhoneOS.platform/Develope >> >> DEFINE SOURCERY_CYGWIN_TOOLS = /cygdrive/c/Program Files/CodeSourcery/Sourcery G++ Lite/bin >> >> +DEFINE DTC_BIN = ENV(DTC_PREFIX)dtc >> + > > Call this UNIX_DTC_BIN, for future-proofing? (Could be a terrible > comment, sorry about that.) > Not sure why you would want to configured a single environment for both Windows and Unix, so having UNIX and WINDOWS flavors seems rather pointless to me. > Also, I have zero clue if this is the right location in the file to > define this macro. > >> #################################################################################### >> # >> # format: TARGET_TOOLCHAIN_ARCH_COMMANDTYPE_ATTRIBUTE = >> @@ -4362,6 +4364,7 @@ DEFINE GCC_VFRPP_FLAGS = -x c -E -P -DVFRCOMPILE --include $(DEST_DI >> DEFINE GCC_ASLPP_FLAGS = -x c -E -include AutoGen.h >> DEFINE GCC_ASLCC_FLAGS = -x c >> DEFINE GCC_WINDRES_FLAGS = -J rc -O coff >> +DEFINE GCC_DTCPP_FLAGS = -E -x assembler-with-cpp -imacros $(DEST_DIR_DEBUG)/AutoGen.h -nostdinc > > OK, this makes sense. I tried to see if another (preexistent) define > would be reusable, but nothing seems to. > Nope, not really. dtc barfs on any C style constructs, but we can't use Trim, so using this with -imacros is the most appropriate. >> DEFINE GCC_IA32_RC_FLAGS = -I binary -O elf32-i386 -B i386 --rename-section .data=.hii >> DEFINE GCC_X64_RC_FLAGS = -I binary -O elf64-x86-64 -B i386 --rename-section .data=.hii >> DEFINE GCC_IPF_RC_FLAGS = -I binary -O elf64-ia64-little -B ia64 --rename-section .data=.hii >> @@ -4745,6 +4748,7 @@ RELEASE_GCC45_X64_CC_FLAGS = DEF(GCC45_X64_CC_FLAGS) -Os >> *_GCC46_*_MAKE_PATH = DEF(GCC46_IA32_PREFIX)make >> *_GCC46_*_*_DLL = ENV(GCC46_DLL) >> *_GCC46_*_ASL_PATH = DEF(UNIX_IASL_BIN) >> +*_GCC46_*_DTC_PATH = DEF(DTC_BIN) >> >> *_GCC46_*_PP_FLAGS = DEF(GCC_PP_FLAGS) >> *_GCC46_*_ASLPP_FLAGS = DEF(GCC_ASLPP_FLAGS) >> @@ -4833,6 +4837,7 @@ RELEASE_GCC46_X64_CC_FLAGS = DEF(GCC46_X64_CC_FLAGS) -Os -Wno-unused-but-s >> *_GCC46_ARM_ASM_FLAGS = DEF(GCC46_ARM_ASM_FLAGS) >> *_GCC46_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS) >> *_GCC46_ARM_DLINK2_FLAGS = DEF(GCC46_ARM_DLINK2_FLAGS) >> +*_GCC46_ARM_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC46_ARM_PLATFORM_FLAGS = -march=armv7-a >> *_GCC46_ARM_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC46_ARM_RC_FLAGS = DEF(GCC_ARM_RC_FLAGS) >> @@ -4854,6 +4859,7 @@ RELEASE_GCC46_ARM_CC_FLAGS = DEF(GCC46_ARM_CC_FLAGS) -Wno-unused-but-set-v >> *_GCC47_*_MAKE_PATH = DEF(GCC47_IA32_PREFIX)make >> *_GCC47_*_*_DLL = ENV(GCC47_DLL) >> *_GCC47_*_ASL_PATH = DEF(UNIX_IASL_BIN) >> +*_GCC47_*_DTC_PATH = DEF(DTC_BIN) >> >> *_GCC47_*_PP_FLAGS = DEF(GCC_PP_FLAGS) >> *_GCC47_*_ASLPP_FLAGS = DEF(GCC_ASLPP_FLAGS) >> @@ -4941,6 +4947,7 @@ RELEASE_GCC47_X64_CC_FLAGS = DEF(GCC47_X64_CC_FLAGS) -Os -Wno-unused-but-s >> *_GCC47_ARM_ASM_FLAGS = DEF(GCC47_ARM_ASM_FLAGS) >> *_GCC47_ARM_DLINK_FLAGS = DEF(GCC47_ARM_DLINK_FLAGS) >> *_GCC47_ARM_DLINK2_FLAGS = DEF(GCC47_ARM_DLINK2_FLAGS) >> +*_GCC47_ARM_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC47_ARM_PLATFORM_FLAGS = -march=armv7-a >> *_GCC47_ARM_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC47_ARM_RC_FLAGS = DEF(GCC_ARM_RC_FLAGS) >> @@ -4970,6 +4977,7 @@ RELEASE_GCC47_ARM_CC_FLAGS = DEF(GCC47_ARM_CC_FLAGS) -Wno-unused-but-set-v >> *_GCC47_AARCH64_ASM_FLAGS = DEF(GCC47_AARCH64_ASM_FLAGS) >> *_GCC47_AARCH64_DLINK_FLAGS = DEF(GCC47_AARCH64_DLINK_FLAGS) >> *_GCC47_AARCH64_DLINK2_FLAGS = DEF(GCC47_AARCH64_DLINK2_FLAGS) >> +*_GCC47_AARCH64_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC47_AARCH64_PLATFORM_FLAGS = >> *_GCC47_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC47_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS) >> @@ -4991,6 +4999,7 @@ RELEASE_GCC47_AARCH64_CC_FLAGS = DEF(GCC47_AARCH64_CC_FLAGS) -Wno-unused-but-s >> *_GCC48_*_MAKE_PATH = DEF(GCC48_IA32_PREFIX)make >> *_GCC48_*_*_DLL = ENV(GCC48_DLL) >> *_GCC48_*_ASL_PATH = DEF(UNIX_IASL_BIN) >> +*_GCC48_*_DTC_PATH = DEF(DTC_BIN) >> >> *_GCC48_*_PP_FLAGS = DEF(GCC_PP_FLAGS) >> *_GCC48_*_ASLPP_FLAGS = DEF(GCC_ASLPP_FLAGS) >> @@ -5078,6 +5087,7 @@ RELEASE_GCC48_X64_CC_FLAGS = DEF(GCC48_X64_CC_FLAGS) -Os -Wno-unused-but-s >> *_GCC48_ARM_ASM_FLAGS = DEF(GCC48_ARM_ASM_FLAGS) >> *_GCC48_ARM_DLINK_FLAGS = DEF(GCC48_ARM_DLINK_FLAGS) >> *_GCC48_ARM_DLINK2_FLAGS = DEF(GCC48_ARM_DLINK2_FLAGS) >> +*_GCC48_ARM_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC48_ARM_PLATFORM_FLAGS = -march=armv7-a >> *_GCC48_ARM_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC48_ARM_RC_FLAGS = DEF(GCC_ARM_RC_FLAGS) >> @@ -5107,6 +5117,7 @@ RELEASE_GCC48_ARM_CC_FLAGS = DEF(GCC48_ARM_CC_FLAGS) -Wno-unused-but-set-v >> *_GCC48_AARCH64_ASM_FLAGS = DEF(GCC48_AARCH64_ASM_FLAGS) >> *_GCC48_AARCH64_DLINK_FLAGS = DEF(GCC48_AARCH64_DLINK_FLAGS) >> *_GCC48_AARCH64_DLINK2_FLAGS = DEF(GCC48_AARCH64_DLINK2_FLAGS) >> +*_GCC48_AARCH64_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC48_AARCH64_PLATFORM_FLAGS = >> *_GCC48_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC48_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS) >> @@ -5128,6 +5139,7 @@ RELEASE_GCC48_AARCH64_CC_FLAGS = DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s >> *_GCC49_*_MAKE_PATH = DEF(GCC49_IA32_PREFIX)make >> *_GCC49_*_*_DLL = ENV(GCC49_DLL) >> *_GCC49_*_ASL_PATH = DEF(UNIX_IASL_BIN) >> +*_GCC49_*_DTC_PATH = DEF(DTC_BIN) >> >> *_GCC49_*_PP_FLAGS = DEF(GCC_PP_FLAGS) >> *_GCC49_*_ASLPP_FLAGS = DEF(GCC_ASLPP_FLAGS) >> @@ -5215,6 +5227,7 @@ RELEASE_GCC49_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -Os -Wno-unused-but-s >> *_GCC49_ARM_ASM_FLAGS = DEF(GCC49_ARM_ASM_FLAGS) >> *_GCC49_ARM_DLINK_FLAGS = DEF(GCC49_ARM_DLINK_FLAGS) >> *_GCC49_ARM_DLINK2_FLAGS = DEF(GCC49_ARM_DLINK2_FLAGS) >> +*_GCC49_ARM_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC49_ARM_PLATFORM_FLAGS = -march=armv7-a >> *_GCC49_ARM_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC49_ARM_RC_FLAGS = DEF(GCC_ARM_RC_FLAGS) >> @@ -5243,6 +5256,7 @@ RELEASE_GCC49_ARM_CC_FLAGS = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v >> *_GCC49_AARCH64_ASLDLINK_FLAGS = DEF(GCC49_AARCH64_ASLDLINK_FLAGS) >> *_GCC49_AARCH64_ASM_FLAGS = DEF(GCC49_AARCH64_ASM_FLAGS) >> *_GCC49_AARCH64_DLINK2_FLAGS = DEF(GCC49_AARCH64_DLINK2_FLAGS) >> +*_GCC49_AARCH64_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC49_AARCH64_PLATFORM_FLAGS = >> *_GCC49_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC49_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS) >> @@ -5271,6 +5285,7 @@ RELEASE_GCC49_AARCH64_DLINK_FLAGS = DEF(GCC49_AARCH64_DLINK_FLAGS) >> *_GCC5_*_MAKE_PATH = DEF(GCC5_IA32_PREFIX)make >> *_GCC5_*_*_DLL = ENV(GCC5_DLL) >> *_GCC5_*_ASL_PATH = DEF(UNIX_IASL_BIN) >> +*_GCC5_*_DTC_PATH = DEF(DTC_BIN) >> >> *_GCC5_*_PP_FLAGS = DEF(GCC_PP_FLAGS) >> *_GCC5_*_ASLPP_FLAGS = DEF(GCC_ASLPP_FLAGS) >> @@ -5363,6 +5378,7 @@ RELEASE_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os >> *_GCC5_ARM_ASLDLINK_FLAGS = DEF(GCC5_ARM_ASLDLINK_FLAGS) >> *_GCC5_ARM_ASM_FLAGS = DEF(GCC5_ARM_ASM_FLAGS) >> *_GCC5_ARM_DLINK2_FLAGS = DEF(GCC5_ARM_DLINK2_FLAGS) >> +*_GCC5_ARM_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC5_ARM_PLATFORM_FLAGS = -march=armv7-a >> *_GCC5_ARM_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC5_ARM_RC_FLAGS = DEF(GCC_ARM_RC_FLAGS) >> @@ -5396,6 +5412,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS >> *_GCC5_AARCH64_ASLDLINK_FLAGS = DEF(GCC5_AARCH64_ASLDLINK_FLAGS) >> *_GCC5_AARCH64_ASM_FLAGS = DEF(GCC5_AARCH64_ASM_FLAGS) >> *_GCC5_AARCH64_DLINK2_FLAGS = DEF(GCC5_AARCH64_DLINK2_FLAGS) >> +*_GCC5_AARCH64_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> *_GCC5_AARCH64_PLATFORM_FLAGS = >> *_GCC5_AARCH64_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) >> *_GCC5_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS) >> @@ -5425,12 +5442,14 @@ RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(W >> *_CLANG35_*_MAKE_PATH = make >> *_CLANG35_*_*_DLL = ENV(CLANG35_DLL) >> *_CLANG35_*_ASL_PATH = DEF(UNIX_IASL_BIN) >> +*_CLANG35_*_DTC_PATH = DEF(DTC_BIN) >> >> *_CLANG35_*_PP_FLAGS = DEF(GCC_PP_FLAGS) >> *_CLANG35_*_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) >> *_CLANG35_*_APP_FLAGS = >> *_CLANG35_*_ASL_FLAGS = DEF(IASL_FLAGS) >> *_CLANG35_*_ASL_OUTFLAGS = DEF(IASL_OUTFLAGS) >> +*_CLANG35_*_DTCPP_FLAGS = DEF(GCC_DTCPP_FLAGS) >> >> *_CLANG35_*_CC_PATH = ENV(CLANG35_BIN)clang >> *_CLANG35_*_ASM_PATH = ENV(CLANG35_BIN)clang >> > > I asked some idea-questions to hide how confused I generally am about > these two files. > > Acked-by: Laszlo Ersek > Thanks!