Re: [edk2] [PATCH v2 04/17] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0.

2018-05-10 Thread Supreeth Venkatesh
On Wed, 2018-05-09 at 21:09 +, Udit Kumar wrote:
> Hi Supreeth, 
> One question on this patch 
> We are asking permission on base-address and changing the permission
> of 
> memory based upon base and size. 
> I haven't looked at other part of code which manage this , 
> But will there be possibility that, base address is given correctly
> and length 
> may over-lap the other MMU entry.
> 

Please refer to https://github.com/ARM-software/arm-trusted-firmware/bl
ob/master/docs/secure-partition-manager-design.rst#communication-
initiated-by-secure-partition for "SP_MEMORY_ATTRIBUTES_GET_AARCH64"
Syntax.

For the time being, SPM in arm-tf does a translation table walk to find
the block or page descriptor that maps the address. It checks whether
the table entry is part of the S-EL0 ( MM) space.

Also, SPM defines resources/address block mappings at boot time for MM
image.  

However, this interface is being modified/updated in secure partition
runtime interface (spec) which will address your concern.
We will revisit this, once this gets implemented in arm-tf.


> Regards
> Udit 
> 
> > 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > Of
> > Supreeth Venkatesh
> > Sent: Saturday, May 5, 2018 2:11 AM
> > To: edk2-devel@lists.01.org
> > Cc: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; jiewen.yao
> > @intel.com;
> > liming@intel.com; michael.d.kin...@intel.com
> > Subject: [edk2] [PATCH v2 04/17] ArmPkg/ArmMmuLib: Add MMU Library
> > suitable for use in S-EL0.
> > 
> > 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: Achin Gupta <achin.gu...@arm.com>
> > Signed-off-by: Supreeth Venkatesh <supreeth.venkat...@arm.com>
> > ---
> >  .../ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c  | 195
> > +
> >  1 file changed, 195 insertions(+)
> >  create mode 100644
> > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> > 
> > diff --git
> > a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> > new file mode 100644
> > index 00..0f5e68d2d4
> > --- /dev/null
> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> > @@ -0,0 +1,195 @@
> > +/** @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
> > +*
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fo
> > penso
> > urce.org%2Flicenses%2Fbsd-
> > license.php=02%7C01%7Cudit.kumar%40nxp.com%7C776b728240f7402b
> > 029708d5b1ff7179%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> > 6610633113917388=KGmnTNpIKqIXyS2sdVH1I2EaCd8rhm%2BKI05JuxYv8
> > Aw%3D=0
> > +*
> > +*  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 
>

Re: [edk2] [PATCH v2 04/17] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0.

2018-05-09 Thread Udit Kumar
Hi Supreeth, 
One question on this patch 
We are asking permission on base-address and changing the permission of 
memory based upon base and size. 
I haven't looked at other part of code which manage this , 
But will there be possibility that, base address is given correctly and length 
may over-lap the other MMU entry.

Regards
Udit 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Supreeth Venkatesh
> Sent: Saturday, May 5, 2018 2:11 AM
> To: edk2-devel@lists.01.org
> Cc: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; jiewen@intel.com;
> liming@intel.com; michael.d.kin...@intel.com
> Subject: [edk2] [PATCH v2 04/17] ArmPkg/ArmMmuLib: Add MMU Library
> suitable for use in S-EL0.
> 
> 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: Achin Gupta <achin.gu...@arm.com>
> Signed-off-by: Supreeth Venkatesh <supreeth.venkat...@arm.com>
> ---
>  .../ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c  | 195
> +
>  1 file changed, 195 insertions(+)
>  create mode 100644
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> 
> diff --git
> a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> new file mode 100644
> index 00..0f5e68d2d4
> --- /dev/null
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
> @@ -0,0 +1,195 @@
> +/** @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
> +*
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopenso
> urce.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cudit.kumar%40nxp.com%7C776b728240f7402b
> 029708d5b1ff7179%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 6610633113917388=KGmnTNpIKqIXyS2sdVH1I2EaCd8rhm%2BKI05JuxYv8
> Aw%3D=0
> +*
> +*  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
> +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 ();
> +  if (GetMemoryPermissionsSvcArgs.Arg0 ==
> ARM_SVC_SPM_RET_INVALID_PARAMS) {
> +*MemoryAttributes = 0;
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *MemoryAttributes = GetMemoryPermissionsSvcArgs.Arg0;
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +RequestMemoryPermissionChange (
> +  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
> +  IN  UINT64Length,
> +  IN  UINTN Permissions
> +  )
> +{
> +  EFI_STATUSStatus;
> +  ARM_SVC_ARGS  ChangeMemoryPermissionsSvcArgs = {0};
> +
> +  ChangeMemoryPermissionsSvcArgs.Arg0 =
> ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
> +  ChangeMemoryPermissionsSvcArgs.Arg1 = BaseAddress;
> +  ChangeMemoryPermissionsSvcArgs.Arg2 = (Length >= EFI_PAGE_SIZE) ? \
> +  

[edk2] [PATCH v2 04/17] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0.

2018-05-04 Thread Supreeth Venkatesh
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: Achin Gupta 
Signed-off-by: Supreeth Venkatesh 
---
 .../ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c  | 195 +
 1 file changed, 195 insertions(+)
 create mode 100644 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
new file mode 100644
index 00..0f5e68d2d4
--- /dev/null
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuStandaloneMmCoreLib.c
@@ -0,0 +1,195 @@
+/** @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 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+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 ();
+  if (GetMemoryPermissionsSvcArgs.Arg0 == ARM_SVC_SPM_RET_INVALID_PARAMS) {
+*MemoryAttributes = 0;
+return EFI_INVALID_PARAMETER;
+  }
+
+  *MemoryAttributes = GetMemoryPermissionsSvcArgs.Arg0;
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+RequestMemoryPermissionChange (
+  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINT64Length,
+  IN  UINTN Permissions
+  )
+{
+  EFI_STATUSStatus;
+  ARM_SVC_ARGS  ChangeMemoryPermissionsSvcArgs = {0};
+
+  ChangeMemoryPermissionsSvcArgs.Arg0 = 
ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
+  ChangeMemoryPermissionsSvcArgs.Arg1 = BaseAddress;
+  ChangeMemoryPermissionsSvcArgs.Arg2 = (Length >= EFI_PAGE_SIZE) ? \
+ Length >> EFI_PAGE_SHIFT : 1;
+  ChangeMemoryPermissionsSvcArgs.Arg3 = Permissions;
+
+  ArmCallSvc ();
+
+  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  UINT64Length
+  )
+{
+  EFI_STATUSStatus;
+  UINT32 MemoryAttributes;
+
+  Status = GetMemoryPermissions (BaseAddress, );
+  if (Status != EFI_INVALID_PARAMETER) {
+return RequestMemoryPermissionChange (BaseAddress,
+  Length,
+  MemoryAttributes |
+  (SET_MEM_ATTR_CODE_PERM_XN << 
SET_MEM_ATTR_CODE_PERM_SHIFT));
+  }
+  return EFI_INVALID_PARAMETER;
+}
+
+EFI_STATUS
+ArmClearMemoryRegionNoExec (
+  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINT64Length
+  )
+{
+  EFI_STATUSStatus;
+  UINT32 MemoryAttributes;
+
+  Status = GetMemoryPermissions (BaseAddress, );
+  if (Status != EFI_INVALID_PARAMETER) {
+return