|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC XEN PATCH] docs/designs: Add a design document for 'xen' Rust crate
Hello Alejandro,
Thanks for the review !
Le 22/11/2024 à 22:55, Alejandro Vallejo a écrit :
>> +Some principles are proposed :
>> +
>> +Use or provide idiomatic abstractions when relevant (reuse standard traits).
>> +
>> +Examples:
>> + Provide (optional) Future<...> abstractions for event channels
>> + Provide a iterator-based (Stream ? [2]) abstraction for guest console.
>> +
>> +Don't restrict features to some execution environment, use modular
>> abstractions (e.g traits)
>> +to allow the user to specify the missing bits himself / provide its own
>> implementation.
>> +Note that it doesn't prevent us from exposing the platform-specific bits
>> onto the
>> +types themselves (e.g UnixXenEventChannel can still expose its file
>> descriptor).
>
> Traits are superhelpful for dynamic dispatch and building type constraints.
> Sometimes for enforcing interfaces if they are not all built at the same time.
>
> What exactly are the benefits of subjecting all interfaces to traits? What do
> they enable that cannot otherwise be done by extending the interface exported
> by some struct (say, an opaque filedesc wrapper)?
>
>> +
>> +Example:
>> + If we provide a event channel abstraction based on the hypercall but it
>> doesn't implement Future<...>,
>> + the user can still implement its own type on top of the hypercall
>> implementation, and
>> + use its own async runtime (e.g based on interrupts) to implement
>> Future<...> himself.
>> + There could be 2 traits for varying needs :
>> + EventChannel (base trait) and AsyncEventChannel (await-able
>> EventChannel)
>> +
>> + We can have both RawEventChannel based on XenHypercall that only
>> implements EventChannel
>> + and another type TokioEventChannel that provides both EventChannel and
>> AsyncEventChannel
>> + and integrates with tokio runtime.
>
> Not sure why traits would be essential here either. It's hard to assess
> without
> looking at a particular case where using traits effectively simplifies the
> problem.
>
Some xen primitives are exposed differently depending on the
platform/context, for instance, event channels in a freestanding context
are exposed by hypercalls while in a hosted context, they are often
exposed from a file descriptor that we can make with '/dev/xen/evtchn'.
While they work differently, they actually share some behavior : e.g
they all have some event channel operations you can do, and you may be
able to wait for a event (or not).
To not make assumption on how the platform/context works and what it
provides/expects, I use traits to define a "shared interface" that will
be followed by all platforms (and expose some platform specificities
like how you prepare a pointer to the hypercall).
(event channels is a bit more complex, because xenctrl/xenevtchn has
actually 2 different ways of making event channels, one using
/dev/xen/evtchn, and another using evtchn_op hypercalls; we can expose
both by having one relying on XenHypercall (that exists in hosted) while
the other relies on evtchn device)
Behind the scenes, it's still going to be structs, and by impl Traits,
it's going to be statically dispatched (e.g the impl Type is replaced
with the actual type).
>> +
>> +Safe wrappers must be "sound" and unsafe code shall not cause undefined
>> behavior.
>> +- safe wrappers must not cause undefined behavior on their own
>> +- unsafe code should not cause undefined behavior if properly used
>
> I agree. The external interface must be soundly safe as far as reasonably
> possible. And unsafe code must be adequately annotated with the cares that the
> caller must have.
>
>> +
>> +This is a bit tricky due to some Xen specificities, but we expect hypercall
>> to be well
>> +formed (we can add validation tools for that) including have its parameter
>> indirectly
>> +respect the aliasing rules [3].
>
>> +Although, we assume that Xen is well-behaving regarding its ABI.
>> +We don't define misuse of a hypercall that can harm the guest himself, but
>> we care
>> +about not causing a undefined behavior (e.g by passing a buggy pointer)
>> through the
>> +hypercall interface that can overwrite unrelated/arbitrary kernel memory.
>> +
>> +## Hypercall interface
>> +
>> +We introduce a XenHypercall trait that exposes a raw primitive for making
>> hypercalls.
>> +This interface supposes nothing on the ABI used in Xen, and its the
>> responsibility
>> +of the user of such interface (often safe wrappers) that the hypercall made
>> is
>> +meaningful.
>> +
>> +This interface is mostly to only be used by the crate developpers to build
>> safe
>> +wrappers on top, or by advanced user for using non-exposed/WIP hypercall
>> interfaces
>> +or bypassing the safe wrappers.
>> +
>> +We can implement this interface for freestanding platforms using direct
>> native hypercalls
>> +(e.g using inline assembly) for freestanding platforms or in userland using
>> special devices
>> +like privcmd/xencall on most Unix platforms.
>> +
>> +```rust
>> +pub trait XenHypercall: Sized {
>> + unsafe fn hypercall5(&self, cmd: usize, param: [usize; 5]) -> usize;
>> +
>> + unsafe fn hypercall4(&self, cmd: usize, param: [usize; 4]) -> usize;
>> + unsafe fn hypercall3(&self, cmd: usize, param: [usize; 3]) -> usize;
>> + unsafe fn hypercall2(&self, cmd: usize, param: [usize; 2]) -> usize;
>> + unsafe fn hypercall1(&self, cmd: usize, param: usize) -> usize;
>> + unsafe fn hypercall0(&self, cmd: usize) -> usize;
>> +
>> + /* ... */
>> +}
>> +```
>> +
>> +### Hypercall-safe buffers
>> +
>> +One difficulty is that in a freestanding environment, we need to use
>> pointers to
>> +original data. But in a hosted environment, we need to make special buffers
>> instead
>> +for that.
>> +
>> +We introduce the Xen{Const/Mut}Buffer generic trait that wraps a reference
>> in a
>> +"hypercall-safe" buffer that may or may not be a bounce buffer.
>> +
>> +```rust
>> +/// Wrapper of a reference into a hypercall-safe buffer.
>> +pub trait XenConstBuffer<T> {
>> + /// Get a hypercall-safe reference to underlying data.
>> + fn as_hypercall_ptr(&self) -> *const T;
>> +}
>> +
>> +/// Wrapper of a mutable reference into a mutable hypercall-safe buffer.
>> +pub trait XenMutBuffer<T> {
>> + /// Get a hypercall-safe mutable reference to underlying data.
>> + fn as_hypercall_ptr(&mut self) -> *mut T;
>> +
>> + /// Update original reference with new data.
>> + unsafe fn update(&mut self);
>> +}
>
> Rather than trying to wrap the borrows I think it makes more sense to wrap
> ownership. If we provide a xen::Box<T> type (not necessarily tied to a
> standard
> allocator) that wraps ownership of its content then we can use the same type
> for const and mut references (because it's just &T and &mut T after Deref and
> DerefMut are implemented). Getting the pointers for hypercalls can be done via
> the references. Like so:
>
> puf foo(p: *mut u8) {
> // something
> }
>
> puf main() {
> let mut var = 8;
> foo(&mut var);
> }
>
> This is tip-toeing on UB by getting a *mut from a &mut, but
> because the &mut is coerced into a *mut on function call this is both safe and
> sound. Miri is happy with it too because it obeys stacked borrows.
>
> That's for inputs. Pointer outputs are profoundly unsafe and I'm really hoping
> there's nothing that requires it. Fingers crossed...
>
Wrapping ownership would be useful but I don't think it is better than
mapping borrows. If we expects the user to use xen::Box<T> as
input/output, it may not be practical to use. And using xen::Box<T>
internally implies having some custom way of allocating data, which may
be platform specific (freestanding that may not have a xen-aware allocator).
>> +
>> +// The user will make those wrappers using dedicated functions in
>> XenHypercall trait.
>> +
>> +trait XenHypercall: Sized {
>> + /* ... */
>> + type Error;
>> +
>> + fn make_const_object<T: Copy + ?Sized>(
>> + &self,
>> + buffer: &T,
>> + ) -> Result<impl XenConstBuffer<T>, Self::Error>;
>> +
>> + fn make_mut_buffer<T: Copy + ?Sized>(
>> + &self,
>> + buffer: &mut T,
>> + ) -> Result<impl XenMutBuffer<T>, Self::Error>;
>> +
>> + fn make_const_slice<T: Copy + Sized>(
>> + &self,
>> + slice: &[T],
>> + ) -> Result<impl XenConstBuffer<T>, Self::Error>;
>> +
>> + fn make_mut_slice<T: Copy + Sized>(
>> + &self,
>> + slice: &mut [T],
>> + ) -> Result<impl XenMutBuffer<T>, Self::Error>;
>> +}
>> +```
>
> This is what I meant at the beginning. What's the advantage of having
> XenConstBuffer (et al.) being a trait rather than a struct? You can access
> it's
> interior via Deref and DerefMut (like Box<T> and Mutex<T> do) already, so they
> can implement the same interface as T.
>
Some additional context
/// Wrapper of a reference into a hypercall-safe buffer.
pub trait XenConstBuffer<T> {
/// Get a hypercall-safe reference to underlying data.
fn as_hypercall_ptr(&self) -> *const T;
}
/// Wrapper of a mutable reference into a mutable hypercall-safe buffer.
pub trait XenMutBuffer<T> {
/// Get a hypercall-safe mutable reference to underlying data.
fn as_hypercall_ptr(&mut self) -> *mut T;
/// Update original reference with new data.
unsafe fn update(&mut self);
}
---
For freestanding
https://gitlab.com/TSnake41/xen/-/blob/rust-rfc/tools/rust/xen/src/hypercall/none/mod.rs?ref_type=heads
/// Constant xen buffer that passes the reference as-is.
pub(super) struct DirectConstXenBuffer<'a, T>(&'a T);
impl<T> XenConstBuffer<T> for DirectConstXenBuffer<'_, T> {
fn as_hypercall_ptr(&self) -> *const T {
self.0
}
}
pub(super) struct DirectConstXenSlice<'a, T>(&'a [T]);
impl<T> XenConstBuffer<T> for DirectConstXenSlice<'_, T> {
fn as_hypercall_ptr(&self) -> *const T {
self.0.as_ptr()
}
}
/// Mutable xen buffer that passes the reference as-is.
pub(super) struct DirectMutXenBuffer<'a, T>(&'a mut T);
impl<T> XenMutBuffer<T> for DirectMutXenBuffer<'_, T> {
fn as_hypercall_ptr(&mut self) -> *mut T {
self.0
}
unsafe fn update(&mut self) {
// The buffer is passed as is, we don't need to bounce the changes.
}
}
pub(super) struct DirectMutXenSlice<'a, T>(&'a mut [T]);
impl<T> XenMutBuffer<T> for DirectMutXenSlice<'_, T> {
fn as_hypercall_ptr(&mut self) -> *mut T {
self.0.as_mut_ptr()
}
unsafe fn update(&mut self) {
// The buffer is passed as is, we don't need to bounce the changes.
}
}
---
For Unix platforms (bounce buffer)
https://gitlab.com/TSnake41/xen/-/blob/rust-rfc/tools/rust/xen/src/hypercall/unix/buffer.rs?ref_type=heads
pub struct XenCallBuffer<'hyp, T> {
interface: PhantomData<&'hyp UnixXenHypercall>,
ptr: NonNull<T>, // aligned
page_count: usize,
length: usize,
}
pub struct UnixConstXenBuffer<'a, 'hyp, T: Copy + ?Sized> {
// As const objects are actually being copied they actually don't
// need to hold a reference to their original counterpart.
// Use a PhantomData to make the borrow checker happy.
pub(super) original: PhantomData<&'a T>,
pub(super) buffer: XenCallBuffer<'hyp, T>,
}
pub struct UnixMutXenBuffer<'a, 'hyp, T: Copy + ?Sized> {
pub(super) original: &'a mut T,
pub(super) buffer: XenCallBuffer<'hyp, T>,
}
impl<T: Copy + ?Sized> XenConstBuffer<T> for UnixConstXenBuffer<'_, '_, T> {
fn as_hypercall_ptr(&self) -> *const T {
self.buffer.ptr.as_ptr()
}
}
impl<T: Copy + ?Sized> XenMutBuffer<T> for UnixMutXenBuffer<'_, '_, T> {
fn as_hypercall_ptr(&mut self) -> *mut T {
self.buffer.ptr.as_ptr()
}
unsafe fn update(&mut self) {
// SAFETY: Caller must ensure that data pointed in `buffer` is
valid for T.
*self.original = self.buffer.read();
}
}
It can add additional operations to have what's needed to have a usable
pointer to the data (e.g making a bounce buffer for hosted/amd-sev).
As it may or may not be needed, may rely on external things, I kept it
as a trait to let the implementor define how it actually works.
>> +
>> +Example use:
>> +
>> +```rust
>> +fn demo_hypercall<H: XenHypercall>(interface: &H, buffer: &mut [u8]) ->
>> Result<(), H::Error> {
>> + let buffer_size = buffer.len();
>> + // make a hypercall-safe wrapper of `buffer`
>> + let hyp_buffer = interface.make_mut_slice(buffer)?;
>> +
>> + let op = SomeHypercallStruct {
>> + buffer: hyp_buffer.as_hypercall_ptr(),
>> + buffer_size: buffer_size as _,
>> + };
>> + // Do the same for hyp_op
>> + let hyp_op = interface.make_const_object(&op)?;
>> +
>> + unsafe {
>> + interface.hypercall1(SOME_CMD, hyp_op.as_hypercall_ptr().addr());
>> + // assume success
>> + hyp_buffer.update(); // update buffer back
>> + }
>> +
>> + Ok(())
>> +}
>> +```
>
> One of the bits I liked about the original libxen I showed at Xen Summit were
> the `IsSysCtl` and `IsDomctl` traits. We can generate them from xenbindgen
> itself and use them as constraints so you don't try to use an invalid type for
> a hypercall. It'll need tweaking, I guess. But the interface cannot be safe
> until you can't mess up the arguments.
>
> evtchn and gntops work differently, but the same idea ought to work.
>
I think it depends on what we want to expose to the user, whether it is
the safer variant of raw hypercalls or something less similar.
>> +need to use special functions for that (`core::ptr::slice_from_raw_parts`
>> ?).
>> +But for that, we need to know that T is actually a slice before using this
>> function.
>> +
>> +## Event channels
>> +
>> +TODO
>> +
>> +[1] - Interfacing Rust with Xen - Alejandro Vallejo, XenServer BU, Cloud
>> Software Group
>> +https://youtu.be/iFh4n2kbAwM
>> +
>> +[2] - The Stream Trait
>> +https://rust-lang.github.io/async-book/05_streams/01_chapter.html
>> +
>> +[3] - Aliasing
>> +https://doc.rust-lang.org/nomicon/aliasing.html
>> +
>> +[4] - https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html
>> +https://rust-lang.github.io/rfcs/3498-lifetime-capture-rules-2024.html
>
> I think it'll be a while under we settle on a design.
>
> My latest advances with xenbindgen involve formalizing evtchn, and that should
> be a fantastic playing ground for experimenting. I suspect we'll struggle to
> see the big picture until the foundational stuff is in place and we try to
> expose real hypercalls.
>
Yes, we need to experiment and discuss on what would be best.
Regarding event channels, I tried to design a interface that works with
either hypercalls or with evtchn device in a similar fashion to what we
can do with xenctrl/xenevtchn.
https://gitlab.com/TSnake41/xen/-/tree/rust-rfc/tools/rust/xen/src/event
> Cheers,
> Alejandro
Teddy
Teddy Astie | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |