Thanks Laszlo. That is very good feedback. For ARM, I think we need use *CSDB*. :-)
Thank you Yao Jiewen > -----Original Message----- > From: Wu, Hao A > Sent: Friday, September 21, 2018 10:15 AM > To: Laszlo Ersek <ler...@redhat.com>; edk2-devel@lists.01.org > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Yao, Jiewen > <jiewen....@intel.com>; Gao, Liming <liming....@intel.com> > Subject: RE: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Laszlo > > 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 > > > > On 09/20/18 08:40, Hao Wu wrote: > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1193 > > > > > > This commit will add a new BaseLib API LoadFence(). This API will > perform > > > 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 > within > > > SMM. More details can be referred at the 'Bounds check bypass > mitigation' > > > section at the below link: > > > > > > https://software.intel.com/security-software-guidance/insights/host- > > firmware-speculative-execution-side-channel-mitigation > > > > > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > > > Cc: Laszlo Ersek <ler...@redhat.com> > > > Cc: Jiewen Yao <jiewen....@intel.com> > > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > > Cc: Liming Gao <liming....@intel.com> > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Hao Wu <hao.a...@intel.com> > > > --- > > > 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 > instructions 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 > LongJump() > > > 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.<BR> > > > + > > > + 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 > instructions 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.<BR> > > > + Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> > > > 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 > > > @@ -52,6 +52,19 @@ MemoryFence ( > > > } > > > > > > /** > > > + Performs a serializing operation on all load-from-memory > instructions 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.<BR> > > > +; 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. > > > +; > > > +; Module Name: > > > +; > > > +; LoadFence.nasm > > > +; > > > +; Abstract: > > > +; > > > +; Performs a serializing operation on all load-from-memory > instructions > > 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.<BR> > > > +; 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. > > > +; > > > +; Module Name: > > > +; > > > +; LoadFence.nasm > > > +; > > > +; Abstract: > > > +; > > > +; Performs a serializing operation on all load-from-memory > instructions > > 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 > > > + > > > > > > > Comments in no particular order: > > > > (1) I think the EBC stub implementation should go into a separate file > > under "MdePkg/Library/BaseLib/Ebc". > > Yes, I will do this in later version of the series. > > > > > (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. > > > > (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. > > Thanks for the comments. > I will start a new discussion to loop in Ard and Leif to have a discussion > on this. > > Best Regards, > Hao Wu > > > > > 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. > > > > Thanks > > Laszlo > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
