subject:"\[edk2\] \[PATCH v1 06\/18\] StandaloneMmPkg\: Add an AArch64 specific entry point library."

Re: [edk2] [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library.

2018-05-04 Thread Supreeth Venkatesh

My response inline.

-Original Message-
From: Achin Gupta
Sent: Monday, April 16, 2018 9:04 AM
To: Supreeth Venkatesh 
Cc: edk2-devel@lists.01.org; michael.d.kin...@intel.com; liming@intel.com; 
jiewen@intel.com; leif.lindh...@linaro.org; ard.biesheu...@linaro.org; nd 

Subject: Re: [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry 
point library.

Hi Supreeth,

On Fri, Apr 06, 2018 at 03:42:11PM +0100, Supreeth Venkatesh wrote:
> The Standalone MM environment runs in S-EL0 in AArch64 on ARM Standard
> Platforms and is initialised during the SEC phase. ARM Trusted firmware
> in EL3 is responsible for initialising the architectural context for
> S-EL0 and loading the Standalone MM image. The memory allocated to this
> image is marked as RO+X. Heap memory is marked as RW+XN.
>
> Certain actions have to be completed prior to executing the generic code
> in the Standalone MM Core module. These are:
>
> 1. Memory permission attributes for each section of the Standalone MM
>Core module need to be changed prior to accessing any RW data.
>
> 2. A Hob list has to be created with information that allows the MM
>environment to initialise and dispatch drivers.
>
> Furthermore, this module is responsible for handing over runtime MM
> events to the Standalone MM CPU driver and returning control to ARM
> Trusted Firmware upon event completion. Hence it needs to know the CPU
> driver entry point.
>
> This patch implements an entry point module that ARM Trusted Firmware
> jumps to in S-EL0. It then performs the above actions before calling the
> Standalone MM Foundation entry point and handling subsequent MM events.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Library/Arm/StandaloneMmCoreEntryPoint.h   | 232 +
>  .../Include/Library/MmCoreStandaloneEntryPoint.h   | 101 
>  .../StandaloneMmCoreEntryPoint/Arm/CreateHobList.c | 200 +++
>  .../Arm/SetPermissions.c   | 278 
> +
>  .../Arm/StandaloneMmCoreEntryPoint.c   | 264 +++
>  .../StandaloneMmCoreEntryPoint.inf |  53 
>  StandaloneMmPkg => StandaloneMmPkg~HEAD|   0
>  7 files changed, 1128 insertions(+)
>  create mode 100644 
> StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h

The names of this file and the one below are re-arrangements of the same
words. This is confusing. It would be better to stick to one convention. Since
the name of the package starts with StandaloneMm, everything else should follow
suit.

So can this file be renamed to ArmStandaloneMmCoreEntryPoint.h and the one below
to StandaloneMmCoreEntryPoint.h unless you think this can be done differently?
[Supreeth] Ok. I have renamed these as " StandaloneMmCoreEntryPoint.h" and 
"AArch64/ StandaloneMmCoreEntryPoint.h".

>  create mode 100644 
> StandaloneMmPkg/Include/Library/MmCoreStandaloneEntryPoint.h



>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c

PeCoffExtraActionLib and the HobLib are pre-requisites for these two
files. Should they not appear first in the patch stack?
[Supreeth] Hoblib - yes.
PeCoffExtraActionLib - No, as it has no compilation dependencies. Of course, it 
will fail at runtime, but PeCoffExtraActionLib has a reverse dependency
on StandaloneMmPkg PCD. Anyway, since this is being sent as a series. I hope it 
will be checked in together.

>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf

Can this and the tagged file above go in a separate commit? Cannot see how they
are related to the Arm specific entry point.
[Supreeth] I will move the commit of 
.../Include/Library/StandaloneMmCoreEntryPoint.h  to the "core" commit id. Rest 
of them are AArch64 specific entry points.

>  rename StandaloneMmPkg => StandaloneMmPkg~HEAD (100%)
>
> diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
> b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> new file mode 100644
> index 00..029c6c476c
> --- /dev/null
> +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> @@ -0,0 +1,232 @@
> +/** @file
> +  Entry point to the Standalone MM Foundation when initialised during the SEC
> +  phase on ARM platforms
> +
> +Copyright (c) 2017, ARM Ltd. All rights reserved.

Copyright year needs to be updated.
[Supreeth] Ok.

> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD 
> License
> +which

Re: [edk2] [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library.

2018-04-16 Thread Achin Gupta

Hi Supreeth,

On Fri, Apr 06, 2018 at 03:42:11PM +0100, Supreeth Venkatesh wrote:
> The Standalone MM environment runs in S-EL0 in AArch64 on ARM Standard
> Platforms and is initialised during the SEC phase. ARM Trusted firmware
> in EL3 is responsible for initialising the architectural context for
> S-EL0 and loading the Standalone MM image. The memory allocated to this
> image is marked as RO+X. Heap memory is marked as RW+XN.
> 
> Certain actions have to be completed prior to executing the generic code
> in the Standalone MM Core module. These are:
> 
> 1. Memory permission attributes for each section of the Standalone MM
>Core module need to be changed prior to accessing any RW data.
> 
> 2. A Hob list has to be created with information that allows the MM
>environment to initialise and dispatch drivers.
> 
> Furthermore, this module is responsible for handing over runtime MM
> events to the Standalone MM CPU driver and returning control to ARM
> Trusted Firmware upon event completion. Hence it needs to know the CPU
> driver entry point.
> 
> This patch implements an entry point module that ARM Trusted Firmware
> jumps to in S-EL0. It then performs the above actions before calling the
> Standalone MM Foundation entry point and handling subsequent MM events.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Library/Arm/StandaloneMmCoreEntryPoint.h   | 232 +
>  .../Include/Library/MmCoreStandaloneEntryPoint.h   | 101 
>  .../StandaloneMmCoreEntryPoint/Arm/CreateHobList.c | 200 +++
>  .../Arm/SetPermissions.c   | 278 
> +
>  .../Arm/StandaloneMmCoreEntryPoint.c   | 264 +++
>  .../StandaloneMmCoreEntryPoint.inf |  53 
>  StandaloneMmPkg => StandaloneMmPkg~HEAD|   0
>  7 files changed, 1128 insertions(+)
>  create mode 100644 
> StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h

The names of this file and the one below are re-arrangements of the same
words. This is confusing. It would be better to stick to one convention. Since
the name of the package starts with StandaloneMm, everything else should follow
suit.

So can this file be renamed to ArmStandaloneMmCoreEntryPoint.h and the one below
to StandaloneMmCoreEntryPoint.h unless you think this can be done differently?

>  create mode 100644 
> StandaloneMmPkg/Include/Library/MmCoreStandaloneEntryPoint.h



>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c

PeCoffExtraActionLib and the HobLib are pre-requisites for these two
files. Should they not appear first in the patch stack?

>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf

Can this and the tagged file above go in a separate commit? Cannot see how they
are related to the Arm specific entry point.

>  rename StandaloneMmPkg => StandaloneMmPkg~HEAD (100%)
> 
> diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
> b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> new file mode 100644
> index 00..029c6c476c
> --- /dev/null
> +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> @@ -0,0 +1,232 @@
> +/** @file
> +  Entry point to the Standalone MM Foundation when initialised during the SEC
> +  phase on ARM platforms
> +
> +Copyright (c) 2017, ARM Ltd. All rights reserved.

Copyright year needs to be updated.

> +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.
> +
> +**/
> +
> +#ifndef __MODULE_ENTRY_POINT_H__
> +#define __MODULE_ENTRY_POINT_H__

Name of this define needs to be changed

> +
> +#include 
> +#include 
> +
> +#define CPU_INFO_FLAG_PRIMARY_CPU  0x0001
> +
> +typedef
> +EFI_STATUS
> +(*PI_MM_CPU_TP_FW_ENTRYPOINT) (
> +  IN UINTN EventId,
> +  IN UINTN CpuNumber,
> +  IN UINTN NsCommBufferAddr
> +  );

This typedef is not being used.

> +
> +typedef struct {
> +  UINT8  Type;   /* type of the structure */
> +  UINT8  Version;/* version of this structure */
> +  UINT16 Size;  /* size of this structure in bytes */
> +  UINT32 Attr;  /* attributes: unused bits SBZ */
> +} EFI_PARAM_HEADER;
> +
> +typedef struct {
> +  UINT64

[edk2] [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library.

2018-04-06 Thread Supreeth Venkatesh

The Standalone MM environment runs in S-EL0 in AArch64 on ARM Standard
Platforms and is initialised during the SEC phase. ARM Trusted firmware
in EL3 is responsible for initialising the architectural context for
S-EL0 and loading the Standalone MM image. The memory allocated to this
image is marked as RO+X. Heap memory is marked as RW+XN.

Certain actions have to be completed prior to executing the generic code
in the Standalone MM Core module. These are:

1. Memory permission attributes for each section of the Standalone MM
   Core module need to be changed prior to accessing any RW data.

2. A Hob list has to be created with information that allows the MM
   environment to initialise and dispatch drivers.

Furthermore, this module is responsible for handing over runtime MM
events to the Standalone MM CPU driver and returning control to ARM
Trusted Firmware upon event completion. Hence it needs to know the CPU
driver entry point.

This patch implements an entry point module that ARM Trusted Firmware
jumps to in S-EL0. It then performs the above actions before calling the
Standalone MM Foundation entry point and handling subsequent MM events.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Supreeth Venkatesh 
---
 .../Library/Arm/StandaloneMmCoreEntryPoint.h   | 232 +
 .../Include/Library/MmCoreStandaloneEntryPoint.h   | 101 
 .../StandaloneMmCoreEntryPoint/Arm/CreateHobList.c | 200 +++
 .../Arm/SetPermissions.c   | 278 +
 .../Arm/StandaloneMmCoreEntryPoint.c   | 264 +++
 .../StandaloneMmCoreEntryPoint.inf |  53 
 StandaloneMmPkg => StandaloneMmPkg~HEAD|   0
 7 files changed, 1128 insertions(+)
 create mode 100644 
StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
 create mode 100644 StandaloneMmPkg/Include/Library/MmCoreStandaloneEntryPoint.h
 create mode 100644 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
 create mode 100644 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
 create mode 100644 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 create mode 100644 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 rename StandaloneMmPkg => StandaloneMmPkg~HEAD (100%)

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
new file mode 100644
index 00..029c6c476c
--- /dev/null
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -0,0 +1,232 @@
+/** @file
+  Entry point to the Standalone MM Foundation when initialised during the SEC
+  phase on ARM platforms
+
+Copyright (c) 2017, 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.
+
+**/
+
+#ifndef __MODULE_ENTRY_POINT_H__
+#define __MODULE_ENTRY_POINT_H__
+
+#include 
+#include 
+
+#define CPU_INFO_FLAG_PRIMARY_CPU  0x0001
+
+typedef
+EFI_STATUS
+(*PI_MM_CPU_TP_FW_ENTRYPOINT) (
+  IN UINTN EventId,
+  IN UINTN CpuNumber,
+  IN UINTN NsCommBufferAddr
+  );
+
+typedef struct {
+  UINT8  Type;   /* type of the structure */
+  UINT8  Version;/* version of this structure */
+  UINT16 Size;  /* size of this structure in bytes */
+  UINT32 Attr;  /* attributes: unused bits SBZ */
+} EFI_PARAM_HEADER;
+
+typedef struct {
+  UINT64 Mpidr;
+  UINT32 LinearId;
+  UINT32 Flags;
+} EFI_SECURE_PARTITION_CPU_INFO;
+
+typedef struct {
+  EFI_PARAM_HEADER  Header;
+  UINT64SpMemBase;
+  UINT64SpMemLimit;
+  UINT64SpImageBase;
+  UINT64SpStackBase;
+  UINT64SpHeapBase;
+  UINT64SpNsCommBufBase;
+  UINT64SpSharedBufBase;
+  UINT64SpImageSize;
+  UINT64SpPcpuStackSize;
+  UINT64SpHeapSize;
+  UINT64SpNsCommBufSize;
+  UINT64SpPcpuSharedBufSize;
+  UINT32NumSpMemRegions;
+  UINT32NumCpus;
+  EFI_SECURE_PARTITION_CPU_INFO *CpuInfo;
+} EFI_SECURE_PARTITION_BOOT_INFO;
+
+typedef struct {
+  EFI_PARAM_HEADER   h;
+  UINT64 SpStackBase;
+  UINT64 SpSharedBufBase;
+  UINT32

Re: [edk2] [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library.

Re: [edk2] [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library.

[edk2] [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library.

3 matches

Site Navigation

Mail list logo

Footer information