From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::143; helo=mail-it1-x143.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x143.google.com (mail-it1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E6882208AE8F5 for ; Mon, 11 Feb 2019 09:32:32 -0800 (PST) Received: by mail-it1-x143.google.com with SMTP id c9so177905itj.1 for ; Mon, 11 Feb 2019 09:32:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=D/3RX8i20Ty1fKKcQErGt6wbnaHtCPT2x/PLevDy+FA=; b=Gc2AzyoS26fJqYhLMLkFiJg6UWNm99sbLtz/wsx7G4FM9i9o+tWPK7LoVLpZaonPQn 6htVW4/yeUp+1oS6YkqOTHGK0v1j7BizNgeSs0D6z82V1zT+1M15f6UoXxSwL7j0Bz/L 9/IcQA1UJSkNX5C8pIuUlIXD+6iD24YPd8ApaILf6EgtorsikpEHwAE3c0hNFEpo9C+2 S5QsZi4Wqjy1sEhzYdRbLc3GTcH3euHjuzF1ixTV1kddRdLS+lV50lcbRL0P1wsmdbXz oCT6sEEgWId8HrnoyiZsGL6ZdBkqqzHCcFJ5JuaPDkAZK8dyx9PZTDDQejSoQpbKHOa+ cizg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=D/3RX8i20Ty1fKKcQErGt6wbnaHtCPT2x/PLevDy+FA=; b=YHHV47XpJo0kLop6X67JH5Z9DGJuwS2iroWIOmuSRCshOAint47R/wK3YK4c9VqKUt HXcuajIoyLYlMAgCOnubft0uvCIgbgYCrU0Gl7tCGVm1zbl9r7ObYJJ2/vFWcvp6NysP 8kQJ6CMFR/ptwY4GBs/CBerHMO14uMWbR8SUNWs6YvBMy2d/p55IpfrlOq0r/6V6+g90 4YMp41OaHewtR6fXVhpxoUPb7wlhRfXQDElepy2jWTqi25JvaHb5nUahpgNDgIJ/Hflb wbg0fOPCvaY9i79aui9XcgdMsP7JwIAbh5FSULZ6GMzdxw4OtFN0WjPT4ao3gFi1kiMP VF6Q== X-Gm-Message-State: AHQUAubSZ/jaG/R1EW3Vk4sGtC8bBjPOST1j8yiGdqYTWQGp/KQYz+VL EpuFClAd0xf+Kel/TYTUj0fjEQicJdJ0PrtRqzeQng== X-Google-Smtp-Source: AHgI3IbIIzP+O9xFIhMaD7WsqdVCu0wTMNH7QBdjqzq/0AoSz15MhQK5+q4zkXr+DdGbhT+ECR/rfvYJtMNEyiF7KGo= X-Received: by 2002:a5d:8410:: with SMTP id i16mr19822241ion.173.1549906351875; Mon, 11 Feb 2019 09:32:31 -0800 (PST) MIME-Version: 1.0 References: <20190206000822.577-1-ard.biesheuvel@linaro.org> <20190211144144.qt2hdupd3mmdghqb@bivouac.eciton.net> In-Reply-To: <20190211144144.qt2hdupd3mmdghqb@bivouac.eciton.net> From: Ard Biesheuvel Date: Mon, 11 Feb 2019 18:32:19 +0100 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , "Gao, Liming" , "Kinney, Michael D" 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: Mon, 11 Feb 2019 17:32:33 -0000 Content-Type: text/plain; charset="UTF-8" On Mon, 11 Feb 2019 at 15:41, Leif Lindholm wrote: > > 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 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. > > +# > > +##------------------------------------------------------------------------------ > > + > > +.text > > +.p2align 2 > > + > > +GCC_ASM_EXPORT(SpeculationBarrier) > > + > > + > > +#/** > > +# 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 > > +# ); > > +# > > +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 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. > > +; > > +;------------------------------------------------------------------------------ > > + > > + EXPORT SpeculationBarrier > > + AREA BaseLib_LowLevel, CODE, READONLY > > + > > +;/** > > +; 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 > > +; ); > > +; > > +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 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. > > +# > > +##------------------------------------------------------------------------------ > > + > > +.text > > +.p2align 2 > > + > > +GCC_ASM_EXPORT(SpeculationBarrier) > > + > > + > > +#/** > > +# 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 > > +# ); > > +# > > +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 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. > > +; > > +;------------------------------------------------------------------------------ > > + > > + EXPORT SpeculationBarrier > > + > > + AREA MemoryBarriers, CODE, READONLY > > + > > +;/** > > +; 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 > > +; ); > > +; > > +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 > >