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.65; helo=mga03.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (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 CAE78201B0400 for ; Mon, 11 Feb 2019 16:26:40 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Feb 2019 16:26:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,360,1544515200"; d="scan'208";a="114157456" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga007.jf.intel.com with ESMTP; 11 Feb 2019 16:26:39 -0800 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 11 Feb 2019 16:26:39 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 11 Feb 2019 16:26:39 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.102]) by shsmsx102.ccr.corp.intel.com ([169.254.2.207]) with mapi id 14.03.0415.000; Tue, 12 Feb 2019 08:26:37 +0800 From: "Gao, Liming" To: Ard Biesheuvel , Leif Lindholm CC: "Kinney, Michael D" , "edk2-devel@lists.01.org" Thread-Topic: [edk2] [PATCH] MdePkg/BaseLib: implement SpeculationBarrier() for ARM and AArch64 Thread-Index: AQHUvbAdFtk/4Bn2IEGhsDUn5RO+yKXaLlQAgAAvqYCAAPmd8A== Date: Tue, 12 Feb 2019 00:26:36 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E3DB3BF@SHSMSX104.ccr.corp.intel.com> References: <20190206000822.577-1-ard.biesheuvel@linaro.org> <20190211144144.qt2hdupd3mmdghqb@bivouac.eciton.net> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdePkg/BaseLib: implement SpeculationBarrier() for ARM and AArch64 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: Tue, 12 Feb 2019 00:26:41 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ard: I agree your comments not to add PCD until there is the real problem.=20 Thanks Liming >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard >Biesheuvel >Sent: Tuesday, February 12, 2019 1:32 AM >To: Leif Lindholm >Cc: Kinney, Michael D ; edk2- >devel@lists.01.org; Gao, Liming >Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: implement SpeculationBarrier() >for ARM and AArch64 > >On Mon, 11 Feb 2019 at 15:41, Leif Lindholm wro= te: >> >> On Wed, Feb 06, 2019 at 12:08:22AM +0000, Ard Biesheuvel wrote: >> > Replace the dummy C implementation of SpeculationBarrier() with >> > implementations consisting of the recommended DSB SY + ISB sequence, >> > as recommended by ARM in the whitepaper "Cache Speculation Side- >channels" >> > version 2.4, dated October 2018. >> > >> > Contributed-under: TianoCore Contribution Agreement 1.1 >> > Signed-off-by: Ard Biesheuvel >> >> Patch looks fine. >> Reviewed-by: Leif Lindholm >> >> Question: do we expect performance impact to be sufficient to >> motivate a Pcd to be able to disable the barrier on unaffected >> processors? >> > >Currently, these are only used on some codepaths in the MM component >of the variable store, which do not look like hot paths to me. > >In general, I think it should be fine to defer doing something like >this until someone highlights it as an actual problem (and has the >numbers to prove it) > > >> > --- >> > MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.S | 39 >++++++++++++++++++++ >> > MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.asm | 38 >+++++++++++++++++++ >> > MdePkg/Library/BaseLib/Arm/SpeculationBarrier.S | 39 >++++++++++++++++++++ >> > MdePkg/Library/BaseLib/Arm/SpeculationBarrier.asm | 39 >++++++++++++++++++++ >> > MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c | 30 ----------= ----- >> > MdePkg/Library/BaseLib/BaseLib.inf | 7 +++- >> > 6 files changed, 160 insertions(+), 32 deletions(-) >> > >> > diff --git a/MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.S >b/MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.S >> > new file mode 100644 >> > index 000000000000..500bdadca5d2 >> > --- /dev/null >> > +++ b/MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.S >> > @@ -0,0 +1,39 @@ >> > +##-------------------------------------------------------------------= ----------- >> > +# >> > +# SpeculationBarrier() for AArch64 >> > +# >> > +# Copyright (c) 2019, Linaro Ltd. All rights reserved. >> > +# >> > +# This program and the accompanying materials >> > +# are licensed and made available under the terms and conditions of t= he >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. >> > +# >> > +##-------------------------------------------------------------------= ----------- >> > + >> > +.text >> > +.p2align 2 >> > + >> > +GCC_ASM_EXPORT(SpeculationBarrier) >> > + >> > + >> > +#/** >> > +# Uses as a barrier to stop speculative execution. >> > +# >> > +# Ensures that no later instruction will execute speculatively, unti= l all >prior >> > +# instructions have completed. >> > +# >> > +#**/ >> > +#VOID >> > +#EFIAPI >> > +#SpeculationBarrier ( >> > +# VOID >> > +# ); >> > +# >> > +ASM_PFX(SpeculationBarrier): >> > + dsb sy >> > + isb >> > + ret >> > diff --git a/MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.asm >b/MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.asm >> > new file mode 100644 >> > index 000000000000..0c4b915b7798 >> > --- /dev/null >> > +++ b/MdePkg/Library/BaseLib/AArch64/SpeculationBarrier.asm >> > @@ -0,0 +1,38 @@ >> > +;--------------------------------------------------------------------= ---------- >> > +; >> > +; SpeculationBarrier() for AArch64 >> > +; >> > +; Copyright (c) 2019, Linaro Ltd. All rights reserved. >> > +; >> > +; This program and the accompanying materials >> > +; are licensed and made available under the terms and conditions of t= he >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. >> > +; >> > +;--------------------------------------------------------------------= ---------- >> > + >> > + EXPORT SpeculationBarrier >> > + AREA BaseLib_LowLevel, CODE, READONLY >> > + >> > +;/** >> > +; Uses as a barrier to stop speculative execution. >> > +; >> > +; Ensures that no later instruction will execute speculatively, unti= l all prior >> > +; instructions have completed. >> > +; >> > +;**/ >> > +;VOID >> > +;EFIAPI >> > +;SpeculationBarrier ( >> > +; VOID >> > +; ); >> > +; >> > +SpeculationBarrier >> > + dsb sy >> > + isb >> > + ret >> > + >> > + END >> > diff --git a/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.S >b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.S >> > new file mode 100644 >> > index 000000000000..7857558aba17 >> > --- /dev/null >> > +++ b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.S >> > @@ -0,0 +1,39 @@ >> > +##-------------------------------------------------------------------= ----------- >> > +# >> > +# SpeculationBarrier() for AArch64 >> > +# >> > +# Copyright (c) 2019, Linaro Ltd. All rights reserved. >> > +# >> > +# This program and the accompanying materials >> > +# are licensed and made available under the terms and conditions of t= he >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. >> > +# >> > +##-------------------------------------------------------------------= ----------- >> > + >> > +.text >> > +.p2align 2 >> > + >> > +GCC_ASM_EXPORT(SpeculationBarrier) >> > + >> > + >> > +#/** >> > +# Uses as a barrier to stop speculative execution. >> > +# >> > +# Ensures that no later instruction will execute speculatively, unti= l all >prior >> > +# instructions have completed. >> > +# >> > +#**/ >> > +#VOID >> > +#EFIAPI >> > +#SpeculationBarrier ( >> > +# VOID >> > +# ); >> > +# >> > +ASM_PFX(SpeculationBarrier): >> > + dsb >> > + isb >> > + bx lr >> > diff --git a/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.asm >b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.asm >> > new file mode 100644 >> > index 000000000000..425cd3de9e22 >> > --- /dev/null >> > +++ b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.asm >> > @@ -0,0 +1,39 @@ >> > +;--------------------------------------------------------------------= ---------- >> > +; >> > +; SpeculationBarrier() for AArch64 >> > +; >> > +; Copyright (c) 2019, Linaro Ltd. All rights reserved. >> > +; >> > +; This program and the accompanying materials >> > +; are licensed and made available under the terms and conditions of t= he >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. >> > +; >> > +;--------------------------------------------------------------------= ---------- >> > + >> > + EXPORT SpeculationBarrier >> > + >> > + AREA MemoryBarriers, CODE, READONLY >> > + >> > +;/** >> > +; Uses as a barrier to stop speculative execution. >> > +; >> > +; Ensures that no later instruction will execute speculatively, unti= l all prior >> > +; instructions have completed. >> > +; >> > +;**/ >> > +;VOID >> > +;EFIAPI >> > +;SpeculationBarrier ( >> > +; VOID >> > +; ); >> > +; >> > +SpeculationBarrier >> > + dsb >> > + isb >> > + bx lr >> > + >> > + END >> > diff --git a/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c >b/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c >> > deleted file mode 100644 >> > index 8a6165a10227..000000000000 >> > --- a/MdePkg/Library/BaseLib/Arm/SpeculationBarrier.c >> > +++ /dev/null >> > @@ -1,30 +0,0 @@ >> > -/** @file >> > - SpeculationBarrier() 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. >> > - >> > -**/ >> > - >> > - >> > -/** >> > - Uses as a barrier to stop speculative execution. >> > - >> > - Ensures that no later instruction will execute speculatively, until= all prior >> > - instructions have completed. >> > - >> > -**/ >> > -VOID >> > -EFIAPI >> > -SpeculationBarrier ( >> > - VOID >> > - ) >> > -{ >> > -} >> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf >b/MdePkg/Library/BaseLib/BaseLib.inf >> > index d195c5417b2e..f25a067a2335 100644 >> > --- a/MdePkg/Library/BaseLib/BaseLib.inf >> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf >> > @@ -552,7 +552,6 @@ >> > [Sources.ARM] >> > Arm/InternalSwitchStack.c >> > Arm/Unaligned.c >> > - Arm/SpeculationBarrier.c >> > Math64.c | RVCT >> > Math64.c | MSFT >> > >> > @@ -564,6 +563,7 @@ >> > Arm/CpuPause.asm | RVCT >> > Arm/CpuBreakpoint.asm | RVCT >> > Arm/MemoryFence.asm | RVCT >> > + Arm/SpeculationBarrier.S | RVCT >> > >> > Arm/SwitchStack.asm | MSFT >> > Arm/SetJumpLongJump.asm | MSFT >> > @@ -573,6 +573,7 @@ >> > Arm/CpuPause.asm | MSFT >> > Arm/CpuBreakpoint.asm | MSFT >> > Arm/MemoryFence.asm | MSFT >> > + Arm/SpeculationBarrier.asm | MSFT >> > >> > Arm/Math64.S | GCC >> > Arm/SwitchStack.S | GCC >> > @@ -582,11 +583,11 @@ >> > Arm/SetJumpLongJump.S | GCC >> > Arm/CpuBreakpoint.S | GCC >> > Arm/MemoryFence.S | GCC >> > + Arm/SpeculationBarrier.S | GCC >> > >> > [Sources.AARCH64] >> > Arm/InternalSwitchStack.c >> > Arm/Unaligned.c >> > - Arm/SpeculationBarrier.c >> > Math64.c >> > >> > AArch64/MemoryFence.S | GCC >> > @@ -596,6 +597,7 @@ >> > AArch64/GetInterruptsState.S | GCC >> > AArch64/SetJumpLongJump.S | GCC >> > AArch64/CpuBreakpoint.S | GCC >> > + AArch64/SpeculationBarrier.S | GCC >> > >> > AArch64/MemoryFence.asm | MSFT >> > AArch64/SwitchStack.asm | MSFT >> > @@ -604,6 +606,7 @@ >> > AArch64/GetInterruptsState.asm | MSFT >> > AArch64/SetJumpLongJump.asm | MSFT >> > AArch64/CpuBreakpoint.asm | MSFT >> > + AArch64/SpeculationBarrier.asm | MSFT >> > >> > [Packages] >> > MdePkg/MdePkg.dec >> > -- >> > 2.17.1 >> > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org >https://lists.01.org/mailman/listinfo/edk2-devel