From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web10.7265.1584069675258392404 for ; Thu, 12 Mar 2020 20:21:15 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Mar 2020 20:21:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,546,1574150400"; d="scan'208";a="236833050" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga008.fm.intel.com with ESMTP; 12 Mar 2020 20:21:15 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 12 Mar 2020 20:21:14 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 12 Mar 2020 20:21:14 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.206]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.137]) with mapi id 14.03.0439.000; Fri, 13 Mar 2020 11:21:11 +0800 From: "Ni, Ray" To: "Oram, Isaac W" , Ray Ni , "devel@edk2.groups.io" CC: "Dong, Eric" , "Chan, Amy" , "Chaganty, Rangasai V" Subject: Re: [PATCH] Features/Intel/Readme.md: Document meaning of "Complete" for features Thread-Topic: [PATCH] Features/Intel/Readme.md: Document meaning of "Complete" for features Thread-Index: AQHV+GujmO4vJDYOokCEm1ct3RfbvahEjtMAgAFBBMA= Date: Fri, 13 Mar 2020 03:21:11 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C49533F@SHSMSX104.ccr.corp.intel.com> References: <20200312124117.288336-1-niruiyu@users.noreply.github.com> <3155A53C14BABF45A364D10949B7414C973CEA9F@ORSMSX114.amr.corp.intel.com> In-Reply-To: <3155A53C14BABF45A364D10949B7414C973CEA9F@ORSMSX114.amr.corp.intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Isaac, Thanks for the comments. Reply in below. > -----Original Message----- > From: Oram, Isaac W > Sent: Thursday, March 12, 2020 11:29 PM > To: Ray Ni ; devel@edk2.groups.io > Cc: Ni, Ray ; Dong, Eric ; Chan, A= my ; Chaganty, Rangasai > V > Subject: RE: [PATCH] Features/Intel/Readme.md: Document meaning of "Compl= ete" for features >=20 > Ray, >=20 > I don't think that this is a desirable rule. >=20 > I want to create feature packages that bundle frequently used together ex= isting capabilities. See the NetworkFeaturePkg > for an example. I also want to make feature packages for the USB stack, = debug capabilities, and the like that are often > aggregations of existing modules. Thanks for reminding me the NetworkFeaturePkg case. NetworkFeaturePkg is a = valid case. I want to add this rule to avoid creating a feature package that only conta= ins header files, but the implementations are in each Board package. Do you agree this should be avoided? How about: "A feature package must not contain only interfaces which are implemented b= y board source code packages." >=20 > The Minimum Platform Architecture spec targets advanced features that are= easy to enable for relatively inexperienced > developers. One way of doing that is to leverage the UEFI PI arch and it= s binary component support features. The > Minimum Platform Architecture aims to use this to enable a use case lever= aging Firmware Volumes that looks like: > 1: Build NetworkFeaturePkg (this produces an FV, customized via PCD and/= or static libraries as needed) > 2: Load FV (from shell, by injecting into an existing image using FMMT, = Fiano, etc) > 3: Use network features and functionality >=20 > The model where the only way people extend a UEFI firmware image is by re= building a complete solution needs to end. It > is a misuse of the architecture in my estimation. We have not had much s= uccess with fine granularity component binary > use, i.e. individual PEIM and drivers. Perhaps there is too much experti= se needed. Minimum Platform Architecture and > Advanced Features aim to improve this by enabling larger granularity bina= ry components that require less UEFI knowledge > to use effectively. Is your concern that binary modularity may be not always practical today? I= f that's it, I agree with your concerns. I do find that /Features/Intel/Debugging/Usb3DebugFeaturePkg only contains = library. I think the goal is binary modularity. Before that, source modularity is the bottom-line requirement f= or each feature package. >=20 > I recognize that there is a competing vision that wants to make many smal= l feature packages that are easy to build in or > out based on simple PCD feature flags. As that may improve developer's e= xperience, it is not something I am strongly > contesting. However, I just don't see it as any different than MdeModule= Pkg. It is the same strategy, just using packages > to organize instead of directories. The key difference I can see between package and module is that package gro= ups the module and the accordingly public interfaces together. While if putting lots of modules inside a combo packag= e, all the public interfaces (like header files) are together and it's hard to tell which interfaces are used by which modules. >=20 > The other consideration should include that we have a lot of existing use= rs. I don't want to move existing code around to > make usable features. If we move existing code to create the feature in = the first place, we affect all the existing users, > often for no immediate benefit. If features become successful and widely= used, then is a good time to refactor the code. > The difference is that at that time, the change is essentially behind an = abstraction and so the change doesn't cause as > much pointless work. AdvancedFeaturePkg is the abstraction layer that aims to hide the future ch= anges. >=20 > Regards, > Isaac >=20 >=20 > -----Original Message----- > From: Ray Ni > Sent: Thursday, March 12, 2020 5:41 AM > To: devel@edk2.groups.io > Cc: Ni, Ray ; Dong, Eric ; Chan, A= my ; Chaganty, Rangasai > V ; Oram, Isaac W > Subject: [PATCH] Features/Intel/Readme.md: Document meaning of "Complete"= for features >=20 > Today's document doesn't forbidden creation of a feature package with onl= y interfaces and no code to implement the > interfaces. Such feature package is useless. >=20 > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Amy Chan > Cc: Rangasai V Chaganty > Cc: Isaac W Oram > --- > Features/Intel/Readme.md | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) >=20 > diff --git a/Features/Intel/Readme.md b/Features/Intel/Readme.md index 97= 29f90a41..f0923e3d56 100644 > --- a/Features/Intel/Readme.md > +++ b/Features/Intel/Readme.md > @@ -18,7 +18,7 @@ document as needed. > Advanced features should be: > * _Cohesive_, the feature should not contain any functionality unrelated= to the feature. > * _Complete_, the feature must have a complete design that minimizes dep= endencies. A feature package cannot directly > - depend on another feature package. > + depend on another feature package. A feature package must contain modu= le(s) to implement the feature interfaces. > * _Easy to Integrate_, the feature should expose well-defined software i= nterfaces to use and configure the feature. > * It should also present a set of simple and well-documented standard = EDK II configuration options such as PCDs to > configure the feature. > -- > 2.21.0.windows.1