Skip to content

improve: remove Sized restriction from memory parameter #303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions virtio-blk/src/stdio_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ impl<B: Backend> StdIoBackend<B> {
/// # Arguments
/// * `mem` - A reference to the guest memory.
/// * `request` - The request to execute.
pub fn process_request<M: GuestMemory>(
pub fn process_request<M: GuestMemory + ?Sized>(
&mut self,
mem: &M,
request: &Request,
Expand Down Expand Up @@ -301,7 +301,7 @@ impl<B: Backend> StdIoBackend<B> {
/// # Arguments
/// * `mem` - A reference to the guest memory.
/// * `request` - The request to execute.
pub fn execute<M: GuestMemory>(&mut self, mem: &M, request: &Request) -> Result<u32> {
pub fn execute<M: GuestMemory + ?Sized>(&mut self, mem: &M, request: &Request) -> Result<u32> {
let offset = request
.sector()
.checked_shl(u32::from(SECTOR_SHIFT))
Expand Down
3 changes: 3 additions & 0 deletions virtio-queue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
## Added
- `Reader`/`Writer` classes to iterate over descriptors

### Changed
- `Queue` accepts memory representation without `Sized`.

# v0.11.0

## Changed
Expand Down
4 changes: 2 additions & 2 deletions virtio-queue/src/descriptor_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl<'a, B: BitmapSlice> Reader<'a, B> {
M: GuestMemory,
<<M as GuestMemory>::R as GuestMemoryRegion>::B: WithBitmapSlice<'a, S = B>,
T: Deref,
T::Target: GuestMemory + Sized,
T::Target: GuestMemory,
{
let mut total_len: usize = 0;
let buffers = desc_chain
Expand Down Expand Up @@ -274,7 +274,7 @@ impl<'a, B: BitmapSlice> Writer<'a, B> {
M: GuestMemory,
<<M as GuestMemory>::R as GuestMemoryRegion>::B: WithBitmapSlice<'a, S = B>,
T: Deref,
T::Target: GuestMemory + Sized,
T::Target: GuestMemory,
{
let mut total_len: usize = 0;
let buffers = desc_chain
Expand Down
30 changes: 20 additions & 10 deletions virtio-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ pub trait QueueT: for<'a> QueueGuard<'a> {
Self: Sized;

/// Check whether the queue configuration is valid.
fn is_valid<M: GuestMemory>(&self, mem: &M) -> bool;
fn is_valid<M: GuestMemory + ?Sized>(&self, mem: &M) -> bool;

/// Reset the queue to the initial state.
fn reset(&mut self);
Expand Down Expand Up @@ -198,38 +198,48 @@ pub trait QueueT: for<'a> QueueGuard<'a> {
/// # Panics
///
/// Panics if order is Release or AcqRel.
fn avail_idx<M>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>
where
M: GuestMemory + ?Sized;
fn avail_idx<M: GuestMemory + ?Sized>(
&self,
mem: &M,
order: Ordering,
) -> Result<Wrapping<u16>, Error>;

/// Read the `idx` field from the used ring.
///
/// # Panics
///
/// Panics if order is Release or AcqRel.
fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>;
fn used_idx<M: GuestMemory + ?Sized>(
&self,
mem: &M,
order: Ordering,
) -> Result<Wrapping<u16>, Error>;

/// Put a used descriptor head into the used ring.
fn add_used<M: GuestMemory>(&mut self, mem: &M, head_index: u16, len: u32)
-> Result<(), Error>;
fn add_used<M: GuestMemory + ?Sized>(
&mut self,
mem: &M,
head_index: u16,
len: u32,
) -> Result<(), Error>;

/// Enable notification events from the guest driver.
///
/// Return true if one or more descriptors can be consumed from the available ring after
/// notifications were enabled (and thus it's possible there will be no corresponding
/// notification).
fn enable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
fn enable_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<bool, Error>;

/// Disable notification events from the guest driver.
fn disable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<(), Error>;
fn disable_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<(), Error>;

/// Check whether a notification to the guest is needed.
///
/// Please note this method has side effects: once it returns `true`, it considers the
/// driver will actually be notified, remember the associated index in the used ring, and
/// won't return `true` again until the driver updates `used_event` and/or the notification
/// conditions hold once more.
fn needs_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
fn needs_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<bool, Error>;

/// Return the index of the next entry in the available ring.
fn next_avail(&self) -> u16;
Expand Down
41 changes: 27 additions & 14 deletions virtio-queue/src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl Queue {

// Helper method that writes `val` to the `avail_event` field of the used ring, using
// the provided ordering.
fn set_avail_event<M: GuestMemory>(
fn set_avail_event<M: GuestMemory + ?Sized>(
&self,
mem: &M,
val: u16,
Expand All @@ -211,7 +211,7 @@ impl Queue {
}

// Set the value of the `flags` field of the used ring, applying the specified ordering.
fn set_used_flags<M: GuestMemory>(
fn set_used_flags<M: GuestMemory + ?Sized>(
&mut self,
mem: &M,
val: u16,
Expand All @@ -225,7 +225,11 @@ impl Queue {
//
// Every access in this method uses `Relaxed` ordering because a fence is added by the caller
// when appropriate.
fn set_notification<M: GuestMemory>(&mut self, mem: &M, enable: bool) -> Result<(), Error> {
fn set_notification<M: GuestMemory + ?Sized>(
&mut self,
mem: &M,
enable: bool,
) -> Result<(), Error> {
if enable {
if self.event_idx_enabled {
// We call `set_avail_event` using the `next_avail` value, instead of reading
Expand Down Expand Up @@ -254,7 +258,11 @@ impl Queue {
// Neither of these interrupt suppression methods are reliable, as they are not synchronized
// with the device, but they serve as useful optimizations. So we only ensure access to the
// virtq_avail.used_event is atomic, but do not need to synchronize with other memory accesses.
fn used_event<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error> {
fn used_event<M: GuestMemory + ?Sized>(
&self,
mem: &M,
order: Ordering,
) -> Result<Wrapping<u16>, Error> {
// This can not overflow an u64 since it is working with relatively small numbers compared
// to u64::MAX.
let used_event_offset =
Expand Down Expand Up @@ -296,7 +304,7 @@ impl QueueT for Queue {
})
}

fn is_valid<M: GuestMemory>(&self, mem: &M) -> bool {
fn is_valid<M: GuestMemory + ?Sized>(&self, mem: &M) -> bool {
let queue_size = self.size as u64;
let desc_table = self.desc_table;
// The multiplication can not overflow an u64 since we are multiplying an u16 with a
Expand Down Expand Up @@ -419,10 +427,11 @@ impl QueueT for Queue {
self.event_idx_enabled = enabled;
}

fn avail_idx<M>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>
where
M: GuestMemory + ?Sized,
{
fn avail_idx<M: GuestMemory + ?Sized>(
&self,
mem: &M,
order: Ordering,
) -> Result<Wrapping<u16>, Error> {
let addr = self
.avail_ring
.checked_add(2)
Expand All @@ -434,7 +443,11 @@ impl QueueT for Queue {
.map_err(Error::GuestMemory)
}

fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error> {
fn used_idx<M: GuestMemory + ?Sized>(
&self,
mem: &M,
order: Ordering,
) -> Result<Wrapping<u16>, Error> {
let addr = self
.used_ring
.checked_add(2)
Expand All @@ -446,7 +459,7 @@ impl QueueT for Queue {
.map_err(Error::GuestMemory)
}

fn add_used<M: GuestMemory>(
fn add_used<M: GuestMemory + ?Sized>(
&mut self,
mem: &M,
head_index: u16,
Expand Down Expand Up @@ -504,7 +517,7 @@ impl QueueT for Queue {
// break;
// }
// }
fn enable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error> {
fn enable_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<bool, Error> {
self.set_notification(mem, true)?;
// Ensures the following read is not reordered before any previous write operation.
fence(Ordering::SeqCst);
Expand All @@ -519,11 +532,11 @@ impl QueueT for Queue {
.map(|idx| idx != self.next_avail)
}

fn disable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<(), Error> {
fn disable_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<(), Error> {
self.set_notification(mem, false)
}

fn needs_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error> {
fn needs_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<bool, Error> {
let used_idx = self.next_used;

// Complete all the writes in add_used() before reading the event.
Expand Down
16 changes: 10 additions & 6 deletions virtio-queue/src/queue_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl QueueT for QueueSync {
})
}

fn is_valid<M: GuestMemory>(&self, mem: &M) -> bool {
fn is_valid<M: GuestMemory + ?Sized>(&self, mem: &M) -> bool {
self.lock_state().is_valid(mem)
}

Expand Down Expand Up @@ -115,11 +115,15 @@ impl QueueT for QueueSync {
self.lock_state().avail_idx(mem, order)
}

fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error> {
fn used_idx<M: GuestMemory + ?Sized>(
&self,
mem: &M,
order: Ordering,
) -> Result<Wrapping<u16>, Error> {
self.lock_state().used_idx(mem, order)
}

fn add_used<M: GuestMemory>(
fn add_used<M: GuestMemory + ?Sized>(
&mut self,
mem: &M,
head_index: u16,
Expand All @@ -128,15 +132,15 @@ impl QueueT for QueueSync {
self.lock_state().add_used(mem, head_index, len)
}

fn enable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error> {
fn enable_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<bool, Error> {
self.lock_state().enable_notification(mem)
}

fn disable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<(), Error> {
fn disable_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<(), Error> {
self.lock_state().disable_notification(mem)
}

fn needs_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error> {
fn needs_notification<M: GuestMemory + ?Sized>(&mut self, mem: &M) -> Result<bool, Error> {
self.lock_state().needs_notification(mem)
}

Expand Down