Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improving BaseTools for ARM/ARM64 systems #5953

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Improving BaseTools for ARM/ARM64 systems #5953

wants to merge 3 commits into from
+295 −24

Conversation

kuqin12
Copy link
Contributor

@kuqin12 kuqin12 commented Jul 23, 2024 •
edited
Loading

Description

This patch series is made to be able to build base tools for ARM/AARCH64 systems using Visual Studio. The change also covers the support for cross compilation of BaseTools using Visual Studio.

In addition, the change also updated the building process to link custom library instead of system inbox ones to support cross compilation on Linux systems.

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

This built output was tested on Windows ARM and Linux ARM systems, and used for building UEFI firmware.

Integration Instructions

N/A

@kuqin12 kuqin12 marked this pull request as ready for review July 23, 2024 21:20
@Javagedes Javagedes requested review from a user, lgao4, BobCF and YuweiChen1110 July 23, 2024 21:36
@ghost
Copy link

ghost commented Jul 24, 2024

Tested on Ubuntu 24.04 with gcc 13.2.0 and with clang (make CC=clang CXX=clang++) 18.

lgao4
lgao4 reviewed Jul 24, 2024
View reviewed changes
BaseTools/Source/C/DevicePath/GNUmakefile
@@ -30,6 +30,10 @@ ifeq ($(CYGWIN), CYGWIN)
endif

ifeq ($(LINUX), Linux)
ifndef CROSS_LIB_UUID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which tool defines CROSS_LIB_UUID? Does gcc compiler define it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgao4 thanks for catching that. It was meant to be declared in the python script: see example here: https://github.com/microsoft/mu_basecore/blob/d9c18c384889d0c445865f96ac660f6abee6115f/BaseTools/Edk2ToolsBuild.py#L263

This is done because the in-box uuid library will not work for cross compilation so that we need to pull down uuid source and build from scratch. But please let me know if you prefer other methods in doing so, otherwise I will include the python script change in the next push.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcran , how do you resolve this problem? Do you also pull UUID source code and compile it? I don't want user to compile UUID source. Could we upload the pre-built UUID lib file?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgao4 I'm not sure we want to support cross-building BaseTools - do we support that on Linux?
Why not just build BaseTools on an ARM version of Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because there is no pipeline support of Windows on ARM :(

@lgao4
Copy link
Contributor

lgao4 commented Jul 24, 2024

Tested on Ubuntu 24.04 with gcc 13.2.0 and with clang (make CC=clang CXX=clang++) 18.

Do you test on ARM ubuntun?

@leiflindholm
Copy link
Member

BaseTools have been buildable on ARM/AArch64 since at least e0ba625, merged in September 2014. Please update the description to reflect what this set actually does.

@leiflindholm
Copy link
Member

In fact, having looked at the actual set, I feel this should be reworked after the description has been corrected, since it seems to be confusing several unrelated changes with misleading description.
It can also do with some overall cleanup, since it's carrying across comments from project MU.

@leiflindholm leiflindholm self-requested a review July 24, 2024 13:42
leiflindholm
leiflindholm suggested changes Jul 24, 2024
View reviewed changes
Copy link
Member

@leiflindholm leiflindholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Please update the PR to describe what it is intended to do.
  2. Please rework as at least three commits:
  • Enable support for building BaseTools for Arm/AArch64 using visual studio.
  • Enable support for crossbuilding BaseTools using Visual Studio.
  • Enable support for linking against custom libraries instead of those provided with the system.
  1. Clean up patches so we're not importing random project MU comments only to subsequently delete them.

@kuqin12 kuqin12 changed the title Introduce BaseTools for ARM/ARM64 systems Improving BaseTools for ARM/ARM64 systems Jul 24, 2024
@ghost
Copy link

ghost commented Jul 24, 2024

Do you test on ARM ubuntun?

I tested it on x86 Ubuntu 24.04, aarch64 openSUSE Tumbleweed and aarch64 FreeBSD 15-CURRENT.

@github-actions GitHub Actions
Copy link

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added stale Due to lack of updates, this item is pending deletion. and removed stale Due to lack of updates, this item is pending deletion. labels Sep 28, 2024
@bexcran
Copy link
Contributor

bexcran commented Oct 18, 2024

@kuqin12 I see this PR is still marked as having changes requested by @leiflindholm. Are you still working on it?

@kuqin12
Copy link
Contributor Author

kuqin12 commented Oct 18, 2024

@kuqin12 I see this PR is still marked as having changes requested by @leiflindholm. Are you still working on it?

Sorry for the delay. Yes, I made the change shortly after the feedback was provided. But then got distracted by some other items. I will get back to this in the next week. Please keep this open. Thanks.

@kuqin12 kuqin12 force-pushed the base_arm branch 4 times, most recently from 20edc2d to 4ec98bb Compare November 14, 2024 23:09
kuqin12 and others added 3 commits November 15, 2024 00:10
@kuqin12
BaseTools: Adding support of building BaseTool on Windows ARM/ARM64
e4e2459 
This change focuses on the support of building basetool natively for
Windows ARM/ARM64 host system, which will enable the ARM based platforms
to build UEFI and unit tests.

Note that the warnings due to integer conversions are suppressed for
this specific target to avoid too much local changes carried in MU. The
formal change should drop all these binaries and move to pythonic
scripts.

The binary output is tested on Windows ARM based hardware systems.

Cc: Rebecca Cran <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Bob Feng <[email protected]>
Cc: Yuwei Chen <[email protected]>
Signed-off-by: Kun Qin <[email protected]>
@kuqin12
BaseTools: Adding cross compilation of BaseTool for Windows ARM/ARM64
4731c01 
This change adds the support of crossbuilding basetool for Windows ARM/
ARM64 systems, which will enable the generally available pipeline agents
to build binary tools and make releases as they see fit.

The EDK2 base tools build script is also updated to support cross
compilation using this script.

The crossbuilt binary output is tested on Windows ARM based hardware
systems.

Cc: Rebecca Cran <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Bob Feng <[email protected]>
Cc: Yuwei Chen <[email protected]>
Signed-off-by: Kun Qin <[email protected]>
@kuqin12
BaseTools: Support custom library build for base tools on Linux ARM
12bd42f 
This change added the build script to cross compile the base tool
binaries for Linux ARM/AARCH64 systems.

The needed libuuid system library is pulled from source file and rebuilt
to support the corresponding library dependencies. Individual tools'
makefiles are also updated to link the cross compiled library as well.

The EDK2 base tool build script was also updated to support such change.

This was tested functional on Linux ARM host system.

Cc: Rebecca Cran <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Bob Feng <[email protected]>
Cc: Yuwei Chen <[email protected]>
Signed-off-by: Kun Qin <[email protected]>
@kuqin12
Copy link
Contributor Author

kuqin12 commented Nov 15, 2024

  1. Please update the PR to describe what it is intended to do.
  2. Please rework as at least three commits:
  • Enable support for building BaseTools for Arm/AArch64 using visual studio.
  • Enable support for crossbuilding BaseTools using Visual Studio.
  • Enable support for linking against custom libraries instead of those provided with the system.
  1. Clean up patches so we're not importing random project MU comments only to subsequently delete them.

The PR is updated with suggested changes. Could you please re-review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgao4 lgao4 lgao4 approved these changes

@leiflindholm leiflindholm leiflindholm requested changes

@BobCF BobCF Awaiting requested review from BobCF

@YuweiChen1110 YuweiChen1110 Awaiting requested review from YuweiChen1110

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kuqin12 @lgao4 @leiflindholm @bexcran