From 0b597d84a0925d973f4eba5990abbd1de41bbfb2 Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Thu, 23 Oct 2025 23:59:33 +0800 Subject: [PATCH] Use `IoMem` in local APIC --- ostd/src/arch/x86/kernel/apic/mod.rs | 22 ++-- ostd/src/arch/x86/kernel/apic/x2apic.rs | 41 +++---- ostd/src/arch/x86/kernel/apic/xapic.rs | 150 +++++++++++++----------- ostd/src/boot/smp.rs | 6 +- 4 files changed, 115 insertions(+), 104 deletions(-) diff --git a/ostd/src/arch/x86/kernel/apic/mod.rs b/ostd/src/arch/x86/kernel/apic/mod.rs index 25dc7232e..44e1fbb69 100644 --- a/ostd/src/arch/x86/kernel/apic/mod.rs +++ b/ostd/src/arch/x86/kernel/apic/mod.rs @@ -4,9 +4,12 @@ use alloc::boxed::Box; use bit_field::BitField; use spin::Once; -use xapic::get_xapic_base_address; -use crate::{cpu::PinCurrentCpu, cpu_local, io::IoMemAllocatorBuilder}; +use crate::{ + cpu::PinCurrentCpu, + cpu_local, + io::{IoMem, IoMemAllocatorBuilder, Sensitive}, +}; mod x2apic; mod xapic; @@ -71,8 +74,8 @@ pub fn get_or_init(_guard: &dyn PinCurrentCpu) -> &(dyn Apic + 'static) { // Initialize the APIC instance now. apic_instance.call_once(|| match APIC_TYPE.get().unwrap() { - ApicType::XApic => { - let mut xapic = xapic::XApic::new().unwrap(); + ApicType::XApic(io_mem) => { + let mut xapic = xapic::XApic::new(io_mem).unwrap(); xapic.enable(); let version = xapic.version(); log::info!( @@ -136,7 +139,7 @@ pub trait ApicTimer { } enum ApicType { - XApic, + XApic(IoMem), X2Apic, } @@ -251,7 +254,7 @@ impl ApicId { impl From for ApicId { fn from(value: u32) -> Self { match APIC_TYPE.get().unwrap() { - ApicType::XApic => ApicId::XApic(value as u8), + ApicType::XApic(_) => ApicId::XApic(value as u8), ApicType::X2Apic => ApicId::X2Apic(value), } } @@ -354,9 +357,10 @@ pub fn init(io_mem_builder: &IoMemAllocatorBuilder) -> Result<(), ApicInitError> Ok(()) } else if xapic::XApic::has_xapic() { log::info!("xAPIC found!"); - let base_address = get_xapic_base_address(); - io_mem_builder.remove(base_address..(base_address + size_of::<[u32; 256]>())); - APIC_TYPE.call_once(|| ApicType::XApic); + // SAFETY: xAPIC is present. + let base_address = unsafe { xapic::read_xapic_base_address() }; + let io_mem = io_mem_builder.reserve(base_address..(base_address + xapic::XAPIC_MMIO_SIZE)); + APIC_TYPE.call_once(|| ApicType::XApic(io_mem)); Ok(()) } else { log::warn!("Neither x2APIC nor xAPIC found!"); diff --git a/ostd/src/arch/x86/kernel/apic/x2apic.rs b/ostd/src/arch/x86/kernel/apic/x2apic.rs index b03b8c1a2..901d2342a 100644 --- a/ostd/src/arch/x86/kernel/apic/x2apic.rs +++ b/ostd/src/arch/x86/kernel/apic/x2apic.rs @@ -9,7 +9,7 @@ use x86::msr::{ use super::ApicTimer; #[derive(Debug)] -pub struct X2Apic { +pub(super) struct X2Apic { _private: (), } @@ -19,10 +19,11 @@ impl !Send for X2Apic {} impl !Sync for X2Apic {} impl X2Apic { - pub(crate) fn new() -> Option { + pub(super) fn new() -> Option { if !Self::has_x2apic() { return None; } + Some(Self { _private: () }) } @@ -32,28 +33,27 @@ impl X2Apic { has_extensions(IsaExtensions::X2APIC) } - pub fn enable(&mut self) { + pub(super) fn enable(&mut self) { const X2APIC_ENABLE_BITS: u64 = { // IA32_APIC_BASE MSR's EN bit: xAPIC global enable/disable const EN_BIT_IDX: u8 = 11; // IA32_APIC_BASE MSR's EXTD bit: Enable x2APIC mode const EXTD_BIT_IDX: u8 = 10; + (1 << EN_BIT_IDX) | (1 << EXTD_BIT_IDX) }; - // SAFETY: - // This is safe because we are ensuring that the operations are performed on valid MSRs. - // We are using them to read and write to the `IA32_APIC_BASE` and `IA32_X2APIC_SIVR` MSRs, which are well-defined and valid MSRs in x86 systems. - // Therefore, we are not causing any undefined behavior or violating any of the requirements of the `rdmsr` and `wrmsr` functions. + + // SAFETY: These operations enable x2APIC, which is safe because `X2Apic` will only be + // constructed if x2APIC is known to be present. unsafe { - // Enable x2APIC mode globally + // Enable x2APIC and xAPIC if they are not enabled by default. let mut base = rdmsr(IA32_APIC_BASE); - // Enable x2APIC and xAPIC if they are not enabled by default if base & X2APIC_ENABLE_BITS != X2APIC_ENABLE_BITS { base |= X2APIC_ENABLE_BITS; wrmsr(IA32_APIC_BASE, base); } - // Set SVR, Enable APIC and set Spurious Vector to 15 (Reserved irq number) + // Set SVR. Enable APIC and set Spurious Vector to 15 (reserved IRQ number). let svr: u64 = (1 << 8) | 15; wrmsr(IA32_X2APIC_SIVR, svr); } @@ -70,15 +70,14 @@ impl super::Apic for X2Apic { } fn eoi(&self) { - unsafe { - wrmsr(IA32_X2APIC_EOI, 0); - } + unsafe { wrmsr(IA32_X2APIC_EOI, 0) }; } unsafe fn send_ipi(&self, icr: super::Icr) { let _guard = crate::irq::disable_local(); - // SAFETY: These `rdmsr` and `wrmsr` instructions write the interrupt command to APIC and - // wait for results. The caller guarantees it's safe to execute this interrupt command. + + // SAFETY: These operations write the interrupt command to APIC and wait for results. The + // caller guarantees it's safe to execute this interrupt command. unsafe { wrmsr(IA32_X2APIC_ESR, 0); wrmsr(IA32_X2APIC_ICR, icr.0); @@ -97,9 +96,7 @@ impl super::Apic for X2Apic { impl ApicTimer for X2Apic { fn set_timer_init_count(&self, value: u64) { - unsafe { - wrmsr(IA32_X2APIC_INIT_COUNT, value); - } + unsafe { wrmsr(IA32_X2APIC_INIT_COUNT, value) }; } fn timer_current_count(&self) -> u64 { @@ -107,14 +104,10 @@ impl ApicTimer for X2Apic { } fn set_lvt_timer(&self, value: u64) { - unsafe { - wrmsr(IA32_X2APIC_LVT_TIMER, value); - } + unsafe { wrmsr(IA32_X2APIC_LVT_TIMER, value) }; } fn set_timer_div_config(&self, div_config: super::DivideConfig) { - unsafe { - wrmsr(IA32_X2APIC_DIV_CONF, div_config as u64); - } + unsafe { wrmsr(IA32_X2APIC_DIV_CONF, div_config as u64) }; } } diff --git a/ostd/src/arch/x86/kernel/apic/xapic.rs b/ostd/src/arch/x86/kernel/apic/xapic.rs index 731d8baf8..90b49cbcc 100644 --- a/ostd/src/arch/x86/kernel/apic/xapic.rs +++ b/ostd/src/arch/x86/kernel/apic/xapic.rs @@ -1,21 +1,19 @@ // SPDX-License-Identifier: MPL-2.0 -#![expect(dead_code)] - -use x86::apic::xapic; +use x86::{ + apic::xapic, + msr::{rdmsr, wrmsr, IA32_APIC_BASE}, +}; use super::ApicTimer; -use crate::mm; - -const IA32_APIC_BASE_MSR: u32 = 0x1B; -const IA32_APIC_BASE_MSR_BSP: u32 = 0x100; // Processor is a BSP -const IA32_APIC_BASE_MSR_ENABLE: u64 = 0x800; - -const APIC_LVT_MASK_BITS: u32 = 1 << 16; +use crate::{ + io::{IoMem, Sensitive}, + mm::{HasPaddr, HasSize}, +}; #[derive(Debug)] -pub struct XApic { - mmio_start: *mut u32, +pub(super) struct XApic { + io_mem: IoMem, } // The APIC instance can be shared among threads running on the same CPU, but not among those @@ -24,13 +22,16 @@ impl !Send for XApic {} impl !Sync for XApic {} impl XApic { - pub fn new() -> Option { + pub(super) fn new(io_mem: &IoMem) -> Option { if !Self::has_xapic() { return None; } - let address = mm::paddr_to_vaddr(get_xapic_base_address()); + + // SAFETY: xAPIC is present. + assert_eq!(io_mem.paddr(), unsafe { read_xapic_base_address() }); + assert_eq!(io_mem.size(), XAPIC_MMIO_SIZE); Some(Self { - mmio_start: address as *mut u32, + io_mem: io_mem.clone(), }) } @@ -40,60 +41,65 @@ impl XApic { has_extensions(IsaExtensions::XAPIC) } - /// Reads a register from the MMIO region. - fn read(&self, offset: u32) -> u32 { - assert!(offset as usize % 4 == 0); - let index = offset as usize / 4; - debug_assert!(index < 256); - unsafe { core::ptr::read_volatile(self.mmio_start.add(index)) } - } + pub(super) fn enable(&mut self) { + const XAPIC_ENABLE_BITS: u64 = { + // IA32_APIC_BASE MSR's EN bit: xAPIC global enable/disable + const EN_BIT_IDX: u8 = 11; - /// Writes a register in the MMIO region. - fn write(&self, offset: u32, val: u32) { - assert!(offset as usize % 4 == 0); - let index = offset as usize / 4; - debug_assert!(index < 256); - unsafe { core::ptr::write_volatile(self.mmio_start.add(index), val) } - } + 1 << EN_BIT_IDX + }; - pub fn enable(&mut self) { - // Enable xAPIC - set_apic_base_address(get_xapic_base_address()); + // SAFETY: These operations enable xAPIC, which is safe because `XApic` will only be + // constructed if xAPIC is known to be present. + unsafe { + // Enable xAPIC if it is not enabled by default. + wrmsr( + IA32_APIC_BASE, + self.io_mem.paddr() as u64 | XAPIC_ENABLE_BITS, + ); - // Set SVR, Enable APIC and set Spurious Vector to 15 (Reserved irq number) - let svr: u32 = (1 << 8) | 15; - self.write(xapic::XAPIC_SVR, svr); + // Set SVR. Enable APIC and set Spurious Vector to 15 (reserved IRQ number). + let svr: u32 = (1 << 8) | 15; + self.io_mem.write_once(xapic::XAPIC_SVR as usize, &svr); + } } } impl super::Apic for XApic { fn id(&self) -> u32 { - self.read(xapic::XAPIC_ID) + unsafe { self.io_mem.read_once(xapic::XAPIC_ID as usize) } } fn version(&self) -> u32 { - self.read(xapic::XAPIC_VERSION) + unsafe { self.io_mem.read_once(xapic::XAPIC_VERSION as usize) } } fn eoi(&self) { - self.write(xapic::XAPIC_EOI, 0); + unsafe { self.io_mem.write_once(xapic::XAPIC_EOI as usize, &0u32) }; } unsafe fn send_ipi(&self, icr: super::Icr) { let _guard = crate::irq::disable_local(); - self.write(xapic::XAPIC_ESR, 0); - // The upper 32 bits of ICR must be written into XAPIC_ICR1 first, - // because writing into XAPIC_ICR0 will trigger the action of - // interrupt sending. - self.write(xapic::XAPIC_ICR1, icr.upper()); - self.write(xapic::XAPIC_ICR0, icr.lower()); - loop { - let icr = self.read(xapic::XAPIC_ICR0); - if ((icr >> 12) & 0x1) == 0 { - break; - } - if self.read(xapic::XAPIC_ESR) > 0 { - break; + + // SAFETY: These operations write the interrupt command to APIC and wait for results. The + // caller guarantees it's safe to execute this interrupt command. + unsafe { + self.io_mem.write_once(xapic::XAPIC_ESR as usize, &0u32); + // The upper 32 bits of ICR must be written into XAPIC_ICR1 first, + // because writing into XAPIC_ICR0 will trigger the action of + // interrupt sending. + self.io_mem + .write_once(xapic::XAPIC_ICR1 as usize, &icr.upper()); + self.io_mem + .write_once(xapic::XAPIC_ICR0 as usize, &icr.lower()); + loop { + let icr = self.io_mem.read_once::(xapic::XAPIC_ICR0 as usize); + if ((icr >> 12) & 0x1) == 0 { + break; + } + if self.io_mem.read_once::(xapic::XAPIC_ESR as usize) > 0 { + break; + } } } } @@ -101,34 +107,42 @@ impl super::Apic for XApic { impl ApicTimer for XApic { fn set_timer_init_count(&self, value: u64) { - self.write(xapic::XAPIC_TIMER_INIT_COUNT, value as u32); + unsafe { + self.io_mem + .write_once(xapic::XAPIC_TIMER_INIT_COUNT as usize, &(value as u32)) + }; } fn timer_current_count(&self) -> u64 { - self.read(xapic::XAPIC_TIMER_CURRENT_COUNT) as u64 + unsafe { + self.io_mem + .read_once::(xapic::XAPIC_TIMER_CURRENT_COUNT as usize) as u64 + } } fn set_lvt_timer(&self, value: u64) { - self.write(xapic::XAPIC_LVT_TIMER, value as u32); + unsafe { + self.io_mem + .write_once(xapic::XAPIC_LVT_TIMER as usize, &(value as u32)) + }; } fn set_timer_div_config(&self, div_config: super::DivideConfig) { - self.write(xapic::XAPIC_TIMER_DIV_CONF, div_config as u32); + unsafe { + self.io_mem + .write_once(xapic::XAPIC_TIMER_DIV_CONF as usize, &(div_config as u32)) + }; } } -/// Sets APIC base address and enables it -fn set_apic_base_address(address: usize) { - unsafe { - x86_64::registers::model_specific::Msr::new(IA32_APIC_BASE_MSR) - .write(address as u64 | IA32_APIC_BASE_MSR_ENABLE); - } +/// Reads xAPIC base address from the MSR. +/// +/// # Safety +/// +/// The caller must ensure that xAPIC is present. +pub(super) unsafe fn read_xapic_base_address() -> usize { + (unsafe { rdmsr(IA32_APIC_BASE) & 0xffff_f000 }) as usize } -/// Gets xAPIC base address -pub(super) fn get_xapic_base_address() -> usize { - unsafe { - (x86_64::registers::model_specific::Msr::new(IA32_APIC_BASE_MSR).read() & 0xf_ffff_f000) - as usize - } -} +/// The size of the xAPIC MMIO region. +pub(super) const XAPIC_MMIO_SIZE: usize = size_of::<[u32; 256]>(); diff --git a/ostd/src/boot/smp.rs b/ostd/src/boot/smp.rs index 7b0020579..695f534bb 100644 --- a/ostd/src/boot/smp.rs +++ b/ostd/src/boot/smp.rs @@ -136,15 +136,15 @@ fn ap_early_entry(cpu_id: u32) -> ! { // SAFETY: This function is called in the boot context of the AP. unsafe { crate::arch::trap::init() }; + // SAFETY: This function is only called once on this AP. + unsafe { crate::mm::kspace::activate_kernel_page_table() }; + // SAFETY: This function is only called once on this AP, after the BSP has // done the architecture-specific initialization. unsafe { crate::arch::init_on_ap() }; crate::arch::irq::enable_local(); - // SAFETY: This function is only called once on this AP. - unsafe { crate::mm::kspace::activate_kernel_page_table() }; - // SAFETY: // 1. The kernel page table is activated on this AP. // 2. The function is called only once on this AP.