feat(virtq): add high-level packed virtqueue producer/consumer api#1514
Open
andreiltd wants to merge 1 commit into
Open
feat(virtq): add high-level packed virtqueue producer/consumer api#1514andreiltd wants to merge 1 commit into
andreiltd wants to merge 1 commit into
Conversation
andreiltd
requested review from
danbugs,
dblnz,
devigned,
jprendes,
jsturtevant,
ludfjig,
simongdavies,
squillace and
syntactically
as code owners
andreiltd
force-pushed
the
tandr/virtq-api
branch
from
650cf31 to
c6ddbad
Compare
andreiltd
added
the
kind/enhancement
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a higher-level packed virtqueue API in hyperlight_common::virtq that sits on top of the existing packed-ring primitives, adding buffer management, message framing helpers, concurrency validation (loom), and performance benchmarks.
Changes:
- Add high-level producer/consumer APIs (
VirtqProducer/VirtqConsumer) that manage descriptor-chain lifecycle and notifications. - Add buffer allocation abstractions (
BufferProvider) plus concrete allocators (BufferPool,RecyclePool) and zero-copy segment types. - Add a small wire header (
VirtqMsgHeader), loom-based concurrency tests, and Criterion benchmarks for the new APIs/pools.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_common/src/virtq/ring.rs | Exposes mutable access to readable elements to support high-level send-chain writing. |
| src/hyperlight_common/src/virtq/producer.rs | Implements VirtqProducer, chain builder/write APIs, submission/batching, and used-chain reclamation. |
| src/hyperlight_common/src/virtq/consumer.rs | Implements VirtqConsumer, receive+reply types, payload copying, and completion/notification logic. |
| src/hyperlight_common/src/virtq/buffer.rs | Defines BufferProvider, allocator errors/types, Segments, and zero-copy Bytes ownership glue. |
| src/hyperlight_common/src/virtq/pool.rs | Adds BufferPool and RecyclePool implementations for virtqueue buffer allocation. |
| src/hyperlight_common/src/virtq/msg.rs | Adds an 8-byte message header for kind/flags/correlation/payload sizing. |
| src/hyperlight_common/src/virtq/mod.rs | Re-exports new APIs, adds docs/quick-start examples, and defines shared error/notification types. |
| src/hyperlight_common/src/virtq/concurrency.rs | Adds loom-based interleaving tests with a shadow-atomic memory model. |
| src/hyperlight_common/Cargo.toml | Adds dependencies/dev-deps for the new virtq APIs, pools, loom tests, and benches. |
| src/hyperlight_common/benches/buffer_pool.rs | Criterion benchmarks for allocation/free patterns in the pools. |
| src/hyperlight_common/benches/virtq_api.rs | Criterion benchmarks for high-level virtq roundtrips under different allocator strategies. |
| src/hyperlight_common/benches/common/mod.rs | Shared benchmark harness (in-memory MemOps, pair construction, roundtrip drivers). |
| Justfile | Adds a test-loom target to run loom concurrency tests. |
| Cargo.toml | Adds unexpected_cfgs lint config for cfg(loom). |
| Cargo.lock | Locks new crate dependencies introduced by the virtq/pool/bench work. |
| .github/workflows/dep_code_checks.yml | Runs loom concurrency tests in CI. |
Comment on lines
+161
to
+167
| let mut first_err = None; | ||
| for elem in elems { | ||
| if let Err(err) = self.pool.dealloc(elem.addr) | ||
| && first_err.is_none() | ||
| { | ||
| first_err = Some(VirtqError::Alloc(err)); | ||
| } |
Comment on lines
+177
to
+183
| /// Begin building a descriptor chain for submission. | ||
| /// | ||
| /// Returns a [`ChainBuilder`] that allocates buffers from the pool. | ||
| /// ``` | ||
| pub fn chain(&self) -> ChainBuilder<M, P> { | ||
| ChainBuilder::new(self.inner.mem().clone(), self.pool.clone()) | ||
| } |
Comment on lines
+219
to
+222
| let inf = send.into_inflight(token); | ||
| debug_assert!(self.inflight[id as usize].is_none()); | ||
|
|
||
| self.inflight[id as usize] = Some(inf); |
Comment on lines
+321
to
+326
| pub unsafe fn reset_with_pool(&mut self, pool: P) { | ||
| self.pending.clear(); | ||
| self.inflight.iter_mut().for_each(|slot| *slot = None); | ||
| self.inner.reset(); | ||
| self.pool = pool; | ||
| } |
Comment on lines
+318
to
+320
| if readables.is_empty() && writables.is_empty() { | ||
| return Err(VirtqError::BadChain); | ||
| } |
Comment on lines
+319
to
+338
| pub fn new(base_addr: u64, region_len: usize) -> Result<Self, AllocError> { | ||
| const LOWER_FRACTION: usize = 8; | ||
|
|
||
| let lower_region = region_len / LOWER_FRACTION; | ||
| let upper_region = region_len - lower_region; | ||
|
|
||
| let mut aligned = base_addr; | ||
| aligned = align_up(aligned as usize, L) as u64; | ||
| let lower = Slab::<L>::new(aligned, lower_region)?; | ||
|
|
||
| // advance and align upper base to N | ||
| aligned = aligned | ||
| .checked_add(lower.capacity() as u64) | ||
| .ok_or(AllocError::Overflow)?; | ||
|
|
||
| aligned = align_up(aligned as usize, U) as u64; | ||
| let upper = Slab::<U>::new(aligned, upper_region)?; | ||
|
|
||
| Ok(Self { lower, upper }) | ||
| } |
Comment on lines
+260
to
+274
| unsafe fn as_slice(&self, addr: u64, len: usize) -> Result<&[u8], Self::Error> { | ||
| let (info, offset) = self.region(addr).ok_or(MemErr)?; | ||
|
|
||
| match info.kind { | ||
| RegionKind::Pool => { | ||
| // Safety: pool memory is a contiguous Vec<u8>; caller ensures | ||
| // no concurrent writes for the lifetime of the returned slice. | ||
| Ok(self | ||
| .pool | ||
| .get() | ||
| .with(|buf| unsafe { &(&*buf)[offset..offset + len] })) | ||
| } | ||
| _ => Err(MemErr), | ||
| } | ||
| } |
Comment on lines
+276
to
+286
| unsafe fn as_mut_slice(&self, addr: u64, len: usize) -> Result<&mut [u8], Self::Error> { | ||
| let (info, offset) = self.region(addr).ok_or(MemErr)?; | ||
|
|
||
| match info.kind { | ||
| RegionKind::Pool => Ok(self | ||
| .pool | ||
| .get_mut() | ||
| .with(|buf| unsafe { &mut (&mut *buf)[offset..offset + len] })), | ||
| _ => Err(MemErr), | ||
| } | ||
| } |
| anyhow = { version = "1.0.102", default-features = false } | ||
| bitflags = "2.10.0" | ||
| bytemuck = { version = "1.24", features = ["derive"] } | ||
| bytes = { version = "1", default-features = false } |
Comment on lines
+351
to
+358
| let data = match self.read_elements(readables) { | ||
| Ok(d) => d, | ||
| Err(e) => { | ||
| // Read failed - clear inflight before propagating | ||
| self.inflight.set(id_idx, false); | ||
| return Err(e); | ||
| } | ||
| }; |
andreiltd
force-pushed
the
tandr/virtq-api
branch
2 times, most recently
from
d01a3c6 to
11a9c24
Compare
This patch adds a higher level api on top of the packed ring primitives in hyperlight_common so callers don't manage raw descriptors directly. - Add VirtqProducer/VirtqConsumer in producer.rs and consumer.rs that handle buffer allocation, chain lifecycle, and event suppression/notification decisions; expand virtq/mod.rs with the public API, builders, and docs. - Add buffer management: the BufferProvider trait and shared types in buffer.rs plus two concrete allocators - a two-tier variable-size BufferPool and a fixed-slot RecyclePool. - Add an 8-byte wire message header in msg.rs for message type discrimination and request/response correlation. - Add loom-based concurrency tests in concurrency.rs using shadow atomics - Add criterion benchmarks for the buffer pool and virtq API. Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
andreiltd
force-pushed
the
tandr/virtq-api
branch
from
11a9c24 to
5b74480
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch adds a higher level api on top of the packed ring primitives in hyperlight_common so callers don't manage raw descriptors directly.