Bump volatile to v0.6.1

Cargo.lock

@@ -1306,7 +1306,7 @@ dependencies = [
  "static_assertions",
  "tdx-guest",
  "unwinding",
- "volatile",
+ "volatile 0.6.1",
  "x86",
  "x86_64 0.14.13",
  "xarray",
@@ -1905,6 +1905,12 @@ version = "0.4.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "442887c63f2c839b346c192d047a7c87e73d0689c9157b00b53dcc27dd5ea793"
 
+[[package]]
+name = "volatile"
+version = "0.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "af8ca9a5d4debca0633e697c88269395493cebf2e10db21ca2dbde37c1356452"
+
 [[package]]
 name = "wasi"
 version = "0.11.0+wasi-snapshot-preview1"
@@ -1949,7 +1955,7 @@ dependencies = [
  "bit_field",
  "bitflags 2.6.0",
  "rustversion",
- "volatile",
+ "volatile 0.4.6",
 ]
 
 [[package]]
@@ -1961,7 +1967,7 @@ dependencies = [
  "bit_field",
  "bitflags 2.6.0",
  "rustversion",
- "volatile",
+ "volatile 0.4.6",
 ]
 
 [[package]]

ostd/Cargo.toml

@@ -39,7 +39,7 @@ smallvec = "1.13.2"
 static_assertions = "1.1.0"
 # The version is pinned to "0.2.3" due to a compilation error with version "0.2.5". Unpin it once the issue is resolved.
 unwinding = { version = "=0.2.3", default-features = false, features = ["fde-gnu-eh-frame-hdr", "hide-trace", "panic", "personality", "unwinder"] }
-volatile = { version = "0.4.5", features = ["unstable"] }
+volatile = "0.6.1"
 xarray = { git = "https://github.com/asterinas/xarray", version = "0.1.0" }
 
 [target.x86_64-unknown-none.dependencies]

ostd/src/arch/x86/iommu/fault.rs

@@ -4,35 +4,35 @@
 #![expect(unused_variables)]
 
 use alloc::vec::Vec;
-use core::fmt::Debug;
+use core::{fmt::Debug, ptr::NonNull};
 
 use bitflags::bitflags;
 use log::info;
 use spin::Once;
-use volatile::{access::ReadWrite, Volatile};
+use volatile::{
+    access::{ReadOnly, ReadWrite},
+    VolatileRef,
+};
 
 use super::registers::Capability;
-use crate::{
+use crate::trap::{IrqLine, TrapFrame};
-    mm::Vaddr,
-    trap::{IrqLine, TrapFrame},
-};
 
 #[derive(Debug)]
 pub struct FaultEventRegisters {
-    status: Volatile<&'static mut u32, ReadWrite>,
+    status: VolatileRef<'static, u32, ReadOnly>,
     /// bit31: Interrupt Mask; bit30: Interrupt Pending.
-    control: Volatile<&'static mut u32, ReadWrite>,
+    control: VolatileRef<'static, u32, ReadWrite>,
-    data: Volatile<&'static mut u32, ReadWrite>,
+    data: VolatileRef<'static, u32, ReadWrite>,
-    address: Volatile<&'static mut u32, ReadWrite>,
+    address: VolatileRef<'static, u32, ReadWrite>,
-    upper_address: Volatile<&'static mut u32, ReadWrite>,
+    upper_address: VolatileRef<'static, u32, ReadWrite>,
-    recordings: Vec<Volatile<&'static mut u128, ReadWrite>>,
+    recordings: Vec<VolatileRef<'static, u128, ReadOnly>>,
 
     fault_irq: IrqLine,
 }
 
 impl FaultEventRegisters {
     pub fn status(&self) -> FaultStatus {
-        FaultStatus::from_bits_truncate(self.status.read())
+        FaultStatus::from_bits_truncate(self.status.as_ptr().read())
     }
 
     /// Creates an instance from base address.
@@ -40,31 +40,54 @@ impl FaultEventRegisters {
     /// # Safety
     ///
     /// User must ensure the base_register_vaddr is read from DRHD
-    unsafe fn new(base_register_vaddr: Vaddr) -> Self {
+    unsafe fn new(base_register_vaddr: NonNull<u8>) -> Self {
-        let capability_reg =
+        let (capability, status, mut control, mut data, mut address, upper_address) = unsafe {
-            Volatile::new_read_only(&*((base_register_vaddr + 0x08) as *const u64));
+            let base = base_register_vaddr;
-        let capability = Capability::new(capability_reg.read());
+            (
+                // capability
+                VolatileRef::new_read_only(base.add(0x08).cast::<u64>()),
+                // status
+                VolatileRef::new_read_only(base.add(0x34).cast::<u32>()),
+                // control
+                VolatileRef::new(base.add(0x38).cast::<u32>()),
+                // data
+                VolatileRef::new(base.add(0x3c).cast::<u32>()),
+                // address
+                VolatileRef::new(base.add(0x40).cast::<u32>()),
+                // upper_address
+                VolatileRef::new(base.add(0x44).cast::<u32>()),
+            )
+        };
 
-        let length = capability.fault_recording_number() + 1;
+        let capability_val = Capability::new(capability.as_ptr().read());
-        let mut recordings = Vec::with_capacity(length as usize);
+        let length = capability_val.fault_recording_number() as usize + 1;
-        let offset = capability.fault_recording_register_offset();
+        let offset = (capability_val.fault_recording_register_offset() as usize) * 16;
+
+        // FIXME: We now trust the hardware. We should instead find a way to check that `length`
+        // and `offset` are reasonable values before proceeding.
+
+        let mut recordings = Vec::with_capacity(length);
         for i in 0..length {
-            recordings.push(Volatile::new(
+            // SAFETY: The safety is upheld by the caller and the correctness of the capability
-                &mut *((base_register_vaddr + 16 * (offset + i) as usize) as *mut u128),
+            // value.
-            ))
+            recordings.push(unsafe {
+                VolatileRef::new_read_only(
+                    base_register_vaddr
+                        .add(offset)
+                        .add(i * 16)
+                        .cast::<u128>(),
+                )
+            })
         }
-        let status = Volatile::new(&mut *((base_register_vaddr + 0x34) as *mut u32));
+
-        let mut control = Volatile::new(&mut *((base_register_vaddr + 0x38) as *mut u32));
-        let mut data = Volatile::new(&mut *((base_register_vaddr + 0x3c) as *mut u32));
-        let mut address = Volatile::new(&mut *((base_register_vaddr + 0x40) as *mut u32));
-        let upper_address = Volatile::new(&mut *((base_register_vaddr + 0x44) as *mut u32));
         let mut fault_irq = IrqLine::alloc().unwrap();
+        fault_irq.on_active(iommu_page_fault_handler);
 
         // Set page fault interrupt vector and address
-        data.write(fault_irq.num() as u32);
+        data.as_mut_ptr().write(fault_irq.num() as u32);
-        address.write(0xFEE0_0000);
+        address.as_mut_ptr().write(0xFEE0_0000);
-        control.write(0);
+        control.as_mut_ptr().write(0);
-        fault_irq.on_active(iommu_page_fault_handler);
+
         FaultEventRegisters {
             status,
             control,
@@ -208,13 +231,13 @@ pub(super) static FAULT_EVENT_REGS: Once<FaultEventRegisters> = Once::new();
 /// # Safety
 ///
 /// User must ensure the base_register_vaddr is read from DRHD
-pub(super) unsafe fn init(base_register_vaddr: Vaddr) {
+pub(super) unsafe fn init(base_register_vaddr: NonNull<u8>) {
     FAULT_EVENT_REGS.call_once(|| FaultEventRegisters::new(base_register_vaddr));
 }
 
 fn iommu_page_fault_handler(frame: &TrapFrame) {
     let fault_event = FAULT_EVENT_REGS.get().unwrap();
     let index = (fault_event.status().bits & FaultStatus::FRI.bits) >> 8;
-    let recording = FaultRecording(fault_event.recordings[index as usize].read());
+    let recording = FaultRecording(fault_event.recordings[index as usize].as_ptr().read());
     info!("Catch iommu page fault, recording:{:x?}", recording)
 }

ostd/src/arch/x86/iommu/registers/invalidation.rs

@@ -2,30 +2,31 @@
 
 //! Invalidation-related registers
 
+use core::ptr::NonNull;
+
 use volatile::{
     access::{ReadOnly, ReadWrite, WriteOnly},
-    Volatile,
+    VolatileRef,
 };
 
 use super::ExtendedCapability;
-use crate::prelude::Vaddr;
 
 #[derive(Debug)]
 pub struct InvalidationRegisters {
-    pub(super) queue_head: Volatile<&'static u64, ReadOnly>,
+    pub(super) queue_head: VolatileRef<'static, u64, ReadOnly>,
-    pub(super) queue_tail: Volatile<&'static mut u64, ReadWrite>,
+    pub(super) queue_tail: VolatileRef<'static, u64, ReadWrite>,
-    pub(super) queue_addr: Volatile<&'static mut u64, ReadWrite>,
+    pub(super) queue_addr: VolatileRef<'static, u64, ReadWrite>,
 
-    pub(super) completion_status: Volatile<&'static mut u32, ReadWrite>,
+    pub(super) completion_status: VolatileRef<'static, u32, ReadWrite>,
-    pub(super) _completion_event_control: Volatile<&'static mut u32, ReadWrite>,
+    pub(super) _completion_event_control: VolatileRef<'static, u32, ReadWrite>,
-    pub(super) _completion_event_data: Volatile<&'static mut u32, ReadWrite>,
+    pub(super) _completion_event_data: VolatileRef<'static, u32, ReadWrite>,
-    pub(super) _completion_event_addr: Volatile<&'static mut u32, ReadWrite>,
+    pub(super) _completion_event_addr: VolatileRef<'static, u32, ReadWrite>,
-    pub(super) _completion_event_upper_addr: Volatile<&'static mut u32, ReadWrite>,
+    pub(super) _completion_event_upper_addr: VolatileRef<'static, u32, ReadWrite>,
 
-    pub(super) _queue_error_record: Volatile<&'static mut u64, ReadOnly>,
+    pub(super) _queue_error_record: VolatileRef<'static, u64, ReadOnly>,
 
-    pub(super) _invalidate_address: Volatile<&'static mut u64, WriteOnly>,
+    pub(super) _invalidate_address: VolatileRef<'static, u64, WriteOnly>,
-    pub(super) _iotlb_invalidate: Volatile<&'static mut u64, ReadWrite>,
+    pub(super) iotlb_invalidate: VolatileRef<'static, u64, ReadWrite>,
 }
 
 impl InvalidationRegisters {
@@ -34,28 +35,37 @@ impl InvalidationRegisters {
     /// # Safety
     ///
     /// User must ensure the address is valid.
-    pub(super) unsafe fn new(base_vaddr: Vaddr) -> Self {
+    pub(super) unsafe fn new(base: NonNull<u8>) -> Self {
-        let extended_capability: Volatile<&u64, ReadOnly> =
+        let offset = {
-            Volatile::new_read_only(&*((base_vaddr + 0x10) as *const u64));
+            // SAFETY: The safety is upheld by the caller.
-        let extend_cap = ExtendedCapability::new(extended_capability.read());
+            let extended_capability =
-        let offset = extend_cap.iotlb_register_offset() as usize * 16;
+                unsafe { VolatileRef::new_read_only(base.add(0x10).cast::<u64>()) };
+            let extend_cap = ExtendedCapability::new(extended_capability.as_ptr().read());
+            extend_cap.iotlb_register_offset() as usize * 16
+        };
 
-        let invalidate_address =
+        // FIXME: We now trust the hardware. We should instead find a way to check that `offset`
-            Volatile::new_write_only(&mut *((base_vaddr + offset) as *mut u64));
+        // are reasonable values before proceeding.
-        let iotlb_invalidate = Volatile::new(&mut *((base_vaddr + offset + 0x8) as *mut u64));
 
+        // SAFETY: The safety is upheld by the caller and the correctness of the capability value.
+        unsafe {
             Self {
-            queue_head: Volatile::new_read_only(&*((base_vaddr + 0x80) as *mut u64)),
+                queue_head: VolatileRef::new_read_only(base.add(0x80).cast::<u64>()),
-            queue_tail: Volatile::new(&mut *((base_vaddr + 0x88) as *mut u64)),
+                queue_tail: VolatileRef::new(base.add(0x88).cast::<u64>()),
-            queue_addr: Volatile::new(&mut *((base_vaddr + 0x90) as *mut u64)),
+                queue_addr: VolatileRef::new(base.add(0x90).cast::<u64>()),
-            completion_status: Volatile::new(&mut *((base_vaddr + 0x9C) as *mut u32)),
+                completion_status: VolatileRef::new(base.add(0x9C).cast::<u32>()),
-            _completion_event_control: Volatile::new(&mut *((base_vaddr + 0xA0) as *mut u32)),
+                _completion_event_control: VolatileRef::new(base.add(0xA0).cast::<u32>()),
-            _completion_event_data: Volatile::new(&mut *((base_vaddr + 0xA4) as *mut u32)),
+                _completion_event_data: VolatileRef::new(base.add(0xA4).cast::<u32>()),
-            _completion_event_addr: Volatile::new(&mut *((base_vaddr + 0xA8) as *mut u32)),
+                _completion_event_addr: VolatileRef::new(base.add(0xA8).cast::<u32>()),
-            _completion_event_upper_addr: Volatile::new(&mut *((base_vaddr + 0xAC) as *mut u32)),
+                _completion_event_upper_addr: VolatileRef::new(base.add(0xAC).cast::<u32>()),
-            _queue_error_record: Volatile::new_read_only(&mut *((base_vaddr + 0xB0) as *mut u64)),
+                _queue_error_record: VolatileRef::new_read_only(base.add(0xB0).cast::<u64>()),
-            _invalidate_address: invalidate_address,
+
-            _iotlb_invalidate: iotlb_invalidate,
+                _invalidate_address: VolatileRef::new_restricted(
+                    WriteOnly,
+                    base.add(offset).cast::<u64>(),
+                ),
+                iotlb_invalidate: VolatileRef::new(base.add(offset).add(0x08).cast::<u64>()),
+            }
         }
     }
 }

ostd/src/arch/x86/iommu/registers/mod.rs

@@ -8,6 +8,8 @@ mod extended_cap;
 mod invalidation;
 mod status;
 
+use core::ptr::NonNull;
+
 use bit_field::BitField;
 pub use capability::Capability;
 use command::GlobalCommand;
@@ -19,7 +21,7 @@ use spin::Once;
 use status::GlobalStatus;
 use volatile::{
     access::{ReadOnly, ReadWrite, WriteOnly},
-    Volatile,
+    VolatileRef,
 };
 
 use super::{
@@ -64,15 +66,15 @@ impl IommuVersion {
 /// Important registers used by IOMMU.
 #[derive(Debug)]
 pub struct IommuRegisters {
-    version: Volatile<&'static u32, ReadOnly>,
+    version: VolatileRef<'static, u32, ReadOnly>,
-    capability: Volatile<&'static u64, ReadOnly>,
+    capability: VolatileRef<'static, u64, ReadOnly>,
-    extended_capability: Volatile<&'static u64, ReadOnly>,
+    extended_capability: VolatileRef<'static, u64, ReadOnly>,
-    global_command: Volatile<&'static mut u32, WriteOnly>,
+    global_command: VolatileRef<'static, u32, WriteOnly>,
-    global_status: Volatile<&'static u32, ReadOnly>,
+    global_status: VolatileRef<'static, u32, ReadOnly>,
-    root_table_address: Volatile<&'static mut u64, ReadWrite>,
+    root_table_address: VolatileRef<'static, u64, ReadWrite>,
-    context_command: Volatile<&'static mut u64, ReadWrite>,
+    context_command: VolatileRef<'static, u64, ReadWrite>,
 
-    interrupt_remapping_table_addr: Volatile<&'static mut u64, ReadWrite>,
+    interrupt_remapping_table_addr: VolatileRef<'static, u64, ReadWrite>,
 
     invalidate: InvalidationRegisters,
 }
@@ -81,7 +83,7 @@ impl IommuRegisters {
     /// Reads the version of IOMMU
     #[expect(dead_code)]
     pub fn read_version(&self) -> IommuVersion {
-        let version = self.version.read();
+        let version = self.version.as_ptr().read();
         IommuVersion {
             major: version.get_bits(4..8) as u8,
             minor: version.get_bits(0..4) as u8,
@@ -90,17 +92,17 @@ impl IommuRegisters {
 
     /// Reads the capability of IOMMU
     pub fn read_capability(&self) -> Capability {
-        Capability::new(self.capability.read())
+        Capability::new(self.capability.as_ptr().read())
     }
 
     /// Reads the extended Capability of IOMMU
     pub fn read_extended_capability(&self) -> ExtendedCapability {
-        ExtendedCapability::new(self.extended_capability.read())
+        ExtendedCapability::new(self.extended_capability.as_ptr().read())
     }
 
     /// Reads the global Status of IOMMU
     pub fn read_global_status(&self) -> GlobalStatus {
-        GlobalStatus::from_bits_truncate(self.global_status.read())
+        GlobalStatus::from_bits_truncate(self.global_status.as_ptr().read())
     }
 
     /// Enables DMA remapping with static RootTable
@@ -110,6 +112,7 @@ impl IommuRegisters {
     ) {
         // Set root table address
         self.root_table_address
+            .as_mut_ptr()
             .write(root_table.lock().root_paddr() as u64);
         self.write_global_command(GlobalCommand::SRTP, true);
         while !self.read_global_status().contains(GlobalStatus::RTPS) {}
@@ -126,7 +129,9 @@ impl IommuRegisters {
             .flags()
             .contains(ExtendedCapabilityFlags::IR));
         // Set interrupt remapping table address
-        self.interrupt_remapping_table_addr.write(table.encode());
+        self.interrupt_remapping_table_addr
+            .as_mut_ptr()
+            .write(table.encode());
         self.write_global_command(GlobalCommand::SIRTP, true);
         while !self.read_global_status().contains(GlobalStatus::IRTPS) {}
 
@@ -141,15 +146,21 @@ impl IommuRegisters {
             // Construct global invalidation of interrupt cache and invalidation wait.
             queue.append_descriptor(InterruptEntryCache::global_invalidation().0);
             let tail = queue.tail();
-            self.invalidate.queue_tail.write((tail << 4) as u64);
+            self.invalidate
-            while (self.invalidate.queue_head.read() >> 4) + 1 == tail as u64 {}
+                .queue_tail
+                .as_mut_ptr()
+                .write((tail << 4) as u64);
+            while (self.invalidate.queue_head.as_ptr().read() >> 4) + 1 == tail as u64 {}
 
             // We need to set the interrupt flag so that the `Invalidation Completion Status Register` can report the completion status.
             queue.append_descriptor(InvalidationWait::with_interrupt_flag().0);
-            self.invalidate.queue_tail.write((queue.tail() << 4) as u64);
+            self.invalidate
+                .queue_tail
+                .as_mut_ptr()
+                .write((queue.tail() << 4) as u64);
 
             // Wait for completion
-            while self.invalidate.completion_status.read() == 0 {}
+            while self.invalidate.completion_status.as_ptr().read() == 0 {}
         } else {
             self.global_invalidation()
         }
@@ -166,7 +177,7 @@ impl IommuRegisters {
             .read_extended_capability()
             .flags()
             .contains(ExtendedCapabilityFlags::QI));
-        self.invalidate.queue_tail.write(0);
+        self.invalidate.queue_tail.as_mut_ptr().write(0);
 
         let mut write_value = queue.base_paddr() as u64;
         // By default, we set descriptor width to 128-bit(0)
@@ -197,7 +208,7 @@ impl IommuRegisters {
 
         write_value |= write_queue_size;
 
-        self.invalidate.queue_addr.write(write_value);
+        self.invalidate.queue_addr.as_mut_ptr().write(write_value);
 
         // Enable Queued invalidation
         self.write_global_command(GlobalCommand::QIE, true);
@@ -206,17 +217,20 @@ impl IommuRegisters {
 
     fn global_invalidation(&mut self) {
         // Set ICC(63) to 1 to requests invalidation and CIRG(62:61) to 01 to indicate global invalidation request.
-        self.context_command.write(0xA000_0000_0000_0000);
+        self.context_command
+            .as_mut_ptr()
+            .write(0xA000_0000_0000_0000);
 
         // Wait for invalidation complete (ICC set to 0).
         let mut value = 0x8000_0000_0000_0000;
         while (value & 0x8000_0000_0000_0000) != 0 {
-            value = self.context_command.read();
+            value = self.context_command.as_ptr().read();
         }
 
         // Set IVT(63) to 1 to requests IOTLB invalidation and IIRG(61:60) to 01 to indicate global invalidation request.
         self.invalidate
-            ._iotlb_invalidate
+            .iotlb_invalidate
+            .as_mut_ptr()
             .write(0x9000_0000_0000_0000);
     }
 
@@ -224,57 +238,55 @@ impl IommuRegisters {
     /// is serviced. User need to check the global status register.
     fn write_global_command(&mut self, command: GlobalCommand, enable: bool) {
         const ONE_SHOT_STATUS_MASK: u32 = 0x96FF_FFFF;
-        let status = self.global_status.read() & ONE_SHOT_STATUS_MASK;
+        let status = self.global_status.as_ptr().read() & ONE_SHOT_STATUS_MASK;
         if enable {
-            self.global_command.write(status | command.bits());
+            self.global_command
+                .as_mut_ptr()
+                .write(status | command.bits());
         } else {
-            self.global_command.write(status & !command.bits());
+            self.global_command
+                .as_mut_ptr()
+                .write(status & !command.bits());
         }
     }
 
     /// Creates an instance from base address
     fn new() -> Option<Self> {
         let dmar = Dmar::new()?;
+        debug!("DMAR: {:#x?}", dmar);
 
-        debug!("DMAR:{:#x?}", dmar);
+        let base_address = dmar
-        let base_address = {
+            .remapping_iter()
-            let mut addr = 0;
+            .find_map(|remapping| match remapping {
-            for remapping in dmar.remapping_iter() {
+                Remapping::Drhd(drhd) => Some(drhd.register_base_addr()),
-                if let Remapping::Drhd(drhd) = remapping {
+                _ => None,
-                    addr = drhd.register_base_addr()
+            })
-                }
+            .expect("no DRHD structure found in the DMAR table");
-            }
+        assert_ne!(base_address, 0, "IOMMU address should not be zero");
-            if addr == 0 {
+        debug!("IOMMU base address: {:#x?}", base_address);
-                panic!("There should be a DRHD structure in the DMAR table");
-            }
-            addr
-        };
 
-        let vaddr: usize = paddr_to_vaddr(base_address as usize);
+        let base = NonNull::new(paddr_to_vaddr(base_address as usize) as *mut u8).unwrap();
-        // SAFETY: All offsets and sizes are strictly adhered to in the manual, and the base address is obtained from Drhd.
+
+        // SAFETY: All offsets and sizes are strictly adhered to in the manual, and the base
+        // address is obtained from DRHD.
         let iommu_regs = unsafe {
-            fault::init(vaddr);
+            fault::init(base);
-            let version = Volatile::new_read_only(&*(vaddr as *const u32));
-            let capability = Volatile::new_read_only(&*((vaddr + 0x08) as *const u64));
-            let extended_capability: Volatile<&u64, ReadOnly> =
-                Volatile::new_read_only(&*((vaddr + 0x10) as *const u64));
-            let global_command = Volatile::new_write_only(&mut *((vaddr + 0x18) as *mut u32));
-            let global_status = Volatile::new_read_only(&*((vaddr + 0x1C) as *const u32));
-            let root_table_address = Volatile::new(&mut *((vaddr + 0x20) as *mut u64));
-            let context_command = Volatile::new(&mut *((vaddr + 0x28) as *mut u64));
-
-            let interrupt_remapping_table_addr = Volatile::new(&mut *((vaddr + 0xb8) as *mut u64));
 
             Self {
-                version,
+                version: VolatileRef::new_read_only(base.cast::<u32>()),
-                capability,
+                capability: VolatileRef::new_read_only(base.add(0x08).cast::<u64>()),
-                extended_capability,
+                extended_capability: VolatileRef::new_read_only(base.add(0x10).cast::<u64>()),
-                global_command,
+                global_command: VolatileRef::new_restricted(
-                global_status,
+                    WriteOnly,
-                root_table_address,
+                    base.add(0x18).cast::<u32>(),
-                context_command,
+                ),
-                invalidate: InvalidationRegisters::new(vaddr),
+                global_status: VolatileRef::new_read_only(base.add(0x1C).cast::<u32>()),
-                interrupt_remapping_table_addr,
+                root_table_address: VolatileRef::new(base.add(0x20).cast::<u64>()),
+                context_command: VolatileRef::new(base.add(0x28).cast::<u64>()),
+
+                interrupt_remapping_table_addr: VolatileRef::new(base.add(0xb8).cast::<u64>()),
+
+                invalidate: InvalidationRegisters::new(base),
             }
         };
 

ostd/src/arch/x86/timer/hpet.rs

@@ -3,12 +3,13 @@
 #![expect(dead_code)]
 
 use alloc::vec::Vec;
+use core::ptr::NonNull;
 
 use acpi::{AcpiError, HpetInfo};
 use spin::Once;
 use volatile::{
     access::{ReadOnly, ReadWrite},
-    Volatile,
+    VolatileRef,
 };
 
 use crate::{
@@ -34,46 +35,58 @@ struct HpetTimerRegister {
 }
 
 struct Hpet {
-    information_register: Volatile<&'static u32, ReadOnly>,
+    information_register: VolatileRef<'static, u32, ReadOnly>,
-    general_configuration_register: Volatile<&'static mut u32, ReadWrite>,
+    general_configuration_register: VolatileRef<'static, u32, ReadWrite>,
-    general_interrupt_status_register: Volatile<&'static mut u32, ReadWrite>,
+    general_interrupt_status_register: VolatileRef<'static, u32, ReadWrite>,
 
-    timer_registers: Vec<Volatile<&'static mut HpetTimerRegister, ReadWrite>>,
+    timer_registers: Vec<VolatileRef<'static, HpetTimerRegister, ReadWrite>>,
     irq: IrqLine,
 }
 
 impl Hpet {
-    fn new(base_address: usize) -> Hpet {
+    /// # Safety
-        let information_register_ref = unsafe {
+    ///
-            &*(paddr_to_vaddr(base_address + OFFSET_ID_REGISTER) as *mut usize as *mut u32)
+    /// The caller must ensure that the address is valid and points to the HPET MMIO region.
-        };
+    unsafe fn new(base_address: NonNull<u8>) -> Hpet {
-        let general_configuration_register_ref = unsafe {
+        // SAFETY: The safety is upheld by the caller.
-            &mut *(paddr_to_vaddr(base_address + OFFSET_CONFIGURATION_REGISTER) as *mut usize
+        let (
-                as *mut u32)
+            information_register,
-        };
+            general_configuration_register,
-        let general_interrupt_status_register_ref = unsafe {
+            general_interrupt_status_register,
-            &mut *(paddr_to_vaddr(base_address + OFFSET_INTERRUPT_STATUS_REGISTER) as *mut usize
+        ) = unsafe {
-                as *mut u32)
+            (
-        };
+                VolatileRef::new_read_only(base_address.add(OFFSET_ID_REGISTER).cast::<u32>()),
-
+                VolatileRef::new(
-        let information_register = Volatile::new_read_only(information_register_ref);
-        let general_configuration_register = Volatile::new(general_configuration_register_ref);
-        let general_interrupt_status_register =
-            Volatile::new(general_interrupt_status_register_ref);
-
-        let num_comparator = ((information_register.read() & 0x1F00) >> 8) as u8 + 1;
-
-        let mut comparators = Vec::with_capacity(num_comparator as usize);
-
-        // Ensure that the addresses in the loop will not overflow
                     base_address
-            .checked_add(0x100 + num_comparator as usize * 0x20)
+                        .add(OFFSET_CONFIGURATION_REGISTER)
-            .unwrap();
+                        .cast::<u32>(),
+                ),
+                VolatileRef::new(
+                    base_address
+                        .add(OFFSET_INTERRUPT_STATUS_REGISTER)
+                        .cast::<u32>(),
+                ),
+            )
+        };
+
+        let num_comparator = ((information_register.as_ptr().read() & 0x1F00) >> 8) as u8 + 1;
+        let num_comparator = num_comparator as usize;
+
+        // FIXME: We now trust the hardware. We should instead find a way to check that
+        // `num_comparator` are reasonable values before proceeding.
+
+        let mut comparators = Vec::with_capacity(num_comparator);
         for i in 0..num_comparator {
-            let comp = Volatile::new(unsafe {
+            // SAFETY: The safety is upheld by the caller and the correctness of the information
-                &mut *(paddr_to_vaddr(base_address + 0x100 + i as usize * 0x20) as *mut usize
+            // value.
-                    as *mut HpetTimerRegister)
+            let comp = unsafe {
-            });
+                VolatileRef::new(
+                    base_address
+                        .add(0x100)
+                        .add(i * 0x20)
+                        .cast::<HpetTimerRegister>(),
+                )
+            };
             comparators.push(comp);
         }
 
@@ -93,34 +106,39 @@ impl Hpet {
     }
 
     pub fn hardware_rev(&self) -> u8 {
-        (self.information_register.read() & 0xFF) as u8
+        (self.information_register.as_ptr().read() & 0xFF) as u8
     }
 
     pub fn num_comparators(&self) -> u8 {
-        ((self.information_register.read() & 0x1F00) >> 8) as u8 + 1
+        ((self.information_register.as_ptr().read() & 0x1F00) >> 8) as u8 + 1
     }
 
     pub fn main_counter_is_64bits(&self) -> bool {
-        (self.information_register.read() & 0x2000) != 0
+        (self.information_register.as_ptr().read() & 0x2000) != 0
     }
 
     pub fn legacy_irq_capable(&self) -> bool {
-        (self.information_register.read() & 0x8000) != 0
+        (self.information_register.as_ptr().read() & 0x8000) != 0
     }
 
     pub fn pci_vendor_id(&self) -> u16 {
-        ((self.information_register.read() & 0xFFFF_0000) >> 16) as u16
+        ((self.information_register.as_ptr().read() & 0xFFFF_0000) >> 16) as u16
     }
 }
 
 /// HPET init, need to init IOAPIC before init this function
 pub fn init() -> Result<(), AcpiError> {
+    let hpet_info = {
         let lock = ACPI_TABLES.get().unwrap().lock();
+        HpetInfo::new(&*lock)?
+    };
 
-    let hpet_info = HpetInfo::new(&*lock)?;
+    assert_ne!(hpet_info.base_address, 0, "HPET address should not be zero");
 
-    // config IO APIC entry
+    let base = NonNull::new(paddr_to_vaddr(hpet_info.base_address) as *mut u8).unwrap();
-    let hpet = Hpet::new(hpet_info.base_address);
+    // SAFETY: The base address is from the ACPI table and points to the HPET MMIO region.
+    let hpet = unsafe { Hpet::new(base) };
     HPET_INSTANCE.call_once(|| hpet);
+
     Ok(())
 }