Skip to content

refine: Refine API #93

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

Merged
junyu0312 merged 2 commits into from
Mar 5, 2026
Merged

refine: Refine API #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da5ddd7
refine: Refine virtio API
junyu0312 Mar 5, 2026
4248fdc
refine: Rename VirtQueue
junyu0312 Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/vm-device/src/device/virtio/virtio_balloon_traditional.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::sync::Arc;
use std::sync::Mutex;

use tokio::sync::Notify;
use vm_core::arch::irq::InterruptController;
Expand Down Expand Up @@ -81,7 +82,7 @@ where
&self,
queue: usize,
notifier: Arc<Notify>,
dev: VirtioDev<C, Self>,
dev: Arc<Mutex<VirtioDev<C, Self>>>,
) -> Option<VirtqueueHandler<C, Self>> {
match VirtioBalloonTranditionalVirtqueue::from_repr(queue) {
Some(virtq) => match virtq {
Expand Down
3 changes: 2 additions & 1 deletion crates/vm-device/src/device/virtio/virtio_blk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::sync::Arc;
use std::sync::Mutex;

use tokio::sync::Notify;
use vm_core::arch::irq::InterruptController;
Expand Down Expand Up @@ -110,7 +111,7 @@ where
&self,
queue_sel: usize,
notifier: Arc<Notify>,
dev: VirtioDev<C, Self>,
dev: Arc<Mutex<VirtioDev<C, Self>>>,
) -> Option<VirtqueueHandler<C, Self>> {
if queue_sel != 0 {
return None;
Expand Down
4 changes: 2 additions & 2 deletions crates/vm-device/src/device/virtio/virtio_mmio_kbd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use vm_virtio::types::device_features::VIRTIO_F_VERSION_1;
use vm_virtio::types::driver_features::DriverFeatures;
use vm_virtio::types::interrupt::InterruptStatus;
use vm_virtio::types::status::Status;
use vm_virtio::types::virtqueue::split_virtqueue::VirtQueue;
use vm_virtio::types::virtqueue::split_virtqueue::Virtqueue;

fn bit_to_index(bit: usize) -> (usize, usize) {
(bit / 8, bit % 8)
Expand Down Expand Up @@ -157,7 +157,7 @@ struct VirtIOMmioKbdInternal<const IRQ: u32, C: MemoryContainer> {
driver_features: DriverFeatures,
driver_feature_sel: Option<u32>,
queue_sel: Option<u32>,
virtqueues: [VirtQueue<QUEUE_SIZE_MAX>; VIRTIO_INPUT_VIRT_QUEUE as usize],
virtqueues: [Virtqueue<QUEUE_SIZE_MAX>; VIRTIO_INPUT_VIRT_QUEUE as usize],
interrupt_status: InterruptStatus,
status: Status,
config_generation: u32,
Expand Down
9 changes: 7 additions & 2 deletions crates/vm-machine/src/device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use vm_mm::allocator::MemoryContainer;
use vm_mm::manager::MemoryAddressSpace;
use vm_pci::root_complex::mmio::PciRootComplexMmio;
use vm_virtio::device::pci::VirtioPciDevice;
use vm_virtio::transport::VirtioDev;

use crate::error::Error;

Expand Down Expand Up @@ -74,7 +75,7 @@ impl InitDevice for DeviceManager {

{
let virtio_mmio_blk = VirtioMmioBlkDevice::new(
VirtioBlkDevice::new(2, irq_chip.clone(), mm.clone()),
VirtioDev::new(VirtioBlkDevice::new(2, irq_chip.clone(), mm.clone())),
MmioRange {
start: 0x0900_1000,
len: 0x1000,
Expand All @@ -89,7 +90,11 @@ impl InitDevice for DeviceManager {
Device::VirtioMmioBalloon => {
// TODO: use mmio allocator
let virtio_mmio_balloon = VirtioMmioBalloonDevice::new(
VirtioBalloonTranditional::new(3, irq_chip.clone(), mm.clone()),
VirtioDev::new(VirtioBalloonTranditional::new(
3,
irq_chip.clone(),
mm.clone(),
)),
MmioRange {
start: 0x0900_2000,
len: 0x1000,
Expand Down
10 changes: 5 additions & 5 deletions crates/vm-virtio/src/device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::sync::Arc;
use std::sync::Mutex;

use tokio::sync::Notify;
use vm_core::arch::irq::InterruptController;
Expand All @@ -7,23 +8,22 @@ use vm_mm::manager::MemoryAddressSpace;

use crate::result::Result;
use crate::transport::VirtioDev;
use crate::transport::VirtioDevInternal;
use crate::types::interrupt_status::InterruptStatus;
use crate::virt_queue::virtq_desc_table::VirtqDescTableRef;
use crate::virtqueue::virtq_desc_table::VirtqDescTableRef;

pub mod blk;
pub mod pci;

pub type VirtqueueHandlerFn<C, D> = Box<
dyn Fn(&MemoryAddressSpace<C>, &mut VirtioDevInternal<C, D>, &VirtqDescTableRef, u16) -> u32
dyn Fn(&MemoryAddressSpace<C>, &mut VirtioDev<C, D>, &VirtqDescTableRef, u16) -> u32
+ Send
+ Sync,
>;

pub struct VirtqueueHandler<C, D> {
pub queue_sel: usize,
pub notifier: Arc<Notify>,
pub dev: VirtioDev<C, D>,
pub dev: Arc<Mutex<VirtioDev<C, D>>>,
pub mm: Arc<MemoryAddressSpace<C>>,
pub irq_chip: Arc<dyn InterruptController>,
pub irq_line: u32,
Expand Down Expand Up @@ -114,7 +114,7 @@ pub trait VirtioDevice<C>: Sized + Send + Sync + 'static {
&self,
queue: usize,
notifier: Arc<Notify>,
dev: VirtioDev<C, Self>,
dev: Arc<Mutex<VirtioDev<C, Self>>>,
) -> Option<VirtqueueHandler<C, Self>>;

fn read_config(&self, offset: usize, len: usize, buf: &mut [u8]) -> Result<()>;
Expand Down
5 changes: 4 additions & 1 deletion crates/vm-virtio/src/device/pci.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use vm_pci::device::function::type0::Type0Function;
use vm_pci::device::pci_device::PciDevice;

use crate::device::VirtioDevice;
use crate::transport::VirtioDev;
use crate::transport::pci::VirtioPciFunction;

pub trait VirtioPciDevice<C>: VirtioDevice<C>
Expand All @@ -14,7 +15,9 @@ where
const IRQ_PIN: u8;

fn into_pci_device(self) -> PciDevice {
let virtio_function = VirtioPciFunction::<C, _> { dev: self.into() };
let virtio_function = VirtioPciFunction::<C, _> {
dev: VirtioDev::new(self),
};
let function = Type0Function::new(virtio_function).unwrap();
PciDevice::from_single_function(Box::new(function))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/vm-virtio/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ pub mod device;
pub mod result;
pub mod transport;
pub mod types;
pub mod virt_queue;
pub mod virtqueue;
Copy link

@coderabbitai coderabbitai bot Mar 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if virtqueue module exists in any expected form
echo "Checking for virtqueue module files:"
fd -t f -e rs "virtqueue" crates/vm-virtio/src/
fd -t d "virtqueue" crates/vm-virtio/src/

echo ""
echo "Checking for old virt_queue module files:"
fd -t f -e rs "virt_queue" crates/vm-virtio/src/
fd -t d "virt_queue" crates/vm-virtio/src/

echo ""
echo "Listing vm-virtio src directory structure:"
ls -la crates/vm-virtio/src/

Repository: junyu0312/rust-vm

Length of output: 770

🏁 Script executed:

cat -n crates/vm-virtio/src/lib.rs

Repository: junyu0312/rust-vm

Length of output: 214

🏁 Script executed:

# Check if transport.rs references virtqueue
rg "virtqueue" crates/vm-virtio/src/ -n

Repository: junyu0312/rust-vm

Length of output: 3824

Build failure: module virtqueue not found.

The module declaration on line 7 exists but the corresponding source file (virtqueue.rs or virtqueue/mod.rs) is missing. This breaks the build since transport.rs and device.rs both import from crate::virtqueue. Include the virtqueue module implementation in this PR.

🧰 Tools
🪛 GitHub Actions: CI

[error] 7-7: file not found for module virtqueue.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/vm-virtio/src/lib.rs` at line 7, The project declares pub mod
virtqueue but the implementation file is missing; add the missing module source
(either virtqueue.rs or a virtqueue/mod.rs) implementing the symbols expected by
transport.rs and device.rs (e.g., the VirtQueue struct/functions and any
traits/types they import from crate::virtqueue), ensuring the module exposes the
same public API used by transport:: and device:: (match function/struct names
and signatures) and re-run the build to confirm imports resolve.

58 changes: 20 additions & 38 deletions crates/vm-virtio/src/transport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::marker::PhantomData;
use std::sync::Arc;
use std::sync::LockResult;
use std::sync::Mutex;
use std::sync::MutexGuard;

use bitflags::Flags;
use tokio::sync::Notify;
Expand All @@ -14,32 +12,34 @@ use crate::result::Result;
use crate::transport::control_register::ControlRegister;
use crate::types::interrupt_status::InterruptStatus;
use crate::types::status::Status;
use crate::virt_queue::VirtQueue;
use crate::virtqueue::Virtqueue;

pub mod control_register;
pub mod mmio;
pub mod pci;

pub struct VirtioDev<C, D>(Arc<Mutex<VirtioDevInternal<C, D>>>);
pub struct VirtioDev<C, D> {
device: D,

impl<C, D> Clone for VirtioDev<C, D> {
fn clone(&self) -> Self {
Self(self.0.clone())
}
}
device_feature_sel: Option<u32>,
driver_features: u64,
driver_feature_sel: Option<u32>,
queue_sel: Option<u32>,
virtqueues: Vec<Option<Virtqueue>>,
virtqueue_notifiers: Vec<Option<Arc<Notify>>>,
interrupt_status: InterruptStatus,
status: Status,
config_generation: u32,

impl<C, D> VirtioDev<C, D> {
pub fn lock(&self) -> LockResult<MutexGuard<'_, VirtioDevInternal<C, D>>> {
self.0.lock()
}
_mark: PhantomData<C>,
}

impl<C, D> From<D> for VirtioDev<C, D>
impl<C, D> VirtioDev<C, D>
where
C: MemoryContainer,
D: VirtioDevice<C>,
{
fn from(device: D) -> Self {
pub fn new(device: D) -> Arc<Mutex<Self>> {
let virtqueues_size_max = device.virtqueues_size_max();

let virtqueue_notifiers = virtqueues_size_max
Expand All @@ -49,10 +49,10 @@ where

let virtqueues = virtqueues_size_max
.iter()
.map(|size_max| size_max.map(VirtQueue::new))
.map(|size_max| size_max.map(Virtqueue::new))
.collect();

let internal = Arc::new(Mutex::new(VirtioDevInternal {
let virtio_dev = Arc::new(Mutex::new(VirtioDev {
device,
device_feature_sel: Default::default(),
driver_features: Default::default(),
Expand All @@ -66,8 +66,6 @@ where
_mark: PhantomData,
}));

let virtio_dev = VirtioDev(internal);

{
let dev = virtio_dev.lock().unwrap();

Expand All @@ -94,23 +92,7 @@ where
}
}

pub struct VirtioDevInternal<C, D> {
device: D,

device_feature_sel: Option<u32>,
driver_features: u64,
driver_feature_sel: Option<u32>,
queue_sel: Option<u32>,
virtqueues: Vec<Option<VirtQueue>>,
virtqueue_notifiers: Vec<Option<Arc<Notify>>>,
interrupt_status: InterruptStatus,
status: Status,
config_generation: u32,

_mark: PhantomData<C>,
}

impl<C, D> VirtioDevInternal<C, D>
impl<C, D> VirtioDev<C, D>
where
D: VirtioDevice<C>,
{
Expand Down Expand Up @@ -322,11 +304,11 @@ where
self.device.write_config(offset, len, buf)
}

pub fn get_virtqueue(&self, queue_sel: usize) -> Option<&VirtQueue> {
pub fn get_virtqueue(&self, queue_sel: usize) -> Option<&Virtqueue> {
self.virtqueues.get(queue_sel).unwrap().as_ref()
}

pub fn get_virtqueue_mut(&mut self, queue_sel: usize) -> Option<&mut VirtQueue> {
pub fn get_virtqueue_mut(&mut self, queue_sel: usize) -> Option<&mut Virtqueue> {
self.virtqueues.get_mut(queue_sel).unwrap().as_mut()
Comment on lines +307 to 312
Copy link

@coderabbitai coderabbitai bot Mar 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid panicking in get_virtqueue* when the API returns Option.

Both methods currently unwrap() the vector slot lookup, so out-of-range input panics instead of returning None.

Suggested patch 
     pub fn get_virtqueue(&self, queue_sel: usize) -> Option<&Virtqueue> {
-        self.virtqueues.get(queue_sel).unwrap().as_ref()
+        self.virtqueues.get(queue_sel).and_then(|q| q.as_ref())
     }

     pub fn get_virtqueue_mut(&mut self, queue_sel: usize) -> Option<&mut Virtqueue> {
-        self.virtqueues.get_mut(queue_sel).unwrap().as_mut()
+        self.virtqueues.get_mut(queue_sel).and_then(|q| q.as_mut())
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn get_virtqueue(&self, queue_sel: usize) -> Option<&Virtqueue> {
self.virtqueues.get(queue_sel).unwrap().as_ref()
}
pub fn get_virtqueue_mut(&mut self, queue_sel: usize) -> Option<&mut VirtQueue> {
pub fn get_virtqueue_mut(&mut self, queue_sel: usize) -> Option<&mut Virtqueue> {
self.virtqueues.get_mut(queue_sel).unwrap().as_mut()
pub fn get_virtqueue(&self, queue_sel: usize) -> Option<&Virtqueue> {
self.virtqueues.get(queue_sel).and_then(|q| q.as_ref())
}
pub fn get_virtqueue_mut(&mut self, queue_sel: usize) -> Option<&mut Virtqueue> {
self.virtqueues.get_mut(queue_sel).and_then(|q| q.as_mut())
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/vm-virtio/src/transport.rs` around lines 307 - 312, The methods
get_virtqueue and get_virtqueue_mut currently call unwrap() on
self.virtqueues.get(...) which causes a panic for out-of-range indexes; change
them to propagate the Option from the vector lookup instead of unwrapping—use
the get/get_mut result and map/and_then to convert the Option<Option<Virtqueue>>
slot into Option<&Virtqueue> (for get_virtqueue) and Option<&mut Virtqueue> (for
get_virtqueue_mut) by calling as_ref()/as_mut() on the inner option; update the
bodies of get_virtqueue and get_virtqueue_mut to return None for out-of-range
indexes rather than panicking.

}

Expand Down
25 changes: 15 additions & 10 deletions crates/vm-virtio/src/transport/mmio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use std::sync::Arc;
use std::sync::Mutex;

use vm_core::device::Device;
use vm_core::device::mmio::MmioRange;
use vm_core::device::mmio::mmio_device::MmioDevice;
Expand All @@ -14,6 +17,9 @@ mod control_register;
const CONFIGURATION_SPACE_OFFSET: usize = 0x100;

mod handler {
use std::sync::Arc;
use std::sync::Mutex;

use tracing::debug;
use tracing::error;
use tracing::warn;
Expand All @@ -24,29 +30,28 @@ mod handler {
use crate::device::VirtioDevice;
use crate::result::Result as VirtioResult;
use crate::transport::VirtioDev;
use crate::transport::VirtioDevInternal;
use crate::transport::control_register::ControlRegister;
use crate::transport::mmio::CONFIGURATION_SPACE_OFFSET;
use crate::transport::mmio::control_register::MmioControlRegister;
use crate::types::interrupt_status::InterruptStatus;

pub struct Handler<C, D> {
mmio_range: MmioRange,
transport: VirtioDev<C, D>,
transport: Arc<Mutex<VirtioDev<C, D>>>,
}

impl<C, D> Handler<C, D>
where
D: VirtioDevice<C>,
{
pub fn new(mmio_range: MmioRange, transport: VirtioDev<C, D>) -> Self {
pub fn new(mmio_range: MmioRange, transport: Arc<Mutex<VirtioDev<C, D>>>) -> Self {
Handler {
mmio_range,
transport,
}
}

fn read_reg(&self, transport: &VirtioDevInternal<C, D>, reg: MmioControlRegister) -> u32 {
fn read_reg(&self, transport: &VirtioDev<C, D>, reg: MmioControlRegister) -> u32 {
match reg {
MmioControlRegister::MagicValue => u32::from_le_bytes(*b"virt"),
MmioControlRegister::Version => 0x2,
Expand All @@ -73,7 +78,7 @@ mod handler {

fn write_reg(
&self,
transport: &mut VirtioDevInternal<C, D>,
transport: &mut VirtioDev<C, D>,
reg: MmioControlRegister,
val: u32,
) -> VirtioResult<()> {
Expand Down Expand Up @@ -218,19 +223,17 @@ mod handler {

pub struct VirtioMmioTransport<C, D> {
mmio_range: MmioRange,
irq: Option<u32>,
dev: VirtioDev<C, D>,
dev: Arc<Mutex<VirtioDev<C, D>>>,
}

impl<C, D> VirtioMmioTransport<C, D>
where
C: MemoryContainer,
D: VirtioDevice<C>,
{
pub fn new(device: D, mmio_range: MmioRange) -> Self {
pub fn new(device: Arc<Mutex<VirtioDev<C, D>>>, mmio_range: MmioRange) -> Self {
VirtioMmioTransport {
mmio_range,
irq: device.irq(),
dev: device.into(),
}
}
Expand Down Expand Up @@ -260,11 +263,13 @@ where
}

fn generate_dt(&self, fdt: &mut FdtWriter) -> Result<(), vm_fdt::Error> {
let dev = self.dev.lock().unwrap();

let node = fdt.begin_node(&format!("{}@{:x}", self.name(), self.mmio_range.start))?;

fdt.property_string("compatible", "virtio,mmio")?;
fdt.property_array_u64("reg", &[self.mmio_range.start, self.mmio_range.len as u64])?;
if let Some(_irq) = self.irq {
if let Some(_irq) = dev.device.irq() {
#[cfg(target_arch = "aarch64")]
{
use vm_core::arch::aarch64::irq::GIC_SPI;
Expand Down
Loading
Loading