From c45ff897c0935b54a5997136adcce35704ab9d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Mon, 10 Jul 2023 12:03:46 +0100 Subject: [PATCH 1/2] vhost: promote get/set_status from vdpa to vhost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although originally introduced for VDPA we also have support for get/set status messages for vhost-user. So lets promote the trait functions to the main VhostBackend traits while providing a default implementation which responds for errors (which the Kernel variant would inherit). As the status bits are common between backends most of the implementation details can be handled within the vhost-user traits. Signed-off-by: Alex Bennée --- v2 - also add impls for DummySlaveReqHandler - move VhostVdpa impls to impl VhostBackend for T --- crates/vhost-user-backend/src/handler.rs | 12 ++++ crates/vhost/src/backend.rs | 66 ++++++++++++++++++- crates/vhost/src/vdpa.rs | 11 ---- crates/vhost/src/vhost_kern/mod.rs | 16 +++++ crates/vhost/src/vhost_kern/vdpa.rs | 16 ----- crates/vhost/src/vhost_user/dummy_slave.rs | 10 +++ crates/vhost/src/vhost_user/master.rs | 21 ++++++ .../vhost/src/vhost_user/slave_req_handler.rs | 26 ++++++++ 8 files changed, 149 insertions(+), 29 deletions(-) diff --git a/crates/vhost-user-backend/src/handler.rs b/crates/vhost-user-backend/src/handler.rs index 2b75232b..3761633a 100644 --- a/crates/vhost-user-backend/src/handler.rs +++ b/crates/vhost-user-backend/src/handler.rs @@ -88,6 +88,8 @@ pub struct VhostUserHandler { atomic_mem: GM, vrings: Vec, worker_threads: Vec>>, + /// VirtIO Device Status field, when using get/set status + status: u8, } // Ensure VhostUserHandler: Clone + Send + Sync + 'static. @@ -147,6 +149,7 @@ where atomic_mem, vrings, worker_threads, + status: 0, }) } } @@ -586,6 +589,15 @@ where Ok(()) } + + fn get_status(&self) -> VhostUserResult { + Ok(self.status) + } + + fn set_status(&mut self, status: u8) -> VhostUserResult<()> { + self.status = status; + Ok(()) + } } impl Drop for VhostUserHandler { diff --git a/crates/vhost/src/backend.rs b/crates/vhost/src/backend.rs index bd4a83b2..76b99261 100644 --- a/crates/vhost/src/backend.rs +++ b/crates/vhost/src/backend.rs @@ -308,6 +308,30 @@ pub trait VhostBackend: std::marker::Sized { /// * `queue_index` - Index of the queue to modify. /// * `fd` - EventFd that will be signaled from guest. fn set_vring_err(&self, queue_index: usize, fd: &EventFd) -> Result<()>; + + /// Set the status. + /// The status bits follow the same definition of the device + /// status defined in virtio-spec. + /// + /// As not all backends can implement this we provide a default + /// implementation that returns an Error. + /// + /// # Arguments + /// * `status` - Status bits to set + fn set_status(&self, _status: u8) -> Result<()> { + Err(Error::InvalidOperation) + } + + /// Get the status. + /// + /// The status bits follow the same definition of the device + /// status defined in virtio-spec. + /// + /// As not all backends can implement this we provide a default + /// implementation that returns an Error. + fn get_status(&self) -> Result { + Err(Error::InvalidOperation) + } } /// An interface for setting up vhost-based backend drivers. @@ -394,6 +418,17 @@ pub trait VhostBackendMut: std::marker::Sized { /// * `queue_index` - Index of the queue to modify. /// * `fd` - EventFd that will be signaled from guest. fn set_vring_err(&mut self, queue_index: usize, fd: &EventFd) -> Result<()>; + + /// Set the status. + /// The status bits follow the same definition of the device status defined in virtio-spec. + /// + /// # Arguments + /// * `status` - Status bits to set + fn set_status(&mut self, status: u8) -> Result<()>; + + /// Get the status. + /// The status bits follow the same definition of the device status defined in virtio-spec. + fn get_status(&self) -> Result; } impl VhostBackend for RwLock { @@ -454,6 +489,14 @@ impl VhostBackend for RwLock { fn set_vring_err(&self, queue_index: usize, fd: &EventFd) -> Result<()> { self.write().unwrap().set_vring_err(queue_index, fd) } + + fn set_status(&self, status: u8) -> Result<()> { + self.write().unwrap().set_status(status) + } + + fn get_status(&self) -> Result { + self.write().unwrap().get_status() + } } impl VhostBackend for RefCell { @@ -512,6 +555,14 @@ impl VhostBackend for RefCell { fn set_vring_err(&self, queue_index: usize, fd: &EventFd) -> Result<()> { self.borrow_mut().set_vring_err(queue_index, fd) } + + fn set_status(&self, status: u8) -> Result<()> { + self.borrow_mut().set_status(status) + } + + fn get_status(&self) -> Result { + self.borrow_mut().get_status() + } } #[cfg(any(test, feature = "test-utils"))] @@ -543,7 +594,9 @@ impl VhostUserMemoryRegionInfo { mod tests { use super::*; - struct MockBackend {} + struct MockBackend { + status: u8, + } impl VhostBackendMut for MockBackend { fn get_features(&mut self) -> Result { @@ -625,11 +678,20 @@ mod tests { assert_eq!(queue_index, 1); Ok(()) } + + fn set_status(&mut self, status: u8) -> Result<()> { + self.status = status; + Ok(()) + } + + fn get_status(&self) -> Result { + Ok(self.status) + } } #[test] fn test_vring_backend_mut() { - let b = RwLock::new(MockBackend {}); + let b = RwLock::new(MockBackend { status: 0 }); assert_eq!(b.get_features().unwrap(), 0x1); b.set_features(0x1).unwrap(); diff --git a/crates/vhost/src/vdpa.rs b/crates/vhost/src/vdpa.rs index 6af01cf3..a890f067 100644 --- a/crates/vhost/src/vdpa.rs +++ b/crates/vhost/src/vdpa.rs @@ -33,17 +33,6 @@ pub trait VhostVdpa: VhostBackend { /// The device ids follow the same definition of the device id defined in virtio-spec. fn get_device_id(&self) -> Result; - /// Get the status. - /// The status bits follow the same definition of the device status defined in virtio-spec. - fn get_status(&self) -> Result; - - /// Set the status. - /// The status bits follow the same definition of the device status defined in virtio-spec. - /// - /// # Arguments - /// * `status` - Status bits to set - fn set_status(&self, status: u8) -> Result<()>; - /// Get the device configuration. /// /// # Arguments diff --git a/crates/vhost/src/vhost_kern/mod.rs b/crates/vhost/src/vhost_kern/mod.rs index 1fa50000..5c85abb6 100644 --- a/crates/vhost/src/vhost_kern/mod.rs +++ b/crates/vhost/src/vhost_kern/mod.rs @@ -291,6 +291,22 @@ impl VhostBackend for T { let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_ERR(), &vring_file) }; ioctl_result(ret, ()) } + + fn get_status(&self) -> Result { + let mut status: u8 = 0; + + // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its + // return value checked. + let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_STATUS(), &mut status) }; + ioctl_result(ret, status) + } + + fn set_status(&self, status: u8) -> Result<()> { + // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its + // return value checked. + let ret = unsafe { ioctl_with_ref(self, VHOST_VDPA_SET_STATUS(), &status) }; + ioctl_result(ret, ()) + } } /// Interface to handle in-kernel backend features. diff --git a/crates/vhost/src/vhost_kern/vdpa.rs b/crates/vhost/src/vhost_kern/vdpa.rs index 65e01231..ecc9e59f 100644 --- a/crates/vhost/src/vhost_kern/vdpa.rs +++ b/crates/vhost/src/vhost_kern/vdpa.rs @@ -101,22 +101,6 @@ impl VhostVdpa for VhostKernVdpa { ioctl_result(ret, device_id) } - fn get_status(&self) -> Result { - let mut status: u8 = 0; - - // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its - // return value checked. - let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_STATUS(), &mut status) }; - ioctl_result(ret, status) - } - - fn set_status(&self, status: u8) -> Result<()> { - // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its - // return value checked. - let ret = unsafe { ioctl_with_ref(self, VHOST_VDPA_SET_STATUS(), &status) }; - ioctl_result(ret, ()) - } - fn get_config(&self, offset: u32, buffer: &mut [u8]) -> Result<()> { let mut config = VhostVdpaConfig::new(buffer.len()) .map_err(|_| Error::IoctlError(IOError::from_raw_os_error(libc::ENOMEM)))?; diff --git a/crates/vhost/src/vhost_user/dummy_slave.rs b/crates/vhost/src/vhost_user/dummy_slave.rs index ae728a02..c11ab280 100644 --- a/crates/vhost/src/vhost_user/dummy_slave.rs +++ b/crates/vhost/src/vhost_user/dummy_slave.rs @@ -17,6 +17,7 @@ pub struct DummySlaveReqHandler { pub features_acked: bool, pub acked_features: u64, pub acked_protocol_features: u64, + pub status: u8, pub queue_num: usize, pub vring_num: [u32; MAX_QUEUE_NUM], pub vring_base: [u32; MAX_QUEUE_NUM], @@ -291,4 +292,13 @@ impl VhostUserSlaveReqHandlerMut for DummySlaveReqHandler { fn remove_mem_region(&mut self, _region: &VhostUserSingleMemoryRegion) -> Result<()> { Ok(()) } + + fn get_status(&self) -> Result { + Ok(self.status) + } + + fn set_status(&mut self, status: u8) -> Result<()> { + self.status = status; + Ok(()) + } } diff --git a/crates/vhost/src/vhost_user/master.rs b/crates/vhost/src/vhost_user/master.rs index feeb984d..e4819480 100644 --- a/crates/vhost/src/vhost_user/master.rs +++ b/crates/vhost/src/vhost_user/master.rs @@ -3,6 +3,7 @@ //! Traits and Struct for vhost-user master. +use std::convert::TryFrom; use std::fs::File; use std::mem; use std::os::unix::io::{AsRawFd, RawFd}; @@ -330,6 +331,26 @@ impl VhostBackend for Master { let hdr = node.send_fd_for_vring(MasterReq::SET_VRING_ERR, queue_index, fd.as_raw_fd())?; node.wait_for_ack(&hdr).map_err(|e| e.into()) } + + /// Set the status at the remote end (if supported) + fn set_status(&self, status: u8) -> Result<()> { + let mut node = self.node(); + // depends on VhostUserProtocolFeatures::STATUS + node.check_proto_feature(VhostUserProtocolFeatures::STATUS)?; + let val = VhostUserU64::new(status.into()); + node.send_request_with_body(MasterReq::SET_STATUS, &val, None)?; + Ok(()) + } + + /// Get the status from the remote end (if supported) + fn get_status(&self) -> Result { + let mut node = self.node(); + // depends on VhostUserProtocolFeatures::STATUS + node.check_proto_feature(VhostUserProtocolFeatures::STATUS)?; + let hdr = node.send_request_header(MasterReq::GET_STATUS, None)?; + let reply = node.recv_reply::(&hdr)?; + u8::try_from(reply.value).or(error_code(VhostUserError::InvalidParam)) + } } impl VhostUserMaster for Master { diff --git a/crates/vhost/src/vhost_user/slave_req_handler.rs b/crates/vhost/src/vhost_user/slave_req_handler.rs index e9983398..95ba0e75 100644 --- a/crates/vhost/src/vhost_user/slave_req_handler.rs +++ b/crates/vhost/src/vhost_user/slave_req_handler.rs @@ -1,6 +1,7 @@ // Copyright (C) 2019 Alibaba Cloud Computing. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +use std::convert::TryFrom; use std::fs::File; use std::mem; use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; @@ -70,6 +71,8 @@ pub trait VhostUserSlaveReqHandler { fn get_max_mem_slots(&self) -> Result; fn add_mem_region(&self, region: &VhostUserSingleMemoryRegion, fd: File) -> Result<()>; fn remove_mem_region(&self, region: &VhostUserSingleMemoryRegion) -> Result<()>; + fn get_status(&self) -> Result; + fn set_status(&self, status: u8) -> Result<()>; } /// Services provided to the master by the slave without interior mutability. @@ -118,6 +121,8 @@ pub trait VhostUserSlaveReqHandlerMut { fn get_max_mem_slots(&mut self) -> Result; fn add_mem_region(&mut self, region: &VhostUserSingleMemoryRegion, fd: File) -> Result<()>; fn remove_mem_region(&mut self, region: &VhostUserSingleMemoryRegion) -> Result<()>; + fn get_status(&self) -> Result; + fn set_status(&mut self, status: u8) -> Result<()>; } impl VhostUserSlaveReqHandler for Mutex { @@ -226,6 +231,14 @@ impl VhostUserSlaveReqHandler for Mutex { fn remove_mem_region(&self, region: &VhostUserSingleMemoryRegion) -> Result<()> { self.lock().unwrap().remove_mem_region(region) } + + fn get_status(&self) -> Result { + self.lock().unwrap().get_status() + } + + fn set_status(&self, status: u8) -> Result<()> { + self.lock().unwrap().set_status(status) + } } /// Server to handle service requests from masters from the master communication channel. @@ -519,6 +532,19 @@ impl SlaveReqHandler { let res = self.backend.remove_mem_region(&msg); self.send_ack_message(&hdr, res)?; } + Ok(MasterReq::SET_STATUS) => { + self.check_proto_feature(VhostUserProtocolFeatures::STATUS)?; + self.check_request_size(&hdr, size, hdr.get_size() as usize)?; + let msg = self.extract_request_body::(&hdr, size, &buf)?; + let status = u8::try_from(msg.value).or(Err(Error::InvalidParam))?; + self.backend.set_status(status)?; + } + Ok(MasterReq::GET_STATUS) => { + self.check_proto_feature(VhostUserProtocolFeatures::STATUS)?; + let num = self.backend.get_status()?; + let msg = VhostUserU64::new(num.into()); + self.send_reply_message(&hdr, &msg)?; + } _ => { return Err(Error::InvalidMessage); } From be83316b120951554d106163a7fb5fa21c563f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 6 Jul 2023 16:51:15 +0100 Subject: [PATCH 2/2] vhost: add vhost-user GET_BACKEND_SPECS handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This seems like an awful lot of boilerplate (without even any test) to implement a single message. Signed-off-by: Alex Bennée --- crates/vhost-user-backend/src/backend.rs | 24 ++++++++++++- crates/vhost-user-backend/src/handler.rs | 4 +++ .../tests/vhost-user-server.rs | 7 +++- crates/vhost/src/vhost_user/dummy_slave.rs | 4 +++ crates/vhost/src/vhost_user/message.rs | 35 ++++++++++++++++++- .../vhost/src/vhost_user/slave_req_handler.rs | 11 ++++++ 6 files changed, 82 insertions(+), 3 deletions(-) diff --git a/crates/vhost-user-backend/src/backend.rs b/crates/vhost-user-backend/src/backend.rs index 43ab7b95..3eff5627 100644 --- a/crates/vhost-user-backend/src/backend.rs +++ b/crates/vhost-user-backend/src/backend.rs @@ -22,7 +22,7 @@ use std::io::Result; use std::ops::Deref; use std::sync::{Arc, Mutex, RwLock}; -use vhost::vhost_user::message::VhostUserProtocolFeatures; +use vhost::vhost_user::message::{VhostUserBackendSpecs, VhostUserProtocolFeatures}; use vhost::vhost_user::Slave; use vm_memory::bitmap::Bitmap; use vmm_sys_util::epoll::EventSet; @@ -54,6 +54,9 @@ where /// Get available vhost protocol features. fn protocol_features(&self) -> VhostUserProtocolFeatures; + /// Get the backends specs + fn specs(&self) -> VhostUserBackendSpecs; + /// Enable or disable the virtio EVENT_IDX feature fn set_event_idx(&self, enabled: bool); @@ -135,6 +138,9 @@ where /// Get available vhost protocol features. fn protocol_features(&self) -> VhostUserProtocolFeatures; + /// Get specs + fn specs(&self) -> VhostUserBackendSpecs; + /// Enable or disable the virtio EVENT_IDX feature fn set_event_idx(&mut self, enabled: bool); @@ -220,6 +226,10 @@ where self.deref().protocol_features() } + fn specs(&self) -> VhostUserBackendSpecs { + self.deref().specs() + } + fn set_event_idx(&self, enabled: bool) { self.deref().set_event_idx(enabled) } @@ -285,6 +295,10 @@ where self.lock().unwrap().protocol_features() } + fn specs(&self) -> VhostUserBackendSpecs { + self.lock().unwrap().specs() + } + fn set_event_idx(&self, enabled: bool) { self.lock().unwrap().set_event_idx(enabled) } @@ -351,6 +365,10 @@ where self.read().unwrap().protocol_features() } + fn specs(&self) -> VhostUserBackendSpecs { + self.read().unwrap().specs() + } + fn set_event_idx(&self, enabled: bool) { self.write().unwrap().set_event_idx(enabled) } @@ -436,6 +454,10 @@ pub mod tests { VhostUserProtocolFeatures::all() } + fn specs(&self) -> VhostUserBackendSpecs { + VhostUserBackendSpecs::new(2, 32, 4, 8) + } + fn set_event_idx(&mut self, enabled: bool) { self.event_idx = enabled; } diff --git a/crates/vhost-user-backend/src/handler.rs b/crates/vhost-user-backend/src/handler.rs index 3761633a..b330d764 100644 --- a/crates/vhost-user-backend/src/handler.rs +++ b/crates/vhost-user-backend/src/handler.rs @@ -455,6 +455,10 @@ where Ok(()) } + fn specs(&self) -> VhostUserResult { + Ok(self.backend.specs()) + } + fn get_queue_num(&mut self) -> VhostUserResult { Ok(self.num_queues as u64) } diff --git a/crates/vhost-user-backend/tests/vhost-user-server.rs b/crates/vhost-user-backend/tests/vhost-user-server.rs index f6fdea7f..8e1e1a1e 100644 --- a/crates/vhost-user-backend/tests/vhost-user-server.rs +++ b/crates/vhost-user-backend/tests/vhost-user-server.rs @@ -8,7 +8,8 @@ use std::sync::{Arc, Barrier, Mutex}; use std::thread; use vhost::vhost_user::message::{ - VhostUserConfigFlags, VhostUserHeaderFlag, VhostUserInflight, VhostUserProtocolFeatures, + VhostUserBackendSpecs, VhostUserConfigFlags, VhostUserHeaderFlag, VhostUserInflight, + VhostUserProtocolFeatures, }; use vhost::vhost_user::{Listener, Master, Slave, VhostUserMaster}; use vhost::{VhostBackend, VhostUserMemoryRegionInfo, VringConfigData}; @@ -56,6 +57,10 @@ impl VhostUserBackendMut for MockVhostBackend { VhostUserProtocolFeatures::all() } + fn specs(&self) -> VhostUserBackendSpecs { + VhostUserBackendSpecs::new(1, 32, 4, 8) + } + fn set_event_idx(&mut self, enabled: bool) { self.event_idx = enabled; } diff --git a/crates/vhost/src/vhost_user/dummy_slave.rs b/crates/vhost/src/vhost_user/dummy_slave.rs index c11ab280..370c80b5 100644 --- a/crates/vhost/src/vhost_user/dummy_slave.rs +++ b/crates/vhost/src/vhost_user/dummy_slave.rs @@ -204,6 +204,10 @@ impl VhostUserSlaveReqHandlerMut for DummySlaveReqHandler { Ok(()) } + fn specs(&self) -> Result { + Ok(VhostUserBackendSpecs::new(1, 2, 3, 4)) + } + fn get_queue_num(&mut self) -> Result { Ok(MAX_QUEUE_NUM as u64) } diff --git a/crates/vhost/src/vhost_user/message.rs b/crates/vhost/src/vhost_user/message.rs index bbd8eb9e..dee7c1b2 100644 --- a/crates/vhost/src/vhost_user/message.rs +++ b/crates/vhost/src/vhost_user/message.rs @@ -146,8 +146,10 @@ pub enum MasterReq { /// Query the backend for its device status as defined in the VIRTIO /// specification. GET_STATUS = 40, + /// Query the backend for its emulation specification + GET_BACKEND_SPECS = 41, /// Upper bound of valid commands. - MAX_CMD = 41, + MAX_CMD = 42, } impl From for u32 { @@ -429,6 +431,8 @@ bitflags! { const STATUS = 0x0001_0000; /// Support Xen mmap. const XEN_MMAP = 0x0002_0000; + /// Support GET_BACKEND_SPECS; + const STANDALONE = 0x0004_0000; } } @@ -667,6 +671,35 @@ impl VhostUserSingleMemoryRegion { unsafe impl ByteValued for VhostUserSingleMemoryRegion {} impl VhostUserMsgValidator for VhostUserSingleMemoryRegion {} +/// Supported specs of the backend. +#[repr(C)] +#[derive(Default, Clone, Copy)] +pub struct VhostUserBackendSpecs { + /// VirtIO device ID + device_id: u32, + /// Size of config space + config_size: u32, + /// Minimum number of VQs + min_vqs: u32, + /// Maximum number of VQs + max_vqs: u32, +} + +impl VhostUserBackendSpecs { + /// Create a new instance. + pub fn new(device_id: u32, config_size: u32, min_vqs: u32, max_vqs: u32) -> Self { + VhostUserBackendSpecs { + device_id, + config_size, + min_vqs, + max_vqs, + } + } +} + +// SAFETY: Safe because all fields of VhostUserBackendSpecs are POD +unsafe impl ByteValued for VhostUserBackendSpecs {} + /// Vring state descriptor. #[repr(packed)] #[derive(Copy, Clone, Default)] diff --git a/crates/vhost/src/vhost_user/slave_req_handler.rs b/crates/vhost/src/vhost_user/slave_req_handler.rs index 95ba0e75..66da6cb7 100644 --- a/crates/vhost/src/vhost_user/slave_req_handler.rs +++ b/crates/vhost/src/vhost_user/slave_req_handler.rs @@ -61,6 +61,7 @@ pub trait VhostUserSlaveReqHandler { fn get_protocol_features(&self) -> Result; fn set_protocol_features(&self, features: u64) -> Result<()>; + fn specs(&self) -> Result; fn get_queue_num(&self) -> Result; fn set_vring_enable(&self, index: u32, enable: bool) -> Result<()>; fn get_config(&self, offset: u32, size: u32, flags: VhostUserConfigFlags) -> Result>; @@ -103,6 +104,7 @@ pub trait VhostUserSlaveReqHandlerMut { fn get_protocol_features(&mut self) -> Result; fn set_protocol_features(&mut self, features: u64) -> Result<()>; + fn specs(&self) -> Result; fn get_queue_num(&mut self) -> Result; fn set_vring_enable(&mut self, index: u32, enable: bool) -> Result<()>; fn get_config( @@ -192,6 +194,10 @@ impl VhostUserSlaveReqHandler for Mutex { self.lock().unwrap().set_protocol_features(features) } + fn specs(&self) -> Result { + self.lock().unwrap().specs() + } + fn get_queue_num(&self) -> Result { self.lock().unwrap().get_queue_num() } @@ -545,6 +551,11 @@ impl SlaveReqHandler { let msg = VhostUserU64::new(num.into()); self.send_reply_message(&hdr, &msg)?; } + Ok(MasterReq::GET_BACKEND_SPECS) => { + self.check_proto_feature(VhostUserProtocolFeatures::STANDALONE)?; + let msg = self.backend.specs()?; + self.send_reply_message(&hdr, &msg)?; + } _ => { return Err(Error::InvalidMessage); }