RFC: extend Queue to support packed queue

benches/queue/mock.rs

@@ -260,11 +260,11 @@ impl<'a, M: GuestMemory> MockSplitQueue<'a, M> {
         self.update_avail_idx(head_idx);
     }
 
-    pub fn create_queue<A: GuestAddressSpace>(&self, a: A) -> Queue<A> {
+    pub fn create_queue<A: GuestAddressSpace + Clone>(&self, a: A) -> Queue<A> {
         let mut q = Queue::new(a, self.len);
-        q.desc_table = self.desc_table_addr;
-        q.avail_ring = self.avail_addr;
-        q.used_ring = self.used_addr;
+        q.set_desc_table_address(self.desc_table_addr);
+        q.set_avail_ring_address(self.avail_addr);
+        q.set_used_ring_address(self.used_addr);
         q
     }
 }

benches/queue/mod.rs

@@ -5,7 +5,7 @@ mod mock;
 
 use criterion::{black_box, BatchSize, Criterion};
 use vm_memory::{GuestAddress, GuestAddressSpace, GuestMemoryAtomic, GuestMemoryMmap};
-use vm_virtio::Queue;
+use vm_virtio::{DescriptorChainUsed, Queue};
 
 use mock::MockSplitQueue;
 
@@ -86,7 +86,7 @@ pub fn benchmark_queue(c: &mut Criterion) {
         || empty_queue(),
         |mut q| {
             for _ in 0..128 {
-                q.add_used(123, 0x1000).unwrap();
+                q.add_used(123, DescriptorChainUsed::Split(0x1000)).unwrap();
             }
         },
     );

coverage_config_x86_64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 88.8,
+  "coverage_score": 90.5,
   "exclude_path": "",
   "crate_features": "backend-stdio"
 }

src/block/request.rs

