DO NOT MERGE test

src/firecracker/examples/uffd/uffd_utils.rs

@@ -191,7 +191,7 @@ impl UffdHandler {
 
     fn zero_out(&mut self, addr: u64) -> bool {
         match unsafe { self.uffd.zeropage(addr as *mut _, self.page_size, true) } {
-            Ok(r) if r >= 0 => true,
+            Ok(_) => true,
             Err(Error::ZeropageFailed(error)) if error as i32 == libc::EAGAIN => false,
             r => panic!("Unexpected zeropage result: {:?}", r),
         }

src/vmm/src/devices/virtio/balloon/device.rs

@@ -362,6 +362,7 @@ impl Balloon {
                 }
             }
         }
+        queue.advance_used_ring_idx();
 
         if needs_interrupt {
             self.signal_used_queue()?;
@@ -380,6 +381,7 @@ impl Balloon {
             queue.add_used(head.index, 0)?;
             needs_interrupt = true;
         }
+        queue.advance_used_ring_idx();
 
         if needs_interrupt {
             self.signal_used_queue()
@@ -453,6 +455,7 @@ impl Balloon {
         // and sending a used buffer notification
         if let Some(index) = self.stats_desc_index.take() {
             self.queues[STATS_INDEX].add_used(index, 0)?;
+            self.queues[STATS_INDEX].advance_used_ring_idx();
             self.signal_used_queue()
         } else {
             error!("Failed to update balloon stats, missing descriptor.");
@@ -606,8 +609,7 @@ impl VirtioDevice for Balloon {
 
     fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> {
         for q in self.queues.iter_mut() {
-            q.initialize(&mem)
-                .map_err(ActivateError::QueueMemoryError)?;
+            q.initialize(&mem).map_err(ActivateError::QueueError)?;
         }
 
         self.device_state = DeviceState::Activated(mem);

src/vmm/src/devices/virtio/block/vhost_user/device.rs

@@ -332,8 +332,7 @@ impl<T: VhostUserHandleBackend + Send + 'static> VirtioDevice for VhostUserBlock
 
     fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> {
         for q in self.queues.iter_mut() {
-            q.initialize(&mem)
-                .map_err(ActivateError::QueueMemoryError)?;
+            q.initialize(&mem).map_err(ActivateError::QueueError)?;
         }
 
         let start_time = get_time_us(ClockType::Monotonic);

src/vmm/src/devices/virtio/block/virtio/device.rs

@@ -12,6 +12,7 @@
 use std::os::linux::fs::MetadataExt;
 use std::path::PathBuf;
 use std::sync::Arc;
+use std::time::Duration;
 
 use block_io::FileEngine;
 use serde::{Deserialize, Serialize};
@@ -384,24 +385,6 @@
         }
     }
 
-    fn add_used_descriptor(
-        queue: &mut Queue,
-        index: u16,
-        len: u32,
-        irq_trigger: &IrqTrigger,
-        block_metrics: &BlockDeviceMetrics,
-    ) {
-        queue.add_used(index, len).unwrap_or_else(|err| {
-            error!("Failed to add available descriptor head {}: {}", index, err)
-        });
-
-        if queue.prepare_kick() {
-            irq_trigger.trigger_irq(IrqType::Vring).unwrap_or_else(|_| {
-                block_metrics.event_fails.inc();
-            });
-        }
-    }
-
     /// Device specific function for peaking inside a queue and processing descriptors.
     pub fn process_queue(&mut self, queue_index: usize) -> Result<(), InvalidAvailIdx> {
         // This is safe since we checked in the event handler that the device is activated.
@@ -423,6 +406,11 @@
                     }
 
                     used_any = true;
+                    if self.id == "scratch"
+                        && (request.r#type == RequestType::In || request.r#type == RequestType::Out)
+                    {
+                        std::thread::sleep(Duration::from_millis(60));
+                    }
                     request.process(&mut self.disk, head.index, mem, &self.metrics)
                 }
                 Err(err) => {
@@ -443,16 +431,26 @@
                     break;
                 }
                 ProcessingResult::Executed(finished) => {
-                    Self::add_used_descriptor(
-                        queue,
-                        head.index,
-                        finished.num_bytes_to_mem,
-                        &self.irq_trigger,
-                        &self.metrics,
-                    );
+                    queue
+                        .add_used(head.index, finished.num_bytes_to_mem)
+                        .unwrap_or_else(|err| {
+                            error!(
+                                "Failed to add available descriptor head {}: {}",
+                                head.index, err
+                            )
+                        });
                 }
             }
         }
+        queue.advance_used_ring_idx();
+
+        if queue.prepare_kick() {
+            self.irq_trigger
+                .trigger_irq(IrqType::Vring)
+                .unwrap_or_else(|_| {
+                    self.metrics.event_fails.inc();
+                });
+        }
 
         if let FileEngine::Async(ref mut engine) = self.disk.file_engine {
             if let Err(err) = engine.kick_submission_queue() {
@@ -495,17 +493,26 @@
                         ),
                     };
                     let finished = pending.finish(mem, res, &self.metrics);
-
-                    Self::add_used_descriptor(
-                        queue,
-                        finished.desc_idx,
-                        finished.num_bytes_to_mem,
-                        &self.irq_trigger,
-                        &self.metrics,
-                    );
+                    queue
+                        .add_used(finished.desc_idx, finished.num_bytes_to_mem)
+                        .unwrap_or_else(|err| {
+                            error!(
+                                "Failed to add available descriptor head {}: {}",
+                                finished.desc_idx, err
+                            )
+                        });
                 }
             }
         }
