From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from BN6PR00CU002.outbound.protection.outlook.com (BN6PR00CU002.outbound.protection.outlook.com [52.101.57.25]) by mx.groups.io with SMTP id smtpd.web11.42259.1683659858397998474 for ; Tue, 09 May 2023 12:17:38 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=f2nVSOmx; spf=pass (domain: microsoft.com, ip: 52.101.57.25, mailfrom: michael.kubacki@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EjnEe8CVJT2dB+OpI6m3CpDtPmfrn1MVW6Qs46cJbOR6jJzZQVHKCvMkcJFwZ1qivn5dvlxeUXiiruVUx0ltLWYO0gg7qt/9AGfYA/L8a3Qfh5MqbscML55y1+x/Y8AaLD5mD2BhaoUr8C4WInP61Zqg8gDYEyWAsU/Qu7H2OTJUnXiIMODtInXfq4VzZVSrw+OYCDd7byJCD5KqUQCd0iTY8wOLhTNFD1Z+7OUPrBlZYzGi6YF0pU0ccnabf1KyiInWFxFbMsONpYt943v0Kiad+/IZBTa5XhWsGLS25JixUwbNfjsqt6vVDVDNJQ8zTzzX6IC72Tk/LGP+TjPk6g== 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=eoM/d9VbdBnB7zxOBUBSvYdvzsGcI8Y2fq6Ph6xEIKE=; b=Kq9MdjBxn06TZ24PFvrBzTpq1K6Q771s5atOWCgrYwz60/lCHvbC5WWy9LtMOgr7eSs9qUVa5zc6PKXItUx+WFyTrQMwi9JeurKnEAhPf6WnFjaRFFbqLrt+YXHrU3dpKogHXJyJJJ9Ec7cfhw/zWO0Nz4bFhppE4taz2iah+IguDvAF1FHnaAlD8+ZNCKyBsoePRIPisrOKQVWjhG7okYIW0EHVyomnhOvL0rwg2Cop2NpyUGkcN0vFab8WFiQXM/hb9sSwASIIrzICuLeV2atKf9AjtokPASEj7cOHIMjFiBjeFT6d1v50u5C7z/jNmwh3SxWA/WFco6342rU0Hw== 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=eoM/d9VbdBnB7zxOBUBSvYdvzsGcI8Y2fq6Ph6xEIKE=; b=f2nVSOmx74srFWeYb1UkSMXOKuxTYHpdakWg5mJLQA14ET58+tY2pmHwATHHHN3g1evmbDli46Y52LtvGAiaBdvCmpz8vUGt7/HWY+iVJbl8hTjvaf1mRY8Um7ktePyB6FjqnkLKACROSxj7PD4gxd9ppeHEVqgqDCZ8YcVAY54= Received: from LV2PR21MB3351.namprd21.prod.outlook.com (2603:10b6:408:14d::7) by PH7PR21MB3213.namprd21.prod.outlook.com (2603:10b6:510:1d6::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6411.2; Tue, 9 May 2023 19:17:33 +0000 Received: from LV2PR21MB3351.namprd21.prod.outlook.com ([fe80::299a:391b:a599:3b2b]) by LV2PR21MB3351.namprd21.prod.outlook.com ([fe80::299a:391b:a599:3b2b%5]) with mapi id 15.20.6411.003; Tue, 9 May 2023 19:17:33 +0000 From: "Michael Kubacki" 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. Thread-Topic: [edk2-devel] [edk2-platforms:PATCH V1] MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. Thread-Index: AQHZgeF1TbtILFnsXEu+2BK9kUZpBK9QxSGAgAAkyYCAASdogIAAJBCAgAABsoCAAAbwgIAADhsAgAAFCoA= Date: Tue, 9 May 2023 19:17:33 +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=microsoft.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: LV2PR21MB3351:EE_|PH7PR21MB3213:EE_ x-ms-office365-filtering-correlation-id: fee0f0a4-af2e-4652-0131-08db50c20a81 x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: PKaDsvpq+4bkMy1y7LuDPsliZ3IunpAEDHv8lqFXlHKe+VYcigg2c0nEVciIfasgur2eGC6noJExNnn6sSPrP3HwfTXIjm0TTPuwX191T/sIcI4OHTZdHTd/GR7qF6yBzIWFzDAC5flei276QyWj/y3YOh7gdVCA3FyS96T/2i346JzlPzZeEn4YYpx4ZtNNtIflHW/2hezMzEeH9uJp17fzQnC9I0dqhHWKagG1b70xIPNUiVPRH2qcM92TTvlwSE+qyHIL926gBg0HJvTl0NWYIB2U+7h31feVjNAFG/0CakSUwu5lf4H8Oq0jqM6o93jM3+OZUOCOoEjhmy7HaIhvF5SVkFgmjNRHMd27ru/0w3ii7HDy3lG9pE4KzyTHZex9NxVpddI8ZspwBAinHDm5nh6i9kAGb4F+MdbsPutCodGW15SmLUOGMmi1USLf2ukjOSioB0dXt6SWt++CFAN4qXhrDbj9KIm3ncpzv4DRztIRkIhCurDeVmRhdSItR8IziwGMVdAQJ58TFcjx6rEf9yYQELrfHOdkrBUxoXUDhFU54Jj2ENVfC86LI89At5w2zZmtHhTZm7FB59fT1xg0jduC9JfbFLz1GGix3L1/fMzI27mr1X2IPmqbMKijr06VSdDZnQO2vj43mK7db0hmTnUvILEI4kxDvBJJUiHqFyC+g5OVc3tUnDEeJzJa x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:LV2PR21MB3351.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(136003)(396003)(346002)(366004)(376002)(39860400002)(451199021)(66899021)(966005)(7696005)(54906003)(2906002)(83380400001)(55016003)(82960400001)(82950400001)(38100700002)(33656002)(921005)(10290500003)(122000001)(478600001)(6506007)(186003)(786003)(110136005)(8936002)(30864003)(53546011)(26005)(19627235002)(71200400001)(8676002)(41300700001)(52536014)(5660300002)(86362001)(4326008)(38070700005)(316002)(64756008)(9686003)(76116006)(8990500004)(66476007)(66556008)(66946007)(66446008);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?x7D4Am64E5E8NsciX1nMGUOVFu0Ov58N84R1AwDm0doMN4Fy1mEU3mMcBPsE?= =?us-ascii?Q?LidHy/S/wyZoVM5guK2iQdV6gy5K/IrKUsc6HpoHSsmasCtP9pFs4Ap76KQX?= =?us-ascii?Q?4zEqgIhE7dhWnHEb4JYAbtjUzepqOhLQhxba+z+4SNPI06cCzyRJ/FWVpsuc?= =?us-ascii?Q?6ges52uoDECcUA3nVFq7USbTCq4EAlt1oZc6qEHsHLjCxKGLCEIpHoX1qSJL?= =?us-ascii?Q?dvGaC+qz77VjitF3YOtmKnaRthlgchs+UE4GUSpT0LW8sQGQmaM9XJP6lgFz?= =?us-ascii?Q?UhWJsMeqJnGMgoGAUVTJ39eG8tetTaIma9wJRKsuRLUiuqR6tckYJJ+Z1Gnn?= =?us-ascii?Q?QoZhCbAK9/mrhkrz6VyEUp4FTkXLTNXsXg4snlPhkkNRF0DZB/W5121vXDRm?= =?us-ascii?Q?C6tuURnGdrAYzPwf1s/h4iMf0g2F0QVcTp4bTR4Igiaal60FncSWdDWYyHxl?= =?us-ascii?Q?s/WEB2IQyDwchQpc26KrPv93rW/iRCK6uY+JIBzbiYureX7IUVjR+7ssDKm0?= =?us-ascii?Q?Rtpxe1eTYoy7FmJ3PngubRMcaMDAmNrUcNxMWxLGttIZxuQ6kD70FB+H7l1+?= =?us-ascii?Q?zXZY8FcglQoF4fm4HJ1y7ae2Drf1fLxzXSXbjbGA3a9lGYa59x4DauIWcR5x?= =?us-ascii?Q?ErFhgPIwyKNB0BAA83LPtNRUbZwqytqhHtdXhO3H6yQDFYHNZcACBqJmdAsL?= =?us-ascii?Q?bHGxPH8LiUUVsQl60RBqZ/1QdIiFHejaOSr6B57wNeWXXkLtwC/grAjP/Ndg?= =?us-ascii?Q?cp3H6W1jibOasUJmyXNBJItg7CucWahI3QzwrImHLb6usRz1pUP/vwELbm34?= =?us-ascii?Q?I4DYR6zlb9jHcRcC0MBmEJa6uTWUxmKhbvec0hESLn0IKlb3zutdPd1MYUDc?= =?us-ascii?Q?lVx5sIrw9+TcmSZV8sih8KQAwb+xOEQNhBYewQ8c+DWTwV9qR9G3pKBF6NVd?= =?us-ascii?Q?mB4NmMt6Q/HcX+cb5SrvMev9L0jXIfiZDT1LOwHpf3XtyBV7eSlCPrQ/AQ8w?= =?us-ascii?Q?dx05yddNMe/VbgaJN38fO6MQDCCUinlpi1UdtaZFjAp/A4LGoSbqiNjEX9yg?= =?us-ascii?Q?OriZXTNn3vBpwyb7ycx+Tyv/+UqhV5fjp7E4d1d1TBm9RMv8/PaQ0u8jMZzW?= =?us-ascii?Q?6a9IjMHzeOkcl78CoryCdA0DPjDfC19k0jQ2v4IEe8tldYhiMhivqlm9/tMk?= =?us-ascii?Q?XqUnXtvW6is25Ezo8mfiIQV2ogfKA50gW0OkiPeTu7imHaW8D2WmlgtPwEu1?= =?us-ascii?Q?7W2czBHSDSGKsHDRdPO25h3S7A5j4MiHfxopKZF+L1oDMghvosC4sc2QNy8A?= =?us-ascii?Q?qP/nkEuRai8/WH+vii6Ba1SIm7d9cbVS775C6ENVdRN2EXv2IRSBXaBLn92C?= =?us-ascii?Q?FkZzfJVrGzPi6zlYWmVlaUEO8XN/pPxWwrpTTJaN231eFTgk7Km9vItYWYqm?= =?us-ascii?Q?/T8OrxffE3spMSSY+DI76Xra60+7SfTvkdzdcld0idr0DCMAUKMnFfiOuC2x?= =?us-ascii?Q?VHc2iqVHNXRZbMI=3D?= MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: LV2PR21MB3351.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: fee0f0a4-af2e-4652-0131-08db50c20a81 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 May 2023 19:17:33.2993 (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: xF3aR1tvLEhEq7PTfoVl3jackmH4cPB5zMzH3AlExVkpnFEgnddHMsizKLMuR3lfzWI6pt6RXRXwjSRBWypsLQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR21MB3213 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 =20 Sent: Tuesday, May 9, 2023 2:56 PM To: Michael Kubacki ; Gudla, Raghava ; Oram, Isaac W ; devel@edk2.group= s.io; Kinney, Michael D Cc: Desimone, Nathaniel L ; Michael Kubacki= ; Chaganty, Rangasai V Subject: [EXTERNAL] RE: [edk2-devel] [edk2-platforms:PATCH V1] MinPlatformP= kg/SaveMemoryConfig: Support NVS Data compression. Hi Michael Kubacki, Since Rahava has rebased the patch it might be easier to add " Signed-off-b= y: Michael Kubacki " to Rahave's patch commi= t message so both of you are authors. What do you think? By the way, I found a bug in SaveMemoryConfig.c: FreePool() should be FreeP= ages (CompressedData, CompressedAllocationPages); Raghava, please help to f= ix 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=20 > ; devel@edk2.groups.io; Kinney, Michael D=20 > ; Chiu, Chasel > Cc: Desimone, Nathaniel L ; Kubacki,=20 > Michael ; Chaganty, Rangasai V=20 > > Subject: Re: [edk2-devel] [edk2-platforms:PATCH V1] > MinPlatformPkg/SaveMemoryConfig: Support NVS Data compression. >=20 > Thanks for reraising this. If you need me to rebase or help in anyway=20 > let me know. >=20 > 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=20 > > while back > and totally forgot that his patch is still available. We can proceed=20 > 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,=20 > > Michael D ; Chiu, Chasel=20 > > ; Gudla, Raghava > > Cc: Desimone, Nathaniel L ; Kubacki,=20 > > Michael ; Chaganty, Rangasai V=20 > > > > 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=20 > > convinced > that the simpler interface and board responsibility is the correct=20 > short/medium term answer. I don't have an opinion on Mike Kinney's=20 > question on encapsulated services. I generally like that design,=20 > though I am sensitive to Michael Kubacki's feedback that variable=20 > services are too complex as it is. I guess it probably depends a lot=20 > on the specifics of the proposal. That said, it is pretty easy to=20 > migrate from a board specific solution to a more base layer solution in t= he future, so adopting this now doesn't seem harmful to me. > > > > Raghava, can you look at Michael's fix? It looks nearly identical=20 > > to yours and > general convention is to accept the earlier one in case of collision I=20 > 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=20 > > Michael Kubacki > > Sent: Tuesday, May 9, 2023 8:26 AM > > To: devel@edk2.groups.io; Kinney, Michael D=20 > > ; Chiu, Chasel ;=20 > > Oram, Isaac W ; Gudla, Raghava=20 > > > > Cc: Desimone, Nathaniel L ; Kubacki,=20 > > Michael ; Chaganty, Rangasai V=20 > > > > 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=20 > > while > > back: > > > > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fed > > k2.groups.io%2Fg%2Fdevel%2Fmessage%2F92644&data=3D05%7C01%7Cmichael.ku > > backi%40microsoft.com%7C05c93413c875400acef708db50bf197d%7C72f988bf8 > > 6f141af91ab2d7cd011db47%7C1%7C0%7C638192553936149621%7CUnknown%7CTWF > > pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI > > 6Mn0%3D%7C3000%7C%7C%7C&sdata=3DE4hpQLMgLy0BKHngjpec88AI0v3TZ8xgxebjU0 > > 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=20 > > the > > patch: > > > > https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fgi > > thub.com%2Fmicrosoft%2Fmu_common_intel_min_platform%2Fblob%2Frelease > > &data=3D05%7C01%7Cmichael.kubacki%40microsoft.com%7C05c93413c875400ace > > f708db50bf197d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63819255 > > 3936305834%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3DlXFLZV78CPev > > UL7De1%2B6RtHaKWQcjnXx6IFABcmWJ54%3D&reserved=3D0 > > > /202208/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig. > in > > f > > > > I believe a challenge that led to adding compression (in our fork=20 > > code) to > SaveMemoryConfig was the fact that the data was produced in pre-mem=20 > PEI and compression is expensive in CAR. > > > > In general though, I still believe that it is simpler to separate=20 > > data mutation > from service APIs. Platforms can manipulate data to achieve their=20 > goals whether size, security, and so on and the service APIs provide a=20 > simple interface to store and retrieve that data blob. > > > > I'd also like to see FSP reduce in size and eliminate operations=20 > > 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=20 > >> confidentiality, I suggested that the interface between the=20 > >> Variable services and the NVStorage could provide an abstraction to=20 > >> encode/decode the stored data that would support encryption,=20 > >> compression, or both. Could also support a platform policy for=20 > >> which > variables the encode/decode operation is applied. > >> > >> Wouldn't that be a better abstraction than piecemeal adding these=20 > >> features? > >> > >> Doesn't mean that this can't go in as-is. But would be an=20 > >> opportunity to consolidate in the future. > >> > >> Mike > >> > >>> -----Original Message----- > >>> From: devel@edk2.groups.io On Behalf Of=20 > >>> Chiu, Chasel > >>> Sent: Monday, May 8, 2023 12:37 PM > >>> To: Oram, Isaac W ; Gudla, Raghava=20 > >>> ; devel@edk2.groups.io > >>> Cc: Desimone, Nathaniel L ;=20 > >>> Kubacki, Michael ; Chaganty,=20 > >>> Rangasai V ; Chiu, Chasel=20 > >>> > >>> 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=20 > >>> compressing the variable data before saving to NVRAM or not. > >>> It should be optional and when platform having big SPI flash they=20 > >>> 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 ;=20 > >>>> devel@edk2.groups.io > >>>> Cc: Chiu, Chasel ; Desimone, Nathaniel L=20 > >>>> ; Kubacki, Michael=20 > >>>> ; Chaganty, Rangasai V=20 > >>>> > >>>> Subject: RE: [edk2-platforms:PATCH V1] > MinPlatformPkg/SaveMemoryConfig: > >>>> Support NVS Data compression. > >>>> > >>>> The proposed implementation is fine and I will reviewed-by and=20 > >>>> push if that > >>> is > >>>> the desired direction. > >>>> > >>>> My question is if we generally like the design of doing=20 > >>>> compression in > >>> common > >>>> MinPlatform code, decompression in board specific FSP wrapper code. > >>>> The alternative design is to do compression and decompression=20 > >>>> inside the > FSP. > >>> This > >>>> seems like a slightly simpler separation of responsibilities. > >>>> The board code is responsible for save/restore, the FSP code is=20 > >>>> responsible > >>> for > >>>> data blob creation and use. Data integrity, authentication,=20 > >>>> 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=20 > >>>> effectively forces both bootloader and FSP to carry=20 > >>>> compression/decompression. The compression/decompression aren't=20 > >>>> likely to need to be silicon specific. And bootloader may have=20 > >>>> more requirements to balance than just the silicon requirements. > >>>> > >>>> Can I get some votes on preferred answer for=20 > >>>> 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=20 > >>>> ; Desimone, Nathaniel L=20 > >>>> ; Oram, Isaac W > >>> > >>>> Subject: [edk2-platforms:PATCH V1] MinPlatformPkg/SaveMemoryConfig: > >>>> Support NVS Data compression. > >>>> > >>>> Around 50KB "FspNonVolatileStorageHob" data can be compressed to=20 > >>>> approximately 3 KB which can save NVRAM space and enhance life of=20 > >>>> 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=20 > >>>> #include=20 > >>>> +#include > +#include > >>>> /** This is the standard EFI driver point= that > >>> detects > >>>> whether there is a@@ -45,6 +47,9 @@ SaveMemoryConfigEntryPoint ( > >>>> UINTN DataSize; UINTN BufferSize; B= OOLEAN > >>>> DataIsIdentical;+ VOID *CompressedData;+ UINT64 > >>>> CompressedSize;+ UINTN CompressedAllocationPages; Da= taSize > >>> =3D 0; > >>>> BufferSize =3D 0;@@ -73,6 +78,35 @@ SaveMemoryConfigEntryPoint = ( > >>>> } } + if (PcdGetBool (PcdEnableCompressFspNvsHob) =3D=3D 1= ) {+ > >>>> CompressedData =3D NULL;+ CompressedSize = =3D 0;+ > >>>> CompressedAllocationPages =3D 0;++ DEBUG ((DEBUG_INFO, "compressi= ng > >>> mem > >>>> config nvs variable\n"));+ if (DataSize > 0) {+ > >>> CompressedAllocationPages =3D > >>>> EFI_SIZE_TO_PAGES (DataSize);+ CompressedData =3D AllocatePages > >>>> (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, Dat= aSize, > >>>> 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 CompressedData;+ = DataSize > >>> =3D > >>>> (UINTN) CompressedSize;+ }+ }+ }+ if (HobData !=3D NULL) = { DEBUG > >>>> ((DEBUG_INFO, "FspNvsHob.NvsDataLength:%d\n", DataSize)); DEBUG > >>>> ((DEBUG_INFO, "FspNvsHob.NvsDataPtr : 0x%x\n", HobData));diff --gi= t > >>>> > >>> > 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+ Compre= ssLib > >>>> [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=20 > >>>> + 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=20 > >>>> 31+as > >>>> compressed data.+# Data compression can significantly reduce=20 > >>>> variable > >>> storage > >>>> usage for FSP NVS buffer data.+# Platforms that choose to=20 > >>>> 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 > > > >