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.

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

arch: Add aarch64 support #205

Merged
merged 15 commits into from
Jan 22, 2023

Conversation

retrage
Copy link
Contributor

@retrage retrage commented Nov 1, 2022

This PR proposes introducing an initial aarch64 support (#198) for aarch64/KVM Cloud Hypervisor.

NOTE: This is a draft PR. It is not ready for review. The main blocking issue is that the changes in commit a2a986f leads to fail existing CI tests. I need to fix the breaking changes.

@retrage retrage mentioned this pull request Nov 1, 2022
9 tasks
@retrage retrage force-pushed the aarch64-ch-support-clean-v2 branch 4 times, most recently from 8628f53 to 568ed71 Compare November 4, 2022 09:29
@retrage retrage force-pushed the aarch64-ch-support-clean-v2 branch 3 times, most recently from 7ee0cf2 to edeabe7 Compare November 20, 2022 14:16
@retrage
Copy link
Contributor Author

retrage commented Nov 20, 2022

I mark this PR as ready for review as I added aarch64 support to some basic tests and CI builds.
There are several remaining tasks as tracked in issue #198:

  • The unit tests are not enabled for aarch64 in GitHub Actions because GitHub hosted runner is x86_64 only. Added to Jenkins CI (907c346).
  • The integration tests are not implemented for aarch64 yet. I tested this aarch64 support with Ubuntu Bionic cloud image, but it needs another effort to add to integration tests because the network config seems to be different from the x86_64 image.

@retrage retrage marked this pull request as ready for review November 20, 2022 15:37
@retrage retrage requested a review from rbradford November 20, 2022 15:37
@michael2012z
Copy link
Member

For the unit test and integration test, is it possible to deploy them to the AArch64 server that is carrying the CI of the Cloud hypervisor repo?
I know nothing about Jenkins. It would be good if we can use that server for both repos.

@retrage retrage force-pushed the aarch64-ch-support-clean-v2 branch 2 times, most recently from 3b65650 to 907c346 Compare November 21, 2022 13:35
@retrage
Copy link
Contributor Author

retrage commented Nov 21, 2022

Thank you for your comment. I added the unit tests to Jenkins CI (907c346).

Copy link
Member

@MrXinWang MrXinWang left a comment

Choose a reason for hiding this comment

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

Hi @retrage, this is such a great work! Thank you very much for introducing this :)

I quickly went through the series and have two questions. Will have a more detailed review later.

src/fdt.rs Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
@michael2012z
Copy link
Member

The READMD.md doc should be updated as well to cover the test instructions on AArch64.

src/arch/aarch64/ram64.s Outdated Show resolved Hide resolved
src/arch/aarch64/ram64.s Outdated Show resolved Hide resolved
@retrage
Copy link
Contributor Author

retrage commented Nov 25, 2022

Let me explain the entry code to answer #205 (comment) and #205 (comment).
The head of the entry code is compatible with a Linux kernel image [1]. Some magic numbers comes from the spec. The hypervisor first tries to load the given image as a PE image [3] if valid magic number found [4].

I need to explain the magic instruction:

  add x13, x18, #0x16   /* code0 */

The assembler encodes this instruction to MZ, which is the magic number of a PE binary [2]. (The output binary format is raw.) This magic instruction is just to be loaded as a PE binary. This trick seems to be widely used by aarch64 kernels such as Linux [5].

Anyway, I'll add source code comments to give contexts.

[1] https://www.kernel.org/doc/Documentation/arm64/booting.txt
[2] https://en.wikipedia.org/wiki/Portable_Executable
[3] https://github.com/cloud-hypervisor/cloud-hypervisor/blob/e23f4e0783eb10d99ff3b3063a1f7a1de20e61d1/vmm/src/vm.rs#L918
[4] https://github.com/rust-vmm/linux-loader/blob/8a6eed8556987a46e95f2dfdf6fdb93a05ee457c/src/loader/aarch64/pe/mod.rs#L130
[5] https://github.com/torvalds/linux/blob/08ad43d554bacb9769c6a69d5f771f02f5ba411c/arch/arm64/kernel/efi-header.S#L17

retrage added a commit to retrage/rust-hypervisor-firmware that referenced this pull request Nov 27, 2022
`E820Entry` is used in `boot::Info` trait, which is x86_64-dependent
memory entry data structure. It's confusing if e820 is used in other
architectures. This commit introduces an architecture-independent memory
entry `bootinfo::MemoryEntry` and replaces `boot::Info` with
`bootinfo::Info`. This change is suggested in [1].

[1]
cloud-hypervisor#205 (comment)

Signed-off-by: Akira Moroo <retrage01@gmail.com>
retrage added a commit to retrage/rust-hypervisor-firmware that referenced this pull request Nov 27, 2022
`E820Entry` is used in `boot::Info` trait, which is x86_64-dependent
memory entry data structure. It's confusing if e820 is used in other
architectures. This commit introduces an architecture-independent memory
entry `bootinfo::MemoryEntry` and replaces `boot::Info` with
`bootinfo::Info`. This change is suggested in [1].

[1]
cloud-hypervisor#205 (comment)

