virtio fixes based on https://github.com/weltling/virtio-villain (2/n)

alioth/src/virtio/dev/dev.rs

@@ -24,7 +24,7 @@ pub mod vsock;
 
 use std::fmt::Debug;
 use std::sync::Arc;
-use std::sync::atomic::{AtomicU8, AtomicU16, AtomicU32};
+use std::sync::atomic::{AtomicU8, AtomicU16, AtomicU32, Ordering};
 use std::thread::JoinHandle;
 
 use bitflags::Flags;
@@ -84,6 +84,16 @@ pub struct Register {
     pub status: AtomicU8,
 }
 
+impl Register {
+    pub fn get_driver_feature(&self) -> u128 {
+        let mut result = 0u128;
+        for (i, feature) in self.driver_feature.iter().enumerate() {
+            result |= (feature.load(Ordering::Acquire) as u128) << (i << 5);
+        }
+        result
+    }
+}
+
 const TOKEN_WARKER: u64 = 1 << 63;
 
 #[derive(Debug, Clone)]
@@ -438,3 +448,7 @@ pub trait DevSpec {
         false
     }
 }
+
+#[cfg(test)]
+#[path = "dev_test.rs"]
+mod tests;

alioth/src/virtio/dev/dev_test.rs

@@ -0,0 +1,34 @@
+// Copyright 2026 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use std::sync::atomic::Ordering;
+
+use super::Register;
+
+#[test]
+fn test_virtio_register_get_driver_feature() {
+    let reg = Register::default();
+    assert_eq!(reg.get_driver_feature(), 0);
+
+    reg.driver_feature[0].store(0x1234_5678, Ordering::Release);
+    reg.driver_feature[1].store(0x9abc_def0, Ordering::Release);
+    reg.driver_feature[2].store(0xfeed_cafe, Ordering::Release);
+    reg.driver_feature[3].store(0x0123_4567, Ordering::Release);
+
+    let expected = (0x1234_5678u128)
+        | ((0x9abc_def0u128) << 32)
+        | ((0xfeed_cafeu128) << 64)
+        | ((0x0123_4567u128) << 96);
+    assert_eq!(reg.get_driver_feature(), expected);
+}

alioth/src/virtio/pci.rs

@@ -42,7 +42,7 @@ use crate::sync::notifier::Notifier;
 use crate::utils::{get_atomic_high32, get_atomic_low32, set_atomic_high32, set_atomic_low32};
 use crate::virtio::dev::{Register, StartParam, VirtioDevice, WakeEvent};
 use crate::virtio::queue::QueueReg;
-use crate::virtio::{DevStatus, DeviceId, IrqSender, Result, error};
+use crate::virtio::{DevStatus, DeviceId, IrqSender, Result, VirtioFeature, error};
 use crate::{consts, impl_mmio_for_zerocopy, mem};
 
 const VIRTIO_MSI_NO_VECTOR: u16 = 0xffff;
