| Message ID | 20250211214328.640374-7-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust/block: Add minimal block driver bindings | expand |
On 2/11/25 22:43, Kevin Wolf wrote: > +/// Implementing `SizedIoBuffer` provides an implementation for [`IoBuffer`] without having to > +/// implement any functions manually. > +/// > +/// # Safety > +/// > +/// Types implementing `SizedIoBuffer` guarantee that the whole object can be accessed as an I/O > +/// buffer that is safe to contain any byte patterns. > +pub unsafe trait SizedIoBuffer: Sized { This is similar to the ByteValued trait in rust-vmm. Can you name it the same so that we can later consider replacing it? > + fn from_byte_slice(buf: &[u8]) -> Option<&Self> { > + if buf.len() < std::mem::size_of::<Self>() { > + return None; > + } > + > + let ptr = buf.as_ptr() as *const Self; > + > + // TODO Use ptr.is_aligned() when MSRV is updated to at least 1.79.0 > + if (ptr as usize) % std::mem::align_of::<Self>() != 0 { > + return None; > + } > + > + // SAFETY: This function checked that the byte slice is large enough and aligned. > + // Implementing SizedIoBuffer promises that any byte pattern is valid for the type. > + Some(unsafe { &*ptr }) If you want, the function can be written also // SAFETY: implementing SizedIoBuffer promises that any byte pattern // is valid for the type match unsafe { buf.align_to::<Self>() } { ([], mid, _) => mid.get(0), _ => None } (trick stolen from rust-vmm, in fact). Paolo
Am 12.02.2025 um 17:48 hat Paolo Bonzini geschrieben: > On 2/11/25 22:43, Kevin Wolf wrote: > > +/// Implementing `SizedIoBuffer` provides an implementation for [`IoBuffer`] without having to > > +/// implement any functions manually. > > +/// > > +/// # Safety > > +/// > > +/// Types implementing `SizedIoBuffer` guarantee that the whole object can be accessed as an I/O > > +/// buffer that is safe to contain any byte patterns. > > +pub unsafe trait SizedIoBuffer: Sized { > > This is similar to the ByteValued trait in rust-vmm. Can you name it > the same so that we can later consider replacing it? I'm not sure if it's the best name, but could be done, of course. Though the more interesting thing to replace it with eventually might be the zerocopy crate which has derive macros that check that the condition is actually fulfilled. I just didn't feel like bringing in new external dependencies in this first series. > > + fn from_byte_slice(buf: &[u8]) -> Option<&Self> { > > + if buf.len() < std::mem::size_of::<Self>() { > > + return None; > > + } This is a semantic difference compared to ByteValued::from_slice(), which requires the sizes to match exactly. For the probe function, I actually make use of the relaxed requirement here to access a header struct in a larger buffer passed from C. > > + let ptr = buf.as_ptr() as *const Self; > > + > > + // TODO Use ptr.is_aligned() when MSRV is updated to at least 1.79.0 > > + if (ptr as usize) % std::mem::align_of::<Self>() != 0 { > > + return None; > > + } > > + > > + // SAFETY: This function checked that the byte slice is large enough and aligned. > > + // Implementing SizedIoBuffer promises that any byte pattern is valid for the type. > > + Some(unsafe { &*ptr }) > > If you want, the function can be written also > > // SAFETY: implementing SizedIoBuffer promises that any byte pattern > // is valid for the type > match unsafe { buf.align_to::<Self>() } { > ([], mid, _) => mid.get(0), > _ => None > } > > (trick stolen from rust-vmm, in fact). Clever way to avoid ptr::is_aligned(), but I feel a bit harder to understand than just open-coding it like above? (And probably less efficient, but I don't know how relevant that is.) Kevin
Il mer 12 feb 2025, 18:23 Kevin Wolf <kwolf@redhat.com> ha scritto: > Am 12.02.2025 um 17:48 hat Paolo Bonzini geschrieben: > > On 2/11/25 22:43, Kevin Wolf wrote: > > > +/// Implementing `SizedIoBuffer` provides an implementation for > [`IoBuffer`] without having to > > > +/// implement any functions manually. > > > +/// > > > +/// # Safety > > > +/// > > > +/// Types implementing `SizedIoBuffer` guarantee that the whole > object can be accessed as an I/O > > > +/// buffer that is safe to contain any byte patterns. > > > +pub unsafe trait SizedIoBuffer: Sized { > > > > This is similar to the ByteValued trait in rust-vmm. Can you name it > > the same so that we can later consider replacing it? > > I'm not sure if it's the best name, but could be done, of course. > > Though the more interesting thing to replace it with eventually might be > the zerocopy crate which has derive macros that check that the condition > is actually fulfilled. I just didn't feel like bringing in new external > dependencies in this first series. > Good idea though. zerocopy has no extra dependencies, and I agree that sooner or later we're going to include it, so you might as well go for it. The build.rs file is ludicrously overengineered, but converting it to meson should be easy. > > + fn from_byte_slice(buf: &[u8]) -> Option<&Self> { > > > + if buf.len() < std::mem::size_of::<Self>() { > > > + return None; > > > + } > > This is a semantic difference compared to ByteValued::from_slice(), > which requires the sizes to match exactly. For the probe function, I > actually make use of the relaxed requirement here to access a header > struct in a larger buffer passed from C. > Indeed it's similar but not the same. I haven't checked how you'd write it with vm-memory (it could be hdr.as_bytes().read_obj(0), or maybe there's something better), but it's something that could be added there too. > If you want, the function can be written also > > > > // SAFETY: implementing SizedIoBuffer promises that any byte pattern > > // is valid for the type > > match unsafe { buf.align_to::<Self>() } { > > ([], mid, _) => mid.get(0), > > _ => None > > } > > > > (trick stolen from rust-vmm, in fact). > > Clever way to avoid ptr::is_aligned(), but I feel a bit harder to > understand than just open-coding it like above? (And probably less > efficient, but I don't know how relevant that is.) > Probably not much and a lot of dead code elimination can happen, but either way is fine of course. Paolo > Kevin > >
diff --git a/rust/block/src/iobuffer.rs b/rust/block/src/iobuffer.rs new file mode 100644 index 0000000000..d61370c961 --- /dev/null +++ b/rust/block/src/iobuffer.rs @@ -0,0 +1,94 @@ +// Copyright Red Hat Inc. +// Author(s): Kevin Wolf <kwolf@redhat.com> +// SPDX-License-Identifier: GPL-2.0-or-later + +use std::mem::MaybeUninit; + +/// Types that implement IoBuffer can be used with safe I/O functions. +/// +/// # Safety +/// +/// `buffer_ptr()` and `buffer_mut_ptr()` must return pointers to the address of the same I/O +/// buffer with the size returned by `buffer_len()` which remain valid for the lifetime of the +/// object. It must be safe for the I/O buffer to contain any byte patterns. +pub unsafe trait IoBuffer { + /// Returns a const pointer to be used as a raw I/O buffer + fn buffer_ptr(&self) -> *const u8; + + /// Returns a mutable pointer to be used as a raw I/O buffer + fn buffer_mut_ptr(&mut self) -> *mut u8; + + /// Returns the length in bytes for the raw I/O buffer returned by [`buffer_ptr`] and + /// [`buffer_mut_ptr`] + /// + /// [`buffer_ptr`]: IoBuffer::buffer_ptr + /// [`buffer_mut_ptr`]: IoBuffer::buffer_mut_ptr + fn buffer_len(&self) -> usize; +} + +/// Implementing `SizedIoBuffer` provides an implementation for [`IoBuffer`] without having to +/// implement any functions manually. +/// +/// # Safety +/// +/// Types implementing `SizedIoBuffer` guarantee that the whole object can be accessed as an I/O +/// buffer that is safe to contain any byte patterns. +pub unsafe trait SizedIoBuffer: Sized { + /// Safely converts a byte slice into a shared reference to the type implementing + /// `SizedIoBuffer` + fn from_byte_slice(buf: &[u8]) -> Option<&Self> { + if buf.len() < std::mem::size_of::<Self>() { + return None; + } + + let ptr = buf.as_ptr() as *const Self; + + // TODO Use ptr.is_aligned() when MSRV is updated to at least 1.79.0 + if (ptr as usize) % std::mem::align_of::<Self>() != 0 { + return None; + } + + // SAFETY: This function checked that the byte slice is large enough and aligned. + // Implementing SizedIoBuffer promises that any byte pattern is valid for the type. + Some(unsafe { &*ptr }) + } +} + +unsafe impl<T: SizedIoBuffer> IoBuffer for T { + fn buffer_ptr(&self) -> *const u8 { + self as *const Self as *const u8 + } + + fn buffer_mut_ptr(&mut self) -> *mut u8 { + self as *mut Self as *mut u8 + } + + fn buffer_len(&self) -> usize { + std::mem::size_of::<Self>() + } +} + +unsafe impl<T: SizedIoBuffer> IoBuffer for [T] { + fn buffer_ptr(&self) -> *const u8 { + self.as_ptr() as *const u8 + } + + fn buffer_mut_ptr(&mut self) -> *mut u8 { + self.as_mut_ptr() as *mut u8 + } + + fn buffer_len(&self) -> usize { + std::mem::size_of_val(self) + } +} + +unsafe impl<T: SizedIoBuffer> SizedIoBuffer for MaybeUninit<T> {} + +unsafe impl SizedIoBuffer for u8 {} +unsafe impl SizedIoBuffer for u16 {} +unsafe impl SizedIoBuffer for u32 {} +unsafe impl SizedIoBuffer for u64 {} +unsafe impl SizedIoBuffer for i8 {} +unsafe impl SizedIoBuffer for i16 {} +unsafe impl SizedIoBuffer for i32 {} +unsafe impl SizedIoBuffer for i64 {} diff --git a/rust/block/src/lib.rs b/rust/block/src/lib.rs index 8b13789179..1c03549821 100644 --- a/rust/block/src/lib.rs +++ b/rust/block/src/lib.rs @@ -1 +1,3 @@ +mod iobuffer; +pub use iobuffer::{IoBuffer, SizedIoBuffer};
Types that implement IoBuffer can be used with safe I/O functions. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- rust/block/src/iobuffer.rs | 94 ++++++++++++++++++++++++++++++++++++++ rust/block/src/lib.rs | 2 + 2 files changed, 96 insertions(+) create mode 100644 rust/block/src/iobuffer.rs