-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UefiPayloadPkg: Add AARCH64 support #6545
base: master
Are you sure you want to change the base?
Conversation
01d072a
to
f06afd7
Compare
PR can not be merged due to conflict. Please rebase and resubmit |
75210b2
to
c9a715a
Compare
a505283
to
b52c670
Compare
@leiflindholm @benjamindoron This PR (6545) is created to address comments in PR 6339, using existing DSC/FDF to build UPL for both X64 and AARCH64 architectures. I have built X64 and AARCH64 separately by command: "build -a X64" and "build -a AARCh64" successfully. However, CI check will build X64, AARCh64 at one build by command "build -a X64 -a AARCH64". This is CI check, for instance module BdsDXE will be built for both X64/AARCH64, then trigger this error: Any suggestion to fix this issue? |
13bc27f
to
15d6e39
Compare
c2e331e
to
15d6e39
Compare
That's the same issue I ran into with #6262, and a good solution was not obvious to me. And I still haven't thought of an appealing workaround for it. Let me fire off an email to the mailing list on that topic. |
OK, not the same issue - related to that issue. |
666c68b
to
5263c81
Compare
Thanks @leiflindholm, you provided a constructive suggestion. :) As you suggested, I used following content to passed all build: I have tried following build commands, they all succeed to build corresponding binaries:
Let's wait for github CI result. |
FLASH_DEFINITION does not take effort, since FV is not generated for each architecture. |
Add basic support for FIT image on the AARCH64 architecture, reuse exsitting DSC and FDF files for IA32, X64 and AARCH64 architectures. Introduce new PCD: PcdUseUniversalPayloadSerialPort to indicate which serial port module is used due to some serial port parameters are fixed for ARM SoC and Platform. Please use following command to build AARCH64 UPL FIT image: " export GCC5_AARCH64_PREFIX=aarch64-linux-gnu- python UefiPayloadPkg/UniversalPayloadBuild.py -a AARCH64 -t GCC5 -b DEBUG --Fit " Signed-off-by: Amos Bu <[email protected]> Signed-off-by: Ajan Zhong <[email protected]>
AARCH64 CpuDxe takes charge of constructing Translation Tables with memory map information provided by bootloader, and enabling MMU, setting EOImode to perform both priority drop and deactivation. Signed-off-by: Ajan Zhong <[email protected]>
Enable CpuDxeAArch64 in UefiPayloadPkg for AARCH64 architecture. Signed-off-by: Ajan Zhong <[email protected]>
Azure Ubuntu GCC5 pipeline builds packages with all architectures. However, UefiPayloadPkg supports multiple architectures, including IA32, X64 and AARCH64. In this case, Azure pipeline builds modules for IA32, X64 and AARCH64 architectures, generates FD with same FDF file. It leads build system failed to locate which module should be integrated into Flash image. Add Build.Archlist to specify architectures to be build with, and separate TARGET_UEFIPAYLOAD to TARGET_UEFIPAYLOAD_IA32_X64 and TARGET_UEFIPAYLOAD_AARCH64 to cover all supported architectures for UefiPayloadPkg package. Signed-off-by: Ajan Zhong <[email protected]>
5263c81
to
fa0e801
Compare
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future. Non-collaborators requested: Attn Admins: Admin Instructions:
|
@@ -0,0 +1,149 @@ | |||
/** @file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit (which github does not let me comment on directly as a whole) seems to pull in source code from multiple existing libraries into this new CpuDxeAArch64.
I would very much like to avoid that if possible.
Is this in order to not have dependencies on the ArmPkg versions?
If so, I'd rather take this opportunity to chip away a bit further on #10289
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right. ArmPkg cannot be used in UefiCpuPkg, so I porting required methods from ArmPkg.
About removing ArmPkg, how about let us do it in coming commits which focus on #10289, since we will have other modules which resides in ArmPkg. If we do it (remove ArmPkg) in this PR, we need plenty of effort for porting, validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no porting or validation needed - it would just be moving of code.
Meanwhile, duplicating existing architectural operations would absolutely not be acceptable.
We did a partway thing recently - f2b9d54. I think we should do the similar thing here. But don't fret about it being invasive - it's just mechanics.
SPDX-License-Identifier: BSD-2-Clause-Patent | ||
**/ | ||
|
||
#include <PiPei.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to be a copy of MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c, with mainly the if (PcdGetBool (PcdSetNxForStack)) {
part removed.
I'm guessing UefiPayloadPkg effectively replaces DxeIpl, but is there a particular reason for why this needs to become architecture-specific?
(This does not need resolving now, but I would like to understand.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. This file is introduced here to keep architecture design logic unchanged, since UefiPayloadEntry has already introduced Ia32/RiscV64/X64. This commit is used to keep UefiPayloadEntry module design philosophy unchanged.
VOID | ||
) | ||
{ | ||
return 44; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No :)
(Setting aside previous comment about ArmPkg, this should be a call that ends up in
Library/ArmLib/AArch64/ArmLibSupport.S:ASM_FUNC(ArmGetPhysicalAddressBits)
If we need to create a wrapper function to export that, we can absolutely do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No :) (Setting aside previous comment about ArmPkg, this should be a call that ends up in
Library/ArmLib/AArch64/ArmLibSupport.S:ASM_FUNC(ArmGetPhysicalAddressBits)
If we need to create a wrapper function to export that, we can absolutely do that.
Sure, let will refine in.
@@ -0,0 +1,25 @@ | |||
/** @file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a lot cleaner to break this change, and the corresonding ones in UefiPayloadEntry.c and UefiPayloadEntry.h, out into a separate commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will create a new commit (something like 'introduce GetPhyAddrBit to adapt multiple architectures') in this PR for Ia32 and X64.
@@ -0,0 +1,25 @@ | |||
/** @file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and merge this change with the Ia32 one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get you, let me introduce a new commit to fix it for both Ia32 and X64
That's me done commenting - I'll leave someone else to comment on the CI aspect, but thank you very much for this rework. It greatly simplified reviewing. |
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4849
Description
Add base FIT image support on the AARCH64 architecture, introduce new dsc and fdf files for AARCH64 architecture, and introduce new PCD: PcdUseUniversalPayloadSerialPort to indicate which serial port component is used due to some serial port parameters are fixed for ARM SoC and Platform.
How This Was Tested
Validated on top of qemu-system-x86_64 and qemu-system-aarch64.
Integration Instructions
export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
python UefiPayloadPkg/UniversalPayloadBuild.py -a AARCH64 -t GCC5 -b DEBUG --Fit