From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web11.563.1660092616585409453 for ; Tue, 09 Aug 2022 17:50:17 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=VOYYp5pV; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: chasel.chiu@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1660092617; x=1691628617; h=from:to:cc:subject:date:message-id:references: in-reply-to:mime-version; bh=wuyMYJqsqddS4/MLeonY/0DV9op7bJuYd1Wk8yeUqrM=; b=VOYYp5pVro1FSl998MYKZEBVQYLO3j62rX0BC8iB3e2Q9679f6XTJOeZ oqEADvo91ZSHWfBXdr/5e3PrI9E/gzINqS2b69vssqTOnftJhu4hExCeh yYKQU8r4oOCfmRv7IYWfhjUmXSo3dcJexM7Z+7Mwx48eZpz1h4feo86Lc t1eNEwZvZQGu2VwMITBsbwJa4V7eH48ZJRFrrTRPdBYrQtEJQDCyCotbO Y8wpScuu+zS0R7uMckIWBpgPiiQZbokT9pQSrejBbA8XNihLZRafbRSsz 2Y977bYa3jJkBLzjVi0JcEfHDKkwLnsg4k0G34S6Hh3/ZWr1BZZdZIjs7 w==; X-IronPort-AV: E=McAfee;i="6400,9594,10434"; a="316910213" X-IronPort-AV: E=Sophos;i="5.93,225,1654585200"; d="scan'208,217";a="316910213" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2022 17:50:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,225,1654585200"; d="scan'208,217";a="694335484" Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by FMSMGA003.fm.intel.com with ESMTP; 09 Aug 2022 17:50:12 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.28; Tue, 9 Aug 2022 17:50:11 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.28; Tue, 9 Aug 2022 17:50:11 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.28 via Frontend Transport; Tue, 9 Aug 2022 17:50:11 -0700 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.100) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2375.28; Tue, 9 Aug 2022 17:50:10 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JBd8WzrunjOp1ZUnv0GLDjTyzhNG4z+i/lRsDkBxV028gV0/QC5ecEL5eG2elKYe7fgsQdMPrx9CvDLZnzIGZRK0soSmeGelsTyxFNCPQN1rXOzUr5Pc/2fzyDIrCL+4SHDS7jtke+S+5iCtDrSAh3ys5+Az/a0BHeC2OEVwqH3QlmlAHdV50eJtFUMlA0yVKFoL6zuB+gIGUOLcvrdwMVeWv7ey0uMXZhU9LaREaub6sWUz/7zWM6VFAur/+IfPkQSyPEXbfp0B7irVJHBm5nbbCqEXYQaCfg6ufAwwjHoduacD3BYW8qj3BEM0jM41sJQzyQBLs2sUd7KJooqurQ== 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=1mKbXlkbqCzgPJOqZpIcpC9XVzGApB6bxc2Fuh3nQi0=; b=EXyDTq+Sy8Z3vSs8jkyOw+KZJDCdjuY3Xr1LtbXjxsjOlv6LRwYk9jRRxPX5ZqRPus6B/qQox0/+yFFqZRPaORxI3a4Z0/w3qxB5cZP80Q3NEQlpLXpMptos7Sf9pLhPL+HW+/CtWC2C4rkQURMiQmPSXFX33B/yZhuXAF8ehFGweFx1wxWRy+jEKEWJe0dBQZbl92cxRi7WEz/R1X7onyr1TMsGlQuYJj0uBMdfSfC96IFJaunrklsg6MA6RceNQI8ck9RpA+FMIftOSdJGQskdWVNQDOjsbxpmvR7mzK0xpklMMkDzOYgUPmxYJxWXQQTD9/m4G5kn3CjK2j88Ig== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from BN9PR11MB5483.namprd11.prod.outlook.com (2603:10b6:408:104::10) by DS0PR11MB6542.namprd11.prod.outlook.com (2603:10b6:8:d2::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.21; Wed, 10 Aug 2022 00:50:09 +0000 Received: from BN9PR11MB5483.namprd11.prod.outlook.com ([fe80::d482:3ee0:a92b:bc39]) by BN9PR11MB5483.namprd11.prod.outlook.com ([fe80::d482:3ee0:a92b:bc39%4]) with mapi id 15.20.5504.021; Wed, 10 Aug 2022 00:50:09 +0000 From: "Chiu, Chasel" To: "Desimone, Nathaniel L" , "devel@edk2.groups.io" CC: "Zeng, Star" Subject: Re: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase. Thread-Topic: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase. Thread-Index: AQHYqGEou0eBmtCqO0yHTJM0G0gY7K2fegQAgAfbJ6A= Date: Wed, 10 Aug 2022 00:50:08 +0000 Message-ID: References: <20220805001951.3687-1-chasel.chiu@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.6.500.17 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: 6e30e129-4656-489f-b948-08da7a6a4635 x-ms-traffictypediagnostic: DS0PR11MB6542:EE_ x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 9ZpGT84RUpsK/uG+RHIlsce/MjiZ7wn0IQeqZvTWTw1yJNfB80q3w+i7mOgdOw1vwYj/gIxyAZj2fOjsYVLWTU/tczapWwSbhRWlcnsxpQdUNzHCoPTplDXSHfq9ckht/ErfXwMyXJc3vjQLuTaI79j4nTC8wqoGXpsUdppkhJxq/7AtNYqA26OgTWUAeu3J3MADxOHzvmm3JIHNP60CH1ewkwuhhe1xLiRaYSfgBppFMEroPdfnCjijv5pDKmJZhAYSh0iisN5KVT2/fCI3vN/xLIJCYITPT7Ifob0fzT8hMrJW6YLHfQfefsFBr1WKSYIEl3A3cfHBYaD1L6iFisqy0nf6hdh0P2PBk9oqjFnloH7wwE8PYcAsCEgvQ1fE8s3QcQ0rsggDFNpPQ6YYQ8mZdeYsm+uBnh+f4gedF4ENf/hfLP619lA0gniCQ281lSrs27p4Mt2DXW3HTyjPFkp+7Fr69fW6pqBtUqpxLmEVIi9bb2lZLx7isLFjVQTPuRp7q0uKZe/+H5FAR3pYOLE3TtzKyaOKcRu5Rmh5eJtwolnJJ7yEH9QM5ICIQ2Hz9z07qEKtaA601d8leV4V24Uwjb/S8IUjWRdKbCG/Tr0pnsN0G9ROamPncNxVozTnGE/e6n1hNeF37T/aVkV0lrX9yDQFVGOycww6YHv0IlcCSuZX7GcrA4uJCnJCXDkvSOJtSnKg/1u8fHELpu0I2JYgZC1tAl7Fu+zIWG259tdIfsZmdX1edLhvSFQe+lshxpoJqry+p1R1Sks3sy/PutY4DE3j2U1AAZd1Hp5rfAs6E4uWYHZntL6jxira9yfo5Zk1N1nN1UZ+jw4Ua31l+g== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN9PR11MB5483.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(346002)(396003)(136003)(39860400002)(376002)(366004)(66446008)(4326008)(64756008)(8676002)(66476007)(66946007)(316002)(55016003)(5660300002)(76116006)(66556008)(52536014)(8936002)(122000001)(38100700002)(38070700005)(86362001)(166002)(33656002)(82960400001)(478600001)(966005)(71200400001)(26005)(41300700001)(2906002)(110136005)(6506007)(107886003)(9686003)(7696005)(186003)(83380400001)(53546011);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?m3D/0N3pGMWNjlPQeR2t74uRMCZnPeL6r0qMwSr3WKUSuDhhpm2w9i+F4XQz?= =?us-ascii?Q?ciuVGFYv4sTnIZg5zJBMxyN+OGNDvmm+D7UDuJL3EzuU+6Gi0sFwf5eO2b7z?= =?us-ascii?Q?7xiJaQVN4a6B9LxwwMNpOqt8Ei3gEEf+cxFQyy7Y8YGW9QTC2nP2MNFcz97l?= =?us-ascii?Q?XcXg2eQfURoaJL2ENkhBoQfPmy39bLetmZW2LCqO5f/tDJJ92bCoQtJTWslN?= =?us-ascii?Q?8LYCnZ/04O6eu7uCfURMwFmAng350irltApQa8/RrF1P2PabPEWhYdpzdJsx?= =?us-ascii?Q?lKW9zsG1h0wbheWZc3c5l+CwwxsE3+F30STH2EQm0nV+FLQEF3PG0CFguaUV?= =?us-ascii?Q?yfDOuiv+nrZ9wdEwGouopOIQIVL4zffribw0LponDRl+d27GtWHO8qztmual?= =?us-ascii?Q?80FdwXxwujCDOejjFwt/yYMdw9LW6MdvtmkIkeKCfs6OU5/XWDDOuHQ6Ow48?= =?us-ascii?Q?dNcJ+CHwWmeWJvIn3Mb843SvZ7UTZpjpJB6ewPhoCW0/nZ6PImJz27+eVoP6?= =?us-ascii?Q?23ZCZIJdHjtLptroojeHF5aYQxAo3a0P/C0PlAVY555fefrMcQEZJA/qmSNC?= =?us-ascii?Q?5kttZ80Wo1mz5hfqV/bRqmR+1v9KNBJTpLp/KaGbLQ2nhcjLIgfo7ThUTgcQ?= =?us-ascii?Q?5MM36NdICgPZjJcrifCvqtRfznxdA6SnMs08kUW1xnJ3JrYAyycApFZT2Cnw?= =?us-ascii?Q?K+F0offdvI3NLTb38LGh8xC/PmSh0ZCwXSXE2ospkLlv4Oq7KfzRsKXD2Ogp?= =?us-ascii?Q?aVpwakCTzdx6+s669XmW2b/HtQPcUmxli9j+Yku+/v9iGftVVHoGnDtlARb8?= =?us-ascii?Q?/Nw6WQx2yAdtSUZ2BChM2emMzCPNRrWcVBiFbLEXmvPcvLtEfNQHPRue3qov?= =?us-ascii?Q?vJiqv3XzPlW78Rr/lpOQaLyjRFXL6hVOnNvA9sB494fFcZ8T+AJU0K5CkLJt?= =?us-ascii?Q?5ekNuqYiK9c8IRc1XREo5lByPS6GXx01catCUqnMW+NBXMaNVsGMob2RkIpq?= =?us-ascii?Q?TVwNCHv4dRh14bLjXeqDG+Z9sKZD+9ykJdEWPe+1HD2ezRsjedNhKAsKcvcH?= =?us-ascii?Q?F/TQVKB6b5vuagaA+tOXCbYH/nDLBhovVV6dyQR0dDDsCrb7nXFxGL/pX7Vd?= =?us-ascii?Q?C5YjIZ6/zBHGaSQaSqfQoOyeZgiWKUOhp9emI5JhYPfx3p40CLERx02gV56k?= =?us-ascii?Q?sqrPQEuU1a0OHiaPqhyjjxHCcaNgfYmBySsMi/1Lq0DGMDhWfYjSRecXzq+K?= =?us-ascii?Q?G77tBaX7yNODa413Xu8cUqJnK46VFJGVbKEhxa2GlUnrWL79iUcgZNfYL3i2?= =?us-ascii?Q?t579uQqJJH6aRagqNxJws834MrOf8mU5vrW/UfeLA80RAMZQgtPyMSinGnTc?= =?us-ascii?Q?VG6Zjvg6qf89eTY6JHnhYWHl12TxSdTP47yucboXin8Y6Tj/NEB5poffGmNs?= =?us-ascii?Q?FLJHId206idKt2PKZeB2B4XkvL5vxI3NQm1We5Z4WReBAIshZ1mkd75ngWT2?= =?us-ascii?Q?pNDYxYzejaIuv+iATev1uWfxMqH7rdmDWfRriT2FeK0InySGT6zx4hPZqJAt?= =?us-ascii?Q?VsR5dqxXyUq6bfoVJpcyN1O7XVl4hbjSKhD9eBBG?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5483.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6e30e129-4656-489f-b948-08da7a6a4635 X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Aug 2022 00:50:08.9082 (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: XI6DBGn+MMRGVRZ07zN39Yu3j8IJ1aX3+fWiGlj5dctCRPZEUdmIC5scE8IufjAtaoP5Shc3bsSLcgzuuX1d/A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6542 Return-Path: chasel.chiu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_BN9PR11MB5483FB78A6C9F500370F4953E6659BN9PR11MB5483namp_" --_000_BN9PR11MB5483FB78A6C9F500370F4953E6659BN9PR11MB5483namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks Nate for detail reviewing and all the good feedbacks! I have applied all of them and sent a V2 patch series, please help to revie= w again. From: Desimone, Nathaniel L Sent: Thursday, August 4, 2022 5:51 PM To: Chiu, Chasel ; devel@edk2.groups.io Cc: Zeng, Star Subject: RE: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase. Hi Chasel, I have a few comments for you. First, we should have a platform provided LibraryClass for running code in = between multi-phase actions. Right now you just have this comment in IntelF= sp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhas= eProcessLib.c: // // Platform handling can be added here to take care specific actions fo= r each phase // before returning control back to FSP. // I would like a new LibraryClass added: FspWrapperPlatformMultiPhaseLib This would implement a single function: VOID EFIAPI FspWrapperPlatformMultiPhaseHandler ( IN UINT8 ComponentIndex, IN UINT32 PhaseIndex ); Add an implementation of this in IntelFsp2WrapperPkg/Library/BaseFspWrapper= PlatformMultiPhaseLibSample. That .inf will provide an implementation of Fs= pWrapperPlatformMultiPhaseHandler() that doesn't do anything (just leave it= empty). Then, invoke FspWrapperPlatformMultiPhaseHandler() at the point where you h= ave that comment above in FspWrapperMultiPhaseHandler(). The *BoardPkg can provide an SOC specific implementation. Typically the rea= l *BoardPkg implementation will look something like this: VOID EFIAPI FspWrapperPlatformMultiPhaseHandler ( IN UINT8 ComponentIndex, IN UINT32 PhaseIndex ) { switch (ComponentIndex) { case FspMultiPhaseMemInitApiIndex: switch (PhaseIndex) { case 1: PeiServicesInstallPpi (mSomePlatformSpecificNotifyPpi1); break; } break; case FspMultiPhaseSiInitApiIndex: switch (PhaseIndex) { case 1: PeiServicesInstallPpi (mSomePlatformSpecificNotifyPpi2); break; } break; } } The exact specifics would vary by SOC design and are out of scope for this = patch series. But you do need to provide the base case of "do nothing" in I= ntelFsp2WrapperPkg. Second, in IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFs= pWrapperMultiPhaseProcessLib.c: case EnumFspVariableRequestSetVariable: if (WriteVariableSupport) { Status =3D VariablePpi->SetVariable ( VariablePpi, FspVariableRequestParams->VariableName, FspVariableRequestParams->VariableGuid, *FspVariableRequestParams->Attributes, (UINTN)*FspVariableRequestParams->DataSiz= e, FspVariableRequestParams->Data ); } else { return EFI_UNSUPPORTED; } Instead of return EFI_UNSUPPORTED; it should be Status =3D EFI_UNSUPPORTED;= . Same thing with EnumFspVariableRequestQueryVariableInfo. Third, in FspWrapperVariableRequestHandler(), after you call EnumMultiPhase= CompleteVariableRequest you need to check if one of the FSP_STATUS_RESET_RE= QUIRED_* status codes is returned and if so invoke CallFspWrapperResetSyste= m(). Fourth, there is a bug in IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPe= im.c: FspWrapperVariableRequestHandler (&FspHobListPtr, FspMultiPhaseMemInitA= piIndex); Should be this: FspWrapperVariableRequestHandler (&FspHobListPtr, FspMultiPhaseSiInitAp= iIndex); Fifth, I noticed some spelling errors in IntelFsp2WrapperPkg/Library/FspWra= pperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c: // Firstly querry variable request informaiton from FSP. Should be: // Get the variable request information from FSP. And this: // Firstly querry FSP for how many phases supported. Should be: // Query FSP for the number of phases supported. Thanks, Nate -----Original Message----- From: Chiu, Chasel > Sent: Thursday, August 4, 2022 5:20 PM To: devel@edk2.groups.io Cc: Chiu, Chasel >; Des= imone, Nathaniel L >; Zeng, Star > Subject: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3916 Add FSP 2.4 MultiPhase interfaces and implementation. Cc: Nate DeSimone > Cc: Star Zeng > Signed-off-by: Chasel Chiu > Chasel Chiu (4): IntelFsp2Pkg: Add FSP 2.4 MultiPhase interface. IntelFsp2WrapperPkg: Add FSP 2.4 MultiPhase interface. IntelFsp2Pkg: Adopt FSP 2.4 MultiPhase functions. IntelFsp2WrapperPkg: Implement FSP 2.4 MultiPhase wrapper handlers. IntelFsp2Pkg/FspSecCore/SecFsp.c = | 4 ++++ IntelFsp2Pkg/FspSecCore/SecFspApiChk.c = | 9 +++++++++ IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/FspMultiPhaseLib.c = | 176 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++++++++++++++++++++++++++++++++++++++++ IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c = | 33 +++++++++++++++++++++++++-------- IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c = | 27 +++++++++++++++++++++------ IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMul= tiPhaseProcessLib.c | 337 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf = | 75 +++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf = | 59 +++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++ IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm = | 304 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryS.nasm = | 101 +++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++++++++++++++++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm = | 3 +++ IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryM.nasm = | 303 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryS.nasm = | 108 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm = | 3 +++ IntelFsp2Pkg/Include/FspEas/FspApi.h = | 62 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++-- IntelFsp2Pkg/Include/FspGlobalData.h = | 5 ++++- IntelFsp2Pkg/Include/Library/FspMultiPhaseLib.h = | 54 +++++++++++++++++++++++++++++++++++++++++++++++++= +++++ IntelFsp2Pkg/IntelFsp2Pkg.dec = | 12 ++++++++++-- IntelFsp2Pkg/IntelFsp2Pkg.dsc = | 4 ++++ IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/BaseFspMultiPhaseLib.inf = | 50 +++++++++++++++++++++++++++++++++++++++++++++++++= + IntelFsp2Pkg/Tools/SplitFspBin.py = | 48 +++++++++++++++++++++++++----------------------- IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf = | 1 + IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf = | 3 ++- IntelFsp2WrapperPkg/Include/Library/FspWrapperMultiPhaseProcessLib.h = | 38 ++++++++++++++++++++++++++++++++++++++ IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec = | 6 +++++- IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc = | 4 +++- IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/FspWrapperMultiP= haseProcessLib.inf | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 27 files changed, 1831 insertions(+), 45 deletions(-) create mode 100644 I= ntelFsp2Pkg/Library/BaseFspMultiPhaseLib/FspMultiPhaseLib.c create mode 100644 IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessL= ib/PeiFspWrapperMultiPhaseProcessLib.c create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryS.nasm create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryM.nasm create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryS.nasm create mode 100644 IntelFsp2Pkg/Include/Library/FspMultiPhaseLib.h create mode 100644 IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/BaseFspMultiPh= aseLib.inf create mode 100644 IntelFsp2WrapperPkg/Include/Library/FspWrapperMultiPhase= ProcessLib.h create mode 100644 IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessL= ib/FspWrapperMultiPhaseProcessLib.inf -- 2.35.0.windows.1 --_000_BN9PR11MB5483FB78A6C9F500370F4953E6659BN9PR11MB5483namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

 

