From 7e58955dd7a50e1b62539e178ad4375c7c8b85da Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Mon, 17 Feb 2025 12:20:51 +0800 Subject: [PATCH] Bump `volatile` to v0.6.1 --- Cargo.lock | 12 +- ostd/Cargo.toml | 2 +- ostd/src/arch/x86/iommu/fault.rs | 91 +++++++----- .../arch/x86/iommu/registers/invalidation.rs | 76 +++++----- ostd/src/arch/x86/iommu/registers/mod.rs | 134 ++++++++++-------- ostd/src/arch/x86/timer/hpet.rs | 98 +++++++------ 6 files changed, 241 insertions(+), 172 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 48e8b02e2..dea7fe3be 100644 --- a/Cargo.lock +++ b/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]] diff --git a/ostd/Cargo.toml b/ostd/Cargo.toml index 612978fc7..ea49f0747 100644 --- a/ostd/Cargo.toml +++ b/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] diff --git a/ostd/src/arch/x86/iommu/fault.rs b/ostd/src/arch/x86/iommu/fault.rs index bec06a42c..4970e8544 100644 --- a/ostd/src/arch/x86/iommu/fault.rs +++ b/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::{ - mm::Vaddr, - trap::{IrqLine, TrapFrame}, -}; +use crate::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>, - data: Volatile<&'static mut u32, ReadWrite>, - address: Volatile<&'static mut u32, ReadWrite>, - upper_address: Volatile<&'static mut u32, ReadWrite>, - recordings: Vec>, + control: VolatileRef<'static, u32, ReadWrite>, + data: VolatileRef<'static, u32, ReadWrite>, + address: VolatileRef<'static, u32, ReadWrite>, + upper_address: VolatileRef<'static, u32, ReadWrite>, + recordings: Vec>, 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 { - let capability_reg = - Volatile::new_read_only(&*((base_register_vaddr + 0x08) as *const u64)); - let capability = Capability::new(capability_reg.read()); + unsafe fn new(base_register_vaddr: NonNull) -> Self { + let (capability, status, mut control, mut data, mut address, upper_address) = unsafe { + let base = base_register_vaddr; + ( + // capability + VolatileRef::new_read_only(base.add(0x08).cast::()), + // status + VolatileRef::new_read_only(base.add(0x34).cast::()), + // control + VolatileRef::new(base.add(0x38).cast::()), + // data + VolatileRef::new(base.add(0x3c).cast::()), + // address + VolatileRef::new(base.add(0x40).cast::()), + // upper_address + VolatileRef::new(base.add(0x44).cast::()), + ) + }; - let length = capability.fault_recording_number() + 1; - let mut recordings = Vec::with_capacity(length as usize); - let offset = capability.fault_recording_register_offset(); + let capability_val = Capability::new(capability.as_ptr().read()); + let length = capability_val.fault_recording_number() as usize + 1; + 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( - &mut *((base_register_vaddr + 16 * (offset + i) as usize) as *mut u128), - )) + // SAFETY: The safety is upheld by the caller and the correctness of the capability + // value. + recordings.push(unsafe { + VolatileRef::new_read_only( + base_register_vaddr + .add(offset) + .add(i * 16) + .cast::(), + ) + }) } - 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); - address.write(0xFEE0_0000); - control.write(0); - fault_irq.on_active(iommu_page_fault_handler); + data.as_mut_ptr().write(fault_irq.num() as u32); + address.as_mut_ptr().write(0xFEE0_0000); + control.as_mut_ptr().write(0); + FaultEventRegisters { status, control, @@ -208,13 +231,13 @@ pub(super) static FAULT_EVENT_REGS: Once = 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) { 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) } diff --git a/ostd/src/arch/x86/iommu/registers/invalidation.rs b/ostd/src/arch/x86/iommu/registers/invalidation.rs index 3d3ddf3ff..05c582d0a 100644 --- a/ostd/src/arch/x86/iommu/registers/invalidation.rs +++ b/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_tail: Volatile<&'static mut u64, ReadWrite>, - pub(super) queue_addr: Volatile<&'static mut u64, ReadWrite>, + pub(super) queue_head: VolatileRef<'static, u64, ReadOnly>, + pub(super) queue_tail: VolatileRef<'static, u64, ReadWrite>, + pub(super) queue_addr: VolatileRef<'static, u64, ReadWrite>, - pub(super) completion_status: Volatile<&'static mut u32, ReadWrite>, - pub(super) _completion_event_control: Volatile<&'static mut u32, ReadWrite>, - pub(super) _completion_event_data: Volatile<&'static mut u32, ReadWrite>, - pub(super) _completion_event_addr: Volatile<&'static mut u32, ReadWrite>, - pub(super) _completion_event_upper_addr: Volatile<&'static mut u32, ReadWrite>, + pub(super) completion_status: VolatileRef<'static, u32, ReadWrite>, + pub(super) _completion_event_control: VolatileRef<'static, u32, ReadWrite>, + pub(super) _completion_event_data: VolatileRef<'static, u32, ReadWrite>, + pub(super) _completion_event_addr: VolatileRef<'static, 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) _iotlb_invalidate: Volatile<&'static mut u64, ReadWrite>, + pub(super) _invalidate_address: VolatileRef<'static, u64, WriteOnly>, + 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 { - let extended_capability: Volatile<&u64, ReadOnly> = - Volatile::new_read_only(&*((base_vaddr + 0x10) as *const u64)); - let extend_cap = ExtendedCapability::new(extended_capability.read()); - let offset = extend_cap.iotlb_register_offset() as usize * 16; + pub(super) unsafe fn new(base: NonNull) -> Self { + let offset = { + // SAFETY: The safety is upheld by the caller. + let extended_capability = + unsafe { VolatileRef::new_read_only(base.add(0x10).cast::()) }; + let extend_cap = ExtendedCapability::new(extended_capability.as_ptr().read()); + extend_cap.iotlb_register_offset() as usize * 16 + }; - let invalidate_address = - Volatile::new_write_only(&mut *((base_vaddr + offset) as *mut u64)); - let iotlb_invalidate = Volatile::new(&mut *((base_vaddr + offset + 0x8) as *mut u64)); + // FIXME: We now trust the hardware. We should instead find a way to check that `offset` + // are reasonable values before proceeding. - Self { - queue_head: Volatile::new_read_only(&*((base_vaddr + 0x80) as *mut u64)), - queue_tail: Volatile::new(&mut *((base_vaddr + 0x88) as *mut u64)), - queue_addr: Volatile::new(&mut *((base_vaddr + 0x90) as *mut u64)), - completion_status: Volatile::new(&mut *((base_vaddr + 0x9C) as *mut u32)), - _completion_event_control: Volatile::new(&mut *((base_vaddr + 0xA0) as *mut u32)), - _completion_event_data: Volatile::new(&mut *((base_vaddr + 0xA4) as *mut u32)), - _completion_event_addr: Volatile::new(&mut *((base_vaddr + 0xA8) as *mut u32)), - _completion_event_upper_addr: Volatile::new(&mut *((base_vaddr + 0xAC) as *mut u32)), - _queue_error_record: Volatile::new_read_only(&mut *((base_vaddr + 0xB0) as *mut u64)), - _invalidate_address: invalidate_address, - _iotlb_invalidate: iotlb_invalidate, + // SAFETY: The safety is upheld by the caller and the correctness of the capability value. + unsafe { + Self { + queue_head: VolatileRef::new_read_only(base.add(0x80).cast::()), + queue_tail: VolatileRef::new(base.add(0x88).cast::()), + queue_addr: VolatileRef::new(base.add(0x90).cast::()), + completion_status: VolatileRef::new(base.add(0x9C).cast::()), + _completion_event_control: VolatileRef::new(base.add(0xA0).cast::()), + _completion_event_data: VolatileRef::new(base.add(0xA4).cast::()), + _completion_event_addr: VolatileRef::new(base.add(0xA8).cast::()), + _completion_event_upper_addr: VolatileRef::new(base.add(0xAC).cast::()), + _queue_error_record: VolatileRef::new_read_only(base.add(0xB0).cast::()), + + _invalidate_address: VolatileRef::new_restricted( + WriteOnly, + base.add(offset).cast::(), + ), + iotlb_invalidate: VolatileRef::new(base.add(offset).add(0x08).cast::()), + } } } } diff --git a/ostd/src/arch/x86/iommu/registers/mod.rs b/ostd/src/arch/x86/iommu/registers/mod.rs index d6dc245c1..385266578 100644 --- a/ostd/src/arch/x86/iommu/registers/mod.rs +++ b/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>, - capability: Volatile<&'static u64, ReadOnly>, - extended_capability: Volatile<&'static u64, ReadOnly>, - global_command: Volatile<&'static mut u32, WriteOnly>, - global_status: Volatile<&'static u32, ReadOnly>, - root_table_address: Volatile<&'static mut u64, ReadWrite>, - context_command: Volatile<&'static mut u64, ReadWrite>, + version: VolatileRef<'static, u32, ReadOnly>, + capability: VolatileRef<'static, u64, ReadOnly>, + extended_capability: VolatileRef<'static, u64, ReadOnly>, + global_command: VolatileRef<'static, u32, WriteOnly>, + global_status: VolatileRef<'static, u32, ReadOnly>, + root_table_address: VolatileRef<'static, 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); - while (self.invalidate.queue_head.read() >> 4) + 1 == tail as u64 {} + self.invalidate + .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 { let dmar = Dmar::new()?; + debug!("DMAR: {:#x?}", dmar); - debug!("DMAR:{:#x?}", dmar); - let base_address = { - let mut addr = 0; - for remapping in dmar.remapping_iter() { - if let Remapping::Drhd(drhd) = remapping { - addr = drhd.register_base_addr() - } - } - if addr == 0 { - panic!("There should be a DRHD structure in the DMAR table"); - } - addr - }; + let base_address = dmar + .remapping_iter() + .find_map(|remapping| match remapping { + Remapping::Drhd(drhd) => Some(drhd.register_base_addr()), + _ => None, + }) + .expect("no DRHD structure found in the DMAR table"); + assert_ne!(base_address, 0, "IOMMU address should not be zero"); + debug!("IOMMU base address: {:#x?}", base_address); - let vaddr: usize = paddr_to_vaddr(base_address as usize); - // SAFETY: All offsets and sizes are strictly adhered to in the manual, and the base address is obtained from Drhd. + 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. let iommu_regs = unsafe { - fault::init(vaddr); - 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)); + fault::init(base); Self { - version, - capability, - extended_capability, - global_command, - global_status, - root_table_address, - context_command, - invalidate: InvalidationRegisters::new(vaddr), - interrupt_remapping_table_addr, + version: VolatileRef::new_read_only(base.cast::()), + capability: VolatileRef::new_read_only(base.add(0x08).cast::()), + extended_capability: VolatileRef::new_read_only(base.add(0x10).cast::()), + global_command: VolatileRef::new_restricted( + WriteOnly, + base.add(0x18).cast::(), + ), + global_status: VolatileRef::new_read_only(base.add(0x1C).cast::()), + root_table_address: VolatileRef::new(base.add(0x20).cast::()), + context_command: VolatileRef::new(base.add(0x28).cast::()), + + interrupt_remapping_table_addr: VolatileRef::new(base.add(0xb8).cast::()), + + invalidate: InvalidationRegisters::new(base), } }; diff --git a/ostd/src/arch/x86/timer/hpet.rs b/ostd/src/arch/x86/timer/hpet.rs index 51235a259..68de48d2f 100644 --- a/ostd/src/arch/x86/timer/hpet.rs +++ b/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>, - general_configuration_register: Volatile<&'static mut u32, ReadWrite>, - general_interrupt_status_register: Volatile<&'static mut u32, ReadWrite>, + information_register: VolatileRef<'static, u32, ReadOnly>, + general_configuration_register: VolatileRef<'static, u32, ReadWrite>, + general_interrupt_status_register: VolatileRef<'static, u32, ReadWrite>, - timer_registers: Vec>, + timer_registers: Vec>, irq: IrqLine, } impl Hpet { - fn new(base_address: usize) -> Hpet { - let information_register_ref = unsafe { - &*(paddr_to_vaddr(base_address + OFFSET_ID_REGISTER) as *mut usize as *mut u32) - }; - let general_configuration_register_ref = unsafe { - &mut *(paddr_to_vaddr(base_address + OFFSET_CONFIGURATION_REGISTER) as *mut usize - as *mut u32) - }; - let general_interrupt_status_register_ref = unsafe { - &mut *(paddr_to_vaddr(base_address + OFFSET_INTERRUPT_STATUS_REGISTER) as *mut usize - as *mut u32) + /// # Safety + /// + /// The caller must ensure that the address is valid and points to the HPET MMIO region. + unsafe fn new(base_address: NonNull) -> Hpet { + // SAFETY: The safety is upheld by the caller. + let ( + information_register, + general_configuration_register, + general_interrupt_status_register, + ) = unsafe { + ( + VolatileRef::new_read_only(base_address.add(OFFSET_ID_REGISTER).cast::()), + VolatileRef::new( + base_address + .add(OFFSET_CONFIGURATION_REGISTER) + .cast::(), + ), + VolatileRef::new( + base_address + .add(OFFSET_INTERRUPT_STATUS_REGISTER) + .cast::(), + ), + ) }; - 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.as_ptr().read() & 0x1F00) >> 8) as u8 + 1; + let num_comparator = num_comparator as usize; - let num_comparator = ((information_register.read() & 0x1F00) >> 8) as u8 + 1; + // 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 as usize); - - // Ensure that the addresses in the loop will not overflow - base_address - .checked_add(0x100 + num_comparator as usize * 0x20) - .unwrap(); + let mut comparators = Vec::with_capacity(num_comparator); for i in 0..num_comparator { - let comp = Volatile::new(unsafe { - &mut *(paddr_to_vaddr(base_address + 0x100 + i as usize * 0x20) as *mut usize - as *mut HpetTimerRegister) - }); + // SAFETY: The safety is upheld by the caller and the correctness of the information + // value. + let comp = unsafe { + VolatileRef::new( + base_address + .add(0x100) + .add(i * 0x20) + .cast::(), + ) + }; 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 lock = ACPI_TABLES.get().unwrap().lock(); + 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 hpet = Hpet::new(hpet_info.base_address); + let base = NonNull::new(paddr_to_vaddr(hpet_info.base_address) as *mut u8).unwrap(); + // 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(()) }