From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web10.799.1683666412873444670 for ; Tue, 09 May 2023 14:06:53 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=mjkq2lMA; spf=pass (domain: intel.com, ip: 192.55.52.43, 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=1683666412; x=1715202412; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=VX7R0SELnFVfhvzYVkCjfDW0VrSVZOLOsgAiNXlR4qo=; b=mjkq2lMADB6BShVxuCJ89SzksQXAV8zvTyoSRpGrHqkL/oof4vsSw6if 6JtaRF1xI8zuzexNw/pNiMsjviXHxHEXZbx6o6yj4xNdjZQLsp+t7YArC EwMsvdV9HZ4J8APZ4BxpLTSR9mMHaO3uxrrWN2bgUQSwsEadbri20cOOE KGHVFd8tQMIYHFacPP9NQlJVby/IKuV1AJ7n4hsLkbS4jpjtFTeFeigaE grZ3wxv20wP+VjV1wCtjeJD5u19njsubmdIqqgeSJGSRV3Vtu/Ett2iOS ssJRT2p6tpq0gt3lfQXQf+ZsV+3XBflkbcluP2KiGF1Lpj7Z5SjfApCSB g==; X-IronPort-AV: E=McAfee;i="6600,9927,10705"; a="436373743" X-IronPort-AV: E=Sophos;i="5.99,262,1677571200"; d="scan'208";a="436373743" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 May 2023 14:06:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10705"; a="810871200" X-IronPort-AV: E=Sophos;i="5.99,262,1677571200"; d="scan'208";a="810871200" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga002.fm.intel.com with ESMTP; 09 May 2023 14:06:40 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX601.amr.corp.intel.com (10.22.229.14) 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 14:06:39 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX610.amr.corp.intel.com (10.22.229.23) 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 14:06:39 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx610.amr.corp.intel.com (10.22.229.23) 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 14:06:39 -0700 Received: from NAM04-MW2-obe.outbound.protection.outlook.com (104.47.73.172) by edgegateway.intel.com (134.134.137.103) 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 14:06:38 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RXNUyvMAcL0CWX4O8um3ToLzxJP4Gsix16zXXqTxGyM4gk4P+OWuFx0nAhKtJrqjFiwU6FsF8VNSjlwIa6LvpzHHoTP+XONH4wv6sEqhPXMeYy6iAO5ZZafdyDrBtI2iWrJBRcmV3jLsFHr0ncBGv3iVeNi1b0eTmhqXUHBnxaSOnDtg283lC7MrjkTMKLJEZp9BBtQiqFr6Bq9Jd9wM38U6BgRpb91DAiuSYn6CPauBQlsuiHkJ5y8sB3S3qVvOBEH3XFrN7ZXOF63CUywh5r5Q7ta15dPk/SfkfQtv2R/0bp/65ZsyAFlEI9N6F3wQplD6MVWlUGPXXp7Dx2LvbQ== 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=5isRGuJqTf+VxrdsopZW5Lv8YXHlirefUjrWD/9lU0Q=; b=V85DCMOEFL3jZWrf8Qyoe8nVwNw9IGSzgYMNGe9D+v5kMI9xuFez6p/AqoqNaNt48RWYkdHmxspkhg8ZiT0dcPdKn1INBHatDQb7UtxeEO2HkkvtMZnXDDRm053qu13gmtYs+k12Lxzk0y2CyKNOwzaLdPQFV1jTC40AErP4UOgJCey6UfjSzTeJVkvRrVlGItISm7AEz/9adQLPjRYFwkML9FW1Z31aDa7GlLvfoUIMbs+8/vC4+4lPBUpeYyaxacpeebIKplENfFWlFy3cMdPgcqkHoVAaJSNVHel52Hm5Nz7mtlYO9KRPHJQu80pJCDWQJ02ocL0oVv+FxS+hkg== 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 PH7PR11MB6907.namprd11.prod.outlook.com (2603:10b6:510:203::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6363.32; Tue, 9 May 2023 21:06:35 +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 21:06:35 +0000 From: "Chiu, Chasel" To: "Kubacki, Michael" , Michael Kubacki CC: "Desimone, Nathaniel L" , "Chaganty, Rangasai V" , "Gudla, Raghava" , "Oram, Isaac W" , "Kinney, Michael D" , "devel@edk2.groups.io" 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: AQHZf64xuQvoctZH9EWbk8VzvnYjSq9Qw4GAgAACr4CAACghgIABJ2iAgAAkD4CAAAGygIAABvCAgAALPxCAAAjKgIAAAtgAgAAa7yA= Date: Tue, 9 May 2023 21:06:35 +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_|PH7PR11MB6907:EE_ x-ms-office365-filtering-correlation-id: 9defd076-bcd8-41bc-d77a-08db50d145df 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: UYmdAmK/LatlZ7U7ta2NE6bltjMkNJ3v8tFQlt0pPUredECN/mpY8vB3a5HNm7tjnKxTDglsK6w9Idj0rarrjcBp6YvLp8ovst/xPUNvjWYSu73k9SpWAo5o74Up7ywY8EN2hLl6IdWWzjydm84xg9GFPSTne+wmVmqZQLTWHIdNDKBrW8XiJApEuNvyaRkWENbCsH156cTMK9d9ipmuexPhZmOCBRxuuGwmnRqUTTLSwq8BLB7Oi+qekiKZHWERcJF+Khdzl5CepI3OEQwBujS8o9eJx8TnTbcJdqLnJycj2CrbPSU6nf4OcZn0fbtEfyNfKNYWFsLOA8qeZKsemcp5GbKPTQ/JoQXECj97M208oxOZE8mmhDlTiaqeixFDywFoTO5oTf1whAAPiIUXrf4kSgIOXaZdJ2LHfjddOcr60tOCtsD8ntAHVyTHE9QFTo98snbqiZgzKoo239WK7hKfYveMbO6NF8O4P/B0xgMceHJyy1nxsgtUqQ7FvrrvcSsshpTLyyAEHRKj2M403+DwpLxBzBvAAjbLUUn4B7XiacE2m7/tQ0bmtKTeR5di8xLpBbvEIY+F8lKuQX39PJC8XJPg9cEY3DEey1vajdCFcpOSzpQo0hG0xZYpbgjC1x1D8NZjiXtKk5oHdC4V7Q== 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)(366004)(39860400002)(396003)(346002)(376002)(136003)(451199021)(478600001)(83380400001)(45080400002)(966005)(186003)(19627235002)(71200400001)(110136005)(122000001)(4326008)(7696005)(54906003)(8676002)(2940100002)(53546011)(26005)(9686003)(6506007)(2906002)(30864003)(38100700002)(52536014)(33656002)(64756008)(66946007)(41300700001)(66556008)(38070700005)(66476007)(76116006)(66446008)(8936002)(86362001)(5660300002)(82960400001)(55016003)(316002)(66899021)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?EKaqI49JCpUqzgW+P/Y5f9NWYxfWkfbd4CR5vpULUMIu37LzdwUwZ04WL/kW?= =?us-ascii?Q?C0TT95w3GJ6bpgLd4+CwBVvSb9FQ6YsFhqvn9mXUlG8oHne715OhgzUMFQur?= =?us-ascii?Q?D5RVn0kkEgW8Pr8/XLYMfoag9+V+0y+Ter5k8E3a01CILviyLrPOvhPtYrXx?= =?us-ascii?Q?QjcCvUzqjA8Cab9FoyHrObNF3v5JtOzwoEAACtjWa2TDKRCojnb41G3r3xpQ?= =?us-ascii?Q?I/3WdYrXX/NXLCU8HWoe/DhnhtF2YFOj7g8Jd034ogL9ase4jKKWkB8bYZOs?= =?us-ascii?Q?WuLKpgz4nX/BAgjSTlKGVyU7NtvbGe35zqlJkMeAcalHaFmK8p/F8XrInZ2y?= =?us-ascii?Q?qtvK9HDasEq0oEKOiJDatNDdJonKj9UBT8rRTH1TPlFclt0inFUitVahvIM7?= =?us-ascii?Q?XypNZy6coFxCuwCVytxInM80bNiX6414Wb1fR/6smpM40uhMdNyl3thFtq9e?= =?us-ascii?Q?aWWvODI17VM/+uMp56z4hTIATqhf8GHU9FRxgK1Ae8cyHLLH0h496a8QoyHE?= =?us-ascii?Q?FEL1VR7DKMyTjwxdKZF1RRPNggKwoeHbFE5kyi6DZ1sIyG1rZlfk+z+f/IQS?= =?us-ascii?Q?AMh9vtZ7tHouKtbXWe7zUBv+GZuzuskV9IL33F7pDVhtafOJ49aI5Vg/oH7E?= =?us-ascii?Q?m11gHIlbbURbFB7AGReFJVXM38Cs3sTkJx7uXPRdUYQvkneJ77LsYlDkIOJK?= =?us-ascii?Q?jDQou/MCExAOvoIoKny+w/yx3eUnzfpZ9Wp6fo/SleXqbYVdcccpCA2mdB6X?= =?us-ascii?Q?fOt7Ezx1xSdk4zPEE5Br6IH0nUX9kTS29jhBmeRIK/xxJSLPyXqxsYmFdoUO?= =?us-ascii?Q?/4qSxdXATBXDkm4se77srgNxr8Ei//XIKmG8tZB19F+6OqvVTHjHXQ35MLnk?= =?us-ascii?Q?oO2rQVrm9bb6znjMjf0jkgAnm8N0XaQ4lWh+Be1Vl8jgjR2629vv+qb5AICU?= =?us-ascii?Q?VxpOO/e8ZpFPOltvq3CPo3qP86ozeT7QsejIpsDmYahhud2AUmEYPLUrWEMP?= =?us-ascii?Q?tyMfWesFp7aRIwlPkk12MoWaPNWSfpI1j/RunM2BS7YhCDtZjGkFfcAhWtUo?= =?us-ascii?Q?LRpePXXSJI5vWyWvovuBwbAoGBV2Hy+7aJPGT0wa0txAEJJWYRu176X+gRCa?= =?us-ascii?Q?lFxkeivYvhRtU2mn4d8/7QL601vS7bGKb9YtW4sdVnQdKUttlTtO9AJFxG0t?= =?us-ascii?Q?uRHhY2nHfXu0xtbNeQ6OZqA4Tq8L3o9yrtr7KDECONZtIyVKNew/aezP4Vip?= =?us-ascii?Q?a778gShA+lVrOaAzVZ1dEKoLcIxWiYJcDE1VhKvkeINnCVIrWprj3UtB91bg?= =?us-ascii?Q?MVIMLKKweNNNGWXwr2VMBCQoqeUOaf6kB9wA81/+eysL3YIpsPmijUPFO5TD?= =?us-ascii?Q?r9Uo2Sp47eCeSZI1UhPXTT/N8q4Wgu3633R9R2Mhec+uzYtI8pY6G+kEVFeQ?= =?us-ascii?Q?iVXGGKkmX81WH2qZGWBTbZVxB/1RpKELsBmuOalLMfcAHMO/9gKzm0acswh8?= =?us-ascii?Q?zjTxtLdGwvVp+d8P6HGW6uDEVDZLF6udPk/Q4uM2CM0zHAJxw76vcZKboD+K?= =?us-ascii?Q?fYZGwLlk/jAeQyuERv/AQLmpzcctglhG3xzU1nj7?= 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: 9defd076-bcd8-41bc-d77a-08db50d145df X-MS-Exchange-CrossTenant-originalarrivaltime: 09 May 2023 21:06:35.3733 (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: YZPrNam91De7Gy2/+IIEWwjdbjxVqmcY/aPGdSvhpEdbVVyqxoh7xbOi0Xs97Hw1Ft7arSeNFPfGVkyGVf6dsQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6907 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 Hi Michael, Please also add CompressLib to CoreCommonLib.dsc to prevent build failure f= rom open source platforms. Thanks, Chasel > -----Original Message----- > From: Chiu, Chasel > Sent: Tuesday, May 9, 2023 12:30 PM > 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. >=20 >=20 > Just reviewed the branch code and it is functionality the same so agree w= e can > switch to Mike's implementation. > Would you help to rebase and send updated patch for reviewing? >=20 > Thanks, > Chasel >=20 >=20 >=20 > > -----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. > > > > I haven't done a deep comparison of the changes, but I'd lean toward > > using what is in the branch that I referenced earlier since it has > > been used for a while now. > > > > Is there anything of value in the new set of changes? > > > > Thanks, > > Michael > > > > -----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 > > Kubacki ; Chaganty, Rangasai V > > > > Subject: [EXTERNAL] RE: [edk2-devel] [edk2-platforms:PATCH V1] > > MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. > > > > > > Hi Michael Kubacki, > > > > Since Rahava has rebased the patch it might be easier to add " Signed-o= ff-by: > > Michael Kubacki " to Rahave's patch > > commit message so both of you are authors. > > What do you think? > > > > 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 > > command) > > > > Thanks, > > Chasel > > > > > > > > > -----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 me. > > > > > > > > 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%2= F > > > > ed > > > > > > > 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%2= F > > > > gi > > > > > > > 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 co= de. > > > >>>> 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 p= oint 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 @@ SaveMemoryConfigEntryPo= int ( > > > >>>> } } + 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 AllocateP= ages > > > >>>> (CompressedAllocationPages);+ if (CompressedData =3D=3D NUL= L) {+ > > > >>> 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, > DataSize, > > > >>>> 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);+ > > > > > > > > Bug: it should be FreePages (CompressedData, > > CompressedAllocationPages); > > > > > > > > > >>>> return Status;+ } else {+ HobData =3D CompressedDat= a;+ > > DataSize > > > >>> =3D > > > >>>> (UINTN) CompressedSize;+ }+ }+ }+ if (HobData !=3D NU= LL) > > { DEBUG > > > >>>> ((DEBUG_INFO, "FspNvsHob.NvsDataLength:%d\n", DataSize)); > DEBUG > > > >>>> ((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+ > CompressLib > > > >>>> [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.d > > > >>>> +++ sc > > > >>>> @@ -147,6 +147,7 @@ > > > >>>> > > > >>>> > > > >>> > > > > BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSuppor > > > >>> tLi > > > >>>> b.inf > > > >>>> > > > >>> LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableLib > > > >>> LargeVariableReadLib|/B > > > >>> LargeVariableReadLib|as > > > >>> LargeVariableReadLib|e > > > >>> LargeVariableReadLib|Larg > > > >>>> eVariableReadLib.inf > > > >>>> > > > >>> LargeVariableWriteLib|MinPlatformPkg/Library/BaseLargeVariableLi > > > >>> LargeVariableWriteLib|b/ > > > >>> 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 > >