From 771ad835754dc3fc7090c34a64f6896d649943c4 Mon Sep 17 00:00:00 2001 From: joaopeixoto13 Date: Sun, 14 Apr 2024 13:26:48 +0100 Subject: [PATCH] ref(VirtioDeviceActions): include default implementations 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 --- Cargo.lock | 62 +++++++++++++-------------- src/virtio/src/block/virtio/device.rs | 43 ------------------- src/virtio/src/net/vhost/device.rs | 47 -------------------- src/virtio/src/net/virtio/device.rs | 43 ------------------- src/virtio/src/vsock/vhost/device.rs | 47 -------------------- src/virtio/src/vsock/virtio/device.rs | 43 ------------------- 6 files changed, 31 insertions(+), 254 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5dce4fa..61a7e75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -30,9 +30,9 @@ dependencies = [ [[package]] name = "arc-swap" -version = "1.6.0" +version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6" +checksum = "69f7f8c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" [[package]] name = "atty" @@ -47,9 +47,9 @@ dependencies = [ [[package]] name = "autocfg" -version = "1.1.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +checksum = "f1fdabc7756949593fe60f30ec81974b613357de856987752631dea1e3394c80" [[package]] name = "bao-virtio" @@ -73,9 +73,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.4.2" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed570934406eb16438a4e976b1b4500774099c13b8cb96eec99f620f05090ddf" +checksum = "cf4b9d6a944f767f8e5e0db018570623c85f3d925ac718db4e06d0187adb21c1" [[package]] name = "clap" @@ -107,7 +107,7 @@ version = "4.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74351c3392ea1ff6cd2628e0042d268ac2371cb613252ff383b6dfa50d22fa79" dependencies = [ - "bitflags 2.4.2", + "bitflags 2.5.0", "libc", ] @@ -176,15 +176,15 @@ checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" [[package]] name = "log" -version = "0.4.20" +version = "0.4.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5e6163cb8c49088c2c36f57875e58ccd8c87c7427f7fbd50ea6710b2f3f2e8f" +checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" [[package]] name = "memoffset" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a634b1c61a95585bd15607c6ab0c4e5b226e695ff2800ba0cdccddf208c406c" +checksum = "488016bfae457b036d996092f6cb448677611ce4449e970ceaf42695203f218a" dependencies = [ "autocfg", ] @@ -203,18 +203,18 @@ checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" [[package]] name = "proc-macro2" -version = "1.0.78" +version = "1.0.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2422ad645d89c99f8f3e6b88a9fdeca7fabeac836b1002371c4367c8f984aae" +checksum = "e835ff2298f5721608eb1a980ecaee1aef2c132bf95ecc026a11b7bf3c01c02e" dependencies = [ "unicode-ident", ] [[package]] name = "quote" -version = "1.0.35" +version = "1.0.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "291ec9ab5efd934aaf503a6466c5d5251535d108ee747472c3977cc5acc868ef" +checksum = "0fa76aaf39101c457836aec0ce2316dbdc3ab723cdda1c6bd4e6ad4208acaca7" dependencies = [ "proc-macro2", ] @@ -251,18 +251,18 @@ checksum = "92d43fe69e652f3df9bdc2b85b2854a0825b86e4fb76bc44d945137d053639ca" [[package]] name = "serde" -version = "1.0.196" +version = "1.0.197" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "870026e60fa08c69f064aa766c10f10b1d62db9ccd4d0abb206472bee0ce3b32" +checksum = "3fb1c873e1b9b056a4dc4c0c198b24c3ffa059243875552b2bd0933b1aee4ce2" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.196" +version = "1.0.197" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33c85360c95e7d137454dc81d9a4ed2b8efd8fbe19cee57357b32b9771fccb67" +checksum = "7eb0b34b42edc17f6b7cac84a52a1c5f0e1bb2227e997ca9011ea3dd34e8610b" dependencies = [ "proc-macro2", "quote", @@ -289,9 +289,9 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" [[package]] name = "syn" -version = "2.0.49" +version = "2.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "915aea9e586f80826ee59f8453c1101f9d1c4b3964cd2460185ee8e299ada496" +checksum = "44cfb93f38070beee36b3fef7d4f5a16f27751d94b187b666a5cc5e9b0d30687" dependencies = [ "proc-macro2", "quote", @@ -315,18 +315,18 @@ checksum = "23d434d3f8967a09480fb04132ebe0a3e088c173e6d0ee7897abbdf4eab0f8b9" [[package]] name = "thiserror" -version = "1.0.57" +version = "1.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e45bcbe8ed29775f228095caf2cd67af7a4ccf756ebff23a306bf3e8b47b24b" +checksum = "03468839009160513471e86a034bb2c5c0e4baae3b43f79ffc55c4a5427b3297" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.57" +version = "1.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a953cb265bef375dae3de6663da4d3804eee9682ea80d8e2542529b73c531c81" +checksum = "c61f3ba182994efc43764a46c018c347bc492c79f024e705f46567b418f6d4f7" dependencies = [ "proc-macro2", "quote", @@ -350,7 +350,7 @@ name = "vhost" version = "0.10.0" source = "git+https://github.com/joaopeixoto13/vhost?branch=vhost-user-frontend#cf0c49322d8bff5f55591421298c9cb6699279a1" dependencies = [ - "bitflags 2.4.2", + "bitflags 2.5.0", "libc", "vm-memory", "vmm-sys-util", @@ -403,12 +403,12 @@ checksum = "878bcb1b2812a10c30d53b0ed054999de3d98f25ece91fc173973f9c57aaae86" [[package]] name = "virtio-bindings" version = "0.2.2" -source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#8e7c29ce0a606c4beeef2e09356ca58f41a72e4b" +source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#eec91192f87f1107fa9b94524c352f5351747513" [[package]] name = "virtio-blk" version = "0.1.0" -source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#8e7c29ce0a606c4beeef2e09356ca58f41a72e4b" +source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#eec91192f87f1107fa9b94524c352f5351747513" dependencies = [ "log", "virtio-bindings 0.2.2 (git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor)", @@ -421,7 +421,7 @@ dependencies = [ [[package]] name = "virtio-device" version = "0.1.0" -source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#8e7c29ce0a606c4beeef2e09356ca58f41a72e4b" +source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#eec91192f87f1107fa9b94524c352f5351747513" dependencies = [ "log", "virtio-bindings 0.2.2 (git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor)", @@ -432,7 +432,7 @@ dependencies = [ [[package]] name = "virtio-queue" version = "0.11.0" -source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#c608baf80cb8b6aa73257b58346b7e7f27d7e489" +source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#eec91192f87f1107fa9b94524c352f5351747513" dependencies = [ "log", "virtio-bindings 0.2.2 (git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor)", @@ -443,7 +443,7 @@ dependencies = [ [[package]] name = "virtio-vsock" version = "0.5.0" -source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#8e7c29ce0a606c4beeef2e09356ca58f41a72e4b" +source = "git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor#eec91192f87f1107fa9b94524c352f5351747513" dependencies = [ "virtio-bindings 0.2.2 (git+https://github.com/joaopeixoto13/vm-virtio?branch=bao-hypervisor)", "virtio-queue", diff --git a/src/virtio/src/block/virtio/device.rs b/src/virtio/src/block/virtio/device.rs index 1f57866..5940a96 100644 --- a/src/virtio/src/block/virtio/device.rs +++ b/src/virtio/src/block/virtio/device.rs @@ -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; @@ -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 { - // 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. diff --git a/src/virtio/src/net/vhost/device.rs b/src/virtio/src/net/vhost/device.rs index fa95c32..09a5a3b 100644 --- a/src/virtio/src/net/vhost/device.rs +++ b/src/virtio/src/net/vhost/device.rs @@ -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; @@ -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). diff --git a/src/virtio/src/net/virtio/device.rs b/src/virtio/src/net/virtio/device.rs index 3cb0481..da56681 100644 --- a/src/virtio/src/net/virtio/device.rs +++ b/src/virtio/src/net/virtio/device.rs @@ -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::{ @@ -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 { - // 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. diff --git a/src/virtio/src/vsock/vhost/device.rs b/src/virtio/src/vsock/vhost/device.rs index 66e143a..804e2a9 100644 --- a/src/virtio/src/vsock/vhost/device.rs +++ b/src/virtio/src/vsock/vhost/device.rs @@ -6,9 +6,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::vhost_kern::vsock::Vsock; @@ -207,51 +205,6 @@ impl VirtioDeviceActions for VhostVsockDevice { 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). diff --git a/src/virtio/src/vsock/virtio/device.rs b/src/virtio/src/vsock/virtio/device.rs index df9af10..622566f 100644 --- a/src/virtio/src/vsock/virtio/device.rs +++ b/src/virtio/src/vsock/virtio/device.rs @@ -6,10 +6,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_device::{VirtioConfig, VirtioDeviceActions, VirtioDeviceType, VirtioMmioDevice}; use virtio_queue::Queue; @@ -135,46 +132,6 @@ impl VirtioDeviceActions for VirtioVsock { // 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 { - // 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.