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.web10.1791.1659660646698603493 for ; Thu, 04 Aug 2022 17:50:47 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=avfxVIO+; spf=pass (domain: intel.com, ip: 134.134.136.31, 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=1659660646; x=1691196646; h=from:to:cc:subject:date:message-id:references: in-reply-to:mime-version; bh=MuE7eeC49HMCXrSqQFg9iy0KCzUow1eZIdaIEP611SU=; b=avfxVIO+bDe3Jw9rXPMP0Xl7UqtFCDJas97qeEFlfBqDjg34wFZIoi2h FIoxegn5G+sVA4p1XkagiZYZNqWMrqUo/aFds2WqWembGw9F06glMPAFT QUORUtNqb4fDYPJVhwrbrR1iWDHlVT23lbgqowV4QljOeMcRihsZ5TbLU TKl8p7Acza+UhizvfC0e7S7GfY6pH9TUR/ikm1cmR33qY/+dVVdnleJIj yHgPB7ak0p+PokJY2F9RWbHquI3k6uOzhPzJLYTDmxH0FGt8oIYFt/p46 H5BS4Xwlc81Ct+zPiiPd5xwH4WlxCGTRHSZbejcziT7YdfyeaT4da6mNk Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10429"; a="351803152" X-IronPort-AV: E=Sophos;i="5.93,216,1654585200"; d="scan'208,217";a="351803152" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Aug 2022 17:50:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,216,1654585200"; d="scan'208,217";a="636311651" Received: from orsmsx606.amr.corp.intel.com ([10.22.229.19]) by orsmga001.jf.intel.com with ESMTP; 04 Aug 2022 17:50:45 -0700 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.2375.28; Thu, 4 Aug 2022 17:50:44 -0700 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.2375.28 via Frontend Transport; Thu, 4 Aug 2022 17:50:44 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.177) 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.2375.28; Thu, 4 Aug 2022 17:50:44 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AMMUS8kO8em70bWCukiEO9Av4sGoQUG+EUMOjWcjFVQe+bNSy6dDVmb0yHBR5swzZTpKnkRz6QvYqlfP/nDajfvJzfazMcWnZ96O6DtDol4cTP41c+Tir6R8jQxtWOPrpy9DM6uTdBwvZXor9T0ML0pOyS8No9AsPhzn7e6fooSF/uOoQyukl4rV4gQVp3mv4ATPk1R0t4f4Br6tJOfXM7duPpKM4K74k7YDk1X3CKWaiiwmvOt30HAl7aPKuz88f7ApLsSsUhabo48OIyAwaKb3arK34HbXOzkTE4JbyVG2XzOdrTZzkkc2ElK9qGYqyBoa2hn2lT4osm/FtzPfBg== 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=WUvudK1YchY7flIpbLCJ+wlXAcAmLBkbrAYDLsjvh+c=; b=Ih3NJy9bIj0huFXBstJnLST6wpxp+KZ1fCYjsBb3KmLZBW64no6+CGKOgPHqCnaPvYII2SA1/2omxpZ4W7zl/pCRtzsps1hvo6jX52EDJbAuJ0MzoV1hx2BwlLqE07cXv1IT4PokzJgCEAEfbGsFZkJnQSdIT8fYxxMcKnV2T/p/89El8QtgEsH0eqI0UXC6VJyoEC/1ufFNMTWgaE//dGCkIF6xQcl4V8j0p/9j+Epxomd1LBvruFLvC2xcrSRBMyjCRmaEMEJxavkHKOnQTEX1/ThIlhQBC9L4dB5OBcgAmYKBNjbQUqAUvG0QOusNEpAaGXWLbU2RAi1KZuVpOw== 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 MW4PR11MB5821.namprd11.prod.outlook.com (2603:10b6:303:184::5) by DM6PR11MB2666.namprd11.prod.outlook.com (2603:10b6:5:c9::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5482.14; Fri, 5 Aug 2022 00:50:42 +0000 Received: from MW4PR11MB5821.namprd11.prod.outlook.com ([fe80::1a2:55c0:2bd6:c120]) by MW4PR11MB5821.namprd11.prod.outlook.com ([fe80::1a2:55c0:2bd6:c120%3]) with mapi id 15.20.5504.014; Fri, 5 Aug 2022 00:50:42 +0000 From: "Nate DeSimone" To: "Chiu, Chasel" , "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: AQHYqGEnbkLdLJPUskWxPJUaynOfiq2fcttg Date: Fri, 5 Aug 2022 00:50:42 +0000 Message-ID: References: <20220805001951.3687-1-chasel.chiu@intel.com> In-Reply-To: <20220805001951.3687-1-chasel.chiu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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: 2aaa8165-caee-42c2-b735-08da767c861c x-ms-traffictypediagnostic: DM6PR11MB2666:EE_ x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: nh9vo4Rn/9RyU/wYGE4u6xckyGLXq+EBUG6ti8MzfalYfL+EzCiCreQIiAdCi3qAU/b1V3NLfRr8FtnPuiKtmvKDlr/Yr+O20IosR/Cxo10tIF3SYT+it9OWzjthNxt407T/PE8ZZ+I0pppAc+r/uKG/JTL4UIteyoBNWKtgH2xn0FApF/WYOnxN2H5+AtS1wmyaKeCfCWfmC2bRLBMin6cfUh073YezFhBCA2zDX0OcPzQPKtl3rqcL8Jq4N4LK1VuKEb50HplHa5xmn0Owfm9CaYB0mdm2i2AtvSyzX2rcDcnBZlZmguQMIIRR+GXI0Er99a6G+hY9yYJmErkO+Ja8ZHDkUHjQqy6gO/Veq2vCWtEAhaQIg1e1AonL9EE8ar5l7l/539OZMeQr0lFis4uF7NZIrlnV6Y9RZgfOWOUex7i7YHF5KszeLbnRCJ2y9spd7hJIIrwCtTcS+IF4IV4DcSLLuN/Uq5fsEvhKq7UDaawwNen6VdwnJkfmS7w/ZRm6oP0jUXo51Oi9MfVrDECjoKkAXpBJliKH/B28Xrm1DMpey2w7SUlbdJowxTjT5HS50d2WrVwiRYTlVzuIOSs53oFTgL89LaUw9QiYVGZIWjS+V8s1UzUVE8ZL9/7vd3mZUh8C2xaryL4jwAK6/NyDzTy0OMRVEmhOI8KBpIUyVZQHVSobhb9hCGmgTKKpL9hA6A3j1COE94mhn+4x1bwQ8GhnMeUXPQURWWHSVg6tI1zeFvn3bzFAFekUHBWF8A3o112ibLUlzclREM2LcW27hNrrjImQK/mnocuNxnoU+gxBPyqO5Jz+tk+sQi99n+ZThS1qInyjg+lBmofd3g== 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:(13230016)(346002)(39860400002)(376002)(136003)(396003)(366004)(41300700001)(9686003)(38100700002)(186003)(83380400001)(122000001)(2906002)(6506007)(26005)(478600001)(966005)(66476007)(71200400001)(9326002)(4326008)(66556008)(66446008)(8676002)(7696005)(8936002)(64756008)(110136005)(107886003)(166002)(52536014)(66946007)(55016003)(33656002)(76116006)(53546011)(86362001)(5660300002)(38070700005)(316002)(82960400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?phMrtMJ1jkxPx8xm6ZhYbcPf/F5FAgHxDoACU+9PIzmE+qEbygKJ63x+/xWe?= =?us-ascii?Q?Jr8sShuWdzBLh3Ek2HwM0h8svp2kR9Fcz2545z6OG1DM7t6URhAyQ3+6QDtq?= =?us-ascii?Q?25f8Zxads3Sb0zuWUHSwmLxXWkrebVfOBPbl+qJnngLnkuvSPDssVfXdkBBN?= =?us-ascii?Q?VaMXrWhf7Tn6g73WCpcuKooLRPcCpDUDnxOD76eaJ62olFhVGXQU/X0f+G5v?= =?us-ascii?Q?wfuft1KM9hGS/s23o/e0n0uSeGyJCBYCa3Cuefn3qzVOTNaiNhaRtT4kJe7W?= =?us-ascii?Q?X1OjYwHv0G+9QprY4d/r5cHzxUAiETyMOeXIZkRKZb8CMYSe941eijS7nYUg?= =?us-ascii?Q?G5p04kwAuPMhZAuYt586Oncp1h299mziNPR5CdQ63nDiN/WutXyPbhypAtMQ?= =?us-ascii?Q?X139LjhGJS9ikbllHXCCeuJTfArNh482ji04Ej76xTVwfBbUlVfzVOPQ/7gy?= =?us-ascii?Q?8yk4SZhYv+n9Jjx+kvq0BJ0Y9vv2uuTTH/zPqrMjuKKdMXjmtnAeAQB391vB?= =?us-ascii?Q?fuHngHfghv/Z0ZzmNUgaPEKypp8Vn2u2UHNDcL1X4rrcBglTxW5sqXMDz9JU?= =?us-ascii?Q?FMS5V87zghDZRjOt32RFBSC6JdbOEx5I/yxIUg0GNqEgyLcheJrkhAHPepXH?= =?us-ascii?Q?u3UhO4obfKCXYFK69WCbVP3VTIrgZK/wjiJiH1iXimWlbUbcdHJRncrbiS0f?= =?us-ascii?Q?8ho0lRTrgWPt2fH9uZtQR23pjJN81h+AKCkBCX6Xt/CkB28AtxhR4s4pJyNA?= =?us-ascii?Q?WDhkOOds/ucxabnOUE/Xjl1xJxG4JBczMbpT9UGPJPVMcrFaPBsxVhm/ulsB?= =?us-ascii?Q?rM3ulGCGYY+w9fY52YUHnJdMseOgTZkYbSAMBtcNq1hjUi+Q9syDo/sDTQBH?= =?us-ascii?Q?+aVSNxud2NcYv/B5BMpJJTZ64LYNY6dAunlufmRnWVXBFQvrkZ85i/F2AYX1?= =?us-ascii?Q?FlptpP9UHmBKtXnrdHhUnafw9gxMT/gdkXfAb1BynMtrvQse/IUXkr7qtGyj?= =?us-ascii?Q?K420uZCQ91UHKZKHcR1oMJb1toJl0q/kpyeUco2Sj6ZIvxNsEjrhX1TL4VJs?= =?us-ascii?Q?/BNwZyIx95yQOf5bx72BEuNwQGeiMSj9z2Qc8oxTHz/Jwf1UZoYFWNR8Pph+?= =?us-ascii?Q?t+6jLpYrOyHQMwbCw/mg+qLog5ex0+iTy/XaLKesdbSh0IVJ/jSeKU1n1qv2?= =?us-ascii?Q?tlBMm/LYQy3a5HfXYZtEv8yMqWTtTEjYSGS43/HCLeMiiAeQRJyFR8bVyDlG?= =?us-ascii?Q?jicuEmymydYuLF+i//EiDI9l9tpjKAYkfzT92KHPuD+2XceHkwZWA4z7aa1C?= =?us-ascii?Q?poheKBdKapHPE1PQ++cbfW345/IDBX8V8rAYr0ERYcpvgRH/bOMNngUfL6F1?= =?us-ascii?Q?B72Pv5sJYmfgsfXfCThz+TWInPnUS5rC3jjJLfa3S/YIWEJWFwmp4SO27UH6?= =?us-ascii?Q?DJnaXR5v9ASJS1do6bB/0ss3PLEGBgfXQtK4rgvxP5jTL1tsv8kJNJTMpIFg?= =?us-ascii?Q?1ySs6dcRn/PUvjqN44Bxx7HnUNv8jwLgRBvFJVAn6nGAjCCaDLufcI9VNuti?= =?us-ascii?Q?3HLWu3y9gQYfIORHKtGLoyV0hh1j1EJ+XaqZaNZr9RsCbODDSFNU5pPtvs4c?= =?us-ascii?Q?HA=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: 2aaa8165-caee-42c2-b735-08da767c861c X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Aug 2022 00:50:42.3928 (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: RDDz5oD9D6E6SVDKK74KHoFhgbO2gSSW7ZqFSuaJF4FerKHwM+Sf9LVgkFHxHV0T3slCG+0baNiJxvfDnWKs8cMhq11EdiFn6WMjIfRcKXo= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB2666 Return-Path: nathaniel.l.desimone@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MW4PR11MB5821D43F0BF9BEB25AAFEDF6CD9E9MW4PR11MB5821namp_" --_000_MW4PR11MB5821D43F0BF9BEB25AAFEDF6CD9E9MW4PR11MB5821namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 ; Desimone, 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_MW4PR11MB5821D43F0BF9BEB25AAFEDF6CD9E9MW4PR11MB5821namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

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@intel.com>; Desimone, Nathaniel L <n= athaniel.l.desimone@intel.com>; Zeng, Star <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_MW4PR11MB5821D43F0BF9BEB25AAFEDF6CD9E9MW4PR11MB5821namp_--