mmio: use one `VolatilePtr` instead of many `AtomicPtr`

[mkroening]

For MMIO the serial port is defined as:

pub struct MmioSerialPort {
    data: AtomicPtr<u8>,
    int_en: AtomicPtr<u8>,
    fifo_ctrl: AtomicPtr<u8>,
    line_ctrl: AtomicPtr<u8>,
    modem_ctrl: AtomicPtr<u8>,
    line_sts: AtomicPtr<u8>,
}

with

        Self {
            data: AtomicPtr::new(base_pointer),
            int_en: AtomicPtr::new(base_pointer.add(1)),
            fifo_ctrl: AtomicPtr::new(base_pointer.add(2)),
            line_ctrl: AtomicPtr::new(base_pointer.add(3)),
            modem_ctrl: AtomicPtr::new(base_pointer.add(4)),
            line_sts: AtomicPtr::new(base_pointer.add(5)),
        }

Instead, it should be only one pointer in size and using volatile operations instead of atomic ones via #[derive(VolatileFieldAccess).

I can take a look at this in the coming weeks.

[ChocolateLoverRaj]

I think besides being volatile, the only thing an atomic pointer would do is make sure that part of a byte isn't written before the other part of the byte (which Rust wouldn't do anyways, but it doesn't promise that it won't). So nothing should break with this change right? I was also confused why the code uses AtomicPtr and not volatile.

[ChocolateLoverRaj]

@mkroening how would this work with stride (for example with each register takes up 32 bits in MMIO).

[mkroening]

We could make VolatileFieldAccess work with const generics and then do

#[derive(VolatileFieldAccess)]
pub struct MmioSerialPort<const PAD: usize = 0> {
    data: u8,
    __pad1: [u8; PAD],
    int_en: u8,
    __pad2: [u8; PAD],
    fifo_ctrl: u8,
    __pad3: [u8; PAD],
    line_ctrl: u8,
    __pad4: [u8; PAD],
    modem_ctrl: u8,
    __pad5: [u8; PAD],
    line_sts: u8,
    __pad6: [u8; PAD],
}

What do you think about this approach?

[ChocolateLoverRaj]

What do you think about this approach?

The PAD would have to be stride - 1, which I'm not sure if it's possible. And I have a use case where I don't know the stride in compile time. I only know the stride at run time, which I don't think would would work in this case.

I made #39, which still uses 6 different volatile pointers instead of a volatile struct, but I could adjust it to instead have functions like get_register_with_offset(offset: usize) so that only a single u16 (for ports) or pointer (for MMIO) will need to be stored. Then we could keep using volatile pointers but we could just add index * stride in run time instead of using VolatileFieldAccess.

[mkroening]

I see. Having ptr_base(&self) -> VolatilePtr<'_, u8>, ptr_data(&self) -> VolatilePtr<'_, u8> while only having one VolatilePtr<'a, u8> and the stride inside the struct sounds sensible to me. 👍