Thanks Nate for detail reviewing and all the good fe= edbacks!

I have applied all of them and sent a V2 patch serie= s, please help to review again.

 

 

From: Desimone, Nathaniel L <nathaniel.l.d= esimone@intel.com>
Sent: Thursday, August 4, 2022 5:51 PM
To: Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io=
Cc: Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 Mult= iPhase.

 

Hi Chasel,

 

I have a few comments for you.

 

First, we should have a platform provided LibraryCla= ss for running code in between multi-phase actions. Right now you just have= this comment in IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib= /PeiFspWrapperMultiPhaseProcessLib.c:

 

    //

    // Platform handling can be added= here to take care specific actions for each phase

    // before returning control back = to FSP.

    //

 

I would like a new LibraryClass added: FspWrapperPla= tformMultiPhaseLib

 

This would implement a single function:

 

VOID=

EFIAPI

FspWrapperPlatf= ormMultiPhaseHandler (

  IN UINT8=      ComponentIndex,

  IN UINT3= 2    PhaseIndex

 );

 

Add an implementation of this in IntelFsp2WrapperPkg= /Library/BaseFspWrapperPlatformMultiPhaseLibSample. That .inf will provide = an implementation of FspWrapperPlatformMultiPhaseHandler() that doesn’= ;t do anything (just leave it empty).

 

