From c6646b4e67dcd64545bc0cd77146201d5bf538e1 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 14:16:54 +0200 Subject: [PATCH 1/4] The local APIC runs in xAPIC mode where CPUID offers no x2APIC The kernel set IA32_APIC_BASE.EXTD unconditionally, which is #GP on a CPU whose CPUID.01H:ECX[21] is clear; on the BSP that fault lands before the IDT loads, so such a machine triple-faults with nothing on any channel. An AMD laptop without x2APIC is one. IA32_APIC_BASE joins control_regs' declaration: written whole, the registers at FEE00000H (a firmware that moved them is refused), x2APIC where CPUID offers it and xAPIC where it does not, decided by the BSP and asserted on every AP. apic.rs reaches every register through one Reg type keyed by its xAPIC MMIO offset, whose x2APIC MSR is 0x800 + offset/16; the xAPIC page is mapped uncacheable by the BSP. The ICR is the one register the modes lay out differently: x2APIC writes it once behind the fence its non-serializing WRMSR needs, xAPIC writes the 8-bit destination half and then the command half with interrupts closed. An APIC id an 8-bit destination cannot carry (or 0xFF, the broadcast) is refused by name in the ICR, the compatibility-format MSI and the I/O APIC entry; msi_message used to shift any id into the address unchecked. The two asm stubs that wrote x2APIC MSRs directly (the timer's re-arm and EOI, the TLB IPI's EOI) now call into apic.rs. VT-d's EIME follows the local APICs' mode as well as ECAP.EIM: an xAPIC compares the 8-bit destination field. Checked against the Intel SDM Vol. 3A (325384-093US) chapter 13 and the AMD APM Vol. 2 (24593 rev. 3.45) chapter 16, cited at the register sites. The new guest test desktop_without_x2apic boots tests/panelcase on Profile::MetalNoX2apic (TCG, the one CPU declaration with -x2apic) to the compositor holding the panel on four CPUs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- kernel/pure/sched/hw.rs | 2 +- kernel/src/arch/x86_64/apic.rs | 270 +++++++++++++++++-------- kernel/src/arch/x86_64/control_regs.rs | 109 +++++++++- kernel/src/arch/x86_64/idt/spurious.rs | 4 +- kernel/src/arch/x86_64/idt/timer.rs | 44 ++-- kernel/src/arch/x86_64/idt/tlb.rs | 5 +- kernel/src/arch/x86_64/ioapic.rs | 7 +- kernel/src/arch/x86_64/percpu.rs | 5 + kernel/src/arch/x86_64/vtd/mod.rs | 5 +- kernel/src/deadline.rs | 2 +- tests/common/qemu.rs | 25 ++- tests/toyos.rs | 50 +++++ 12 files changed, 384 insertions(+), 144 deletions(-) diff --git a/kernel/pure/sched/hw.rs b/kernel/pure/sched/hw.rs index defdd656803..49cf297da6f 100644 --- a/kernel/pure/sched/hw.rs +++ b/kernel/pure/sched/hw.rs @@ -4,7 +4,7 @@ //! task-blind surface (clock, one-shot timer, interrupt gate, halt, trace); //! [`Hw`] adds the two operations that carry a task — the context switch and //! the finalize sink. The kernel implements them with LAPIC one-shot, -//! targeted x2APIC ICR, TSC and the asm switch; the simulator over a virtual +//! targeted ICR, TSC and the asm switch; the simulator over a virtual //! clock and vcpu bookkeeping. No scheduling decision, state transition or //! ordering-sensitive code may live behind them. diff --git a/kernel/src/arch/x86_64/apic.rs b/kernel/src/arch/x86_64/apic.rs index 03079ebe015..a4faf1f3205 100644 --- a/kernel/src/arch/x86_64/apic.rs +++ b/kernel/src/arch/x86_64/apic.rs @@ -1,136 +1,216 @@ +//! The local APIC, in the mode `control_regs` declared for every CPU: x2APIC +//! through MSRs where CPUID offers it, xAPIC through its MMIO page where it +//! does not. Every register is reached through [`Reg`], which is the only +//! place the two modes differ but for the ICR's destination. +//! +//! Sections cited here are the Intel SDM's, Vol. 3A order 325384-093US, +//! chapter 13, and the AMD APM's, Vol. 2 publication 24593 rev. 3.45, +//! chapter 16. + use core::sync::atomic::{AtomicBool, AtomicU32, Ordering}; +use super::control_regs::{self, ApicMode}; use super::{cpu, percpu}; use crate::hw::MIN_ONE_SHOT; use crate::log; +use crate::mm::policy::MmioPolicy; +use crate::mm::{DirectMap, Mmio}; use crate::time::{Delay, Duration}; -/// The local APIC registers and MSRs this file may name. -// Every variant is an architectural local-APIC register touching no memory or control transfer, so none can make `Reg::write`'s unsafe wrmsr unsound. -// Addresses are x2APIC's: 0x800 plus the xAPIC MMIO offset shifted right four; ApicBase is the one exception, the MSR that turns x2APIC on. +/// A local APIC register by its xAPIC MMIO offset (SDM Table 13-1, APM Table +/// 16-2); its x2APIC MSR is `0x800 + offset / 16` (SDM §13.12.1.2, APM +/// §16.11.1). Only the constants below exist, so every register named is an +/// architectural one touching no memory or control transfer. #[derive(Clone, Copy)] -#[repr(u32)] -enum Reg { - ApicBase = 0x1B, - Id = 0x802, - Eoi = 0x80B, - Svr = 0x80F, - /// First of the eight in-service words (0x810..=0x817); see `in_service`. - Isr0 = 0x810, - Icr = 0x830, - LvtTimer = 0x832, - /// The performance-monitoring counters' LVT entry, xAPIC offset 0x340. - LvtPmc = 0x834, - TimerInit = 0x838, - TimerCurrent = 0x839, - TimerDivide = 0x83E, -} +struct Reg(u32); impl Reg { + const ID: Reg = Reg(0x20); + const EOI: Reg = Reg(0xB0); + const SVR: Reg = Reg(0xF0); + /// The ICR's command half; in x2APIC the whole 64-bit ICR (SDM §13.6.1, + /// §13.12.9; APM §16.5, §16.13). + const ICR: Reg = Reg(0x300); + /// The ICR's destination half, xAPIC only: x2APIC has no MSR 831H. + const ICR_HIGH: Reg = Reg(0x310); + const LVT_TIMER: Reg = Reg(0x320); + /// The performance-monitoring counters' LVT entry. + const LVT_PMC: Reg = Reg(0x340); + const TIMER_INIT: Reg = Reg(0x380); + const TIMER_CURRENT: Reg = Reg(0x390); + const TIMER_DIVIDE: Reg = Reg(0x3E0); + + /// The in-service word holding `vector`'s bit, one of eight at 100H–170H + /// (SDM §13.8.4 Figure 13-20, APM Table 16-2). + fn isr(vector: u8) -> Reg { + Reg(0x100 + 0x10 * (u32::from(vector) >> 5)) + } + #[inline] - fn read(self) -> u64 { - cpu::rdmsr(self as u32) + fn read(self) -> u32 { + match control_regs::apic_mode() { + ApicMode::X2apic => cpu::rdmsr(self.msr()) as u32, + ApicMode::Xapic => window().read_u32(u64::from(self.0)), + } } #[inline] - fn write(self, value: u64) { - // SAFETY: every value written here is a local-APIC word built in this module from architectural field encodings, so no reserved-bit #GP is possible. - unsafe { cpu::wrmsr(self as u32, value) }; + fn write(self, value: u32) { + match control_regs::apic_mode() { + // SAFETY: an architectural local-APIC register (the constants + // above), written a word this module built from its field + // encodings, so no reserved-bit `#GP` is possible. + ApicMode::X2apic => unsafe { cpu::wrmsr(self.msr(), u64::from(value)) }, + ApicMode::Xapic => window().write_u32(u64::from(self.0), value), + } } + + fn msr(self) -> u32 { + 0x800 + (self.0 >> 4) + } +} + +/// The xAPIC register page, a whole 4 KiB (SDM §13.4.1, APM §16.3.2), which +/// [`init`] maps uncacheable before anything reaches it: every access to it +/// is then serializing (SDM §13.12.3's note), so none owes a fence. +fn window() -> Mmio { + Mmio::new(DirectMap::from_phys(control_regs::APIC_REGISTERS), 0x1000) } pub const TIMER_VECTOR: u8 = 0x20; /// Where a device writes a message-signalled interrupt: the local APIC's -/// message window (SDM Vol. 3A §11.11.1). The one spelling of it — the +/// message window (SDM §13.11.1). The one spelling of it — the /// compatibility format below, VT-d's remappable format and VT-d's own fault /// event all start here. pub const MSI_DOORBELL: u32 = 0xFEE0_0000; +/// `apic_id` as the eight bits an xAPIC ICR, a compatibility-format MSI and +/// an I/O APIC entry carry it in (SDM §13.6.1, §13.11.1), or the refusal: +/// `0xFF` there is the broadcast, not a CPU. +pub fn narrow_destination(apic_id: u32) -> Result { + u8::try_from(apic_id) + .ok() + .filter(|id| *id != 0xFF) + .ok_or("the APIC id does not fit an 8-bit destination, where 0xFF is broadcast") +} + /// The compatibility-format message that raises `vector` on the CPU whose APIC /// ID is `dest`: the destination in address bits 19:12, the vector in the data. pub fn msi_message(dest: u32, vector: u8) -> Result<(u32, u32), &'static str> { - Ok((MSI_DOORBELL | (dest << 12), vector as u32)) + Ok((MSI_DOORBELL | u32::from(narrow_destination(dest)?) << 12, vector as u32)) } /// Calibrated LAPIC timer ticks per 10ms (computed on BSP, reused by APs), /// which is one quantum: what the Ring 0 timer branch re-arms with. -pub(crate) static TIMER_TICKS: AtomicU32 = AtomicU32::new(0); +static TIMER_TICKS: AtomicU32 = AtomicU32::new(0); const _: () = assert!(kernel::sched::fair::QUANTUM_NS == 10_000_000); -/// The spurious-interrupt vector register, whole (SDM Vol. 3A §11.9): the APIC -/// enabled, the vector `arch::idt::spurious` gates, and every other bit +/// The spurious-interrupt vector register, whole (SDM §13.9, APM §16.4.7): the +/// APIC enabled, the vector `arch::idt::spurious` gates, and every other bit /// clear — EOI-broadcast suppression among them, so an EOI for a level line /// reaches the I/O APIC and clears its Remote IRR. -const SVR: u64 = 1 << 8 | super::idt::spurious::SPURIOUS_VECTOR as u64; +const SVR: u32 = 1 << 8 | super::idt::spurious::SPURIOUS_VECTOR as u32; -/// Guards IPI sends before the APIC is enabled. -static X2APIC_ENABLED: AtomicBool = AtomicBool::new(false); +/// Guards IPI sends before the BSP's APIC is enabled. +static ENABLED: AtomicBool = AtomicBool::new(false); -fn enable_x2apic() { - let mut base = Reg::ApicBase.read(); - base |= (1 << 11) | (1 << 10); - Reg::ApicBase.write(base); - - Reg::Svr.write(SVR); - let held = Reg::Svr.read(); +/// This CPU's `IA32_APIC_BASE` as declared, then its SVR, read back. +fn enable(cpu_id: u32) -> ApicMode { + let mode = control_regs::init_apic(cpu_id); + // The BSP's alone: every AP reaches the same page through the kernel's tables. + if cpu_id == 0 && mode == ApicMode::Xapic { + crate::mm::paging::map_mmio(control_regs::APIC_REGISTERS, 0x1000, MmioPolicy::Uncacheable); + } + Reg::SVR.write(SVR); + let held = Reg::SVR.read(); assert!(held == SVR, "LAPIC: SVR reads {held:#x} after {SVR:#x} was written"); + mode } -/// Initialize the BSP's Local APIC in x2APIC mode. +/// Enable the BSP's local APIC. pub fn init() { - enable_x2apic(); - X2APIC_ENABLED.store(true, Ordering::Release); - log!("LAPIC: x2APIC enabled (ID {})", id()); + let mode = enable(0); + ENABLED.store(true, Ordering::Release); + log!("LAPIC: {mode:?} enabled (ID {})", id()); } -/// Enable the AP's local APIC in x2APIC mode. +/// Enable this AP's local APIC, in the mode the BSP's took. pub fn init_ap() { - enable_x2apic(); + enable(percpu::cpu_id()); } +/// This CPU's APIC id: all 32 bits in x2APIC, bits 31:24 in xAPIC (SDM +/// §13.4.6 Figure 13-6, APM §16.3.3, §16.12). pub fn id() -> u32 { - Reg::Id.read() as u32 + match control_regs::apic_mode() { + ApicMode::X2apic => Reg::ID.read(), + ApicMode::Xapic => Reg::ID.read() >> 24, + } } -/// Raise an interrupt `icr` names whole, after every store before it. An -/// x2APIC ICR write is not serializing (SDM Vol. 3A, "MSR Access in x2APIC -/// Mode"), so without the fence a target could take the interrupt before the -/// store it announces. -fn send(icr: u64) { - cpu::wrmsr_fence(); - Reg::Icr.write(icr); +/// Raise the interrupt `command` names — the ICR's low half — at the CPU +/// whose APIC id is `apic_id`, or at the destination its shorthand names +/// when `None`, after every store before it. +/// +/// x2APIC: one write, behind a fence, since that write is not serializing +/// (SDM §13.12.3, APM §16.11.2) and a target could otherwise take the +/// interrupt before the store it announces. xAPIC: the destination half and +/// then the command half, whose write sends (SDM §13.6.1, APM §16.5), with +/// interrupts closed between them so a handler's own send cannot retarget +/// this one — an NMI handler's targeted send could, and the only one is the +/// boot actuators' nested-NMI staging. Delivery Status is not polled: APM +/// §16.5 lets the ICR be written again without it, and SDM §13.6.1 asks no +/// wait. +fn send(apic_id: Option, command: u32) { + match control_regs::apic_mode() { + ApicMode::X2apic => { + let icr = u64::from(apic_id.unwrap_or(0)) << 32 | u64::from(command); + cpu::wrmsr_fence(); + // SAFETY: the ICR, written a command this module encodes and a + // 32-bit destination, which every x2APIC accepts. + unsafe { cpu::wrmsr(Reg::ICR.msr(), icr) }; + } + ApicMode::Xapic => { + let _closed = super::IrqGuard::close(); + if let Some(apic_id) = apic_id { + let dest = narrow_destination(apic_id) + .unwrap_or_else(|why| panic!("LAPIC: no IPI can reach APIC id {apic_id:#x}: {why}")); + Reg::ICR_HIGH.write(u32::from(dest) << 24); + } + Reg::ICR.write(command); + } + } } /// Send INIT IPI to the specified APIC ID. pub fn send_init(apic_id: u32) { - // ICR write: destination in the high 32 bits, 0x4500 = delivery INIT, level assert. - send(((apic_id as u64) << 32) | 0x4500); + // 0x4500 = delivery INIT, level assert. + send(Some(apic_id), 0x4500); } /// Send Startup IPI (SIPI) with the given vector (trampoline page number). pub fn send_sipi(apic_id: u32, vector: u8) { - send(((apic_id as u64) << 32) | 0x4600 | vector as u64); + send(Some(apic_id), 0x4600 | u32::from(vector)); } /// Send EOI. #[inline] pub fn eoi() { - Reg::Eoi.write(0); + Reg::EOI.write(0); } /// Whether `vector` is in service on this CPU. pub fn in_service(vector: u8) -> bool { - // ISR is eight 32-bit words (SDM Vol. 3A §12.8.4): word = vector >> 5, bit = vector & 31. - let word = cpu::rdmsr(Reg::Isr0 as u32 + (vector as u32 >> 5)); + let word = Reg::isr(vector).read(); (word >> (vector & 31)) & 1 != 0 } /// The highest vector in service — the one being handled, since the LAPIC only -/// delivers above the ISR top (SDM Vol. 3A §12.8.4). `None` outside a handler. +/// delivers above the ISR top (SDM §13.8.4). `None` outside a handler. pub fn in_service_highest() -> Option { for word_index in (0..8u32).rev() { - let word = cpu::rdmsr(Reg::Isr0 as u32 + word_index) as u32; + let word = Reg::isr((word_index * 32) as u8).read(); if word != 0 { return Some((word_index * 32 + (31 - word.leading_zeros())) as u8); } @@ -141,21 +221,21 @@ pub fn in_service_highest() -> Option { /// Send an IPI to this CPU (self shorthand). #[cfg(feature = "boot-actuators")] pub fn send_self(vector: u8) { - if !X2APIC_ENABLED.load(Ordering::Relaxed) { + if !ENABLED.load(Ordering::Relaxed) { return; } // Destination shorthand = self (0b01 << 18), fixed delivery, level assert. - send(0x0004_4000 | vector as u64); + send(None, 0x0004_4000 | u32::from(vector)); } fn ipi_all_excluding_self(vector: u8) { // destination shorthand = all-excluding-self (0b11 << 18), fixed delivery - send(0x000C_0000 | vector as u64); + send(None, 0x000C_0000 | u32::from(vector)); } /// Ask every other CPU to flush its TLB. pub(super) fn tlb_ipi() { - if X2APIC_ENABLED.load(Ordering::Relaxed) { + if ENABLED.load(Ordering::Relaxed) { ipi_all_excluding_self(0xFE); } } @@ -163,9 +243,9 @@ pub(super) fn tlb_ipi() { /// Send the kick IPI to one CPU, waking it if halted. // Targeted, not broadcast: a broadcast kick would preempt every sibling per wake and cannot scale. pub fn kick_cpu(cpu_id: u32) { - if !X2APIC_ENABLED.load(Ordering::Relaxed) { return; } + if !ENABLED.load(Ordering::Relaxed) { return; } let apic_id = crate::smp::hardware_id(cpu_id); - send(((apic_id as u64) << 32) | 0x4000 | u64::from(super::idt::KICK_VECTOR)); + send(Some(apic_id), 0x4000 | u32::from(super::idt::KICK_VECTOR)); } // Kicked, and not left to arrive on their own: a CPU halted in the idle path has stopped its own timer, so nothing else brings it to the next scheduler pass. @@ -182,44 +262,44 @@ pub fn kick_all_but_self() { // Test kernels only: an NMI can land inside any critical section, which this kernel cannot make NMI-safe. #[cfg(feature = "boot-actuators")] pub fn send_nmi(cpu_id: u32) { - if !X2APIC_ENABLED.load(Ordering::Relaxed) { return; } + if !ENABLED.load(Ordering::Relaxed) { return; } let apic_id = crate::smp::hardware_id(cpu_id); - send(((apic_id as u64) << 32) | 0x4400); + send(Some(apic_id), 0x4400); } /// Point this CPU's performance-counter LVT entry at NMI delivery, unmasked — /// the one interrupt a CPU that has cleared `IF` still takes, and so the only /// way `crate::hardlockup` can sample one. /// -/// **Written again after every delivery, not once at arm.** SDM Vol. 3A §12.5.1: -/// the local APIC sets this entry's mask flag when it handles a +/// **Written again after every delivery, not once at arm.** SDM §13.5.1, APM +/// §16.4.3: the local APIC sets this entry's mask flag when it handles a /// performance-monitoring interrupt, and only software clears it, so a handler /// that does not write this gets exactly one NMI for the machine's life. /// /// Declared whole: delivery mode 100b (NMI) in bits 10:8, mask clear, and a /// vector field the CPU ignores under that delivery mode. pub fn arm_perf_nmi() { - if !X2APIC_ENABLED.load(Ordering::Relaxed) { return; } - Reg::LvtPmc.write(0b100 << 8); + if !ENABLED.load(Ordering::Relaxed) { return; } + Reg::LVT_PMC.write(0b100 << 8); } /// Send every other CPU the halt IPI, where the machine has released any: before /// that no sibling has been sent its `SIPI`, so there are only CPUs still waiting /// for one rather than CPUs that need halting. pub fn stop_other_cpus() { - if X2APIC_ENABLED.load(Ordering::Relaxed) && crate::smp::is_ready() { - send(0x000C_0000 | 0xFD); + if ENABLED.load(Ordering::Relaxed) && crate::smp::is_ready() { + send(None, 0x000C_0000 | 0xFD); } } /// Calibrate the LAPIC timer on the BSP (requires HPET); does not start it. pub fn init_timer() { // Divide by 1 for maximum resolution. - Reg::TimerDivide.write(0b1011); + Reg::TIMER_DIVIDE.write(0b1011); // Masked one-shot mode for calibration. - Reg::LvtTimer.write(1 << 16); - Reg::TimerInit.write(0xFFFF_FFFF); + Reg::LVT_TIMER.write(1 << 16); + Reg::TIMER_INIT.write(0xFFFF_FFFF); const CALIBRATION: Delay = Delay::to_measure( Duration::from_millis(10), @@ -229,11 +309,11 @@ pub fn init_timer() { while crate::clock::nanos_since_boot() - start < CALIBRATION.nanos() {} let elapsed = crate::clock::nanos_since_boot() - start; - let remaining = Reg::TimerCurrent.read() as u32; + let remaining = Reg::TIMER_CURRENT.read(); let ticks_elapsed = 0xFFFF_FFFFu32.wrapping_sub(remaining); let ticks_10ms = (ticks_elapsed as u64 * 10_000_000 / elapsed) as u32; - Reg::TimerInit.write(0); + Reg::TIMER_INIT.write(0); TIMER_TICKS.store(ticks_10ms, Ordering::Release); // Fallback for any Ring 0 fire before the scheduler arms its first quantum. percpu::set_last_armed_ticks(OneShot::ticks(ticks_10ms as u64).0); @@ -245,7 +325,7 @@ pub fn init_timer() { log!("LAPIC timer: {} ticks/10ms, so {}Hz", ticks_10ms, ticks_10ms as u64 * 100); } -// The only path to Reg::TimerInit / last_armed_ticks — the floor is enforced once here, not at each of the three call sites. +// The floor is enforced once here, not at each of the three call sites. struct OneShot(u32); impl OneShot { @@ -262,11 +342,11 @@ impl OneShot { } fn arm(self) { - Reg::TimerDivide.write(0b1011); + Reg::TIMER_DIVIDE.write(0b1011); // LVT resets masked; an AP may reach here before this register was ever written. - Reg::LvtTimer.write(TIMER_VECTOR as u64); + Reg::LVT_TIMER.write(u32::from(TIMER_VECTOR)); percpu::set_last_armed_ticks(self.0); - Reg::TimerInit.write(self.0 as u64); + Reg::TIMER_INIT.write(self.0); } } @@ -281,7 +361,7 @@ pub fn arm_one_shot(nanos: u64) { pub fn arm_within(nanos: u64) { let want = OneShot::after(nanos); // Zero here means stop_timer, not an imminent expiry — a running count never reaches zero on its own. - let remaining = Reg::TimerCurrent.read() as u32; + let remaining = Reg::TIMER_CURRENT.read(); let ticks = if remaining == 0 { want.0 } else { want.0.min(remaining) }; OneShot::ticks(ticks as u64).arm(); } @@ -289,18 +369,32 @@ pub fn arm_within(nanos: u64) { /// What the timer was last armed with: `TIMER_INIT`, a count of ticks. #[cfg(feature = "test-actuators")] pub fn comparator() -> u64 { - Reg::TimerInit.read() + u64::from(Reg::TIMER_INIT.read()) } /// Whether a Ring 0 fire since [`comparator`] read `_armed` re-armed one quantum. #[cfg(feature = "test-actuators")] pub fn rearmed_a_quantum(_armed: u64) -> bool { - Reg::TimerInit.read() == u64::from(TIMER_TICKS.load(Ordering::Relaxed)) + Reg::TIMER_INIT.read() == TIMER_TICKS.load(Ordering::Relaxed) +} + +/// The Ring 3 fire's re-arm, before its Rust half runs: what this CPU last +/// armed, again, so the timer survives a handler that panics first. +pub(crate) extern "sysv64" fn rearm_last() { + Reg::TIMER_INIT.write(percpu::last_armed_ticks()); +} + +/// The Ring 0 fire's local-APIC work, called from its stub with interrupts +/// closed: the EOI, then one quantum on, or a stopped timer left stopped. +pub(crate) extern "sysv64" fn ring0_fire() { + eoi(); + let stopped = percpu::last_armed_ticks() == 0; + Reg::TIMER_INIT.write(if stopped { 0 } else { TIMER_TICKS.load(Ordering::Relaxed) }); } /// Stop the timer. No more interrupts until re-armed. pub fn stop_timer() { percpu::set_last_armed_ticks(0); - Reg::TimerInit.write(0); + Reg::TIMER_INIT.write(0); crate::trace::trace(crate::trace::Kind::TimerStop, 0); } diff --git a/kernel/src/arch/x86_64/control_regs.rs b/kernel/src/arch/x86_64/control_regs.rs index 81267b30008..0258a62931d 100644 --- a/kernel/src/arch/x86_64/control_regs.rs +++ b/kernel/src/arch/x86_64/control_regs.rs @@ -1,9 +1,12 @@ -//! What `CR0`, `CR4`, `IA32_EFER` and the performance request hold on every -//! CPU in this machine. One declaration, applied by the BSP and every AP and -//! checked on each; nothing else may write any of them. Each register is -//! written whole: `CR0` and `EFER` are constants, `CR4` is required bits plus -//! whatever optional bits this CPU offers. `EFER.NXE` lets bit 63 of a paging -//! entry mean *not executable* ([`Prot`](crate::mm::policy::Prot)). +//! What `CR0`, `CR4`, `IA32_EFER`, `IA32_APIC_BASE` and the performance +//! request hold on every CPU in this machine. One declaration, applied by the +//! BSP and every AP and checked on each; nothing else may write any of them. +//! Each register is written whole: `CR0` and `EFER` are constants, `CR4` is +//! required bits plus whatever optional bits this CPU offers, and +//! `IA32_APIC_BASE` is the local APIC at its architectural address in the one +//! mode CPUID offers — x2APIC where it exists, xAPIC where it does not. +//! `EFER.NXE` lets bit 63 of a paging entry mean *not executable* +//! ([`Prot`](crate::mm::policy::Prot)). //! //! The performance request is HWP's: `IA32_HWP_INTERRUPT` where it exists, //! `IA32_PM_ENABLE`, `IA32_HWP_REQUEST` as `toyos_cpuvuln::hwp_request` derives @@ -58,6 +61,38 @@ mod cr4 { pub const SMAP: u64 = 1 << 21; } +/// `IA32_APIC_BASE`: SDM Vol. 3A (325384-093US) §13.4.4 Figure 13-5 and +/// §13.12.1 Figure 13-26; AMD APM Vol. 2 (24593 rev. 3.45) §16.3.1 Figure +/// 16-2 and §16.9 Figure 16-31. +mod apic_base { + pub const MSR: u32 = 0x1B; + /// The CPU's own: set on the one reset chose to boot. + pub const BSP: u64 = 1 << 8; + /// x2APIC mode, which only a CPU whose `CPUID.01H:ECX[21]` is set accepts. + pub const EXTD: u64 = 1 << 10; + pub const EN: u64 = 1 << 11; + /// Bits 12 up to `MAXPHYADDR`: where the xAPIC's registers decode. + pub const ADDRESS_MASK: u64 = 0x000F_FFFF_FFFF_F000; +} + +/// Where every CPU's xAPIC registers decode: the address reset leaves them at +/// (SDM §13.4.4, APM §16.3.1). A machine whose firmware moved them is +/// refused, not followed. +pub const APIC_REGISTERS: u64 = 0xFEE0_0000; + +/// How every local APIC in this machine is driven. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum ApicMode { + /// Registers in a 4 KiB MMIO page at [`APIC_REGISTERS`], an 8-bit + /// destination in every message. + Xapic, + /// Registers as MSRs, a 32-bit destination. + X2apic, +} + +/// The mode the BSP declared, as [`ApicMode`] plus one; zero until then. +static DECLARED_APIC: AtomicU8 = AtomicU8::new(0); + /// The performance request's registers, SDM Vol. 4. mod hwp { pub const PLATFORM_INFO: u32 = 0xCE; @@ -244,6 +279,65 @@ pub fn envelope() -> Option { }) } +/// Puts this CPU's `IA32_APIC_BASE` into the declaration — the local APIC +/// enabled, in [`ApicMode::X2apic`] where `CPUID.01H:ECX[21]` offers it and +/// [`ApicMode::Xapic`] where it does not — checks it, and answers the mode. +/// A CPU that reaches a different mode than the BSP declared is named. +/// +/// One write reaches either mode from the enabled xAPIC firmware hands over, +/// and x2APIC from x2APIC (SDM §13.12.5 Figure 13-27, APM §16.9 Figure +/// 16-32); `INIT` changes neither (APM §16.10). The one illegal step, x2APIC +/// to xAPIC, needs a CPU in x2APIC whose CPUID denies it. +pub fn init_apic(cpu_id: u32) -> ApicMode { + let mine = if cpu::cpuid(1, 0).2 & (1 << 21) != 0 { ApicMode::X2apic } else { ApicMode::Xapic }; + let code = mine as u8 + 1; + if let Err(machine) = DECLARED_APIC.compare_exchange(0, code, Ordering::Release, Ordering::Acquire) { + assert!( + machine == code, + "control_regs: cpu{cpu_id} offers the local APIC in {mine:?} and the BSP declared {:?}", + apic_mode(), + ); + } + let live = cpu::rdmsr(apic_base::MSR); + assert!( + live & apic_base::ADDRESS_MASK == APIC_REGISTERS, + "control_regs: cpu{cpu_id} holds apic_base={live:#x}: firmware moved its local APIC's \ + registers from {APIC_REGISTERS:#x}", + ); + let declared = APIC_REGISTERS + | apic_base::EN + | if mine == ApicMode::X2apic { apic_base::EXTD } else { 0 } + | live & apic_base::BSP; + // SAFETY: the registers stay where they decode and `EXTD` is set only + // where CPUID offers x2APIC; an illegal transition is a `#GP`, never a + // state this kernel did not declare. + unsafe { cpu::wrmsr(apic_base::MSR, declared) }; + let held = cpu::rdmsr(apic_base::MSR); + log!("control_regs: cpu{cpu_id} apic_base={held:#x} {mine:?}"); + assert!( + held == declared, + "control_regs: cpu{cpu_id} holds apic_base={held:#x}, the declaration is {declared:#x}", + ); + mine +} + +/// The mode every local APIC was declared in. Panics before the BSP's +/// [`init_apic`]: no register has an address until then. +#[inline] +pub fn apic_mode() -> ApicMode { + match DECLARED_APIC.load(Ordering::Relaxed) { + 1 => ApicMode::Xapic, + 2 => ApicMode::X2apic, + _ => undeclared_apic(), + } +} + +#[cold] +#[inline(never)] +fn undeclared_apic() -> ! { + panic!("control_regs: a local APIC register was reached before the BSP declared its mode") +} + /// Whether the declaration carries `PCIDE`, and therefore whether `INVPCID` is this machine's flush. pub fn pcid_active() -> bool { DECLARED_CR4.load(Ordering::Acquire) & cr4::PCIDE != 0 @@ -431,11 +525,12 @@ fn hwp_check(cpu_id: u32, request: u64, notify: bool) { /// the CPUs it got rather than to crash on that one. pub fn report(cpus: u32) { log!( - "control_regs: {} of {cpus} cpus hold cr0={:#010x} cr4={:#010x} efer={:#06x}", + "control_regs: {} of {cpus} cpus hold cr0={:#010x} cr4={:#010x} efer={:#06x} apic={:?}", CHECKED.load(Ordering::Relaxed), CR0, DECLARED_CR4.load(Ordering::Acquire), EFER, + apic_mode(), ); } diff --git a/kernel/src/arch/x86_64/idt/spurious.rs b/kernel/src/arch/x86_64/idt/spurious.rs index 87033cd496d..06190a061ae 100644 --- a/kernel/src/arch/x86_64/idt/spurious.rs +++ b/kernel/src/arch/x86_64/idt/spurious.rs @@ -1,4 +1,4 @@ -//! Vector 0xFF, the spurious vector `apic::enable_x2apic` writes into the SVR +//! Vector 0xFF, the spurious vector `apic::enable` writes into the SVR //! on every CPU. //! //! The EOI is conditional: sent only when the ISR bit is set (SDM Vol. 3A @@ -10,7 +10,7 @@ use core::arch::naked_asm; use crate::arch::apic; -/// The vector `apic::enable_x2apic` writes into the SVR; pub because the IDT +/// The vector `apic::enable` writes into the SVR; pub because the IDT /// gate row hardcodes `0xFF` on its own and must be kept in sync by hand. pub const SPURIOUS_VECTOR: u8 = 0xFF; diff --git a/kernel/src/arch/x86_64/idt/timer.rs b/kernel/src/arch/x86_64/idt/timer.rs index 312af4ed4ad..8424a0dad22 100644 --- a/kernel/src/arch/x86_64/idt/timer.rs +++ b/kernel/src/arch/x86_64/idt/timer.rs @@ -20,11 +20,8 @@ pub(super) extern "sysv64" fn timer_entry() { save_user_state!(), - // Re-arm before Rust runs so the timer survives even if the handler path panics before scheduler::do_preempt → arm_one_shot. - "mov ecx, 0x838", - "mov eax, dword ptr gs:[{armed_ticks}]", - "xor edx, edx", - "wrmsr", + // Re-arm before the handler runs so the timer survives even if the handler path panics before scheduler::do_preempt → arm_one_shot. + "call {rearm_last}", "call {handler}", @@ -41,36 +38,23 @@ pub(super) extern "sysv64" fn timer_entry() { "iretq", "2:", - // No Rust half on this branch, so this `add` inlines what `timer_handler` does for Ring 3; flags are dead after the `test` above, so none are saved. + // This `add` inlines what `timer_handler` does for Ring 3; flags are dead after the `test` above, so none are saved. "add qword ptr gs:[{irq_timer}], 1", - "push rax", - "push rcx", - "push rdx", - "mov ecx, 0x80B", // X2APIC_EOI - "xor eax, eax", - "xor edx, edx", - "wrmsr", - "mov ecx, 0x838", // X2APIC_TIMER_INIT - "mov eax, dword ptr gs:[{armed_ticks}]", - "test eax, eax", - "jz 3f", // 0: stopped, and written back as stopped. - "mov eax, dword ptr [rip + {quantum_ticks}]", - "3:", - "xor edx, edx", - "wrmsr", + // The caller-saved set, then the stack realigned for a `sysv64` call — + // no target touches an `xmm`, since the kernel's own target has no + // hardware float (`src/build.rs`'s check). + "push rax", "push rcx", "push rdx", + "push rsi", "push rdi", "push r8", "push r9", "push r10", "push r11", + "push rbp", + "mov rbp, rsp", + "and rsp, -16", + "call {ring0_fire}", "mov byte ptr gs:[{need_resched}], 1", // No lock on the fire count: single writer, IF=0. "inc dword ptr gs:[{ring0_fires}]", // The boot deadline, from the one branch that runs while a lock is // held: a CPU spinning on a ticket still takes this interrupt, and that // is the whole reason the poll is here and not only in the Rust half. - // The rest of the caller-saved set, then the stack realigned for a - // `sysv64` call — the target touches no `xmm`, since the kernel's own - // target has no hardware float (`src/build.rs`'s check). - "push rsi", "push rdi", "push r8", "push r9", "push r10", "push r11", - "push rbp", - "mov rbp, rsp", - "and rsp, -16", // The interrupted `rip`, above the ten registers pushed on this branch. "mov rdi, [rbp + 80]", "call {deadline}", @@ -87,8 +71,8 @@ pub(super) extern "sysv64" fn timer_entry() { deadline = sym ring0_tick, handler = sym timer_handler, exit_to_user = sym crate::arch::idt::kernel_exit_to_user_check, - armed_ticks = const crate::arch::percpu::OFF_LAST_ARMED_TICKS, - quantum_ticks = sym crate::arch::apic::TIMER_TICKS, + rearm_last = sym crate::arch::apic::rearm_last, + ring0_fire = sym crate::arch::apic::ring0_fire, need_resched = const crate::arch::percpu::OFF_NEED_RESCHED, ring0_fires = const crate::arch::percpu::OFF_RING0_TIMER_FIRES, irq_timer = const crate::arch::percpu::irq_slot_offset(crate::irq_census::Source::Timer as usize), diff --git a/kernel/src/arch/x86_64/idt/tlb.rs b/kernel/src/arch/x86_64/idt/tlb.rs index 17551416d73..00a4544c4ba 100644 --- a/kernel/src/arch/x86_64/idt/tlb.rs +++ b/kernel/src/arch/x86_64/idt/tlb.rs @@ -22,10 +22,6 @@ pub(super) extern "sysv64" fn tlb_flush_entry() { "call {windows_entered}", "call {flush}", "mov rsp, rbp", - "mov ecx, 0x80B", - "xor eax, eax", - "xor edx, edx", - "wrmsr", #[cfg(feature = "mask-windows")] "mov rdi, [rsp + 88]", #[cfg(feature = "mask-windows")] @@ -67,4 +63,5 @@ pub(super) extern "sysv64" fn tlb_flush_entry() { fn flush() { crate::arch::percpu::irq_took!(Tlb); crate::arch::tlb::serve_ipi(); + crate::arch::apic::eoi(); } diff --git a/kernel/src/arch/x86_64/ioapic.rs b/kernel/src/arch/x86_64/ioapic.rs index a46b441292e..02ee136e689 100644 --- a/kernel/src/arch/x86_64/ioapic.rs +++ b/kernel/src/arch/x86_64/ioapic.rs @@ -258,10 +258,9 @@ pub fn route( let (index, high) = match crate::iommu::remap_pin(unit.id, vector, dest_apic_id, level) { Delivery::Direct => { - if dest_apic_id >= 0xFF { - return Err(RouteError::DestTooWide(dest_apic_id)); - } - (0, dest_apic_id << 24) + let dest = super::apic::narrow_destination(dest_apic_id) + .map_err(|_| RouteError::DestTooWide(dest_apic_id))?; + (0, u32::from(dest) << 24) } Delivery::Remapped(pin) => (pin.low, pin.high), Delivery::Refused(why) => return Err(RouteError::NotRemappable(gsi, why)), diff --git a/kernel/src/arch/x86_64/percpu.rs b/kernel/src/arch/x86_64/percpu.rs index 747e719d3b9..dc4fbaf84c3 100644 --- a/kernel/src/arch/x86_64/percpu.rs +++ b/kernel/src/arch/x86_64/percpu.rs @@ -642,6 +642,11 @@ pub fn set_last_armed_ticks(ticks: u32) { gs::write_u32::(ticks); } +/// The one-shot count this CPU last armed; zero is a stopped timer. +pub fn last_armed_ticks() -> u32 { + gs::read_u32::() +} + /// The last byte of this CPU's idle guard page — the first byte an overflow reaches. #[cfg(feature = "test-actuators")] pub fn idle_guard_byte() -> u64 { diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index df574044349..b9ffa4247b2 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -304,7 +304,10 @@ fn remappable(ready: &[(Unit, Plan)], described: usize, flags: u8) -> Option bool { AT_TSC.load(Relaxed) != 0 diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 448a000089d..54362f1d495 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -708,6 +708,10 @@ pub enum Profile { /// ===TEST_START=== protocol like any other. [`BootOptions::mute`] takes /// it away for the one test that certifies the T14's literal shape. Metal, + /// [`Profile::Metal`] on a CPU whose CPUID offers no x2APIC, emulated + /// whatever the host, so its local APICs run in xAPIC mode: the machine + /// an AMD laptop without x2APIC is. + MetalNoX2apic, /// QEMU `virt` on AArch64 (GICv3, AAVMF): a GOP from `ramfb`, the boot /// stick on an xHCI, the PL011, a virtio-rng for firmware's /// `EFI_RNG_PROTOCOL`, and nothing else — no NIC, NVMe or IOMMU. The @@ -750,24 +754,32 @@ impl Profile { | Self::HeadlessE1000e | Self::HeadlessNoUsb | Self::HeadlessUsbSpare - | Self::Metal => Arch::X86_64, + | Self::Metal + | Self::MetalNoX2apic => Arch::X86_64, } } /// How this host provides the machine. pub fn accel(self) -> Accel { match self { - Self::VirtEl2 | Self::VirtEl2NoVhe | Self::VirtTcg => Accel::Tcg, + Self::VirtEl2 | Self::VirtEl2NoVhe | Self::VirtTcg | Self::MetalNoX2apic => Accel::Tcg, _ => self.arch().accel(), } } /// The CPU this machine has. - fn cpu(self) -> &'static str { + fn cpu(self) -> String { match self { - Self::VirtEl2NoVhe => "cortex-a72", - Self::VirtNoRng if !self.accel().is_hardware() => "cortex-a72", - _ => self.arch().cpu(self.accel()), + Self::VirtEl2NoVhe => "cortex-a72".to_string(), + Self::VirtNoRng if !self.accel().is_hardware() => "cortex-a72".to_string(), + // The one declaration's CPU with its x2APIC taken away, refused by + // name the day that declaration stops offering one. + Self::MetalNoX2apic => { + let offered = self.arch().cpu(self.accel()); + assert!(offered.contains(",+x2apic"), "{offered:?} offers no x2APIC to take away"); + offered.replace(",+x2apic", ",-x2apic") + } + _ => self.arch().cpu(self.accel()).to_string(), } } } @@ -985,6 +997,7 @@ impl Profile { smmu: Smmu::Absent, rng: false, }, + Self::MetalNoX2apic => Self::Metal.shape(), Self::HeadlessNoIommu => Shape { iommu: None, ..Self::Headless.shape() }, Self::HeadlessE1000e => Shape { nic: Nic::E1000e, ..Self::Headless.shape() }, Self::HeadlessNoUsb => Shape { xhci: &[], usb: &[], ..Self::Headless.shape() }, diff --git a/tests/toyos.rs b/tests/toyos.rs index 5dc34455148..aee9a6abcea 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -378,6 +378,11 @@ const MACHINE_TESTS: &[&str] = &[ // `console/system.toml`'s image, which runs no job and hands no machine // back: a metal boot of it ends with a hand on the power button. "console_image_boots", + // The desktop on CPUs that offer no x2APIC, whose local APICs run in + // xAPIC mode: every IPI, EOI and timer write goes through the MMIO page, + // which no host build has and the T14, whose CPUs offer x2APIC, never + // reaches; the AMD laptop that needs it is not on the bench. + "desktop_without_x2apic", // A machine whose one NVMe disk is all it has: its firmware boots the // image off it, and `/log` and `/home` are `diskserver`'s partitions of // it across a reset. The claim on the controller and the firmware that @@ -1975,6 +1980,50 @@ fn console_image_boots() -> Result<(), String> { serial::Serial::named("the console image's boot", console).must_be_clean() } +/// Boot `tests/panelcase` on [`qemu::Profile::MetalNoX2apic`]'s CPUs, which +/// offer no x2APIC, and wait for its compositor to hold the panel: every CPU +/// declared its local APIC in xAPIC mode, every CPU the firmware named came +/// up — started by an xAPIC ICR's two halves — and the desktop ran across +/// them, which every timer re-arm, EOI and TLB shootdown on the way reached +/// through the xAPIC's MMIO page. +fn desktop_without_x2apic() -> Result<(), String> { + const CPUS: u32 = 4; + let mut qemu = QemuInstance::boot_with_options( + &compile::repo_root().join("tests/panelcase"), + &[], + &[], + BootOptions { + profile: qemu::Profile::MetalNoX2apic, + smp: CPUS, + qmp: true, + ready_marker: bootlog::COMPLETE, + ..Default::default() + }, + ); + let mut console = format!("{}\n", qemu.boot_log()); + await_marker(&mut qemu, &mut console, "compositor: ready", "the compositor to take its panel")?; + let taken = qemu.screendump_while(Duration::from_secs(30), Duration::from_millis(200), |d| d.fill() != FILL_BOOT); + if taken.fill() == FILL_BOOT { + return Err(format!("the compositor said it is ready and never took the screen\n{console}")); + } + let said = serial::Serial::named("the boot without x2APIC", console); + said.must_be_clean()?; + said.must_say("LAPIC: Xapic enabled (ID 0)")?; + for cpu in 0..CPUS { + let line = said.must_say(&format!("control_regs: cpu{cpu} apic_base="))?; + if !line.trim_end().ends_with(" Xapic") { + return Err(format!("cpu{cpu} did not declare its local APIC in xAPIC mode: {line:?}")); + } + } + let held = said.must_say(&format!("control_regs: {CPUS} of {CPUS} cpus hold "))?; + if !held.trim_end().ends_with(" apic=Xapic") { + return Err(format!("the machine's declaration is not xAPIC: {held:?}")); + } + smp_roster_and_tsc_trail(said.text(), CPUS)?; + eprintln!(" [xapic] {CPUS} CPUs in xAPIC mode, and the compositor holds the panel"); + Ok(()) +} + /// A `mask-windows` boot's windows: `common::irqcensus::windows`'s verdict, /// with `cpus` CPUs reporting. fn mask_windows(capture: &str, cpus: u32) -> Result<(), String> { @@ -3697,6 +3746,7 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "machine_shutdown_wire_at_the_seal" => power::machine_shutdown_wire_at_the_seal(test_config), "bar_map_again" => bar_map_again(test_config), "console_image_boots" => console_image_boots(), + "desktop_without_x2apic" => desktop_without_x2apic(), "nvme_disk_keeps_log_and_home" => nvme_disk_keeps_log_and_home(test_config), "block_grants_reach_their_partitions" => block_grants_reach_their_partitions(), "update_writes_the_idle_slot_through_the_block_service" => update_writes_the_idle_slot_through_the_block_service(), From 6c8192e5b237a077a419f81cbc531f0ba8b5a6e9 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 14:53:12 +0200 Subject: [PATCH 2/4] The T14's smp_roster_and_tsc_trail row holds every CPU to x2APIC A kernel that declared xAPIC on a CPU that offers x2APIC would still boot every guest of the suite, since each runs either mode; the T14, whose CPUs offer x2APIC, is where that choice is visible. The row's judge and desktop_without_x2apic now read the same helper, apic_mode_held: every CPU's apic_base line, the machine's report and the BSP's LAPIC line name the mode. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- tests/toyos.rs | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/tests/toyos.rs b/tests/toyos.rs index aee9a6abcea..424ed23eec2 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -665,7 +665,12 @@ const METAL: &[(&str, metal::Metal)] = &[ "smp_roster_and_tsc_trail", metal::Metal { arms: TESTCASES, - judge: |b| smp_roster_and_tsc_trail(b[0].kernel().text(), b[0].cpus()?), + // The T14's CPUs offer x2APIC, so the declaration keeps it there. + judge: |b| { + let (log, cpus) = (b[0].kernel(), b[0].cpus()?); + smp_roster_and_tsc_trail(log.text(), cpus)?; + apic_mode_held(&log, cpus, "X2apic") + }, }, ), ( @@ -2008,19 +2013,26 @@ fn desktop_without_x2apic() -> Result<(), String> { } let said = serial::Serial::named("the boot without x2APIC", console); said.must_be_clean()?; - said.must_say("LAPIC: Xapic enabled (ID 0)")?; - for cpu in 0..CPUS { - let line = said.must_say(&format!("control_regs: cpu{cpu} apic_base="))?; - if !line.trim_end().ends_with(" Xapic") { - return Err(format!("cpu{cpu} did not declare its local APIC in xAPIC mode: {line:?}")); + apic_mode_held(&said, CPUS, "Xapic")?; + smp_roster_and_tsc_trail(said.text(), CPUS)?; + eprintln!(" [xapic] {CPUS} CPUs in xAPIC mode, and the compositor holds the panel"); + Ok(()) +} + +/// Every one of `cpus` CPUs declared its local APIC in `mode`, as +/// `control_regs` names it, and the machine's report says the same. +fn apic_mode_held(log: &serial::Serial, cpus: u32, mode: &str) -> Result<(), String> { + for cpu in 0..cpus { + let line = log.must_say(&format!("control_regs: cpu{cpu} apic_base="))?; + if !line.trim_end().ends_with(&format!(" {mode}")) { + return Err(format!("cpu{cpu} did not declare its local APIC {mode}: {line:?}")); } } - let held = said.must_say(&format!("control_regs: {CPUS} of {CPUS} cpus hold "))?; - if !held.trim_end().ends_with(" apic=Xapic") { - return Err(format!("the machine's declaration is not xAPIC: {held:?}")); + let held = log.must_say(&format!("control_regs: {cpus} of {cpus} cpus hold "))?; + if !held.trim_end().ends_with(&format!(" apic={mode}")) { + return Err(format!("the machine's declaration is not {mode}: {held:?}")); } - smp_roster_and_tsc_trail(said.text(), CPUS)?; - eprintln!(" [xapic] {CPUS} CPUs in xAPIC mode, and the compositor holds the panel"); + log.must_say(&format!("LAPIC: {mode} enabled ("))?; Ok(()) } From e1d7370a922afdbbb4c1a2c9fc3a1396a8557d69 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 15:31:33 +0200 Subject: [PATCH 3/4] The BSP maps its xAPIC page after the IDT, and VT-d's narrow destination is apic's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `apic::init` ran before `percpu::init_bsp` loaded the IDT, and under xAPIC it maps the register page: a heap push, perhaps a page-table page, a TLB flush and a log line, any of which faulting triple-faults with nothing on any channel — the failure this branch exists to remove. The BSP's per-CPU block now takes its id from `cpu::hardware_id()`, as every AP's does, and the APIC is declared after it. VT-d's remapping entry carried its own copy of the eight-bit bound (`NARROW_DESTINATIONS = 0xFF`). Its refusal and its `MSG_DEST` const assert now ask `apic::narrow_destination`, now a `const fn`. The x2APIC opt-out issue is made true of the tree: the mode follows CPUID in `control_regs::init_apic`, and the untested `x2apic &&` gate on `EIME` in `vtd::remappable` is recorded there, its exit reaching it on the T14. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...abled-without-reading-firmwares-opt-out.md | 19 ++++++++++++++----- kernel/src/arch/x86_64/apic.rs | 12 +++++++----- kernel/src/arch/x86_64/boot.rs | 4 +++- kernel/src/arch/x86_64/vtd/interrupt.rs | 8 ++++---- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/issues/x2apic-is-enabled-without-reading-firmwares-opt-out.md b/issues/x2apic-is-enabled-without-reading-firmwares-opt-out.md index 711348e329c..de121758fd7 100644 --- a/issues/x2apic-is-enabled-without-reading-firmwares-opt-out.md +++ b/issues/x2apic-is-enabled-without-reading-firmwares-opt-out.md @@ -8,8 +8,9 @@ opened: 2026-10-04 VT-d Rev. 4.1 §8.1, DMAR Flags bit 1 `X2APIC_OPT_OUT`: firmware asks system software "to opt out of enabling Extended xAPIC (X2APIC) mode", and software -checks it "as part of detecting X2APIC mode support". This kernel enables -x2APIC unconditionally in `arch::apic::init` (`enable_x2apic`), reached from +checks it "as part of detecting X2APIC mode support". This kernel puts every +local APIC in x2APIC wherever `CPUID.01H:ECX[21]` offers it, in +`arch::control_regs::init_apic`, reached from `arch::apic::init` in `arch::boot::interrupts`, before the DMAR is first read in `iommu::init` (`kernel/src/main.rs`). The flag is logged (`x2apic_opt_out=` on the `iommu: DMAR` line) and decides nothing. @@ -19,6 +20,14 @@ The owner's ruling on extended interrupt mode remapping entry's destination, not whether x2APIC is on, so it does not cover this. The T14 clears the flag (`flags=0x05`), so no machine in reach sets it. -Owner: the IOMMU track. Exit: the flag is read before the local APIC is put in -x2APIC mode, and a machine that sets it either runs xAPIC or is refused by -name; an actuator standing in for the flag reaches both arms on the T14. +The xAPIC arm exists, for CPUs without x2APIC, but no tier runs it beside +VT-d: QEMU's `intel-iommu` takes `eim=on` only with KVM's split irqchip, the +harness's default leaves `eim` off, the T14 runs x2APIC and the AMD laptop has +no VT-d. So `vtd::remappable`'s `x2apic &&`, which keeps `EIME` off under an +xAPIC, passes every test with it deleted. + +Owner: the IOMMU track. Exit: the flag is read before `init_apic` declares +the mode, and a machine that sets it runs xAPIC, or is refused by name where +an APIC id does not fit eight bits; an actuator standing in for the flag +reaches both arms on the T14, its xAPIC arm with `EIME` off on a unit that +reports `ECAP.EIM`. diff --git a/kernel/src/arch/x86_64/apic.rs b/kernel/src/arch/x86_64/apic.rs index a4faf1f3205..7ec30fbf1cf 100644 --- a/kernel/src/arch/x86_64/apic.rs +++ b/kernel/src/arch/x86_64/apic.rs @@ -88,11 +88,13 @@ pub const MSI_DOORBELL: u32 = 0xFEE0_0000; /// `apic_id` as the eight bits an xAPIC ICR, a compatibility-format MSI and /// an I/O APIC entry carry it in (SDM §13.6.1, §13.11.1), or the refusal: /// `0xFF` there is the broadcast, not a CPU. -pub fn narrow_destination(apic_id: u32) -> Result { - u8::try_from(apic_id) - .ok() - .filter(|id| *id != 0xFF) - .ok_or("the APIC id does not fit an 8-bit destination, where 0xFF is broadcast") +/// VT-d's remapping entry without `EIME` holds the same eight bits. +pub const fn narrow_destination(apic_id: u32) -> Result { + if apic_id < 0xFF { + Ok(apic_id as u8) + } else { + Err("the APIC id does not fit an 8-bit destination, where 0xFF is broadcast") + } } /// The compatibility-format message that raises `vector` on the CPU whose APIC diff --git a/kernel/src/arch/x86_64/boot.rs b/kernel/src/arch/x86_64/boot.rs index 0fefc11be38..64171279d5e 100644 --- a/kernel/src/arch/x86_64/boot.rs +++ b/kernel/src/arch/x86_64/boot.rs @@ -88,8 +88,10 @@ pub fn interrupts(rsdp_addr: u64) -> Platform { // reportable: a panic that can be reported but not ended leaves the machine // holding its panel for a hand that may not be in the room. super::power::init_reset(rsdp_addr); + percpu::init_bsp(super::cpu::hardware_id()); + // After the IDT: an xAPIC's register page is mapped here, and a fault + // there must diagnose rather than triple-fault. apic::init(); - percpu::init_bsp(apic::id()); super::power::init_control(rsdp_addr); ioapic::init(&madt); idt::enable_interrupts(); diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index a4d8b254607..abb9bf5dc43 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -35,6 +35,7 @@ use crate::log; use alloc::vec::Vec; +use crate::arch::apic::narrow_destination; use crate::drivers::pci::MSG_DEST; use crate::iommu::{Refused, StreamId}; use crate::sync::Lock; @@ -60,9 +61,8 @@ const VECTOR_SHIFT: u64 = 16; const DESTINATION_SHIFT: u64 = 32; /// `SVT=01b` over `SQ=00b`: a message reaching this entry carries `SID` in all sixteen bits or is refused. const VERIFY_SOURCE_ID: u64 = 1 << 18; -/// Without `EIME`, `DST` 47:40 holds eight bits and `0xFF` there is broadcast, not a CPU. +/// Without `EIME`, `DST` 47:40 holds what [`narrow_destination`] makes of an id. const NARROW_DESTINATION_SHIFT: u64 = 8; -const NARROW_DESTINATIONS: u32 = 0xFF; const MESSAGE_REMAPPABLE: u32 = 1 << 4; const MESSAGE_SUBHANDLE_VALID: u32 = 1 << 3; @@ -98,7 +98,7 @@ const _: () = assert!(crate::pcidev::MAX_FUNCTIONS < ENTRIES as usize); /// A claimed function's message reaches [`MSG_DEST`], which an entry holds with /// or without `EIME`, so writing a claim slot's entry is never refused. -const _: () = assert!(MSG_DEST < NARROW_DESTINATIONS); +const _: () = assert!(narrow_destination(MSG_DEST).is_ok()); pub fn describe_apic(apic_id: u8, source: StreamId) { REMAP.lock().apics.push((apic_id, source)); @@ -169,7 +169,7 @@ fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result= NARROW_DESTINATIONS { + if !remap.extended && narrow_destination(dest).is_err() { Err(Refused::DestinationTooWide(dest)) } else if remap.used == ENTRIES { Err(Refused::TableFull) From 91a8049dc8e92c2da626512d2c06e13cfae9840b Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 19:53:09 +0200 Subject: [PATCH 4/4] A remapping entry without EIME holds only the u8 narrow_destination produced entry's narrow arm took the raw u32 and shifted it, so allocate checked the 8-bit bound in one place and used the unchecked id in another, and claim checked nothing, resting on MSG_DEST_NARROW being evaluated only because msi_message names it. entry now takes a Destination whose narrow arm is a u8: allocate passes narrow_destination's Ok value, claim passes MSG_DEST_NARROW. The Delivery Status issue records a measurement owed, which issues/README.md calls tooling, not defect. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...ivery-status-and-no-machine-has-read-it.md | 2 +- kernel/src/arch/x86_64/apic.rs | 2 +- kernel/src/arch/x86_64/vtd/interrupt.rs | 58 +++++++++++++------ 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/issues/the-xapic-icr-send-polls-no-delivery-status-and-no-machine-has-read-it.md b/issues/the-xapic-icr-send-polls-no-delivery-status-and-no-machine-has-read-it.md index e45747437b3..14393c0d4b3 100644 --- a/issues/the-xapic-icr-send-polls-no-delivery-status-and-no-machine-has-read-it.md +++ b/issues/the-xapic-icr-send-polls-no-delivery-status-and-no-machine-has-read-it.md @@ -1,6 +1,6 @@ --- status: open -kind: defect +kind: tooling opened: 2026-10-10 --- diff --git a/kernel/src/arch/x86_64/apic.rs b/kernel/src/arch/x86_64/apic.rs index d380dc9998f..3e1a8389e09 100644 --- a/kernel/src/arch/x86_64/apic.rs +++ b/kernel/src/arch/x86_64/apic.rs @@ -101,7 +101,7 @@ pub const fn narrow_destination(apic_id: u32) -> Result { /// for a message and for a remapping entry, with or without `EIME`. pub(crate) const MSG_DEST: u32 = 0; /// [`MSG_DEST`] in eight bits, so no message or entry naming it is refused. -const MSG_DEST_NARROW: u8 = match narrow_destination(MSG_DEST) { +pub(crate) const MSG_DEST_NARROW: u8 = match narrow_destination(MSG_DEST) { Ok(dest) => dest, Err(_) => panic!("MSG_DEST does not fit an 8-bit destination"), }; diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index 4b3a384c1ff..675a666b57b 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -35,7 +35,7 @@ use crate::log; use alloc::vec::Vec; -use super::super::apic::{narrow_destination, MSG_DEST}; +use super::super::apic::{narrow_destination, MSG_DEST, MSG_DEST_NARROW}; use super::super::idt::DriverIrq; use crate::iommu::StreamId; use crate::sync::Lock; @@ -148,11 +148,21 @@ pub fn is_armed() -> bool { REMAP.lock().table.is_some() } +/// The destination an entry encodes: the whole id under `EIME`, and without it +/// only the eight bits [`narrow_destination`] produced. +enum Destination { + Extended(u32), + Narrow(u8), +} + /// The entry that delivers `vector` to `dest` for `source` alone: delivery mode /// 000b, destination mode physical and no redirection hint, which is what the /// compatibility message it replaces said. -fn entry(extended: bool, source: StreamId, vector: u8, dest: u32, level: bool) -> (u64, u64) { - let destination = if extended { dest as u64 } else { (dest as u64) << NARROW_DESTINATION_SHIFT }; +fn entry(source: StreamId, vector: u8, dest: Destination, level: bool) -> (u64, u64) { + let destination = match dest { + Destination::Extended(id) => id as u64, + Destination::Narrow(id) => (id as u64) << NARROW_DESTINATION_SHIFT, + }; ( PRESENT | if level { TRIGGER_LEVEL } else { 0 } @@ -191,21 +201,28 @@ fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result Err(why), + Ok(_) if remap.used == ENTRIES => Err(Refused::TableFull), + Ok(destination) => { + let index = remap.used; + remap.used += 1; + let (lo, hi) = entry(source, vector, destination, level); + table.write_pair(index as usize, lo, hi); + // Under the same lock as the write, so no other entry can be + // written between an entry and the invalidation that publishes it. + // Section 6.4: a unit reporting `CAP.CM`, as these do, may cache the + // entry a fault was taken on. + super::invalidate_interrupt_entries(); + Ok((index, table.read_pair(index as usize))) + } } }; match written { @@ -264,7 +281,12 @@ pub fn claim(slot: usize, source: StreamId) -> Result { table.read_pair(slot).0 & PRESENT == 0, "iommu: claim slot {slot}'s entry is still present" ); - let (lo, hi) = entry(remap.extended, source, vector, MSG_DEST, false); + let destination = if remap.extended { + Destination::Extended(MSG_DEST) + } else { + Destination::Narrow(MSG_DEST_NARROW) + }; + let (lo, hi) = entry(source, vector, destination, false); table.write_pair(slot, lo, hi); super::invalidate_interrupt_entries(); (index, table.read_pair(slot))