Skip to content

Commit

Permalink
devices: vsock: pass memory during activate
Browse files Browse the repository at this point in the history
Allow Vsock creation without guest memory. Access to memory will
be given to the device during its activation.

Signed-off-by: Adrian Catangiu <acatan@amazon.com>
  • Loading branch information
acatangiu committed Mar 24, 2020
1 parent 8a51967 commit ad12fa5
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 32 deletions.
42 changes: 26 additions & 16 deletions src/devices/src/virtio/vsock/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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:
Expand All @@ -77,7 +76,6 @@ where
{
pub(crate) fn with_queues(
cid: u64,
mem: GuestMemoryMmap,
backend: B,
queues: Vec<VirtQueue>,
) -> super::Result<Vsock<B>> {
Expand All @@ -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 {
Expand All @@ -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() {
Expand All @@ -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
Expand All @@ -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;
}
};
Expand All @@ -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
Expand Down Expand Up @@ -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 {}",
Expand All @@ -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,
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/devices/src/virtio/vsock/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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));
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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));
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down
20 changes: 10 additions & 10 deletions src/devices/src/virtio/vsock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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(),
}
}

Expand Down Expand Up @@ -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(),
}
}
}
Expand All @@ -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
Expand Down
8 changes: 2 additions & 6 deletions src/vmm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ad12fa5

Please sign in to comment.