From c638d9355d44ab0eca64742bc579b41c5c12f19d Mon Sep 17 00:00:00 2001 From: Ryan Zezeski Date: Wed, 1 Jul 2026 16:23:33 -0400 Subject: [PATCH 1/6] viona: want F_EVENT_IDX support This adds F_EVENT_IDX support for all virtqueues, but for this commit we enable it for viona only. This is needed to support F_EVENT_IDX for the viona control queue, which is required so that the illumos viona device may offer it for the data queues. It was found that Tx completion interrupts are greatly reduced with this feature support, presumably due to the Linux driver's better use of `avail.used_event` for interrupt moderation. --- lib/propolis/src/hw/virtio/bits.rs | 3 + lib/propolis/src/hw/virtio/mod.rs | 49 ++++++ lib/propolis/src/hw/virtio/queue.rs | 242 ++++++++++++++++++++++------ lib/propolis/src/hw/virtio/viona.rs | 48 ++++-- 4 files changed, 276 insertions(+), 66 deletions(-) diff --git a/lib/propolis/src/hw/virtio/bits.rs b/lib/propolis/src/hw/virtio/bits.rs index a7372886a..802dc2e37 100644 --- a/lib/propolis/src/hw/virtio/bits.rs +++ b/lib/propolis/src/hw/virtio/bits.rs @@ -2,6 +2,9 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +// device-independent features +pub const VIRTIO_F_EVENT_IDX: u64 = 1 << 29; + // virtio-net feature bits pub const VIRTIO_NET_F_CSUM: u64 = 1 << 0; pub const VIRTIO_NET_F_GUEST_CSUM: u64 = 1 << 1; diff --git a/lib/propolis/src/hw/virtio/mod.rs b/lib/propolis/src/hw/virtio/mod.rs index 88894c759..834c6e0e0 100644 --- a/lib/propolis/src/hw/virtio/mod.rs +++ b/lib/propolis/src/hw/virtio/mod.rs @@ -30,11 +30,28 @@ use crate::common::RWOp; use crate::hw::pci as pci_hw; use crate::lifecycle::Lifecycle; use queue::VirtQueue; +use serde::{Deserialize, Serialize}; pub use block::PciVirtioBlock; pub use viona::PciVirtioViona; pub use vsock::PciVirtioSock; +#[cfg(target_os = "illumos")] + +mod os { + unsafe extern "C" { + pub fn membar_enter(); + } +} + +#[inline(always)] +pub fn membar_enter() { + #[cfg(target_os = "illumos")] + unsafe { + os::membar_enter(); + } +} + bitflags! { pub struct LegacyFeatures: u64 { const NOTIFY_ON_EMPTY = 1 << 24; @@ -246,6 +263,7 @@ pub enum VqChange { IntrCfg, } +#[derive(Debug)] pub enum VqIntr { /// Pin (lintr) interrupt Pin, @@ -254,12 +272,43 @@ pub enum VqIntr { Msi(u64, u32, bool), } +#[derive(Copy, Clone, Default, Deserialize, Serialize)] +pub struct VqNeedsIntrProbeInfo { + used_event: u16, + last_chk_uidx: u16, + uidx: u16, + needed: bool, +} + #[usdt::provider(provider = "propolis")] mod probes { fn virtio_vq_notify(virtio_dev_addr: u64, virtqueue_id: u16) {} fn virtio_vq_pop(vq_addr: u64, desc_idx: u16, avail_idx: u16) {} fn virtio_vq_push(vq_addr: u64, used_idx: u16, used_len: u32) {} + fn virtio_vq_needs_intr( + vq_addr: u64, + vq_id: u16, + f_event_idx: u64, + info: crate::hw::virtio::VqNeedsIntrProbeInfo, + ) { + } + fn virtio_vq_send_intr(vq_addr: u64, vq_id: u16, sent: u64) {} + fn virtio_vq_enable_intr( + vq_addr: u64, + vq_id: u16, + f_event_idx: u64, + avail_event: u16, + ) { + } + fn virtio_vq_disable_intr( + vq_addr: u64, + vq_id: u16, + f_event_idx: u64, + avail_event: u16, + ) { + } + fn virtio_viona_mq_set_use_pairs(cause: u8, npairs: u16) {} fn virtio_device_needs_reset() {} diff --git a/lib/propolis/src/hw/virtio/queue.rs b/lib/propolis/src/hw/virtio/queue.rs index 9c9925505..bc6494cbc 100644 --- a/lib/propolis/src/hw/virtio/queue.rs +++ b/lib/propolis/src/hw/virtio/queue.rs @@ -85,6 +85,7 @@ pub struct VqAvail { cur_avail_idx: Wrapping, gpa_desc: GuestAddr, + gpa_used_event: GuestAddr, } impl VqAvail { @@ -130,12 +131,16 @@ impl VqAvail { self.cur_avail_idx = Wrapping(0); } - fn map_split(&mut self, desc_addr: u64, avail_addr: u64) { + fn map_split(&mut self, desc_addr: u64, avail_addr: u64, sz: u16) { self.gpa_desc = GuestAddr(desc_addr); - // 16-bit flags, followed by 16-bit idx, followed by avail desc ring + // 16-bit flags, followed by 16-bit idx, avail desc ring + // (u16[sz]), followed by 16-bit used_event. self.gpa_flags = GuestAddr(avail_addr); self.gpa_idx = GuestAddr(avail_addr + 2); self.gpa_ring = GuestAddr(avail_addr + 4); + self.gpa_used_event = GuestAddr( + avail_addr + (2 + sz as u64) * mem::size_of::() as u64, + ); } /// Returns guest flags. @@ -145,11 +150,13 @@ impl VqAvail { AvailFlags::from_bits_truncate(value) } - /// Returns true IFF interrupts are supressed. - #[allow(dead_code)] - fn _intr_supressed(&self, mem: &MemCtx) -> bool { - let flags = self.flags(mem); - flags.contains(AvailFlags::NO_INTERRUPT) + /// Return used_event value. + fn used_event(&self, mem: &MemCtx) -> u16 { + if self.valid { + *mem.read(self.gpa_used_event).unwrap() + } else { + 0 + } } } @@ -160,7 +167,13 @@ pub struct VqUsed { gpa_flags: GuestAddr, gpa_idx: GuestAddr, gpa_ring: GuestAddr, + /// Used by device to control notification when F_EVENT_IDX is + /// negotiated. + gpa_avail_event: GuestAddr, used_idx: Wrapping, + /// The value of uidx the last time we checked if the guest + /// required notification. + last_chk_uidx: Wrapping, interrupt: Option>, } @@ -193,25 +206,28 @@ impl VqUsed { mem.write(self.gpa_flags, &value); } - /// Disables notifications on this queue; returns whether notfications were - /// enabled before. - fn disable_notify(&self, mem: &MemCtx) -> bool { - let flags = self.flags(mem); - let current = !flags.contains(UsedFlags::NO_NOTIFY); - self.set_flags(flags | UsedFlags::NO_NOTIFY, mem); - current - } - - fn enable_notify(&self, mem: &MemCtx) { - let mut flags = self.flags(mem); - flags.remove(UsedFlags::NO_NOTIFY); - self.set_flags(flags, mem); + /// Disables notifications on this queue. + fn disable_notify(&self, avail_idx: Option>, mem: &MemCtx) { + if let Some(idx) = avail_idx { + assert!(self.gpa_avail_event.0 != 0); + mem.write(self.gpa_avail_event, &idx); + } else { + let flags = self.flags(mem); + self.set_flags(flags | UsedFlags::NO_NOTIFY, mem); + } } - /// Returns true iff notifications are supressed for this queue. - fn notify_supressed(&self, mem: &MemCtx) -> bool { - let flags = self.flags(mem); - flags.contains(UsedFlags::NO_NOTIFY) + /// Let the driver know that we are interested in receiving + /// interrupts. + fn enable_notify(&self, avail_idx: Option, mem: &MemCtx) { + if let Some(idx) = avail_idx { + assert!(self.gpa_avail_event.0 != 0); + mem.write(self.gpa_avail_event, &idx); + } else { + let mut flags = self.flags(mem); + flags.remove(UsedFlags::NO_NOTIFY); + self.set_flags(flags, mem); + } } fn reset(&mut self) { @@ -221,11 +237,15 @@ impl VqUsed { self.gpa_ring = GuestAddr(0); self.used_idx = Wrapping(0); } - fn map_split(&mut self, gpa: u64) { - // 16-bit flags, followed by 16-bit idx, followed by used desc ring + + fn map_split(&mut self, gpa: u64, sz: u16) { + // 16-bit flags, followed by 16-bit idx, followed by used desc + // ring, followed by avail_event self.gpa_flags = GuestAddr(gpa); self.gpa_idx = GuestAddr(gpa + 2); self.gpa_ring = GuestAddr(gpa + 4); + self.gpa_avail_event = + GuestAddr(gpa + 4 + mem::size_of::() as u64 * sz as u64); } } @@ -282,6 +302,8 @@ pub struct VirtQueue { avail: Mutex, used: Mutex, pub acc_mem: MemAccessor, + /// Is the F_EVENT_IDX feature in use? + f_event_idx: AtomicBool, } const fn qalign(addr: u64, align: u64) -> u64 { @@ -306,16 +328,20 @@ impl VirtQueue { gpa_ring: GuestAddr(0), cur_avail_idx: Wrapping(0), gpa_desc: GuestAddr(0), + gpa_used_event: GuestAddr(0), }), used: Mutex::new(VqUsed { valid: false, gpa_flags: GuestAddr(0), gpa_idx: GuestAddr(0), gpa_ring: GuestAddr(0), + gpa_avail_event: GuestAddr(0), used_idx: Wrapping(0), + last_chk_uidx: Wrapping(0), interrupt: None, }), acc_mem: MemAccessor::new_orphan(), + f_event_idx: AtomicBool::new(false), } } @@ -355,6 +381,14 @@ impl VirtQueue { self.is_control.store(val, Ordering::Release); } + fn f_event_idx(&self) -> bool { + self.f_event_idx.load(Ordering::Acquire) + } + + fn set_f_event_idx(&self, val: bool) { + self.f_event_idx.store(val, Ordering::Release); + } + #[inline(always)] pub fn size(&self) -> u16 { let size = *self.size.lock().unwrap(); @@ -376,8 +410,8 @@ impl VirtQueue { ) { let mut avail = self.avail.lock().expect("avail is initialized"); let mut used = self.used.lock().expect("used is initialized"); - avail.map_split(desc_addr, avail_addr); - used.map_split(used_addr); + avail.map_split(desc_addr, avail_addr, self.size()); + used.map_split(used_addr, self.size()); avail.valid = true; used.valid = true; } @@ -440,8 +474,12 @@ impl VirtQueue { let mut avail = self.avail.lock().unwrap(); let mut used = self.used.lock().unwrap(); - avail.map_split(info.mapping.desc_addr, info.mapping.avail_addr); - used.map_split(info.mapping.used_addr); + avail.map_split( + info.mapping.desc_addr, + info.mapping.avail_addr, + self.size(), + ); + used.map_split(info.mapping.used_addr, self.size()); avail.valid = info.mapping.valid; used.valid = info.mapping.valid; avail.cur_avail_idx = Wrapping(info.avail_idx); @@ -538,19 +576,16 @@ impl VirtQueue { pub fn push_used(&self, chain: &mut Chain, mem: &MemCtx) { assert!(chain.idx.is_some()); + let avail = self.avail.lock().unwrap(); let mut used = self.used.lock().unwrap(); let id = mem::replace(&mut chain.idx, None).unwrap(); // XXX: for now, just go off of the write stats let len = chain.write_stat.bytes - chain.write_stat.bytes_remain; probes::virtio_vq_push!(|| (self as *const VirtQueue as u64, id, len)); used.write_used(id, len, self.size(), mem); - // XXX: This is wrong. Interrupt notification is on the avail ring, - // not used. - #[allow(clippy::overly_complex_bool_expr)] - if true || !used.notify_supressed(mem) { - if let Some(intr) = used.interrupt.as_ref() { - intr.notify(); - } + + if self.needs_intr_locked(&avail, &mut used, mem) { + self.send_intr_locked(&used); } chain.reset(); } @@ -570,28 +605,123 @@ impl VirtQueue { /// Disables interrupts (notifications) on the `Used` ring. /// /// Returns `true` if notifications were previously enabled. - pub(super) fn disable_intr(&self, mem: &MemCtx) -> bool { - let used = self.used.lock().unwrap(); - used.disable_notify(mem) + pub(super) fn disable_intr(&self, mem: &MemCtx) { + let idx = if self.f_event_idx() { + let aidx = self.avail.lock().unwrap().cur_avail_idx - Wrapping(1); + probes::virtio_vq_disable_intr!(|| ( + self as *const VirtQueue as u64, + self.id, + 1, + aidx.0, + )); + Some(aidx) + } else { + probes::virtio_vq_disable_intr!(|| ( + self as *const VirtQueue as u64, + self.id, + 0, + 0, + )); + None + }; + + self.used.lock().unwrap().disable_notify(idx, mem); } /// Enables interrupts (notifications) on the `Used` ring pub(super) fn enable_intr(&self, mem: &MemCtx) { - let used = self.used.lock().unwrap(); - used.enable_notify(mem); + let aidx = if self.f_event_idx() { + let aidx = self.avail.lock().unwrap().cur_avail_idx; + probes::virtio_vq_enable_intr!(|| ( + self as *const VirtQueue as u64, + self.id, + 1, + aidx.0, + )); + Some(aidx.0) + } else { + probes::virtio_vq_enable_intr!(|| ( + self as *const VirtQueue as u64, + self.id, + 0, + 0, + )); + None + }; + + self.used.lock().unwrap().enable_notify(aidx, mem); + } + + pub fn needs_intr(&self, mem: &MemCtx) -> bool { + let avail = self.avail.lock().unwrap(); + let mut used = self.used.lock().unwrap(); + self.needs_intr_locked(&avail, &mut used, mem) + } + + /// Returns true if the guest should receive an interrupt. + fn needs_intr_locked( + &self, + avail: &VqAvail, + used: &mut VqUsed, + mem: &MemCtx, + ) -> bool { + if self.f_event_idx() { + let used_event = Wrapping(avail.used_event(mem)); + let uidx = used.used_idx; + let last_chk_uidx = used.last_chk_uidx; + let res = + (uidx - used_event - Wrapping(1)) < (uidx - last_chk_uidx); + used.last_chk_uidx = uidx; + let info = crate::hw::virtio::VqNeedsIntrProbeInfo { + used_event: used_event.0, + last_chk_uidx: last_chk_uidx.0, + uidx: uidx.0, + needed: res, + }; + probes::virtio_vq_needs_intr!(|| ( + self as *const VirtQueue as u64, + self.id, + 1, + info, + )); + res + } else { + let flags = avail.flags(mem); + let res = !flags.contains(AvailFlags::NO_INTERRUPT); + let info = crate::hw::virtio::VqNeedsIntrProbeInfo { + used_event: 0, + last_chk_uidx: 0, + uidx: 0, + needed: res, + }; + probes::virtio_vq_needs_intr!(|| ( + self as *const VirtQueue as u64, + self.id, + 0, + info, + )); + res + } } /// Send an interrupt for this virtual queue. - pub(super) fn send_intr(&self, mem: &MemCtx) { + pub(super) fn send_intr(&self) { let used = self.used.lock().unwrap(); - // XXX: This is wrong. Interrupt notification is on the avail ring, - // not used. - #[allow(clippy::overly_complex_bool_expr)] - if true || !used.notify_supressed(mem) { - if let Some(intr) = used.interrupt.as_ref() { - intr.notify(); - } - } + self.send_intr_locked(&used); + } + + fn send_intr_locked(&self, used: &VqUsed) { + let sent = if let Some(intr) = used.interrupt.as_ref() { + intr.notify(); + true + } else { + false + }; + probes::virtio_vq_send_intr!(|| ( + self as *const VirtQueue as u64, + self.id, + sent as u64, + )); } pub fn export(&self) -> migrate::VirtQueueV1 { @@ -676,11 +806,11 @@ impl VirtQueue { ))); } - avail.map_split(state.descr_gpa, state.avail_gpa); + avail.map_split(state.descr_gpa, state.avail_gpa, self.size()); avail.valid = state.mapping_valid; avail.cur_avail_idx = Wrapping(state.avail_cur_idx); - used.map_split(state.used_gpa); + used.map_split(state.used_gpa, self.size()); used.valid = state.mapping_valid; used.used_idx = Wrapping(state.used_idx); @@ -1107,6 +1237,12 @@ impl VirtQueues { Ok(()) } + pub fn set_f_event_idx(&self, f_event_idx: bool) { + for vq in &self.queues { + vq.set_f_event_idx(f_event_idx); + } + } + pub fn count(&self) -> NonZeroU16 { NonZeroU16::try_from(self.iter().count() as u16) .expect("queue count already validated") diff --git a/lib/propolis/src/hw/virtio/viona.rs b/lib/propolis/src/hw/virtio/viona.rs index 4f2dc8ea6..3a500b704 100644 --- a/lib/propolis/src/hw/virtio/viona.rs +++ b/lib/propolis/src/hw/virtio/viona.rs @@ -624,7 +624,9 @@ impl PciVirtioViona { .intr_poll(self.virtio_state.queues.len(), |vq_idx| { self.hdl.ring_intr_clear(vq_idx).unwrap(); let vq = self.virtio_state.queues.get(vq_idx).unwrap(); - vq.send_intr(&mem); + if vq.needs_intr(&mem) { + vq.send_intr(); + } }) .unwrap(); } @@ -632,19 +634,32 @@ impl PciVirtioViona { fn ctl_queue_notify(&self, vq: &VirtQueue) { if let Some(mem) = self.pci_state.acc_mem.access() { - while !vq.avail_is_empty(&mem) { - let mut chain = Chain::with_capacity(4); - let intrs_en = vq.disable_intr(&mem); - while let Some((_idx, _len)) = vq.pop_avail(&mut chain, &mem) { - let res = match self.ctl_msg(&mut chain, &mem) { - Ok(_) => control::Ack::Ok, - Err(_) => control::Ack::Err, - } as u8; - chain.write(&res, &mem); - vq.push_used(&mut chain, &mem); + loop { + vq.disable_intr(&mem); + + while !vq.avail_is_empty(&mem) { + let mut chain = Chain::with_capacity(4); + + while let Some((_idx, _len)) = + vq.pop_avail(&mut chain, &mem) + { + let res = match self.ctl_msg(&mut chain, &mem) { + Ok(_) => control::Ack::Ok, + Err(_) => control::Ack::Err, + } as u8; + chain.write(&res, &mem); + vq.push_used(&mut chain, &mem); + } } - if intrs_en { - vq.enable_intr(&mem); + + vq.enable_intr(&mem); + + crate::hw::virtio::membar_enter(); + + // Check if enabling the interrupt raced with the + // driver publishing new entries to avail. + if vq.avail_is_empty(&mem) { + break; } } } @@ -1101,6 +1116,13 @@ impl VirtioDevice for PciVirtioViona { fn set_features(&self, feat: u64) -> Result<(), ()> { self.hdl.set_features(feat).map_err(|_| ())?; + eprintln!("set_features: {:?}", feat); + + if (feat & VIRTIO_F_EVENT_IDX) != 0 { + eprintln!("set f_event_idx on all queues"); + self.virtio_state.queues.set_f_event_idx(true); + } + // Any remaining setup is for control-queue based features. let control_queue = if (feat & VIRTIO_NET_F_CTRL_VQ) == 0 { None From a72bd75cb4717ed0ba6ef1f315e30e21cad46ef7 Mon Sep 17 00:00:00 2001 From: Ryan Zezeski Date: Mon, 20 Jul 2026 13:52:37 -0400 Subject: [PATCH 2/6] xxx event_idx fixes --- crates/viona-api/src/ffi.rs | 3 ++- lib/propolis/src/hw/virtio/queue.rs | 16 +++++++--------- lib/propolis/src/hw/virtio/viona.rs | 3 +++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/crates/viona-api/src/ffi.rs b/crates/viona-api/src/ffi.rs index e67f97d11..d20cfdcf7 100644 --- a/crates/viona-api/src/ffi.rs +++ b/crates/viona-api/src/ffi.rs @@ -127,6 +127,7 @@ pub struct vioc_ring_state { pub vrs_qaddr_desc: u64, pub vrs_qaddr_avail: u64, pub vrs_qaddr_used: u64, + pub vrs_last_chk_uidx: u16, } pub const VIONA_PROMISC_NONE: i32 = 0; @@ -154,7 +155,7 @@ pub struct vioc_set_params { /// This is the viona interface version which viona_api expects to operate /// against. All constants and structs defined by the crate are done so in /// terms of that specific version. -pub const VIONA_CURRENT_INTERFACE_VERSION: u32 = 6; +pub const VIONA_CURRENT_INTERFACE_VERSION: u32 = 7; /// Maximum size of packed nvlists used in viona parameter ioctls pub const VIONA_MAX_PARAM_NVLIST_SZ: usize = 4096; diff --git a/lib/propolis/src/hw/virtio/queue.rs b/lib/propolis/src/hw/virtio/queue.rs index bc6494cbc..d51f1a9eb 100644 --- a/lib/propolis/src/hw/virtio/queue.rs +++ b/lib/propolis/src/hw/virtio/queue.rs @@ -138,9 +138,7 @@ impl VqAvail { self.gpa_flags = GuestAddr(avail_addr); self.gpa_idx = GuestAddr(avail_addr + 2); self.gpa_ring = GuestAddr(avail_addr + 4); - self.gpa_used_event = GuestAddr( - avail_addr + (2 + sz as u64) * mem::size_of::() as u64, - ); + self.gpa_used_event = self.gpa_ring.offset::(usize::from(sz)); } /// Returns guest flags. @@ -244,8 +242,7 @@ impl VqUsed { self.gpa_flags = GuestAddr(gpa); self.gpa_idx = GuestAddr(gpa + 2); self.gpa_ring = GuestAddr(gpa + 4); - self.gpa_avail_event = - GuestAddr(gpa + 4 + mem::size_of::() as u64 * sz as u64); + self.gpa_avail_event = self.gpa_ring.offset::(usize::from(sz)); } } @@ -467,6 +464,7 @@ impl VirtQueue { }, avail_idx: avail.cur_avail_idx.0, used_idx: used.used_idx.0, + last_chk_uidx: used.last_chk_uidx.0, } } @@ -484,6 +482,7 @@ impl VirtQueue { used.valid = info.mapping.valid; avail.cur_avail_idx = Wrapping(info.avail_idx); used.used_idx = Wrapping(info.used_idx); + used.last_chk_uidx = Wrapping(info.last_chk_uidx); } /// Accummulates a sequence of available descriptors into a `Chain`. @@ -602,9 +601,7 @@ impl VirtQueue { used.interrupt.as_ref().map(|x| x.read()) } - /// Disables interrupts (notifications) on the `Used` ring. - /// - /// Returns `true` if notifications were previously enabled. + /// Disable interrupts (notifications) from driver to device. pub(super) fn disable_intr(&self, mem: &MemCtx) { let idx = if self.f_event_idx() { let aidx = self.avail.lock().unwrap().cur_avail_idx - Wrapping(1); @@ -628,7 +625,7 @@ impl VirtQueue { self.used.lock().unwrap().disable_notify(idx, mem); } - /// Enables interrupts (notifications) on the `Used` ring + /// Enable interrupts (notifications) from driver to device. pub(super) fn enable_intr(&self, mem: &MemCtx) { let aidx = if self.f_event_idx() { let aidx = self.avail.lock().unwrap().cur_avail_idx; @@ -1165,6 +1162,7 @@ pub struct Info { pub mapping: MapInfo, pub avail_idx: u16, pub used_idx: u16, + pub last_chk_uidx: u16, } pub struct VirtQueues { diff --git a/lib/propolis/src/hw/virtio/viona.rs b/lib/propolis/src/hw/virtio/viona.rs index 3a500b704..2a47b3116 100644 --- a/lib/propolis/src/hw/virtio/viona.rs +++ b/lib/propolis/src/hw/virtio/viona.rs @@ -1461,6 +1461,7 @@ impl From<&VirtQueue> for viona_api::vioc_ring_state { let used_addr = state.mapping.used_addr; let avail_idx = state.avail_idx; let used_idx = state.used_idx; + let last_chk_uidx = state.last_chk_uidx; viona_api::vioc_ring_state { vrs_index: id, vrs_qsize: size, @@ -1468,6 +1469,7 @@ impl From<&VirtQueue> for viona_api::vioc_ring_state { vrs_qaddr_avail: avail_addr, vrs_qaddr_used: used_addr, vrs_used_idx: used_idx, + vrs_last_chk_uidx: last_chk_uidx, vrs_avail_idx: avail_idx, } } @@ -1584,6 +1586,7 @@ impl VionaHdl { }, avail_idx: cfg.vrs_avail_idx, used_idx: cfg.vrs_used_idx, + last_chk_uidx: cfg.vrs_last_chk_uidx, }) } fn ring_cfg_msi( From 5d95de6445c41a34f87255232d29019ece464f6f Mon Sep 17 00:00:00 2001 From: Ryan Zezeski Date: Thu, 10 Sep 2026 12:06:16 -0400 Subject: [PATCH 3/6] no longer needed --- crates/viona-api/src/ffi.rs | 1 - lib/propolis/src/hw/virtio/queue.rs | 4 +--- lib/propolis/src/hw/virtio/viona.rs | 3 --- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/viona-api/src/ffi.rs b/crates/viona-api/src/ffi.rs index d20cfdcf7..45fc32482 100644 --- a/crates/viona-api/src/ffi.rs +++ b/crates/viona-api/src/ffi.rs @@ -127,7 +127,6 @@ pub struct vioc_ring_state { pub vrs_qaddr_desc: u64, pub vrs_qaddr_avail: u64, pub vrs_qaddr_used: u64, - pub vrs_last_chk_uidx: u16, } pub const VIONA_PROMISC_NONE: i32 = 0; diff --git a/lib/propolis/src/hw/virtio/queue.rs b/lib/propolis/src/hw/virtio/queue.rs index d51f1a9eb..05c706dc0 100644 --- a/lib/propolis/src/hw/virtio/queue.rs +++ b/lib/propolis/src/hw/virtio/queue.rs @@ -464,7 +464,6 @@ impl VirtQueue { }, avail_idx: avail.cur_avail_idx.0, used_idx: used.used_idx.0, - last_chk_uidx: used.last_chk_uidx.0, } } @@ -482,7 +481,7 @@ impl VirtQueue { used.valid = info.mapping.valid; avail.cur_avail_idx = Wrapping(info.avail_idx); used.used_idx = Wrapping(info.used_idx); - used.last_chk_uidx = Wrapping(info.last_chk_uidx); + used.last_chk_uidx = Wrapping(info.used_idx); } /// Accummulates a sequence of available descriptors into a `Chain`. @@ -1162,7 +1161,6 @@ pub struct Info { pub mapping: MapInfo, pub avail_idx: u16, pub used_idx: u16, - pub last_chk_uidx: u16, } pub struct VirtQueues { diff --git a/lib/propolis/src/hw/virtio/viona.rs b/lib/propolis/src/hw/virtio/viona.rs index 2a47b3116..3a500b704 100644 --- a/lib/propolis/src/hw/virtio/viona.rs +++ b/lib/propolis/src/hw/virtio/viona.rs @@ -1461,7 +1461,6 @@ impl From<&VirtQueue> for viona_api::vioc_ring_state { let used_addr = state.mapping.used_addr; let avail_idx = state.avail_idx; let used_idx = state.used_idx; - let last_chk_uidx = state.last_chk_uidx; viona_api::vioc_ring_state { vrs_index: id, vrs_qsize: size, @@ -1469,7 +1468,6 @@ impl From<&VirtQueue> for viona_api::vioc_ring_state { vrs_qaddr_avail: avail_addr, vrs_qaddr_used: used_addr, vrs_used_idx: used_idx, - vrs_last_chk_uidx: last_chk_uidx, vrs_avail_idx: avail_idx, } } @@ -1586,7 +1584,6 @@ impl VionaHdl { }, avail_idx: cfg.vrs_avail_idx, used_idx: cfg.vrs_used_idx, - last_chk_uidx: cfg.vrs_last_chk_uidx, }) } fn ring_cfg_msi( From cae286153ebd75b03bae6996866a394f29786dc9 Mon Sep 17 00:00:00 2001 From: Ryan Zezeski Date: Sun, 27 Sep 2026 15:14:20 -0400 Subject: [PATCH 4/6] update tests --- lib/propolis/src/hw/virtio/viona.rs | 40 +++++++++++++++++++---------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/propolis/src/hw/virtio/viona.rs b/lib/propolis/src/hw/virtio/viona.rs index 3a500b704..875207d3e 100644 --- a/lib/propolis/src/hw/virtio/viona.rs +++ b/lib/propolis/src/hw/virtio/viona.rs @@ -1935,8 +1935,8 @@ mod test { use crate::hw::pci::Bdf; use crate::hw::virtio::pci::Status; use crate::hw::virtio::viona::{ - VIRTIO_NET_F_CTRL_VQ, VIRTIO_NET_F_MAC, VIRTIO_NET_F_MQ, - VIRTIO_NET_F_STATUS, + VIRTIO_F_EVENT_IDX, VIRTIO_NET_F_CTRL_VQ, VIRTIO_NET_F_MAC, + VIRTIO_NET_F_MQ, VIRTIO_NET_F_STATUS, }; use crate::hw::virtio::PciVirtioViona; use crate::lifecycle::Lifecycle; @@ -2347,7 +2347,7 @@ mod test { self.common_config.read_le16(common_cfg::queue_enable) == 1; assert!(!already_enabled); - let queue_size = + let mut queue_size = self.common_config.read_le16(common_cfg::queue_size); assert_ne!(queue_size, 0); // In "2.7 Split Virtqueues", @@ -2370,6 +2370,7 @@ mod test { if chosen_size < queue_size { self.common_config .write_le16(common_cfg::queue_size, chosen_size); + queue_size = chosen_size; } let acc_mem = @@ -2385,11 +2386,15 @@ mod test { // > * The driver MUST set flags to 0 or 1. // > * The driver MAY set flags to 1 to advise the device that // notifications are not needed. - acc_mem.write::(GuestAddr(avail_gpa), &0); + acc_mem.write::(GuestAddr(avail_gpa), &0); // Index. "This starts at 0, and increases." - acc_mem.write::(GuestAddr(avail_gpa + 4), &0); - // Leave all the `ring` entries uninitialized, and we've not - // negotiated VIRTIO_F_EVENT_IDX so no `used_event` for now. + acc_mem.write::(GuestAddr(avail_gpa + 2), &0); + // Leave all the `ring` entries uninitialized. Initialize + // `used_event` to zero. + acc_mem.write::( + GuestAddr(avail_gpa + 4 + (queue_size as u64 * 2)), + &0, + ); self.common_config.write_le64(common_cfg::queue_driver, avail_gpa); let used_gpa = avail_gpa.next_multiple_of(page_u64); @@ -2644,8 +2649,10 @@ mod test { } fn basic_operation_modern(test_ctx: TestCtx) -> TestCtx { - let expected_feats = - VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS | VIRTIO_NET_F_CTRL_VQ; + let expected_feats = VIRTIO_F_EVENT_IDX + | VIRTIO_NET_F_MAC + | VIRTIO_NET_F_STATUS + | VIRTIO_NET_F_CTRL_VQ; // Go through setting up the virtio NIC in a few scenarios, but don't // try using it or setting any interesting features. @@ -2688,7 +2695,8 @@ mod test { fn basic_operation_multiqueue(test_ctx: TestCtx) -> TestCtx { // All the same operation as `basic_operation_modern`, but with // `VIRTIO_NET_F_MQ`. - let expected_feats = VIRTIO_NET_F_MAC + let expected_feats = VIRTIO_F_EVENT_IDX + | VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS | VIRTIO_NET_F_CTRL_VQ | VIRTIO_NET_F_MQ; @@ -2739,8 +2747,10 @@ mod test { fn multiqueue_migration(test_ctx: TestCtx) -> TestCtx { // All the same operation as `basic_operation_modern`, but with // `VIRTIO_NET_F_MQ`. - let expected_feats = - VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS | VIRTIO_NET_F_CTRL_VQ; + let expected_feats = VIRTIO_F_EVENT_IDX + | VIRTIO_NET_F_MAC + | VIRTIO_NET_F_STATUS + | VIRTIO_NET_F_CTRL_VQ; let mut driver = test_ctx.create_driver(); driver.modern_device_init(expected_feats | VIRTIO_NET_F_MQ); @@ -2804,8 +2814,10 @@ mod test { /// but were still "enabled" because reset did not cover them, and would /// make guests determine the device was simply broken. They were right! fn multiqueue_migration_after_boot(test_ctx: TestCtx) -> TestCtx { - let expected_feats = - VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS | VIRTIO_NET_F_CTRL_VQ; + let expected_feats = VIRTIO_F_EVENT_IDX + | VIRTIO_NET_F_MAC + | VIRTIO_NET_F_STATUS + | VIRTIO_NET_F_CTRL_VQ; let mut driver = test_ctx.create_driver(); driver.modern_device_init(expected_feats | VIRTIO_NET_F_MQ); From 55c9bd7fd8788344cf6730844e071fe043e9b856 Mon Sep 17 00:00:00 2001 From: Ryan Zezeski Date: Sun, 27 Sep 2026 15:46:52 -0400 Subject: [PATCH 5/6] update viona api version --- crates/viona-api/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/viona-api/src/lib.rs b/crates/viona-api/src/lib.rs index d8535972c..4a75bc4f4 100644 --- a/crates/viona-api/src/lib.rs +++ b/crates/viona-api/src/lib.rs @@ -197,6 +197,9 @@ fn minor(meta: &std::fs::Metadata) -> u32 { #[repr(u32)] #[derive(Copy, Clone)] pub enum ApiVersion { + /// Adds support for F_EVENT_IDX. No change to API or data structures. + V8 = 8, + /// Adds multi-queue support and change the data structure for per-queue /// interrupt polling to a compact bitmap. V6 = 6, @@ -218,7 +221,7 @@ pub enum ApiVersion { } impl ApiVersion { pub const fn current() -> Self { - Self::V6 + Self::V8 } } impl PartialEq for u32 { From 88c04fe2b88a94249cc82629945bc405074dd374 Mon Sep 17 00:00:00 2001 From: Ryan Zezeski Date: Sun, 27 Sep 2026 16:31:25 -0400 Subject: [PATCH 6/6] use correct type for avail_event offset --- lib/propolis/src/hw/virtio/queue.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/propolis/src/hw/virtio/queue.rs b/lib/propolis/src/hw/virtio/queue.rs index 05c706dc0..c3723dc8b 100644 --- a/lib/propolis/src/hw/virtio/queue.rs +++ b/lib/propolis/src/hw/virtio/queue.rs @@ -242,7 +242,7 @@ impl VqUsed { self.gpa_flags = GuestAddr(gpa); self.gpa_idx = GuestAddr(gpa + 2); self.gpa_ring = GuestAddr(gpa + 4); - self.gpa_avail_event = self.gpa_ring.offset::(usize::from(sz)); + self.gpa_avail_event = self.gpa_ring.offset::(usize::from(sz)); } }