diff --git a/ostd/src/arch/x86/device/serial.rs b/ostd/src/arch/x86/device/serial.rs index 8421a344..7a7d0f58 100644 --- a/ostd/src/arch/x86/device/serial.rs +++ b/ostd/src/arch/x86/device/serial.rs @@ -34,23 +34,20 @@ impl SerialPort { /// /// # Safety /// - /// User must ensure the `port` is valid serial port. + /// The caller must ensure that the base port is a valid serial base port and that it has + /// exclusive ownership of the serial ports. pub const unsafe fn new(port: u16) -> Self { - let data = IoPort::new(port); - let int_en = IoPort::new(port + 1); - let fifo_ctrl = IoPort::new(port + 2); - let line_ctrl = IoPort::new(port + 3); - let modem_ctrl = IoPort::new(port + 4); - let line_status = IoPort::new(port + 5); - let modem_status = IoPort::new(port + 6); - Self { - data, - int_en, - fifo_ctrl, - line_ctrl, - modem_ctrl, - line_status, - modem_status, + // SAFETY: The safety is upheld by the caller. + unsafe { + Self { + data: IoPort::new(port), + int_en: IoPort::new(port + 1), + fifo_ctrl: IoPort::new(port + 2), + line_ctrl: IoPort::new(port + 3), + modem_ctrl: IoPort::new(port + 4), + line_status: IoPort::new(port + 5), + modem_status: IoPort::new(port + 6), + } } } diff --git a/ostd/src/arch/x86/iommu/fault.rs b/ostd/src/arch/x86/iommu/fault.rs index 9ec3f5fc..77b2b661 100644 --- a/ostd/src/arch/x86/iommu/fault.rs +++ b/ostd/src/arch/x86/iommu/fault.rs @@ -32,12 +32,14 @@ impl FaultEventRegisters { FaultStatus::from_bits_truncate(self.status.as_ptr().read()) } - /// Creates an instance from base address. + /// Creates an instance from the IOMMU base address. /// /// # Safety /// - /// User must ensure the base_register_vaddr is read from DRHD + /// The caller must ensure that the base address is a valid IOMMU base address and that it has + /// exclusive ownership of the IOMMU fault event registers. unsafe fn new(base_register_vaddr: NonNull) -> Self { + // SAFETY: The safety is upheld by the caller. let (capability, status, mut control, mut data, mut address, upper_address) = unsafe { let base = base_register_vaddr; ( @@ -231,9 +233,12 @@ pub(super) static FAULT_EVENT_REGS: Once) { - FAULT_EVENT_REGS.call_once(|| SpinLock::new(FaultEventRegisters::new(base_register_vaddr))); + FAULT_EVENT_REGS + // SAFETY: The safety is upheld by the caller. + .call_once(|| SpinLock::new(unsafe { FaultEventRegisters::new(base_register_vaddr) })); } fn iommu_fault_handler(_frame: &TrapFrame) { diff --git a/ostd/src/arch/x86/iommu/registers/invalidation.rs b/ostd/src/arch/x86/iommu/registers/invalidation.rs index 05c582d0..86dab8f0 100644 --- a/ostd/src/arch/x86/iommu/registers/invalidation.rs +++ b/ostd/src/arch/x86/iommu/registers/invalidation.rs @@ -30,11 +30,12 @@ pub struct InvalidationRegisters { } impl InvalidationRegisters { - /// Creates an instance from IOMMU base address. + /// Creates an instance from the IOMMU base address. /// /// # Safety /// - /// User must ensure the address is valid. + /// The caller must ensure that the base address is a valid IOMMU base address and that it has + /// exclusive ownership of the IOMMU invalidation registers. pub(super) unsafe fn new(base: NonNull) -> Self { let offset = { // SAFETY: The safety is upheld by the caller. diff --git a/ostd/src/arch/x86/iommu/registers/mod.rs b/ostd/src/arch/x86/iommu/registers/mod.rs index 224cd1fd..b297c37a 100644 --- a/ostd/src/arch/x86/iommu/registers/mod.rs +++ b/ostd/src/arch/x86/iommu/registers/mod.rs @@ -278,8 +278,11 @@ impl IommuRegisters { io_mem_builder.remove(base_address as usize..(base_address as usize + PAGE_SIZE)); 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: + // - We trust the ACPI tables (as well as the DRHD in them), from which the base address is + // obtained, so it is a valid IOMMU base address. + // - `io_mem_builder.remove()` guarantees that we have exclusive ownership of all the IOMMU + // registers. let iommu_regs = unsafe { fault::init(base); diff --git a/ostd/src/arch/x86/kernel/apic/ioapic.rs b/ostd/src/arch/x86/kernel/apic/ioapic.rs index f34174cc..e4496e11 100644 --- a/ostd/src/arch/x86/kernel/apic/ioapic.rs +++ b/ostd/src/arch/x86/kernel/apic/ioapic.rs @@ -136,6 +136,7 @@ impl IoApicAccess { /// The caller must ensure that the base address is a valid I/O APIC base address. unsafe fn new(base_address: usize, io_mem_builder: &IoMemAllocatorBuilder) -> Self { io_mem_builder.remove(base_address..(base_address + 0x20)); + if_tdx_enabled!({ assert_eq!( base_address % crate::mm::PAGE_SIZE, @@ -156,9 +157,18 @@ impl IoApicAccess { unsafe { tdx_guest::unprotect_gpa_range(base_address, 1).unwrap() }; }); - let base = NonNull::new(paddr_to_vaddr(base_address) as *mut u8).unwrap(); - let register = VolatileRef::new_restricted(WriteOnly, base.cast::()); - let data = VolatileRef::new(base.add(0x10).cast::()); + let register_addr = NonNull::new(paddr_to_vaddr(base_address) as *mut u32).unwrap(); + // SAFETY: + // - The caller guarantees that the memory is an I/O ACPI register. + // - `io_mem_builder.remove()` guarantees that we have exclusive ownership of the register. + let register = unsafe { VolatileRef::new_restricted(WriteOnly, register_addr) }; + + let data_addr = NonNull::new(paddr_to_vaddr(base_address + 0x10) as *mut u32).unwrap(); + // SAFETY: + // - The caller guarantees that the memory is an I/O ACPI register. + // - `io_mem_builder.remove()` guarantees that we have exclusive ownership of the register. + let data = unsafe { VolatileRef::new(data_addr) }; + Self { register, data } }