devices: vsock: pass memory during activate

acatangiu · iulianbarbu · commit 8f2d564df62f · 2020-04-01T12:48:21.000+03:00
Allow Vsock creation without guest memory. Access to memory will
be given to the device during its activation.

Signed-off-by: Adrian Catangiu &lt;acatan@amazon.com&gt;
diff --git a/src/devices/src/virtio/vsock/device.rs b/src/devices/src/virtio/vsock/device.rs
@@ -29,7 +29,7 @@ use vm_memory::GuestMemoryMmap;
 
 use super::super::super::Error as DeviceError;
 use super::super::{
-    ActivateError, ActivateResult, Queue as VirtQueue, VirtioDevice, VsockError,
+    ActivateError, ActivateResult, DeviceState, Queue as VirtQueue, VirtioDevice, VsockError,
     VIRTIO_MMIO_INT_VRING,
 };
 use super::packet::VsockPacket;
@@ -51,7 +51,6 @@ pub struct Vsock<B> {
     cid: u64,
     pub(crate) queues: Vec<VirtQueue>,
     pub(crate) queue_events: Vec<EventFd>,
-    mem: GuestMemoryMmap,
     pub(crate) backend: B,
     avail_features: u64,
     acked_features: u64,
@@ -63,7 +62,7 @@ pub struct Vsock<B> {
     // mostly something we wanted to happen for the backend events, to prevent (potentially)
     // continuous triggers from happening before the device gets activated.
     pub(crate) activate_evt: EventFd,
-    device_activated: bool,
+    pub(crate) device_state: DeviceState,
 }
 
 // TODO: Detect / handle queue deadlock:
@@ -77,7 +76,6 @@ where
 {
     pub(crate) fn with_queues(
         cid: u64,
-        mem: GuestMemoryMmap,
         backend: B,
         queues: Vec<VirtQueue>,
     ) -> super::Result<Vsock<B>> {
@@ -90,24 +88,23 @@ where
             cid,
             queues,
             queue_events,
-            mem,
             backend,
             avail_features: AVAIL_FEATURES,
             acked_features: 0,
             interrupt_status: Arc::new(AtomicUsize::new(0)),
             interrupt_evt: EventFd::new(libc::EFD_NONBLOCK).map_err(VsockError::EventFd)?,
             activate_evt: EventFd::new(libc::EFD_NONBLOCK).map_err(VsockError::EventFd)?,
-            device_activated: false,
+            device_state: DeviceState::Inactive,
         })
     }
 
     /// Create a new virtio-vsock device with the given VM CID and vsock backend.
