Add a design document for the 'xen' rust crate.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
Follows Alejandro works with https://lore.kernel.org/xen-devel/D5SQEZIL2SZV.QR3X5MRVQJJP@cloud.com/T/#t
There is also a WIP branch at https://gitlab.com/TSnake41/xen/-/tree/rust-rfc
---
 docs/designs/xen-rust-crate.md | 199 +++++++++++++++++++++++++++++++++
 1 file changed, 199 insertions(+)
 create mode 100644 docs/designs/xen-rust-crate.md

diff --git a/docs/designs/xen-rust-crate.md b/docs/designs/xen-rust-crate.md
new file mode 100644
index 0000000000..5eab603d4d
--- /dev/null
+++ b/docs/designs/xen-rust-crate.md
@@ -0,0 +1,199 @@
+# Rust 'xen' crate interface design
+
+See [1] for more context.
+
+This RFC proposes parts of the 'xen' crate interface that would directly or indirectly
+(through internal wrappers) be used by users.
+Those users could be a userland toolstack, a unikernel application (e.g XTF, Unikraft),
+some other freestanding environment (e.g OVMF, Linux, Redox OS), ...
+
+These users can have a very different execution environment, this crate aims to provide
+a uniform interface while allowing flexibility for exposing platform-specific bits.
+
+## Design philosophy
+
+This crate should feel natural for a Rust developper, thus, any Rust developper with some
+understanding on the Xen hypercall operations should be able to use this crate.
+Moreover, we want this crate to be maintainable and feel "idiomatic", and not introduce
+confusing behavior onto the user. Note that this crate will heavily use unsafe code.
+
+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).
+
+Example:
+  If we provide a event channel abstraction based on 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.
+
+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
+
+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);
+}
+
+// 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>;
+}
+```
+
+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(())
+}
+```
+
+Note that freestanding case, we can use a thin zero-copy wrapper :
+```rust
+/// 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
+    }
+}
+// ...
+```
+
+TODO:
+Do we need to clarify the lifetimes (e.g should trait indicate a lifetime binding with
+original data) ? Try to answer with RPITIT and Rust 2024 capture rules [4].
+
+Try to unify make_const_object and make_const_slice (along with mut variant). `*const [T]`
+is a bit more subtle to create and we cannot trivially cast a address into a pointer and
+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
-- 
2.45.2



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Hi,

I've given some feedback on the primitives for buffers and some questions on
the choice of aggressive use of traits.

On Fri Nov 22, 2024 at 2:01 PM GMT, Teddy Astie wrote:
> Add a design document for the 'xen' rust crate.
>
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> Follows Alejandro works with https://lore.kernel.org/xen-devel/D5SQEZIL2SZV.QR3X5MRVQJJP@cloud.com/T/#t
> There is also a WIP branch at https://gitlab.com/TSnake41/xen/-/tree/rust-rfc
> ---
>  docs/designs/xen-rust-crate.md | 199 +++++++++++++++++++++++++++++++++
>  1 file changed, 199 insertions(+)
>  create mode 100644 docs/designs/xen-rust-crate.md
>
> diff --git a/docs/designs/xen-rust-crate.md b/docs/designs/xen-rust-crate.md
> new file mode 100644
> index 0000000000..5eab603d4d
> --- /dev/null
> +++ b/docs/designs/xen-rust-crate.md
> @@ -0,0 +1,199 @@
> +# Rust 'xen' crate interface design
> +
> +See [1] for more context.
> +
> +This RFC proposes parts of the 'xen' crate interface that would directly or indirectly
> +(through internal wrappers) be used by users.
> +Those users could be a userland toolstack, a unikernel application (e.g XTF, Unikraft),
> +some other freestanding environment (e.g OVMF, Linux, Redox OS), ...
> +
> +These users can have a very different execution environment, this crate aims to provide
> +a uniform interface while allowing flexibility for exposing platform-specific bits.
> +
> +## Design philosophy
> +
> +This crate should feel natural for a Rust developper, thus, any Rust developper with some
> +understanding on the Xen hypercall operations should be able to use this crate.
> +Moreover, we want this crate to be maintainable and feel "idiomatic", and not introduce
> +confusing behavior onto the user. Note that this crate will heavily use unsafe code.
> +
> +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.

> +
> +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...

> +
> +// 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.

> +
> +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.

> +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.

Cheers,
Alejandro

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