Skip to content

Commit f85cd72

Browse files
committed
feat(iovec): add ability to reload IoVecBufferMut
Now IoVecBufferMut can be reloaded from DescriptorChain same way as IoVecBuffer does it. This is helpful to avoid unnecessary allocations/deallocations when reusing same buffer. Signed-off-by: Egor Lazarchuk <[email protected]>
1 parent 167b1b0 commit f85cd72

File tree

3 files changed

+57
-17
lines changed

3 files changed

+57
-17
lines changed

src/vmm/src/devices/virtio/iovec.rs

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ impl IoVecBuffer {
217217
/// It describes a write-only buffer passed to us by the guest that is scattered across multiple
218218
/// memory regions. Additionally, this wrapper provides methods that allow reading arbitrary ranges
219219
/// of data from that buffer.
220-
#[derive(Debug)]
220+
#[derive(Debug, Default, Clone)]
221221
pub struct IoVecBufferMut {
222222
// container of the memory regions included in this IO vector
223223
vecs: IoVecVec,
@@ -226,12 +226,19 @@ pub struct IoVecBufferMut {
226226
}
227227

228228
impl IoVecBufferMut {
229-
/// Create an `IoVecBufferMut` from a `DescriptorChain`
230-
pub fn from_descriptor_chain(head: DescriptorChain) -> Result<Self, IoVecError> {
231-
let mut vecs = IoVecVec::new();
232-
let mut len = 0u32;
229+
/// Create an `IoVecBuffer` from a `DescriptorChain`
230+
///
231+
/// # Safety
232+
///
233+
/// The descriptor chain cannot be referencing the same memory location as another chain
234+
pub unsafe fn load_descriptor_chain(
235+
&mut self,
236+
head: DescriptorChain,
237+
) -> Result<(), IoVecError> {
238+
self.clear();
233239

234-
for desc in head {
240+
let mut next_descriptor = Some(head);
241+
while let Some(desc) = next_descriptor {
235242
if !desc.is_write_only() {
236243
return Err(IoVecError::ReadOnlyDescriptor);
237244
}
@@ -247,23 +254,45 @@ impl IoVecBufferMut {
247254
slice.bitmap().mark_dirty(0, desc.len as usize);
248255

249256
let iov_base = slice.ptr_guard_mut().as_ptr().cast::<c_void>();
250-
vecs.push(iovec {
257+
self.vecs.push(iovec {
251258
iov_base,
252259
iov_len: desc.len as size_t,
253260
});
254-
len = len
261+
self.len = self
262+
.len
255263
.checked_add(desc.len)
256264
.ok_or(IoVecError::OverflowedDescriptor)?;
265+
266+
next_descriptor = desc.next_descriptor();
257267
}
258268

259-
Ok(Self { vecs, len })
269+
Ok(())
270+
}
271+
272+
/// Create an `IoVecBuffer` from a `DescriptorChain`
273+
///
274+
/// # Safety
275+
///
276+
/// The descriptor chain cannot be referencing the same memory location as another chain
277+
pub unsafe fn from_descriptor_chain(head: DescriptorChain) -> Result<Self, IoVecError> {
278+
let mut new_buffer: Self = Default::default();
279+
280+
new_buffer.load_descriptor_chain(head)?;
281+
282+
Ok(new_buffer)
260283
}
261284

262285
/// Get the total length of the memory regions covered by this `IoVecBuffer`
263286
pub(crate) fn len(&self) -> u32 {
264287
self.len
265288
}
266289

290+
/// Clears the `iovec` array
291+
pub fn clear(&mut self) {
292+
self.vecs.clear();
293+
self.len = 0u32;
294+
}
295+
267296
/// Writes a number of bytes into the `IoVecBufferMut` starting at a given offset.
268297
///
269298
/// This will try to fill `IoVecBufferMut` writing bytes from the `buf` starting from
@@ -468,11 +497,13 @@ mod tests {
468497

469498
let (mut q, _) = read_only_chain(&mem);
470499
let head = q.pop(&mem).unwrap();
471-
IoVecBufferMut::from_descriptor_chain(head).unwrap_err();
500+
// SAFETY: This descriptor chain is only loaded into one buffer
501+
unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap_err() };
472502

473503
let (mut q, _) = write_only_chain(&mem);
474504
let head = q.pop(&mem).unwrap();
475-
IoVecBufferMut::from_descriptor_chain(head).unwrap();
505+
// SAFETY: This descriptor chain is only loaded into one buffer
506+
unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap() };
476507
}
477508

478509
#[test]
@@ -493,7 +524,7 @@ mod tests {
493524
let head = q.pop(&mem).unwrap();
494525

495526
// SAFETY: This descriptor chain is only loaded once in this test
496-
let iovec = IoVecBufferMut::from_descriptor_chain(head).unwrap();
527+
let iovec = unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap() };
497528
assert_eq!(iovec.len(), 4 * 64);
498529
}
499530

@@ -558,7 +589,8 @@ mod tests {
558589
// This is a descriptor chain with 4 elements 64 bytes long each.
559590
let head = q.pop(&mem).unwrap();
560591

561-
let mut iovec = IoVecBufferMut::from_descriptor_chain(head).unwrap();
592+
// SAFETY: This descriptor chain is only loaded into one buffer
593+
let mut iovec = unsafe { IoVecBufferMut::from_descriptor_chain(head).unwrap() };
562594
let buf = vec![0u8, 1, 2, 3, 4];
563595

564596
// One test vector for each part of the chain

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,10 @@ impl Entropy {
132132
let index = desc.index;
133133
METRICS.entropy_event_count.inc();
134134

135-
let bytes = match IoVecBufferMut::from_descriptor_chain(desc) {
135+
// SAFETY: This descriptor chain is only loaded once
136+
// virtio requests are handled sequentially so no two IoVecBuffers
137+
// are live at the same time, meaning this has exclusive ownership over the memory
138+
let bytes = match unsafe { IoVecBufferMut::from_descriptor_chain(desc) } {
136139
Ok(mut iovec) => {
137140
debug!(
138141
"entropy: guest request for {} bytes of entropy",
@@ -428,13 +431,15 @@ mod tests {
428431
// This should succeed, we just added two descriptors
429432
let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop(&mem).unwrap();
430433
assert!(matches!(
431-
IoVecBufferMut::from_descriptor_chain(desc),
434+
// SAFETY: This descriptor chain is only loaded into one buffer
435+
unsafe { IoVecBufferMut::from_descriptor_chain(desc) },
432436
Err(crate::devices::virtio::iovec::IoVecError::ReadOnlyDescriptor)
433437
));
434438

435439
// This should succeed, we should have one more descriptor
436440
let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop(&mem).unwrap();
437-
let mut iovec = IoVecBufferMut::from_descriptor_chain(desc).unwrap();
441+
// SAFETY: This descriptor chain is only loaded into one buffer
442+
let mut iovec = unsafe { IoVecBufferMut::from_descriptor_chain(desc).unwrap() };
438443
entropy_dev.handle_one(&mut iovec).unwrap();
439444
}
440445

src/vmm/src/devices/virtio/vsock/packet.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,10 @@ impl VsockPacket {
161161
/// Returns [`VsockError::DescChainTooShortForHeader`] if the descriptor chain's total buffer
162162
/// length is insufficient to hold the 44 byte vsock header
163163
pub fn from_rx_virtq_head(chain: DescriptorChain) -> Result<Self, VsockError> {
164-
let buffer = IoVecBufferMut::from_descriptor_chain(chain)?;
164+
// SAFETY: This descriptor chain is only loaded once
165+
// virtio requests are handled sequentially so no two IoVecBuffers
166+
// are live at the same time, meaning this has exclusive ownership over the memory
167+
let buffer = unsafe { IoVecBufferMut::from_descriptor_chain(chain)? };
165168

166169
if buffer.len() < VSOCK_PKT_HDR_SIZE {
167170
return Err(VsockError::DescChainTooShortForHeader(buffer.len() as usize));

0 commit comments

Comments
 (0)