RFC: virtqueue interface
- Dominant language
- Rust
- Stars
- 1.5k
- Forks
- 132
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 39
Description
I have been considering how to rewrite the virtio-queue interface to make it more unified, safer and easier to understand. Since @mustermeiszer is working on this as well, I figured I'd post some notes here. Feedback very welcome.
## Requirements:
- Usability
+ virtio-fs needs to send/receive in the same transmission (packed queue)
+ virtio-fs wants to hand in multiple buffers, which get concatenated in a single descriptor chain. This way we can easily prepend a header to the data.
+ virtio-net needs to have a filled receive queue, since the device can initiate transmissions on it's own. Once a buffer is used, the same descriptor is inserted into the queue again.
+ virtio-net wants to also recycle transmit descriptors for speed. On initialization, it allocates all tx buffers, constructs descriptor chains for them. When we want to send we can now simply place the correct index into the ring.
- Memory Safety
+ While a transfer is ongoing, the buffer should not be read/writable for the initiator
+ Check if given buffers are physical contiguous, if not split into multiple descriptors on page boundaries
+ `mem::forget` is safe, so we should consider what happens when destructors don't run
## Proposal:
Use `TransferTokens`, which take ownership of the buffers which are to be transferred. On initialization it creates the descritor chain. Can be placed into a virtqueue, upon which it gets converted into a `Transfer`. Transfer only gives back ownership (or references) to the buffers, once the transfer is complete.
To accomodate the virtio-net usecase of recycling the buffer, we have two modes:
1) consume the `Transfer` and return the buffers. This frees up the descriptor chain
2) extract only a reference to the buffer, use it, later 'recycle' the transfer by re-inserting it into the queue.
### Questions:
- is the interface sufficient for virtio-net? afaik it should be, but I have not looked too deep yet.
- do we need a 'fire and forget' functionality, where we are allowed to drop an ongoing transfer, which is then automatically freed once done? Currently both virtio-fs and virtio-net don't need it. We could default to warning + leaking.
- doing it the way I propose below is slightly unsafe, since when a transfer is dropped with `mem::forget`, we do NOT free the descriptors associated with the transfer, so permanently exhaust some of them. They are limited by the virtqueue length, so doing this often would block all transfers. But I think this is okay in our case, since we can just be careful to not use `mem::forget`. The only other option I see is doing more housekeeping within the virtqueue itself. I feel this is too complex (and likely slower?), since we would have to keep track of all transfertokens in the virtqueue.
- Is the interface `prepare_transfer(send, recv) -> Result` appropriate for a split queue?
- the function argument types (`Option]>>`) look a bit wild. We could use a newtype with .into() to make it look cleaner
- should we use boxed arrays, or vectors for storing the buffers? (`Box<[Box<[u8]>]>` vs `Vec>` vs `Vec>`). Both store an array with size on the heap, a vector has an additional capacity field. I feel that boxes are better since we can easily convert vec to box with `.into_boxed_slice()`, but not the other way around.
- We could, if we wanted to be extra safe, unmap the pages the buffers are stored in while a transfer is happening. This way, userspace (or other kernel code) cannot write to buffers if it has kept 'illegal' references around. This could prevent an VM escape via a broken device. But since this is not guaranteed by the spec, it should not be a problem. I know virtio-fs safeguards against changing buffer contents by copying the headers before parsing them.
## Interface
```rust
/// A prepared transfer, that can be inserted into a virtqueue
pub struct TransferToken<'a> {
/// memory that is to be send. Multiple, virtual-contiguous u8 arrays
send_bufs: Option]>>,
/// memory that is to be received into. Multiple, virtual-contiguous u8 arrays
recv_bufs: Option]>>,
/// descriptor chain, that references send/recv bufs
desc_chain: Rc>,
/// virtq on which the transfer is being done
virtq: &'a Virtq<'a>,
}
/// A single transfer inside a virtqueue
pub struct Transfer<'a> {
/// virtq-index is not enough, since it wraps easily. Since transfers do not have to be handled in-order, this might be an issue so we use a large id.
id: u64,
/// Token that was converted to this transfer
token: TransferToken<'a>,
}
impl<'a> TransferToken<'a> {
/// Places the already prepared descriptor chain into the virtqueue, returns a Transfer
///
/// Since Transfer's give back the token once they are completed, we can insert the same
/// token with the same descriptor chain multiple times if we want
pub fn send(self) -> Transfer<'a> {
unimplemented!()
}
/// Returns mutable references to the send and receive buffers.
pub fn buf_ref_mut(&mut self) -> (Option<&[&mut [u8]]>, Option<&[&mut [u8]]>) {
unimplemented!()
}
/// Consumes the token, returning ownership of the buffers
pub fn consume(self) -> (Option]>>, Option]>>) {
unimplemented!()
}
}
impl<'a> Drop for TransferToken<'a> {
fn drop(&mut self) {
// Free descriptor chain in the virtq, so it can be reused by new transfers
unimplemented!()
}
}
impl<'a> Transfer<'a> {
/// Checks whether the transfer is completed yet. If it is, returns the TransferToken
pub fn check(&self) -> Option {
// checks current ID against virtq's last received id.
unimplemented!()
}
/// Waits until transfer is completed. Returns the TransferToken used.
pub fn wait(&self) -> TransferToken {
unimplemented!()
}
}
impl<'a> Drop for Transfer<'a> {
fn drop(&mut self) {
// If a transfer is dropped while it is ongoing, do NOT free the memory, leak it instead.
// We could register it in the virtq, so it can be freed later.
unimplemented!()
}
}
impl<'a> Virtq<'a> {
/// Places send and recv buffers in a descriptor chain, returns a Transfer Token.
/// Transfer is not placed in the queue yet.
/// Specifying both send and receive is only possible in a packed queue, else Err
/// For a split queue, send/recv have to match the queue mode, else Err
///
/// Checks whether the buffers are physically contiguous, split into multiple descriptors if not
pub fn prepare_transfer(send: Option]>>, recv: Option]>>) -> Result, ()> {
unimplemented!()
}
/// Places send and recv buffers in a descriptor chain, returns a Transfer Token.
/// Transfer is not placed in the queue yet,
/// Specifying both send and receive is only possible in a packed queue, else Err
/// For a split queue, send/recv have to match the queue mode, else Err
///
/// Does NOT check whether the buffers are physically contiguous! Caller has to guarantee this!
pub unsafe fn prepare_transfer_unchecked(send: Option]>>, recv: Option]>>) -> TransferToken<'a> {
unimplemented!()
}
}
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.