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.web12.4574.1584585197490239086 for ; Wed, 18 Mar 2020 19:33:17 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: ray.ni@intel.com) IronPort-SDR: WUepYRhWa+6y6eSMOad5chgp6sKyvZfnR0PafN8GcnLO1ifQUgONIhafGFXGmzn/k0+9O62Y16 zwV7x1bLwjKQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Mar 2020 19:33:17 -0700 IronPort-SDR: a9gF73Nc883SgkIi8I2c8XRxpC1dVC+Ha6kymbm1sGRz2YYE41zo/TMNLHzfsl4NIAKotQbvtD VSedm3BtU17A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,570,1574150400"; d="scan'208";a="268577974" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga004.fm.intel.com with ESMTP; 18 Mar 2020 19:33:16 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 18 Mar 2020 19:33:17 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 18 Mar 2020 19:33:16 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 18 Mar 2020 19:33:16 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.206]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.201]) with mapi id 14.03.0439.000; Thu, 19 Mar 2020 10:33:12 +0800 From: "Ni, Ray" To: "Oram, Isaac W" , "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+GujmO4vJDYOokCEm1ct3RfbvahEjtMAgAFBBMCACLr7AIAArOWw Date: Thu, 19 Mar 2020 02:33:12 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C49FD1F@SHSMSX104.ccr.corp.intel.com> References: <20200312124117.288336-1-niruiyu@users.noreply.github.com> <3155A53C14BABF45A364D10949B7414C973CEA9F@ORSMSX114.amr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C49533F@SHSMSX104.ccr.corp.intel.com> <3155A53C14BABF45A364D10949B7414C973ED0DB@ORSMSX116.amr.corp.intel.com> In-Reply-To: <3155A53C14BABF45A364D10949B7414C973ED0DB@ORSMSX116.amr.corp.intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: 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 > -----Original Message----- > From: Oram, Isaac W > Sent: Thursday, March 19, 2020 7:57 AM > To: Ni, Ray ; 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 >=20 > I agree that I cannot think of a good reason that the interface would be = in > feature packages and the only use and implementation be in board packages= . Thanks for acknowledging the change. I want to have this clearly documented because I did see such try from deve= lopers. I will post a formal V2 patch. >=20 > With respect to fine grain binary modularity, I don't have strong data or= a > strong intuition as to why attempts at driver level modularity have not > worked well. My intuitions say that it is something like: we haven't fo= und > the right use cases, binary re-use of stable code isn't valuable enough, = and if > features are too small it is too complicated to use effectively. > I think that we have emerging use cases around build time, partial update= s, > and firmware scaling. By scaling I mean that firmware continues to grow = and > to control the impacts of growth, it is often nice to break things into s= maller > pieces that evolve more independently. To be clear, in this context I me= an > breaking the monolithic thing into smaller pieces. My focus is on useful= FV > full of related features. I hope we can reduce visible interdependencies= , get > build time benefits, and eventually validation and update benefits. It > remains to be proven though. I totally agree with you. >=20 > With respect to packages vs directories, I concur that packaging has some > advantages. I am just skeptical that the cost is justified without reali= zing > more developer value for the change. >=20 > With respect to AdvancedFeaturePackage abstracting future change. My > request is obtain wide adoption before impacting existing consumers. Thanks for the comments. We will balance between consolidating common code and customer impact. >=20 > Regards, > Isaac >=20 > -----Original Message----- > From: Ni, Ray > Sent: Thursday, March 12, 2020 8:21 PM > 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 >=20 > Isaac, > Thanks for the comments. Reply in below. >=20 > > -----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, Amy ; Chaganty, Rangasai V > > > > Subject: RE: [PATCH] Features/Intel/Readme.md: Document meaning of > > "Complete" for features > > > > Ray, > > > > I don't think that this is a desirable rule. > > > > I want to create feature packages that bundle frequently used together > > existing capabilities. See the NetworkFeaturePkg for an example. I > > also want to make feature packages for the USB stack, debug capabilitie= s, > and the like that are often aggregations of existing modules. >=20 > 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 con= tains > 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 > by 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 its binary component > support features. The Minimum Platform Architecture aims to use this to > enable a use case leveraging 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 > > > > The model where the only way people extend a UEFI firmware image is by > > rebuilding a complete solution needs to end. It is a misuse of the > > architecture in my estimation. We have not had much success with fine > > granularity component binary use, i.e. individual PEIM and drivers. > > Perhaps there is too much expertise needed. Minimum Platform > Architecture and Advanced Features aim to improve this by enabling larger > granularity binary components that require less UEFI knowledge to use > effectively. >=20 > Is your concern that binary modularity may be not always practical today?= If > 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, sou= rce > modularity is the bottom-line requirement for each feature package. >=20 > > > > I recognize that there is a competing vision that wants to make many > > small feature packages that are easy to build in or out based on > > simple PCD feature flags. As that may improve developer's experience, > > it is not something I am strongly contesting. However, I just don't se= e it as > any different than MdeModulePkg. It is the same strategy, just using > packages to organize instead of directories. >=20 > The key difference I can see between package and module is that package > groups the module and the accordingly public interfaces together. While i= f > putting lots of modules inside a combo package, all the public interfaces= (like > header files) are together and it's hard to tell which interfaces are use= d by > which modules. >=20 > > > > The other consideration should include that we have a lot of existing > > users. 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 fea= tures > become successful and widely used, then is a good time to refactor the co= de. > > 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. >=20 > AdvancedFeaturePkg is the abstraction layer that aims to hide the future > changes. >=20 > > > > Regards, > > Isaac > > > > > > -----Original Message----- > > From: Ray Ni > > Sent: Thursday, March 12, 2020 5:41 AM > > To: devel@edk2.groups.io > > Cc: Ni, Ray ; Dong, Eric ; > > Chan, Amy ; Chaganty, Rangasai V > > ; Oram, Isaac W > > > > Subject: [PATCH] Features/Intel/Readme.md: Document meaning of > > "Complete" for features > > > > Today's document doesn't forbidden creation of a feature package with > > only interfaces and no code to implement the interfaces. Such feature > package is useless. > > > > 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(-) > > > > diff --git a/Features/Intel/Readme.md b/Features/Intel/Readme.md index > > 9729f90a41..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 unrelat= ed > to the feature. > > * _Complete_, the feature must have a complete design that minimizes > > dependencies. A feature package cannot directly > > - depend on another feature package. > > + depend on another feature package. A feature package must contain > module(s) to implement the feature interfaces. > > * _Easy to Integrate_, the feature should expose well-defined software > interfaces to use and configure the feature. > > * It should also present a set of simple and well-documented standar= d > EDK II configuration options such as PCDs to > > configure the feature. > > -- > > 2.21.0.windows.1