Best way to volatile update structured data as view on [u8] array

#1

Hi there,

I do have an array of [u8;4096] at a specific memory address. This "region" will be treated as some generic buffer that can hold structured data. As the contents are read from different hardware/process that is not in control of my rust implementation this region need to be updated in a volatile way. Further more the structured data has varying length.

This is what I actually was able to come up with:

use core::{mem, slice, ptr};

#[repr(C)]
struct Slot {
    data: [u8; 4096],
}

/// A message that is stored within the [Slot]. Access to the message is only available through the [SlotAccessor] 
/// functions.
#[repr(C)]
pub struct SlotMessage {
    pub header: SlotMessageHeader,
    pub data: Vec<u8>, //[u8; 0] 
}

#[repr(C)]
pub struct SlotMessageHeader {
    pub msgid: u32,
    pub size: u32,
}

struct SlotAccessor {
    /// Raw pointer to the array of [Slot]s
    inner: *mut Slot,
    /// The len of the array the raw pointer points to
    len: usize,
}

impl SlotAccessor {
    /// Create the [SlotAccessor] based on the raw pointer and the desired array length.
    ///
    /// # Safety
    /// This is safe if the raw pointer is ensured to point to a memory region of the [Slot] format with at least 'len'
    /// concecutive elements.
    pub unsafe fn new(slot_array: *mut Slot, len: usize) -> Self {
        Self {
            inner: slot_array,
            len,
        }
    }
    
    pub fn store_message<F: FnOnce(&mut SlotMessage)>(
        &mut self,
        slot_index: isize,
        slot_offset: usize,
        payload_size: usize,
        f: F)
    {
        assert!(slot_index > 0);
        assert!((slot_index as usize) < self.len);
        unsafe {
            let slot_ptr = self.inner.offset(slot_index);
            let msg_ptr = &mut (*slot_ptr).data[slot_offset & 4095] as *mut u8 as *mut SlotMessageHeader;
            let payload_ptr = (msg_ptr as *mut u8).offset(mem::size_of::<SlotMessageHeader>() as isize);
            let msg_hdr = ptr::read_volatile(msg_ptr);
            let msg_payload = slice::from_raw_parts_mut(payload_ptr, payload_size).to_vec();
            let mut message = SlotMessage {
                header: msg_hdr,
                data: msg_payload,
            };
            f(&mut message);
            ptr::write_volatile(msg_ptr, message.header);
            for p in 0..message.data.len() {
                ptr::write_volatile(payload_ptr.offset(p as isize), message.data[p]);
            }
        }
    }
}

But the store_message really "smells" to me and feels odd. Is there any better way to achieve this? I'd liked to pass the message payload data as a slice instead of a vec to the closure but if I need to do this intermediate step it's fine assuming the performance hit is not to dramatic :wink:

Thanks in advance for your time and hints...

#2

The only alternative I can think of is for SlotMessage to hold a private reference (or pointer) to a slice, and allow mutation only through a public setter. For example:

pub struct SlotMessage {
    pub header: SlotMessageHeader,
    data: *mut [u8],
}

impl SlotMessage {
    pub fn as_slice(&self) -> &[u8] {
        unsafe { &*self.data }
    }
    
    pub fn set(&mut self, i: usize, val: u8) {
        unsafe {
            (*self.data)[i..].as_mut_ptr().write_volatile(val);
        }
    }
}

You can't safely give out &mut [u8] access, nor use the IndexMut trait to provide syntax sugar for set, because those would both allow callers to make non-volatile writes to the volatile region.

#3

It seems like you should be using raw pointers for that, as the compiler would assume that the program has exclusive access to the mutable slice, as that's what mutability means on references?

#4

Yes, good point. Edited my previous comment to use a raw pointer.

The as_slice method might also be unsound if the memory might change while the SlotMessage is borrowed. It could be replaced by a .get(i) method similar to set.

#5

The set function also appears incorrect now, as the (*self.data)[i..] expression creates a mutable reference to (part of) the memory region, the creation of which is an assertion that we have exclusive access, no?

#6

Ugh, yes. Should be something like

(self.data as *mut u8).add(i).write_volatile(val)

plus an explicit bounds check.

(Though if you actually do have exclusive access to this memory, then this change isn’t necessary.)

#7

Hi,

thanks to all of you for your valuable input so far. As I'm still a bit struggling with how I would properly create the SlotMessage as proposed by @mbrubeck as a view into a [u8;4096] array/slice and using the set function to update an individual element in the payload of the SlotMessage I thought whether it could be an option to make the SlotMessage generic over the data/payload and then rewrite the function to store a message like this:

#[repr(C)]
pub struct SlotMessage<T> {
    pub header: SlotMessageHeader,
    pub data: T, //[u8; 0] 
}

pub fn store_message<T, F: FnOnce(&mut SlotMessage<T>)>(
        &mut self,
        slot_index: isize,
        slot_offset: usize,
        //payload_size: usize,
        f: F)
    {
        assert!(slot_index > 0);
        assert!((slot_index as usize) < self.len);
        unsafe {
            let slot_ptr = self.inner.offset(slot_index);
            let msg_ptr = &mut (*slot_ptr).data[slot_offset & 4095] as *mut u8 as *mut SlotMessage<T>;
            let mut message = ptr::read_volatile(msg_ptr);
            f(&mut message);
            ptr::write_volatile(msg_ptr, message);
        }
    }

The usage then will look like this:

fn main() {
    let mut slots = [Slot {data: [0; 4096]}; 10];
    let mut accessor = unsafe { SlotAccessor::new(slots.as_mut_ptr(), slots.len()) };
    accessor.store_message(0, 0, |msg: &mut SlotMessage<[u8;10]>| {
        msg.header.msgid = 100;
        msg.header.size = core::mem::size_of::<SlotMessage<[u8;10]>>() as u32;
        msg.data = [0;10];
    });
}

This pretty much compiles fine so far, but I'm still a bit concerned thit it might be sound? Another question then would also be: is there a way to constrain T in the excample above to be an array with any size? (create a custom trait and implement it for an arbitrary number of array length to be used as trait bound for T does not feel as usefull apprach to me)

#8

That looks like a reasonable approach to me.

The language design team is now working on stabilizing a subset of const generics that will provide a much cleaner way to do this. Until then, however, the best options I know are the one you suggested, or a similar approach using type-level numbers as in generic-array.

#9

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.