-
Notifications
You must be signed in to change notification settings - Fork 43
First-step interrupt injection support #195
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
base: master
Are you sure you want to change the base?
Conversation
- Add VGICv2 support for virtual interrupt controller - Integrate VirtIO drivers for block and network devices - Update VM configuration to include VGICv2 and VirtIO devices - Remove unused timer code and mock timer device
- Add 'clk_ignore_unused' to the first Linux's bootargs so that it will configure but won't disable clocks assigned to the second VM's devices. - Add 'fixed-clock' clock nodes in the second Linux's DTS so that devices which require clocks can be probed properly. Meanwhile, the second Linux cannot mess with these clocks since they are fixed. - Serial port and 1G ethernet are happy with fixed frequency clocks; however MMC controller needs a variable clock to probe a SD card which is hard to implement. So we just boot the second Linux with NFS rootfs.
- Passthrough SATA controller and PHY to VM2 DTS - Keep them in VM1 DTS then VM1 can reset and negotiate properly - Disable SATA drive in VM1's bootargs, so that it won't try accessing the drive and conflict with VM2.
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.
Pull Request Overview
This pull request implements first-step interrupt injection support for the ArceOS hypervisor. The changes introduce Inter-VM Communication (IVC) capabilities, hypercall handling infrastructure, and comprehensive interrupt management for virtualized environments.
- Adds IVC channel management with publish/subscribe mechanisms for VM communication
- Implements hypercall handler supporting IVC operations and SendIPI functionality
- Integrates interrupt injection APIs and timer management for VM scheduling
Reviewed Changes
Copilot reviewed 40 out of 45 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
src/vmm/vcpus.rs | Core vCPU management with hypercall handling and interrupt processing |
src/vmm/ivc.rs | Inter-VM communication channel implementation |
src/vmm/hvc.rs | Hypercall handler for IVC operations |
src/vmm/mod.rs | VMM module integration with new APIs |
src/hal.rs | Hardware abstraction layer with memory and interrupt APIs |
Multiple config files | VM and platform configuration updates |
Comments suppressed due to low confidence (1)
src/vmm/mod.rs:81
- Function name 'with_wm' is unclear and inconsistent with similar functions 'with_vm_and_vcpu'. Consider renaming to 'with_vm' for clarity and consistency.
pub fn with_wm<T>(vm_id: usize, f: impl FnOnce(VMRef) -> T) -> Option<T> {
.ok_or_else(|| ax_err_type!(NotFound))?; | ||
|
||
use std::os::arceos::modules::axipi; | ||
Ok(axipi::send_ipi_event_to_one(pcpu_id as usize, move || { |
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.
Recursive call to 'with_vm_and_vcpu_on_pcpu' inside the IPI handler could lead to infinite recursion or stack overflow if the target vCPU task is not found. Consider adding recursion depth tracking or alternative handling.
Copilot uses AI. Check for mistakes.
/// The base address of the shared memory region in guest physical address of the publisher VM. | ||
/// `None` if the channel has been unpublished (but still has subscribers). | ||
base_gpa: Option<GuestPhysAddr>, | ||
_phatom: core::marker::PhantomData<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.
Typo in field name '_phatom' should be '_phantom'.
_phatom: core::marker::PhantomData<H>, | |
_phantom: core::marker::PhantomData<H>, |
Copilot uses AI. Check for mistakes.
shared_region_base, | ||
shared_region_size, | ||
base_gpa: Some(base_gpa), | ||
_phatom: core::marker::PhantomData, |
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.
Typo in field name '_phatom' should be '_phantom'.
_phatom: core::marker::PhantomData, | |
_phantom: core::marker::PhantomData, |
Copilot uses AI. Check for mistakes.
let vcpu_id = vcpu.id(); | ||
|
||
// boot delay | ||
let boot_delay_sec = (vm_id - 1) * 5; |
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.
Magic number '5' for boot delay calculation should be defined as a named constant to improve maintainability and make the delay configurable.
let boot_delay_sec = (vm_id - 1) * 5; | |
const BOOT_DELAY_MULTIPLIER: u64 = 5; | |
let boot_delay_sec = (vm_id - 1) * BOOT_DELAY_MULTIPLIER; |
Copilot uses AI. Check for mistakes.
} | ||
|
||
// Load Ramdisk image | ||
if let Some(buffer) = vm_imags.ramdisk { |
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.
Variable name 'vm_imags' appears to be a typo, should likely be 'vm_images' based on context.
Copilot uses AI. Check for mistakes.
* fix: update toml crate * Style: cargo fmt
No description provided.