@@ -231,7 +231,7 @@ impl Request {
 
         let mut desc = desc_chain.next().ok_or(Error::DescriptorChainTooShort)?;
 
-        while desc.has_next() {
+        while desc_chain.has_next(&desc) {
             Request::check_data_desc::<<M>::M>(desc_chain.memory(), desc, request.request_type)?;
 
             request.data.push((desc.addr(), desc.len()));

src/device/mmio.rs

@@ -98,7 +98,7 @@ pub trait VirtioMmioDevice<M: GuestAddressSpace>: WithDriverSelect<M> {
                         .into(),
                     0x44 => self
                         .selected_queue()
-                        .map(|q| q.ready)
+                        .map(|q| q.ready())
                         .unwrap_or(false)
                         .into(),
                     0x60 => self.interrupt_status().load(Ordering::SeqCst).into(),
@@ -158,8 +158,8 @@ pub trait VirtioMmioDevice<M: GuestAddressSpace>: WithDriverSelect<M> {
                     // data type specified by the virtio standard (we simply use `as` conversion
                     // for now).
                     0x30 => self.set_queue_select(v as u16),
-                    0x38 => update_queue_field(self, |q| q.size = v as u16),
-                    0x44 => update_queue_field(self, |q| q.ready = v == 1),
+                    0x38 => update_queue_field(self, |q| q.set_queue_size(v as u16)),
+                    0x44 => update_queue_field(self, |q| q.set_ready(v == 1)),
                     0x50 => self.queue_notify(v),
                     0x64 => {
                         if self.check_device_status(status::DRIVER_OK, 0) {
@@ -168,12 +168,36 @@ pub trait VirtioMmioDevice<M: GuestAddressSpace>: WithDriverSelect<M> {
                         }
                     }
                     0x70 => self.ack_device_status(v as u8),
-                    0x80 => update_queue_field(self, |q| set_low(&mut q.desc_table, v)),
-                    0x84 => update_queue_field(self, |q| set_high(&mut q.desc_table, v)),
-                    0x90 => update_queue_field(self, |q| set_low(&mut q.avail_ring, v)),
-                    0x94 => update_queue_field(self, |q| set_high(&mut q.avail_ring, v)),
-                    0xa0 => update_queue_field(self, |q| set_low(&mut q.used_ring, v)),
-                    0xa4 => update_queue_field(self, |q| set_high(&mut q.used_ring, v)),
+                    0x80 => update_queue_field(self, |q| {
+                        let mut addr = q.desc_table_address();
+                        set_low(&mut addr, v);
+                        q.set_desc_table_address(addr);
+                    }),
+                    0x84 => update_queue_field(self, |q| {
+                        let mut addr = q.desc_table_address();
+                        set_high(&mut addr, v);
+                        q.set_desc_table_address(addr);
+                    }),
+                    0x90 => update_queue_field(self, |q| {
+                        let mut addr = q.avail_ring_address();
+                        set_low(&mut addr, v);
+                        q.set_avail_ring_address(addr);
+                    }),
+                    0x94 => update_queue_field(self, |q| {
+                        let mut addr = q.avail_ring_address();
+                        set_high(&mut addr, v);
+                        q.set_avail_ring_address(addr);
+                    }),
+                    0xa0 => update_queue_field(self, |q| {
+                        let mut addr = q.used_ring_address();
+                        set_low(&mut addr, v);
+                        q.set_used_ring_address(addr);
+                    }),
+                    0xa4 => update_queue_field(self, |q| {
+                        let mut addr = q.used_ring_address();
+                        set_high(&mut addr, v);
+                        q.set_used_ring_address(addr);
+                    }),
                     _ => {
                         warn!("unknown virtio mmio register write: 0x{:x}", offset);
                     }
@@ -257,16 +281,16 @@ mod tests {
         // The max size for the queue in `Dummy` is 256.
         assert_eq!(mmio_read(&d, 0x34), 256);
 
-        assert_eq!(d.cfg.queues[0].size, 256);
+        assert_eq!(d.cfg.queues[0].queue_size(), 256);
         d.write(0x38, &32u32.to_le_bytes());
         // Updating the queue field has no effect due to invalid device status.
-        assert_eq!(d.cfg.queues[0].size, 256);
+        assert_eq!(d.cfg.queues[0].queue_size(), 256);
 
         d.cfg.device_status |= status::FEATURES_OK;
 
         // Let's try the update again.
         d.write(0x38, &32u32.to_le_bytes());
-        assert_eq!(d.cfg.queues[0].size, 32);
+        assert_eq!(d.cfg.queues[0].queue_size(), 32);
 
         // The queue in `Dummy` is not ready yet.
         assert_eq!(mmio_read(&d, 0x44), 0);
@@ -280,23 +304,23 @@ mod tests {
         d.write(0x50, &2u32.to_le_bytes());
         assert_eq!(d.last_queue_notify, 2);
 
-        assert_eq!(d.cfg.queues[0].desc_table.0, 0);
+        assert_eq!(d.cfg.queues[0].desc_table_address().0, 0);
         d.write(0x80, &1u32.to_le_bytes());
-        assert_eq!(d.cfg.queues[0].desc_table.0, 1);
+        assert_eq!(d.cfg.queues[0].desc_table_address().0, 1);
         d.write(0x84, &2u32.to_le_bytes());
-        assert_eq!(d.cfg.queues[0].desc_table.0, (2 << 32) + 1);
+        assert_eq!(d.cfg.queues[0].desc_table_address().0, (2 << 32) + 1);
 
-        assert_eq!(d.cfg.queues[0].avail_ring.0, 0);
+        assert_eq!(d.cfg.queues[0].avail_ring_address().0, 0);
         d.write(0x90, &1u32.to_le_bytes());
-        assert_eq!(d.cfg.queues[0].avail_ring.0, 1);
+        assert_eq!(d.cfg.queues[0].avail_ring_address().0, 1);
         d.write(0x94, &2u32.to_le_bytes());
-        assert_eq!(d.cfg.queues[0].avail_ring.0, (2 << 32) + 1);
+        assert_eq!(d.cfg.queues[0].avail_ring_address().0, (2 << 32) + 1);
 
-        assert_eq!(d.cfg.queues[0].used_ring.0, 0);
+        assert_eq!(d.cfg.queues[0].used_ring_address().0, 0);
         d.write(0xa0, &1u32.to_le_bytes());
-        assert_eq!(d.cfg.queues[0].used_ring.0, 1);
+        assert_eq!(d.cfg.queues[0].used_ring_address().0, 1);
         d.write(0xa4, &2u32.to_le_bytes());
-        assert_eq!(d.cfg.queues[0].used_ring.0, (2 << 32) + 1);
+        assert_eq!(d.cfg.queues[0].used_ring_address().0, (2 << 32) + 1);
 
         // Let's select a non-existent queue.
         d.write(0x30, &1u32.to_le_bytes());

src/device/mod.rs

@@ -281,7 +281,7 @@ mod tests {
             assert_eq!(d.cfg.device_features & (1 << VIRTIO_F_RING_EVENT_IDX), 0);
 
             for q in d.cfg.queues.iter() {
-                assert_eq!(q.event_idx_enabled, false);
+                assert_eq!(q.event_idx(), false);
             }
 
             // Revert status.
@@ -299,7 +299,7 @@ mod tests {
             assert_eq!(d.cfg.device_status, status);
 
             for q in d.cfg.queues.iter() {
-                assert_eq!(q.event_idx_enabled, true);
+                assert_eq!(q.event_idx(), true);
             }
         }