From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0b-002e3701.pphosted.com (mx0b-002e3701.pphosted.com [148.163.143.35]) by mx.groups.io with SMTP id smtpd.web12.2037.1571138052619360504 for ; Tue, 15 Oct 2019 04:14:12 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: hpe.com, ip: 148.163.143.35, mailfrom: prvs=019184da0c=abner.chang@hpe.com) Received: from pps.filterd (m0134425.ppops.net [127.0.0.1]) by mx0b-002e3701.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id x9FBBcuC004436; Tue, 15 Oct 2019 11:14:09 GMT Received: from g4t3426.houston.hpe.com (g4t3426.houston.hpe.com [15.241.140.75]) by mx0b-002e3701.pphosted.com with ESMTP id 2vn8e44uua-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 15 Oct 2019 11:14:09 +0000 Received: from G2W6311.americas.hpqcorp.net (g2w6311.austin.hp.com [16.197.64.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by g4t3426.houston.hpe.com (Postfix) with ESMTPS id 618ED5C; Tue, 15 Oct 2019 11:14:08 +0000 (UTC) Received: from G9W8672.americas.hpqcorp.net (16.220.49.31) by G2W6311.americas.hpqcorp.net (16.197.64.53) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Tue, 15 Oct 2019 11:13:51 +0000 Received: from G4W10204.americas.hpqcorp.net (2002:10cf:5210::10cf:5210) by G9W8672.americas.hpqcorp.net (2002:10dc:311f::10dc:311f) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Tue, 15 Oct 2019 11:13:51 +0000 Received: from NAM03-DM3-obe.outbound.protection.outlook.com (15.241.52.10) by G4W10204.americas.hpqcorp.net (16.207.82.16) with Microsoft SMTP Server (TLS) id 15.0.1367.3 via Frontend Transport; Tue, 15 Oct 2019 11:13:51 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nRE0DVhyOjzRqtKWZ0mpWqRShieHLQbFxs0sm/jLtxRWxKsp1AU6Cedo3t936KK2G8G9CKMFkcUOfjvC0dAMEzhssDOQtqjnzXRzrnuxiVWpT/pR0QjnHUZVQv/IvcYDUmyShvEQkJoQpx2ph+9LAw2IUOtBmhdgFcduNPd66e2iHVanik5z9fNHyGdao9YYGR4zP+tdzWNSYhMDyEZCaME9EJihK/IHaatWn8dklWszv5DMnl8Aymxtmyg1bdX3h2mRxzdYbuJrZbS96PufNbapTBYLUUxT+le8NKA9ExPLeZoXjwhA5URP9IP6jSrRN+dBRCSU71rFFNbGaAiK/g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cQL0+NUVdJV7aj3SECN0rp5jjYFqZtjTWEeTm4q42rA=; b=KSG3fVUDIHSFyBXCVRKeJdwRJtvOYeheBv3pfjAdCVY6NBQJvGAjDyISltCrQbFlwgYCtpII/uPsmnCfJUHtKkr/Dwv0uGPR1qGJdAugHrA1+HcHtjmPP496r2uP/VNHLdB/FS5WX8H5hbCaV3uZzKrcqCxcrfwYjFNDLl4jtNEnD01dIsrhquh1ibHP+AcLlWLQOpIrKXLNyVKMGTnqNu1r+ZBdQcV+m5QVSeY2G2or9Pt2VSqy8LThCVlAql7Bh38k4+dW2jKPAQGxZlrmMpIrBeLSEqqXvxkDpH+xmOtaidQax+c/Clip7vBR2st8GZoYC6VuwKq8nA6VLY6r4Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=hpe.com; dmarc=pass action=none header.from=hpe.com; dkim=pass header.d=hpe.com; arc=none Received: from CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM (10.169.12.151) by CS1PR8401MB0709.NAMPRD84.PROD.OUTLOOK.COM (10.169.15.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2347.18; Tue, 15 Oct 2019 11:13:49 +0000 Received: from CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM ([fe80::4fb:84b9:76e6:1cde]) by CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM ([fe80::4fb:84b9:76e6:1cde%8]) with mapi id 15.20.2347.023; Tue, 15 Oct 2019 11:13:49 +0000 From: "Abner Chang" To: "devel@edk2.groups.io" , "leif.lindholm@linaro.org" Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 24/29] BaseTools: BaseTools changes for RISC-V platform. Thread-Topic: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 24/29] BaseTools: BaseTools changes for RISC-V platform. Thread-Index: AQHVcaqxRb5T/rOTfk+2Gike4ULAv6c+iuAAgBzDYNCAAFzvAIAAAxng Date: Tue, 15 Oct 2019 11:13:49 +0000 Message-ID: References: <1569198715-31552-1-git-send-email-abner.chang@hpe.com> <1569198715-31552-26-git-send-email-abner.chang@hpe.com> <20190926220946.GV28454@bivouac.eciton.net> <20191015105656.GK25504@bivouac.eciton.net> In-Reply-To: <20191015105656.GK25504@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [16.242.247.131] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 45cedb5d-72b1-41ea-fe43-08d75160c165 x-ms-office365-filtering-ht: Tenant x-ms-traffictypediagnostic: CS1PR8401MB0709: x-ms-exchange-purlcount: 1 x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:235; x-forefront-prvs: 01917B1794 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(136003)(376002)(346002)(366004)(396003)(39860400002)(189003)(199004)(13464003)(55016002)(8936002)(11346002)(446003)(9686003)(6306002)(5660300002)(30864003)(966005)(6116002)(3846002)(14454004)(71200400001)(71190400001)(52536014)(478600001)(229853002)(76176011)(64756008)(66946007)(186003)(81166006)(8676002)(6436002)(2906002)(256004)(14444005)(33656002)(26005)(66556008)(66476007)(66446008)(86362001)(25786009)(74316002)(476003)(66066001)(6246003)(7736002)(305945005)(486006)(81156014)(102836004)(6506007)(2501003)(7696005)(19627235002)(316002)(110136005)(99286004)(53546011)(76116006);DIR:OUT;SFP:1102;SCL:1;SRVR:CS1PR8401MB0709;H:CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; received-spf: None (protection.outlook.com: hpe.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: MXT3ch/UET4bvm6uD4pyfOVb8n7FY6wYs4+onKj55fllq9kvkt0q46u9E31PFCtDFljSfL/0y/UqK5UgQZO0IQAoEefppjVt9t3XClYIhoz1brJXnG9/mso87Z5aZu4ifECjNboS2novqw0BW0HytrYskY85SWvdbAZ4ylNsY10x72rzjfzeTSSsDkE6K42qvXSw+VppF4o2T/egcatFUpA7LLlS7hfobOVA+OBkihkR26jBvmHItmLWe5WKtHrG0/adsWVIcQ6jUxjJZIwh4Tth9CFPEeBTt8YigYzjyH6FVbxBNEtqgleVGJ3oCARuV4NkUIM40L0VZpXxAbYsw2jqpKpmSczm5UyKdcctB6jhIiq6oQ5/VhiZseh8rJJHKePT1XrQwqHpkFYndYVCbBRIQYFUHEYvecj8g9pTMkWWrlNS/QkxSa4zZ8P41Xw/8NkjAjYKHRYK2RsG5RsE5g== x-ms-exchange-transport-forked: True X-MS-Exchange-CrossTenant-Network-Message-Id: 45cedb5d-72b1-41ea-fe43-08d75160c165 X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Oct 2019 11:13:49.8312 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: /Xz1qZBmYoXg6VKkl8mpulSqncKTTr/L5doE3Xjh0aO50APjiM/5PA5X9qi3Kf8hBo86L+dn+qonJZCuvaElUA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR8401MB0709 X-OriginatorOrg: hpe.com X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-HPE-SCL: -1 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,1.0.8 definitions=2019-10-15_05:2019-10-15,2019-10-15 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 suspectscore=0 impostorscore=0 lowpriorityscore=0 phishscore=0 spamscore=0 bulkscore=0 clxscore=1015 mlxlogscore=999 malwarescore=0 priorityscore=1501 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-1908290000 definitions=main-1910150104 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Leif Lindholm > Sent: Tuesday, October 15, 2019 6:57 PM > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) > > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 24/29] > BaseTools: BaseTools changes for RISC-V platform. >=20 > On Tue, Oct 15, 2019 at 06:18:29AM +0000, Abner Chang wrote: > > > -----Original Message----- > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf > > > Of Leif Lindholm > > > Sent: Friday, September 27, 2019 6:10 AM > > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) > > > > > > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 24/29] > > > BaseTools: BaseTools changes for RISC-V platform. > > > > > > On Mon, Sep 23, 2019 at 08:31:50AM +0800, Abner Chang wrote: > > > > BaseTools changes for building EDK2 RISC-V platform. > > > > The changes made to build_rule.template is to avoid build errors > > > > cause by GCC711RISCV tool chain. > > > > > > Thank you, this is much cleaner. > > > There are however some issues in this patch that prevent building on > > > any platform. Please ensure to give a local build test before > > > submitting a 3. > > > > > > First of all, this still does not contain the addition to > > > BaseTools/Source/Python/Common/buildoptions.py that I mentioned in > > > INVALID URI REMOVED > > > > 3A__edk2.groups.io_g_devel_message_47036&d=3DDwIBAg&c=3DC5b8zRQO1mi > > > > GmBeVZ2LFWg&r=3D_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=3D > > > YclXVT- > > > > dumczX_RwFNv_GDdWAp1gvJXUN0KRfNaGEtw&s=3DGp1kHhT9Z6PR93PmPN > > > ZD-_0h0rPDXLsODbhLWyQs8NA&e=3D - meaning that attempting to build > > > anything for RISCV64 gives an error. > > > > I thought you were saying to use ENV(GCC5_RISCV64_PREFIX) to point to > > build tool binaries, no? >=20 > That is unrelated. >=20 > I am talking about that the build command needs to be aware of the > existence of the RISCV64 architecture. The version of the build command > included in v2 cannot be used to build the tree. Just like I commented o= n, > and provided the patch for, for v1. In the message linked to above. >=20 > How *have* you tested the build without a working build command? >=20 > > > > > > Other minor issues reviewed inline: > > > > > > > Signed-off-by: Abner Chang > > > > --- > > > > BaseTools/Conf/build_rule.template | 62 ++--- > > > > BaseTools/Conf/tools_def.template | 64 ++++- > > > > BaseTools/Source/C/Common/BasePeCoff.c | 15 +- > > > > BaseTools/Source/C/Common/PeCoffLoaderEx.c | 95 ++++++++ > > > > BaseTools/Source/C/GenFv/GenFvInternalLib.c | 128 ++++++++= +- > > > > BaseTools/Source/C/GenFw/Elf32Convert.c | 5 +- > > > > BaseTools/Source/C/GenFw/Elf64Convert.c | 260 > > > ++++++++++++++++++++- > > > > BaseTools/Source/C/GenFw/elf_common.h | 62 +++++ > > > > .../Source/C/Include/IndustryStandard/PeImage.h | 6 + > > > > BaseTools/Source/Python/Common/DataType.py | 7 +- > > > > 10 files changed, 659 insertions(+), 45 deletions(-) > > > > > > > > diff --git a/BaseTools/Conf/build_rule.template > > > b/BaseTools/Conf/build_rule.template > > > > index db06d3a..fab3926 100755 > > > > --- a/BaseTools/Conf/build_rule.template > > > > +++ b/BaseTools/Conf/build_rule.template > > > > @@ -321,6 +314,21 @@ > > > > "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst} > > > > > > > > > > > > +[Static-Library-File.COMMON.RISCV64, Static-Library- > > > File.COMMON.RISCV32] > > > > + > > > > + *.lib > > > > + > > > > + > > > > + $(MAKE_FILE) > > > > + > > > > + > > > > + $(DEBUG_DIR)(+)$(MODULE_NAME).dll > > > > + > > > > + > > > > + "$(DLINK)" -o ${dst} $(DLINK_FLAGS) --start-group > > > > + $(DLINK_SPATH) > > > @$(STATIC_LIBRARY_FILES_LIST) --end-group $(DLINK2_FLAGS) > > > > > > This line looks to me like the only thing that is actually changed > > > here, and I am not convinced it is necessary. > > > "$(DLINK)" -o ${dst} $(DLINK_FLAGS) -Wl,--start- > > > group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS) > > > $(DLINK2_FLAGS) > > > > > > On the ARM/AARCH64 side, we use gcc as the DLINK, and pass the > > > required flags through to the linker with -Wl. Please have a look > > > and try to rework at that end rather than fundamentally revamping > > > the basic build rules differently for RISCV than other architectures= . > > > > > > Basically, please discard all changes to this file, apply the below > > > diff, and rework the flags to resolve the builds. (Basically, add a > > > bunch of -Wl,) > > > > I got build error when use -Wl with the specific version of RISC-V GCC > > toolchain (the old and workable one). I will revisit this when I > > investigate the issue caused by latest RISC-V build tool. >=20 > OK. >=20 > > > > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c > > > b/BaseTools/Source/C/GenFw/Elf64Convert.c > > > > index 3d6319c..2aa09fd 100644 > > > > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c > > > > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c > > > > @@ -3,6 +3,7 @@ Elf64 convert solution > > > > > > > > Copyright (c) 2010 - 2018, Intel Corporation. All rights > > > > reserved.
Portions copyright (c) 2013-2014, ARM Ltd. All > > > > rights reserved.
> > > > +Portions Copyright (c) 2016 - 2019 Hewlett Packard Enterprise > > > Development LP. All rights reserved.
> > > > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > @@ -31,6 +32,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > > > #include "ElfConvert.h" > > > > #include "Elf64Convert.h" > > > > > > > > +#define RV_X(x, s, n) (((x) >> (s)) & ((1<<(n))-1)) #define > > > > +RISCV_IMM_BITS 12 #define RISCV_IMM_REACH > (1LL< > > > +#define RISCV_CONST_HIGH_PART(VALUE) \ > > > > + (((VALUE) + (RISCV_IMM_REACH/2)) & ~(RISCV_IMM_REACH-1)) > > > > + > > > > STATIC > > > > VOID > > > > ScanSections64 ( > > > > @@ -153,8 +160,8 @@ InitializeElf64 ( > > > > Error (NULL, 0, 3000, "Unsupported", "ELF e_type not ET_EXEC > > > > or > > > ET_DYN"); > > > > return FALSE; > > > > } > > > > - if (!((mEhdr->e_machine =3D=3D EM_X86_64) || (mEhdr->e_machine = = =3D=3D > > > EM_AARCH64))) { > > > > - Error (NULL, 0, 3000, "Unsupported", "ELF e_machine not > EM_X86_64 or > > > EM_AARCH64"); > > > > + if (!((mEhdr->e_machine =3D=3D EM_X86_64) || (mEhdr->e_machine = = =3D=3D > > > EM_AARCH64) || (mEhdr->e_machine =3D=3D EM_RISCV64))) { > > > > + Error (NULL, 0, 3000, "Unsupported", "ELF e_machine is not > > > > + Elf64 > > > machine."); > > > > return FALSE; > > > > } > > > > if (mEhdr->e_version !=3D EV_CURRENT) { @@ -481,6 +488,7 @@ > > > > ScanSections64 ( > > > > switch (mEhdr->e_machine) { > > > > case EM_X86_64: > > > > case EM_AARCH64: > > > > + case EM_RISCV64: > > > > mCoffOffset +=3D sizeof (EFI_IMAGE_NT_HEADERS64); > > > > break; > > > > default: > > > > @@ -690,6 +698,11 @@ ScanSections64 ( > > > > NtHdr->Pe32Plus.FileHeader.Machine =3D > > > EFI_IMAGE_MACHINE_AARCH64; > > > > NtHdr->Pe32Plus.OptionalHeader.Magic =3D > > > EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC; > > > > break; > > > > + case EM_RISCV64: > > > > + NtHdr->Pe32Plus.FileHeader.Machine =3D > > > EFI_IMAGE_MACHINE_RISCV64; > > > > + NtHdr->Pe32Plus.OptionalHeader.Magic =3D > > > EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC; > > > > + break; > > > > + > > > > default: > > > > VerboseMsg ("%s unknown e_machine type. Assume X64", > > > (UINTN)mEhdr->e_machine); > > > > NtHdr->Pe32Plus.FileHeader.Machine =3D EFI_IMAGE_MACHINE_X64; > > > > @@ -769,6 +782,11 @@ WriteSections64 ( > > > > Elf_Shdr *SecShdr; > > > > UINT32 SecOffset; > > > > BOOLEAN (*Filter)(Elf_Shdr *); > > > > + UINT32 Value; > > > > + UINT32 Value2; > > > > + UINT8 *Pass1Targ =3D NULL; > > > > + Elf_Shdr *Pass1Sym =3D NULL; > > > > + Elf64_Half Pass1SymSecIndex =3D 0; > > > > Elf64_Addr GOTEntryRva; > > > > > > > > // > > > > @@ -893,13 +911,14 @@ WriteSections64 ( > > > > if (SymName =3D=3D NULL) { > > > > SymName =3D (const UINT8 *)""; > > > > } > > > > + if (mEhdr->e_machine !=3D EM_RISCV64) { > > > > > > This needs a comment explaining why this does not apply to RISCV. > > > > > > > > > + Error (NULL, 0, 3000, "Invalid", > > > > + "%s: Bad definition for symbol '%s'@%#llx or > > > > + unsupported > > > symbol type. " > > > > + "For example, absolute and undefined symbols > > > > + are not > > > supported.", > > > > + mInImageName, SymName, Sym->st_value); > > > > > > > > - Error (NULL, 0, 3000, "Invalid", > > > > - "%s: Bad definition for symbol '%s'@%#llx or uns= upported > symbol > > > type. " > > > > - "For example, absolute and undefined symbols are= not > > > supported.", > > > > - mInImageName, SymName, Sym->st_value); > > > > - > > > > - exit(EXIT_FAILURE); > > > > + exit(EXIT_FAILURE); > > > > + } > > > > } > > > > SymShdr =3D GetShdrByIndex(Sym->st_shndx); > > > > > > > > @@ -1114,6 +1133,128 @@ WriteSections64 ( > > > > default: > > > > Error (NULL, 0, 3000, "Invalid", "WriteSections64(): > > > > %s unsupported > > > ELF EM_AARCH64 relocation 0x%x.", mInImageName, (unsigned) > > > ELF_R_TYPE(Rel->r_info)); > > > > } > > > > + } else if (mEhdr->e_machine =3D=3D EM_RISCV64) { > > > > > > Yeah, this code block is just *waaaay* too big. > > > Please break it out into its own helper function. > > > > Leif, I am not going to address this issue this time. I just follow > > what other archs done in this function. I agree with you this > > function is way too long. I could create a task to refine this > > function once RISC-V part is reviewed and pushed to the mainstream. >=20 > I don't understand this logic. > Breaking this out into a helper function would take no more time than yo= u > spent on typing the above response that you don't intend to do. Come on man~ >=20 > Yes, the code for the other architectures is also bad, and we should rev= isit > and fix it. But that doesn't mean we should keep making the file worse. >=20 > I am already giving a pass on how even if you break this hunk out into i= ts own > helper function, that one is itself way too long and needs to be broken = up. > But at least if we move it out, we've compartmentalised the problem. >=20 > Merging something bad in order to fix it later is never the answer. > (And not only because in 95% in cases, that later never happens.) I totally understand what is the best engineer practice... Yes. I know you won't let this code to get into staging if I don't fix thi= s. You win~ :) >=20 > Best Regards, >=20 > Leif >=20 >=20