From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web09.310.1614819297351404624 for ; Wed, 03 Mar 2021 16:54:57 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=TBrP6gLM; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: ray.ni@intel.com) IronPort-SDR: 4uY2uvBK45kSrBdA/I7ANB22XJJHFU3tmbxA3+TNuB2yOChdTRm6Ldexy0n79gONu5g3lFyemv b6zzOfmiwU7g== X-IronPort-AV: E=McAfee;i="6000,8403,9912"; a="186656658" X-IronPort-AV: E=Sophos;i="5.81,221,1610438400"; d="scan'208,217";a="186656658" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Mar 2021 16:54:56 -0800 IronPort-SDR: YdcGMm6EFXvCDnKYbCKS9xhn/IZmPDkMTnGShl7zpVxUmpMFmkngTRR4cXx3Q5rzxULHGuUbBA OMTp82bcoiLA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,221,1610438400"; d="scan'208,217";a="596562024" Received: from orsmsx605.amr.corp.intel.com ([10.22.229.18]) by fmsmga006.fm.intel.com with ESMTP; 03 Mar 2021 16:54:56 -0800 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX605.amr.corp.intel.com (10.22.229.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Wed, 3 Mar 2021 16:54:55 -0800 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2 via Frontend Transport; Wed, 3 Mar 2021 16:54:55 -0800 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.108) 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.2106.2; Wed, 3 Mar 2021 16:54:55 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SRUhvTjV/hnpminsIfEd5fhLuhbM4uZdylTNHWxgJScfi91WpvEsx8IgYxNMpcMZv0ztVPLoq3Da+m9jA7MMXtStZzaMHNGscDgX9/pP1th//DeJ00YuzVuDBDoDmF/5kj4jj9JF7SFwBaWwEAjDjGDzsZxMRG1LeOwnMUIq8myuvhuGKPLGbdpV5HDPJ5+B7jlqzyvLeSKea0PikyqBjjF0Xedu5jskrWAk0npxpjV1U8RExFYMn9vbZGW+ODhTW/W+OJC0aaFZ62lglL6KCVxhryxvBbDwOinX0wx93vhO8ai0j1jWQvO2niR00gGAMrs+KJhcRn/yLv4+hJXfLg== 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-SenderADCheck; bh=KiKACIboMx5gQ5U6QaOCqMDBHBoqaug5AyFnvnYmxII=; b=kjz+0Lwz8W4unYfgT2mpPyGmo/4oKliZVTC0T8Ei0jZLcTreIxk56I8YRDIUCvXd+Tdp7yVopUoL95VzbOqmbHqVNcQuVe/jJXfqQRgQEozBe89CeALD72rr2a5YyrigxlWEO0PQjzIbw1EiZneztW+3oI2RRgMvJeAcvZKGn4OZI+uU3afYB01F0ic0kbKI5ZC625c1C+61PgePrI9NN7QBUgufXznHC8WR2XSF46pUFqIrHiJ2XwhRuGahe6a6z71QwQ0Jarxu3p04mY5ApJSYyjDgCu+P7zmO2bqvfYKqfeYQydWouYjIeXU7qXc+hXyZiLeDFNERdHqyGO6aLQ== 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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=KiKACIboMx5gQ5U6QaOCqMDBHBoqaug5AyFnvnYmxII=; b=TBrP6gLMul40Dn5dZgHsJ1U/pEcBlgi/R+UpOSTD/R2wpyKSfvPrFt+u62brY7Hel0DTKXQqn6E7pEcSlvIlRbt4suI6lUUHnvDAPqNqQxKlNAfKVpHBheDnu+yqyseie+/VSrYlRfDcXKCEbqdibMK66ByOSLhb9OqkA1Y/4GQ= Received: from CO1PR11MB4930.namprd11.prod.outlook.com (2603:10b6:303:9b::11) by MWHPR11MB1406.namprd11.prod.outlook.com (2603:10b6:300:23::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3890.28; Thu, 4 Mar 2021 00:54:53 +0000 Received: from CO1PR11MB4930.namprd11.prod.outlook.com ([fe80::8d64:91ed:c259:e95]) by CO1PR11MB4930.namprd11.prod.outlook.com ([fe80::8d64:91ed:c259:e95%7]) with mapi id 15.20.3890.030; Thu, 4 Mar 2021 00:54:53 +0000 From: "Ni, Ray" To: Bret Barkelew , "devel@edk2.groups.io" , "Dong, Guo" , Michael Kubacki CC: "Chaganty, Rangasai V" , "Chiu, Chasel" , "Desimone, Nathaniel L" , "Luo, Heng" , "Agyeman, Prince" , "gaoliming@byosoft.com.cn" , "Dong, Eric" Subject: Re: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib Refactor Thread-Topic: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib Refactor Thread-Index: AQHXDndNOYWYxx12jEeC2Ob81yhUn6pu1iOwgACrkwCAAF24wIAC9FIAgAAPkACAAAfUgIAAA6fAgAAGqQCAAA82YA== Date: Thu, 4 Mar 2021 00:54:53 +0000 Message-ID: References: <2a2a1cc0-3de9-fbd5-1bfb-eb5dbc9d1bc6@linux.microsoft.com> <9a764723-06b6-555a-dfb8-6dee940e7276@linux.microsoft.com> , In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2021-03-03T23:57:57.7235196Z; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged dlp-version: 11.5.1.3 dlp-reaction: no-action dlp-product: dlpe-windows authentication-results: microsoft.com; dkim=none (message not signed) header.d=none;microsoft.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.216] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: e489cc70-5960-4e07-ec2b-08d8dea81f70 x-ms-traffictypediagnostic: MWHPR11MB1406: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Hpi3ogG3PonPHxvav2UzVgzPAgoxuTeoPuS3TvHGABJaawLGOwOqbH+Er5+pz+Ye9HmMFAZGWLVBp3AqDvsftFTnIarL8g7PMRFZue0tx0b3BBe7+i0ITwu8SCr4WJsYf8Mz0yUs4BCczvAmhRYAaUAKS8ksmJNI8lLyesu0pLtB7Y8qSuaRexQDd1Nb+Dv3d43S+6IBD/P5qBlglhWnO5JiHxW52bdYWcO6Spaf/+Tt41lXSXsZ/KB3XUsSTVAEYttl/TKPhLOTtHnU0erjjlqPFgQVA9tNhWJPtDnBdzC5waT7Ei8uDHestopxSAC5yAnuTAvDhfQkHhHrWDzrlRUMnMEMeq/sQmsTVnGiFod1nxHMBCeMLyYR1Gtv2c0og3DtcU9A797HiGs++fn0iHSk9OFs2sbPSdDfX4rPlk+FOHr3GEk/KCCYHZTgw26n7kxeaitYTHY3y62x8tFP9Hk+V+OA9HL3Kp0/bs4/PRSDnufe7cX436WpdjPsMSJ64KiE5be4M9CQO+r5WEFhHYy01+5wGVMXa10s8XYtDjFBun5NEnpPKArGfb++h9WZbsuSzETC5YRSwRY/OvBkYuNxepAjWcA6icxAbbHd/Dg= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CO1PR11MB4930.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(376002)(346002)(366004)(39860400002)(136003)(396003)(478600001)(83380400001)(2906002)(76116006)(33656002)(186003)(45080400002)(52536014)(316002)(8676002)(166002)(966005)(30864003)(54906003)(4326008)(5660300002)(66476007)(71200400001)(55016002)(107886003)(26005)(64756008)(9686003)(66556008)(8936002)(86362001)(66946007)(53546011)(7696005)(66446008)(110136005)(6506007);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?TXNA2TayOEdGqyXE1nu0CzCzFfDsT92YsGfDEMjMUF1fD+1QD9wFPy7BHY8v?= =?us-ascii?Q?dltvqOGJcmfCwUviqkaPNhIu55awJMAAmJ2ij6PjOQegzScLVwkmiNxy64jn?= =?us-ascii?Q?57VLMyMjQMTfTBBGsolMrhR+VgGk52V+hP9bWgg0R8FqUZwgOrEJorDGVcEE?= =?us-ascii?Q?fFWMPtx/S6D9ctE9C6mX0lC0Yu8KYst0V2MdmmPbVloJY6OuLaYgTWkUcwK3?= =?us-ascii?Q?IdjdnL02bvHZYaZGqb8zLlqravRcuqhRL42hLbZ93E3uuN90bgLCfYVJynck?= =?us-ascii?Q?j9zzNiKc3PcMGH8uSRKDUQ6obxBAH2K8Xl7eMBrhMygkUCPpHGTZMH54IAch?= =?us-ascii?Q?mtvFH9B3bSa8becvKHh8nPp86zq/b9icPM7XHU2oHARCkUMfAWPyIGnrup3M?= =?us-ascii?Q?fgWNq18KjZK39kEjTMWr9V3sPJgCxu1+2cRDvvn2Cq/6D8Ce7EUpxLy5Ioal?= =?us-ascii?Q?88G8Saer2DeJXFAxHF80pBikfnIixsoiSFTvhK/hoL9mHvHdnLmUlTxtbK0L?= =?us-ascii?Q?kHE5OYos8n4yH3w/aRRrtVNNo+8kGH3bDeaQCVC9WyoMZQ1ZjONoYSBByrRf?= =?us-ascii?Q?EZIoY/JsU+TNV7CbG7jB2PxsLW32xXJa9GoTPee+ZWTiAo9YWrppHh8hgOdu?= =?us-ascii?Q?kwM0Z8po0Da5+Q0wIZKmEYLVVWJYc1Ct6ZI/TareU6b/3E2KkF4JhYaZm9mF?= =?us-ascii?Q?WKde9O44ci6cgfItKJSWg/5klPVwtPA+u1wx7pwuOnwB+wtq8LF3aPq4TICJ?= =?us-ascii?Q?CW08BLn+MRoPOrftsSEXZ1tx6IAqLq8DKpb08m94/Wvz25fch0AH7CFlrkKT?= =?us-ascii?Q?Cs6GwTlNv1IlgraxYqcN7WCB3jdgjPnrvFCVTvV1GDGD7lccLvJHO7LTMNsG?= =?us-ascii?Q?DtDAu3UcdaloEkreqoyAdm5mEpSMrGmaE1O2qvxV9ml4awsdMX7O1TGSiOtr?= =?us-ascii?Q?6ZvLVLsFxWqkeuXW1LrP5t0JL/6/byyuZwrPbtbm3iTdSa9UQ3qgSW1mhKdo?= =?us-ascii?Q?wIyRgcL85gGwf85y2dhMHLeCzCDANxfnj9EG9bR09Usi4lqrHhNqEIoCZKSY?= =?us-ascii?Q?UGIToOIgAJ42LP+5IGSYxvX7EWNrRQE5aBuK1OntQycdRQYhO/St61KTcGbN?= =?us-ascii?Q?nyxc1lmlqGUc8D89nP8NA4ZVFfn22SN+ps+5bnCJwC8CoPpIhxUy9S5Z369p?= =?us-ascii?Q?ai3dl20zIizNCQGAxX4SQ65tifusg77K1y9/VBlOtu28kyD84JrdNXCpkaJv?= =?us-ascii?Q?SVHzxmAXPgkIZauHVYPYQd8gM5PzMPZ3L2I1KKfYD9f+ZhbU/LcLEd4Z7bxN?= =?us-ascii?Q?7x/zYN7TQFsn2nLXtCjJ/8b+?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4930.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: e489cc70-5960-4e07-ec2b-08d8dea81f70 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Mar 2021 00:54:53.2239 (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: xSlhhpIPa2VXicWQimPu4afGDMMPlfacVWGefV81AmHlTopfrTd/V6ayRLMUm3AnJLK0DrXdN33YeICpaJAMPg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1406 Return-Path: ray.ni@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CO1PR11MB49301002ACD7B6CD859323BD8C979CO1PR11MB4930namp_" --_000_CO1PR11MB49301002ACD7B6CD859323BD8C979CO1PR11MB4930namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Bret, UefiPayloadPkg contains UEFI modules that're needed for booting UEFI OS. So, it's about that UefiPayloadPkg cannot contain code outside of edk2 rep= o. Thanks, Ray From: Bret Barkelew Sent: Thursday, March 4, 2021 7:59 AM To: devel@edk2.groups.io; Ni, Ray ; Dong, Guo ; Michael Kubacki Cc: Chaganty, Rangasai V ; Chiu, Chasel ; Desimone, Nathaniel L ; Luo, Heng ; Agyeman, Prince ; gaoliming@byosoft.com.cn; Dong, Eric Subject: RE: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib Refactor Furthermore, is it the code that cannot cross the repo boundary, or the in= terface? Is there any use in defining the interface at a lower level such as MdeMod= ulePkg? Then a platform can consume the implementation from IntelSiliconPkg= if it so chooses? - Bret From: Ni, Ray via groups.io Sent: Wednesday, March 3, 2021 3:38 PM To: Dong, Guo; Michael Kubacki; devel@edk2.groups.io Cc: Chaganty, Rangasai V; Chiu, Chas= el; Desimone, Nathaniel L; Luo, Heng; Agyeman, Prince<= mailto:prince.agyeman@intel.com>; gaoliming@byosoft.com.cn; Dong, Eric Subject: [EXTERNAL] Re: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonL= ib Refactor Guo, I understand your concern that UefiPayloadPkg should NOT use any component= s outside of edk2 repo (edk2-platform repo in this case). If the two SPI drivers you are talking about are the same one, we could go= back to put the driver to PcAtchipsetPkg. Let's see what the driver contains and then decide where to put. Thanks, Ray > -----Original Message----- > From: Dong, Guo > > Sent: Thursday, March 4, 2021 7:22 AM > To: Michael Kubacki >; devel@edk2.groups.io; Ni, Ray= > > Cc: Chaganty, Rangasai V >; Chiu, Chasel >; Desimone, Nathaniel L > >;= Luo, Heng >; Agyeman, Prince= >; > gaoliming@byosoft.com.cn; Dong, Eric > > Subject: RE: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib Refact= or > > > Hi Michael, > > I didn't initiate any discussion on this yet. And I am not sure if this = idea could be accepted. > From my view point, IntelSiliconPkg is a proper place for SPI flash libr= ary. > But UefiPayloadPkg could not use that package since it is in another rep= o. > Since these dependencies, you could go a head to put it into IntelSilico= nPkg. > > Once I clean up my branch (expected complete next week), I could send my= patch for your > reference so that we could at least share code if possible to reduce the= code maintenance. > > Thanks, > Guo > > > -----Original Message----- > > From: Michael Kubacki > > > Sent: Wednesday, March 3, 2021 3:54 PM > > To: devel@edk2.groups.io; Dong, Guo >; Ni, Ray > > > > > Cc: Chaganty, Rangasai V >; Chiu, Chasel > > >; Desimone, Natha= niel L > > = >; Luo, Heng >; > > Agyeman, Prince >; gaoliming@byosoft.com.cn; > > Dong, Eric > > > Subject: Re: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib > > Refactor > > > > Hi Guo, > > > > That's good to hear. > > > > Does this new "common SPI flash library" and "SMM FVB driver" have > > equivalent functionality to the instances being discussed here? > > > > For the platforms I have in mind, IntelSiliconPkg is an allowed > > dependency whereas UefiPayloadPkg is not. > > > > I have begun the work (at low priority) discussed earlier in the threa= d, > > please let me know if I should continue. > > > > Thanks, > > Michael > > > > On 3/3/2021 1:58 PM, Guo Dong wrote: > > > > > > Hi Michael, > > > > > > We had created a common SPI flash library and a common SMM FVB drive= r > > for all the platforms I have (including Apollo lake, Coffee lake, Kaby= Lake, > > Comet Lake, Tiger Lake, Elkhart Lake, etc.). we plan to upstream this= for UEFI > > Payload. > > > If this one could be upstream to UefiPayloadPkg, then each platform = could > > leverage it. > > > > > > BTW, together with this, we plan to upstream SMM support, secure boo= t > > and measured boot for UEFI Payload. > > > So we could use a single UEFI Payload with these advanced features = on > > different platforms. > > > > > > Thanks, > > > Guo > > > > > > > > >> -----Original Message----- > > >> From: Ni, Ray > > > >> Sent: Monday, March 1, 2021 5:52 PM > > >> To: Michael Kubacki >; > > >> devel@edk2.groups.io > > >> Cc: Chaganty, Rangasai V >; Chiu, Chasel > > >> >; Desimone, Na= thaniel L > > >> >; Luo, Heng >; > > >> Agyeman, Prince >; > > gaoliming@byosoft.com.cn; > > >> Dong, Eric >; Dong,= Guo > > > >> Subject: RE: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib > > >> Refactor > > >> > > >> Michael, > > >> I am good with that. I am also curious how you merge all the differ= ent SPI > > >> flash implementation into one. > > >> > > >> Thanks, > > >> Ray > > >> > > >>> -----Original Message----- > > >>> From: Michael Kubacki > > > >>> Sent: Tuesday, March 2, 2021 3:16 AM > > >>> To: devel@edk2.groups.io; Ni, Ray > > > >>> Cc: Chaganty, Rangasai V >; Chiu, Chasel > > >> >; Desimone, Na= thaniel L > > >>> >; Luo, Heng >; > > >> Agyeman, Prince >; > > >>> gaoliming@byosoft.com.cn; Dong, E= ric > > > >>> Subject: Re: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib > > >> Refactor > > >>> > > >>> Hi Ray, > > >>> > > >>> That sounds reasonable to me. > > >>> > > >>> I was attempting to preserve the design that isolates the > > >>> silicon-specific logic to a library via an interface to a silicon > > >>> package. However, the real abstraction here is the firmware volume > > block > > >>> protocol which could simply be produced by a silicon driver with t= he > > >>> separation of such logic to a library being an implementation deta= il of > > >>> the driver. > > >>> > > >>> In summary, here is the updated proposal: > > >>> > > >>> 1. Consolidate the library interface into a single header in > > >>> IntelSiliconPkg. > > >>> > > >>> 2. Consolidate the library implementation into a single instance i= n > > >>> IntelSiliconPkg. > > >>> > > >>> 3. Move SpiFvbServiceSmm out of MinPlatformPkg into IntelSiliconPk= g. > > >>> > > >>> 4. Add SpiFvbServiceStandaloneMm to IntelSiliconPkg sharing > > >>> implementation with SpiFvbServiceSmm where appropriate. > > >>> > > >>> Intel board packages would then use the SpiFlashCommonLib from > > >>> IntelSiliconPkg (a generation specific instance could be created i= f > > >>> needed) and use the SpiFvbServiceXyz driver from IntelSiliconPkg. > > >>> > > >>> Please let me know if this is acceptable and I'd be happy to send = the > > >>> patches. > > >>> > > >>> Thanks, > > >>> Michael > > >>> > > >>> On 3/1/2021 1:07 AM, Ni, Ray wrote: > > >>>> Michael, > > >>>> I agree with your thoughts to consolidate the lib header and > > >> implementation to IntelSiliconPkg. > > >>>> I didn't read the different implementations. But the implementati= on > > >> consolidation means you see all the existing > > >>> implementations are the same. Right? > > >>>> > > >>>> But why don't you put the driver in IntelSiliconPkg as well? Crea= ting an > > >> advanced feature for this fundamental service seems > > >>> over-kill. > > >>>> > > >>>> Thanks, > > >>>> Ray > > >>>> > > >>>>> -----Original Message----- > > >>>>> From: Chaganty, Rangasai V > > > >>>>> Sent: Monday, March 1, 2021 4:46 PM > > >>>>> To: Ni, Ray > > > >>>>> Subject: RE: [edk2-platforms][RFC] SpiFlashCommonLib Refactor > > >>>>> > > >>>>> Did you get a chance to look into this ? > > >>>>> > > >>>>> -----Original Message----- > > >>>>> From: Michael Kubacki > > > >>>>> Sent: Tuesday, February 16, 2021 4:58 PM > > >>>>> To: devel@edk2.groups.io; Ni, Ray <= ray.ni@intel.com>; Chaganty, > > >> Rangasai V >; Chiu, > > >>> Chasel > > >>>>> >; Desimone,= Nathaniel L > > >> >; Luo, Heng >; > > >>>>> Agyeman, Prince >; > > >> gaoliming@byosoft.com.cn; Dong, Er= ic > > > >>>>> Subject: [edk2-platforms][RFC] SpiFlashCommonLib Refactor > > >>>>> > > >>>>> Hello, > > >>>>> > > >>>>> I'm planning to submit support for Standalone MM in > > >> SpiFlashCommonLib soon. Currently, there's quite a bit of duplicati= on > > >>> with > > >>>>> SpiFlashCommonLib. > > >>>>> > > >>>>> I would like to have this Standalone MM support be available in = as > > >> consistent of a location as possible so I'd like to see if > > >>> there is > > >>>>> anything I can do to help clean this up in the early part of the= patch > > >> series. > > >>>>> > > >>>>> > > >>>>> The library interface is currently defined in the following head= er files: > > >>>>> > > >>>>> 1. > > >> Platform\Intel\MinPlatformPkg\Include\Library\SpiFlashCommonLib.h > > >>>>> > > >>>>> 2. Silicon\Intel\SimicsIch10Pkg\Include\Library\SpiFlashCommonLi= b.h > > >>>>> > > >>>>> 3. > > >> > > Silicon\Intel\KabylakeSiliconPkg\Pch\Include\Library\SpiFlashCommonLib= .h > > >>>>> > > >>>>> 4. > > >>>>> > > >> > > Silicon\Intel\CoffeelakeSiliconPkg\Pch\Include\Library\SpiFlashCommonL= ib. > > >> h > > >>>>> > > >>>>> > > >>>>> Instances of SmmSpiFlashCommonLib implementation exist in a mix > > of > > >> platform and silicon packages: > > >>>>> > > >>>>> 1. > > >>>>> > > >> > > Silicon\Intel\SimicsIch10Pkg\Library\SmmSpiFlashCommonLib\SmmSpiFlashC > > >> ommonLib.inf > > >>>>> > > >>>>> 2. > > >>>>> > > >> > > Platform\Intel\TigerlakeOpenBoardPkg\Library\SmmSpiFlashCommonLib\S > > >> mmSpiFlashCommonLib.inf > > >>>>> > > >>>>> 3. > > >>>>> > > >> > > Silicon\Intel\KabylakeSiliconPkg\Pch\Library\SmmSpiFlashCommonLib\Smm > > >> SpiFlashCommonLib.inf > > >>>>> > > >>>>> 4. > > >>>>> > > >> > > Silicon\Intel\CoffeelakeSiliconPkg\Pch\Library\SmmSpiFlashCommonLib\Sm > > >> mSpiFlashCommonLib.inf > > >>>>> > > >>>>> 5. > > >>>>> > > >> > > Platform\Intel\MinPlatformPkg\Flash\Library\SpiFlashCommonLibNull\SpiF= la > > >> shCommonLibNull.inf > > >>>>> > > >>>>> > > >>>>> The library class is currently consumed in the following INFs: > > >>>>> > > >>>>> 1. > > >> > > Platform\Intel\MinPlatformPkg\Flash\SpiFvbService\SpiFvbServiceSmm.inf > > >>>>> > > >>>>> 2. > > >>>>> > > >> > > Platform\Intel\MinPlatformPkg\Flash\SpiFvbService\SpiFvbServiceStandal= on > > >> eMm.inf > > >>>>> > > >>>>> > > >>>>> My understanding is: > > >>>>> > > >>>>> 1. The header file is defined in each silicon package because si= licon > > >> cannot depend upon platform (i.e. the MinPlatformPkg > > >>>>> header). > > >>>>> > > >>>>> 2. The header is present in each silicon package because the > > >> implementation is silicon-specific and these packages cannot > > >>>>> depend on one another. > > >>>>> > > >>>>> 3. The header is defined in MinPlatformPkg because MinPlatformPk= g > > >> should be silicon vendor agnostic (cannot depend on the > > >>>>> silicon packages). > > >>>>> > > >>>>> 4. The header is needed in MinPlatformPkg because the > > SpiFvbService > > >> there depends on SPI flash operations implemented in > > >>>>> SpiFlashCommonLib. > > >>>>> > > >>>>> > > >>>>> Here's an initial proposal: > > >>>>> > > >>>>> 1. Consolidate the library interface into a single header. In > > >>>>> IntelSiliconPkg? > > >>>>> > > >>>>> 2. Consolidate library implementation into a single instance. In > > >>>>> IntelSiliconPkg? > > >>>>> > > >>>>> 3. Move SpiFvbServiceXyz out of MinPlatformPkg. > > >>>>> 3.a. Make a "SPI flash" feature? > > >>>>> 3.b. Allow the Intel implementation of this feature to dep= end on > > >>>>> SpiFlashCommonLib defined in IntelSiliconPkg. > > >>>>> > > >>>>> Intel board packages could then use the SpiFlashCommonLib from > > >>>>> IntelSiliconPkg (a generation specific instance could be created= if > > >>>>> needed) and use the SpiFvbServiceXyz driver from the "SpiFlash" > > >> feature. > > >>>>> > > >>>>> Look forward to your thoughts and feedback. > > >>>>> > > >>>>> Thanks, > > >>>>> Michael > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > > > > > > > > > > > > > > --_000_CO1PR11MB49301002ACD7B6CD859323BD8C979CO1PR11MB4930namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Bret,

