diff --git a/vhost/CHANGELOG.md b/vhost/CHANGELOG.md index aa13a7be..78b562e2 100644 --- a/vhost/CHANGELOG.md +++ b/vhost/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Added +- [[#301]](https://github.com/rust-vmm/vhost/pull/301) Add support for packed virtqueues in vhost-user and vhost-kern backends ### Changed ### Deprecated ### Fixed diff --git a/vhost/src/backend.rs b/vhost/src/backend.rs index bd4a83b2..2b7f8930 100644 --- a/vhost/src/backend.rs +++ b/vhost/src/backend.rs @@ -24,7 +24,27 @@ use super::{Error, Result}; /// Maximum number of memory regions supported. pub const VHOST_MAX_MEMORY_REGIONS: usize = 255; +/// Vring flags used in vhost backends. +pub mod vring_flags { + /// Support log of vring operations. + /// Modifications to "used" vring should be logged. + pub const VHOST_VRING_F_LOG: u32 = 0x1; + /// Indicates packed virtqueue format. + /// When set, the vring uses packed layout instead of split layout. + pub const VHOST_VRING_F_PACKED: u32 = 0x2; +} + /// Vring configuration data. +/// +/// For split virtqueues (traditional layout): +/// - `desc_table_addr`: Descriptor table address +/// - `used_ring_addr`: Used ring buffer address +/// - `avail_ring_addr`: Available ring buffer address +/// +/// For packed virtqueues (when VHOST_VRING_F_PACKED flag is set): +/// - `desc_table_addr`: Packed descriptor ring address +/// - `used_ring_addr`: Driver event suppression structure address +/// - `avail_ring_addr`: Device event suppression structure address #[derive(Default, Clone, Copy)] pub struct VringConfigData { /// Maximum queue size supported by the driver. @@ -33,11 +53,11 @@ pub struct VringConfigData { pub queue_size: u16, /// Bitmask of vring flags. pub flags: u32, - /// Descriptor table address. + /// Descriptor table address (split) / Packed descriptor ring address (packed). pub desc_table_addr: u64, - /// Used ring buffer address. + /// Used ring buffer address (split) / Driver event suppression address (packed). pub used_ring_addr: u64, - /// Available ring buffer address. + /// Available ring buffer address (split) / Device event suppression address (packed). pub avail_ring_addr: u64, /// Optional address for logging. pub log_addr: Option, diff --git a/vhost/src/vhost_kern/mod.rs b/vhost/src/vhost_kern/mod.rs index 97f5819c..27ea2ba9 100644 --- a/vhost/src/vhost_kern/mod.rs +++ b/vhost/src/vhost_kern/mod.rs @@ -23,7 +23,7 @@ use vmm_sys_util::ioctl::{ioctl, ioctl_with_mut_ref, ioctl_with_ptr, ioctl_with_ use super::{ Error, Result, VhostAccess, VhostBackend, VhostIotlbBackend, VhostIotlbMsg, VhostIotlbMsgParser, VhostIotlbType, VhostUserDirtyLogRegion, VhostUserMemoryRegionInfo, - VringConfigData, VHOST_MAX_MEMORY_REGIONS, + VringConfigData, VHOST_MAX_MEMORY_REGIONS, vring_flags, }; pub mod vhost_binding; @@ -54,6 +54,49 @@ fn io_result(rc: isize, res: T) -> Result { } } +/// Check if the configuration data indicates packed virtqueue format. +fn is_packed_vring(config_data: &VringConfigData) -> bool { + config_data.flags & vring_flags::VHOST_VRING_F_PACKED != 0 +} + +/// Helper function to validate that a memory region is accessible. +fn validate_memory_region( + addr: u64, + size: GuestUsize, + memory: &M, +) -> bool { + GuestAddress(addr) + .checked_add(size) + .map_or(false, |end_addr| memory.address_in_range(end_addr)) +} + +/// Validate virtqueue memory layout for both packed and split formats. +fn validate_vring_memory( + config_data: &VringConfigData, + queue_size: u16, + memory: &AS::M, + is_packed: bool, +) -> bool { + if is_packed { + // Packed ring: single descriptor ring + event suppression structures + let desc_ring_size = 16 * u64::from(queue_size) as GuestUsize; + let event_size = 4; // 4 bytes for event suppression structures + + validate_memory_region(config_data.desc_table_addr, desc_ring_size, memory) + && validate_memory_region(config_data.avail_ring_addr, event_size, memory) + && validate_memory_region(config_data.used_ring_addr, event_size, memory) + } else { + // Split ring: separate descriptor, available, and used rings + let desc_table_size = 16 * u64::from(queue_size) as GuestUsize; + let avail_ring_size = 6 + 2 * u64::from(queue_size) as GuestUsize; + let used_ring_size = 6 + 8 * u64::from(queue_size) as GuestUsize; + + validate_memory_region(config_data.desc_table_addr, desc_table_size, memory) + && validate_memory_region(config_data.avail_ring_addr, avail_ring_size, memory) + && validate_memory_region(config_data.used_ring_addr, used_ring_size, memory) + } +} + /// Represent an in-kernel vhost device backend. pub trait VhostKernBackend: AsRawFd { /// Associated type to access guest memory. @@ -73,25 +116,10 @@ pub trait VhostKernBackend: AsRawFd { } let m = self.mem().memory(); - let desc_table_size = 16 * u64::from(queue_size) as GuestUsize; - let avail_ring_size = 6 + 2 * u64::from(queue_size) as GuestUsize; - let used_ring_size = 6 + 8 * u64::from(queue_size) as GuestUsize; - if GuestAddress(config_data.desc_table_addr) - .checked_add(desc_table_size) - .is_none_or(|v| !m.address_in_range(v)) - { - return false; - } - if GuestAddress(config_data.avail_ring_addr) - .checked_add(avail_ring_size) - .is_none_or(|v| !m.address_in_range(v)) - { - return false; - } - if GuestAddress(config_data.used_ring_addr) - .checked_add(used_ring_size) - .is_none_or(|v| !m.address_in_range(v)) - { + + // Validate vring memory layout based on format + let is_packed = is_packed_vring(config_data); + if !validate_vring_memory::(config_data, queue_size, &m, is_packed) { return false; } @@ -466,3 +494,29 @@ impl VringConfigData { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_packed_vring_detection() { + // Test that packed vring detection works independently of vhost-user feature + let mut config = VringConfigData::default(); + + // Test split virtqueue (default) + assert!(!is_packed_vring(&config)); + + // Test packed virtqueue + config.flags = vring_flags::VHOST_VRING_F_PACKED; + assert!(is_packed_vring(&config)); + + // Test with log flag combined + config.flags = vring_flags::VHOST_VRING_F_LOG | vring_flags::VHOST_VRING_F_PACKED; + assert!(is_packed_vring(&config)); + + // Test with only log flag + config.flags = vring_flags::VHOST_VRING_F_LOG; + assert!(!is_packed_vring(&config)); + } +} diff --git a/vhost/src/vhost_user/message.rs b/vhost/src/vhost_user/message.rs index c66bd446..86a322a8 100644 --- a/vhost/src/vhost_user/message.rs +++ b/vhost/src/vhost_user/message.rs @@ -381,11 +381,19 @@ impl VhostUserMsgValidator for VhostUserMsgHeader { bitflags! { #[derive(Copy, Clone, Debug, Eq, PartialEq)] /// Transport specific flags in VirtIO feature set defined by vhost-user. + /// + /// NOTE: This is for vhost-user transport-specific features only. + /// Standard VirtIO device features (like VIRTIO_F_RING_PACKED) should be + /// negotiated through the normal VirtIO device feature mechanism, not here. pub struct VhostUserVirtioFeatures: u64 { /// Log dirtied shared memory pages. const LOG_ALL = 0x400_0000; /// Feature flag for the protocol feature. const PROTOCOL_FEATURES = 0x4000_0000; + // NOTE: VIRTIO_F_RING_PACKED was incorrectly placed here. It's a standard + // VirtIO device feature and should be negotiated through device features, + // not vhost-user transport features. Packed virtqueue configuration is + // handled through VhostUserVringAddrFlags::VHOST_VRING_F_PACKED. } } @@ -708,11 +716,23 @@ impl VhostUserMsgValidator for VhostUserVringState {} // Bit mask for vring address flags. bitflags! { - /// Flags for vring address. + /// Flags for vring address configuration. + /// + /// These flags control vring setup and are used AFTER device feature negotiation. + /// The proper flow is: + /// 1. Device features (like VIRTIO_F_RING_PACKED) are negotiated through VirtIO standard mechanism + /// 2. These flags configure the actual vring layout based on negotiated features + /// + /// NOTE: These values must match the constants in crate::backend::vring_flags + /// to ensure compatibility between vhost-user and vhost-kern backends. pub struct VhostUserVringAddrFlags: u32 { /// Support log of vring operations. /// Modifications to "used" vring should be logged. const VHOST_VRING_F_LOG = 0x1; + /// Indicates packed virtqueue format. + /// When set, the vring uses packed layout instead of split layout. + /// This should only be set if VIRTIO_F_RING_PACKED was negotiated as a device feature. + const VHOST_VRING_F_PACKED = 0x2; } } @@ -777,13 +797,31 @@ impl VhostUserMsgValidator for VhostUserVringAddr { fn is_valid(&self) -> bool { if (self.flags & !VhostUserVringAddrFlags::all().bits()) != 0 { return false; - } else if self.descriptor & 0xf != 0 { - return false; - } else if self.available & 0x1 != 0 { + } + + // Common validation for both packed and split rings + // Descriptor table must be 16-byte aligned for both formats + if self.descriptor & 0xf != 0 { return false; - } else if self.used & 0x3 != 0 { + } + // Used ring must be 4-byte aligned for both formats + if self.used & 0x3 != 0 { return false; } + + // Available ring alignment depends on format + let is_packed = self.flags & VhostUserVringAddrFlags::VHOST_VRING_F_PACKED.bits() != 0; + if is_packed { + // Packed: available ring (device event suppression) must be 4-byte aligned + if self.available & 0x3 != 0 { + return false; + } + } else { + // Split: available ring must be 2-byte aligned + if self.available & 0x1 != 0 { + return false; + } + } true } } @@ -1417,4 +1455,72 @@ mod tests { msg.flags |= 0x4; assert!(!msg.is_valid()); } + + #[test] + fn test_packed_virtqueue_vring_flags() { + // Test vring address flags for packed virtqueue configuration + // NOTE: VIRTIO_F_RING_PACKED device feature negotiation happens separately + + let a = VhostUserVringAddrFlags::VHOST_VRING_F_PACKED.bits(); + assert_eq!(a, 0x2); + + let combined = VhostUserVringAddrFlags::VHOST_VRING_F_LOG + | VhostUserVringAddrFlags::VHOST_VRING_F_PACKED; + let a = combined.bits(); + assert_eq!(a, 0x3); + } + + #[test] + fn test_packed_vring_addr_validation() { + let mut addr = VhostUserVringAddr::new( + 0, + VhostUserVringAddrFlags::VHOST_VRING_F_PACKED, + 0x1000, + 0x2000, + 0x3000, + 0x4000, + ); + + let a = addr.index; + assert_eq!(a, 0); + let a = addr.flags; + assert_eq!(a, VhostUserVringAddrFlags::VHOST_VRING_F_PACKED.bits()); + let a = addr.descriptor; + assert_eq!(a, 0x1000); + let a = addr.used; + assert_eq!(a, 0x2000); + let a = addr.available; + assert_eq!(a, 0x3000); + let a = addr.log; + assert_eq!(a, 0x4000); + assert!(addr.is_valid()); + + addr.descriptor = 0x1001; + assert!(!addr.is_valid()); + addr.descriptor = 0x1000; + + addr.available = 0x3001; + assert!(!addr.is_valid()); + addr.available = 0x3000; + + addr.used = 0x2001; + assert!(!addr.is_valid()); + addr.used = 0x2000; + assert!(addr.is_valid()); + } + + #[test] + fn test_vring_flags_compatibility() { + // Ensure vhost-user flags match the shared backend constants + use crate::backend::vring_flags; + + assert_eq!( + VhostUserVringAddrFlags::VHOST_VRING_F_LOG.bits(), + vring_flags::VHOST_VRING_F_LOG + ); + assert_eq!( + VhostUserVringAddrFlags::VHOST_VRING_F_PACKED.bits(), + vring_flags::VHOST_VRING_F_PACKED + ); + } }