From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web10.42467.1683660605016825384 for ; Tue, 09 May 2023 12:30:05 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=SEAJhvCB; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: chasel.chiu@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1683660605; x=1715196605; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=kI7jD4dhaz2csxpaZOwQtIp8jtrx9b7v3Ch86jnbauM=; b=SEAJhvCBUJZy+LAGPZmGwBkWpibXl6tM5knxQAxQiLkxt3PctNvZ5W03 IImYeC9CwUAfqI0DMaaJT1YHWnaHqfK0rWnMos+bTHY6gSiWBnc6sKuzn vHeppx5uQJ52uqhO2sBU/lCZ4oJW6PhTJ0osvb1w6zoNEWDSMs2NQD0Q+ kWU8iTO1zoLJPGtO9V8i+di6H0R34CFvI6JbWZ/iFMJ4cKJVgTnphhYKq j0CQmo7DaF9bOzO/Z4AWAaLWqikv2i7FkhHFCJ6OfO6wB3/CL8xw1t2AL IqBZOYiC67ZZ4S78x3Zk/s/hIXXK6SkwiQaPyrLe3DRY6efxXCUwUuFjk g==; X-IronPort-AV: E=McAfee;i="6600,9927,10705"; a="415603550" X-IronPort-AV: E=Sophos;i="5.99,262,1677571200"; d="scan'208";a="415603550" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 May 2023 12:30:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10705"; a="693106263" X-IronPort-AV: E=Sophos;i="5.99,262,1677571200"; d="scan'208";a="693106263" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga007.jf.intel.com with ESMTP; 09 May 2023 12:30:03 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 9 May 2023 12:30:03 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 9 May 2023 12:30:03 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Tue, 9 May 2023 12:30:03 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.172) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.23; Tue, 9 May 2023 12:30:03 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EbgCCAYSN2Zb7wpZK8xKmn8jKqSdZWeEBNLt92iB7VJhyPgeHbKuYUztVB50GPYkm1k0bv8WUQNHkpX30wqbtiRFdNe9D+HBjVp7fVRhbFuoqInFdJHZcCR0bixhM+3TYaOVzDGbfWwuOfq6SF9Nb1HrttVku8QStA2t4dUie3tqzrhZxXSpWpr9CdaV1wdTPkmlAu/4wYJ4TO7JWahkImTN0DcxeAcEBHX2NV4Y2ZEhjf6HTfMVNd3g3GQAEH7/nFk/ozvbGgGz1BdoTDg4uMJeq/kdr6QYTLjEZWOymlyGEAJmgFWij5ruR3AjVNuvlIGxKsvlF6gyRVx2iEiWaA== 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=HgljkhxSKzIbcMrhcaQaouNIlERg6fsrBI+zdZMvrkk=; b=GHXdyr03YBZR/tRBlJ4OhAUMW30RYrypGkmZMVQ8VTCut+bVm5xU75mawFvgnoBA7ObwQ+2yBWKfj8ZYWm/EqBoJtas4bsyCad6je6pkHU85wXZ/K+2vDhC4BPZrlXDtE5PrBelrOXfRirXTFlzUloW6QnGlAg+UQBVoTM26BBxc19yAitzNmbJBWzEXigsDOE+ktZnz+a+WI1BPqcEoHSH7tBu/rpaHfuKGjcr2Lp32EJeNG8idxqM7lWZ4A846T7kujLtMNmEy/ymHAdIit6fl2spG/YA9KYrpnIooOl1vXg5TLyPeMeiCikhwn9bvu7tuXoGpmVAHa38YRSUqPg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from BN9PR11MB5483.namprd11.prod.outlook.com (2603:10b6:408:104::10) by PH7PR11MB6834.namprd11.prod.outlook.com (2603:10b6:510:1ed::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6363.33; Tue, 9 May 2023 19:29:59 +0000 Received: from BN9PR11MB5483.namprd11.prod.outlook.com ([fe80::1eb1:2222:1823:8e7a]) by BN9PR11MB5483.namprd11.prod.outlook.com ([fe80::1eb1:2222:1823:8e7a%7]) with mapi id 15.20.6363.033; Tue, 9 May 2023 19:29:59 +0000 From: "Chiu, Chasel" To: "devel@edk2.groups.io" , "Kubacki, Michael" , Michael Kubacki , "Gudla, Raghava" , "Oram, Isaac W" , "Kinney, Michael D" CC: "Desimone, Nathaniel L" , "Chaganty, Rangasai V" Subject: Re: [edk2-devel] [edk2-platforms:PATCH V1] MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. Thread-Topic: [edk2-devel] [edk2-platforms:PATCH V1] MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. Thread-Index: AQHZf64xuQvoctZH9EWbk8VzvnYjSq9Qw4GAgAACr4CAACghgIABJ2iAgAAkD4CAAAGygIAABvCAgAALPxCAAAjKgIAAAtgA Date: Tue, 9 May 2023 19:29:59 +0000 Message-ID: References: <20230506000300.389-1-raghava.gudla@intel.com> <51099a60-02d6-5bb0-6e9a-e3fbcf6a643c@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_ActionId=29460a78-2b19-408c-a4ac-19ba45bd7974;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=true;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=Internal;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2023-05-09T19:14:22Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47; authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: BN9PR11MB5483:EE_|PH7PR11MB6834:EE_ x-ms-office365-filtering-correlation-id: 879f3991-cfdb-4539-ffdb-08db50c3c740 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: FW7HjBBcTiEbtYYQ4DiiwAVpJ6Wfsr8cvrKLEUkrdPq7CMUX6eMqzlayddXB8D6nTA+4/LhpuSnA1ofxvRZsAa7fO5QK7slss/aTELokOkRe+kDwdLHZZvcZrsIGATfBV1YffwDD4q8Ro1yw8ocCg9OuOsSUANA84ls0BaTm5rux2OYSZTngPpmuIo4Dpit5cKy6pCVeif/iUg5+agT6F0XTys8EYQ3yn1c4mF1qgirsBUHgNrJkJ7QZCP2BdAMo1BM6/AgXsp8JntdqMrQsHgwV4uJE7WqnJCCUHA/FQN7vUHxUR742JAJLSEEXzl8iSB084/bt6JWZ/nLB1XPcEqfNGg8s0ZPg2FteixyYGD7kWzZiuq2c8FBro5tOXYnjQWTjslPn2+iO/IB905Qwmfn4A4e6oMsCWR/xlxSiQ3MrjoGrr8G53z0SyP/fJG5J+d/LPUuYdJk3CecuSfPGZtbCEEYh/FwT1TZMyhwxSY2/5LFNIpRl8VN8WWuY47BgbjDu02eHFfj1aC9y4k5Nbzr/U4kDk7rbgAPZzKzOIPS0mMUv9danz02NDaY9PzNfj1z3uZiz8Gv0Nb1q/j2o0LmUlsR/8IsCNtu+jxRSL3bi5UXSEnqhRtGs9cc6S9aEZEChh07xW+blKPKH4ln4GsGsbkN/lr2dvbTNWSF5bPc= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN9PR11MB5483.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(39860400002)(366004)(396003)(376002)(346002)(136003)(451199021)(478600001)(7696005)(55016003)(966005)(83380400001)(107886003)(4326008)(82960400001)(41300700001)(38100700002)(921005)(53546011)(54906003)(71200400001)(122000001)(6506007)(110136005)(66476007)(66556008)(66946007)(9686003)(64756008)(66446008)(26005)(86362001)(52536014)(38070700005)(5660300002)(316002)(6636002)(8676002)(8936002)(19627235002)(33656002)(45080400002)(186003)(66899021)(30864003)(2906002)(76116006)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?WczmNAYHUU4RcylZ/KmtsgERP7TMp7vJEdDLQ4nV3e8KNEQ1vvih+Bymeffd?= =?us-ascii?Q?EC6bqYnHM0AY4hIsRBfhHfr3ND6xHpHYbTOoUgB46xM9Q3axy7zyCY0RuSAR?= =?us-ascii?Q?LJDG0DZzIbrh2vLhIfri+l/TWBpYJ4WEJdkXdY+AZnEf+DpGIhhgyaGHDcyi?= =?us-ascii?Q?xSGIw85RqtvUkjQBxmd3Qc57P23eQpY3oFnXClIl+BvgHIxosdNp2zra2nP2?= =?us-ascii?Q?pqkDLaEMeFqhxhbc4KSWZfb9X5eaN+AYSfwsKcuUPlXA6ImO6nrRqhKznRgX?= =?us-ascii?Q?ES4gnK8ZEvnQmfXhdIlgbrXhBk4nTOF6NgJ00599Ty86Z/Umxm3+PTHEGHXY?= =?us-ascii?Q?7nz/2zMHsJowelf2I57JAHwSFUkK3kfjv/VvQw0BIfPITCbbWPT8twta//PO?= =?us-ascii?Q?cGC05ZRgxxTGX0K8VWGljZcpGZPI4QyBZKETJz8vM4DFdJWfDKRCOtFP2G02?= =?us-ascii?Q?sin4yfGLUBPNQ2auI7Sj2l6T+k+hOmItxDyz2TDPqyaY3xnn72gWtRyYW51a?= =?us-ascii?Q?KyY0fFlkTiqtv5uyrZklL+M3+2sVpT5yj1EP4kchCYe5mWciQWYERXnPlCl2?= =?us-ascii?Q?S4qoMCocFjAghynksaYv6+91CZasT+YsV+qJElf1A8KvkGysEcgonYUsOaHj?= =?us-ascii?Q?bVlqEkmJBCGck5Zt6uLAJYlck6/Hth/PZl9V4I4Um5BKzKcVnmppmkUoD1iw?= =?us-ascii?Q?lK/6R076e+s6IvbUVYa6JsO5Y9NHsoy2nDQTDYJMT3PgIeFI5qCoeylxErov?= =?us-ascii?Q?k7wGikOzl0wgdWch2wUitof+CPYQgxYFRFcEYNDkTmRgJj6Zw9c4+G0VsJLH?= =?us-ascii?Q?wr4oDSrhVm6mOUPXSB/CBbPmtRJa6gBY9oYkxLlCz/kN87OFKiOGmKhT/qAI?= =?us-ascii?Q?7+eApzCQY2zN9+nY3mvc9o/DMiyvZ9+YidC+OtZHZGMWatTMQEtZ0h0w8nY/?= =?us-ascii?Q?UFqKscilBVODSsQ8c59J7FYvpYyvmGvDB0/va+sDlmvQJxZMGjBN6giOyh2n?= =?us-ascii?Q?y2FRhVIUVyj9D1ZIqSPF/Rd4lkQTZaEsaVZJ7KC+78WUEu/EAs97hdzLk48T?= =?us-ascii?Q?jfsOe1NZoJ3G3JsmNWPK84hs18fB5cWd04g0wKKPQmS3hH6REyZqx8RL8LQW?= =?us-ascii?Q?mchIswWEdsRdCtknWNpzIjE2A1JhJ+ZACltae1yzs0PIWJNeAMcIk0tl/pEY?= =?us-ascii?Q?9h5O0w7duEz5LjPIbgj/O4/fZofIgk4UCmZRA32WiKb8wmaVmNNWWHB9FTON?= =?us-ascii?Q?y2LQ5UQHOfg81hH+L90zn+78+lSnaHxDC7/sgQrlRBQk98PAGB+9S4sTVK9X?= =?us-ascii?Q?rDlN3TgNiLcFBwW8NoT3IbAH8aLH+3JJgFZLtBMDIGlLPSOWKLrwPktYpqK8?= =?us-ascii?Q?5tCCBilV0ioMdl7UDTPAdwSiSRVS3GBDLYHO3Et0NEVMI3Fanf4FidFH/PRI?= =?us-ascii?Q?77t+LRR7pA9POh4hzHmvdzo7xnIDCPVGls3Br265lTHG3cP0OjPM9PdYaAYX?= =?us-ascii?Q?+8Oyh9tBPK62TClm3ffiVxkNhkfwEOIkIaDiq3P/eSJjiL6OnFVyQYTua5A8?= =?us-ascii?Q?hWShX2A+7naFUQRz2cwCjtE9cnKrxF5S06kT3Yrv?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5483.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 879f3991-cfdb-4539-ffdb-08db50c3c740 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 May 2023 19:29:59.4718 (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: 3G++jUAl+i+KjT/cxtTJVEBNqnSLlvpyEkzAjPGxcsnklhdxOYUT5OoUSfh6Wz0rFybHwuXnIrrKRB8RIE0r8g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6834 Return-Path: chasel.chiu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Just reviewed the branch code and it is functionality the same so agree we = can switch to Mike's implementation. Would you help to rebase and send updated patch for reviewing? Thanks, Chasel > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Michael > Kubacki via groups.io > Sent: Tuesday, May 9, 2023 12:18 PM > To: Chiu, Chasel ; Michael Kubacki > ; Gudla, Raghava ; > Oram, Isaac W ; devel@edk2.groups.io; Kinney, > Michael D > Cc: Desimone, Nathaniel L ; Chaganty, > Rangasai V > Subject: Re: [edk2-devel] [edk2-platforms:PATCH V1] > MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. >=20 > I haven't done a deep comparison of the changes, but I'd lean toward usin= g > what is in the branch that I referenced earlier since it has been used fo= r a while > now. >=20 > Is there anything of value in the new set of changes? >=20 > Thanks, > Michael >=20 > -----Original Message----- > From: Chiu, Chasel > Sent: Tuesday, May 9, 2023 2:56 PM > To: Michael Kubacki ; Gudla, Raghava > ; Oram, Isaac W ; > devel@edk2.groups.io; Kinney, Michael D > Cc: Desimone, Nathaniel L ; Michael Kubac= ki > ; Chaganty, Rangasai V > > Subject: [EXTERNAL] RE: [edk2-devel] [edk2-platforms:PATCH V1] > MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. >=20 >=20 > Hi Michael Kubacki, >=20 > Since Rahava has rebased the patch it might be easier to add " Signed-off= -by: > Michael Kubacki " to Rahave's patch commit > message so both of you are authors. > What do you think? >=20 > By the way, I found a bug in SaveMemoryConfig.c: FreePool() should be > FreePages (CompressedData, CompressedAllocationPages); Raghava, please > help to fix it by sending V2 patch (adding -v2 to git format-patch comma= nd) >=20 > Thanks, > Chasel >=20 >=20 >=20 > > -----Original Message----- > > From: Michael Kubacki > > Sent: Tuesday, May 9, 2023 11:06 AM > > To: Gudla, Raghava ; Oram, Isaac W > > ; devel@edk2.groups.io; Kinney, Michael D > > ; Chiu, Chasel > > Cc: Desimone, Nathaniel L ; Kubacki, > > Michael ; Chaganty, Rangasai V > > > > Subject: Re: [edk2-devel] [edk2-platforms:PATCH V1] > > MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. > > > > Thanks for reraising this. If you need me to rebase or help in anyway > > let me know. > > > > On 5/9/2023 1:41 PM, Gudla, Raghava wrote: > > > My patch is based on Michael's fix. I took his change to do a PoC a > > > while back > > and totally forgot that his patch is still available. We can proceed > > to merge Michael's patch. > > > > > > Thanks, > > > Raghava > > > > > > -----Original Message----- > > > From: Oram, Isaac W > > > Sent: Tuesday, May 9, 2023 10:35 AM > > > To: devel@edk2.groups.io; mikuback@linux.microsoft.com; Kinney, > > > Michael D ; Chiu, Chasel > > > ; Gudla, Raghava > > > Cc: Desimone, Nathaniel L ; Kubacki, > > > Michael ; Chaganty, Rangasai V > > > > > > Subject: RE: [edk2-devel] [edk2-platforms:PATCH V1] > > MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. > > > > > > Rereading that thread and this request, other opinions, etc, I am > > > convinced > > that the simpler interface and board responsibility is the correct > > short/medium term answer. I don't have an opinion on Mike Kinney's > > question on encapsulated services. I generally like that design, > > though I am sensitive to Michael Kubacki's feedback that variable > > services are too complex as it is. I guess it probably depends a lot > > on the specifics of the proposal. That said, it is pretty easy to > > migrate from a board specific solution to a more base layer solution in= the > future, so adopting this now doesn't seem harmful to me. > > > > > > Raghava, can you look at Michael's fix? It looks nearly identical > > > to yours and > > general convention is to accept the earlier one in case of collision I > > believe. I like his PCD naming a little better, but both are fine to m= e. > > > > > > Regards, > > > Isaac > > > > > > -----Original Message----- > > > From: devel@edk2.groups.io On Behalf Of > > > Michael Kubacki > > > Sent: Tuesday, May 9, 2023 8:26 AM > > > To: devel@edk2.groups.io; Kinney, Michael D > > > ; Chiu, Chasel ; > > > Oram, Isaac W ; Gudla, Raghava > > > > > > Cc: Desimone, Nathaniel L ; Kubacki, > > > Michael ; Chaganty, Rangasai V > > > > > > Subject: Re: [edk2-devel] [edk2-platforms:PATCH V1] > > MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. > > > > > > At the surface, this looks similar to the following patch I sent a > > > while > > > back: > > > > > > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fe= d > > > > k2.groups.io%2Fg%2Fdevel%2Fmessage%2F92644&data=3D05%7C01%7Cmichael. > ku > > > > backi%40microsoft.com%7C05c93413c875400acef708db50bf197d%7C72f988bf > 8 > > > > 6f141af91ab2d7cd011db47%7C1%7C0%7C638192553936149621%7CUnknown > %7CTWF > > > > pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI > > > > 6Mn0%3D%7C3000%7C%7C%7C&sdata=3DE4hpQLMgLy0BKHngjpec88AI0v3TZ8xg > xebjU0 > > > SS0FM%3D&reserved=3D0 > > > > > > That triggered a thread where we had a similar discussion about > > LargeVariableLib responsibilities, etc. > > > > > > We still have a fork of SaveMemoryConfig that uses the PCD I sent in > > > the > > > patch: > > > > > > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fg= i > > > > thub.com%2Fmicrosoft%2Fmu_common_intel_min_platform%2Fblob%2Freleas > e > > > > &data=3D05%7C01%7Cmichael.kubacki%40microsoft.com%7C05c93413c875400a > ce > > > > f708db50bf197d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63819 > 255 > > > > 3936305834%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > V2luM > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3DlXFLZV78C > Pev > > > UL7De1%2B6RtHaKWQcjnXx6IFABcmWJ54%3D&reserved=3D0 > > > > > > /202208/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig. > > in > > > f > > > > > > I believe a challenge that led to adding compression (in our fork > > > code) to > > SaveMemoryConfig was the fact that the data was produced in pre-mem > > PEI and compression is expensive in CAR. > > > > > > In general though, I still believe that it is simpler to separate > > > data mutation > > from service APIs. Platforms can manipulate data to achieve their > > goals whether size, security, and so on and the service APIs provide a > > simple interface to store and retrieve that data blob. > > > > > > I'd also like to see FSP reduce in size and eliminate operations > > > that are not > > essential to its role and can be consolidated/reused in the wrapper. > > > > > > Thanks, > > > Michael > > > > > > On 5/8/2023 5:48 PM, Michael D Kinney wrote: > > >> When reviewing the Variable feature that adds integrity and > > >> confidentiality, I suggested that the interface between the > > >> Variable services and the NVStorage could provide an abstraction to > > >> encode/decode the stored data that would support encryption, > > >> compression, or both. Could also support a platform policy for > > >> which > > variables the encode/decode operation is applied. > > >> > > >> Wouldn't that be a better abstraction than piecemeal adding these > > >> features? > > >> > > >> Doesn't mean that this can't go in as-is. But would be an > > >> opportunity to consolidate in the future. > > >> > > >> Mike > > >> > > >>> -----Original Message----- > > >>> From: devel@edk2.groups.io On Behalf Of > > >>> Chiu, Chasel > > >>> Sent: Monday, May 8, 2023 12:37 PM > > >>> To: Oram, Isaac W ; Gudla, Raghava > > >>> ; devel@edk2.groups.io > > >>> Cc: Desimone, Nathaniel L ; > > >>> Kubacki, Michael ; Chaganty, > > >>> Rangasai V ; Chiu, Chasel > > >>> > > >>> Subject: Re: [edk2-devel] [edk2-platforms:PATCH V1] > > >>> MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. > > >>> > > >>> > > >>> Hi Isaac, > > >>> > > >>> Just my thoughts, I would vote for platform/bootloader to decide > > >>> compressing the variable data before saving to NVRAM or not. > > >>> It should be optional and when platform having big SPI flash they > > >>> might not want to do this. > > >>> > > >>> Thanks, > > >>> Chasel > > >>> > > >>> > > >>>> -----Original Message----- > > >>>> From: Oram, Isaac W > > >>>> Sent: Monday, May 8, 2023 12:15 PM > > >>>> To: Gudla, Raghava ; > > >>>> devel@edk2.groups.io > > >>>> Cc: Chiu, Chasel ; Desimone, Nathaniel L > > >>>> ; Kubacki, Michael > > >>>> ; Chaganty, Rangasai V > > >>>> > > >>>> Subject: RE: [edk2-platforms:PATCH V1] > > MinPlatformPkg/SaveMemoryConfig: > > >>>> Support NVS Data compression. > > >>>> > > >>>> The proposed implementation is fine and I will reviewed-by and > > >>>> push if that > > >>> is > > >>>> the desired direction. > > >>>> > > >>>> My question is if we generally like the design of doing > > >>>> compression in > > >>> common > > >>>> MinPlatform code, decompression in board specific FSP wrapper code= . > > >>>> The alternative design is to do compression and decompression > > >>>> inside the > > FSP. > > >>> This > > >>>> seems like a slightly simpler separation of responsibilities. > > >>>> The board code is responsible for save/restore, the FSP code is > > >>>> responsible > > >>> for > > >>>> data blob creation and use. Data integrity, authentication, > > >>>> compression, etc > > >>> all > > >>>> can be done based on more detailed knowledge of the silicon > > >>> implementation > > >>>> requirements. > > >>>> > > >>>> I can see another argument though that doing it inside FSP > > >>>> effectively forces both bootloader and FSP to carry > > >>>> compression/decompression. The compression/decompression aren't > > >>>> likely to need to be silicon specific. And bootloader may have > > >>>> more requirements to balance than just the silicon requirements. > > >>>> > > >>>> Can I get some votes on preferred answer for > > >>>> compressing/decompressing > > >>> FSP > > >>>> non-volatile data in bootloader or FSP? > > >>>> > > >>>> Thanks, > > >>>> Isaac > > >>>> > > >>>> -----Original Message----- > > >>>> From: Gudla, Raghava > > >>>> Sent: Friday, May 5, 2023 5:03 PM > > >>>> To: devel@edk2.groups.io > > >>>> Cc: Gudla, Raghava ; Chiu, Chasel > > >>>> ; Desimone, Nathaniel L > > >>>> ; Oram, Isaac W > > >>> > > >>>> Subject: [edk2-platforms:PATCH V1] > MinPlatformPkg/SaveMemoryConfig: > > >>>> Support NVS Data compression. > > >>>> > > >>>> Around 50KB "FspNonVolatileStorageHob" data can be compressed to > > >>>> approximately 3 KB which can save NVRAM space and enhance life of > > >>>> the SPI part by decreasing the number of reclaim cycles needed. > > >>>> > > >>>> This patch added support to compress "FspNonVolatileStorageHob" > > >>>> data > > >>> before > > >>>> saving to NVRAM. > > >>>> > > >>>> A PcdEnableCompressFspNvsHob is introduced to enable/disable this > > >>> feature per > > >>>> platform usage. > > >>>> > > >>>> Cc: Chasel Chiu > > >>>> Cc: Nate DeSimone > > >>>> Cc: Isaac Oram > > >>>> > > >>>> Signed-off-by: Raghava Gudla > > >>>> --- > > >>>> .../SaveMemoryConfig/SaveMemoryConfig.c | 34 > > +++++++++++++++++++ > > >>>> .../SaveMemoryConfig/SaveMemoryConfig.inf | 6 +++- > > >>>> .../Include/Dsc/CoreCommonLib.dsc | 1 + > > >>>> .../Intel/MinPlatformPkg/MinPlatformPkg.dec | 5 +++ > > >>>> 4 files changed, 45 insertions(+), 1 deletion(-) > > >>>> > > >>>> diff --git > > >>>> > > >>> > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo > > >>> r > > >>>> yConfig.c > > >>>> > > >>> > > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMem > > >>> or > > >>>> yConfig.c > > >>>> index 0215e8eed..8aa935b54 100644 > > >>>> --- > > >>>> > > >>> > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo > > >>> r > > >>>> yConfig.c > > >>>> +++ > > >>>> > > >>> > > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMem > > >>> o > > >>>> +++ ryConfig.c > > >>>> @@ -21,6 +21,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > >>>> #include #include > > >>>> #include > > >>>> +#include > > +#include > > >>>> /** This is the standard EFI driver poi= nt that > > >>> detects > > >>>> whether there is a@@ -45,6 +47,9 @@ SaveMemoryConfigEntryPoint ( > > >>>> UINTN DataSize; UINTN BufferSize; = BOOLEAN > > >>>> DataIsIdentical;+ VOID *CompressedData;+ UINT64 > > >>>> CompressedSize;+ UINTN CompressedAllocationPages; = DataSize > > >>> =3D 0; > > >>>> BufferSize =3D 0;@@ -73,6 +78,35 @@ SaveMemoryConfigEntryPoin= t ( > > >>>> } } + if (PcdGetBool (PcdEnableCompressFspNvsHob) =3D=3D= 1) {+ > > >>>> CompressedData =3D NULL;+ CompressedSize = =3D 0;+ > > >>>> CompressedAllocationPages =3D 0;++ DEBUG ((DEBUG_INFO, > "compressing > > >>> mem > > >>>> config nvs variable\n"));+ if (DataSize > 0) {+ > > >>> CompressedAllocationPages =3D > > >>>> EFI_SIZE_TO_PAGES (DataSize);+ CompressedData =3D AllocatePag= es > > >>>> (CompressedAllocationPages);+ if (CompressedData =3D=3D NULL)= {+ > > >>> DEBUG > > >>>> ((DEBUG_ERROR, "[%a] - Failed to allocate compressed data buffer.\= n", > > >>>> __func__));+ ASSERT_EFI_ERROR (EFI_OUT_OF_RESOURCES);+ > > return > > >>>> EFI_OUT_OF_RESOURCES;+ }++ CompressedSize =3D > > EFI_PAGES_TO_SIZE > > >>>> (CompressedAllocationPages);+ Status =3D Compress (HobData, D= ataSize, > > >>>> CompressedData, &CompressedSize);+ if (EFI_ERROR (Status)) {+ > > >>> DEBUG > > >>>> ((DEBUG_ERROR, "[%a] - failed to compress data. Status =3D %r\n", > > __func__, > > >>>> Status));+ ASSERT_EFI_ERROR (Status);+ > > >>> FreePool(CompressedData);+ >=20 >=20 >=20 > Bug: it should be FreePages (CompressedData, CompressedAllocationPages); >=20 >=20 >=20 > > >>>> return Status;+ } else {+ HobData =3D CompressedData;= + > DataSize > > >>> =3D > > >>>> (UINTN) CompressedSize;+ }+ }+ }+ if (HobData !=3D NULL= ) > { DEBUG > > >>>> ((DEBUG_INFO, "FspNvsHob.NvsDataLength:%d\n", DataSize)); DEBU= G > > >>>> ((DEBUG_INFO, "FspNvsHob.NvsDataPtr : 0x%x\n", HobData));diff --= git > > >>>> > > >>> > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo > > >>> r > > >>>> yConfig.inf > > >>>> > > >>> > > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMem > > >>> or > > >>>> yConfig.inf > > >>>> index 61e85a658..77920d031 100644 > > >>>> --- > > >>>> > > >>> > > > a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo > > >>> r > > >>>> yConfig.inf > > >>>> +++ > > >>>> > > >>> > > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMem > > >>> o > > >>>> +++ ryConfig.inf > > >>>> @@ -26,6 +26,7 @@ > > >>>> LargeVariableReadLib LargeVariableWriteLib BaseLib+ Comp= ressLib > > >>>> [Packages] MdePkg/MdePkg.dec@@ -45,6 +46,9 @@ > > >>>> gFspNonVolatileStorageHob2Guid ## CONSUMES > > >>>> gFspNvsBufferVariableGuid ## PRODUCES +[Pcd]+ > > >>>> gMinPlatformPkgTokenSpaceGuid.PcdEnableCompressFspNvsHob+ > [Depex] > > >>>> gEfiVariableArchProtocolGuid AND- > > gEfiVariableWriteArchProtocolGuid > > >>>> \ No newline at end of file > > >>>> + gEfiVariableWriteArchProtocolGuiddiff --git > > >>>> + a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc > > >>>> + b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc > > >>>> index 5ce21cf31..dfe7d836d 100644 > > >>>> --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc > > >>>> +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc > > >>>> @@ -147,6 +147,7 @@ > > >>>> > > >>>> > > >>> > > BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSuppor > > >>> tLi > > >>>> b.inf > > >>>> > > >>> LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableLib/B > > >>> LargeVariableReadLib|as > > >>> LargeVariableReadLib|e > > >>> LargeVariableReadLib|Larg > > >>>> eVariableReadLib.inf > > >>>> > > >>> LargeVariableWriteLib|MinPlatformPkg/Library/BaseLargeVariableLib/ > > >>> LargeVariableWriteLib|Ba > > >>> LargeVariableWriteLib|s > > >>> LargeVariableWriteLib|eLarg > > >>>> eVariableWriteLib.inf+ > > >>>> CompressLib|MinPlatformPkg/Library/CompressLib/CompressLib.inf = # > # > > >>>> CryptLibdiff --git > > >>>> a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > > >>>> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > > >>>> index 784abb828..e21d55fb3 100644 > > >>>> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > > >>>> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec > > >>>> @@ -348,6 +348,11 @@ > > >>>> > > >>>> > > >>> > > > gMinPlatformPkgTokenSpaceGuid.PcdFadtFlags|0x000086A5|UINT32|0x90000 > > >>>> 027 > > >>>> > > >>> > > > gMinPlatformPkgTokenSpaceGuid.PcdFadtMajorVersion|0x06|UINT8|0x90000 > > >>> 0 > > >>>> 30 > > >>>> > > >>> > > > gMinPlatformPkgTokenSpaceGuid.PcdFadtMinorVersion|0x03|UINT8|0x90000 > > >>> 0 > > >>>> 31+## Controls whether the Memory Config UEFI Variable is saved > > >>>> 31+as > > >>>> compressed data.+# Data compression can significantly reduce > > >>>> variable > > >>> storage > > >>>> usage for FSP NVS buffer data.+# Platforms that choose to > > >>>> compress the > > >>> data > > >>>> will need to decompress the variable data upon+# extraction.+ > > >>>> > > >>> > > gMinPlatformPkgTokenSpaceGuid.PcdEnableCompressFspNvsHob|FALSE|BOO > > >>> L > > >>>> EAN|0x90000032 [PcdsFixedAtBuild] -- > > >>>> 2.19.1.windows.1 > > >>> > > >>> > > >>> > > >>> > > >>> > > >> > > >> > > >> > > >> > > >> > > > > > > > > > > > > > > > >=20 >=20 >=20 >=20