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::d43; helo=mail-io1-xd43.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) (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 0333521A00AE6 for ; Tue, 27 Nov 2018 00:15:04 -0800 (PST) Received: by mail-io1-xd43.google.com with SMTP id t24so10522065ioi.0 for ; Tue, 27 Nov 2018 00:15:04 -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=QiuaRirCeOkQe9817kE4hLgITIoi6CXDdTZCa4mjPPg=; b=idF9leIw5mcOj0kIuJT50/nv5Z3Bc2EjLXu4w2BQQvmLH31GtavY/f6SXRhIiMRhfx Z2Bzs5wAuxO8vJsWInEvOY6kCieoObrsjK66ECke0Tek5/evakmvd/NL4aITARJGUG2y F7aHutGRsT9G8GNdPr1wTYLGKXyXp2mcgTE58= 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=QiuaRirCeOkQe9817kE4hLgITIoi6CXDdTZCa4mjPPg=; b=AmCZrY8lW2NZOavXnTx48OZfHGuMZ8co+UBSmXwqeWt9lArlTyeoZ4uv4S6qn3lUmI iNib4KtBV4oPyFMaOsO3PS3hdKI+OPgjo0iM/utUuG0VB9mBu/pTM7gwshuPKT1TSkVr +XQdrfCr6dJtZ+rB9v45txTxOTWsDP9ZjymotTKI7VC/dmSnlBnQbwoLQ8FoLbBOZPHu +OtL66ncyujKk1diftiGxaYDvjfTIQgiGbnmQuV6tdGkewAO1qIcV2zkfU/t/WZPvf7H QUjeXw/OEyC9MAQjOLyMfOVX526suftcfW1qkT9IVd6mdK5mYO7G2C12ajF7BFoir6AA OW6g== X-Gm-Message-State: AA+aEWbpvEC4T76jdhVj9v+LGlPIetDLrkggEbu9LJWceGsNzVOznVn2 TF9g0ZSqKIlBDUQoS9VCqPqrIjIYHic0xfSnsnaIhg== X-Google-Smtp-Source: AFSGD/WnKqBjlniYCJ0v9CUTp3c8DOQDZ2FL/Nfy727CDBSjlb7RBeqJVV1hADVnR2dRyc3vNpWP+GIO5e1tNZXqve8= X-Received: by 2002:a6b:7a46:: with SMTP id k6mr25167032iop.60.1543306504104; Tue, 27 Nov 2018 00:15:04 -0800 (PST) MIME-Version: 1.0 References: <1543299564-25266-1-git-send-email-sughosh.ganu@arm.com> <1543299564-25266-6-git-send-email-sughosh.ganu@arm.com> In-Reply-To: <1543299564-25266-6-git-send-email-sughosh.ganu@arm.com> From: Ard Biesheuvel Date: Tue, 27 Nov 2018 09:14:52 +0100 Message-ID: To: Sughosh Ganu Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Achin Gupta Subject: Re: [PATCH v4 5/5] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0. 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, 27 Nov 2018 08:15:05 -0000 Content-Type: text/plain; charset="UTF-8" On Tue, 27 Nov 2018 at 07:20, Sughosh Ganu wrote: > > From: Achin Gupta > > The Standalone MM environment runs in S-EL0 in AArch64 on ARM Standard > Platforms. Privileged firmware e.g. ARM Trusted Firmware sets up its > architectural context including the initial translation tables for the > S-EL1/EL0 translation regime. The MM environment will still request ARM > TF to change the memory attributes of memory regions during > initialization. > > The Standalone MM image is a FV that encapsulates the MM foundation > and drivers. These are PE-COFF images with data and text segments. > To initialise the MM environment, Arm Trusted Firmware has to create > translation tables with sane default attributes for the memory > occupied by the FV. This library sends SVCs to ARM Trusted Firmware > to request memory permissions change for data and text segments. > > This patch adds a simple MMU library suitable for execution in S-EL0 and > requesting memory permissions change operations from Arm Trusted Firmware. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Sughosh Ganu > --- > ArmPkg/ArmPkg.dec | 1 + > ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/XenArmGenericTimerVirtCounterLib.inf => ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf | 23 +-- > ArmPkg/Include/Library/{ArmMmuLib.h => StandaloneMmMmuLib.h} | 38 +--- > ArmPkg/Library/StandaloneMmMmuLib/Aarch64/ArmMmuStandaloneMmLib.c | 185 ++++++++++++++++++++ > 4 files changed, 203 insertions(+), 44 deletions(-) > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec > index 0db7aa9d301c..d99eb6769ff1 100644 > --- a/ArmPkg/ArmPkg.dec > +++ b/ArmPkg/ArmPkg.dec > @@ -42,6 +42,7 @@ [LibraryClasses.common] > ArmMtlLib|ArmPlatformPkg/Include/Library/ArmMtlLib.h > ArmSvcLib|Include/Library/ArmSvcLib.h > OpteeLib|Include/Library/OpteeLib.h > + StandaloneMmMmuLib|Include/Library/StandaloneMmMmuLib.h > > [Guids.common] > gArmTokenSpaceGuid = { 0xBB11ECFE, 0x820F, 0x4968, { 0xBB, 0xA6, 0xF7, 0x6A, 0xFE, 0x30, 0x25, 0x96 } } > diff --git a/ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/XenArmGenericTimerVirtCounterLib.inf b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf > similarity index 56% > copy from ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/XenArmGenericTimerVirtCounterLib.inf > copy to ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf > index bd6ac8039844..d589b236033c 100644 > --- a/ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/XenArmGenericTimerVirtCounterLib.inf > +++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf > @@ -1,7 +1,6 @@ > #/** @file > -# Implement ArmGenericTimerCounterLib for Xen using the virtual timer > # > -# Copyright (c) 2014 - 2018, Linaro Ltd. All rights reserved.
> +# Copyright (c) 2017 - 2018, ARM Limited. All rights reserved. > # > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD License > @@ -15,19 +14,23 @@ > > [Defines] > INF_VERSION = 0x0001001A > - BASE_NAME = XenArmGenericTimerVirtCounterLib > - FILE_GUID = e3913319-96ac-4ac0-808b-8edb8776a51c > - MODULE_TYPE = BASE > + BASE_NAME = ArmMmuStandaloneMmCoreLib > + FILE_GUID = da8f0232-fb14-42f0-922c-63104d2c70bd > + MODULE_TYPE = MM_CORE_STANDALONE > VERSION_STRING = 1.0 > - LIBRARY_CLASS = ArmGenericTimerCounterLib > + LIBRARY_CLASS = StandaloneMmMmuLib > + PI_SPECIFICATION_VERSION = 0x00010032 > > -[Sources] > - XenArmGenericTimerVirtCounterLib.c > +[Sources.AARCH64] > + Aarch64/ArmMmuStandaloneMmLib.c > > [Packages] > - MdePkg/MdePkg.dec > ArmPkg/ArmPkg.dec > + MdePkg/MdePkg.dec > > [LibraryClasses] > ArmLib > - BaseLib > + CacheMaintenanceLib > + MemoryAllocationLib > + > + > diff --git a/ArmPkg/Include/Library/ArmMmuLib.h b/ArmPkg/Include/Library/StandaloneMmMmuLib.h > similarity index 55% > copy from ArmPkg/Include/Library/ArmMmuLib.h > copy to ArmPkg/Include/Library/StandaloneMmMmuLib.h > index fb7fd006417c..1f7653d00430 100644 > --- a/ArmPkg/Include/Library/ArmMmuLib.h > +++ b/ArmPkg/Include/Library/StandaloneMmMmuLib.h > @@ -1,6 +1,6 @@ > /** @file > > - Copyright (c) 2015 - 2016, Linaro Ltd. All rights reserved.
> + Copyright (c) 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 > @@ -12,61 +12,31 @@ > > **/ > > -#ifndef __ARM_MMU_LIB__ > -#define __ARM_MMU_LIB__ > - > -#include > - > -#include > - > -EFI_STATUS > -EFIAPI > -ArmConfigureMmu ( > - IN ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable, > - OUT VOID **TranslationTableBase OPTIONAL, > - OUT UINTN *TranslationTableSize OPTIONAL > - ); > +#ifndef __STANDALONEMM_MMU_LIB__ > +#define __STANDALONEMM_MMU_LIB__ > > EFI_STATUS > -EFIAPI > ArmSetMemoryRegionNoExec ( > IN EFI_PHYSICAL_ADDRESS BaseAddress, > IN UINT64 Length > ); > > EFI_STATUS > -EFIAPI > ArmClearMemoryRegionNoExec ( > IN EFI_PHYSICAL_ADDRESS BaseAddress, > IN UINT64 Length > ); > > EFI_STATUS > -EFIAPI > ArmSetMemoryRegionReadOnly ( > IN EFI_PHYSICAL_ADDRESS BaseAddress, > IN UINT64 Length > ); > > EFI_STATUS > -EFIAPI > ArmClearMemoryRegionReadOnly ( > IN EFI_PHYSICAL_ADDRESS BaseAddress, > IN UINT64 Length > ); > > -VOID > -EFIAPI > -ArmReplaceLiveTranslationEntry ( > - IN UINT64 *Entry, > - IN UINT64 Value > - ); > - > -EFI_STATUS > -ArmSetMemoryAttributes ( > - IN EFI_PHYSICAL_ADDRESS BaseAddress, > - IN UINT64 Length, > - IN UINT64 Attributes > - ); > - > -#endif > +#endif /* __STANDALONEMM_MMU_LIB__ */ > diff --git a/ArmPkg/Library/StandaloneMmMmuLib/Aarch64/ArmMmuStandaloneMmLib.c b/ArmPkg/Library/StandaloneMmMmuLib/Aarch64/ArmMmuStandaloneMmLib.c > new file mode 100644 > index 000000000000..d7d87b7d5d69 > --- /dev/null > +++ b/ArmPkg/Library/StandaloneMmMmuLib/Aarch64/ArmMmuStandaloneMmLib.c > @@ -0,0 +1,185 @@ > +/** @file > +* File managing the MMU for ARMv8 architecture in S-EL0 > +* > +* Copyright (c) 2017 - 2018, ARM Limited. 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. > +* > +**/ > + > +#include > +#include Why do you need this include? If you can drop it, can you also make the library generic (i.e., supporting ARM as well as AArch64) ? (apologies for not spotting this before) > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +STATIC > +EFI_STATUS > +GetMemoryPermissions ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + OUT UINT32 *MemoryAttributes > + ) > +{ > + ARM_SVC_ARGS GetMemoryPermissionsSvcArgs = {0}; > + > + GetMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64; > + GetMemoryPermissionsSvcArgs.Arg1 = BaseAddress; > + GetMemoryPermissionsSvcArgs.Arg2 = 0; > + GetMemoryPermissionsSvcArgs.Arg3 = 0; > + > + ArmCallSvc (&GetMemoryPermissionsSvcArgs); > + if (GetMemoryPermissionsSvcArgs.Arg0 == ARM_SVC_SPM_RET_INVALID_PARAMS) { > + *MemoryAttributes = 0; > + return EFI_INVALID_PARAMETER; > + } > + > + *MemoryAttributes = GetMemoryPermissionsSvcArgs.Arg0; > + return EFI_SUCCESS; > +} > + > +STATIC > +EFI_STATUS > +RequestMemoryPermissionChange ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINTN Permissions > + ) > +{ > + EFI_STATUS Status; > + ARM_SVC_ARGS ChangeMemoryPermissionsSvcArgs = {0}; > + > + ChangeMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64; > + ChangeMemoryPermissionsSvcArgs.Arg1 = BaseAddress; > + ChangeMemoryPermissionsSvcArgs.Arg2 = EFI_SIZE_TO_PAGES(Length); > + ChangeMemoryPermissionsSvcArgs.Arg3 = Permissions; > + > + ArmCallSvc (&ChangeMemoryPermissionsSvcArgs); > + > + Status = ChangeMemoryPermissionsSvcArgs.Arg0; > + > + switch (Status) { > + case ARM_SVC_SPM_RET_SUCCESS: > + Status = EFI_SUCCESS; > + break; > + > + case ARM_SVC_SPM_RET_NOT_SUPPORTED: > + Status = EFI_UNSUPPORTED; > + break; > + > + case ARM_SVC_SPM_RET_INVALID_PARAMS: > + Status = EFI_INVALID_PARAMETER; > + break; > + > + case ARM_SVC_SPM_RET_DENIED: > + Status = EFI_ACCESS_DENIED; > + break; > + > + case ARM_SVC_SPM_RET_NO_MEMORY: > + Status = EFI_BAD_BUFFER_SIZE; > + break; > + > + default: > + Status = EFI_ACCESS_DENIED; > + ASSERT (0); > + } > + > + return Status; > +} > + > +EFI_STATUS > +ArmSetMemoryRegionNoExec ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + EFI_STATUS Status; > + UINT32 MemoryAttributes; > + UINT32 CodePermission; > + > + Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); > + if (Status != EFI_INVALID_PARAMETER) { > + CodePermission = SET_MEM_ATTR_CODE_PERM_XN << SET_MEM_ATTR_CODE_PERM_SHIFT; > + return RequestMemoryPermissionChange ( > + BaseAddress, > + Length, > + MemoryAttributes | CodePermission > + ); > + } > + return EFI_INVALID_PARAMETER; > +} > + > +EFI_STATUS > +ArmClearMemoryRegionNoExec ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + EFI_STATUS Status; > + UINT32 MemoryAttributes; > + UINT32 CodePermission; > + > + Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); > + if (Status != EFI_INVALID_PARAMETER) { > + CodePermission = SET_MEM_ATTR_CODE_PERM_XN << SET_MEM_ATTR_CODE_PERM_SHIFT; > + return RequestMemoryPermissionChange ( > + BaseAddress, > + Length, > + MemoryAttributes & ~CodePermission > + ); > + } > + return EFI_INVALID_PARAMETER; > +} > + > +EFI_STATUS > +ArmSetMemoryRegionReadOnly ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + EFI_STATUS Status; > + UINT32 MemoryAttributes; > + UINT32 DataPermission; > + > + Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); > + if (Status != EFI_INVALID_PARAMETER) { > + DataPermission = SET_MEM_ATTR_DATA_PERM_RO << SET_MEM_ATTR_DATA_PERM_SHIFT; > + return RequestMemoryPermissionChange ( > + BaseAddress, > + Length, > + MemoryAttributes | DataPermission > + ); > + } > + return EFI_INVALID_PARAMETER; > +} > + > +EFI_STATUS > +ArmClearMemoryRegionReadOnly ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + EFI_STATUS Status; > + UINT32 MemoryAttributes; > + UINT32 PermissionRequest; > + > + Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); > + if (Status != EFI_INVALID_PARAMETER) { > + PermissionRequest = SET_MEM_ATTR_MAKE_PERM_REQUEST (SET_MEM_ATTR_DATA_PERM_RW, MemoryAttributes); > + return RequestMemoryPermissionChange ( > + BaseAddress, > + Length, > + PermissionRequest > + ); > + } > + return EFI_INVALID_PARAMETER; > +} > -- > 2.7.4 >