Then, invoke FspWrapperPlatformMultiPhaseHandler() a= t the point where you have that comment above in FspWrapperMultiPhaseHandle= r().

 

The *BoardPkg can provide an SOC specific implementa= tion. Typically the real *BoardPkg implementation will look something like = this:

 

VOID=

EFIAPI

FspWrapperPlatf= ormMultiPhaseHandler (

  IN UINT8=      ComponentIndex,

  IN UINT3= 2    PhaseIndex

 )

{

  switch (= ComponentIndex) {

  case Fsp= MultiPhaseMemInitApiIndex:

  &nb= sp; switch (PhaseIndex) {

  &nb= sp;   case 1:

  &nb= sp;     PeiServicesInstallPpi (mSomePlatformSpecificNot= ifyPpi1);

  &nb= sp;   break;

  &nb= sp; }

  &nb= sp; break;

  case Fsp= MultiPhaseSiInitApiIndex:

  &nb= sp; switch (PhaseIndex) {

  &nb= sp;   case 1:

  &nb= sp;     PeiServicesInstallPpi (mSomePlatformSpecificNot= ifyPpi2);

  &nb= sp;   break;

  &nb= sp; }

  &nb= sp; break;

  }

}

 

The exact specifics would vary by SOC design and are= out of scope for this patch series. But you do need to provide the base ca= se of “do nothing” in IntelFsp2WrapperPkg.

 

