Merge pull request #1 from qwandor/registers

Use registers struct rather than offsets, and fix soundness issues

CHANGELOG.md

@@ -0,0 +1,18 @@
+# Changelog
+
+## Unreleased
+
+### Breaking changes
+
+- Changed `get_unix_timestamp` and `set_unix_timestamp` to use u32 rather than u64, to match the
+  size of the device registers.
+- Made `Rtc::new` unsafe, as it must be passed a valid pointer.
+- Made `set_unix_timestamp` take `&mut self` rather than `&self` because it writes to device memory.
+
+### Other changes
+
+- Implemented `Send` and `Sync` for `Rtc`.
+
+## 0.1.0
+
+Initial release.

README.md

@@ -9,10 +9,12 @@ System Real Time Clock (RTC) Drivers for aarch64 based on PL031.
 ```rust
 use arm_pl031::Rtc;
 
-let epoch_time = Rtc::new(0x901_0000).get_unix_timestamp();
+let rtc = unsafe { Rtc::new(0x901_0000 as _) };
+let epoch_time = rtc.get_unix_timestamp();
 ```
 
-`base_addr` needs to be the device virtual address available for mmio, which can be obtained from the device tree, for example:
+`base_addr` needs to be the device virtual address available for mmio, which can be obtained from
+the device tree, for example:
 
 ```
 / {
@@ -32,4 +34,4 @@ let epoch_time = Rtc::new(0x901_0000).get_unix_timestamp();
 
     ...
 }
-```
+```

src/lib.rs

@@ -2,40 +2,72 @@
 
 #![cfg_attr(not(test), no_std)]
 
-const RTC_DR: usize = 0x00; //Data Register
-const RTC_LR: usize = 0x08; //Load Register
+use core::ptr::{addr_of, addr_of_mut};
 
-/// The System Real Time Clock structure for aarch64 based on PL031.
-pub struct Rtc {
-    base_address: usize,
+#[repr(C, align(4))]
+struct Registers {
+    /// Data register
+    dr: u32,
+    /// Match register
+    mr: u32,
+    /// Load register
+    lr: u32,
+    /// Control register
+    cr: u8,
+    _reserved0: [u8; 3],
+    /// Interrupt Mask Set or Clear register
+    imsc: u8,
+    _reserved1: [u8; 3],
+    /// Raw Interrupt Status
+    ris: u8,
+    _reserved2: [u8; 3],
+    /// Masked Interrupt Status
+    mis: u8,
+    _reserved3: [u8; 3],
+    /// Interrupt Clear Register
+    icr: u8,
+    _reserved4: [u8; 3],
 }
 
-impl Rtc {
-    unsafe fn read(&self, reg: usize) -> u32 {
-        core::ptr::read_volatile((self.base_address + reg) as *const u32)
-    }
-
-    unsafe fn write(&self, reg: usize, value: u32) {
-        core::ptr::write_volatile((self.base_address + reg) as *mut u32, value);
-    }
+/// The System Real Time Clock structure for aarch64 based on PL031.
+pub struct Rtc {
+    registers: *mut Registers,
 }
 
 impl Rtc {
-    /// Construct a new PL031 RTC structure.
+    /// Constructs a new instance of the RTC driver for a PL031 device at the given base address.
+    ///
+    /// The base address may be obtained from the device tree.
     ///
-    /// `base_addr` represents the device address
-    ///  (which can be obtained from the device tree).
-    pub fn new(base_address: usize) -> Self {
-        Rtc { base_address }
+    /// # Safety
+    ///
+    /// The given base address must point to the MMIO control registers of a PL031 device, which
+    /// must be mapped into the address space of the process as device memory and not have any other
+    /// aliases. It must be aligned to a 4 byte boundary.
+    pub unsafe fn new(base_address: *mut u32) -> Self {
+        Rtc {
+            registers: base_address as _,
+        }
     }
 
     /// Returns the current time in seconds since UNIX epoch.
-    pub fn get_unix_timestamp(&self) -> u64 {
-        unsafe { self.read(RTC_DR) as u64 }
+    pub fn get_unix_timestamp(&self) -> u32 {
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
+        unsafe { addr_of!((*self.registers).dr).read_volatile() }
     }
 
     /// Sets the current time in seconds since UNIX epoch.
-    pub fn set_unix_timestamp(&self, unix_time: u64) {
-        unsafe { self.write(RTC_LR, unix_time as u32) }
+    pub fn set_unix_timestamp(&mut self, unix_time: u32) {
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
+        unsafe { addr_of_mut!((*self.registers).lr).write_volatile(unix_time) }
     }
 }
+
+// SAFETY: `Rtc` just contains a pointer to device memory, which can be accessed from any context.
+unsafe impl Send for Rtc {}
+
+// SAFETY: An `&Rtc` only allows reading device registers, which can safety be done from multiple
+// places at once.
+unsafe impl Sync for Rtc {}