From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from atlmailgw1.ami.com (atlmailgw1.ami.com [63.147.10.40]) (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 E154781E56 for ; Thu, 10 Nov 2016 08:11:16 -0800 (PST) X-AuditID: ac1060b2-a6bff7000000467b-03-58249c243953 Received: from atlms2.us.megatrends.com (atlms2.us.megatrends.com [172.16.96.152]) (using TLS with cipher AES128-SHA (128/128 bits)) (Client did not present a certificate) by (Symantec Messaging Gateway) with SMTP id 8C.34.18043.42C94285; Thu, 10 Nov 2016 11:11:19 -0500 (EST) Received: from ATLMS1.us.megatrends.com ([fe80::8c55:daf0:ef05:5605]) by atlms2.us.megatrends.com ([fe80::29dc:a91e:ea0c:cdeb%12]) with mapi id 14.03.0123.003; Thu, 10 Nov 2016 11:11:15 -0500 From: Felix Poludov To: "Kinney, Michael D" , "Gao, Liming" , "Yao, Jiewen" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH 0/2] MdePkg: UiLib library Thread-Index: AdI6m0xU2GzkXpsESA6bOeJKPIUocQAWQyagABdz3yAAA/EqMAABgzwwAAED+jAAAB/GcA== Date: Thu, 10 Nov 2016 16:11:15 +0000 Message-ID: <9333E191E0D52B4999CE63A99BA663A00270FF9EAC@atlms1.us.megatrends.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> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.16.99.93] content-transfer-encoding: quoted-printable MIME-Version: 1.0 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprBKsWRmVeSWpSXmKPExsWyRiBhhq76HJUIg447QhZ7Dh1ltlj30cNi xb0N7BYdHf+YHFg8Fu95yeTRPfsfSwBTVAOjTWJeXn5JYkmqQkpqcbKtUkBRZllicqWSQmaK rZKhkkJBTmJyam5qXomtUmJBQWpeipIdlwIGsAEqy8xTSM1Lzk/JzEu3VfIM9te1sDC11DVU sgvJSFXIzEvLL8pNLMnMz1NIzs8rAapOTQGKKiR0cWb8OTeFvaA7rmLx4XPMDYw3fbsYOTkk BEwkZkw9xtLFyMUhJDCbSWLf8+VQzmFGiUMTrzGDVLEJqEhsOnuBGSQhIrCTUWLP3bssIAlh AT2JU5evsYPYIgL6EsuWTGWDsMMkehftYQSxWQRUJXpnbmQCsXkFAiX27dvLCrFhP7PE1KYH rCAJToEQiaYDi8AaGAXEJL6fWgPWwCwgLnHryXwmiFsFJJbsOc8MYYtKvHz8jxXCVpDY8r6T HaJeR2LB7k9sELa2xLKFr5khFgtKnJz5hGUCo8gsJGNnIWmZhaRlFpKWBYwsqxiFEktychMz c9LLDfUSczP1kvNzNzFCEsKmHYwtF80PMQpwMCrx8Ir1qkQIsSaWFVfmHmKU4GBWEuF9PwEo xJuSWFmVWpQfX1Sak1p8iNEJGC4TmaW4QbEEjPZ4YwMDKVEYx9DEzMTcyNzQ0sTc2FhJnLfo j2+4kEA6MPlkp6YWpBbBDGHi4JRqYFym+Vs46KPAF5aZF60fdl22st59aOadsIYXd1rZszhz he4cdXW7IlTd4mhX2nreoeuPw+nwC6k8r3hcX7UfuaF+7J1jZNjKg397a5IkH6+aH9fGm/Bi axrbX+dPYot3dqrc7jaZNzf5peTRmXMmnWx+tuqqf7AgT2WahTf/ut+9ay4V7dJq/qfEUpyR aKjFXFScCADZrnHlKwMAAA== 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:11:17 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Yes, but if I understand Liming's proposal correctly, there will be no depen= dency 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 li= b 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 in= put > 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= library > instance with library constructor only. Its library class is NULL, its > library > constructor() will consume RegisterCreatePopUpHandler() to register its ha= ndler. > 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 the= 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 MdeModulePk= g. > 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 M= dePkg. > > 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 the= n 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 the= n 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 proprietar= y to American Megatrends, Inc. This communication is intended to be read on= ly 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 not= ice that any distribution of this message, in any form, is strictly prohibit= ed. Please promptly notify the sender by reply e-mail or by telephone at 77= 0-246-8600, and then delete or destroy all copies of the transmission.