From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (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 5466C2114A6CA for ; Thu, 20 Sep 2018 19:38:56 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Sep 2018 19:38:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,282,1534834800"; d="scan'208";a="92417560" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga001.jf.intel.com with ESMTP; 20 Sep 2018 19:38:46 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 20 Sep 2018 19:38:46 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 20 Sep 2018 19:38:45 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.227]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.226]) with mapi id 14.03.0319.002; Fri, 21 Sep 2018 10:38:36 +0800 From: "Wu, Hao A" To: Ard Biesheuvel , Leif Lindholm , Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" , "Yao, Jiewen" , "Gao, Liming" Thread-Topic: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API Thread-Index: AQHUUKztoBciGGpH+0S4TvkVCdP37aT4n/KAgAFatDA= Date: Fri, 21 Sep 2018 02:38:36 +0000 Message-ID: References: <20180920064103.14600-1-hao.a.wu@intel.com> <20180920064103.14600-2-hao.a.wu@intel.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API 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: Fri, 21 Sep 2018 02:38:56 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Ard and Leif, This commit aims to add to a new BaseLib API to implement the serializing load operations functionality (for IA32/X64, called LFENCE). For the 1st version of this commit, this API is named as 'LoadFence'. The implementation only covers IA32/X64 arch, and does an empty implementation for ARM/AARCH64. But as Laszlo pointed out (comment (2) below), the empty implementation for ARM/AARCH64 may be inappropriate, I would like to turn to you for some helps or suggestions on the implementation of this API. Also, as Laszlo pointed out in his comment (3) below, if the implementations between ARM & AARCH64 are the same (not sure if this is the case, I am not very familiar with ARM instructions). Do you have comments or preference on the location of the codes? A) Duplicate implementations under BaseLib\Arm and BaseLib\AArch64 (Like the IA32 & X64 implementation) B) Use one common implementation under BaseLib\Arm Thanks in advance. Best Regards, Hao Wu > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of La= szlo > Ersek > Sent: Thursday, September 20, 2018 9:13 PM > To: Wu, Hao A; edk2-devel@lists.01.org > Cc: Kinney, Michael D; Yao, Jiewen; Gao, Liming > Subject: Re: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API >=20 > On 09/20/18 08:40, Hao Wu wrote: > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D1193 > > > > This commit will add a new BaseLib API LoadFence(). This API will perfo= rm > > a serializing operation on all load-from-memory instructions that were > > issued prior to the call of this function. > > > > The purpose of adding this API is to mitigate of the [CVE-2017-5753] > > Bounds Check Bypass issue when untrusted data are being processed withi= n > > SMM. More details can be referred at the 'Bounds check bypass mitigatio= n' > > section at the below link: > > > > https://software.intel.com/security-software-guidance/insights/host- > firmware-speculative-execution-side-channel-mitigation > > > > Cc: Ard Biesheuvel > > Cc: Laszlo Ersek > > Cc: Jiewen Yao > > Cc: Michael D Kinney > > Cc: Liming Gao > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Hao Wu > > --- > > MdePkg/Include/Library/BaseLib.h | 12 +++++++ > > MdePkg/Library/BaseLib/Arm/LoadFence.c | 26 ++++++++++++++ > > MdePkg/Library/BaseLib/BaseLib.inf | 4 +++ > > MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 15 +++++++- > > MdePkg/Library/BaseLib/Ia32/LoadFence.nasm | 37 +++++++++++++++++++ > > MdePkg/Library/BaseLib/X64/LoadFence.nasm | 38 > ++++++++++++++++++++ > > 6 files changed, 131 insertions(+), 1 deletion(-) > > > > diff --git a/MdePkg/Include/Library/BaseLib.h > b/MdePkg/Include/Library/BaseLib.h > > index 123ae19dc2..194726ca35 100644 > > --- a/MdePkg/Include/Library/BaseLib.h > > +++ b/MdePkg/Include/Library/BaseLib.h > > @@ -4939,6 +4939,18 @@ MemoryFence ( > > > > > > /** > > + Performs a serializing operation on all load-from-memory instruction= s that > > + were issued prior to the call of this function. > > + > > +**/ > > +VOID > > +EFIAPI > > +LoadFence ( > > + VOID > > + ); > > + > > + > > +/** > > Saves the current CPU context that can be restored with a call to Lo= ngJump() > > and returns 0. > > > > diff --git a/MdePkg/Library/BaseLib/Arm/LoadFence.c > b/MdePkg/Library/BaseLib/Arm/LoadFence.c > > new file mode 100644 > > index 0000000000..69f0c3a07e > > --- /dev/null > > +++ b/MdePkg/Library/BaseLib/Arm/LoadFence.c > > @@ -0,0 +1,26 @@ > > +/** @file > > + LoadFence() function for ARM. > > + > > + Copyright (C) 2018, Intel Corporation. All rights reserved.
> > + > > + This program and the accompanying materials are licensed and made > available > > + under the terms and conditions of the BSD License which accompanies = this > > + distribution. The full text of the license may be found at > > + http://opensource.org/licenses/bsd-license.php. > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" > BASIS, WITHOUT > > + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > > +**/ > > + > > +/** > > + Performs a serializing operation on all load-from-memory instruction= s that > > + were issued prior to the call of this function. > > + > > +**/ > > +VOID > > +EFIAPI > > +LoadFence ( > > + VOID > > + ) > > +{ > > +} > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf > b/MdePkg/Library/BaseLib/BaseLib.inf > > index a1b5ec4b75..f028fbc75a 100644 > > --- a/MdePkg/Library/BaseLib/BaseLib.inf > > +++ b/MdePkg/Library/BaseLib/BaseLib.inf > > @@ -68,6 +68,7 @@ > > > > [Sources.Ia32] > > Ia32/WriteTr.nasm > > + Ia32/LoadFence.nasm > > > > Ia32/Wbinvd.c | MSFT > > Ia32/WriteMm7.c | MSFT > > @@ -346,6 +347,7 @@ > > X64/EnableCache.nasm > > X64/DisableCache.nasm > > X64/WriteTr.nasm > > + X64/LoadFence.nasm > > > > X64/CpuBreakpoint.c | MSFT > > X64/WriteMsr64.c | MSFT > > @@ -580,6 +582,7 @@ > > [Sources.ARM] > > Arm/InternalSwitchStack.c > > Arm/Unaligned.c > > + Arm/LoadFence.c > > Math64.c | RVCT > > Math64.c | MSFT > > > > @@ -613,6 +616,7 @@ > > [Sources.AARCH64] > > Arm/InternalSwitchStack.c > > Arm/Unaligned.c > > + Arm/LoadFence.c > > Math64.c > > > > AArch64/MemoryFence.S | GCC > > diff --git a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > > index 9b7d875664..a79461cfbf 100644 > > --- a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > > +++ b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c > > @@ -1,7 +1,7 @@ > > /** @file > > Base Library CPU Functions for EBC > > > > - Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved. > > + Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. > > This program and the accompanying materials > > are licensed and made available under the terms and conditions of th= e BSD > License > > which accompanies this distribution. The full text of the license m= ay be > found at > > @@ -52,6 +52,19 @@ MemoryFence ( > > } > > > > /** > > + Performs a serializing operation on all load-from-memory instruction= s that > > + were issued prior to the call of this function. > > + > > +**/ > > +VOID > > +EFIAPI > > +LoadFence ( > > + VOID > > + ) > > +{ > > +} > > + > > +/** > > Disables CPU interrupts. > > > > **/ > > diff --git a/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm > b/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm > > new file mode 100644 > > index 0000000000..11600bea76 > > --- /dev/null > > +++ b/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm > > @@ -0,0 +1,37 @@ > > +;---------------------------------------------------------------------= --------- ; > > +; Copyright (c) 2018, Intel Corporation. All rights reserved.
> > +; This program and the accompanying materials > > +; are licensed and made available under the terms and conditions of th= e BSD > License > > +; which accompanies this distribution. The full text of the license m= ay be > found at > > +; http://opensource.org/licenses/bsd-license.php. > > +; > > +; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" > BASIS, > > +; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > > +; > > +; Module Name: > > +; > > +; LoadFence.nasm > > +; > > +; Abstract: > > +; > > +; Performs a serializing operation on all load-from-memory instructi= ons > that > > +; were issued prior to the call of this function. > > +; > > +; Notes: > > +; > > +;---------------------------------------------------------------------= --------- > > + > > + SECTION .text > > + > > +;---------------------------------------------------------------------= --------- > > +; VOID > > +; EFIAPI > > +; LoadFence ( > > +; VOID > > +; ); > > +;---------------------------------------------------------------------= --------- > > +global ASM_PFX(LoadFence) > > +ASM_PFX(LoadFence): > > + lfence > > + ret > > + > > diff --git a/MdePkg/Library/BaseLib/X64/LoadFence.nasm > b/MdePkg/Library/BaseLib/X64/LoadFence.nasm > > new file mode 100644 > > index 0000000000..c076d9789d > > --- /dev/null > > +++ b/MdePkg/Library/BaseLib/X64/LoadFence.nasm > > @@ -0,0 +1,38 @@ > > +;---------------------------------------------------------------------= --------- ; > > +; Copyright (c) 2018, Intel Corporation. All rights reserved.
> > +; This program and the accompanying materials > > +; are licensed and made available under the terms and conditions of th= e BSD > License > > +; which accompanies this distribution. The full text of the license m= ay be > found at > > +; http://opensource.org/licenses/bsd-license.php. > > +; > > +; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" > BASIS, > > +; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > > +; > > +; Module Name: > > +; > > +; LoadFence.nasm > > +; > > +; Abstract: > > +; > > +; Performs a serializing operation on all load-from-memory instructi= ons > that > > +; were issued prior to the call of this function. > > +; > > +; Notes: > > +; > > +;---------------------------------------------------------------------= --------- > > + > > + DEFAULT REL > > + SECTION .text > > + > > +;---------------------------------------------------------------------= --------- > > +; VOID > > +; EFIAPI > > +; LoadFence ( > > +; VOID > > +; ); > > +;---------------------------------------------------------------------= --------- > > +global ASM_PFX(LoadFence) > > +ASM_PFX(LoadFence): > > + lfence > > + ret > > + > > >=20 > Comments in no particular order: >=20 > (1) I think the EBC stub implementation should go into a separate file > under "MdePkg/Library/BaseLib/Ebc". >=20 > (2) Given that the ARM memory model is laxer than x86, I'm doubtful that > an empty implementation is appropriate. I expect a DMB variant should be > used, but I totally defer to Ard and Leif on that. >=20 > (3) We have Arm/ and AArch64/ subdirectories, but only one common > variant is provided, under Arm/. (I expect this fact (i.e., "common > variant") might remain true even after considering (2).) What I find > inconsistent though is that Ia32 and X64 get separate NASM files, > despite them sharing the implementation between each other as well. >=20 > IOW, this remark isn't about the actual implementation of the new API; > it's about consistency. If we decide for one ISA that the 32-bit and > 64-bit platforms use a common set of files, then the other ISA (also > with 32-bit and 64-bit platforms) should act similarly, if a common > implementation is possible. >=20 > Thanks > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel