From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (NAM12-BN8-obe.outbound.protection.outlook.com [40.107.237.94]) by mx.groups.io with SMTP id smtpd.web12.1495.1614815951136567624 for ; Wed, 03 Mar 2021 15:59:11 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=Rwm7ycOm; spf=pass (domain: microsoft.com, ip: 40.107.237.94, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZbmLw4SZm+i38k1iMb82KWyOcRLkX4sgE1Dw3KmrT4f886QVZHlTIMZN8smzlGG5X33wT2eoRfthp0wipy9HMe+w6gn/rlKDz7Z/6ZRupdeciGlweah79pVeSFn0WMrbHL9H1pdqbafy9ibTugEpiAJSiPLbtOEAjAjedXIPEOHpY2unG91mEWBkWwMclo7HUJCTuzLX2bV7Zd7vZaoZLB6KZhCKTpQ3UjU7Jexh4/u6mYgVRuvaZmAkS062MvwlCohdwykaqQQZS+laBNTFdXdBVl0ttBDRFtW/gylXXnQwuu+ty4YWUYL/aELUko4bvGNtNQKNtwEf29eB+S52wg== 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=ehYIZ+cLT0F3plDLg1N8E44CLDgErv50Kcxh4IYGFHw=; b=PEM9XO7uwmpE+2bbGiKosCKaxrHtzDk2/O8umJAEpNHF0C+71TQEmf4PnOnS4Nma5zeuQLoQ9XifCaAHyr7emeeAgm5lsC2xH5UDi7x/l1qdzjF555xSYpoB1S7+RNm8pESpSJFTcLtwBVj75Myt7wjX0+9po0ZYeEUsMdxTVbNF6KagmlTFbfxMcCdrmmEPqcDhNkUslWyR2dcj2oat6cB316Gysa25X0PDnxkVnn1/W8pr0cpkOARC5bLZtyLBrZeQpfy/z8udyJuH8MAmfoi9HQcDat2f04US++UsDm1ok+9MNTcIdUxq7rDa6o5O4x3L+lJZ4v0Nm05d5kHhaQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ehYIZ+cLT0F3plDLg1N8E44CLDgErv50Kcxh4IYGFHw=; b=Rwm7ycOmzJXEFgQfjEWniaRdh91c6hXEWi4/GDkKLo1NkPMPrCF0XGUXuNc7S+DZ/+z8yA+Hv7KU8E8EhCX0IxB8tZ4koVeCeqU21N60frWDU2KXaUC9/VAvKALMy0ejgagv4zSBSTiUEFkckmfsExo7r8Z1LqdTqz7UsQIgAQQ= Received: from MW4PR21MB1907.namprd21.prod.outlook.com (2603:10b6:303:71::8) by MW4PR21MB1859.namprd21.prod.outlook.com (2603:10b6:303:7f::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3933.8; Wed, 3 Mar 2021 23:59:06 +0000 Received: from MW4PR21MB1907.namprd21.prod.outlook.com ([fe80::8ddb:dbee:7757:2d6a]) by MW4PR21MB1907.namprd21.prod.outlook.com ([fe80::8ddb:dbee:7757:2d6a%7]) with mapi id 15.20.3933.013; Wed, 3 Mar 2021 23:59:06 +0000 From: "Bret Barkelew" 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 Thread-Topic: [edk2-devel] [edk2-platforms][RFC] SpiFlashCommonLib Refactor Thread-Index: AQHXEIQQyB5LEDRshku877q1/2yI06py63gAgAAFkdw= Date: Wed, 3 Mar 2021 23:59:06 +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 authentication-results: edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [71.212.153.143] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 24477125-ceee-47c7-c136-08d8dea05493 x-ms-traffictypediagnostic: MW4PR21MB1859: x-ms-exchange-transport-forked: True x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr 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: /m2qjpeCK+qnNUljzVvneo/nvx5HAD5iZN/Ttbh+QtJ4VVjsvWhhlNkvwxI56hTdvRsawV8ZRRh2bQRGqK99V2EcRRNC5YUt20J77bZVCcHAk6ngJeirfJ+4y8vpDCyb0JRfLzBpCSnu6hxiK3EjJkVwR/aJkurVSVGhS2s95m9H5UxkmV1EfTe2W29Rpv1mK+yDELeUB2vB8Y/kueGP/jHG4OXtSPR1ponXjUPEfbqRqX41VLpRnF4G3AhmwSoPA+xVFd7eTxH5ctu2sonB3Zyz2A/blfZQtZTq1M8+j4OmY3t9rtW+uqURNN3ECF1ZhvZ8x7QLyztq03T78QEtcD4sjqYTqqNWbs59P0FXJfPBJVSf/vdqtIp1t8+npMVzB6PZlhtQUa19/wqk5Gv+g9F1E1Tek0oETdKLd/GpTgESENEQckDwbWZmeiCjOY7bxLiIS9FBuqDzbpy05oTuQiNfy2AWKS1CUa8/2qpL+dbHvx9sWnPrDw5SYAmLGDv2AAHYaJDavYVI9LutjnR7w4hIXNlFgCesmLJG1HK6kDeeBIC8cfWZQq1m9CboFB9/n47UJl5fwAsb63hj0/vV1Qr+FhIVaFD6WwZBy/touNpQKaJaR9DwqHsrNG19jJAlNnV79ogjIyRTpw67+XXPzg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR21MB1907.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(396003)(346002)(39860400002)(376002)(366004)(136003)(52536014)(26005)(4326008)(966005)(5660300002)(8936002)(478600001)(7416002)(186003)(53546011)(55016002)(8990500004)(6506007)(82950400001)(7696005)(10290500003)(2906002)(30864003)(33656002)(71200400001)(66556008)(8676002)(54906003)(82960400001)(316002)(64756008)(66476007)(66446008)(76116006)(86362001)(166002)(66946007)(110136005)(9686003)(83380400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?O/f8lnVCdwZy/bBqOOjleBL9qPbRKwT1QmiASyEiu+PSD4HBzfJbgdXh5okA?= =?us-ascii?Q?xDkyW+tFIPXw/GatwtSajkTbFrIZZmBQalY1rYJxwDggZXgvjrJn+9cQgx4/?= =?us-ascii?Q?T9gRFTjetiZOjKoemUtAN3hoYMYOMHg+4AHa5w2yu0QWMKS24jMYW/l6Uipf?= =?us-ascii?Q?WG+pJwLU1nWYQQzdBzwQ9JRylN80WAGryR9PqFhzWKz4Ro3yPgbFBjl3AyqG?= =?us-ascii?Q?00LQ66ARSWnIrURE09SbLM6XCS8MXEotqS3U+pagaVNeMrH5hj8wrlEyJ73l?= =?us-ascii?Q?EZ0Qcr7YwWy2VlM7d4nbM19aRjzfb+TKYOzjH3B/xgv20sk5Z6tSKVcIn2vg?= =?us-ascii?Q?R8nE4ZbpjJPrHq3ZX+QZiKGNMPW6UwG77y6W0UcU/M37lPoGWJqQjx81Mhep?= =?us-ascii?Q?rffK4EZCMADTjaA90IY95VARhCtcQ0jB9sIW7Z+Uly1/OCP9NnMVommnPj8f?= =?us-ascii?Q?kvXyUQ570Nv530c132fnk2EMQ6tJ5Y7leMporM6VfAljVD4iMOwojWmHPFmF?= =?us-ascii?Q?M1+88ITcTe1XeG/WVUjPxKKwmaMCC04gRMGlChUuis4XbI0d/TxlFXB6YkBB?= =?us-ascii?Q?ctp0QsZD+9zhLo1Pq4hr5Ugypsg49val2KXvGwkvt3Rxig/Wve8k1uozzd3C?= =?us-ascii?Q?h/Dnnfg51Bn5CMVqrMLMAr0JFXeQaFhfOz7UesuLpy5/+yFE5IVnGmALfNtA?= =?us-ascii?Q?+CdvOxlkJ+r8CJ3uV1I+blhX35o8ZkSnMHzUYs9X1S9Gts6QbRk1T07+X7ZT?= =?us-ascii?Q?eLwKHVx7QfmxrRYnm1Het8Mpp+thnYAXkeSoFKPs1rQc+U5jbVXKchowFe+f?= =?us-ascii?Q?jhKrOhk3uzxDfnUOO+ZPnwaNfkYjW0MEYK07LwKLjXXPWTCaQUOhuPEDaEbx?= =?us-ascii?Q?RoBoBIHqxBIxEZ1KG7w2Aoude/7DfZ76Xu5qg2D6dBIzHkAbnHBF3hbAJiJ0?= =?us-ascii?Q?Zp4m/jtRyVsSVa+fD2zDRaCBwi0rCwrNtJ0flNwrMz4IETGEcLd8trOnWVUr?= =?us-ascii?Q?eLYlJB+nM/HiChV41zuNVL96hmNGcvFZr+tknAMVJN9bgsuLMcC6On7v55Lz?= =?us-ascii?Q?jC6if5whHPb3YKH1kbucxvukxZRsjGNI2WIk9iTljs8hUGHxIGTPOURVuzYJ?= =?us-ascii?Q?ZLRzxHIlRnPRv4kDBBC9p+FqcSALivtGq1o6HgRQT2ERQJP2nMiARYPLArU9?= =?us-ascii?Q?QisDJd5jZqnXV61dmJW8igziwQFf3PtJe/5bNxAv/uTMkQreMcOZUyHVsb+R?= =?us-ascii?Q?vK6CKCcvvfhVP71ePg29u9+xfOXvMQBSA/0P1QTNAJHMkaFvnqPeh1JLRHdd?= =?us-ascii?Q?WyiqycHJ2aImbJCi/MuWcryhV4Wzp2bPZK8RaPMhsqWX2Q=3D=3D?= MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR21MB1907.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 24477125-ceee-47c7-c136-08d8dea05493 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Mar 2021 23:59:06.6694 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: pIBaH6BqLdPBmNw4dXvgpOy+0ooMgTwAao05QlVOxb2/r9Ybf2tufrgu6XT8RbVpFF0WEm64UfPB7RM77fSDGg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR21MB1859 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MW4PR21MB1907F7E0E689982E474D40B5EF989MW4PR21MB1907namp_" --_000_MW4PR21MB1907F7E0E689982E474D40B5EF989MW4PR21MB1907namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Furthermore, is it the code that cannot cross the repo boundary, or the int= erface? 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 <= chasel.chiu@intel.com>; Desimone, Nathaniel L > ; Luo, Heng ; Agyema= n, 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, Nathaniel 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, Cha= sel > > >> ; Desimone, Nathaniel 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, Ch= asel > > >> ; Desimone, Nathaniel L > > >>> ; Luo, Heng ; > > >> Agyeman, Prince ; > > >>> gaoliming@byosoft.com.cn; Dong, Eric > > >>> 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 ; Chaganty, > > >> Rangasai V ; Chiu, > > >>> Chasel > > >>>>> ; Desimone, Nathaniel L > > >> ; Luo, Heng ; > > >>>>> Agyeman, Prince ; > > >> gaoliming@byosoft.com.cn; Dong, Eric > > >>>>> 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_MW4PR21MB1907F7E0E689982E474D40B5EF989MW4PR21MB1907namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

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@intel.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, Nathaniel L
> <nathaniel.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 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.com>; = Ni, Ray
> > <ray.ni@intel.com>
> > Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; = Chiu, Chasel
> > <chasel.chiu@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>; Luo, Heng <heng.luo@i= ntel.com>;
> > Agyeman, Prince <prince.agyeman@intel.com>; gaoliming@byos= oft.com.cn;
> > 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 <ray.ni@intel.com>
> > >> Sent: Monday, March 1, 2021 5:52 PM
> > >> To: Michael Kubacki <mikuback@linux.microsoft.com>= ;;
> > >> devel@edk2.groups.io
> > >> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.= com>; Chiu, Chasel
> > >> <chasel.chiu@intel.com>; Desimone, Nathaniel L > > >> <nathaniel.l.desimone@intel.com>; Luo, Heng <h= eng.luo@intel.com>;
> > >> Agyeman, Prince <prince.agyeman@intel.com>;
> > gaoliming@byosoft.com.cn;
> > >> Dong, Eric <eric.dong@intel.com>; Dong, Guo <g= uo.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@edk2.groups.io; Ni, Ray <ray.ni@intel.= com>
> > >>> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@in= tel.com>; Chiu, Chasel
> > >> <chasel.chiu@intel.com>; Desimone, Nathaniel L > > >>> <nathaniel.l.desimone@intel.com>; Luo, Heng &= lt;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] 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.c= haganty@intel.com>
> > >>>>> 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.mi= crosoft.com>
> > >>>>> Sent: Tuesday, February 16, 2021 4:58 PM > > >>>>> To: devel@edk2.groups.io; Ni, Ray <ray.n= i@intel.com>; Chaganty,
> > >> Rangasai V <rangasai.v.chaganty@intel.com>; Chiu,=
> > >>> Chasel
> > >>>>> <chasel.chiu@intel.com>; Desimone, Na= thaniel L
> > >> <nathaniel.l.desimone@intel.com>; Luo, Heng <h= eng.luo@intel.com>;
> > >>>>> Agyeman, Prince <prince.agyeman@intel.co= m>;
> > >> gaoliming@byosoft.com.cn; Dong, Eric <eric.dong@inte= l.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_MW4PR21MB1907F7E0E689982E474D40B5EF989MW4PR21MB1907namp_--