From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web12.597.1573861653103172998 for ; Fri, 15 Nov 2019 15:47:33 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: nathaniel.l.desimone@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Nov 2019 15:47:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,310,1569308400"; d="scan'208";a="288701475" Received: from orsmsx103.amr.corp.intel.com ([10.22.225.130]) by orsmga001.jf.intel.com with ESMTP; 15 Nov 2019 15:47:32 -0800 Received: from orsmsx114.amr.corp.intel.com ([169.254.8.67]) by ORSMSX103.amr.corp.intel.com ([169.254.5.179]) with mapi id 14.03.0439.000; Fri, 15 Nov 2019 15:47:32 -0800 From: "Nate DeSimone" To: "Kubacki, Michael A" , "devel@edk2.groups.io" CC: "Bi, Dandan" , "Chaganty, Rangasai V" , "Chiu, Chasel" , "Wei, David Y" , "Dong, Eric" , "Gao, Liming" , "Agyeman, Prince" Subject: Re: [edk2-platforms][PATCH V1 04/49] Features/Intel: Add Readme.md Thread-Topic: [edk2-platforms][PATCH V1 04/49] Features/Intel: Add Readme.md Thread-Index: AQHVmdJx1mrjj+M9QES+vsCsQyr0rKeMqsgQ Date: Fri, 15 Nov 2019 23:47:32 +0000 Message-ID: <02A34F284D1DA44BB705E61F7180EF0AB5BDB0AF@ORSMSX114.amr.corp.intel.com> References: <20191113032816.4056-1-michael.a.kubacki@intel.com> <20191113032816.4056-5-michael.a.kubacki@intel.com> In-Reply-To: <20191113032816.4056-5-michael.a.kubacki@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjYwMTEwZjMtNDBhMS00MWI0LWJkNGQtNzQ3MGJmNTk2YTdiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNklcL0xXN25rb29iSDhDY1JLQU84cFE1UnI5MmNqajdueG5RTHhhYnlTY0Jua1hQSWphS1wvRU5VaHRLamhyNjVrIn0= x-ctpclassification: CTP_NT x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Readme.md, line 211: Please also add a Linux/Mac example. -----Original Message----- From: Kubacki, Michael A =20 Sent: Tuesday, November 12, 2019 7:28 PM To: devel@edk2.groups.io Cc: Bi, Dandan ; Chaganty, Rangasai V ; Chiu, Chasel ; Wei, David Y ; Desimone, Nathaniel L ; D= ong, Eric ; Gao, Liming ; Agyema= n, Prince Subject: [edk2-platforms][PATCH V1 04/49] Features/Intel: Add Readme.md This change adds the initial Readme.md to the root of the Features/ Intel d= irectory. The features are maintained in an Intel directory because they ar= e not tested on other systems. If there is interest to support a feature on= systems other than Intel, the feature should be tested on those other syst= ems and the feature can be promoted above the Intel directory. This Readme.md is the most important file in the Features/Intel tree as it = provides the desired attributes and definition of advanced features, the ad= vanced feature source layout, and the new feature creation checklist. This = is the primary documentation file for new users to understand advanced feat= ures. Cc: Dandan Bi Cc: Sai Chaganty Cc: Chasel Chiu Cc: Wei David Y Cc: Nate DeSimone Cc: Eric Dong Cc: Liming Gao Cc: Agyeman Prince Signed-off-by: Michael Kubacki --- Features/Intel/Readme.md | 232 ++++++++++++++++++++ 1 file changed, 232 insertions(+) diff --git a/Features/Intel/Readme.md b/Features/Intel/Readme.md new file m= ode 100644 index 0000000000..a2b081eb37 --- /dev/null +++ b/Features/Intel/Readme.md @@ -0,0 +1,232 @@ +# **EDK II Minimum Platform Firmware Advanced Features for Intel®=20 +Platforms** + +In the EDK II Minimum Platform, advanced features are non-essential=20 +features. Essential features are those required to achieve an earlier=20 +boot stage (Stage I through Stage V). The Minimum Platform boot stages=20 +are defined in the [EDK II Minimum Platform Draft Specification](https://e= dk2-docs.gitbooks.io/edk-ii-minimum-platform-specification/). A brief overv= iew is also provided in the [Platfom/Intel/Readme.md](https://github.com/ti= anocore/edk2-platforms/blob/master/Platform/Intel/Readme.md). + +An advanced feature must be implemented as highly cohesive and=20 +stand-alone software to only support a specific feature. Advanced features= are the primary method to extend platform firmware capabilities in a modul= ar fashion. + +If you would like to quickly get started creating a new advanced=20 +feature, you can jump ahead to the [Add New Advanced Feature=20 +Checklist](#Add-New-Advanced-Feature-Checklist) and reference other sectio= ns of this document as needed. + +## Overview + +### Advanced Feature Attributes +Advanced features should be: +* _Cohesive_, the feature should not contain any functionality unrelated t= o the feature. +* _Complete_, the feature must have a complete design that minimizes=20 +dependencies. A feature package cannot directly + depend on another feature package. +* _Easy to Integrate_, the feature should expose well-defined software int= erfaces to use and configure the feature. + * It should also present a set of simple and well-documented standard=20 +EDK II configuration options such as PCDs to + configure the feature. + * In general, features should be self-contained and started by the dispa= tcher. The board firmware should + be required to perform as few steps as possible to enable the feature. + * All features are required to have a feature enable PCD (`PcdFeatureEna= ble`). Any effort to enable the feature + besides this PCD should be carefully considered. Default configuration= values should apply to the common case. +* _Portable_, the feature is not allowed to depend on other advanced=20 +feature or board source code packages. For example, + if Feature A depends on output Feature B, a board integration module=20 +should use a generic interface in Feature A + to get the output and pass it to a generic interface in Feature B.=20 +Structures should not be shared between feature + packages. Most structures should be defined in a common package such=20 +as MdePkg if the structure is industry standard, + IntelSiliconPkg if the structure is specific to Intel silicon=20 +initialization, etc. Feature-specific structures are + of course allowed to be defined within a feature package and used by lib= raries and modules in that package. +* _Self Documenting_, the feature should follow software best practices=20 +to allow others to debug the code and + contribute changes. In addition to source code, advanced features=20 +must have a Readme.md with sufficient + information for a newcomer to understand the feature. +* _Single Instance_, the feature should not have more than one instance=20 +of a source solution. If an existing feature + package does not solve a specific instance of a problem for the=20 +feature, the feature package should be re-worked + to consider new requirements instead of duplicating feature code. + + Features should be written for a specific feature technology. Outside=20 + of technology restrictions, the feature should not make arbitrary=20 + assumptions about the type of board or system that may integrate the=20 + feature. Any board or hardware-specific details that may vary in design = should be given to the feature through a defined and documented software i= nterface. + +### Advanced Feature Packages +Feature portability is a key aspect of board scalability. To maintain=20 +high cohesion within a feature package and reduce potential coupling=20 +between features, each feature is maintained in a dedicated package.=20 +Such encapsulation enables increased flexibility in implementation, rapid = integration with board firmware, improved software maintenance by reducing = coupling between features, and a better scope for feature-focused maintenan= ce and testing over time. + +Two package types exist for advanced features: + * AdvancedFeaturePkg - A special package described further in [AdvancedFe= aturePkg](#AdvancedFeaturePkg). + * XxxFeaturePkg - A specific feature package described further in [XxxFea= turePkg](#XxxFeaturePkg). + +#### AdvancedFeaturePkg +A single package called `AdvancedFeaturePkg` exists with the following res= ponsibilities: +1. Present a consolidated and simplified view of all available advanced fe= atures to board packages. +2. Provide a single package build for all available advanced features. + 2.a. This can save time. Each feature package will send build output to = a package-specific build output directory. + By building a features from a single package build object files can= be reused across features. + +##### Template Resources +A template for a new advanced feature package is provided in=20 +`TemplateFeaturePkg`. It is recommended to start a new advanced feature=20 +package using this template. The minimally required set of files for an=20 +advanced feature to be acceptable is defined in [Advanced Feature Collater= al](#Advanced-Feature-Collateral). Apart from required files, the content o= rganization within the files should follow the layout provided in the templ= ate files. + +##### Consolidated View of All Advanced Features to Board Packages A=20 +board package may consume any number of advanced features. In order to=20 +reduce the overall effort required to add advanced features, all available= advanced features are consolidated into single includable files in `Advanc= edFeaturePkg`. + +_DSC File_ + +All advanced features are importable to a board package DSC file by simply= including `AdvancedFeatures.dsc`. +`AdvancedFeatures.dsc` already includes the conditional logic to only incl= ude features that are enabled by the board package. + +_FDF File_ + +The EDK II Minimum Platform has two firmware volumes to contain advanced f= eatures. + +1. `FvAdvancedPreMemory` - Contains advanced feature modules that should b= e dispatched before permanent memory is enabled. + Typically, these are PEI modules (PEIMs). +2. `FvAdvanced` - Contains advanced feature modules that do not need to be= executed prior to permanent memory being + initialized. Typically, these are post-memory PEIMs or DXE modules. + +##### Single Build of All Advanced Features With many advanced feature=20 +packages, it is convenient to have a single build target that can build al= l advanced features. +The `AdvancedFeaturePkg.dsc` enables all the advanced features for its pac= kage build. + +#### XxxFeaturePkg +Each feature is maintained in its own feature package called=20 +`XxxFeaturePkg` where *Xxx* is replaced by the feature name. + +### Source Code Organization of Advanced Features All advanced feature=20 +code is maintained in `edk2-platforms/Features`. Features that are only=20 +tested on Intel systems are maintained in `edk2-platforms/Features/Intel`.= These features are not intended to be constrained to Intel systems. + +A package resides at the root of `edk2-platforms/Features/Intel` called [A= dvancedFeaturePkg](#AdvancedFeaturePkg). +All feature packages are organized into directories by feature domain.=20 +Each feature domain directory is required to have a `Readme.md` that=20 +explains the scope of features for that domain. Each feature package is=20 +required to have a `Readme.md` that explain the feature. All feature=20 +packages are required to base their `Readme.md` on the +[template](TemplateFeaturePkg/Readme.md) provided in `TemplateFeaturePkg`. + +A generic tree layout of the advanced features is illustrated below.=20 +The contents are shown at the time of writing but of course, are subject t= o change over time. + + 
+    WORKSPACE
+          |------edk2
+          |------edk2-non-osi
+          |------edk2-platforms
+          |       |---Features
+          |       |    |--Intel
+          |       |        |------AdvancedFeaturePkg
+          |       |        |
+          |       |        |------TemplateFeaturePkg
+          |       |        |
+          |       |        |------Debugging: Debug related advanced featur=
es
+          |       |        |       |------AcpiDebugFeaturePkg
+          |       |        |       |       |---AcpiDebugDxeSmm (module dir=
ectories)
+          |       |        |       |       |---.  .  .
+          |       |        |       |       |---Include
+          |       |        |       |       |    |---AcpiDebugFeature.dsc (=
feature build DSC file)
+          |       |        |       |       |    |---PostMemory.fdf (post-m=
emory feature modules)
+          |       |        |       |       |    |---PreMemory.fdf (pre-mem=
ory feature modules)
+          |       |        |       |       |    |---.  .  .
+          |       |        |       |       |---AcpiDebugFeaturePkg.dec (fe=
ature package DEC file)
+          |       |        |       |       |---AcpiDebugFeature.dsc (featu=
re package build DSC file)
+          |       |        |       |
+          |       |        |       |------Usb3DebugFeaturePkg
+          |       |        |       |       |---.  .  .
+          |       |        |       |
+          |       |        |       |------.  .  .
+          |       |        |------Network: Network related advanced featur=
es
+          |       |        |       |------.  .  .
+          |       |        |
+          |       |        |------OutOfBandManagement: Out-of-Band Managem=
ent related advanced features
+          |       |        |       |------.  .  .
+          |       |        |
+          |       |        |------PowerManagement: Power Management relate=
d advanced features
+          |       |        |       |------.  .  .
+          |       |        |
+          |       |        |------SystemInformation: System Information re=
lated advanced features
+          |       |        |       |------.  .  .
+          |       |        |
+          |       |        |------UserInterface: User Interface related ad=
vanced features
+          |       |        |       |------.  .  .
+          |       |        |
+          |------FSP
+
+ +## Adding a New Advanced Feature + +### Advanced Feature Collateral +At a minimum, an advanced feature must consist of the following elements: +1. A feature package directory (`XxxFeaturePkg`) 2. A `Readme.md` file=20 +in the feature package directory root to describe the feature 3. Some=20 +advanced feature source code (e.g. a PEI or DXE module) 4. A feature=20 +DSC file (`XxxFeaturePkg/Include/XxxFeature.dsc`) +5. Feature pre-memory / post-memory FDF files=20 +(`XxxFeaturePkg/Include/PreMemory.fdf` &=20 +`XxxFeaturePkg/Include/PostMemory.fdf`) +6. A feature package DSC file (`XxxFeaturePkg/XxxFeaturePkg.dsc`) +7. A feature package DEC file (`XxxFeaturePkg/XxxFeaturePkg.dec`) +8. A reference in the `AdvancedFeaturePkg` as described in the=20 +[checklist](#Add-New-Advanced-Feature-Checklist) + +### Add New Advanced Feature Checklist +1. Check if a feature package already exists for your new feature. + * If it does, use the existing feature package. If changes are needed = to meet your requirements, work with the + package maintainer to make any required updates to the existing pack= age. + +2. Determine the functional domain of your feature. For example, "Debuggin= g", "Power Management", etc. This should not be + very specific and abstracted so the feature is available alongside simi= lar feature packages for the given domain. It + is preferred to keep the number of feature domain directories minimal b= ut new directories can be created if needed. + +3. Decide on a succinct name for your feature package. + * The name should be descriptive enough to uniquely identify the featu= re from similar features in the same feature + domain. + + * At this point, the feature package location is: `FeatureDomain/= FeaturePkg`. + + * For example, the ACPI Debug feature is located in `Debugging/AcpiDeb= ugFeaturePkg` based on this naming pattern. + +4. Use the package template provided by `TemplateFeaturePkg` to create a n= ew advanced feature package in the + feature location. + +5. Review the [advanced feature attributes](#Advanced-Feature-Attributes) = to draft a feature design proposal. + * It is recommended to use the template in `TemplateFeaturePkg/Readme.= md` to document the feature design for review. + Once the design review is approved, a subsequent review can be sent fo= r the feature implementation. An advantage to + this approach is that the requirement to complete a Readme.md file for= the feature and gain approval in a feature + design review are combined into a single step. + +6. Add the source code for the advanced feature. + * If the feature is large it is recommended to add libraries in one pa= tch and then modules in a following patch. + +7. Update the feature DSC file. This file is in `XxxFeaturePkg/Include/Xxx= Feature.dsc`. + * In most cases, `XxxFeaturePkg/XxxFeaturePkg.dsc` should just `!inclu= de XxxFeaturePkg/Include/XxxFeature.dsc`. + +8. Update the feature FDF files. These files are `XxxFeaturePkg/Include/Pr= eMemory.fdf` and + `XxxFeaturePkg/Include/PostMemory.fdf`. + * Each file should contain the feature pre-memory modules and post-mem= ory modules respectively. + +9. Build the advanced feature package to ensure the build is successful: + + From the workspace root: + 1. cd edk2-platforms/edk2 + 2. Execute edksetup.bat (Windows) or edksetup.sh (Linux). + 3. Verify the "WORKSPACE" environment variable is set to the edk2 dire= ctory in your workspace. + 4. Set the "PACKAGES_PATH" environment variable to include the edk2-pl= atforms/Platform/Intel, edk2-platforms/Silicon/Intel, + and edk2-platforms/Features/Intel directories. + * Windows example: + * set PACKAGES_PATH=3Dc:\Edk2Workspace\edk2-platforms\Platform\In= tel; + c:\Edk2Workspace\edk2-platforms\Silicon\Intel= ; + c:\Edk2Workspace\edk2-platforms\Features\Inte= l + 5. cd edk2-platforms/Features/Intel + 6. 
build -p=20
+ FeatureDomainDirectory/XxxFeaturePkg/XxxFeaturePkg.dsc -a IA32 -a=20
+ X64
+ + *Note:* -a specifies the architecture. Typically IA32 and X64 modul= es are built for 32-bit PEI and 64-bit + DXE though build for your specific requirements. + +10. Add the advanced feature to `AdvancedFeaturePkg` so it is available to= board packages. + 1. Add `XxxFeaturePkg/Include/XxxFeature.dsc` to `AdvancedFeatures.dsc= ` + 2. Add `XxxFeaturePkg/Include/PreMemory.fdf` to `AdvancedFeaturePkg/In= clude/PreMemory.fdf` + 3. Add `XxxFeaturePkg/Include/PostMemory.fdf` to=20 +`AdvancedFeaturePkg/Include/PostMemory.fdf` + +11. Build `AdvancedFeaturePkg` to ensure the build is successful. + 1. Follow the steps in step #9 but change the build command to: + + 
build -p AdvancedFeaturePkg/AdvancedFeaturePkg.dsc -a IA32 -a=20
+ X64
+ +12. Before sending your patch series, ensure the `Readme.md` file in `XxxF= eaturePkg` is completed so others can use it + during the feature review. -- 2.16.2.windows.1