Second, in IntelFsp2WrapperPkg/Library/FspWrapperMul= tiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c:

 

      case EnumFspVariableR= equestSetVariable:

        if (Write= VariableSupport) {

        &nbs= p; Status =3D VariablePpi->SetVariable (

        &nbs= p;            &= nbsp;            Var= iablePpi,

        &nbs= p;            &= nbsp;            Fsp= VariableRequestParams->VariableName,

        &nbs= p;            &= nbsp;            Fsp= VariableRequestParams->VariableGuid,

        &nbs= p;            &= nbsp;            *Fs= pVariableRequestParams->Attributes,

        &nbs= p;            &= nbsp;            (UI= NTN)*FspVariableRequestParams->DataSize,

        &nbs= p;            &= nbsp;            Fsp= VariableRequestParams->Data

        &nbs= p;            &= nbsp;            );<= o:p>

        } else {<= o:p>

        &nbs= p; return EFI_UNSUPPORTED;

        }

 

Instead of return EFI_UNSUPPORTED; it should be Stat= us =3D EFI_UNSUPPORTED;. Same thing with EnumFspVariableRequestQueryVariabl= eInfo.

 

Third, in FspWrapperVariableRequestHandler(), after = you call EnumMultiPhaseCompleteVariableRequest you need to check if one of = the FSP_STATUS_RESET_REQUIRED_* status codes is returned and if so invoke C= allFspWrapperResetSystem().

 

