Conversation
ludfjig
requested review from
danbugs,
dblnz,
devigned,
jprendes,
marosset,
simongdavies and
syntactically
as code owners
ludfjig
added
the
kind/refactor
Add conditional compilation gates and aarch64 stub modules so that cargo build --target aarch64-unknown-linux-gnu -p hyperlight-host succeeds. No aarch64 implementation is added. All aarch64 code paths are stubs that panic at runtime. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Enable hyperlight-host to cross-compile for aarch64-unknown-linux-gnu by adding arch-specific module gating and aarch64 stub implementations, without changing x86_64 behavior.
Changes:
- Added aarch64 stub modules for KVM/MSHV and Hyperlight VM plumbing behind
cfg(target_arch). - Split register and VM backend modules into arch-specific submodules with conditional re-exports.
- Updated build configuration and dependencies to gate x86_64-only functionality (gdb/crashdump) and unblock aarch64 builds.
Reviewed changes
Copilot reviewed 19 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_host/src/mem/memory_region.rs | Adds arch-gated MSHV imports and aarch64 stub TryFrom for intercept messages. |
| src/hyperlight_host/src/hypervisor/virtual_machine/mshv/mod.rs | Introduces arch-split MSHV module selection via cfg(target_arch). |
| src/hyperlight_host/src/hypervisor/virtual_machine/mshv/aarch64.rs | Adds aarch64 MSHV stub VM implementation. |
| src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs | Relaxes “no hypervisor” compile error for aarch64; adjusts VmExit::Debug fields for arch gating. |
| src/hyperlight_host/src/hypervisor/virtual_machine/kvm/mod.rs | Introduces arch-split KVM module selection via cfg(target_arch). |
| src/hyperlight_host/src/hypervisor/virtual_machine/kvm/aarch64.rs | Adds aarch64 KVM stub VM implementation. |
| src/hyperlight_host/src/hypervisor/regs/x86_64/mod.rs | Moves x86_64 register modules under an arch-specific namespace. |
| src/hyperlight_host/src/hypervisor/regs/aarch64/mod.rs | Adds placeholder aarch64 register type stubs. |
| src/hyperlight_host/src/hypervisor/regs.rs | Switches to arch-gated register module re-exports. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs | Extracts arch-independent HyperlightVm logic into a shared module. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs | Adds aarch64 HyperlightVm method stubs to satisfy compilation. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm.rs | Refactors existing HyperlightVm implementation to rely on shared module pieces and visibility changes. |
| src/hyperlight_host/build.rs | Gates gdb and crashdump cfg aliases to x86_64. |
| src/hyperlight_host/Cargo.toml | Makes elfcore optional and ties it to the crashdump feature. |
| src/hyperlight_common/src/vmem.rs | Adds cfg_attr mapping for aarch64 vmem implementation file. |
| src/hyperlight_common/src/layout.rs | Adds cfg_attr mapping for aarch64 layout implementation file. |
| src/hyperlight_common/src/arch/aarch64/vmem.rs | Adds aarch64 vmem stub module with placeholder APIs. |
| src/hyperlight_common/src/arch/aarch64/layout.rs | Adds aarch64 layout stub module with placeholder APIs. |
| docs/aarch64-architecture-plan.md | Documents the phased approach and desired patterns for aarch64 support. |
Comment on lines
+586
to
+604
| Ok(VmExit::MmioRead(addr)) => { | ||
| let all_regions = self.get_mapped_regions(); | ||
| match get_memory_access_violation( | ||
| addr as usize, | ||
| MemoryRegionFlags::WRITE, | ||
| all_regions, | ||
| ) { | ||
| Some(MemoryAccess::AccessViolation(region_flags)) => { | ||
| break Err(RunVmError::MemoryAccessViolation { | ||
| addr, | ||
| access_type: MemoryRegionFlags::READ, | ||
| region_flags, | ||
| }); | ||
| } | ||
| None => { | ||
| break Err(RunVmError::MmioReadUnmapped(addr)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The MMIO read path checks permissions using MemoryRegionFlags::WRITE, which will misclassify missing READ permissions (and can incorrectly report violations based on write permission). Pass MemoryRegionFlags::READ as the tried flag for VmExit::MmioRead.
| } | ||
|
|
||
| /// Get the currently mapped dynamic memory regions (not including initial sandbox region) | ||
| pub(crate) fn get_mapped_regions(&self) -> impl Iterator<Item = &MemoryRegion> + Clone { |
There was a problem hiding this comment.
Requiring the returned iterator to be Clone is an extra API constraint that may make future refactors harder (e.g., if the implementation changes to an iterator that isn’t trivially cloneable). If cloning isn’t required by callers, consider dropping the + Clone bound.
Suggested change
| pub(crate) fn get_mapped_regions(&self) -> impl Iterator<Item = &MemoryRegion> + Clone { | |
| pub(crate) fn get_mapped_regions(&self) -> impl Iterator<Item = &MemoryRegion> { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add conditional compilation gates and aarch64 stub modules so that cargo build --target aarch64-unknown-linux-gnu -p hyperlight-host succeeds. No aarch64 implementation is added. All aarch64 code paths are stubs that panic at runtime.
There will be some code duplication addition for aarch64 initially, espeically regarding HyperlihtVm and KvmVm and MshvVm. It will make sense to factor some of this out when we figure out more what the code looks like with aarch64 support added