From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 ABB5D81E5E for ; Thu, 10 Nov 2016 08:18:24 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 10 Nov 2016 08:18:28 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,619,1473145200"; d="scan'208,217";a="784932897" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by FMSMGA003.fm.intel.com with ESMTP; 10 Nov 2016 08:18:28 -0800 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 10 Nov 2016 08:18:27 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 10 Nov 2016 08:18:27 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.96]) with mapi id 14.03.0248.002; Fri, 11 Nov 2016 00:18:25 +0800 From: "Gao, Liming" To: Felix Poludov , "Kinney, Michael D" , "Yao, Jiewen" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH 0/2] MdePkg: UiLib library Thread-Index: AdI6m0xU2GzkXpsESA6bOeJKPIUocQAWQyagABdz3yAAA/EqMAABgzwwAAED+jAAAB/GcAAAOUBA Date: Thu, 10 Nov 2016 16:18:24 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14B4B083F@shsmsx102.ccr.corp.intel.com> References: <9333E191E0D52B4999CE63A99BA663A00270FF997D@atlms1.us.megatrends.com> <74D8A39837DF1E4DA445A8C0B3885C50386CDB6E@shsmsx102.ccr.corp.intel.com> <9333E191E0D52B4999CE63A99BA663A00270FF9E17@atlms1.us.megatrends.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14B4B07B6@shsmsx102.ccr.corp.intel.com> <9333E191E0D52B4999CE63A99BA663A00270FF9E7D@atlms1.us.megatrends.com> <9333E191E0D52B4999CE63A99BA663A00270FF9EAC@atlms1.us.megatrends.com> In-Reply-To: <9333E191E0D52B4999CE63A99BA663A00270FF9EAC@atlms1.us.megatrends.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDMzYjZlZWUtMDBjZi00YjE5LTk5ODAtMTE3NTkxZDFiOTIxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IklqXC92dVREanpMWDk4N1JMMkFxUlN3MXhOZnplV1FIZXNOVElhWWF3aHpvPSJ9 x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [PATCH 0/2] MdePkg: UiLib library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Nov 2016 16:18:24 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Right. To customize CreatePopup(), new library instance and platform dsc ar= e both updated. Not to customize Createpopup(), there is nothing to do. From: Felix Poludov [mailto:Felixp@ami.com] Sent: Friday, November 11, 2016 12:11 AM To: Kinney, Michael D ; Gao, Liming ; Yao, Jiewen ; edk2-devel@lists.01.org Subject: RE: [PATCH 0/2] MdePkg: UiLib library Yes, but if I understand Liming's proposal correctly, there will be no depe= ndency between UefiLib and UiLib (UefiLib will not use UiLib). It means that just adding a constructor to UiLib will be insufficient. The DSC file will still have to be updated to link either UiLib or a NULL l= ib proposed by Liming with all the CreatePopup consumers. -----Original Message----- From: Kinney, Michael D [mailto:michael.d.kinney@intel.com] Sent: Thursday, November 10, 2016 11:05 AM To: Felix Poludov; Gao, Liming; Yao, Jiewen; edk2-devel@lists.01.org; Kinney, Michael D Subject: RE: [PATCH 0/2] MdePkg: UiLib library Felix, The CONSTRUCTOR for the UiLib could do the registration. Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Felix Poludov > Sent: Thursday, November 10, 2016 7:44 AM > To: Gao, Liming ; Yao, Jiewen > ; edk2- devel@lists.01.org > Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library > > Liming, > > With your proposal if a projects uses RegisterCreatePopUpHandler(), > DSC file has to be updated to link popup handler NULL library with all > the modules that consume CreatePopup(). > Since projects tend to evolve (new modules added, existing code > changes), this sounds like a constant maintenance effort. > Project integrator would have to scan source tree after each commit to > check if a new code that uses CretePopup has been added and if new DSC > file modifications are required. > > Thanks > Felix > > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: Thursday, November 10, 2016 10:10 AM > To: Felix Poludov; Yao, Jiewen; edk2-devel@lists.01.org > Subject: RE: [PATCH 0/2] MdePkg: UiLib library > > Felix: > I think two major concerns are here. > > 1) UefiLib depends on UiLib > > 2) UiLib class is defined in MdePkg. > > To resolve it, I have another idea. > > 1) Introduce new API RegisterCreatePopUpHandler() in UefiLib. Its input > parameter is the handler function that is same to UiCreatePopUpVaList(). > > 2) Update CreatePopup() to use the registered handler if new handler is > registered. If no handler, CreatePopup() still uses current logic. > > 3) If platform wants to customize CreatePopup(), it can provide its libra= ry > instance with library constructor only. Its library class is NULL, its > library > constructor() will consume RegisterCreatePopUpHandler() to register its h= andler. > This solution is compatible. It just updates UefiLib, doesn't need > to update platform DSC files if the platforms don't enable the > customized popup. It doesn't conflict with new UiLib design. New UiLib > class introduce new APIs and update > edk2 codes to use new one. For the driver uses old API, this way can > provide the customization. For the driver uses new API, the different > UiLib instances can provide the customization. > > Thanks > Liming > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Felix Poludov > Sent: Thursday, November 10, 2016 9:38 PM > To: Yao, Jiewen >; > edk2- devel@lists.01.org > Subject: Re: [edk2] [PATCH 0/2] MdePkg: UiLib library > > Hi Jiewen, > > Updating "all the code that uses the old API to use the new API" is > not always possible. > We can certainly update all the EDKII packages; however, it is > trickier with the non-open source packages. > For example, the Reference Code we are getting from a chipset vendor > is using CreatePopUp. > Certainly, if we mark CreatePopUp deprecated (as you have suggested), > the chances are, reference code will get eventually updated not to use th= e function. > However, it might take a significant amount of time. > Moreover, a project integrators that create board firmware by > combining open- source and close-source packages coming from multiple > sources(from multiple vendors), would have to solve this issue again > and again; whereas with my proposal, the issue is solved by upgrading > EDKII code base (no need to touch third party packages). > > Having said that, I do understand your concerns with keeping the > library in MdePkg. > How about finding a middle ground between your proposal and my proposal? > We can keep UiLib instance in MdePkg, but keep only a minimalistic > barebones implementation there. > We can also have another, more advanced, UiLib instance in the MdeModuleP= kg. > This technique is already used in EDKII. > For example, MdePkg defines DebugLib library class and provides > BaseDebugLibSerialPort instance. > MdeModulePkg provides PeiDxeDebugLibReportStatusCode instance for the > same library class. > > Since "advanced" version of Popup is not currently available, there is > no need to create new instance in MdeModulePkg. > However, once Eric is ready with his Popup-on-top-of-HII > implementation, it can be added to a new UiLib instance in MdeModulePkg. > Also, if new functions are added to UiLib in the future, MdePkg UiLib > instance can have a NULL implementation like many other libraries in the = MdePkg. > > What do you think? > > Thanks > Felix > > From: Yao, Jiewen [mailto:jiewen.yao@intel.com] > Sent: Wednesday, November 09, 2016 8:54 PM > To: Felix Poludov; > edk2-devel@lists.01.org > Subject: RE: [PATCH 0/2] MdePkg: UiLib library > > HI Poludov > It is a good idea to have a new UiLib. > > Both Mike and I give the suggestion to "You could define new lib and > update all the code that uses the old API to use the new API." > > IMHO, I do not think it is right way to let UefiLib depend on a UiLib. > We have plan to resolve it, but since you already have proposal to add > UiLib. I hope it can be resolved here. > > There is no real need to put UiLib into MdePkg. MdePkg is designed to > hold EFI/PI speciation or industry standard. > In this case, I believe MdeModulePkg is a better place. > > Thank you > Yao Jiewen > > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > > Of Felix Poludov > > Sent: Wednesday, November 9, 2016 11:44 PM > > To: edk2-devel@lists.01.org > > Subject: [edk2] [PATCH 0/2] MdePkg: UiLib library > > > > This series of two patches introduces new UiLib library. > > 1. New UiLib library class is added to MdePkg. > > The library is intended for functions that produce UI elements. > > The patch introduces two functions that produce a popup window: > > UiCreatePopUp and UiCreatePopUpVaList. > > 2. An instance of UiLib library class is added to MdePkg. > > Popup implementation is copied from UefiLib (CreatePopUp function in > > UefiLib/Console.c). > > 3. MdePkg/UefiLib library is updated to implement CreatePopUp by > > calling UiCreatePopUpVaList. > > 4. Platform DSC files are updated to add UiLib instance. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Felix Polyudov > > > > AppPkg/AppPkg.dsc | 1 + > > ArmPkg/ArmPkg.dsc | 1 + > > BeagleBoardPkg/BeagleBoardPkg.dsc | 1 + > > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 1 + > > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 1 + > > CryptoPkg/CryptoPkg.dsc | 1 + DuetPkg/DuetPkgIa32.dsc | 1 + > > DuetPkg/DuetPkgX64.dsc | 1 + > > EdkCompatibilityPkg/EdkCompatibilityPkg.dsc | 1 + > > EmbeddedPkg/EmbeddedPkg.dsc | 1 + EmulatorPkg/EmulatorPkg.dsc | 1 + > > FatPkg/FatPkg.dsc | 1 + .../IntelFrameworkModulePkg.dsc | 1 + > > IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc | 1 + > > IntelFspWrapperPkg/IntelFspWrapperPkg.dsc | 1 + > > MdeModulePkg/MdeModulePkg.dsc | 1 + MdePkg/Include/Library/UiLib.h | > > 71 +++++ MdePkg/Library/UefiLib/Console.c | 252 > > +--------------- > > MdePkg/Library/UefiLib/UefiLib.inf | 1 + > > MdePkg/Library/UefiLib/UefiLibInternal.h | 1 + > > MdePkg/Library/UiLib/UiLib.c | 334 > > +++++++++++++++++++++ > > MdePkg/Library/UiLib/UiLib.inf | 38 +++ > > MdePkg/Library/UiLib/UiLib.uni | 21 ++ MdePkg/MdePkg.dec | 3 + > > NetworkPkg/NetworkPkg.dsc | 1 + Nt32Pkg/Nt32Pkg.dsc | 1 + > > Omap35xxPkg/Omap35xxPkg.dsc | 1 + OptionRomPkg/OptionRomPkg.dsc | 1 > > + OvmfPkg/OvmfPkgIa32.dsc | 1 + OvmfPkg/OvmfPkgIa32X64.dsc | 1 + > > OvmfPkg/OvmfPkgX64.dsc | 1 + PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 + > > PerformancePkg/PerformancePkg.dsc | 1 + QuarkPlatformPkg/Quark.dsc | > > 1 + QuarkPlatformPkg/QuarkMin.dsc | 1 + QuarkSocPkg/QuarkSocPkg.dsc > > | 1 + SecurityPkg/SecurityPkg.dsc | 1 + ShellPkg/ShellPkg.dsc | 1 + > > SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 + StdLib/StdLib.dsc > > | 1 + UefiCpuPkg/UefiCpuPkg.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 1 + > > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 1 + > > 44 files changed, 506 insertions(+), 251 deletions(-) create mode > > 100644 MdePkg/Include/Library/UiLib.h create mode 100644 > > MdePkg/Library/UiLib/UiLib.c create mode 100644 > > MdePkg/Library/UiLib/UiLib.inf create mode 100644 > > MdePkg/Library/UiLib/UiLib.uni > > > > Please consider the environment before printing this email. > > > > The information contained in this message may be confidential and > > proprietary to American Megatrends, Inc. This communication is > > intended to be read only by the individual or entity to whom it is > > addressed or by their designee. If the reader of this message is not > > the intended recipient, you are on notice that any distribution of > > this message, in any form, is strictly prohibited. Please promptly > > notify the sender by reply e-mail or by telephone at 770-246-8600, > > and then delete or destroy all copies of the transmission. > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > Please consider the environment before printing this email. > > The information contained in this message may be confidential and > proprietary to American Megatrends, Inc. This communication is > intended to be read only by the individual or entity to whom it is > addressed or by their designee. If the reader of this message is not > the intended recipient, you are on notice that any distribution of > this message, in any form, is strictly prohibited. Please promptly > notify the sender by reply e-mail or by telephone at 770-246-8600, and th= en delete or destroy all copies of the transmission. > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > > Please consider the environment before printing this email. > > The information contained in this message may be confidential and > proprietary to American Megatrends, Inc. This communication is > intended to be read only by the individual or entity to whom it is > addressed or by their designee. If the reader of this message is not > the intended recipient, you are on notice that any distribution of > this message, in any form, is strictly prohibited. Please promptly > notify the sender by reply e-mail or by telephone at 770-246-8600, and th= en delete or destroy all copies of the transmission. > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel Please consider the environment before printing this email. The information contained in this message may be confidential and proprieta= ry to American Megatrends, Inc. This communication is intended to be read o= nly by the individual or entity to whom it is addressed or by their designe= e. If the reader of this message is not the intended recipient, you are on = notice that any distribution of this message, in any form, is strictly proh= ibited. Please promptly notify the sender by reply e-mail or by telephone a= t 770-246-8600, and then delete or destroy all copies of the transmission.