From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=104.47.2.60; helo=eur01-db5-obe.outbound.protection.outlook.com; envelope-from=supreeth.venkatesh@arm.com; receiver=edk2-devel@lists.01.org Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0060.outbound.protection.outlook.com [104.47.2.60]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2C0C7203BEA3A for ; Fri, 4 May 2018 16:19:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=hzPGBTmdrMrR+E28byMKXbgChsv15xda8t0ja+Uaoj8=; b=re7p+YRpTEY1oUCN2q9EVIIaFo9wlJNbF82SBIjpeBxiyLyTMn/iAoBXM0CdoI9NM/+hs6DeNXRYFcexB/SUCRnTHQ8ynyJ4tOAwW+n+w+lFSooUpg0RD5GeNwkmxv5Xrb11cpU+Z8KsifbLDqoIiAlbVOCG9e/F2wIsZsUYJwg= Received: from AM4PR0802MB2306.eurprd08.prod.outlook.com (10.172.218.15) by AM4PR0802MB2196.eurprd08.prod.outlook.com (10.172.217.22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.735.16; Fri, 4 May 2018 23:19:32 +0000 Received: from AM4PR0802MB2306.eurprd08.prod.outlook.com ([fe80::e117:6f62:6a9b:6be4]) by AM4PR0802MB2306.eurprd08.prod.outlook.com ([fe80::e117:6f62:6a9b:6be4%8]) with mapi id 15.20.0735.016; Fri, 4 May 2018 23:19:32 +0000 From: Supreeth Venkatesh To: Achin Gupta CC: "edk2-devel@lists.01.org" Thread-Topic: [PATCH v1 04/18] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0. Thread-Index: AQHTzbWR6ipq49YY/0Kamv1HKNkkiaP7+UaAgCKrL9A= Date: Fri, 4 May 2018 23:19:31 +0000 Message-ID: References: <20180406144223.10931-1-supreeth.venkatesh@arm.com> <20180406144223.10931-5-supreeth.venkatesh@arm.com> <20180411192153.GH663@e104320-lin> In-Reply-To: <20180411192153.GH663@e104320-lin> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Supreeth.Venkatesh@arm.com; x-originating-ip: [217.140.111.135] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM4PR0802MB2196; 7:n9kFObPlhAI8ztch83KoE+cMpkKPP/HhAbNPDbGGJou7kj4Zo9zip3yQZ8pLT2T8Txg0SRnftrnZYnwtPFl1uKpGgok/WKPH89KJOKcmjStDQpjr5nF0n4G3SIp2MqhzTMWYWrk3eDt2pHwmmT1703n2ocFjJFZovht4nnfoi3v6njfMBUtko/+ubQvl6BsEKxDkkMZetLQ7Jg5wILPYHqGhTpD+nTma5DJrLIfIoNHzriAOmZqgTpcdE3SJj/wt x-ms-exchange-antispam-srfa-diagnostics: SOS;SOR; x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(2017052603328)(7153060)(7193020); SRVR:AM4PR0802MB2196; x-ms-traffictypediagnostic: AM4PR0802MB2196: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917)(162533806227266)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(3231254)(944501410)(52105095)(10201501046)(93006095)(93001095)(3002001)(6055026)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123560045)(20161123562045)(20161123558120)(6072148)(201708071742011); SRVR:AM4PR0802MB2196; BCL:0; PCL:0; RULEID:; SRVR:AM4PR0802MB2196; x-forefront-prvs: 06628F7CA4 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(979002)(39860400002)(346002)(39380400002)(396003)(376002)(366004)(13464003)(199004)(189003)(40434004)(105586002)(446003)(8936002)(106356001)(4326008)(11346002)(476003)(486006)(966005)(72206003)(5660300001)(6116002)(478600001)(305945005)(7736002)(3846002)(25786009)(81156014)(81166006)(8676002)(66066001)(14454004)(97736004)(316002)(5250100002)(3280700002)(99286004)(6246003)(86362001)(3660700001)(74316002)(6306002)(9686003)(55016002)(6436002)(229853002)(102836004)(6862004)(2906002)(6636002)(6506007)(68736007)(59450400001)(53546011)(15188155005)(16799955002)(53376002)(33656002)(26005)(2900100001)(76176011)(7696005)(53936002)(5890100001)(19627235001)(969003)(989001)(999001)(1009001)(1019001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR0802MB2196; H:AM4PR0802MB2306.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: EzSNJWhg3Wm2XN79JFdNzeMOS5plJVpZX7xbevUMWkW17MJkdg42RF8N6lj5uPK31nhUJ+c190Ss9woxf8VFUVh039ooSSPDRCAhx49gvZZi7Fc9QkUgwQPLUEH8gFHNRwTk+I5wu1lOFIq1PaiuWzkKNVzvyYhmcGfaa/dKG5jRhLnu0/L5MyPfrCU892H7 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 1b6901bc-eb4a-44f5-303a-08d5b2157e04 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1b6901bc-eb4a-44f5-303a-08d5b2157e04 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 May 2018 23:19:31.9804 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0802MB2196 Subject: Re: [PATCH v1 04/18] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 May 2018 23:19:34 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable My response inline. -----Original Message----- From: Achin Gupta Sent: Wednesday, April 11, 2018 2:22 PM To: Supreeth Venkatesh Cc: edk2-devel@lists.01.org; michael.d.kinney@intel.com; liming.gao@intel.c= om; jiewen.yao@intel.com; leif.lindholm@linaro.org; ard.biesheuvel@linaro.o= rg; nd Subject: Re: [PATCH v1 04/18] ArmPkg/ArmMmuLib: Add MMU Library suitable fo= r use in S-EL0. Hi Supreeth, On Fri, Apr 06, 2018 at 03:42:09PM +0100, Supreeth Venkatesh wrote: > 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 could still request > ARM TF to change the memory attributes of memory regions during > initialization. The commit message needs more detail to better flesh out why we are doing w= hat we are doing here i.e. the StandaloneMm image is a FV that encapsulates= the MM foundation and drivers. These are PE-COFF images with data and text= segments. Arm TF does not have visibility of the contents of the FV. Moreo= ver, the driver images are relocated upon dispatch. However, to initialise = the MM environment, Arm TF has to create translation tables with sane defau= lt attributes for the memory occupied by the FV............ I am hoping you can extrapolate from here and clearly describe what problem= this library solves. [Supreeth] Ok. > > This patch adds a simple MMU library suitable for execution in S-EL0 > and requesting operations from higher exception levels. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Achin Gupta > Signed-off-by: Supreeth Venkatesh > --- > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuSecLib.c | 146 > ++++++++++++++++++++++++ > 1 file changed, 146 insertions(+) > create mode 100644 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuSecLib.c > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuSecLib.c > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuSecLib.c I am not sure about the name of the library. ArmMmuSecLib sounds like an MM= U library for the SEC phase in the Normal world. Can we call it ArmMmuSecSt= andaloneMmLib or similar. [Supreeth] I have renamed it as ArmMmuStandaloneMmCoreLib. Please see versi= on 2. > new file mode 100644 > index 0000000000..56969e31d1 > --- /dev/null > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuSecLib.c > @@ -0,0 +1,146 @@ > +/** @file > +* File managing the MMU for ARMv8 architecture in S-EL0 > +* > +* Copyright (c) 2017, ARM Limited. All rights reserved. Nit: Copyright 2018? For this and other files? > +* > +* 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 > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +EFI_STATUS > +RequestMemoryPermissionChange( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINTN Permissions > + ) > +{ > + EFI_STATUS Status; > + ARM_SVC_ARGS ChangeMemoryPermissionsSvcArgs =3D {0}; > + > + ChangeMemoryPermissionsSvcArgs.Arg0 =3D > + ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64; > + ChangeMemoryPermissionsSvcArgs.Arg1 =3D BaseAddress; > + ChangeMemoryPermissionsSvcArgs.Arg2 =3D (Length >=3D EFI_PAGE_SIZE) ? = \ > + Length >> EFI_PAGE_SHIFT : > + 1; > + ChangeMemoryPermissionsSvcArgs.Arg3 =3D Permissions; > + > + ArmCallSvc(&ChangeMemoryPermissionsSvcArgs); > + > + Status =3D ChangeMemoryPermissionsSvcArgs.Arg0; > + > + switch (Status) { > + case ARM_SVC_SPM_RET_SUCCESS: > + Status =3D EFI_SUCCESS; > + break; > + > + case ARM_SVC_SPM_RET_NOT_SUPPORTED: > + Status =3D EFI_UNSUPPORTED; > + break; > + > + case ARM_SVC_SPM_RET_INVALID_PARAMS: > + Status =3D EFI_INVALID_PARAMETER; > + break; > + > + case ARM_SVC_SPM_RET_DENIED: > + Status =3D EFI_ACCESS_DENIED; > + break; > + > + case ARM_SVC_SPM_RET_NO_MEMORY: > + Status =3D EFI_BAD_BUFFER_SIZE; > + break; > + > + default: > + Status =3D EFI_ACCESS_DENIED; > + ASSERT (0); > + } > + > + return Status; > +} > + > +EFI_STATUS > +ArmSetMemoryRegionNoExec ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + return RequestMemoryPermissionChange(BaseAddress, > + Length, > + SET_MEM_ATTR_MAKE_PERM_REQUEST( \ > + SET_MEM_ATTR_DATA_PERM_RO, \ > + SET_MEM_ATTR_CODE_PERM_XN)); > +} > + > +EFI_STATUS > +ArmClearMemoryRegionNoExec ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + return RequestMemoryPermissionChange(BaseAddress, > + Length, > + SET_MEM_ATTR_MAKE_PERM_REQUEST( \ > + SET_MEM_ATTR_DATA_PERM_RO, \ > + SET_MEM_ATTR_CODE_PERM_X)); > +} > + > +EFI_STATUS > +ArmSetMemoryRegionReadOnly ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + return RequestMemoryPermissionChange(BaseAddress, > + Length, > + SET_MEM_ATTR_MAKE_PERM_REQUEST( \ > + SET_MEM_ATTR_DATA_PERM_RO, \ > + SET_MEM_ATTR_CODE_PERM_XN)); > +} > + > +EFI_STATUS > +ArmClearMemoryRegionReadOnly ( > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length > + ) > +{ > + return RequestMemoryPermissionChange(BaseAddress, > + Length, > + SET_MEM_ATTR_MAKE_PERM_REQUEST( \ > + SET_MEM_ATTR_DATA_PERM_RW, \ > + SET_MEM_ATTR_CODE_PERM_XN)); > +} The above four functions were written as prototypes in the edk2-staging bra= nch. I do not think they are adequate for upstreaming since each function m= akes assumptions about the current data or instruction access permission of= the input memory region instead of only doing what the function's name sug= gests. For example, ArmSetMemoryRegionNoExec() is supposed to only set the XN bit.= However, it also sets the data access permission to RO. If the region was = RW then this will lead to incorrect behaviour. Ditto for the other function= s. We need a GetMemoryPermission() function that first uses the ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64 call to obtain the memory attribut= es of the input region. Each of the above functions must use this new funct= ion and only change the data or instruction permission attribute as appropr= iate. [Supreeth] Ok. Please see version 2. cheers, Achin > + > +EFI_STATUS > +EFIAPI > +ArmConfigureMmu ( > + IN ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable, > + OUT VOID **TranslationTableBase OPTIONAL, > + OUT UINTN *TranslationTableSize OPTIONAL > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +EFI_STATUS > +EFIAPI > +ArmMmuSecLibConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *MmSystemTable > + ) > +{ > + return EFI_SUCCESS; > +} > -- > 2.16.2 > IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in = any medium. Thank you.