Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2f5c591
refactor(queue): rename queue fields
ShadowCurse Aug 23, 2024
d99f47e
feat(queue): add pointers to access queue internals
ShadowCurse Aug 23, 2024
a76a1df
refactor(test): update test_dirty_pages_after_full_snapshot
ShadowCurse Aug 29, 2024
6db1085
refactor(queue): update `is_valid` check
ShadowCurse Aug 23, 2024
f9c8dd4
refactor(queue): update `len` and `is_empty` methods
ShadowCurse Aug 23, 2024
4797c30
refactor(queue): remove `write_used_ring` method
ShadowCurse Aug 23, 2024
dd3389f
refactor(queue): remove `avail_idx` method
ShadowCurse Aug 23, 2024
eeef1b9
refactor(queue): remove `used_event` method
ShadowCurse Aug 23, 2024
bdee16d
refactor(queue): remove `set_used_ring_idx` method
ShadowCurse Aug 23, 2024
d9df08a
refactor(queue): remove `set_used_ring_avail_event` method
ShadowCurse Aug 23, 2024
032f4c2
refactor(queue): remove `mem` parameter from `add_used` method
ShadowCurse Aug 23, 2024
2e8013c
refactor(queue): simplify `do_pop_unchecked`
ShadowCurse Aug 23, 2024
33fc038
refactor(queue): update `DescriptorChain` implementation
ShadowCurse Aug 23, 2024
ffae4c0
refactor(queue): remove `mem` from `try_enable_notification` method
ShadowCurse Aug 23, 2024
255bbae
refactor(queue): remove `mem` from `pop` method
ShadowCurse Aug 23, 2024
e3904a5
refactor(queue): remove `mem` from `pop_or_enable_notification` method
ShadowCurse Aug 23, 2024
cf661ab
refactor(queue): remove `mem` from `prepare_kick` method
ShadowCurse Aug 23, 2024
cf7b793
refactor(queue): remove `avail_event` method from unit tests
ShadowCurse Aug 23, 2024
4d35b65
refactor(queue): remove `do` from `do_pop_unchecked`
ShadowCurse Aug 28, 2024
cd690e4
feat(queue): add kani proofs for new methods
ShadowCurse Sep 5, 2024
9b0a795
feat(queue): more generic kani proofs
ShadowCurse Sep 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vmm/benches/block_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn block_request_benchmark(c: &mut Criterion) {
chain.set_header(request_header);

let mut queue = virt_queue.create_queue();
let desc = queue.pop(&mem).unwrap();
let desc = queue.pop().unwrap();

c.bench_function("request_parse", |b| {
b.iter(|| {
Expand Down
20 changes: 10 additions & 10 deletions src/vmm/benches/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn queue_benchmark(c: &mut Criterion) {

set_dtable_one_chain(&rxq, 1);
queue.next_avail = Wrapping(0);
let desc = queue.pop(&mem).unwrap();
let desc = queue.pop().unwrap();
c.bench_function("next_descriptor_1", |b| {
b.iter(|| {
let mut head = Some(desc.clone());
Expand All @@ -73,7 +73,7 @@ pub fn queue_benchmark(c: &mut Criterion) {

set_dtable_one_chain(&rxq, 2);
queue.next_avail = Wrapping(0);
let desc = queue.pop(&mem).unwrap();
let desc = queue.pop().unwrap();
c.bench_function("next_descriptor_2", |b| {
b.iter(|| {
let mut head = Some(desc.clone());
Expand All @@ -85,7 +85,7 @@ pub fn queue_benchmark(c: &mut Criterion) {

set_dtable_one_chain(&rxq, 4);
queue.next_avail = Wrapping(0);
let desc = queue.pop(&mem).unwrap();
let desc = queue.pop().unwrap();
c.bench_function("next_descriptor_4", |b| {
b.iter(|| {
let mut head = Some(desc.clone());
Expand All @@ -97,7 +97,7 @@ pub fn queue_benchmark(c: &mut Criterion) {

set_dtable_one_chain(&rxq, 16);
queue.next_avail = Wrapping(0);
let desc = queue.pop(&mem).unwrap();
let desc = queue.pop().unwrap();
c.bench_function("next_descriptor_16", |b| {
b.iter(|| {
let mut head = Some(desc.clone());
Expand All @@ -113,7 +113,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
c.bench_function("queue_pop_1", |b| {
b.iter(|| {
queue.next_avail = Wrapping(0);
while let Some(desc) = queue.pop(&mem) {
while let Some(desc) = queue.pop() {
std::hint::black_box(desc);
}
})
Expand All @@ -123,7 +123,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
c.bench_function("queue_pop_4", |b| {
b.iter(|| {
queue.next_avail = Wrapping(0);
while let Some(desc) = queue.pop(&mem) {
while let Some(desc) = queue.pop() {
std::hint::black_box(desc);
}
})
Expand All @@ -133,7 +133,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
c.bench_function("queue_pop_16", |b| {
b.iter(|| {
queue.next_avail = Wrapping(0);
while let Some(desc) = queue.pop(&mem) {
while let Some(desc) = queue.pop() {
std::hint::black_box(desc);
}
})
Expand All @@ -146,7 +146,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
for i in 0_u16..1_u16 {
let index = std::hint::black_box(i);
let len = std::hint::black_box(i + 1);
_ = queue.add_used(&mem, index as u16, len as u32);
_ = queue.add_used(index as u16, len as u32);
}
})
});
Expand All @@ -158,7 +158,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
for i in 0_u16..16_u16 {
let index = std::hint::black_box(i);
let len = std::hint::black_box(i + 1);
_ = queue.add_used(&mem, index as u16, len as u32);
_ = queue.add_used(index as u16, len as u32);
}
})
});
Expand All @@ -170,7 +170,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
for i in 0_u16..256_u16 {
let index = std::hint::black_box(i);
let len = std::hint::black_box(i + 1);
_ = queue.add_used(&mem, index as u16, len as u32);
_ = queue.add_used(index as u16, len as u32);
}
})
});
Expand Down
28 changes: 12 additions & 16 deletions src/vmm/src/devices/virtio/balloon/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@
// Internal loop processes descriptors and acummulates the pfns in `pfn_buffer`.
// Breaks out when there is not enough space in `pfn_buffer` to completely process
// the next descriptor.
while let Some(head) = queue.pop(mem) {
while let Some(head) = queue.pop() {
let len = head.len as usize;
let max_len = MAX_PAGES_IN_DESC * SIZE_OF_U32;
valid_descs_found = true;
Expand Down Expand Up @@ -339,9 +339,7 @@

// Acknowledge the receipt of the descriptor.
// 0 is number of bytes the device has written to memory.
queue
.add_used(mem, head.index, 0)
.map_err(BalloonError::Queue)?;
queue.add_used(head.index, 0).map_err(BalloonError::Queue)?;
needs_interrupt = true;
}

Expand Down Expand Up @@ -372,17 +370,13 @@
}

pub(crate) fn process_deflate_queue(&mut self) -> Result<(), BalloonError> {
// This is safe since we checked in the event handler that the device is activated.
let mem = self.device_state.mem().unwrap();
METRICS.deflate_count.inc();

let queue = &mut self.queues[DEFLATE_INDEX];
let mut needs_interrupt = false;

while let Some(head) = queue.pop(mem) {
queue
.add_used(mem, head.index, 0)
.map_err(BalloonError::Queue)?;
while let Some(head) = queue.pop() {
queue.add_used(head.index, 0).map_err(BalloonError::Queue)?;
needs_interrupt = true;
}

Expand All @@ -398,13 +392,13 @@
let mem = self.device_state.mem().unwrap();
METRICS.stats_updates_count.inc();

while let Some(head) = self.queues[STATS_INDEX].pop(mem) {
while let Some(head) = self.queues[STATS_INDEX].pop() {
if let Some(prev_stats_desc) = self.stats_desc_index {
// We shouldn't ever have an extra buffer if the driver follows
// the protocol, but return it if we find one.
error!("balloon: driver is not compliant, more than one stats buffer received");
self.queues[STATS_INDEX]
.add_used(mem, prev_stats_desc, 0)
.add_used(prev_stats_desc, 0)

Check warning on line 401 in src/vmm/src/devices/virtio/balloon/device.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L401 was not covered by tests
.map_err(BalloonError::Queue)?;
}
for index in (0..head.len).step_by(SIZE_OF_STAT) {
Expand Down Expand Up @@ -450,14 +444,11 @@
}

fn trigger_stats_update(&mut self) -> Result<(), BalloonError> {
// This is safe since we checked in the event handler that the device is activated.
let mem = self.device_state.mem().unwrap();

// The communication is driven by the device by using the buffer
// and sending a used buffer notification
if let Some(index) = self.stats_desc_index.take() {
self.queues[STATS_INDEX]
.add_used(mem, index, 0)
.add_used(index, 0)
.map_err(BalloonError::Queue)?;
self.signal_used_queue()
} else {
Expand Down Expand Up @@ -611,6 +602,11 @@
}

fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> {
for q in self.queues.iter_mut() {
q.initialize(&mem)
.map_err(ActivateError::QueueMemoryError)?;
}

self.device_state = DeviceState::Activated(mem);
if self.activate_evt.write(1).is_err() {
METRICS.activate_fails.inc();
Expand Down
5 changes: 5 additions & 0 deletions src/vmm/src/devices/virtio/block/vhost_user/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ 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)?;
}

let start_time = utils::time::get_time_us(utils::time::ClockType::Monotonic);
// Setting features again, because now we negotiated them
// with guest driver as well.
Expand Down
16 changes: 9 additions & 7 deletions src/vmm/src/devices/virtio/block/virtio/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,15 +388,14 @@ impl VirtioBlock {
queue: &mut Queue,
index: u16,
len: u32,
mem: &GuestMemoryMmap,
irq_trigger: &IrqTrigger,
block_metrics: &BlockDeviceMetrics,
) {
queue.add_used(mem, index, len).unwrap_or_else(|err| {
queue.add_used(index, len).unwrap_or_else(|err| {
error!("Failed to add available descriptor head {}: {}", index, err)
});

if queue.prepare_kick(mem) {
if queue.prepare_kick() {
irq_trigger.trigger_irq(IrqType::Vring).unwrap_or_else(|_| {
block_metrics.event_fails.inc();
});
Expand All @@ -411,8 +410,8 @@ impl VirtioBlock {
let queue = &mut self.queues[queue_index];
let mut used_any = false;

while let Some(head) = queue.pop_or_enable_notification(mem) {
self.metrics.remaining_reqs_count.add(queue.len(mem).into());
while let Some(head) = queue.pop_or_enable_notification() {
self.metrics.remaining_reqs_count.add(queue.len().into());
let processing_result = match Request::parse(&head, mem, self.disk.nsectors) {
Ok(request) => {
if request.rate_limit(&mut self.rate_limiter) {
Expand Down Expand Up @@ -448,7 +447,6 @@ impl VirtioBlock {
queue,
head.index,
finished.num_bytes_to_mem,
mem,
&self.irq_trigger,
&self.metrics,
);
Expand Down Expand Up @@ -500,7 +498,6 @@ impl VirtioBlock {
queue,
finished.desc_idx,
finished.num_bytes_to_mem,
mem,
&self.irq_trigger,
&self.metrics,
);
Expand Down Expand Up @@ -633,6 +630,11 @@ impl VirtioDevice for VirtioBlock {
}

fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> {
for q in self.queues.iter_mut() {
q.initialize(&mem)
.map_err(ActivateError::QueueMemoryError)?;
}

let event_idx = self.has_feature(u64::from(VIRTIO_RING_F_EVENT_IDX));
if event_idx {
for queue in &mut self.queues {
Expand Down
7 changes: 3 additions & 4 deletions src/vmm/src/devices/virtio/block/virtio/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,16 +473,15 @@ mod tests {
let memory = self.driver_queue.memory();

assert!(matches!(
Request::parse(&q.pop(memory).unwrap(), memory, NUM_DISK_SECTORS),
Request::parse(&q.pop().unwrap(), memory, NUM_DISK_SECTORS),
Err(_e)
));
}

fn check_parse(&self, check_data: bool) {
let mut q = self.driver_queue.create_queue();
let memory = self.driver_queue.memory();
let request =
Request::parse(&q.pop(memory).unwrap(), memory, NUM_DISK_SECTORS).unwrap();
let request = Request::parse(&q.pop().unwrap(), memory, NUM_DISK_SECTORS).unwrap();
let expected_header = self.header();

assert_eq!(
Expand Down Expand Up @@ -949,7 +948,7 @@ mod tests {
fn parse_random_requests() {
let cfg = ProptestConfig::with_cases(1000);
proptest!(cfg, |(mut request in random_request_parse())| {
let result = Request::parse(&request.2.pop(&request.1).unwrap(), &request.1, NUM_DISK_SECTORS);
let result = Request::parse(&request.2.pop().unwrap(), &request.1, NUM_DISK_SECTORS);
match result {
Ok(r) => prop_assert!(r == request.0.unwrap()),
Err(err) => {
Expand Down
10 changes: 9 additions & 1 deletion src/vmm/src/devices/virtio/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use utils::eventfd::EventFd;

use super::mmio::{VIRTIO_MMIO_INT_CONFIG, VIRTIO_MMIO_INT_VRING};
use super::queue::Queue;
use super::queue::{Queue, QueueError};
use super::ActivateError;
use crate::devices::virtio::AsAny;
use crate::logger::{error, warn};
Expand Down Expand Up @@ -180,6 +180,14 @@
fn reset(&mut self) -> Option<(EventFd, Vec<EventFd>)> {
None
}

/// Mark pages used by queues as dirty.
fn mark_queue_memory_dirty(&self, mem: &GuestMemoryMmap) -> Result<(), QueueError> {
for queue in self.queues() {
queue.mark_memory_dirty(mem)?

Check warning on line 187 in src/vmm/src/devices/virtio/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/device.rs#L185-L187

Added lines #L185 - L187 were not covered by tests
}
Ok(())
}

Check warning on line 190 in src/vmm/src/devices/virtio/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/device.rs#L189-L190

Added lines #L189 - L190 were not covered by tests
}

impl fmt::Debug for dyn VirtioDevice {
Expand Down
Loading
Loading