From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR01-VE1-obe.outbound.protection.outlook.com (EUR01-VE1-obe.outbound.protection.outlook.com [40.107.14.77]) by mx.groups.io with SMTP id smtpd.web11.5257.1613143749480397635 for ; Fri, 12 Feb 2021 07:29:10 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=mOwJBLYl; spf=pass (domain: arm.com, ip: 40.107.14.77, mailfrom: sami.mujawar@arm.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=v5LLCchxd0jFRkFfjQAXafqhrtGoepbARmZpkZ2sE3I=; b=mOwJBLYl2MZXoVKMutptRnePtNzIcpVtpJuj159tWYHC/iybozfdjewaKg4yvO3fZ75gPvlkOy7z5GU80KM5Q3sLClEA1T0YQl0RRnbwifVz/odIZx8g9dogW5MIhsRA1svY96GZWeN911+qrKg9BGG1ikUk2mblPkxjkHwFgyw= Received: from AM6P194CA0036.EURP194.PROD.OUTLOOK.COM (2603:10a6:209:90::49) by AM4PR0802MB2289.eurprd08.prod.outlook.com (2603:10a6:200:62::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.23; Fri, 12 Feb 2021 15:29:06 +0000 Received: from VE1EUR03FT004.eop-EUR03.prod.protection.outlook.com (2603:10a6:209:90:cafe::b9) by AM6P194CA0036.outlook.office365.com (2603:10a6:209:90::49) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3846.25 via Frontend Transport; Fri, 12 Feb 2021 15:29:06 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; edk2.groups.io; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;edk2.groups.io; dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT004.mail.protection.outlook.com (10.152.18.106) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3846.25 via Frontend Transport; Fri, 12 Feb 2021 15:29:06 +0000 Received: ("Tessian outbound 4d8113405d55:v71"); Fri, 12 Feb 2021 15:29:05 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 1fbb71747b22c4a9 X-CR-MTA-TID: 64aa7808 Received: from ff483892a87a.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 15919A04-C0CA-4031-873F-FF308CFB7CCD.1; Fri, 12 Feb 2021 15:28:59 +0000 Received: from EUR05-DB8-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id ff483892a87a.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Fri, 12 Feb 2021 15:28:59 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=J4Lc1c4yFtBOK14WucpDgKq/WK13zTsIN3YB3jYGzy5UTnns1Lx/A6MyuScjYdjX3bibxvMn+4PMYu24OTdT1zN+VbY4zqkPu+YR5bohvGJ4bWx6jIYqoyzZmX226LatgZuK4dtUW1lsBDZpPVPb7sixXAkcbYvzthfcZKUR82z/X1qAeP3eJ/9JnIrUurKKzRvYw32F8Bv1CoIlaqapaJbFNYpxHkOh99lW3Iq3lZhikH8RiLsQ6Ud2XcSoHsaKghsMJi/q9LcylyAmEwP9Dlz4RJ5n7qkQruhpwflziXbzKysjybxJsTdDz6iFhV7aUdi6d+xqeSLUyNDYNeBzeg== 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=v5LLCchxd0jFRkFfjQAXafqhrtGoepbARmZpkZ2sE3I=; b=mwONtX7nRgGPrleX3jwa0kTvsNOdAu2RefbOMnc3KvDaAwtDLj9VA1sa42YSm38as9KrY2kGSP/WL41N1vGEDLcv0oJDkRT70LqPAdqDIq7nuamrhLMhbift9motIEASUGUW4ax4UTkH8jQ31ox8Zk6x591D93JeQM3FKk4HUX5Q/HXpO/gSU/KWQn4zpPr08jubcprLK/6f2BAZFXwOB3mNWsMni/nBu+KVslZpdZs8P/Z7LR6TZ2jPKE98qTy8mVvvemBxTn7e3Bkp1PBjYb6N01q1dQ1ItOg7IANg76Zk2ZQvh3w+Jb0qAeq449ETiWTKNLabE67n6e83koFFqQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=v5LLCchxd0jFRkFfjQAXafqhrtGoepbARmZpkZ2sE3I=; b=mOwJBLYl2MZXoVKMutptRnePtNzIcpVtpJuj159tWYHC/iybozfdjewaKg4yvO3fZ75gPvlkOy7z5GU80KM5Q3sLClEA1T0YQl0RRnbwifVz/odIZx8g9dogW5MIhsRA1svY96GZWeN911+qrKg9BGG1ikUk2mblPkxjkHwFgyw= Received: from DB7PR08MB3097.eurprd08.prod.outlook.com (2603:10a6:5:1d::27) by DB9PR08MB6491.eurprd08.prod.outlook.com (2603:10a6:10:23f::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3846.29; Fri, 12 Feb 2021 15:28:58 +0000 Received: from DB7PR08MB3097.eurprd08.prod.outlook.com ([fe80::8c43:eec3:76be:9001]) by DB7PR08MB3097.eurprd08.prod.outlook.com ([fe80::8c43:eec3:76be:9001%4]) with mapi id 15.20.3846.035; Fri, 12 Feb 2021 15:28:58 +0000 From: "Sami Mujawar" To: Ilias Apalodimas CC: "devel@edk2.groups.io" , "ardb+tianocore@kernel.org" , "sughosh.ganu@linaro.org" , "leif@nuviainc.com" , nd Subject: Re: [PATCH 1/2 v4] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver Thread-Topic: [PATCH 1/2 v4] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver Thread-Index: AQHW+hWt2Qkm2OVERUurwurCWlV8tapUcbbQgAAilICAAB2s8A== Date: Fri, 12 Feb 2021 15:28:57 +0000 Message-ID: References: <20210203101626.1200943-1-ilias.apalodimas@linaro.org> <20210203101626.1200943-2-ilias.apalodimas@linaro.org> In-Reply-To: Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: B40793AAB50FA240B76D6A55B96D208A.0 Authentication-Results-Original: linaro.org; dkim=none (message not signed) header.d=none;linaro.org; dmarc=none action=none header.from=arm.com; x-originating-ip: [217.140.99.251] x-ms-publictraffictype: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 1f524df9-3214-4d6b-e778-08d8cf6aef83 x-ms-traffictypediagnostic: DB9PR08MB6491:|AM4PR0802MB2289: X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true nodisclaimer: true x-ms-oob-tlc-oobclassifiers: OLM:10000;OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: cOIvm74C1XhC8gDo2PpS/Q/87oNe5nFKNGKxzqmi1XZed7o0HvuD8cWH9O7hEcDjf4f6qZmV74mxrI08hiAlrHfsBwkHvjaZmTABbVe7e84HYGafyUrJO60FjjmQarRgCc8QNYDZXurnhytTz3kGatyKO23Vnv+h8DZyvaAOLw20E1QCv2Bf1EIbmVo91NpeUuzyW5cTc0GkAg9PlSu4Yc99F1RQDg3apsmsgJlyPYIrk31dDAtWXOpmjNmsWmfhtGWO96f2qATu5hjvZeGU5ZC+zNmagmFKO5Z6b6c8iXZfJkkrXCcSHr2VFB1z4E/ZvKvbkA090/LhaFgEIlewkX2nNtedR0M1k05LDE6q6yI7SDDOJa3woFmQANQidXbqFvnZ06P2gFqJMSEAFjHvUyVxTXqqk6AHO1oDx6CcJuMGjJM2dpaCgaVC5kOIufz8Hn2xBL83839S5te9O8HqSYLE0uHn2Yy6mBVLmw+6q2Sl56sdAAdngvLRVvUh/JZ58KeN0pGK0NxSDcLtH5xV5iNwefObFGouVnHJTegZfxHYgY5YMkWQ0WI74QP6PUWDw5Qg76TqNGZtnrMv/dvcVR2KTC/PJjbm+o1Iwi0zeGQ= X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DB7PR08MB3097.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(6029001)(4636009)(136003)(376002)(366004)(396003)(39850400004)(346002)(6506007)(316002)(9686003)(76116006)(64756008)(66556008)(66476007)(186003)(66446008)(26005)(478600001)(71200400001)(33656002)(83380400001)(52536014)(8676002)(86362001)(966005)(7696005)(55016002)(8936002)(54906003)(5660300002)(2906002)(53546011)(66946007)(4326008)(6916009);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?/fSyCmoy4RzJeXElkrY1DrC8qgZS30gLpmXdwFfZ9y/3Tjbdf+R6FO0y5wZ4?= =?us-ascii?Q?/7iOX0+niYRrpkRWaWgdHciYVo0bGEotMBj9rUPVdsVprPOYaMtWuCdaBoFJ?= =?us-ascii?Q?boTPHllleY7605O9jyk/3Q6BpVl2XwlhjV/UAoFFTWCNlpaxIB8g0gDr65W5?= =?us-ascii?Q?FHHUez1dP15PWDE3jmsV6RNZf+QyaM/MVTvfNA3cggHgWPTkglwDaDp4Iy34?= =?us-ascii?Q?d0zpdJHYc1lMAl55HnwjphT46AdQnEq/nIbca20RyN36A0VzkDYtTHQii53M?= =?us-ascii?Q?qlLVfVI1Dz8Ajg1HEv321DFqN2DYLeGuz4KnhytwHF2xgPNQKnh20A5MlgRc?= =?us-ascii?Q?B7Fu0kFIAKFws0j+QevMtYRzyUdPtwxgM3EBIl+69HPi6EJdabqVJiiJV2eX?= =?us-ascii?Q?dXjGE/1HO4fEP08lrH434J673c14rhnftgKqls6z/b5YGQY/eVbW9NRb3RAT?= =?us-ascii?Q?XrroKhslH88p9QGHdHtHcjJBwS++n9ME2FvlVCfHmwi8OPwls6EYD/OKJ+Um?= =?us-ascii?Q?BD9LA7zLGy5J3u9w6pyvY2nAFbn8IleQSmrHS4vXWqklZpoX5JvXPBoSfy9M?= =?us-ascii?Q?1h886hKpPhZSYuLjwWbO611ltH/bxCA0oadwr8YYt6C/EVwZ8bIJeHByOVW7?= =?us-ascii?Q?+fBuXGnerRLEMTmOOsJ22TnJUVXP7E8P5kkgfj1BHWIdhD08BLsOqjM63Zk2?= =?us-ascii?Q?6IAFgNPqo+kqbpS78OifUugNJmjqMyzmQu1YE1ag8VKNWEyu/5h/N0C0sgfB?= =?us-ascii?Q?uOxqtJuc1QHmgdi4HkqIB1j8MJCIYOKcMY7M3oz01yonZO9kmXArMQRGMMwz?= =?us-ascii?Q?ckGIfLrqKWzSlCr8zFXlGhOaUzFBlliBAo6GG0R17xExdO77Qxlvk33akmiT?= =?us-ascii?Q?bWU4NbnDCSzMGKKRAAvhO6kvXBEwJ5pwl7JlVJnykkBPjTa4V07GhIWMtlJQ?= =?us-ascii?Q?EqG1K2ePS7aAKqTytwpwmjuCTCDOQmh1zwUKXmRKwXL6EOvRJhX1/wmXzBWn?= =?us-ascii?Q?l3sOwZApkgwKU89ggvwRGzg8+7CkCuyQtCTwfiwA9A1PF8r9rGsRAFWRy/Ku?= =?us-ascii?Q?E51+vAMhcaXNL/N3bP2uZITOGUIyAPCQ2/vzc5TZEo2R67DGVUosRrNC8PbE?= =?us-ascii?Q?QzgHGjgfn0AZSZqrw6zIsKfLf/6QrWLOR6AzuPDg9C3HJL2KiwgqVgcbgVBp?= =?us-ascii?Q?yuCzbCnMy6vI8+jsXimZbOsjTEwLmh5l8s+h23QMs+sQHP4vJmknFBaBXj8S?= =?us-ascii?Q?zwxoeCZg2Nn6Q2xC1Kagqqmr7YbAzuST8RL30bQ3lb+ZQwzZH/gNZ9pUY68D?= =?us-ascii?Q?9sV9oZeAkIbWyrRrzIQZGEfy?= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR08MB6491 Original-Authentication-Results: linaro.org; dkim=none (message not signed) header.d=none;linaro.org; dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT004.eop-EUR03.prod.protection.outlook.com X-MS-Office365-Filtering-Correlation-Id-Prvs: 70a5f0ff-d628-43b2-69d4-08d8cf6aea81 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 9qh8ez87rA4SHXEi3kzqNHlLiSEXthREF++344ZCWVOchDV80mdPNtdF73/XdKGQXk5ZEde0IZ+HrORcN24Tc5WE/nhqSTwQCGy2W+0Mk1WgadGIRL3OCUI+JeOw8p7KZzV9Pq7t77b8hjETw7MFZTarfgASwPKEDuAR9QdTiK6ZFVBSAmBusRlElYO+vsPFiRKrkeC39/9BJ3Kvsqi3pOkhMC5/5fC6H9/ng6myPXXDBUXpDuVL5ry3FYO+tc+VfI8jpNhwQmQsDnST5tp+TTtfAy4csCG/3XVGvc7K1jrI/4ZeNLncpBI4BvxQcCa5Y1KhPkpv5RtDiqrDDMdxefvbh4Efff0m88PikFpPJKOsu7ZpCvg7f/bdPTeMOO39HPM1ipGfX/glaEIha7aE8kfAuRb0Tk48CW/dCuNxaXZ+KuIz6Ajbrn5Tzrj3OKjzvWuwbvs8TXUEZddKK45Rv6j8qEMRl2qVekGxbDISVo2LMugi4R5NNn9wLnaypzAfnWZeyETylHT7nnTrfV4sIirtTyjl04sVQSSPOEIhfMOuOcqLoZ+ZrTMAWWV2M89DFLHXYfnBsmrUiRURDNAfRYItA/6f6Caqo11MugIqEJ/4Fu2dFtBBeTUSr/b03hPWTXRswZawfVnPUhgQQ2bywU0KuEOqTj3MUy+2KrojQtj6HsRYNOqrFXEHN7YVBGQU4DhXd5ceTPrkEE7KxvaRjr3UVJJ1EB36jCxZZVNdSm8= X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(6029001)(4636009)(136003)(376002)(39850400004)(346002)(396003)(46966006)(36840700001)(82740400003)(6506007)(336012)(86362001)(186003)(53546011)(966005)(36860700001)(5660300002)(6862004)(356005)(83380400001)(81166007)(9686003)(4326008)(82310400003)(7696005)(478600001)(52536014)(33656002)(47076005)(8936002)(55016002)(54906003)(8676002)(26005)(316002)(2906002)(70206006)(70586007);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Feb 2021 15:29:06.3078 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 1f524df9-3214-4d6b-e778-08d8cf6aef83 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: VE1EUR03FT004.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0802MB2289 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Ilias, > +#ifndef OPTEE_RPMB_FVB_H > +#define OPTEE_RPMB_FVB_H Just remembered the include file guard should be 'OPTEE_RPMB_FVB_H_' in Dri= vers/OpTeeRpmb/OpTeeRpmbFvb.h Regards, Sami Mujawar -----Original Message----- From: Ilias Apalodimas =20 Sent: 12 February 2021 01:38 PM To: Sami Mujawar Cc: devel@edk2.groups.io; ard+tianocore@kernel.org; sughosh.ganu@linaro.org= ; leif@nuviainc.com; nd Subject: Re: [PATCH 1/2 v4] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB dr= iver Hi Sami,=20 On Fri, Feb 12, 2021 at 01:11:10PM +0000, Sami Mujawar wrote: > Hi Ilias, >=20 > Apologies for the delay in reviewing this patch. >=20 No worries, thanks for the comments > Please find my response inline marked [SAMI]. > There are some coding standard issues remaining. I believe these are not = reported by Ecc.=20 Yea Ecc or PatchCheck didn't catch any of these.=20 > Also, InitializeFvAndVariableStoreHeaders() would need error handling for= a memory allocation failure. >=20 > Regards, >=20 > Sami Mujawar >=20 >=20 [...] > + Instance =3D INSTANCE_FROM_FVB_THIS(FvbProtocol); > [SAMI] Space needed before opening parenthesis. > See https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification= /5_source_files/52_spacing#5-2-2-6-always-put-space-before-an-open-parenthe= sis > Please also check other places in this patch. Ok=20 > [/SAMI] >=20 > + // The Pcd is user defined, so make sure we don't overflow >=20 > + if (Instance->MemBaseAddress > MAX_UINT64 - PcdGet32 (PcdFlashNvStorag= eVariableSize)) { >=20 > + return EFI_INVALID_PARAMETER; >=20 > + } >=20 > + >=20 > + if (Instance->MemBaseAddress > MAX_UINT64 - PcdGet32 (PcdFlashNvStorag= eVariableSize) - >=20 > + PcdGet32 (PcdFlashNvStorageFtwWorkingSize)) { >=20 > + return EFI_INVALID_PARAMETER; >=20 > + } >=20 > + >=20 > + // Patch PCDs with the the correct values >=20 > + PatchPcdSet64 (PcdFlashNvStorageVariableBase64, Instance->MemBaseAddre= ss); >=20 > + PatchPcdSet64 (PcdFlashNvStorageFtwWorkingBase64, Instance->MemBaseAdd= ress + >=20 > + PcdGet32 (PcdFlashNvStorageVariableSize)); > [SAMI] Please see alignment guidelines described in EDKII coding standard= at: > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_s= ource_files/52_spacing#5-2-2-4-subsequent-lines-of-multi-line-function-call= s-should-line-up-two-spaces-from-the-beginning-of-the-function-name > Please also fix this at other places in this patch. Ok=20 > [/SAMI] >=20 [...] > +[Pcd] >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > [SAMI] Please sort in alphabetical order. > [/SAMI] Ok=20 >=20 [...] >=20 > + >=20 > +[Pcd] >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 >=20 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > [SAMI] Please sort in alphabetical order. > [/SAMI] Ok >=20 > + >=20 > +OpTeeRpmbFvbSetAttributes ( >=20 > + IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This, >=20 > + IN OUT EFI_FVB_ATTRIBUTES_2 *Attributes >=20 > + ) >=20 > +{ >=20 > + return EFI_SUCCESS; // ignore for now > [SAMI] Should this be EFI_UNSUPPORTED? If not, a comment may be helpful. Some of the drivers I looked were returning EFI_SUCCESS.=20 Checking again I see some returning EFI_UNSUPPORTED as well, so I'll switch= to that. > [/SAMI] >=20 > +} [...] >=20 > + *NumberOfBlocks =3D Instance->NBlocks - (UINTN) Lba; > [SAMI] There should be no space between (UINTN) and Lba. > Please see point 2 at https://edk2-docs.gitbook.io/edk-ii-c-coding-standa= rds-specification/v/release%2F2.20/3_quick_reference#3-2-3-formatting-horiz= ontal-spacing > [/SAMI] Ok >=20 > + *BlockSize =3D Instance->BlockSize; >=20 > + =20 [...] > +OpTeeRpmbFvbRead ( >=20 > + IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This, >=20 > + IN EFI_LBA Lba, >=20 > + IN UINTN Offset, >=20 > + IN OUT UINTN *NumBytes, >=20 > + IN OUT UINT8 *Buffer >=20 > + ) >=20 > +{ >=20 > + EFI_STATUS Status =3D EFI_SUCCESS; >=20 > + MEM_INSTANCE *Instance; >=20 > + VOID *Base; >=20 > + >=20 > + Instance =3D INSTANCE_FROM_FVB_THIS(This); >=20 > + if (!Instance->Initialized) { >=20 > + Instance->Initialize (Instance); > [SAMI] Should the status be checked here and returned?=20 > Or is the assumption that Initialize will always succeed. If so,=20 > - a comment would be helpful. > - the Status variable here is redundant.=20 > Same comment for OpTeeRpmbFvbWrite(). No you are right 'Status =3D' is missing, I'll add that. > [/SAMI] > + } >=20 > + >=20 > + Base =3D (VOID *)Instance->MemBaseAddress + Lba * Instance->BlockSize = + Offset; > [SAMI] Use of parentheses is encouraged. See https://edk2-docs.gitbook.io= /edk-ii-c-coding-standards-specification/5_source_files/52_spacing#5-2-2-10= -use-extra-parentheses-rather-than-depending-on-in-depth-knowledge-of-the-o= rder-of-precedence-of-c > [/SAMI] ok [...] >=20 > + } >=20 > + NumBytes =3D NumLba * Instance->BlockSize; >=20 > + Base =3D (VOID *)Instance->MemBaseAddress + Start * Instance->BlockS= ize; >=20 > + Buf =3D AllocatePool(NumLba * Instance->BlockSize); >=20 > + if (!Buf) { > [SAMI] if (Buf =3D=3D NULL) { > [/SAMI] Ok >=20 > + return EFI_DEVICE_ERROR; >=20 [...] > +EFI_STATUS >=20 > +InitializeFvAndVariableStoreHeaders ( >=20 > + MEM_INSTANCE *Instance >=20 > + ) >=20 > +{ >=20 > + EFI_FIRMWARE_VOLUME_HEADER *FirmwareVolumeHeader; >=20 > + VARIABLE_STORE_HEADER *VariableStoreHeader; >=20 > + EFI_STATUS Status =3D EFI_SUCCESS; >=20 > + UINTN HeadersLength; >=20 > + VOID* Headers; >=20 > + >=20 > + HeadersLength =3D sizeof(EFI_FIRMWARE_VOLUME_HEADER) + >=20 > + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + >=20 > + sizeof(VARIABLE_STORE_HEADER); >=20 > + Headers =3D AllocateZeroPool(HeadersLength); > [SAMI] Error handling for allocation failure is needed here. > [/SAMI] Yes missed that >=20 [...] > + // > [SAMI] ASSERT (Addr !=3D NULL); could be removed from the line above an i= nstead ASSERT (0); could be added here. > [/SAMI] Ok > + return EFI_OUT_OF_RESOURCES; > + I'll fix those and send a v5.=20 Thanks! /Ilias