-    pub fn new(cid: u64, mem: GuestMemoryMmap, backend: B) -> super::Result<Vsock<B>> {
+    pub fn new(cid: u64, backend: B) -> super::Result<Vsock<B>> {
         let queues: Vec<VirtQueue> = defs::QUEUE_SIZES
             .iter()
             .map(|&max_size| VirtQueue::new(max_size))
             .collect();
-        Self::with_queues(cid, mem, backend, queues)
+        Self::with_queues(cid, backend, queues)
     }
 
     pub fn cid(&self) -> u64 {
@@ -131,10 +128,15 @@ where
     /// otherwise.
     pub fn process_rx(&mut self) -> bool {
         debug!("vsock: process_rx()");
+        let mem = match self.device_state {
+            DeviceState::Activated(ref mem) => mem,
+            // This should never happen, it's been already validated in the event handler.
+            DeviceState::Inactive => return false,
+        };
 
         let mut have_used = false;
 
-        while let Some(head) = self.queues[RXQ_INDEX].pop(&self.mem) {
+        while let Some(head) = self.queues[RXQ_INDEX].pop(mem) {
             let used_len = match VsockPacket::from_rx_virtq_head(&head) {
                 Ok(mut pkt) => {
                     if self.backend.recv_pkt(&mut pkt).is_ok() {
@@ -153,7 +155,7 @@ where
             };
 
             have_used = true;
-            self.queues[RXQ_INDEX].add_used(&self.mem, head.index, used_len);
+            self.queues[RXQ_INDEX].add_used(mem, head.index, used_len);
         }
 
         have_used
@@ -164,16 +166,21 @@ where
     /// ring, and `false` otherwise.
     pub fn process_tx(&mut self) -> bool {
         debug!("vsock::process_tx()");
+        let mem = match self.device_state {
+            DeviceState::Activated(ref mem) => mem,
+            // This should never happen, it's been already validated in the event handler.
+            DeviceState::Inactive => return false,
+        };
 
         let mut have_used = false;
 
-        while let Some(head) = self.queues[TXQ_INDEX].pop(&self.mem) {
+        while let Some(head) = self.queues[TXQ_INDEX].pop(mem) {
             let pkt = match VsockPacket::from_tx_virtq_head(&head) {
                 Ok(pkt) => pkt,
                 Err(e) => {
                     error!("vsock: error reading TX packet: {:?}", e);
                     have_used = true;
-                    self.queues[TXQ_INDEX].add_used(&self.mem, head.index, 0);
+                    self.queues[TXQ_INDEX].add_used(mem, head.index, 0);
                     continue;
                 }
             };
@@ -184,7 +191,7 @@ where
             }
 
             have_used = true;
-            self.queues[TXQ_INDEX].add_used(&self.mem, head.index, 0);
+            self.queues[TXQ_INDEX].add_used(mem, head.index, 0);
         }
 
         have_used
@@ -252,7 +259,7 @@ where
         );
     }
 
-    fn activate(&mut self, _: GuestMemoryMmap) -> ActivateResult {
+    fn activate(&mut self, mem: GuestMemoryMmap) -> ActivateResult {
         if self.queues.len() != defs::NUM_QUEUES {
             error!(
                 "Cannot perform activate. Expected {} queue(s), got {}",
@@ -267,13 +274,16 @@ where
             return Err(ActivateError::BadActivate);
         }
 
-        self.device_activated = true;
+        self.device_state = DeviceState::Activated(mem);
 
         Ok(())
     }
 
     fn is_activated(&self) -> bool {
-        self.device_activated
+        match self.device_state {
+            DeviceState::Inactive => false,
+            DeviceState::Activated(_) => true,
+        }
     }
 }
 
diff --git a/src/devices/src/virtio/vsock/event_handler.rs b/src/devices/src/virtio/vsock/event_handler.rs
@@ -268,6 +268,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             ctx.device.backend.set_pending_rx(false);
             ctx.signal_txq_event();
@@ -284,6 +285,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             ctx.device.backend.set_pending_rx(true);
             ctx.signal_txq_event();
@@ -299,6 +301,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             ctx.device.backend.set_pending_rx(false);
             ctx.device.backend.set_tx_err(Some(VsockError::NoData));
@@ -314,6 +317,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             // Invalidate the packet header descriptor, by setting its length to 0.
             ctx.guest_txvq.dtable[0].len.set(0);
@@ -329,6 +333,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             assert!(!ctx
                 .device
@@ -345,6 +350,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             ctx.device.backend.set_pending_rx(true);
             ctx.device.backend.set_rx_err(Some(VsockError::NoData));
@@ -361,6 +367,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             ctx.device.backend.set_pending_rx(true);
             ctx.signal_rxq_event();
@@ -373,6 +380,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             // Invalidate the packet header descriptor, by setting its length to 0.
             ctx.guest_rxvq.dtable[0].len.set(0);
@@ -387,6 +395,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
             ctx.device.backend.set_pending_rx(false);
             assert!(!ctx
                 .device
@@ -415,6 +424,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             ctx.device.backend.set_pending_rx(true);
             ctx.device.notify_backend(&EpollEvent::new(EventSet::IN, 0));
@@ -433,6 +443,7 @@ mod tests {
         {
             let test_ctx = TestContext::new();
             let mut ctx = test_ctx.create_event_handler_context();
+            ctx.mock_activate(test_ctx.mem.clone());
 
             ctx.device.backend.set_pending_rx(false);
             ctx.device.notify_backend(&EpollEvent::new(EventSet::IN, 0));
diff --git a/src/devices/src/virtio/vsock/mod.rs b/src/devices/src/virtio/vsock/mod.rs
@@ -159,7 +159,7 @@ mod tests {
     use utils::eventfd::EventFd;
 
     use crate::virtio::queue::tests::VirtQueue as GuestQ;
-    use crate::virtio::{VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE};
+    use crate::virtio::{VirtioDevice, VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE};
     use utils::epoll::EpollEvent;
     use vm_memory::{GuestAddress, GuestMemoryMmap};
 
@@ -258,9 +258,9 @@ mod tests {
             let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(0), MEM_SIZE)]).unwrap();
             Self {
                 cid: CID,
-                mem: mem.clone(),
+                mem,
                 mem_size: MEM_SIZE,
-                device: Vsock::new(CID, mem, TestBackend::new()).unwrap(),
+                device: Vsock::new(CID, TestBackend::new()).unwrap(),
             }
         }
 
@@ -296,13 +296,8 @@ mod tests {
                 guest_rxvq,
                 guest_txvq,
                 guest_evvq,
-                device: Vsock::with_queues(
-                    self.cid,
-                    self.mem.clone(),
-                    TestBackend::new(),
-                    vec![rxvq, txvq, evvq],
-                )
-                .unwrap(),
+                device: Vsock::with_queues(self.cid, TestBackend::new(), vec![rxvq, txvq, evvq])
+                    .unwrap(),
             }
         }
     }
@@ -315,6 +310,11 @@ mod tests {
     }
 
     impl<'a> EventHandlerContext<'a> {
+        pub fn mock_activate(&mut self, mem: GuestMemoryMmap) {
+            // Artificially activate the device.
+            self.device.activate(mem).unwrap();
+        }
+
         pub fn signal_txq_event(&mut self) {
             self.device.queue_events[TXQ_INDEX].write(1).unwrap();
             self.device
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
@@ -794,12 +794,8 @@ fn attach_vsock_device(
     .map_err(CreateVsockBackend)?;
 
     let vsock_device = Arc::new(Mutex::new(
-        devices::virtio::Vsock::new(
-            u64::from(vsock.guest_cid),
-            vmm.guest_memory().clone(),
-            backend,
-        )
-        .map_err(CreateVsockDevice)?,
+        devices::virtio::Vsock::new(u64::from(vsock.guest_cid), backend)
+            .map_err(CreateVsockDevice)?,
     ));
 
     event_manager