Signed-off-by: Akira Moroo <retrage01@gmail.com>
retrage added a commit that referenced this pull request Nov 28, 2022
`E820Entry` is used in `boot::Info` trait, which is x86_64-dependent
memory entry data structure. It's confusing if e820 is used in other
architectures. This commit introduces an architecture-independent memory
entry `bootinfo::MemoryEntry` and replaces `boot::Info` with
`bootinfo::Info`. This change is suggested in [1].

[1]
#205 (comment)

Signed-off-by: Akira Moroo <retrage01@gmail.com>
@retrage retrage force-pushed the aarch64-ch-support-clean-v2 branch 2 times, most recently from 8b83669 to b84a04b Compare December 3, 2022 06:35
@rbradford
Copy link
Member

  • The integration tests are not implemented for aarch64 yet. I tested this aarch64 support with Ubuntu Bionic cloud image, but it needs another effort to add to integration tests because the network config seems to be different from the x86_64 image.

@retrage. Great work! Could you elaborate on this so we can get this included?

code_start = .;
.text.boot : { *(.text.boot) }
.text : { *(.text .text.*) }
. = ALIGN(4K);

Choose a reason for hiding this comment

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

Suggested change
. = ALIGN(4K);
. = ALIGN(64K);

Paging and TTable code assumes 64KiB pages, but you are aligning to 4KiB here (rest of file as well)

This commit adds initial support for aarch64 build. The goal is to boot
Linux with this firmware on aarch64 Cloud Hypervisor. This commit
includes a bootstrap assembly and build configs for building a binary
that complies the Linux kernel image format [1]. It also includes some
stub code to make the aarch64 target buildable with minimal changes.
The later commits will implement the code.

The current `pe::tests::test_loader` test uses an x86_64 specific disk
image. We need to provide aarch64 specific target for the test.

[1] https://docs.kernel.org/arm64/booting.html#call-the-kernel-image

Signed-off-by: Akira Moroo <retrage01@gmail.com>
This commit adds initial paging support for aarch64. This implementation
creates identity mapping translation tables for Cloud Hypervisor. The
most of paging implementation is based on [1]. It also introduces use of
`asm_const` to parameterize FDT base address and stack base address.

[1]
https://github.com/rust-embedded/rust-raspberrypi-OS-tutorials/tree/master/10_virtual_mem_part1_identity_mapping

Signed-off-by: Akira Moroo <retrage01@gmail.com>
This commit introduces `fdt::StartInfo` to boot using device info from
FDT.

Signed-off-by: Akira Moroo <retrage01@gmail.com>
Signed-off-by: Akira Moroo <retrage01@gmail.com>
This commit implements aarch64 specific UART device PL011 driver.

Signed-off-by: Akira Moroo <retrage01@gmail.com>
This commit implements aarch64 specific RTC device PL031 driver.

Signed-off-by: Akira Moroo <retrage01@gmail.com>
Signed-off-by: Akira Moroo <retrage01@gmail.com>
Signed-off-by: Akira Moroo <retrage01@gmail.com>
This commit implements aarch64 specific `rust64_start()` to enable
aarch64 boot.

Signed-off-by: Akira Moroo <retrage01@gmail.com>
Signed-off-by: Akira Moroo <retrage01@gmail.com>
Signed-off-by: Akira Moroo <retrage01@gmail.com>
Signed-off-by: Akira Moroo <retrage01@gmail.com>
This commit adds aarch64 support to containerized build and test. The
integration tests are not yet supported for aarch64.

Signed-off-by: Akira Moroo <retrage01@gmail.com>
This commit introduces matrix to add the aarch64 target. The unit tests
are disabled for aarch64 target because default GitHub Actions runners
are x86_64 only.

Signed-off-by: Akira Moroo <retrage01@gmail.com>
NOTE: Because docker pull fallbacks to linux/amd64 even if explicitly
specify image platform when linux/arm64 image not found, the unit tests
will fail. To avoid this, `--local` option is added to build local
container image. This option should be removed after linux/arm64
container image is pushed to the registry.

Signed-off-by: Akira Moroo <retrage01@gmail.com>
@retrage retrage force-pushed the aarch64-ch-support-clean-v2 branch from b84a04b to 9b21de0 Compare January 15, 2023 02:26
Copy link
Member

@rbradford rbradford left a comment

Choose a reason for hiding this comment

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

lgtm from me but I would like to hear review from one of the ARM folks. Also we need to get the integration testing in so this good work doesn't get accidentally broken!

@michael2012z
Copy link
Member

@jongwu , @MrXinWang and me also reviewed and discussed about the code. No more comments from us.

@rbradford
Copy link
Member

@retrage Please merge when you're ready.

Copy link
Member

@MrXinWang MrXinWang left a comment

Choose a reason for hiding this comment

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

As @michael2012z said. No more comment from my side. Thanks for the effort!

@retrage
Copy link
Contributor Author

retrage commented Jan 22, 2023

Thank you for your approval. Although there is a lot of room for improvement, but I would like to merge this PR so far.

I'll keep #198 updated to track remaining issues for aarch64. As for integration testing, I'm still working on getting other disk images other than the custom Ubuntu 18.04 image to work so that we can run integration tests just like x86_64.

@retrage retrage merged commit 8feecb9 into cloud-hypervisor:main Jan 22, 2023
@retrage retrage deleted the aarch64-ch-support-clean-v2 branch January 22, 2023 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants