From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web10.512.1584575810226513349 for ; Wed, 18 Mar 2020 16:56:50 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: isaac.w.oram@intel.com) IronPort-SDR: jJfa8cuAc/ZonOBsxmyNiAgxbQ76Pw9Hv+TNfsvcrE+x6VETWW3FA49RQDfU1GAOhTqrfrCLlk Szn0y8aAZ9LA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Mar 2020 16:56:49 -0700 IronPort-SDR: RkUcwUWIrWANTj53eZ4M5YDbtJOZR+l1jLxYobIbiQudB7wcYLG/h3CV1aZoCTaHT483YJrJNa ce0IAyeTEZrg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,569,1574150400"; d="scan'208";a="238760470" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by orsmga008.jf.intel.com with ESMTP; 18 Mar 2020 16:56:49 -0700 Received: from orsmsx112.amr.corp.intel.com (10.22.240.13) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 18 Mar 2020 16:56:49 -0700 Received: from orsmsx116.amr.corp.intel.com ([169.254.7.196]) by ORSMSX112.amr.corp.intel.com ([169.254.3.76]) with mapi id 14.03.0439.000; Wed, 18 Mar 2020 16:56:48 -0700 From: "Oram, Isaac W" 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 Thread-Topic: [PATCH] Features/Intel/Readme.md: Document meaning of "Complete" for features Thread-Index: AQHV+GuiAzzqjROLBkGrS/FWSmrAN6hE/IhwgAFU3ICAAGHW4A== Date: Wed, 18 Mar 2020 23:56:48 +0000 Message-ID: <3155A53C14BABF45A364D10949B7414C973ED0DB@ORSMSX116.amr.corp.intel.com> References: <20200312124117.288336-1-niruiyu@users.noreply.github.com> <3155A53C14BABF45A364D10949B7414C973CEA9F@ORSMSX114.amr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C49533F@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C49533F@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.2.0.6 x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Return-Path: isaac.w.oram@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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. 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 wo= rked well. My intuitions say that it is something like: we haven't found = the right use cases, binary re-use of stable code isn't valuable enough, an= d if features are too small it is too complicated to use effectively. =20 I think that we have emerging use cases around build time, partial updates,= and firmware scaling. By scaling I mean that firmware continues to grow a= nd to control the impacts of growth, it is often nice to break things into = smaller pieces that evolve more independently. To be clear, in this contex= t I mean breaking the monolithic thing into smaller pieces. My focus is on= useful FV full of related features. I hope we can reduce visible interdep= endencies, get build time benefits, and eventually validation and update be= nefits. It remains to be proven though. With respect to packages vs directories, I concur that packaging has some a= dvantages. I am just skeptical that the cost is justified without realizin= g more developer value for the change. With respect to AdvancedFeaturePackage abstracting future change. My reque= st is obtain wide adoption before impacting existing consumers. Regards, Isaac -----Original Message----- From: Ni, Ray =20 Sent: Thursday, March 12, 2020 8:21 PM To: Oram, Isaac W ; Ray Ni ; devel@edk2.groups.io Cc: Dong, Eric ; Chan, Amy ; Chaga= nty, Rangasai V Subject: RE: [PATCH] Features/Intel/Readme.md: Document meaning of "Complet= e" for features 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 ;=20 > Chan, Amy ; Chaganty, Rangasai V=20 > > Subject: RE: [PATCH] Features/Intel/Readme.md: Document meaning of=20 > "Complete" 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=20 > existing capabilities. See the NetworkFeaturePkg for an example. I=20 > 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=20 > are easy to enable for relatively inexperienced developers. One way=20 > of doing that is to leverage the UEFI PI arch and its binary component su= pport features. The Minimum Platform Architecture aims to use this to enab= le a use case leveraging Firmware Volumes that looks like: > 1: Build NetworkFeaturePkg (this produces an FV, customized via PCD=20 > and/or static libraries as needed) > 2: Load FV (from shell, by injecting into an existing image using=20 > FMMT, Fiano, etc) > 3: Use network features and functionality >=20 > The model where the only way people extend a UEFI firmware image is by=20 > rebuilding a complete solution needs to end. It is a misuse of the=20 > architecture in my estimation. We have not had much success with fine=20 > granularity component binary use, i.e. individual PEIM and drivers. =20 > Perhaps there is too much expertise needed. Minimum Platform Architectur= e and Advanced Features aim to improve this by enabling larger granularity = binary 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 modular= ity is the bottom-line requirement for each feature package. >=20 > I recognize that there is a competing vision that wants to make many=20 > small feature packages that are easy to build in or out based on=20 > simple PCD feature flags. As that may improve developer's experience,=20 > it is not something I am strongly contesting. However, I just don't see = it as any different than MdeModulePkg. 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 put= ting lots of modules inside a combo package, all the public interfaces (lik= e 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=20 > users. I don't want to move existing code around to make usable=20 > features. If we move existing code to create the feature in the first pl= ace, 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=20 > 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 ;=20 > Chan, Amy ; Chaganty, Rangasai V=20 > ; Oram, Isaac W=20 > > Subject: [PATCH] Features/Intel/Readme.md: Document meaning of=20 > "Complete" for features >=20 > Today's document doesn't forbidden creation of a feature package with=20 > only interfaces and no code to implement the interfaces. Such feature pac= kage 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=20 > 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 unrelated= to the feature. > * _Complete_, the feature must have a complete design that minimizes=20 > dependencies. 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