Isaac, Thanks for the comments. Reply in below. > -----Original Message----- > From: Oram, Isaac W <isaac.w.o...@intel.com> > Sent: Thursday, March 12, 2020 11:29 PM > To: Ray Ni <niru...@users.noreply.github.com>; devel@edk2.groups.io > Cc: Ni, Ray <ray...@intel.com>; Dong, Eric <eric.d...@intel.com>; Chan, Amy > <amy.c...@intel.com>; Chaganty, Rangasai > V <rangasai.v.chaga...@intel.com> > 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 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 contains 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." > > 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. 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, source modularity is the bottom-line requirement for each feature package. > > 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 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 groups the module and the accordingly public interfaces together. While if 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 used by which modules. > > 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 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 changes. > > Regards, > Isaac > > > -----Original Message----- > From: Ray Ni <niru...@users.noreply.github.com> > Sent: Thursday, March 12, 2020 5:41 AM > To: devel@edk2.groups.io > Cc: Ni, Ray <ray...@intel.com>; Dong, Eric <eric.d...@intel.com>; Chan, Amy > <amy.c...@intel.com>; Chaganty, Rangasai > V <rangasai.v.chaga...@intel.com>; Oram, Isaac W <isaac.w.o...@intel.com> > 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 <ray...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Amy Chan <amy.c...@intel.com> > Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com> > Cc: Isaac W Oram <isaac.w.o...@intel.com> > --- > 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 unrelated 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 standard EDK > II configuration options such as PCDs to > configure the feature. > -- > 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55841): https://edk2.groups.io/g/devel/message/55841 Mute This Topic: https://groups.io/mt/71904631/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
