From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=siyuan.fu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0E48C21198CDB for ; Sun, 16 Dec 2018 19:22:58 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Dec 2018 19:22:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,363,1539673200"; d="scan'208";a="260006068" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 16 Dec 2018 19:22:57 -0800 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 16 Dec 2018 19:22:57 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 16 Dec 2018 19:22:57 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.59]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.203]) with mapi id 14.03.0415.000; Mon, 17 Dec 2018 11:22:48 +0800 From: "Fu, Siyuan" To: "Gao, Liming" , Laszlo Ersek , Ard Biesheuvel CC: "edk2-devel@lists.01.org" Thread-Topic: [PATCH] ArmVirtPkg/ArmVirt.dsc.inc: define TcpIoLib resolution unconditionally Thread-Index: AQHUk59jIpc+NeMgqkOUMiwNmwGEv6V9vCEAgARuoVD//5LxgIAAipDw Date: Mon, 17 Dec 2018 03:22:48 +0000 Message-ID: References: <20181214112253.10372-1-ard.biesheuvel@linaro.org> <546ee80d-28da-12fa-341b-8850b61aa682@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E38D4C1@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E38D4C1@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTgwZTZlZjAtYjg2Mi00NjE5LWI4ZmQtZDFjOWJmNTI3ZTVmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNXZNVUlBZTVzWnZ4alwvVnFXbzJKTCtjSzY4OXNxdTZFN0xmKzN3Tlk1YUNPK1wvRVwvVmdTYWtYZDdiWXVHOWM5WiJ9 dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] ArmVirtPkg/ArmVirt.dsc.inc: define TcpIoLib resolution unconditionally X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Dec 2018 03:22:59 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks Liming, I will try it. BestRegards Fu Siyuan > -----Original Message----- > From: Gao, Liming > Sent: Monday, December 17, 2018 11:07 AM > To: Fu, Siyuan ; Laszlo Ersek ; A= rd > Biesheuvel > Cc: edk2-devel@lists.01.org > Subject: RE: [PATCH] ArmVirtPkg/ArmVirt.dsc.inc: define TcpIoLib resoluti= on > unconditionally >=20 > Siyuan: > I know the windows GCC pre built binary can be downloaded from > https://sourceforge.net/projects/edk2developertoolsforwindows/files/Tool%= 20Cha > in%20Binaries/. Although pre-built binary GCC is three years ago, they ca= n > still work. For example, after you download gcc492 arm, then you can set > GCC49_ARM_PREFIX and GCC49_DLL to point ARM GCC path, then build ARM plat= form > with -a ARM arch and -t GCC49 tool chain. Below is my setting: > set GCC49_ARM_PREFIX=3DD:\Tools\GCC\gcc492-arm\bin\ > set GCC49_DLL=3DD:\Tools\GCC\gcc492-arm\dll\ >=20 > Thanks > Liming > >-----Original Message----- > >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of F= u, > >Siyuan > >Sent: Monday, December 17, 2018 9:51 AM > >To: Laszlo Ersek ; Ard Biesheuvel > > > >Cc: edk2-devel@lists.01.org > >Subject: Re: [edk2] [PATCH] ArmVirtPkg/ArmVirt.dsc.inc: define TcpIoLib > >resolution unconditionally > > > >Hi, Laszlo > > > >> -----Original Message----- > >> From: Laszlo Ersek [mailto:lersek@redhat.com] > >> Sent: Friday, December 14, 2018 9:56 PM > >> To: Ard Biesheuvel ; Fu, Siyuan > >> > >> Cc: edk2-devel@lists.01.org; julien.grall@linaro.org > >> Subject: Re: [PATCH] ArmVirtPkg/ArmVirt.dsc.inc: define TcpIoLib resol= ution > >> unconditionally > >> > >> On 12/14/18 12:22, Ard Biesheuvel wrote: > >> > Commit 9a67ba261fe9 ("ArmVirtPkg: Replace obsoleted network drivers > >> > from platform DSC/FDF") failed to take into account that the now > >> > unconditionally included IScsiDxe.inf from NetworkPkg requires a > >> > resolution for TcpIoLib. > >> > >> I don't understand why this happened. > >> > >> > >> (a) I warned *precisely* about this issue, when I reviewed the v2 > >> version of said commit. See bullet (5) in the following message: > >> > >> http://mid.mail-archive.com/cdd81f4c-1bdc-8bae-63a9- > >58eb4eb2afbd@redhat.com > >> > >> > >> (b) What's more, my comments for the v3 version were summarily ignored > >> as well. See bullet (2) in: > >> > >> http://mid.mail-archive.com/91d253ae-9d0c-e28b-1bda- > >1be98cee4340@redhat.com > >> > >> And now, the BaseCryptLib, OpensslLib and IntrinsicLib resolutions for > >> [LibraryClasses.common.UEFI_DRIVER] have been added to > >> "ArmVirtPkg/ArmVirtQemuKernel.dsc", despite their being redundant and > >my > >> having pointed out that fact. Worse, "ArmVirtQemuKernel.dsc" uses > >> "OpensslLib.inf", while "OpensslLibCrypto.inf" from "ArmVirt.dsc.inc" = is > >> perfectly sufficient. > >> > >> > >> Commit 9a67ba261fe9 does not carry my R-b, and that's not a random fac= t. > >> The v3 patch was *not* ready for being pushed, to my eyes. And I was > >> pretty explicit about that. > >> > >> > >> > Since specifying such a resolution is harmless > >> > for platforms that have no networking enabled, let's just fix things > >> > by dropping the conditionals around it. > >> > > >> > Contributed-under: TianoCore Contribution Agreement 1.1 > >> > Signed-off-by: Ard Biesheuvel > >> > --- > >> > ArmVirtPkg/ArmVirt.dsc.inc | 2 -- > >> > 1 file changed, 2 deletions(-) > >> > > >> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > >> > index c3549c84d4c6..89c2db074711 100644 > >> > --- a/ArmVirtPkg/ArmVirt.dsc.inc > >> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > >> > @@ -80,9 +80,7 @@ > >> > DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf > >> > UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf > >> > IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf > >> > -!if $(NETWORK_IP6_ENABLE) =3D=3D TRUE > >> > TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf > >> > -!endif > >> > !if $(HTTP_BOOT_ENABLE) =3D=3D TRUE > >> > HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf > >> > !endif > >> > > >> > >> I'm *very strongly* tempted to simply revert 9a67ba261fe9, for blatant= ly > >> ignoring my explicit requests for updates. However, that would only > >> result in my having to review more (possibly incomplete) iterations of > >> the patch. > >> > >> At least, this incremental fix is in line with my request in (a) -- "w= e > >> should make the current TcpIoLib class resolution unconditional". Plea= se > >> go ahead and push it. > >> > >> Reviewed-by: Laszlo Ersek > >> > >> I should really file a TianoCore BZ about the wrong / redundant > >> OpensslLib resolution in ArmVirtQemuKernel.dsc too. It's difficult for > >> me to find te motivation for that right now, seeing the disregard for = my > >> earlier reviews. > >> > >> Laszlo > > > >My apologies for missed your review email of the v3 patch and pushed the > >changes. The original patch set was made one month ago and I didn't care= fully > >checked all the review feedback emails when I started to work on it agai= n > >in the last week. > > > >I have created a new patch upon this fix to remove the redundant librari= es > >and adjust driver order, please check this patch mail. > >https://lists.01.org/pipermail/edk2-devel/2018-December/034070.html > > > >And I'm also sorry that this new patch are also not tested for build, I = tried > >to search the wiki patch for setting up the ARM build toolchain on my > >windows > >OS but failed to make it. I will appreciate if you can help to provide a > guide > >or any link for ARM package build on windows machine, so I could test my > >patch > >in future. > > > >Best Regards, > >Siyuan > > > >_______________________________________________ > >edk2-devel mailing list > >edk2-devel@lists.01.org > >https://lists.01.org/mailman/listinfo/edk2-devel