+        queue.advance_used_ring_idx();
+
+        if queue.prepare_kick() {
+            self.irq_trigger
+                .trigger_irq(IrqType::Vring)
+                .unwrap_or_else(|_| {
+                    self.metrics.event_fails.inc();
+                });
+        }
     }
 
     pub fn process_async_completion_event(&mut self) {
@@ -628,8 +635,7 @@
 
     fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> {
         for q in self.queues.iter_mut() {
-            q.initialize(&mem)
-                .map_err(ActivateError::QueueMemoryError)?;
+            q.initialize(&mem).map_err(ActivateError::QueueError)?;
         }
 
         let event_idx = self.has_feature(u64::from(VIRTIO_RING_F_EVENT_IDX));
@@ -1573,14 +1579,14 @@
 
             // Run scenario that doesn't trigger FullSq BlockError: Add sq_size flush requests.
             add_flush_requests_batch(&mut block, &vq, IO_URING_NUM_ENTRIES);
-            simulate_queue_event(&mut block, Some(false));
+            simulate_queue_event(&mut block, Some(true));
             assert!(!block.is_io_engine_throttled);
             simulate_async_completion_event(&mut block, true);
             check_flush_requests_batch(IO_URING_NUM_ENTRIES, &vq);
 
             // Run scenario that triggers FullSqError : Add sq_size + 10 flush requests.
             add_flush_requests_batch(&mut block, &vq, IO_URING_NUM_ENTRIES + 10);
-            simulate_queue_event(&mut block, Some(false));
+            simulate_queue_event(&mut block, Some(true));
             assert!(block.is_io_engine_throttled);
             // When the async_completion_event is triggered:
             // 1. sq_size requests should be processed processed.
@@ -1607,16 +1613,16 @@
             // Run scenario that triggers FullCqError. Push 2 * IO_URING_NUM_ENTRIES and wait for
             // completion. Then try to push another entry.
             add_flush_requests_batch(&mut block, &vq, IO_URING_NUM_ENTRIES);
-            simulate_queue_event(&mut block, Some(false));
+            simulate_queue_event(&mut block, Some(true));
             assert!(!block.is_io_engine_throttled);
             thread::sleep(Duration::from_millis(150));
             add_flush_requests_batch(&mut block, &vq, IO_URING_NUM_ENTRIES);
-            simulate_queue_event(&mut block, Some(false));
+            simulate_queue_event(&mut block, Some(true));
             assert!(!block.is_io_engine_throttled);
             thread::sleep(Duration::from_millis(150));
 
             add_flush_requests_batch(&mut block, &vq, 1);
-            simulate_queue_event(&mut block, Some(false));
+            simulate_queue_event(&mut block, Some(true));
             assert!(block.is_io_engine_throttled);
             simulate_async_completion_event(&mut block, true);
             assert!(!block.is_io_engine_throttled);
@@ -1672,13 +1678,15 @@
             vq.dtable[1].len.set(512);
             mem.write_obj::<u64>(123_456_789, data_addr).unwrap();
 
-            // Following write procedure should fail because of bandwidth rate limiting.
+            // This will fail because of bandwidth rate limiting.
+            // The irq is still triggered because notification suppression
+            // is not enabled.
             {
                 // Trigger the attempt to write.
                 check_metric_after_block!(
                     &block.metrics.rate_limiter_throttled_events,
                     1,
-                    simulate_queue_event(&mut block, Some(false))
+                    simulate_queue_event(&mut block, Some(true))
                 );
 
                 // Assert that limiter is blocked.
@@ -1740,13 +1748,15 @@
             vq.dtable[1].len.set(512);
             mem.write_obj::<u64>(123_456_789, data_addr).unwrap();
 
-            // Following write procedure should fail because of ops rate limiting.
+            // This will fail because of ops rate limiting.
+            // The irq is still triggered because notification suppression
+            // is not enabled.
             {
                 // Trigger the attempt to write.
                 check_metric_after_block!(
                     &block.metrics.rate_limiter_throttled_events,
                     1,
-                    simulate_queue_event(&mut block, Some(false))
+                    simulate_queue_event(&mut block, Some(true))
                 );
 
                 // Assert that limiter is blocked.
@@ -1755,7 +1765,8 @@
                 assert_eq!(vq.used.idx.get(), 0);
             }
 
-            // Do a second write that still fails but this time on the fast path.
+            // Do a second write that still fails but this time on the fast path
+            // which does not call `process_queue`, so no irq notifications.
             {
                 // Trigger the attempt to write.
                 check_metric_after_block!(

src/vmm/src/devices/virtio/block/virtio/event_handler.rs

@@ -1,6 +1,7 @@
 // Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 use event_manager::{EventOps, Events, MutEventSubscriber};
+use log::info;
 use vmm_sys_util::epoll::EventSet;
 
 use super::io::FileEngine;
@@ -85,7 +86,15 @@
         if self.is_activated() {
             match source {
                 Self::PROCESS_ACTIVATE => self.process_activate_event(ops),
-                Self::PROCESS_QUEUE => self.process_queue_event(),
+                Self::PROCESS_QUEUE => {
+                    let tstamp = std::time::Instant::now();
+                    self.process_queue_event();
+                    info!(
+                        "block[{}]: processed queue for {} usec",
+                        &self.id,
+                        tstamp.elapsed().as_micros()
+                    );
+                }
                 Self::PROCESS_RATE_LIMITER => self.process_rate_limiter_event(),
                 Self::PROCESS_ASYNC_COMPLETION => self.process_async_completion_event(),
                 _ => warn!("Block: Spurious event received: {:?}", source),

src/vmm/src/devices/virtio/mmio.rs

@@ -157,7 +157,7 @@
         //   eventfds, but nothing will happen other than supurious wakeups.
         // . Do not reset config_generation and keep it monotonically increasing
         for queue in self.locked_device().queues_mut() {
-            *queue = Queue::new(queue.get_max_size());
+            *queue = Queue::new(queue.max_size);
         }
     }
 
@@ -253,7 +253,7 @@
                         }
                         features
                     }
-                    0x34 => self.with_queue(0, |q| u32::from(q.get_max_size())),
+                    0x34 => self.with_queue(0, |q| u32::from(q.max_size)),
                     0x44 => self.with_queue(0, |q| u32::from(q.ready)),
                     0x60 => {
                         // For vhost-user backed devices we need some additional
@@ -489,17 +489,17 @@
         assert!(!d.are_queues_valid());
 
         d.queue_select = 0;
-        assert_eq!(d.with_queue(0, Queue::get_max_size), 16);
+        assert_eq!(d.with_queue(0, |q| q.max_size), 16);
         assert!(d.with_queue_mut(|q| q.size = 16));
         assert_eq!(d.locked_device().queues()[d.queue_select as usize].size, 16);
 
         d.queue_select = 1;
-        assert_eq!(d.with_queue(0, Queue::get_max_size), 32);
+        assert_eq!(d.with_queue(0, |q| q.max_size), 32);
         assert!(d.with_queue_mut(|q| q.size = 16));
         assert_eq!(d.locked_device().queues()[d.queue_select as usize].size, 16);
 
         d.queue_select = 2;
-        assert_eq!(d.with_queue(0, Queue::get_max_size), 0);
+        assert_eq!(d.with_queue(0, |q| q.max_size), 0);
         assert!(!d.with_queue_mut(|q| q.size = 16));
 
         assert!(!d.are_queues_valid());

src/vmm/src/devices/virtio/mod.rs

@@ -72,8 +72,8 @@ pub enum ActivateError {
     VhostUser(vhost_user::VhostUserError),
     /// Setting tap interface offload flags failed: {0}
     TapSetOffload(TapError),
-    /// Error setting pointers in the queue: (0)
-    QueueMemoryError(QueueError),
+    /// Error initializing the queue: (0)
+    QueueError(QueueError),
 }
 
 /// Trait that helps in upcasting an object to Any

src/vmm/src/devices/virtio/net/device.rs

@@ -207,7 +207,7 @@ impl RxBuffers {
     /// This will let the guest know that about all the `DescriptorChain` object that has been
     /// used to receive a frame from the TAP.
     fn finish_frame(&mut self, rx_queue: &mut Queue) {
-        rx_queue.advance_used_ring(self.used_descriptors);
+        rx_queue.advance_next_used(self.used_descriptors);
         self.used_descriptors = 0;
         self.used_bytes = 0;
     }
@@ -396,6 +396,7 @@ impl Net {
             NetQueue::Rx => &mut self.queues[RX_INDEX],
             NetQueue::Tx => &mut self.queues[TX_INDEX],
         };
+        queue.advance_used_ring_idx();
 
         if queue.prepare_kick() {
             self.irq_trigger
@@ -1000,8 +1001,7 @@ impl VirtioDevice for Net {
 
     fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> {
         for q in self.queues.iter_mut() {
-            q.initialize(&mem)
-                .map_err(ActivateError::QueueMemoryError)?;
+            q.initialize(&mem).map_err(ActivateError::QueueError)?;
         }
 
         let event_idx = self.has_feature(u64::from(VIRTIO_RING_F_EVENT_IDX));
@@ -1070,6 +1070,7 @@ pub mod tests {
     impl Net {
         pub fn finish_frame(&mut self) {
             self.rx_buffer.finish_frame(&mut self.queues[RX_INDEX]);
+            self.queues[RX_INDEX].advance_used_ring_idx();
         }
     }
 

src/vmm/src/devices/virtio/net/event_handler.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use event_manager::{EventOps, Events, MutEventSubscriber};
+use log::info;
 use vmm_sys_util::epoll::EventSet;
 
 use crate::devices::virtio::device::VirtioDevice;
@@ -97,6 +98,7 @@
         }
 
         if self.is_activated() {
+            let tstamp = std::time::Instant::now();
             match source {
                 Self::PROCESS_ACTIVATE => self.process_activate_event(ops),
                 Self::PROCESS_VIRTQ_RX => self.process_rx_queue_event(),
@@ -109,6 +111,10 @@
                     self.metrics.event_fails.inc();
                 }
             }
+            info!(
+                "net: processed queue for {} usec",
+                tstamp.elapsed().as_micros()
+            );
         } else {
             warn!(
                 "Net: The device is not yet activated. Spurious event received: {:?}",