Fourth, there is a bug in IntelFsp2WrapperPkg/FspsWr= apperPeim/FspsWrapperPeim.c:

 

    FspWrapperVariableRequestHandler = (&FspHobListPtr, FspMultiPhaseMemInitApiIndex);

 

Should be this:

 

    FspWrapperVariableRequestHandler = (&FspHobListPtr, FspMultiPhaseSiInitApiIndex);

 

Fifth, I noticed some spelling errors in IntelFsp2Wr= apperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProc= essLib.c:

 

    // Firstly querry variable reques= t informaiton from FSP.

 

Should be:

 

    // Get the variable request infor= mation from FSP.

 

And this:

 

  // Firstly querry FSP for how many phases sup= ported.

 

Should be:

 

  // Query FSP for the number of phases support= ed.

 

Thanks,

Nate

 

-----Original Message-----
From: Chiu, Chasel <chasel.chiu= @intel.com>
Sent: Thursday, August 4, 2022 5:20 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@i= ntel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <<= a href=3D"mailto:star.zeng@intel.com">star.zeng@intel.com>
Subject: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase.

 

REF: https://bugzilla.tian= ocore.org/show_bug.cgi?id=3D3916

 

Add FSP 2.4 MultiPhase interfaces and implementat= ion.

 

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>

