From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0ED2621196815 for ; Thu, 29 Nov 2018 07:57:13 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Nov 2018 07:57:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,295,1539673200"; d="scan'208";a="97580707" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga008.fm.intel.com with ESMTP; 29 Nov 2018 07:57:12 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 29 Nov 2018 07:57:12 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 29 Nov 2018 07:57:12 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.203]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.176]) with mapi id 14.03.0415.000; Thu, 29 Nov 2018 23:57:10 +0800 From: "Gao, Liming" To: Jagadeesh Ujja , "edk2-devel@lists.01.org" , "Zhang, Chao B" , "leif.lindholm@linaro.org" , "ard.biesheuvel@linaro.org" Thread-Topic: [RFC PATCH v3 00/11] Extend secure variable service to be usable from Standalone MM Thread-Index: AQHUhv26okynB9NX0kWVH3Cq4cHnyqVm4rXA Date: Thu, 29 Nov 2018 15:57:09 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E380CE6@SHSMSX104.ccr.corp.intel.com> References: <1543397709-31847-1-git-send-email-jagadeesh.ujja@arm.com> In-Reply-To: <1543397709-31847-1-git-send-email-jagadeesh.ujja@arm.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGVmMjc5ZDgtMGVjMC00OGJkLWFhYTUtOGViMjJiYzA0YzJkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiempzZytmV093Z1NrSkNKZTVVa2Z3SWcxNGxQdElGV25XY1NnSjZpYXFYRjBIN1ZSSzlEcURnczRucXh0MFpsWSJ9 dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [RFC PATCH v3 00/11] Extend secure variable service to be usable from Standalone MM X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Nov 2018 15:57:14 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable My comment is below.=20 1. Please don't update MemoryFence() implementation. It will impact all con= sumer code. AsmLfence() is X86 specific API. You can implement the internal= function in the arch specific source file to call AsmLfence() for X86 and = call MemoryFence() for ARM. This internal function will be called in the co= mmon logic.=20 2. On StandaloneMmServicesTableLib.h, I suggest to add it into MdePkg, and = add StandaloneMmRuntimeDxe library into MdePkg. This library sets gMmst is = NULL, and always return FALSE in InMm().=20 3. On PcdStandaloneMmEnable, I also suggest to add it into MdePkg. It can b= e used to control the driver logic in the different packages.=20 With 2 & 3, other edk2 packages don't need to depend on StandaloneMmPkg/Sta= ndaloneMmPkg.dec > -----Original Message----- > From: Jagadeesh Ujja [mailto:jagadeesh.ujja@arm.com] > Sent: Wednesday, November 28, 2018 5:35 PM > To: edk2-devel@lists.01.org; Gao, Liming ; Zhang, C= hao B ; leif.lindholm@linaro.org; > ard.biesheuvel@linaro.org > Subject: [RFC PATCH v3 00/11] Extend secure variable service to be usable= from Standalone MM >=20 > Changes since v2: > - Added 'Contributed-under' tag, removed Change-ID tag and > maintained a single signed-off-by for the all the patches. >=20 > Changes since v1: > - Addressed all the comments from Liming Gao > - Removed the use of #ifdef/#else/#endif and used a Pcd instead to > select between MM and non-MM paths. > - Removed all dependencies on edk2-platforms. > - Dropped the use of mMmst and used gSmst instead. > - Added a dummy implementation UefiRuntimeServiceTableLib for > MM_STANDALONE usage > - Replaced all uses of AsmLfence with MemoryFence from variable > service code. > - Add a new StandaloneMmRuntimeDxe library to for use by non-MM code. >=20 > This RFC patch series extends the existing secure variable service suppor= t for > use with Standalone MM. This is applicable to paltforms that use Standalo= ne > Management Mode to protect access to non-volatile memory (NOR flash in ca= se > of these patches) used to store the secure EFI variables. >=20 > The first patch pulls in additional libraries from the staging branch of > StandaloneMmPkg into the edk2's StandaloneMmPkg. The existing secure vari= able > service implementation supports only the traditional MM mode and so the r= est > of the patches extends the existing secure variable service support to be > useable with Standalone MM mode as well. >=20 > This patch series is being posted as an RFC to get feedback on the approa= ch taken > in these patches. >=20 > Jagadeesh Ujja (11): > MdeModulePkg/Variable: replace all uses of AsmLfence with MemoryFence > StandaloneMmPkg: Pull in additonal libraries from staging branch > MdeModulePkg/Library: Add StandaloneMmRuntimeDxe library > ArmPlatformPkg/NorFlashDxe: allow reusability as a MM driver > MdeModulePkg/FaultTolerantWriteDxe: allow reusability as a MM driver > MdeModulePkg/Variable/RuntimeDxe: adapt for usability with MM > Standalone > MdeModulePkg/Variable/RuntimeDxe: adapt as a MM Standalone driver > SecurityPkg/AuthVariableLib: allow MM_STANDALONE drivers to use this > library > MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this > library > CryptoPkg/BaseCryptLib: allow MM_STANDALONE drivers to use this > library > CryptoPkg/BaseCryptLib: Hack to get time in MM Standalone mode >=20 > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > | 3 + > ArmPlatformPkg/Drivers/NorFlashDxe/{NorFlashDxe.inf =3D> NorFlashStandal= oneMm.inf} > | 28 +- > CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > | 8 +- > CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > | 5 + > MdeModulePkg/Library/{VarCheckLib/VarCheckLib.inf =3D> StandaloneMmRunti= meDxe/StandaloneMmRuntimeDxe.inf} > | 22 +- > MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf > | 5 +- > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > | 2 + > MdeModulePkg/Universal/FaultTolerantWriteDxe/{FaultTolerantWriteDxe.inf = =3D> FaultTolerantWriteStandaloneMm.inf} | > 53 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > | 2 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf > | 4 + > MdeModulePkg/Universal/Variable/RuntimeDxe/{VariableRuntimeDxe.inf =3D> = VariableStandaloneMm.inf} > | 107 ++- > SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf > | 5 +- > StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.in= f > | 2 +- > StandaloneMmPkg/Library/{StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.i= nf =3D> > StandaloneMmHobLib/StandaloneMmHobLib.inf} | 11 +- > StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemo= ryAllocationLib.inf > | 45 ++ > StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmService= sTableLib.inf > | 36 + > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h > | 5 +- > MdeModulePkg/Include/Library/StandaloneMmRuntimeDxe.h > | 39 + > StandaloneMmPkg/Include/Library/StandaloneMmServicesTableLib.h > | 47 ++ > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c > | 2 +- > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > | 211 ++++- > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > | 88 ++- > CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c > | 27 +- > MdeModulePkg/Library/StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.c > | 36 + > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c > | 207 +++-- > MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c > | 27 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c > | 2 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > | 37 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > | 201 ++++- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > | 31 +- > MdePkg/Library/BaseLib/X86MemoryFence.c > | 2 +- > StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneMmCoreHobLi= bInternal.c > | 64 ++ > StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c > | 655 ++++++++++++++++ > StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemo= ryAllocationLib.c > | 824 ++++++++++++++++++++ > StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmService= sTableLib.c > | 64 ++ > 35 files changed, 2564 insertions(+), 343 deletions(-) > copy ArmPlatformPkg/Drivers/NorFlashDxe/{NorFlashDxe.inf =3D> NorFlashSt= andaloneMm.inf} (71%) > copy MdeModulePkg/Library/{VarCheckLib/VarCheckLib.inf =3D> StandaloneMm= RuntimeDxe/StandaloneMmRuntimeDxe.inf} (51%) > copy MdeModulePkg/Universal/FaultTolerantWriteDxe/{FaultTolerantWriteDxe= .inf =3D> FaultTolerantWriteStandaloneMm.inf} (54%) > copy MdeModulePkg/Universal/Variable/RuntimeDxe/{VariableRuntimeDxe.inf = =3D> VariableStandaloneMm.inf} (54%) > copy StandaloneMmPkg/Library/{StandaloneMmCoreHobLib/StandaloneMmCoreHob= Lib.inf =3D> > StandaloneMmHobLib/StandaloneMmHobLib.inf} (79%) > create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationL= ib/StandaloneMmMemoryAllocationLib.inf > create mode 100644 StandaloneMmPkg/Library/StandaloneMmServicesTableLib/= StandaloneMmServicesTableLib.inf > create mode 100644 MdeModulePkg/Include/Library/StandaloneMmRuntimeDxe.h > create mode 100644 StandaloneMmPkg/Include/Library/StandaloneMmServicesT= ableLib.h > create mode 100644 MdeModulePkg/Library/StandaloneMmRuntimeDxe/Standalon= eMmRuntimeDxe.c > create mode 100644 StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/St= andaloneMmCoreHobLibInternal.c > create mode 100644 StandaloneMmPkg/Library/StandaloneMmHobLib/Standalone= MmHobLib.c > create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationL= ib/StandaloneMmMemoryAllocationLib.c > create mode 100644 StandaloneMmPkg/Library/StandaloneMmServicesTableLib/= StandaloneMmServicesTableLib.c >=20 > -- > 2.7.4