UefiPayloadPkg contains UEFI modules that’re = needed for booting UEFI OS.

So, it’s about that UefiPayloadPkg cannot con= tain code outside of edk2 repo.

 

Thanks,

Ray

 

From: Bret Barkelew <Bret.Barkelew@micros= oft.com>
Sent: Thursday, March 4, 2021 7:59 AM
To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Dong, G= uo <guo.dong@intel.com>; Michael Kubacki <mikuback@linux.microsoft= .com>
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chi= u, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathanie= l.l.desimone@intel.com>; Luo, Heng <heng.luo@intel.com>; Agyeman, = Prince <prince.agyeman@intel.com>; gaoliming@byosoft.com.cn; Dong, Eric <eric.dong@intel.com>
Subject: RE: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib R= efactor

 

Furthermore, is it the code that cannot cross the r= epo boundary, or the interface?

Is there any use in defining the interface at a low= er level such as MdeModulePkg? Then a platform can consume the implementati= on from IntelSiliconPkg if it so chooses?

 

- Bret

 

From: Ni, Ray via groups.io
Sent: Wednesday, March 3, 2021 3:38 PM
To: Dong, Guo; Michael Kubacki; devel@edk2.gr= oups.io
Cc: Chaganty, Rang= asai V; Chiu, Chasel; Desimone, Nathaniel L; Luo, Heng= ; Agyeman, Prince; gaoliming= @byosoft.com.cn; Dong, Eric
Subject: [EXTERNAL] Re: [edk2-devel] [edk2-platforms][RFC] SpiFlash= CommonLib Refactor

 

