From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=sughosh.ganu@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id 8F4D5210F75BF for ; Mon, 20 Aug 2018 23:50:53 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 71457ED1; Mon, 20 Aug 2018 23:50:52 -0700 (PDT) Received: from a074948-lin.blr.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 235813F2EA; Mon, 20 Aug 2018 23:50:49 -0700 (PDT) Received: by a074948-lin.blr.arm.com (sSMTP sendmail emulation); Tue, 21 Aug 2018 12:20:47 +0530 Date: Tue, 21 Aug 2018 12:20:47 +0530 From: Sughosh Ganu To: ard.biesheuvel@linaro.org, leif.lindholm@linaro.org Cc: supreeth.venkatesh@arm.com, edk2-devel@lists.01.org Message-ID: <20180821065047.GA17216@arm.com> References: <1532090300-5250-1-git-send-email-sughosh.ganu@arm.com> <1532090300-5250-8-git-send-email-sughosh.ganu@arm.com> <1532367194.3302.36.camel@arm.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [PATCH v2 7/7] ArmPkg: Extra action to update permissions for S-ELO MM Image 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, 21 Aug 2018 06:50:53 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline hi Ard, On Tue July 23, 2018 at 11:03PM +0530, Supreeth Venkatesh wrote: > > On Sat, 2018-07-21 at 20:06 +0900, Ard Biesheuvel wrote: > > On 20 July 2018 at 21:38, Sughosh Ganu wrote: > > > > > > From: Achin Gupta > > > > > > The Standalone MM drivers runs in S-EL0 in AArch64 on ARM Standard > > > Platforms and is deployed during SEC phase. The memory allocated to > > > the Standalone MM drivers should be marked as RO+X. > > > > > > During PE/COFF Image section parsing, this patch implements extra > > > action "UpdatePeCoffPermissions" to request the privileged firmware > > > in > > > EL3 to update the permissions. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Sughosh Ganu > > Apologies for bringing this up only now, but I don't think I was ever > > cc'ed on these patches. > > > Apologies if you have missed it. But I am pretty sure it was part of > earlier large patch-set on which you and leif were copied, as it was > part of ArmPkg. > > > > We are relying on a debug hook in the PE/COFF loader to ensure that > > we > > don't end up with memory that is both writable and executable in the > > secure world. Do we really think that is a good idea? > > > > (I know this code was derived from a proof of concept that I did > > years > > ago, but that was just a PoC) > I think we need a little bit more details on what is your suggestion? > > A little bit background here: This code runs in S-EL0 and Request gets > sent to secure world SPM to ensure that the region permissions are > updated correctly via the "ArmMmuStandaloneMmCoreLib" SVC - > ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64. > > DebugPeCoffExtraActionLib is just used to extract image region > information, but the region permission > update request is sent to secure world for validation. > > With the above explanation, can you provide an insight into what was > your thinking? > Do you want us to create a separate library and call it > as PeCoffExtraActionLib to avoid the "Debug" word though it is a hook > to PeCoffExtraActionLib in MdePkg or do we want to create this library > in a separate package (may be in MdePkg?) or something totally > different. Supreeth had replied to your comments on the patch. Can you please check this. If you feel that this needs to be implemented differently, can you please suggest it to us. Thanks. -sughosh