Cc: Star Zeng <star.zeng@int= el.com>

Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>

 

Chasel Chiu (4):

  IntelFsp2Pkg: Add FSP 2.4 MultiPhase inter= face.

  IntelFsp2WrapperPkg: Add FSP 2.4 MultiPhas= e interface.

  IntelFsp2Pkg: Adopt FSP 2.4 MultiPhase fun= ctions.

  IntelFsp2WrapperPkg: Implement FSP 2.4 Mul= tiPhase wrapper handlers.

 

IntelFsp2Pkg/FspSecCore/SecFsp.c   = ;            &n= bsp;            = ;            &n= bsp;            = ;          |   4 +++= +

IntelFsp2Pkg/FspSecCore/SecFspApiChk.c  = ;            &n= bsp;            = ;            &n= bsp;            = ;     |   9 +++++++++

IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/FspMult= iPhaseLib.c          &nbs= p;            &= nbsp;           | 176 +++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++

IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPe= im.c            = ;            &n= bsp;            = ;     |  33 +++++++++++++++++++++++++--------=

IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPe= im.c            = ;            &n= bsp;            &nbs= p;    |  27 +++++++++++++++++++++------=

IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseP= rocessLib/PeiFspWrapperMultiPhaseProcessLib.c | 337 +++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++

IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;  |  75 +++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;  |  59 +++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++

IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm&= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;         | 304 ++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryS.nasm&= nbsp;            &nb= sp;            =             &nb= sp;        | 101 ++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++

IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.na= sm            &= nbsp;           &nbs= p;            &= nbsp;      |   3 +++

IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryM.nasm&n= bsp;            = ;            &n= bsp;            = ;          | 303 +++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryS.nasm&n= bsp;            = ;            &n= bsp;            = ;          | 108 +++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nas= m            &n= bsp;            = ;            &n= bsp;       |   3 +++

IntelFsp2Pkg/Include/FspEas/FspApi.h  &= nbsp;            &nb= sp;            =             &nb= sp;            =       |  62 ++++++++++++++++++++++++++++= ++++++++++++++++++++++++++++++++--

IntelFsp2Pkg/Include/FspGlobalData.h  &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;      |   5 ++++-

IntelFsp2Pkg/Include/Library/FspMultiPhaseLib.h&n= bsp;            = ;            &n= bsp;            = ;          |  54 ++++++++= ++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/IntelFsp2Pkg.dec   &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;            |&nb= sp; 12 ++++++++++--

IntelFsp2Pkg/IntelFsp2Pkg.dsc   &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;            |&nb= sp;  4 ++++

IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/BaseFsp= MultiPhaseLib.inf         &nbs= p;            &= nbsp;      |  50 ++++++++++++++++++++++++++++= ++++++++++++++++++++++

IntelFsp2Pkg/Tools/SplitFspBin.py  &nbs= p;            &= nbsp;           &nbs= p;            &= nbsp;           &nbs= p;         |  48 +++++++++++++= ++++++++++++-----------------------

IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPe= im.inf           &nb= sp;            =              &n= bsp;  |   1 +

IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPe= im.inf           &nb= sp;            =             &nb= sp;   |   3 ++-

IntelFsp2WrapperPkg/Include/Library/FspWrapperMul= tiPhaseProcessLib.h         &n= bsp;            = ;     |  38 ++++++++++++++++++++++++++++++++++++++=

IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec =             &nb= sp;            =             &nb= sp;            = |   6 +++++-

IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc =             &nb= sp;            =             &nb= sp;            = |   4 +++-

IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseP= rocessLib/FspWrapperMultiPhaseProcessLib.inf  |  47 +++++++++++++= ++++++++++++++++++++++++++++++++++

27 files changed, 1831 insertions(+), 45 deletion= s(-)  create mode 100644 IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/Fsp= MultiPhaseLib.c

create mode 100644 IntelFsp2WrapperPkg/Library/Fs= pWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c=

create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24S= ecCoreM.inf

create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24S= ecCoreS.inf

create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/F= sp24ApiEntryM.nasm

create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/F= sp24ApiEntryS.nasm

create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fs= p24ApiEntryM.nasm

create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fs= p24ApiEntryS.nasm

create mode 100644 IntelFsp2Pkg/Include/Library/F= spMultiPhaseLib.h

create mode 100644 IntelFsp2Pkg/Library/BaseFspMu= ltiPhaseLib/BaseFspMultiPhaseLib.inf

create mode 100644 IntelFsp2WrapperPkg/Include/Li= brary/FspWrapperMultiPhaseProcessLib.h

create mode 100644 IntelFsp2WrapperPkg/Library/Fs= pWrapperMultiPhaseProcessLib/FspWrapperMultiPhaseProcessLib.inf<= /p>

 

--

2.35.0.windows.1

 

--_000_BN9PR11MB5483FB78A6C9F500370F4953E6659BN9PR11MB5483namp_--