Guo,
I understand your concern that UefiPayloadPkg should NOT use any component= s outside of edk2 repo (edk2-platform repo in this case).
If the two SPI drivers you are talking about are the same one, we could go= back to put the driver to PcAtchipsetPkg.
Let's see what the driver contains and then decide where to put.

Thanks,
Ray

> -----Original Message-----
> From: Dong, Guo <guo.dong@in= tel.com>
> Sent: Thursday, March 4, 2021 7:22 AM
> To: Michael Kubacki <mikuback@linux.microsoft.com>; devel@edk2.groups.io; Ni, Ray = <ray.ni@intel.com>
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nat= haniel L
> <nathaniel.l.des= imone@intel.com>; Luo, Heng <heng.luo@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> gaoliming@byosoft.com.cn<= /a>; Dong, Eric <eric.dong@intel.= com>
> Subject: RE: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib Ref= actor
>
>
> Hi Michael,
>
> I didn't initiate any discussion on this yet. And I am not sure if th= is idea could be accepted.
> From my view point, IntelSiliconPkg is a proper place for SPI flash l= ibrary.
> But UefiPayloadPkg could not use that package since it is in another = repo.
> Since these dependencies, you could go a head to put it into IntelSil= iconPkg.
>
> Once I clean up my branch (expected complete next week), I could send= my patch for your
> reference so that we could at least share code if possible to reduce = the code maintenance.
>
> Thanks,
> Guo
>
> > -----Original Message-----
> > From: Michael Kubacki <mikuback@linux.microsoft.com>
> > Sent: Wednesday, March 3, 2021 3:54 PM
> > To: devel@edk2.groups.io= ; Dong, Guo <guo.dong@intel.co= m>; Ni, Ray
> > <ray.ni@intel.com>=
> > Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel
> > <chasel.chiu@intel.c= om>; Desimone, Nathaniel L
> > <nathaniel.= l.desimone@intel.com>; Luo, Heng <heng.luo@intel.com>;
> > Agyeman, Prince <= prince.agyeman@intel.com>; gaoliming@byosoft.com.cn;<= br> > > Dong, Eric <eric.dong@= intel.com>
> > Subject: Re: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLi= b
> > Refactor
> >
> > Hi Guo,
> >
> > That's good to hear.
> >
> > Does this new "common SPI flash library" and "SMM= FVB driver" have
> > equivalent functionality to the instances being discussed here?<= br> > >
> > For the platforms I have in mind, IntelSiliconPkg is an allowed<= br> > > dependency whereas UefiPayloadPkg is not.
> >
> > I have begun the work (at low priority) discussed earlier in the= thread,
> > please let me know if I should continue.
> >
> > Thanks,
> > Michael
> >
> > On 3/3/2021 1:58 PM, Guo Dong wrote:
> > >
> > > Hi Michael,
> > >
> > > We had created a common SPI flash library and a common SMM = FVB driver
> > for all the platforms I have (including Apollo lake, Coffee lake= , Kaby Lake,
> > Comet Lake, Tiger Lake, Elkhart Lake, etc.).  we plan to up= stream this for UEFI
> > Payload.
> > > If this one could be upstream to UefiPayloadPkg, then each = platform could
> > leverage it.
> > >
> > > BTW, together with this, we plan to upstream SMM support, s= ecure boot
> > and measured boot for UEFI Payload.
> > > So we could use a single UEFI Payload with these advanced f= eatures  on
> > different platforms.
> > >
> > > Thanks,
> > > Guo
> > >
> > >
> > >> -----Original Message-----
> > >> From: Ni, Ray <r= ay.ni@intel.com>
> > >> Sent: Monday, March 1, 2021 5:52 PM
> > >> To: Michael Kubacki <mikuback@linux.microsoft.com>;
> > >> devel@edk2.grou= ps.io
> > >> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel<= br> > > >> <chasel.chi= u@intel.com>; Desimone, Nathaniel L
> > >> <n= athaniel.l.desimone@intel.com>; Luo, Heng <heng.luo@intel.com>;
> > >> Agyeman, Prince <prince.agyeman@intel.com>;
> > gaoliming@byosoft.co= m.cn;
> > >> Dong, Eric <e= ric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>
> > >> Subject: RE: [edk2-devel] [edk2-platforms][RFC] SpiFlas= hCommonLib
> > >> Refactor
> > >>
> > >> Michael,
> > >> I am good with that. I am also curious how you merge al= l the different SPI
> > >> flash implementation into one.
> > >>
> > >> Thanks,
> > >> Ray
> > >>
> > >>> -----Original Message-----
> > >>> From: Michael Kubacki <mikuback@linux.microsoft.com>
> > >>> Sent: Tuesday, March 2, 2021 3:16 AM
> > >>> To: devel@e= dk2.groups.io; Ni, Ray <ray.ni@i= ntel.com>
> > >>> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Cha= sel
> > >> <chasel.chi= u@intel.com>; Desimone, Nathaniel L
> > >>> <nathaniel.l.desimone@intel.com>; Luo, Heng <heng.luo@intel.com>;
> > >> Agyeman, Prince <prince.agyeman@intel.com>;
> > >>> gaolimi= ng@byosoft.com.cn; Dong, Eric <eric.dong@intel.com>
> > >>> Subject: Re: [edk2-devel] [edk2-platforms][RFC] Spi= FlashCommonLib
> > >> Refactor
> > >>>
> > >>> Hi Ray,
> > >>>
> > >>> That sounds reasonable to me.
> > >>>
> > >>> I was attempting to preserve the design that isolat= es the
> > >>> silicon-specific logic to a library via an interfac= e to a silicon
> > >>> package. However, the real abstraction here is the = firmware volume
> > block
> > >>> protocol which could simply be produced by a silico= n driver with the
> > >>> separation of such logic to a library being an impl= ementation detail of
> > >>> the driver.
> > >>>
> > >>> In summary, here is the updated proposal:
> > >>>
> > >>> 1. Consolidate the library interface into a single = header in
> > >>> IntelSiliconPkg.
> > >>>
> > >>> 2. Consolidate the library implementation into a si= ngle instance in
> > >>> IntelSiliconPkg.
> > >>>
> > >>> 3. Move SpiFvbServiceSmm out of MinPlatformPkg into= IntelSiliconPkg.
> > >>>
> > >>> 4. Add SpiFvbServiceStandaloneMm to IntelSiliconPkg= sharing
> > >>> implementation with SpiFvbServiceSmm where appropri= ate.
> > >>>
> > >>> Intel board packages would then use the SpiFlashCom= monLib from
> > >>> IntelSiliconPkg (a generation specific instance cou= ld be created if
> > >>> needed) and use the SpiFvbServiceXyz driver from In= telSiliconPkg.
> > >>>
> > >>> Please let me know if this is acceptable and I'd be= happy to send the
> > >>> patches.
> > >>>
> > >>> Thanks,
> > >>> Michael
> > >>>
> > >>> On 3/1/2021 1:07 AM, Ni, Ray wrote:
> > >>>> Michael,
> > >>>> I agree with your thoughts to consolidate the l= ib header and
> > >> implementation to IntelSiliconPkg.
> > >>>> I didn't read the different implementations. Bu= t the implementation
> > >> consolidation means you see all the existing
> > >>> implementations are the same. Right?
> > >>>>
> > >>>> But why don't you put the driver in IntelSilico= nPkg as well? Creating an
> > >> advanced feature for this fundamental service seems
> > >>> over-kill.
> > >>>>
> > >>>> Thanks,
> > >>>> Ray
> > >>>>
> > >>>>> -----Original Message-----
> > >>>>> From: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com><= br> > > >>>>> Sent: Monday, March 1, 2021 4:46 PM
> > >>>>> To: Ni, Ray <ray.ni@intel.com>
> > >>>>> Subject: RE: [edk2-platforms][RFC] SpiFlash= CommonLib Refactor
> > >>>>>
> > >>>>> Did you get a chance to look into this ? > > >>>>>
> > >>>>> -----Original Message-----
> > >>>>> From: Michael Kubacki <mikuback@linux.microsoft.com>
> > >>>>> Sent: Tuesday, February 16, 2021 4:58 PM > > >>>>> To: devel@edk2.groups.io; Ni, Ray <= ray.ni@intel.com>; Chaganty,
> > >> Rangasai V <rangasai.v.chaganty@intel.com>; Chiu,
> > >>> Chasel
> > >>>>> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> > >> <n= athaniel.l.desimone@intel.com>; Luo, Heng <heng.luo@intel.com>;
> > >>>>> Agyeman, Prince <prince.agyeman@intel.com>;
> > >> gaoliming@b= yosoft.com.cn; Dong, Eric <er= ic.dong@intel.com>
> > >>>>> Subject: [edk2-platforms][RFC] SpiFlashComm= onLib Refactor
> > >>>>>
> > >>>>> Hello,
> > >>>>>
> > >>>>> I'm planning to submit support for Standalo= ne MM in
> > >> SpiFlashCommonLib soon. Currently, there's quite a bit = of duplication
> > >>> with
> > >>>>> SpiFlashCommonLib.
> > >>>>>
> > >>>>> I would like to have this Standalone MM sup= port be available in as
> > >> consistent of a location as possible so I'd like to see= if
> > >>> there is
> > >>>>> anything I can do to help clean this up in = the early part of the patch
> > >> series.
> > >>>>>
> > >>>>>
> > >>>>> The library interface is currently defined = in the following header files:
> > >>>>>
> > >>>>> 1.
> > >> Platform\Intel\MinPlatformPkg\Include\Library\SpiFlashC= ommonLib.h
> > >>>>>
> > >>>>> 2. Silicon\Intel\SimicsIch10Pkg\Include\Lib= rary\SpiFlashCommonLib.h
> > >>>>>
> > >>>>> 3.
> > >>
> > Silicon\Intel\KabylakeSiliconPkg\Pch\Include\Library\SpiFlashCom= monLib.h
> > >>>>>
> > >>>>> 4.
> > >>>>>
> > >>
> > Silicon\Intel\CoffeelakeSiliconPkg\Pch\Include\Library\SpiFlashC= ommonLib.
> > >> h
> > >>>>>
> > >>>>>
> > >>>>> Instances of SmmSpiFlashCommonLib implement= ation exist in a mix
> > of
> > >> platform and silicon packages:
> > >>>>>
> > >>>>> 1.
> > >>>>>
> > >>
> > Silicon\Intel\SimicsIch10Pkg\Library\SmmSpiFlashCommonLib\SmmSpi= FlashC
> > >> ommonLib.inf
> > >>>>>
> > >>>>> 2.
> > >>>>>
> > >>
> > Platform\Intel\TigerlakeOpenBoardPkg\Library\SmmSpiFlashCommonLi= b\S
> > >> mmSpiFlashCommonLib.inf
> > >>>>>
> > >>>>> 3.
> > >>>>>
> > >>
> > Silicon\Intel\KabylakeSiliconPkg\Pch\Library\SmmSpiFlashCommonLi= b\Smm
> > >> SpiFlashCommonLib.inf
> > >>>>>
> > >>>>> 4.
> > >>>>>
> > >>
> > Silicon\Intel\CoffeelakeSiliconPkg\Pch\Library\SmmSpiFlashCommon= Lib\Sm
> > >> mSpiFlashCommonLib.inf
> > >>>>>
> > >>>>> 5.
> > >>>>>
> > >>
> > Platform\Intel\MinPlatformPkg\Flash\Library\SpiFlashCommonLibNul= l\SpiFla
> > >> shCommonLibNull.inf
> > >>>>>
> > >>>>>
> > >>>>> The library class is currently consumed in = the following INFs:
> > >>>>>
> > >>>>> 1.
> > >>
> > Platform\Intel\MinPlatformPkg\Flash\SpiFvbService\SpiFvbServiceS= mm.inf
> > >>>>>
> > >>>>> 2.
> > >>>>>
> > >>
> > Platform\Intel\MinPlatformPkg\Flash\SpiFvbService\SpiFvbServiceS= tandalon
> > >> eMm.inf
> > >>>>>
> > >>>>>
> > >>>>> My understanding is:
> > >>>>>
> > >>>>> 1. The header file is defined in each silic= on package because silicon
> > >> cannot depend upon platform (i.e. the MinPlatformPkg > > >>>>> header).
> > >>>>>
> > >>>>> 2. The header is present in each silicon pa= ckage because the
> > >> implementation is silicon-specific and these packages c= annot
> > >>>>> depend on one another.
> > >>>>>
> > >>>>> 3. The header is defined in MinPlatformPkg = because MinPlatformPkg
> > >> should be silicon vendor agnostic (cannot depend on the=
> > >>>>> silicon packages).
> > >>>>>
> > >>>>> 4. The header is needed in MinPlatformPkg b= ecause the
> > SpiFvbService
> > >> there depends on SPI flash operations implemented in > > >>>>> SpiFlashCommonLib.
> > >>>>>
> > >>>>>
> > >>>>> Here's an initial proposal:
> > >>>>>
> > >>>>> 1. Consolidate the library interface into a= single header. In
> > >>>>> IntelSiliconPkg?
> > >>>>>
> > >>>>> 2. Consolidate library implementation into = a single instance. In
> > >>>>> IntelSiliconPkg?
> > >>>>>
> > >>>>> 3. Move SpiFvbServiceXyz out of MinPlatform= Pkg.
> > >>>>>       3.a. Ma= ke a "SPI flash" feature?
> > >>>>>       3.b. Al= low the Intel implementation of this feature to depend on
> > >>>>> SpiFlashCommonLib defined in IntelSiliconPk= g.
> > >>>>>
> > >>>>> Intel board packages could then use the Spi= FlashCommonLib from
> > >>>>> IntelSiliconPkg (a generation specific inst= ance could be created if
> > >>>>> needed) and use the SpiFvbServiceXyz driver= from the "SpiFlash"
> > >> feature.
> > >>>>>
> > >>>>> Look forward to your thoughts and feedback.=
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Michael
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >
> > >
> > >
> > >
> > >


 

--_000_CO1PR11MB49301002ACD7B6CD859323BD8C979CO1PR11MB4930namp_--