@@ -394,9 +394,18 @@ where
                 reg.driver_feature_sel.store(val as u32, Ordering::Release);
             }
             VirtioCommonCfg::LAYOUT_DRIVER_FEATURE => {
-                let sel = reg.driver_feature_sel.load(Ordering::Acquire);
-                if let Some(feature) = reg.driver_feature.get(sel as usize) {
-                    feature.store(val as u32, Ordering::Release);
+                let sel = reg.driver_feature_sel.load(Ordering::Acquire) as usize;
+                if let Some(feature) = reg.driver_feature.get(sel) {
+                    let status = DevStatus::from_bits_retain(reg.status.load(Ordering::Acquire));
+                    if status.contains(DevStatus::FEATURES_OK) {
+                        // VirtIO spec 1.4, Sec 3.1.1
+                        log::warn!(
+                            "{}: cannot change driver features after FEATURES_OK",
+                            self.name
+                        );
+                    } else {
+                        feature.store(val as u32 & reg.device_feature[sel], Ordering::Release);
+                    }
                 } else if val != 0 {
                     log::error!("{}: unknown feature {val:#x} for sel {sel}", self.name);
                 }
@@ -418,26 +427,28 @@ where
                 }
             }
             VirtioCommonCfg::LAYOUT_DEVICE_STATUS => {
-                let status = DevStatus::from_bits_truncate(val as u8);
-                let old = reg.status.swap(status.bits(), Ordering::AcqRel);
-                let old = DevStatus::from_bits_retain(old);
-                if (old ^ status).contains(DevStatus::DRIVER_OK) {
-                    let event = if status.contains(DevStatus::DRIVER_OK) {
-                        let mut feature = 0;
-                        for (i, v) in reg.driver_feature.iter().enumerate() {
-                            feature |= (v.load(Ordering::Acquire) as u128) << (i << 5);
-                        }
-                        let param = StartParam {
-                            feature,
-                            irq_sender: self.irq_sender.clone(),
-                            ioeventfds: self.ioeventfds.clone(),
-                        };
-                        WakeEvent::Start { param }
-                    } else {
-                        self.reset();
-                        WakeEvent::Reset
+                let status = DevStatus::from_bits_retain(val as u8);
+                let old = DevStatus::from_bits_retain(reg.status.load(Ordering::Acquire));
+                // Allows multi-step transitions in a single MMIO write
+                if !(status.is_valid() && status.contains(old) || status.is_empty()) {
+                    log::warn!("{}: invalid status change: {old:#x} -> {val:#x}", self.name);
+                    return Ok(Action::None);
+                }
+                reg.status.store(status.bits(), Ordering::Release);
+                if status.is_empty() {
+                    self.reset();
+                    if old.contains(DevStatus::DRIVER_OK) {
+                        self.wake_up_dev(WakeEvent::Reset);
+                    }
+                } else if !old.contains(DevStatus::DRIVER_OK)
+                    && status.contains(DevStatus::DRIVER_OK)
+                {
+                    let param = StartParam {
+                        feature: reg.get_driver_feature(),
+                        irq_sender: self.irq_sender.clone(),
+                        ioeventfds: self.ioeventfds.clone(),
                     };
-                    self.wake_up_dev(event);
+                    self.wake_up_dev(WakeEvent::Start { param });
                 }
             }
             VirtioCommonCfg::LAYOUT_QUEUE_SELECT => {
@@ -481,7 +492,14 @@ where
             VirtioCommonCfg::LAYOUT_QUEUE_DESC_LO => {
                 let q_sel = reg.queue_sel.load(Ordering::Relaxed);
                 if let Some(q) = self.queues.get(q_sel as usize) {
-                    set_atomic_low32(&q.desc, val as u32)
+                    if val.is_multiple_of(16) {
+                        set_atomic_low32(&q.desc, val as u32)
+                    } else {
+                        log::warn!(
+                            "{}: queue {q_sel}: unaligned descriptor area: {val:x}",
+                            self.name
+                        )
+                    }
                 }
             }
             VirtioCommonCfg::LAYOUT_QUEUE_DESC_HI => {
@@ -493,7 +511,20 @@ where
             VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO => {
                 let q_sel = reg.queue_sel.load(Ordering::Relaxed);
                 if let Some(q) = self.queues.get(q_sel as usize) {
-                    set_atomic_low32(&q.driver, val as u32)
+                    let feat = VirtioFeature::from_bits_retain(reg.get_driver_feature());
+                    let align = if feat.contains(VirtioFeature::RING_PACKED) {
+                        4
+                    } else {
+                        2
+                    };
+                    if val.is_multiple_of(align) {
+                        set_atomic_low32(&q.driver, val as u32);
+                    } else {
+                        log::warn!(
+                            "{}: queue {q_sel}: unaligned driver area: {val:x}",
+                            self.name
+                        );
+                    }
                 }
             }
             VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_HI => {
@@ -505,7 +536,14 @@ where
             VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_LO => {
                 let q_sel = reg.queue_sel.load(Ordering::Relaxed);
                 if let Some(q) = self.queues.get(q_sel as usize) {
-                    set_atomic_low32(&q.device, val as u32)
+                    if val.is_multiple_of(4) {
+                        set_atomic_low32(&q.device, val as u32)
+                    } else {
+                        log::warn!(
+                            "{}: queue {q_sel}: unaligned device area: {val:x}",
+                            self.name
+                        )
+                    }
                 }
             }
             VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_HI => {