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

["Michael Kubacki" <mikuback@linux.microsoft.com>]

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(-)
>>>
> 
> 
> 
>