Skip to content
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

Abstract VolatileRef and VolatilePtr over memory access operations #43

Open
nspin opened this issue Jan 29, 2024 · 0 comments
Open

Abstract VolatileRef and VolatilePtr over memory access operations #43

nspin opened this issue Jan 29, 2024 · 0 comments

Comments

@nspin
Copy link
Contributor

nspin commented Jan 29, 2024

This crate could be generalized by abstracting VolatileRef and VolatilePtr over the set of memory access operations to be used. The core idea of declaring a region of memory to be safe to access with a certain set of memory access operations; assigning it type; and safely refining it using field access and slice indexing operations is useful beyond just those cases where volatile operations are appropriate.

Here is a sketch of the idea:

#[repr(transparent)]
pub struct AbstractRef<'a, T, O, A = ReadWrite>
where
    T: ?Sized,
{
    pointer: NonNull<T>,
    reference: PhantomData<&'a T>,
    ops: PhantomData<O>,
    access: PhantomData<A>,
}

#[repr(transparent)]
pub struct AbstractPtr<'a, T, O, A = ReadWrite>
where
    T: ?Sized,
{
    pointer: NonNull<T>,
    reference: PhantomData<&'a T>,
    ops: PhantomData<O>,
    access: PhantomData<A>,
}

pub trait Ops: Copy + Default {}

pub trait UnitaryOps<T>: Ops {
    unsafe fn read(src: *const T) -> T;
    unsafe fn write(dst: *mut T, src: T);
}

pub trait BulkOps<T>: Ops {
    unsafe fn memmove(dst: *mut T, src: *const T, count: usize);
    unsafe fn memcpy(dst: *mut T, src: *const T, count: usize);
    unsafe fn memset(dst: *mut T, val: u8, count: usize);
}

#[derive(Default, Copy, Clone)]
pub struct VolatileOps;

impl Ops for VolatileOps {}

impl<T> UnitaryOps<T> for VolatileOps {
    unsafe fn read(src: *const T) -> T {
        unsafe { ptr::read_volatile(src) }
    }

    unsafe fn write(dst: *mut T, src: T) {
        unsafe { ptr::write_volatile(dst, src) }
    }
}

#[cfg(feature = "unstable")]
impl<T> BulkOps<T> for VolatileOps {
    unsafe fn memmove(dst: *mut T, src: *const T, count: usize) {
        unsafe { intrinsics::volatile_copy_memory(dst, src, count) }
    }

    unsafe fn memcpy(dst: *mut T, src: *const T, count: usize) {
        unsafe { intrinsics::volatile_copy_nonoverlapping_memory(dst, src, count) }
    }

    unsafe fn memset(dst: *mut T, val: u8, count: usize) {
        unsafe { intrinsics::volatile_set_memory(dst, val, count) }
    }
}

pub type VolatileRef<'a, T, A = ReadWrite> = AbstractRef<'a, T, VolatileOps, A>;
pub type VolatilePtr<'a, T, A = ReadWrite> = AbstractPtr<'a, T, VolatileOps, A>;

I work on Rust support for seL4 userspace [1]. I’ve been using a branch [2] of the volatile crate, with this proposal implemented, for regions of memory shared with untrusted neighbor processes, and for DMA. In most of these cases, the type of the top-level AbstractRef is either some kind of buffer queue or a large slice of bytes used as a pool for buffers.

It turns out that the story for accessing memory outside of Rust's memory model (e.g. MMIO or externally shared memory such as in the case of DMA) is quite complicated, and many aspects are still up in the air [3][4]. A design like the one I'm proposing allows library users to choose for themselves which memory operations to use, taking into account not only the basic character of the memory in question, but also their particular hardware and their tolerance for undefined behavior.

For the case of externally shared memory, for example, Fuchsia opts to use normal pointer operations with some extra tricks to prevent unwanted optimization [5]. The thread in [3], however, suggests that using normal pointer operations in a context like this results in undefined behavior, and that using LLVM's unordered atomic intrinsics is safe (or at least safer). However, also according to that thread, codegen for those unordered atomic intrinsics is poor at this time. For seL4 userspace, I've implemented both [6][7].

Abstracting over memory access operations also yields the opportunity for library users to enforce further safety at the type-level. For example, one could reasonably wish to limit accesses to types which implement the zerocopy traits [8]. My wrapper type here [9] allows an AbstractRef to be created for any type, but limits actual memory operations to only types for which reading and writing in untrusted memory is guaranteed to be free of UB in the ways explained in the zerocopy docs.

I would understand if an abstraction like this were outside of this crate's scope. However, I believe that a change like this would make this crate's great core idea available in a much wider range of circumstances, without adding much additional complexity to the crate, neither for developers nor users.

For reference, nspin#1 displays the patch implementing this design that I've been using in my own branch. For the sake of minimizing the diff, I haven't actually renamed the VolatileRef and VolatilePtr types.

[1] https://github.com/seL4/rust-sel4
[2] https://github.com/coliasgroup/volatile/tree/coliasgroup
[3] https://internals.rust-lang.org/t/loads-and-stores-to-from-outside-the-memory-model/10350
[4] rust-lang/unsafe-code-guidelines#152
[5] https://fuchsia.googlesource.com/fuchsia/+/master/src/lib/shared-buffer/src/lib.rs
[6] https://github.com/seL4/rust-sel4/blob/main/crates/sel4-externally-shared/src/ops/normal_ops.rs
[7] https://github.com/seL4/rust-sel4/blob/main/crates/sel4-externally-shared/src/ops/unordered_atomic_ops.rs
[8] https://docs.rs/zerocopy/latest/zerocopy/
[9] https://github.com/seL4/rust-sel4/blob/main/crates/sel4-externally-shared/src/ops/zerocopy_ops.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant