refactor: more descriptive used ring usage calculations

Replace offset calculations with hard coded values with
more descriptive `std::mem::sizeof` with better comments
of why the offset should be calculated this way.
Additionally replace setting used element fields one by one
with single call to `write_obj`.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/devices/virtio/queue.rs

@@ -9,6 +9,8 @@ use std::cmp::min;
 use std::num::Wrapping;
 use std::sync::atomic::{fence, Ordering};
 
+use utils::usize_to_u64;
+
 use crate::logger::error;
 use crate::vstate::memory::{
     Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap,
@@ -49,6 +51,17 @@ struct Descriptor {
 // SAFETY: `Descriptor` is a POD and contains no padding.
 unsafe impl ByteValued for Descriptor {}
 
+/// A virtio used element in the used ring.
+#[repr(C)]
+#[derive(Default, Clone, Copy)]
+struct UsedElement {
+    id: u32,
+    len: u32,
+}
+
+// SAFETY: `UsedElement` is a POD and contains no padding.
+unsafe impl ByteValued for UsedElement {}
+
 /// A virtio descriptor chain.
 #[derive(Debug)]
 pub struct DescriptorChain<'a, M: GuestMemory = GuestMemoryMmap> {
@@ -430,31 +443,52 @@ impl Queue {
     ) -> Result<(), QueueError> {
         debug_assert!(self.is_layout_valid(mem));
 
-        if desc_index >= self.actual_size() {
-            error!(
-                "attempted to add out of bounds descriptor to used ring: {}",
-                desc_index
-            );
-            return Err(QueueError::DescIndexOutOfBounds(desc_index));
-        }
-
-        let used_ring = self.used_ring;
-        let next_used = u64::from(self.next_used.0 % self.actual_size());
-        let used_elem = used_ring.unchecked_add(4 + next_used * 8);
-
-        mem.write_obj(u32::from(desc_index), used_elem)?;
-
-        let len_addr = used_elem.unchecked_add(4);
-        mem.write_obj(len, len_addr)?;
+        let next_used = self.next_used.0 % self.actual_size();
+        self.write_used_ring(mem, next_used, desc_index, len)?;
 
         self.num_added += Wrapping(1);
         self.next_used += Wrapping(1);
 
         // This fence ensures all descriptor writes are visible before the index update is.
         fence(Ordering::Release);
 
-        let next_used_addr = used_ring.unchecked_add(2);
-        mem.write_obj(self.next_used.0, next_used_addr)
+        self.set_next_used(self.next_used.0, mem);
+        Ok(())
+    }
+
+    fn write_used_ring<M: GuestMemory>(
+        &self,
+        mem: &M,
+        index: u16,
+        id: u16,
+        len: u32,
+    ) -> Result<(), QueueError> {
+        if id >= self.actual_size() {
+            error!(
+                "attempted to add out of bounds descriptor to used ring: {}",
+                id
+            );
+            return Err(QueueError::DescIndexOutOfBounds(id));
+        }
+
+        // Used ring has layout:
+        // struct UsedRing {
+        //     flags: u16,
+        //     idx: u16,
+        //     ring: [UsedElement; <queue size>],
+        //     avail_event: u16,
+        // }
+        // We calculate offset into `ring` field.
+        let used_ring_offset = std::mem::size_of::<u16>()
+            + std::mem::size_of::<u16>()
+            + std::mem::size_of::<UsedElement>() * usize::from(index);
+        let used_element_address = self.used_ring.unchecked_add(usize_to_u64(used_ring_offset));
+        let used_element = UsedElement {
+            id: u32::from(id),
+            len,
+        };
+
+        mem.write_obj(used_element, used_element_address)
             .map_err(QueueError::UsedRing)
     }
 
@@ -488,13 +522,40 @@ impl Queue {
     fn set_avail_event<M: GuestMemory>(&mut self, val: u16, mem: &M) {
         debug_assert!(self.is_layout_valid(mem));
 
+        // Used ring has layout:
+        // struct UsedRing {
+        //     flags: u16,
+        //     idx: u16,
+        //     ring: [UsedElement; <queue size>],
+        //     avail_event: u16,
+        // }
+        // We calculate offset into `avail_event` field.
+        let avail_event_offset = std::mem::size_of::<u16>()
+            + std::mem::size_of::<u16>()
+            + std::mem::size_of::<UsedElement>() * usize::from(self.actual_size());
         let avail_event_addr = self
             .used_ring
-            .unchecked_add(u64::from(4 + 8 * self.actual_size()));
+            .unchecked_add(usize_to_u64(avail_event_offset));
 
         mem.write_obj(val, avail_event_addr).unwrap();
     }
 
+    fn set_next_used<M: GuestMemory>(&mut self, val: u16, mem: &M) {
+        debug_assert!(self.is_layout_valid(mem));
+
+        // Used ring has layout:
+        // struct UsedRing {
+        //     flags: u16,
+        //     idx: u16,
+        //     ring: [UsedElement; <queue size>],
+        //     avail_event: u16,
+        // }
+        // We calculate offset into `idx` field.
+        let idx_offset = std::mem::size_of::<u16>();
+        let next_used_addr = self.used_ring.unchecked_add(usize_to_u64(idx_offset));
+        mem.write_obj(val, next_used_addr).unwrap();
+    }
+
     /// Try to enable notification events from the guest driver. Returns true if notifications were
     /// successfully enabled. Otherwise it means that one or more descriptors can still be consumed
     /// from the available ring and we can't guarantee that there will be a notification. In this