Skip to content

Commit

Permalink
ref(VirtioDeviceActions): include default implementations
Browse files Browse the repository at this point in the history
This patch is based on restructuring the implementations of
some functions of the VirtioDeviceActions trait as it now
includes default methods. In this case, now only the functions
strictly necessary for the device type must be implemented
(e.g. for devices of the vhost-user type, the read_config
function must be implemented if the device has negotiated
the CONFIG protocol feature).

Signed-off-by: joaopeixoto13 <[email protected]>
  • Loading branch information
joaopeixoto13 committed Apr 14, 2024
1 parent 120cd79 commit 771ad83
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 254 deletions.
62 changes: 31 additions & 31 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 0 additions & 43 deletions src/virtio/src/block/virtio/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ use api::device_model::BaoDeviceModel;
use api::error::{Error, Result};
use api::types::DeviceConfig;
use event_manager::{EventManager, MutEventSubscriber};
use log::error;
use std::borrow::{Borrow, BorrowMut};
use std::cmp;
use std::sync::atomic::AtomicU8;
use std::sync::{Arc, Mutex};
use virtio_bindings::virtio_blk::{VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO};
use virtio_blk::stdio_executor::StdIoBackend;
Expand Down Expand Up @@ -190,46 +187,6 @@ impl VirtioDeviceActions for VirtioBlock {
// Not implemented for now.
Ok(())
}

fn read_config(&self, offset: usize, data: &mut [u8]) {
let config_space = &self.common.config.config_space;
let config_len = config_space.len();
if offset >= config_len {
error!("Failed to read from config space");
return;
}

// TODO: Are partial reads ok?
let end = cmp::min(offset.saturating_add(data.len()), config_len);
let read_len = end - offset;
// Cannot fail because the lengths are identical and we do bounds checking beforehand.
data[..read_len].copy_from_slice(&config_space[offset..end])
}

fn write_config(&mut self, offset: usize, data: &[u8]) {
let config_space = &mut self.common.config.config_space;
let config_len = config_space.len();
if offset >= config_len {
error!("Failed to write to config space");
return;
}

// TODO: Are partial writes ok?
let end = cmp::min(offset.saturating_add(data.len()), config_len);
let write_len = end - offset;
// Cannot fail because the lengths are identical and we do bounds checking beforehand.
config_space[offset..end].copy_from_slice(&data[..write_len]);
}

fn negotiate_driver_features(&mut self) {
// Do nothing here since the features are already negotiated.
}

fn interrupt_status(&self) -> &Arc<AtomicU8> {
// Simply return the interrupt status, since the backend (that is inside the VMM) will
// update it.
&self.common.config.interrupt_status
}
}

/// Implement the `VirtioMmioDevice` trait to add VirtIO MMIO support to our device.
Expand Down
47 changes: 0 additions & 47 deletions src/virtio/src/net/vhost/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ use api::device_model::BaoDeviceModel;
use api::error::{Error, Result};
use api::types::DeviceConfig;
use event_manager::{EventManager, MutEventSubscriber};
use log::error;
use std::borrow::{Borrow, BorrowMut};
use std::cmp;
use std::sync::atomic::{AtomicU8, Ordering};
use std::sync::{Arc, Mutex};
use vhost::net::VhostNet as VhostNetBackend;
Expand Down Expand Up @@ -241,51 +239,6 @@ impl VirtioDeviceActions for VhostNet {
Ok(())
}

// This method is called when the driver wants to read information from the device configuration space.
// Since the device configuration space is managed by the device and the device can be implemented in
// different handlers outside of the VMM (vhost or vhost-user) we need to invoke dedicated logic.
fn read_config(&self, offset: usize, data: &mut [u8]) {
let config_space = &self.virtio.config.config_space;
let config_len = config_space.len();
if offset >= config_len {
error!("Failed to read from config space");
return;
}

// TODO: Are partial reads ok?
let end = cmp::min(offset.saturating_add(data.len()), config_len);
let read_len = end - offset;
// Cannot fail because the lengths are identical and we do bounds checking beforehand.
data[..read_len].copy_from_slice(&config_space[offset..end])
}

// This method is called when the driver wants to write information to the device configuration space.
// Since the device configuration space is managed by the device and the device can be implemented in
// different handlers outside of the VMM (vhost or vhost-user) we need to invoke dedicated logic.
fn write_config(&mut self, offset: usize, data: &[u8]) {
let config_space = &mut self.virtio.config.config_space;
let config_len = config_space.len();
if offset >= config_len {
error!("Failed to write to config space");
return;
}

// TODO: Are partial writes ok?
let end = cmp::min(offset.saturating_add(data.len()), config_len);
let write_len = end - offset;
// Cannot fail because the lengths are identical and we do bounds checking beforehand.
config_space[offset..end].copy_from_slice(&data[..write_len]);
}

// This method is called when the driver finishes the negotiation of the device features
// with the frontend device (selecting page 0). This method is crucial when the device handlers are
// implemented outside of the VMM (vhost or vhost-user) as the frontend device needs to negotiate the
// features with the backend device. Otherwise, the device is not prepared to support, for example,
// multiple queues and configuration space reads and writes.
fn negotiate_driver_features(&mut self) {
// Do nothing.
}

// This method is called when the driver needs to read the interrupt status from the device.
// Since it's the frontend device responsibility to manage the interrupt status, we need to invoke
// dedicated logic to update the interrupt status accordingly (Used Buffer Notification or Configuration Change Notification).
Expand Down
43 changes: 0 additions & 43 deletions src/virtio/src/net/virtio/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ use api::device_model::BaoDeviceModel;
use api::error::{Error, Result};
use api::types::DeviceConfig;
use event_manager::{EventManager, MutEventSubscriber};
use log::error;
use std::borrow::{Borrow, BorrowMut};
use std::cmp;
use std::sync::atomic::AtomicU8;
use std::sync::{Arc, Mutex};
use virtio_bindings::virtio_config::VIRTIO_F_IN_ORDER;
use virtio_bindings::virtio_net::{
Expand Down Expand Up @@ -187,46 +184,6 @@ impl VirtioDeviceActions for VirtioNet {
// Not implemented for now.
Ok(())
}

fn read_config(&self, offset: usize, data: &mut [u8]) {
let config_space = &self.common.config.config_space;
let config_len = config_space.len();
if offset >= config_len {
error!("Failed to read from config space");
return;
}

// TODO: Are partial reads ok?
let end = cmp::min(offset.saturating_add(data.len()), config_len);
let read_len = end - offset;
// Cannot fail because the lengths are identical and we do bounds checking beforehand.
data[..read_len].copy_from_slice(&config_space[offset..end])
}

fn write_config(&mut self, offset: usize, data: &[u8]) {
let config_space = &mut self.common.config.config_space;
let config_len = config_space.len();
if offset >= config_len {
error!("Failed to write to config space");
return;
}

// TODO: Are partial writes ok?
let end = cmp::min(offset.saturating_add(data.len()), config_len);
let write_len = end - offset;
// Cannot fail because the lengths are identical and we do bounds checking beforehand.
config_space[offset..end].copy_from_slice(&data[..write_len]);
}

fn negotiate_driver_features(&mut self) {
// Do nothing here since the features are already negotiated.
}

fn interrupt_status(&self) -> &Arc<AtomicU8> {
// Simply return the interrupt status, since the backend (that is inside the VMM) will
// update it.
&self.common.config.interrupt_status
}
}

/// Implement the `VirtioMmioDevice` trait to add VirtIO MMIO support to our device.
Expand Down
Loading

0 comments on commit 771ad83

Please sign in to comment.