[PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[Sami Mujawar]

This patch series adds support to build PRM for AARCH64 using GCC.

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/2238_aarch64_prm_support_v1

Jose Marinho (5):
  Basetools/GenFw: Allow AARCH64 builds to use the --prm flag
  PrmPkg: Enable external visibility on PRM symbols
  PrmPkg: Build Prm Samples with GCC for AARCH64
  PrmPkg: Support AArch64 builds using GCC
  PrmPkg: Add details on AArch64 build to the Readme.

 BaseTools/Source/C/GenFw/Elf64Convert.c                                                  |  6 ++---
 PrmPkg/Include/Prm.h                                                                     |  3 +++
 PrmPkg/PrmPkg.ci.yaml                                                                    |  1 +
 PrmPkg/PrmPkg.dsc                                                                        | 24 +++++++++++++++++---
 PrmPkg/Readme.md                                                                         | 11 +++++++++
 PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf |  4 ++++
 PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf             |  4 ++++
 7 files changed, 47 insertions(+), 6 deletions(-)

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

[PATCH v1 1/5] Basetools/GenFw: Allow AARCH64 builds to use the --prm flag

[Sami Mujawar]

From: Jose Marinho <jose.marinho@arm.com>

The GenFw invocation with the --prm flag was previously reserved for
X64.
AArch64 platforms, built with GCC5, can also deploy PRM modules, hence
the --prm flag is also applicable in builds targeting the AARCH64
architecture.

This commit enables the --prm flag to be used for EDK2 builds targeting
AARCH64.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 2aa9bfcc9460c0a68208808e036fc631ae14f90e..35e96dd05bc26f7212d0f80af1ecb998bcb993dd 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -2,7 +2,7 @@
 Elf64 convert solution
 
 Copyright (c) 2010 - 2021, Intel Corporation. All rights reserved.<BR>
-Portions copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
+Portions copyright (c) 2013-2022, ARM Ltd. All rights reserved.<BR>
 Portions Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -186,8 +186,8 @@ InitializeElf64 (
   }
 
   if (mExportFlag) {
-    if (mEhdr->e_machine != EM_X86_64) {
-      Error (NULL, 0, 3000, "Unsupported", "--prm option currently only supports X64 arch.");
+    if ((mEhdr->e_machine != EM_X86_64) && (mEhdr->e_machine != EM_AARCH64)) {
+      Error (NULL, 0, 3000, "Unsupported", "--prm option currently only supports X64 and AArch64 archs.");
       return FALSE;
     }
   }
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

[PATCH v1 2/5] PrmPkg: Enable external visibility on PRM symbols

[Sami Mujawar]

From: Jose Marinho <jose.marinho@arm.com>

Enable GCC compilations to keep external symbols when generating a PRM
module.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 PrmPkg/Include/Prm.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/PrmPkg/Include/Prm.h b/PrmPkg/Include/Prm.h
index e290e53510f05908000935eaa486cabd7856b645..007f73416aac84139782b7cdd1125a6ba85aef9d 100644
--- a/PrmPkg/Include/Prm.h
+++ b/PrmPkg/Include/Prm.h
@@ -3,6 +3,7 @@
   Common Platform Runtime Mechanism (PRM) definitions.
 
   Copyright (c) Microsoft Corporation
+  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -15,6 +16,8 @@
 
 #if defined (_MSC_VER)
 #define PRM_EXPORT_API  __declspec(dllexport)
+#elif defined (__GNUC__)
+#define PRM_EXPORT_API  __attribute__ ((visibility ("default")))
 #else
 #define PRM_EXPORT_API
 #endif
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

[PATCH v1 3/5] PrmPkg: Build Prm Samples with GCC for AARCH64

[Sami Mujawar]

From: Jose Marinho <jose.marinho@arm.com>

- Add the --prm flag to the GENFW_FLAGS
- Add the --no-gc-section to the linker flags so that apparently
unreferenced symbols are not prematurely removed from the .dll which
is used to generate the Prm module .efi.
- Force the linker to maintain the PrmModuleExportDescriptor symbol.
- Force the linker to maintain the PRM handler funtion's symbol.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf | 4 ++++
 PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf             | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf b/PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf
index e8b71a970d83d44485430c2e3dc24e1a80da75c8..04addecc861e643cbb2be07e995708b42059719c 100644
--- a/PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf
+++ b/PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf
@@ -7,6 +7,7 @@
 #  code at OS runtime.
 #
 #  Copyright (c) Microsoft Corporation
+#  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -39,3 +40,6 @@ [Depex]
 [BuildOptions.common]
   MSFT:*_*_*_DLINK_FLAGS  = /DLL /SUBSYSTEM:CONSOLE /VERSION:1.0
   MSFT:*_*_*_GENFW_FLAGS = --keepoptionalheader
+
+  GCC:*_*_AARCH64_GENFW_FLAGS = --prm
+  GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--no-gc-sections -Wl,--require-defined=PrmModuleExportDescriptor -Wl,--require-defined=CheckParamBufferPrmHandler
diff --git a/PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf b/PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf
index b97af923c47a70b280d270302aa8a7d058701c92..ff72a233669657c260f284cd2fbc4126077d4426 100644
--- a/PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf
+++ b/PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf
@@ -7,6 +7,7 @@
 #
 #  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
 #  Copyright (c) Microsoft Corporation
+#  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -42,3 +43,6 @@ [Depex]
 [BuildOptions.common]
   MSFT:*_*_*_DLINK_FLAGS  = /DLL /SUBSYSTEM:CONSOLE /VERSION:1.0
   MSFT:*_*_*_GENFW_FLAGS = --keepoptionalheader
+
+  GCC:*_*_AARCH64_GENFW_FLAGS = --keepoptionalheader --prm
+  GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--no-gc-sections -Wl,--require-defined=PrmModuleExportDescriptor -Wl,--require-defined=CheckStaticDataBufferPrmHandler
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

[PATCH v1 4/5] PrmPkg: Support AArch64 builds using GCC

[Sami Mujawar]

From: Jose Marinho <jose.marinho@arm.com>

Add support to build PrmPkg for AArch64 using the GCC compiler.

Add AARCH64 architecture to the list of supported architectures.
Add BaseStackCheck library to allow for Prm module builds on AARCH64.

Also update the CI to add dependency on ArmPkg.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 PrmPkg/PrmPkg.ci.yaml |  1 +
 PrmPkg/PrmPkg.dsc     | 24 +++++++++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/PrmPkg/PrmPkg.ci.yaml b/PrmPkg/PrmPkg.ci.yaml
index babaf2214ec86ff04531ae216d957be8d3487387..2d4d12815d6b43d9ee34469bb85c113375a89381 100644
--- a/PrmPkg/PrmPkg.ci.yaml
+++ b/PrmPkg/PrmPkg.ci.yaml
@@ -46,6 +46,7 @@
     ## options defined .pytool/Plugin/DependencyCheck
     "DependencyCheck": {
         "AcceptableDependencies": [
+            "ArmPkg/ArmPkg.dec",
             "MdeModulePkg/MdeModulePkg.dec",
             "MdePkg/MdePkg.dec",
             "PrmPkg/PrmPkg.dec",
diff --git a/PrmPkg/PrmPkg.dsc b/PrmPkg/PrmPkg.dsc
index d10aa3d547ce954cba951c63008ecc51ec24b548..3fb7490acaff91bf0ba0ea407b27630642f00e1e 100644
--- a/PrmPkg/PrmPkg.dsc
+++ b/PrmPkg/PrmPkg.dsc
@@ -2,6 +2,7 @@
 # Build description file for PrmPkg
 #
 # Copyright (C) Microsoft Corporation
+# Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 
@@ -11,7 +12,7 @@ [Defines]
   PLATFORM_VERSION               = 0.1
   DSC_SPECIFICATION              = 0x00010005
   OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)
-  SUPPORTED_ARCHITECTURES        = IA32|X64
+  SUPPORTED_ARCHITECTURES        = IA32|X64|AARCH64
   BUILD_TARGETS                  = DEBUG|RELEASE|NOOPT
   SKUID_IDENTIFIER               = DEFAULT
 
@@ -28,7 +29,6 @@ [LibraryClasses.common]
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
   DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
   MemoryAllocationLib|MdeModulePkg/Library/BaseMemoryAllocationLibNull/BaseMemoryAllocationLibNull.inf
-  MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
@@ -38,6 +38,13 @@ [LibraryClasses.common]
   UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
   UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
 
+[LibraryClasses.IA32, LibraryClasses.X64]
+  MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
+
+[LibraryClasses.AARCH64]
+  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
+  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+
 [LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_APPLICATION]
   #
   # EDK II Packages
@@ -129,7 +136,6 @@ [Components]
   # PRM Sample Modules
   #
   $(PLATFORM_PACKAGE)/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf
-  $(PLATFORM_PACKAGE)/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
   $(PLATFORM_PACKAGE)/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf
 
   #
@@ -137,6 +143,18 @@ [Components]
   #
   $(PLATFORM_PACKAGE)/Application/PrmInfo/PrmInfo.inf
 
+[Components.IA32, Components.X64]
+  #
+  # PRM Sample Modules for IA32 and X64
+  #
+  $(PLATFORM_PACKAGE)/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
+
+[Components.AARCH64]
+  ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
+
+  # Add support for GCC stack protector
+  MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+
 [BuildOptions]
 # Force deprecated interfaces off
   *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

[PATCH v1 5/5] PrmPkg: Add details on AArch64 build to the Readme.

[Sami Mujawar]

From: Jose Marinho <jose.marinho@arm.com>

Specify how to build the PrmPkg for the AArch64 architecture.
Make the 2 following notes:
 - the PrmPkg has only been tested on AArch64 using the GCC5
toolchain.
 - All symbols to be listed in the PRMT as well as the
PrmModuleExportDescriptor must be explicitly preserved by resorting to
the --require-defined linker flag.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
 PrmPkg/Readme.md | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/PrmPkg/Readme.md b/PrmPkg/Readme.md
index 4ff8a36123d5f987fdd8b54ebaa87e605fa0b4ea..3191c039fd641307979f0c79c69157dbdebe7eab 100644
--- a/PrmPkg/Readme.md
+++ b/PrmPkg/Readme.md
@@ -52,6 +52,11 @@ To build `PrmPkg` as a standalone package:
      * ``. edksetup.sh``
 
 5. Build PrmPkg \
+
+The PrmPkg can be built targetting the IA32/X64 and AArch64 architectures.
+
+ - IA32/X64
+
    ``build -p PrmPkg/PrmPkg.dsc -a IA32 -a X64``
    > ***Note***: Due to the way PRM modules are compiled with exports, **only building on Visual Studio compiler tool
    chains has been tested**.
@@ -63,6 +68,12 @@ To build `PrmPkg` as a standalone package:
 > You can add your own PRM modules into the build and check them with the `PrmInfo` UEFI application described
 > later in this document and dump the PRMT table in the OS to check if your PRM module is represented as expected.
 
+ - AArch64
+    ``build -p PrmPkg/PrmPkg.dsc -a AARCH64 -t GCC5``
+
+   > ***Note***: Only builds with the GCC5 toolchain have been tested.
+   > ***Note***: For builds with the GCC5 toolchain, the PrmModuleExportDescriptor and any other handler entry points symbols, to be listed in the PRMT, must be explicitly preserved by enumerating these in the AARCH64 linker flags. The --require-defined linker flag must be used for each symbol to be preserved.
+
 ### PRM Platform GUID
 
 **IMPORTANT** PRM has a concept of a "Platform GUID" which associates a specific platform with a set of PRM modules
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[Michael Kubacki]

For the series: Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

The patch formatting seems a little strange:

1. The patches don't have "Cc" lines.

2. What is this GUID message at the bottom of each patch?

    'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

By the way, I tested the AARCH64 PRM build with sample modules on VS2019 
a while back, so I see that you mentioned only testing GCC (thanks for 
helping get the functions in the export table there) but it should work 
in VS as well.

Regards,
Michael

On 8/3/2022 10:35 AM, Sami Mujawar wrote:
> This patch series adds support to build PRM for AARCH64 using GCC.
> 
> The changes can be seen at:
> https://github.com/samimujawar/edk2/tree/2238_aarch64_prm_support_v1
> 
> Jose Marinho (5):
>    Basetools/GenFw: Allow AARCH64 builds to use the --prm flag
>    PrmPkg: Enable external visibility on PRM symbols
>    PrmPkg: Build Prm Samples with GCC for AARCH64
>    PrmPkg: Support AArch64 builds using GCC
>    PrmPkg: Add details on AArch64 build to the Readme.
> 
>   BaseTools/Source/C/GenFw/Elf64Convert.c                                                  |  6 ++---
>   PrmPkg/Include/Prm.h                                                                     |  3 +++
>   PrmPkg/PrmPkg.ci.yaml                                                                    |  1 +
>   PrmPkg/PrmPkg.dsc                                                                        | 24 +++++++++++++++++---
>   PrmPkg/Readme.md                                                                         | 11 +++++++++
>   PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf |  4 ++++
>   PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf             |  4 ++++
>   7 files changed, 47 insertions(+), 6 deletions(-)
> 

Re: [PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[Ard Biesheuvel]

On Wed, 3 Aug 2022 at 16:36, Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> This patch series adds support to build PRM for AARCH64 using GCC.
>
> The changes can be seen at:
> https://github.com/samimujawar/edk2/tree/2238_aarch64_prm_support_v1
>
> Jose Marinho (5):
>   Basetools/GenFw: Allow AARCH64 builds to use the --prm flag
>   PrmPkg: Enable external visibility on PRM symbols
>   PrmPkg: Build Prm Samples with GCC for AARCH64
>   PrmPkg: Support AArch64 builds using GCC
>   PrmPkg: Add details on AArch64 build to the Readme.
>

i don't maintain any of the packages in question, but the changes look
appropriate to me, so

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Re: [PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[Michael Kubacki]

Hi Sami,

I see you created a PR that is marked as draft with these changes:
https://github.com/tianocore/edk2/pull/3167

When you're ready for them to be submitted, if you could please create a 
non-draft PR with the review tags added, I'd be happy to push it.

Thanks,
Michael

On 8/3/2022 10:35 AM, Sami Mujawar wrote:
> This patch series adds support to build PRM for AARCH64 using GCC.
> 
> The changes can be seen at:
> https://github.com/samimujawar/edk2/tree/2238_aarch64_prm_support_v1
> 
> Jose Marinho (5):
>    Basetools/GenFw: Allow AARCH64 builds to use the --prm flag
>    PrmPkg: Enable external visibility on PRM symbols
>    PrmPkg: Build Prm Samples with GCC for AARCH64
>    PrmPkg: Support AArch64 builds using GCC
>    PrmPkg: Add details on AArch64 build to the Readme.
> 
>   BaseTools/Source/C/GenFw/Elf64Convert.c                                                  |  6 ++---
>   PrmPkg/Include/Prm.h                                                                     |  3 +++
>   PrmPkg/PrmPkg.ci.yaml                                                                    |  1 +
>   PrmPkg/PrmPkg.dsc                                                                        | 24 +++++++++++++++++---
>   PrmPkg/Readme.md                                                                         | 11 +++++++++
>   PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf |  4 ++++
>   PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf             |  4 ++++
>   7 files changed, 47 insertions(+), 6 deletions(-)
> 

Re: [PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[Sami Mujawar]

Hi Michael,

Please find the link for the pull request with the review tags updated 
at https://github.com/tianocore/edk2/pull/3183

Regards,

Sami Mujawar

On 05/08/2022 04:08 pm, Michael Kubacki wrote:
> Hi Sami,
>
> I see you created a PR that is marked as draft with these changes:
> https://github.com/tianocore/edk2/pull/3167
>
> When you're ready for them to be submitted, if you could please create 
> a non-draft PR with the review tags added, I'd be happy to push it.
>
> Thanks,
> Michael
>
> On 8/3/2022 10:35 AM, Sami Mujawar wrote:
>> This patch series adds support to build PRM for AARCH64 using GCC.
>>
>> The changes can be seen at:
>> https://github.com/samimujawar/edk2/tree/2238_aarch64_prm_support_v1
>>
>> Jose Marinho (5):
>>    Basetools/GenFw: Allow AARCH64 builds to use the --prm flag
>>    PrmPkg: Enable external visibility on PRM symbols
>>    PrmPkg: Build Prm Samples with GCC for AARCH64
>>    PrmPkg: Support AArch64 builds using GCC
>>    PrmPkg: Add details on AArch64 build to the Readme.
>>
>> BaseTools/Source/C/GenFw/Elf64Convert.c |  6 ++---
>> PrmPkg/Include/Prm.h |  3 +++
>> PrmPkg/PrmPkg.ci.yaml |  1 +
>> PrmPkg/PrmPkg.dsc | 24 +++++++++++++++++---
>> PrmPkg/Readme.md | 11 +++++++++
>> PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf 
>> |  4 ++++
>> PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf 
>> |  4 ++++
>>   7 files changed, 47 insertions(+), 6 deletions(-)
>>

Re: [PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[Sami Mujawar]

Hi Michael,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 04/08/2022 05:09 am, Michael Kubacki wrote:
> For the series: Reviewed-by: Michael Kubacki 
> <michael.kubacki@microsoft.com>
>
> The patch formatting seems a little strange:
>
> 1. The patches don't have "Cc" lines.
>
[SAMI] I have not been following the convention to add Cc lines to the 
patches. However, I will start from the next patch series.
> 2. What is this GUID message at the bottom of each patch?
>
>    'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
[SAMI] I have added this so that I can easily search my patches. It does 
not affect the patch though.
>
> By the way, I tested the AARCH64 PRM build with sample modules on 
> VS2019 a while back, so I see that you mentioned only testing GCC 
> (thanks for helping get the functions in the export table there) but 
> it should work in VS as well.

[SAMI] I have tried building PrmPkg with VS2019 and found that the 
AARCH64 builds do not work.

         "C:\Program Files (x86)\Microsoft Visual 
Studio\2019\Professional\VC\Tools\MSVC\14.28.29910\bin\Hostx86\arm64\armasm64.exe" 
/Fow:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.obj 
/nologo /g 
/Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe\AArch64 
/Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe\Arm 
/Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe\AArch64 
/Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe 
/Iw:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\DEBUG 
/Iw:\edk2-exports\edk2\MdePkg /Iw:\edk2-exports\edk2\MdePkg\Include 
/Iw:\edk2-exports\edk2\MdePkg\Test\UnitTest\Include 
/Iw:\edk2-exports\edk2\MdePkg\Include\AArch64 
w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii
w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(7) 
: error A2034: unknown opcode: .
     .text
w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(8) 
: error A2034: unknown opcode: .
     .align  5
w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(9) 
: error A2034: unknown opcode: InternalMemCompareGuid
ASM_GLOBAL InternalMemCompareGuid
w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(13) 
: error A2173: syntax error in expression
     cbz     x1, 0f
w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(19) 
: warning A4045: missing END directive
     ret
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual 
Studio\2019\Professional\VC\Tools\MSVC\14.28.29910\bin\Hostx86\arm64\armasm64.exe"' 
: return code '0x1'
Stop.

Can you please let me know if this is a configuration/environment issue 
or the AARCH64 assembly files need to be ported for Visual Studio?

I have tried building other Arm packages and though the assembler files 
need porting, which I believe has been done as part of ProjectMu 
(https://github.com/microsoft/mu_silicon_arm_tiano).

It would be good to have AARCH64 builds of edk2 with Visual Studio as 
this would allow to use the VS static analysis tool.

Can you let me know if you have any plans to enable this support in 
Tianocore\edk2 project, please?

[/SAMI]

> Regards,
> Michael
>
> On 8/3/2022 10:35 AM, Sami Mujawar wrote:
>> This patch series adds support to build PRM for AARCH64 using GCC.
>>
>> The changes can be seen at:
>> https://github.com/samimujawar/edk2/tree/2238_aarch64_prm_support_v1
>>
>> Jose Marinho (5):
>>    Basetools/GenFw: Allow AARCH64 builds to use the --prm flag
>>    PrmPkg: Enable external visibility on PRM symbols
>>    PrmPkg: Build Prm Samples with GCC for AARCH64
>>    PrmPkg: Support AArch64 builds using GCC
>>    PrmPkg: Add details on AArch64 build to the Readme.
>>
>> BaseTools/Source/C/GenFw/Elf64Convert.c |  6 ++---
>> PrmPkg/Include/Prm.h |  3 +++
>> PrmPkg/PrmPkg.ci.yaml |  1 +
>> PrmPkg/PrmPkg.dsc | 24 +++++++++++++++++---
>> PrmPkg/Readme.md | 11 +++++++++
>> PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf 
>> |  4 ++++
>> PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf 
>> |  4 ++++
>>   7 files changed, 47 insertions(+), 6 deletions(-)
>>

Re: [edk2-devel] [PATCH v1 0/5] Add support to build PRM for AARCH64 using GCC

[Michael Kubacki]

Responses inline.


On 8/8/2022 6:35 AM, Sami Mujawar wrote:
> Hi Michael,
> 
> Please find my response inline marked [SAMI].
> 
> Regards,
> 
> Sami Mujawar
> 
> On 04/08/2022 05:09 am, Michael Kubacki wrote:
>> For the series: Reviewed-by: Michael Kubacki 
>> <michael.kubacki@microsoft.com>
>>
>> The patch formatting seems a little strange:
>>
>> 1. The patches don't have "Cc" lines.
>>
> [SAMI] I have not been following the convention to add Cc lines to the 
> patches. However, I will start from the next patch series.
>> 2. What is this GUID message at the bottom of each patch?
>>
>>    'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> [SAMI] I have added this so that I can easily search my patches. It does 
> not affect the patch though.
>>

Okay, no problem I had just not seen it before.

>> By the way, I tested the AARCH64 PRM build with sample modules on 
>> VS2019 a while back, so I see that you mentioned only testing GCC 
>> (thanks for helping get the functions in the export table there) but 
>> it should work in VS as well.
> 
> [SAMI] I have tried building PrmPkg with VS2019 and found that the 
> AARCH64 builds do not work.
> 
>          "C:\Program Files (x86)\Microsoft Visual 
> Studio\2019\Professional\VC\Tools\MSVC\14.28.29910\bin\Hostx86\arm64\armasm64.exe" 
> /Fow:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.obj 
> /nologo /g 
> /Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe\AArch64 
> /Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe\Arm 
> /Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe\AArch64 
> /Iw:\edk2-exports\edk2\MdePkg\Library\BaseMemoryLibOptDxe 
> /Iw:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\DEBUG 
> /Iw:\edk2-exports\edk2\MdePkg /Iw:\edk2-exports\edk2\MdePkg\Include 
> /Iw:\edk2-exports\edk2\MdePkg\Test\UnitTest\Include 
> /Iw:\edk2-exports\edk2\MdePkg\Include\AArch64 
> w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii 
> 
> w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(7) 
> : error A2034: unknown opcode: .
>      .text
> w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(8) 
> : error A2034: unknown opcode: .
>      .align  5
> w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(9) 
> : error A2034: unknown opcode: InternalMemCompareGuid
> ASM_GLOBAL InternalMemCompareGuid
> w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(13) 
> : error A2173: syntax error in expression
>      cbz     x1, 0f
> w:\edk2-exports\Build\Prm\DEBUG_VS2019\AARCH64\MdePkg\Library\BaseMemoryLibOptDxe\BaseMemoryLibOptDxe\OUTPUT\AArch64\CompareGuid.iiii(19) 
> : warning A4045: missing END directive
>      ret
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual 
> Studio\2019\Professional\VC\Tools\MSVC\14.28.29910\bin\Hostx86\arm64\armasm64.exe"' 
> : return code '0x1'
> Stop.
> 
> Can you please let me know if this is a configuration/environment issue 
> or the AARCH64 assembly files need to be ported for Visual Studio?
> 
> I have tried building other Arm packages and though the assembler files 
> need porting, which I believe has been done as part of ProjectMu 
> (https://github.com/microsoft/mu_silicon_arm_tiano).
> 
> It would be good to have AARCH64 builds of edk2 with Visual Studio as 
> this would allow to use the VS static analysis tool.
> 
> Can you let me know if you have any plans to enable this support in 
> Tianocore\edk2 project, please?
> 
> [/SAMI]
> 
I did build it with Project Mu picking up PrmPkg as a submodule. Yes, I 
will investigate enabling this in edk2.

>> Regards,
>> Michael
>>
>> On 8/3/2022 10:35 AM, Sami Mujawar wrote:
>>> This patch series adds support to build PRM for AARCH64 using GCC.
>>>
>>> The changes can be seen at:
>>> https://github.com/samimujawar/edk2/tree/2238_aarch64_prm_support_v1
>>>
>>> Jose Marinho (5):
>>>    Basetools/GenFw: Allow AARCH64 builds to use the --prm flag
>>>    PrmPkg: Enable external visibility on PRM symbols
>>>    PrmPkg: Build Prm Samples with GCC for AARCH64
>>>    PrmPkg: Support AArch64 builds using GCC
>>>    PrmPkg: Add details on AArch64 build to the Readme.
>>>
>>> BaseTools/Source/C/GenFw/Elf64Convert.c |  6 ++---
>>> PrmPkg/Include/Prm.h |  3 +++
>>> PrmPkg/PrmPkg.ci.yaml |  1 +
>>> PrmPkg/PrmPkg.dsc | 24 +++++++++++++++++---
>>> PrmPkg/Readme.md | 11 +++++++++
>>> PrmPkg/Samples/PrmSampleAcpiParameterBufferModule/PrmSampleAcpiParameterBufferModule.inf 
>>> |  4 ++++
>>> PrmPkg/Samples/PrmSampleContextBufferModule/PrmSampleContextBufferModule.inf 
>>> |  4 ++++
>>>   7 files changed, 47 insertions(+), 6 deletions(-)
>>>
> 
> 
> 
>