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::142; helo=mail-it1-x142.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x142.google.com (mail-it1-x142.google.com [IPv6:2607:f8b0:4864:20::142]) (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 45914211A43A8 for ; Fri, 14 Dec 2018 05:54:03 -0800 (PST) Received: by mail-it1-x142.google.com with SMTP id h65so9357893ith.3 for ; Fri, 14 Dec 2018 05:54:03 -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=ZLA+PiHvd21UBoZMO1PZPgFoulkB7kSuEBmChesjgw4=; b=Vl6DpbHt5IZwsdm5WG59tzrFSDsWNW7ckQt1YzRVrvWN+c2zXCT920r2ABQg79PGev AbQNouBmG7TNTFK3CjQaltuGAm2gy9vO1JyHF/X1Kl+TDTjgktFNwpj2MIvkmHdl+WWU BYBYpMukP5/Ys8jCv+EaMWPXDoe7A4lp5GwN8= 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=ZLA+PiHvd21UBoZMO1PZPgFoulkB7kSuEBmChesjgw4=; b=LtVrmnEEBR+b7yRkVkFpGzkL9O9hDfc2aLfgK1Btm4Mmzi/UbCG9UglL9WPgMz36aI baFEV8WZq0v2Jg+KiTplQuh1GpD2IkhlZ85ssoZFKqXdV9zm4qJe/Yj66R8/8kaF2aga YNk/HrTwMB2pghyOGOjGhVoSRCUu1qcA4UByDL+bKiAExkbazIQe0R6ROQldkeFWo0TD BI1Fua/BVRtOmjvDlzq4PD6Cyf7C5wf9W75legwSfsayxjO4SSAzYvKqhfo4uD3oToKc x2JAlfvHoWEFzuPsj8KHA0xN9CHMdUR8WFrr6W4Di7hOMiZTG5a3XL5ZsXN5ZyrFYXu8 jSTw== X-Gm-Message-State: AA+aEWZn54fS9h94VqsNkduqbUHD1uK1tisvc2FAne3+kjyKVEkh58e7 laeFmBF/nC1hraHszCa1l/oimk0H6wgi9hCi+KZjqA== X-Google-Smtp-Source: AFSGD/WvzmaYRK9bsyWIcJCjGt32Exht9qg9gqq4MGzVWkCaPYUmkbnCjp83G2hoA/kY/Tp3MK1SQ6p1l2ZnHE9i03U= X-Received: by 2002:a24:710:: with SMTP id f16mr2820596itf.121.1544795642220; Fri, 14 Dec 2018 05:54:02 -0800 (PST) MIME-Version: 1.0 References: <1544789607-11316-1-git-send-email-jagadeesh.ujja@arm.com> <1544789607-11316-6-git-send-email-jagadeesh.ujja@arm.com> In-Reply-To: <1544789607-11316-6-git-send-email-jagadeesh.ujja@arm.com> From: Ard Biesheuvel Date: Fri, 14 Dec 2018 14:53:51 +0100 Message-ID: To: Jagadeesh Ujja , Leif Lindholm Cc: "edk2-devel@lists.01.org" , "Gao, Liming" , "Zhang, Chao B" Subject: Re: [PATCH 05/13] MdePkg/Library/BaseLib/AArch64: Add AsmLfence function 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, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 X-List-Received-Date: Fri, 14 Dec 2018 13:54:03 -0000 Content-Type: text/plain; charset="UTF-8" On Fri, 14 Dec 2018 at 13:13, Jagadeesh Ujja wrote: > > Variable service driver includes a call to AsmLfence. To reuse this > driver on AArch64 based platforms, add an implementation of AsmLfence > that acts as a wrapper on the AArch64 specific MemoryFence function. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jagadeesh Ujja > --- > MdePkg/Include/Library/BaseLib.h | 33 +++++++++------ > MdePkg/Library/BaseLib/AArch64/AsmLfence.S | 42 ++++++++++++++++++++ > MdePkg/Library/BaseLib/AArch64/AsmLfence.asm | 41 +++++++++++++++++++ > MdePkg/Library/BaseLib/BaseLib.inf | 2 + > 4 files changed, 105 insertions(+), 13 deletions(-) > > diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h > index 8cc0869..ca961ee 100644 > --- a/MdePkg/Include/Library/BaseLib.h > +++ b/MdePkg/Include/Library/BaseLib.h > @@ -7697,19 +7697,6 @@ AsmWriteTr ( > ); > > /** > - Performs a serializing operation on all load-from-memory instructions that > - were issued prior the AsmLfence function. > - > - Executes a LFENCE instruction. This function is only available on IA-32 and x64. > - > -**/ > -VOID > -EFIAPI > -AsmLfence ( > - VOID > - ); > - > -/** > Patch the immediate operand of an IA32 or X64 instruction such that the byte, > word, dword or qword operand is encoded at the end of the instruction's > binary representation. > @@ -7752,4 +7739,24 @@ PatchInstructionX86 ( > ); > > #endif // defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) > + > +#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) || defined (MDE_CPU_AARCH64) > + > +/** > + Performs a serializing operation on all load-from-memory instructions that > + were issued prior the AsmLfence function. > + > + In case of IA-32 and x64, Executes a LFENCE instruction. > + > + In case of AArch64 this acts as a wrapper on the AArch64 > + specific MemoryFence function > + > +**/ > +VOID > +EFIAPI > +AsmLfence ( > + VOID > + ); > + > +#endif // defined (MDE_CPU_IA32) || defined (MDE_CPU_X64) || defined (MDE_CPU_AARCH64) > #endif // !defined (__BASE_LIB__) > diff --git a/MdePkg/Library/BaseLib/AArch64/AsmLfence.S b/MdePkg/Library/BaseLib/AArch64/AsmLfence.S > new file mode 100644 > index 0000000..2fd804b > --- /dev/null > +++ b/MdePkg/Library/BaseLib/AArch64/AsmLfence.S > @@ -0,0 +1,42 @@ > +##------------------------------------------------------------------------------ > +# > +# AsmLfence() for AArch64 > +# > +# Copyright (c) 2013-2018, ARM 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(AsmLfence) > + > +# IMPORT > +GCC_ASM_IMPORT(MemoryFence) > + > +#/** > +# Used to serialize load and store operations. > +# > +# All loads and stores that proceed calls to this function are guaranteed to be > +# globally visible when this function returns. > +# > +#**/ > +#VOID > +#EFIAPI > +#AsmLfence ( > +# VOID > +# ); > +# > +ASM_PFX(AsmLfence): > + stp x29, x30, [sp, #-16]! > + bl MemoryFence > + ldp x29, x30, [sp], #0x10 > + ret Any reason we can't simply do b MemoryFence here? Also, why I understand the rationale, I still think it would be better to change callers of the [x86 specific] AsmLfence() than to introduce an alias of MemoryFence() for architectures where Lfence is not defined. This is not only about tidiness, but also about potentially having different semantics, which we can't provide in general on ARM, but only in particular cases [such as the code that is modified in this series] In other words, newly introduced occurrences of AsmLfence() now have to be audited for being appropriate on AArc64 if they are added to generic code. > diff --git a/MdePkg/Library/BaseLib/AArch64/AsmLfence.asm b/MdePkg/Library/BaseLib/AArch64/AsmLfence.asm > new file mode 100644 > index 0000000..7dd5659 > --- /dev/null > +++ b/MdePkg/Library/BaseLib/AArch64/AsmLfence.asm > @@ -0,0 +1,41 @@ > +;------------------------------------------------------------------------------ > +; > +; AsmLfence() for AArch64 > +; > +; Copyright (c) 2013-2018, ARM 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 AsmLfence > + AREA BaseLib_LowLevel, CODE, READONLY > + # IMPORT > + GCC_ASM_IMPORT(MemoryFence) > + > +;/** > +; Used to serialize load and store operations. > +; > +; All loads and stores that proceed calls to this function are guaranteed to be > +; globally visible when this function returns. > +; > +;**/ > +;VOID > +;EFIAPI > +;AsmLfence ( > +; VOID > +; ); > +; > +AsmLfence > + stp x29, x30, [sp, #-16]! > + bl MemoryFence > + ldp x29, x30, [sp], #0x10 > + ret > + > + END > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf > index b84e583..b7d7bcb 100644 > --- a/MdePkg/Library/BaseLib/BaseLib.inf > +++ b/MdePkg/Library/BaseLib/BaseLib.inf > @@ -585,6 +585,7 @@ > Math64.c > > AArch64/MemoryFence.S | GCC > + AArch64/AsmLfence.S | GCC > AArch64/SwitchStack.S | GCC > AArch64/EnableInterrupts.S | GCC > AArch64/DisableInterrupts.S | GCC > @@ -593,6 +594,7 @@ > AArch64/CpuBreakpoint.S | GCC > > AArch64/MemoryFence.asm | MSFT > + AArch64/AsmLfence.asm | MSFT > AArch64/SwitchStack.asm | MSFT > AArch64/EnableInterrupts.asm | MSFT > AArch64/DisableInterrupts.asm | MSFT > -- > 2.7.4 >