From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web10.4486.1642635054895153419 for ; Wed, 19 Jan 2022 15:30:56 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=P0X4Txq+; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: nathaniel.l.desimone@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642635054; x=1674171054; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=zyvJy1oP7bSyPltOfUgnZxk8zmqW+wsSBQvOuFDrVpQ=; b=P0X4Txq+u0D27dqh6uPRd+8cJGH4vrq0vkM00wPoPRIlORiOv/UCpgEQ C27ruae9jt4h1bOMBS0P3Wy5EeylJk2fsRoxCUtDUY+bLwP+quw3A9R8X +tQrCI9PyMzSt1BUpnMSR4AvY1YUh3bV77saw64nd+W0zjfMtOjEootww RBiEM6WoHuVCigv1Sqdk0sIegrFhc5ao7fMtiBICYczZcvdTjQgKnGShe Z+1fxPzqFhOHFpVOx+5jyACugvHqYs8mwXniTbOpDgX6uzb18f+S1Pv09 eKahXbHmz3FIF+WS1A60N2Yrc2Ounqchhv17qEG6NgE290Z9FO25j6ydx Q==; X-IronPort-AV: E=McAfee;i="6200,9189,10231"; a="225897807" X-IronPort-AV: E=Sophos;i="5.88,300,1635231600"; d="scan'208";a="225897807" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jan 2022 15:30:53 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.88,300,1635231600"; d="scan'208";a="595570027" Received: from orsmsx606.amr.corp.intel.com ([10.22.229.19]) by fmsmga004.fm.intel.com with ESMTP; 19 Jan 2022 15:30:53 -0800 Received: from orsmsx606.amr.corp.intel.com (10.22.229.19) by ORSMSX606.amr.corp.intel.com (10.22.229.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20; Wed, 19 Jan 2022 15:30:53 -0800 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx606.amr.corp.intel.com (10.22.229.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20 via Frontend Transport; Wed, 19 Jan 2022 15:30:53 -0800 Received: from NAM04-BN8-obe.outbound.protection.outlook.com (104.47.74.44) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.20; Wed, 19 Jan 2022 15:30:53 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LLVmgUtA7j+vz/uxKAM/Gm/jMgCWdJqJT5E32iiBlzBtEtByr9UDQakzzqAVQfDXc9bDJFSLEO3fBePmg8oNaBNck3jIbOVO1lXWjbiZbO3m2dzbGzQn8QwO+d1BtlnN0ksoe5/7bK4G05i1q58KXewGnX84DoNsGZXvOw6hOWIsB1jz+EMqFNj37FiLWFOiqVxK6jnnvY80ifd+5lIF3JQ7C6CILYblMRId1zb3NJMO/jgV8PBDXOtEA2S6H40W+22PrLYGkKkmlXYqGAfV08QIR9MjZmchjZI+t4EspKwNNiiLS/ekRQHkqF1dMX+rbYxZ5npQ1aZh0wtJ1F0ZWA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=cYNhWaRyLoK/E3p7Gt0tvI+GfOj02/91FoqGV9qOEOQ=; b=oEVNAPA9bsU/wjkQwnhnZiBxhx1EfMy+hMfZ4rrNSzvdBG3ApjJDxxPgBZ5WjyTFVlYrgqiUhf32F57aWJHgyt4kkbGtdL9ue+19kaT6bVC+8QJybcVhK39fHVEMpuyMZ3/8dhCndgo9z8eFtAJ6D65XLLKone9geto1FZqqH/POE4fBYlo33bBFIPsoiFc3hFL8QlZhFm3FHJ+ia6ErTaK/MGwVEAcmucPzKq5ftSh9qZ1L2o52Q+aiLGGw0BS5OvPiqW0o8py3P9kaWFcIgEb4mwuLdOdd62sare5hnGmFviFHEYrzWEYbijlrceUAtDoq9230U3Pxf5Gl1VrxZg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from MW4PR11MB5821.namprd11.prod.outlook.com (2603:10b6:303:184::5) by CH0PR11MB5538.namprd11.prod.outlook.com (2603:10b6:610:d5::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.7; Wed, 19 Jan 2022 23:30:50 +0000 Received: from MW4PR11MB5821.namprd11.prod.outlook.com ([fe80::6920:39b9:e18c:9dc5]) by MW4PR11MB5821.namprd11.prod.outlook.com ([fe80::6920:39b9:e18c:9dc5%7]) with mapi id 15.20.4888.013; Wed, 19 Jan 2022 23:30:50 +0000 From: "Nate DeSimone" To: "Oram, Isaac W" , "devel@edk2.groups.io" CC: "Chaganty, Rangasai V" , "Gao, Liming" , "Dong, Eric" , "Tan, Ming" , "Chiu, Chasel" , "Bi, Dandan" , "Shindo, Miki" , "Abbas, Mohamed" , "KARPAGAVINAYAGAM, MANICKAVASAKAM" Subject: Re: [edk2-devel][edk2-platforms][PATCH V1 00/27] Improve feature build consistency Thread-Topic: [edk2-devel][edk2-platforms][PATCH V1 00/27] Improve feature build consistency Thread-Index: AQHYB1sISarRaTwIlUi+MdpsUKVHSKxgJu7AgAAh5ACAAApv0IABBwMAgAmuq2A= Date: Wed, 19 Jan 2022 23:30:49 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.6.200.16 dlp-product: dlpe-windows authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: d7a81daf-f870-4a14-46ca-08d9dba3ba3d x-ms-traffictypediagnostic: CH0PR11MB5538:EE_ x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: xMHd6vt+1r1DrnwtHhbUuhHhtjShfL2Dhqp5mV53nGwCGK7nEUxDeUuUF0r4kC0Fj3HMKhfxfsxoPi84RB7wXbxYfiYjh5QqzPxsY+of6fx6swxhNL5Ivmsssj3szaytNlQv8D1iXhfaSQwVt6IPFmVnB8I1dplntZuDavfLyCHPcOCoz6djQO25TN7t+BB3IuRdLvJ4PYjEefm8+cCu7Vxv2Q09T3HQ3/s9OsmvN9EBS6hpXgmRn09nKG/Px+T7b9Cca4JPKvRcT3Re3FYCHVrSDvy73Nbj/Gl5XVRNJXFeO8twd1m5lbYg4SkEKRRJHtibi0GnmtDRp9Et0pZdC6BpdxAEtWEUvw2XEbsnXfVKNgi7iAFmGHXvTzhzAdo0ss08JSEbx9jeA8g5PGifGROaGjOxR1oRXRdjw3KrikW/fcSQPZCaF8to6yOUxNNJWiPkyoUJeiH8qstlHuLSo9LjHnKXXGhUvmyPL4j9Tt6DHjKb90CqWgh2Tben8D7/IMiaPFJKx2qu2Fi+8sGlN1n9AxK/JUiSiYz6XjApah18cA6HfitjXfDsK7EyoM+MCYHzW5kHj7JxO0tAWlOeKDZQhwFnY9s5F5VI0huYMaGrhtlT4AS+8CzJFXTw14SwO1w0uptp+hh+TR9y0nue0JLcu1zDkoNxazy0e8+qZ8Y4Z43CoNMGphm8OI5c/M0vAd+FWqfYonOIL54XqdITLaKk35qWRMI3ivS6BOzvbb57hwhZXCXewO9jfpT3PkXCzaWisqJ0cGvIGsYDhvrvzmMEwrsiSGeyA+wIMnYxIXY= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR11MB5821.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(366004)(38100700002)(110136005)(2906002)(122000001)(316002)(55016003)(8676002)(33656002)(38070700005)(9686003)(7696005)(82960400001)(8936002)(4326008)(53546011)(52536014)(83380400001)(66946007)(6506007)(86362001)(966005)(26005)(508600001)(5660300002)(30864003)(76116006)(186003)(19627235002)(66476007)(54906003)(66556008)(64756008)(66446008)(71200400001)(559001)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?MO0i/2SFCKpkPC7YKBXsCbrGSY7dUNJk4g6OT/X41SH0JbDJ/GVqhyrk6fYO?= =?us-ascii?Q?ty5IvmzATF0El81T1PsjlwOsDXnaaXI3W+25YKKg9HdqyRqDwKvaIc65SHo4?= =?us-ascii?Q?JG8av7g8t+2SIuT1myEHDv4lGe3wsR7qzDhObtCfh6M1kIP8Kkf+6dSwHcnq?= =?us-ascii?Q?Sg2sBKZKEId4usrBjxqdB4hpxFvm9quvQqu97AHHWtLygJPX4G6y/WI/5bNX?= =?us-ascii?Q?Zd1ldK7bqC1ZjqZqJKwDaoyV/mKKJtSiF3+tlyVo1F0eutVa0NJMw2UGMD5J?= =?us-ascii?Q?YCHMEKqoI/sMQsyigDCrKJilMsySHEypoHSFxP8rGR3blcqClHG5Mfah2rmf?= =?us-ascii?Q?xKC1UWb9o+Fe4e2Wg3vKWxzcfpWNoz6Fgc4efJzqNvTQEhE1taiVLmOL/uG9?= =?us-ascii?Q?e2iHT6xL0hNv1OgGQMG+qVvPHjoF3TlotXh4Fs37L3jjS54Xj7cDvYUQ3k3Y?= =?us-ascii?Q?T8iVON3PEFABGjuqVro73HBGPqx3nq+T61rNneTeBc5eecOuTUl5zhpb3hnb?= =?us-ascii?Q?IHYLfaNK5SZXotc7mHmMAkIuL6B+LKkL3+U7V8qUV4osSLg8MG9jgog880F8?= =?us-ascii?Q?UMVZVfORmSin1vt0KAU03eM7HuPBS5HJipMIAXtPkqDP5j12GpRkO0FHv94X?= =?us-ascii?Q?FVBDMMaBtIJAJyOvkq+Q3gX2MJ8aNIYjvN5PL8xgpfXF2Fjmq+6Plsw6hBUy?= =?us-ascii?Q?FxhJljEkulG0kjIwEkxb54iwAEKNH1PelBqbd1KeAIJDCks+AcW6euY8MtSy?= =?us-ascii?Q?oTUN2fT4c2gZXSTyvCHbMCUpwGHa2AnsjyKA0LApYi3YCOnJl6FRYKmPg771?= =?us-ascii?Q?zJwAeqTfJmx0KUwJbANpA3S8sIslZR9WV341OkGeVxX925jsDOTb2upGgL0r?= =?us-ascii?Q?thz+A7qLA4LYNEZ9mYy4S3IsZvyPTOWHffrdWnhmuUWrklAKEjULhoKnIU/a?= =?us-ascii?Q?/YuXu0bPohCOKo1wBdW4+4dHX1uhJ/PUv5nWHTDimctp9Y+XT90FFsYcdLLb?= =?us-ascii?Q?AZ7HcOduF/xEAlN6S0NTIPLRBAAfrmX2HWB9az++f6B1aV5CkjO3qmw6pPYv?= =?us-ascii?Q?+EPIW/kGuPD3zjvoHDab/zU6l8REUkBK2iwZaqOcBYVE6+1Z7V5GLgUZVfEu?= =?us-ascii?Q?H8Zq1KOQzMrlF7mKkWXkrhhcPcGoVK78coFV02IxhsmOUFyahj5+VwaQfFdh?= =?us-ascii?Q?8/zvU6S1/KUyZbvG742nlbEif5JwBsYJNXLskBCeoOqBi109DoUsPB/W5ZpI?= =?us-ascii?Q?YjvEVr+Tb57A7K/7DRDkH6ihrTAWb3+9u+GAAjBDa+d604y0YPXoZdf9fx0M?= =?us-ascii?Q?Y2or6o7IMQCG5E0yYcAe0ClZuo03EzqGWugKqXq7xxQAuNcnauRi4XjN5Uim?= =?us-ascii?Q?nDZoAulGBdxpbR7QINyg+t5vFAlCU25zXE5rGDw/x7UWsymuwGN9XgEIDb9u?= =?us-ascii?Q?1sjXGCw6b1OCZ6w69ymtpr/wslDE/p1LXfJUoWKsm6UAJNKUPbm6ucZhOXnY?= =?us-ascii?Q?TQTG2q2jlbYdRPjXWHpd3fH9CMVmKkhdw97bczYn/Hpe2jkrxIo3Ni9sE2JG?= =?us-ascii?Q?Wpkz6I9Kf2zxe4LCqtIQhyVjdeEL67HPNJGKkdIgoZ7Lzzvgmz5xhp+bkWCR?= =?us-ascii?Q?cyd9e1XL0Onx5y3yB5UyNyMNsbGcc0+sHgylvic+oveOcHXBwywvJePOko00?= =?us-ascii?Q?kePVAg=3D=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5821.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: d7a81daf-f870-4a14-46ca-08d9dba3ba3d X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Jan 2022 23:30:49.8477 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: itYbn0u9JviMgD1P0CVK+7b35BXX99h7GmPombe3FunTADkC8KdqxgRiqQ/fwIXfNqWbfPbbDhokjnqmMGsfr2/gAkxlCM2g8/KvVw+66js= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5538 Return-Path: nathaniel.l.desimone@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Isaac, Thank you for digging into the PEI_ARCH/DXE_ARCH issue. It is unfortunate t= hat issue is not totally solved in BaseTools yet. With that new knowledge, = Patch 1 and 18 are now approved. Please fix patch 19 and resend it. I think= that should complete the series. Thanks, Nate -----Original Message----- From: Oram, Isaac W =20 Sent: Thursday, January 13, 2022 11:34 AM To: Desimone, Nathaniel L ; devel@edk2.grou= ps.io Cc: Chaganty, Rangasai V ; Gao, Liming ; Dong, Eric ; Tan, Ming ; Chiu, Chasel ; Bi, Dandan ; Shindo, Miki ; Abbas, Mohamed ; KARPAGAVINAYAGAM, MANICKAVASAKAM Subject: RE: [edk2-devel][edk2-platforms][PATCH V1 00/27] Improve feature b= uild consistency Inline discussion continues. Regards, Isaac -----Original Message----- From: Desimone, Nathaniel L Sent: Wednesday, January 12, 2022 7:54 PM To: Oram, Isaac W ; devel@edk2.groups.io Cc: Chaganty, Rangasai V ; Gao, Liming ; Dong, Eric ; Tan, Ming ; Chiu, Chasel ; Bi, Dandan ; Shindo, Miki ; Abbas, Mohamed ; KARPAGAVINAYAGAM, MANICKAVASAKAM Subject: RE: [edk2-devel][edk2-platforms][PATCH V1 00/27] Improve feature b= uild consistency Hi Isaac, Comments inline. Thanks, Nate -----Original Message----- From: Oram, Isaac W Sent: Wednesday, January 12, 2022 7:15 PM To: Desimone, Nathaniel L ; devel@edk2.grou= ps.io Cc: Chaganty, Rangasai V ; Gao, Liming ; Dong, Eric ; Tan, Ming ; Chiu, Chasel ; Bi, Dandan ; Shindo, Miki ; Abbas, Mohamed ; KARPAGAVINAYAGAM, MANICKAVASAKAM Subject: RE: [edk2-devel][edk2-platforms][PATCH V1 00/27] Improve feature b= uild consistency Thanks Nate. Comments in line. Regards, Isaac -----Original Message----- From: Desimone, Nathaniel L Sent: Wednesday, January 12, 2022 6:47 PM To: Oram, Isaac W ; devel@edk2.groups.io Cc: Chaganty, Rangasai V ; Gao, Liming ; Dong, Eric ; Tan, Ming ; Chiu, Chasel ; Bi, Dandan ; Shindo, Miki ; Abbas, Mohamed ; KARPAGAVINAYAGAM, MANICKAVASAKAM Subject: RE: [edk2-devel][edk2-platforms][PATCH V1 00/27] Improve feature b= uild consistency Hi Isaac, Thank you for doing this cleanup work. I have some comments for you. I have= provided a summary of my feedback below: PATCH 01/27 * Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc Line 69: [Components.IA32] should be changed to [Components.$(PEI_ARCH)] Li= ne 83: [Components.X64] should be changed to [Components.$(DXE_ARCH)] Note: These comments can also be addressed by restoring the @todo comment s= tating that these changes still need to be done (which you deleted.) [Isaac] $(DXE_ARCH) and $(PEI_ARCH) are not fully functional. It appears t= hat they are fine in DSC files, even when inside an include. But they are = not fine inside an include in an FDF file. The PreMemory.fdf and PostMemor= y.fdf do not work when I try to introduce this change. =20 As these are not required, ToDo are against coding style and are bad for co= mprehensibility, I would prefer to not add such useless comments back in. = Comments should improve the comprehensibility of code and should not distra= ct from understanding the code. [Nate] The $(PEI_ARCH)/$(DXE_ARCH) additions are not be necessary in the FD= F files. Adding them to the DSC file should be sufficient. Can you re-test = with just the DSC file change? [Isaac] I can't make the build work with a macros in the DSC. FDF parsing = throws an error. I misspoke slightly in the prior comment, it is not the F= DF include that is the problem. It is the DSC include. The macros aren't used in the FDF, but the presence of the included macro s= omehow interferes with the FDF parser determining the architecture for comp= onents. It only seems to work without DSC includes. If I use $(DXE_ARCH) in an includable DSC file, the FDF parsing throws this= kind of error: For example modifying AcpiDebugFeature.dsc to use $(DXE_AR= CH) Standalone build works fine (no FDF) Enabling in a board build, you get= : build.py... : error F001: Module o:\edk2-platforms\Features\Intel\Debugging\AcpiDebugF= eaturePkg\AcpiDebugDxeSmm\AcpiDebugDxe.inf NOT found in DSC file; Is it rea= lly a binary module? If I move the [Components.$(DXE_ARCH)] to the platform DSC file, then the b= uild succeeds. I have submitted a bug to document this issue: https://bugzilla.tianocore.= org/show_bug.cgi?id=3D3803 PATCH 18/27 * Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dsc Line 33: [PcdsFeatureFlag.X64] should be changed to [PcdsFeatureFlag.$(DXE_= ARCH)] Note: This comment can also be addressed by adding a @todo comment stating = that this change still needs to be done. [Isaac] See prior response. PATCH 19/27 * Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc Line 35: Typo here. Usb3DebugPortParamLibo should be Usb3DebugPortParamLib. [Isaac] Good catch. I guess we don't catch that class of error if the libr= ary class is not used. * Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dsc Line 40: Did you test compilation for the Usb3DebugFeaturePkg? I've general= ly run into issues when a components sections does not specify a machine ar= chitecture through some sort of means. [Isaac] There is no code consuming these libraries. I verified build of 32= and 64 bit modes as well as part of AdvancedFeaturePkg build and a board p= ackage build. I believe that library classes can be specified by module type and the buil= d tool builds the right mode for the consuming driver on demand. Basically= , there is no value to specifying architecture for a library. This does not work with components however. If you leave the architecture = unspecified, you get an error when including the component in an FDF as the= build does not know how to resolve. [Nate] I bring this up because you added a [Components] section and put thi= s package's library classes into that [Components] section for the purposes= of running a build test on those library classes even though they are not = consumed by anything. That new [Components] section does not specify a mach= ine architecture so I'm wondering if the compilation still succeeds. [Isaac] Compilation in 32 and 64 bit are fine. This technique was pre-existing in = the USB feature. I just moved it from the includable feature DSC to the st= andalone build DSC as I don't think that boards should build these librarie= s all the time. =20 I have a mixed feeling about it. It is a clever way to enable build testin= g for all architectures. It is misleading though, as they are libraries an= d not components. If we list the libraries in a bare [Components] section,= the architecture is controlled by the build arch input and you can thus te= st both 32b and 64b builds easily with -a options. As opposed to needing a= component that includes the library to get the build tools to build the li= brary for a given arch and cluttering the codebase with "dummy" components = to enable build testing. I think it is generally a good practice to have t= he standalone package builds verify build for all architectures. But I did= n't extend it to all features as I think that is worthy of an RFC discussio= n, if there should be another kind of build option, and so on. PATCH 26/27 Since FvAdvanced is post-memory and not covered by the boot guard IBB, I su= spect we should probably also support optional signing of that FV. [Isaac] I do not know how to act on that suggestion. That seems out of sco= pe for this change. I restricted my changes to be functionally compatible = as I do not have hardware to test these changes other than minimally. [Nate] Nevermind. I checked all the OpenBoardPkgs we have and none of them = have FV signing enabled anyway. Ignore this comment. Thanks, Nate -----Original Message----- From: Oram, Isaac W Sent: Tuesday, January 11, 2022 6:20 PM To: devel@edk2.groups.io Cc: Oram, Isaac W ; Chaganty, Rangasai V ; Gao, Liming ; Dong, Eric <= eric.dong@intel.com>; Tan, Ming ; Desimone, Nathaniel L= ; Chiu, Chasel ; Bi= , Dandan ; Shindo, Miki ; Abbas= , Mohamed ; KARPAGAVINAYAGAM, MANICKAVASAKAM Subject: [edk2-devel][edk2-platforms][PATCH V1 00/27] Improve feature build= consistency This series addresses inconsistencies in feature implementation and use. So= me inconsistencies are just conventions of the feature design/template/conv= ention. Some are inconsistency with feature design intent that negatively = affect the usability of the features and the amount of work required from b= oard porting engineers. Some features were missing feature enable flags. Some features had non-functional standalone builds. Many features were implemented to include common core build content in thei= r feature include files. Updated some of the Readme content. Added AdvancedFeaturePkg.fdf to build all feature content to support verify= ing no build time issues between features. Removed duplicate and unused content from build files. Modified the TemplateFeaturePkg to use the common MinPlatform include conte= nt. Removed all instances where features were relative to Features/Intel and ma= de them relative to the package roots. This does mean PACKAGES_PATH may need to be extended for all the feature do= mains. Debugging, PowerManagement, etc. However, it should enable packaging tools to function properly as the relat= ive paths violate spec. Use of the common MinPlatformPkg build includes does increase the build tim= e for each individual feature in standalone build modes. It does not negati= vely impact board or AdvancedFeaturePkg builds as the common content is onl= y built once. Part of MinPlatform arch intent is to reduce cognitive comple= xity, so the simpler build is more valuable than fast build time. Cc: Sai Chaganty Cc: Liming Gao Cc: Eric Dong Cc: Ming Tan Cc: Nate DeSimone Cc: Chasel Chiu Cc: Dandan Bi Cc: Miki Shindo Cc: Mohamed Abbas Cc: Manickavasakam Karpagavinayagam Signed-off-by: Isaac Oram Isaac Oram (27): BeepDebugFeaturePkg: Use MinPlatformPkg build include files BeepDebugFeaturePkg: Fix all relative package paths AcpiDebugFeaturePkg: Fix all relative package paths IpmiFeaturePkg: Fix all relative package paths IpmiFeaturePkg: Fix build errors S3FeaturePkg: Fix all relative package paths S3FeaturePkg: Use MinPlatformPkg build include files SmbiosFeaturePkg: Fix all relative package paths SmbiosFeaturePkg: Use MinPlatformPkg build include files UserAuthFeaturePkg: Fix all relative package paths UserAuthFeaturePkg: Use MinPlatformPkg build include files VirtualKeyboardFeaturePkg: Fix all relative package paths VirtualKeyboardFeaturePkg: Use MinPlatformPkg build include files VirtualKeyboardFeaturePkg: Add feature enable PCD NetworkFeaturePkg: Use MinPlatformPkg build include files LogoFeaturePkg: Use MinPlatformPkg build include files PostCodeDebugFeaturePkg: Complete as an advanced feature AcpiDebugFeaturePkg: Use MinPlatformPkg build include files Usb3DebugFeaturePkg: Align with feature design guidelines SpcrFeaturePkg: Use MinPlatform build include files TemplateFeaturePkg: Use MinPlatform build include files AdvancedFeaturePkg: Fix all relative package paths AdvancedFeaturePkg: Add missing features MinPlatformPkg/Build: Add an include file for the common SPI FV info WhitleyOpenBoardPkg/Build: Use common SPI FV Header include AdvancedFeaturePkg/Build: Add FDF to create FV for all features WhitleyOpenBoardPkg/Build: Enable Features/Intel features Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc = | 67 +++++- Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.fdf = | 49 +++++ Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeatures.dsc = | 49 +++-- Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc = | 64 +++++- Features/Intel/AdvancedFeaturePkg/Include/PostMemory.fdf = | 49 +++-- Features/Intel/AdvancedFeaturePkg/Include/PreMemory.fdf = | 49 +++-- Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugDxe.= inf | 2 +- Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugDxeSmm/AcpiDebugSmm.= inf | 2 +- Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec = | 2 +- Features/Intel/Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dsc = | 21 ++ Features/Intel/Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.dsc = | 74 +------ Features/Intel/Debugging/AcpiDebugFeaturePkg/Include/PostMemory.fdf = | 4 +- Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec = | 7 +- Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dsc = | 28 +++ Features/Intel/Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc = | 222 ++++++------------- Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Library/BeepLib.h = | 6 +- Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PostMemory.fdf = | 14 ++ Features/Intel/Debugging/BeepDebugFeaturePkg/Include/PreMemory.fdf = | 13 ++ Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandler= Lib/PeiBeepStatusCodeHandlerLib.inf | 5 +- Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandler= Lib/RuntimeDxeBeepStatusCodeHandlerLib.inf | 3 - Features/Intel/Debugging/BeepDebugFeaturePkg/Library/BeepStatusCodeHandler= Lib/SmmBeepStatusCodeHandlerLib.inf | 3 - Features/Intel/Debugging/BeepDebugFeaturePkg/Readme.md = | 91 +++++--- Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PostCodeDebugFeat= ure.dsc | 231 +++++--------------- Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PostMemory.fdf = | 14 ++ Features/Intel/Debugging/PostCodeDebugFeaturePkg/Include/PreMemory.fdf = | 13 ++ Features/Intel/Debugging/PostCodeDebugFeaturePkg/Library/PostCodeStatusCod= eHandlerLib/PeiPostCodeStatusCodeHandlerLib.inf | 2 +- Features/Intel/Debugging/PostCodeDebugFeaturePkg/PostCodeDebugFeaturePkg.d= ec | 11 + Features/Intel/Debugging/PostCodeDebugFeaturePkg/PostCodeDebugFeaturePkg.d= sc | 30 +++ Features/Intel/Debugging/PostCodeDebugFeaturePkg/Readme.md = | 31 ++- Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc = | 131 ++--------- Features/Intel/Debugging/Usb3DebugFeaturePkg/Readme.md = | 50 +++-- Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dec = | 14 +- Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dsc = | 18 ++ Features/Intel/Network/NetworkFeaturePkg/Include/NetworkFeature.dsc = | 89 +------- Features/Intel/Network/NetworkFeaturePkg/NetworkFeaturePkg.dsc = | 18 ++ Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpi/BmcAcpi.inf = | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/BmcElog.inf = | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Frb/FrbDxe.inf = | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Frb/FrbPei.inf = | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/GenericI= pmi.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGener= icIpmi.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Smm/SmmGener= icIpmi.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc = | 90 ++------ Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PostMemory.fdf = | 16 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PreMemory.fdf = | 6 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dsc = | 18 ++ Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFru/IpmiFru.inf = | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiInit/DxeIpmiInit.inf= | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiInit/PeiIpmiInit.inf= | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiBaseLib/Ipmi= BaseLib.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiBaseLibNull/= IpmiBaseLibNull.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/I= pmiCommandLib.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiPlatformHook= LibNull/IpmiPlatformHookLibNull.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/PeiIpmiBaseLib/P= eiIpmiBaseLib.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/S= mmIpmiBaseLib.inf | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/OsWdt/OsWdt.inf = | 2 +- Features/Intel/OutOfBandManagement/IpmiFeaturePkg/SolStatus/SolStatus.inf = | 2 +- Features/Intel/OutOfBandManagement/SpcrFeaturePkg/Include/Library/SpcrDevi= ceLib.h | 2 +- Features/Intel/OutOfBandManagement/SpcrFeaturePkg/Include/PostMemory.fdf = | 13 ++ Features/Intel/OutOfBandManagement/SpcrFeaturePkg/Include/PreMemory.fdf = | 11 + Features/Intel/OutOfBandManagement/SpcrFeaturePkg/Include/SpcrFeature.dsc = | 62 ------ Features/Intel/OutOfBandManagement/SpcrFeaturePkg/Readme.md = | 12 +- Features/Intel/OutOfBandManagement/SpcrFeaturePkg/SpcrFeaturePkg.dec = | 6 + Features/Intel/OutOfBandManagement/SpcrFeaturePkg/SpcrFeaturePkg.dsc = | 18 ++ Features/Intel/PowerManagement/S3FeaturePkg/Include/PreMemory.fdf = | 2 +- Features/Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc = | 74 +------ Features/Intel/PowerManagement/S3FeaturePkg/S3FeaturePkg.dsc = | 18 ++ Features/Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.inf = | 2 +- Features/Intel/Readme.md = | 49 +++-- Features/Intel/SystemInformation/SmbiosFeaturePkg/Include/PostMemory.fdf = | 2 +- Features/Intel/SystemInformation/SmbiosFeaturePkg/Include/SmbiosFeature.ds= c | 54 +---- Features/Intel/SystemInformation/SmbiosFeaturePkg/SmbiosBasicDxe/SmbiosBas= icDxe.inf | 2 +- Features/Intel/SystemInformation/SmbiosFeaturePkg/SmbiosFeaturePkg.dec = | 10 +- Features/Intel/SystemInformation/SmbiosFeaturePkg/SmbiosFeaturePkg.dsc = | 18 ++ Features/Intel/TemplateFeaturePkg/Include/TemplateFeature.dsc = | 2 +- Features/Intel/TemplateFeaturePkg/TemplateFeaturePkg.dsc = | 18 ++ Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc = | 69 +----- Features/Intel/UserInterface/LogoFeaturePkg/LogoFeaturePkg.dec = | 2 - Features/Intel/UserInterface/LogoFeaturePkg/LogoFeaturePkg.dsc = | 38 +++- Features/Intel/UserInterface/UserAuthFeaturePkg/Include/PostMemory.fdf = | 6 +- Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.ds= c | 92 +------- Features/Intel/UserInterface/UserAuthFeaturePkg/Library/PlatformPasswordLi= bNull/PlatformPasswordLibNull.inf | 2 +- Features/Intel/UserInterface/UserAuthFeaturePkg/Library/UserPasswordLib/Us= erPasswordLib.inf | 2 +- Features/Intel/UserInterface/UserAuthFeaturePkg/Library/UserPasswordUiLib/= UserPasswordUiLib.inf | 2 +- Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dsc = | 18 ++ Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/U= serAuthentication2Dxe.inf | 2 +- Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/U= serAuthenticationDxe.inf | 2 +- Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/U= serAuthenticationSmm.inf | 2 +- Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/Include/PostMemory.= fdf | 2 +- Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/Include/VirtualKeyb= oardFeature.dsc | 64 +----- Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardFeat= urePkg.dec | 7 +- Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardFeat= urePkg.dsc | 18 ++ Platform/Intel/{WhitleyOpenBoardPkg =3D> MinPlatformPkg}/Include/Fdf/Commo= nSpiFvHeaderInfo.fdf | 2 +- Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf = | 48 ++-- Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc = | 44 ++++ Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.fdf = | 54 ++--- 96 files changed, 1159 insertions(+), 1334 deletions(-) create mode 10064= 4 Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.fdf create mode 100644 Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Po= stMemory.fdf create mode 100644 Features/Intel/Debugging/BeepDebugFeaturePkg/Include/Pr= eMemory.fdf create mode 100644 Features/Intel/Debugging/PostCodeDebugFeaturePkg/Includ= e/PostMemory.fdf create mode 100644 Features/Intel/Debugging/PostCodeDebugFeaturePkg/Includ= e/PreMemory.fdf create mode 100644 Features/Intel/OutOfBandManagement/SpcrFeaturePkg/Inclu= de/PostMemory.fdf create mode 100644 Features/Intel/OutOfBandManagement/SpcrFeaturePkg/Inclu= de/PreMemory.fdf rename Platform/Intel/{WhitleyOpenBoardPkg =3D> MinPlatformPkg}/Include/Fd= f/CommonSpiFvHeaderInfo.fdf (88%) -- 2.27.0.windows.1