diff --git a/issues/no-t14-row-arranges-an-acpi-disable-asked-off-the-boot-processor.md b/issues/no-t14-row-arranges-an-acpi-disable-asked-off-the-boot-processor.md new file mode 100644 index 00000000000..a2ae4645bc3 --- /dev/null +++ b/issues/no-t14-row-arranges-an-acpi-disable-asked-off-the-boot-processor.md @@ -0,0 +1,76 @@ +--- +status: open +kind: tooling +opened: 2026-10-07 +--- + +# No T14 row arranges an ACPI_DISABLE asked off the boot processor + +`smi_cmd::write` (`kernel/src/arch/x86_64/smi_cmd.rs`) makes every write to +`SMI_CMD` on the boot processor: a caller on another CPU kicks it and spins +until it has written. The T14's `acpi_server_death` row reads that crossing +for the enable, because its job claims from a thread it found off the boot +processor and the judge refuses `asked from cpu0` there. Three things it +leaves to the scheduler or does not read: + +- **The disable asked from another CPU.** `acpi_mode::release` runs in the + task that drops the claim's last handle. On the `acpicase` boot that is the + killed `/system/bin/acpiserver`'s last thread, in its teardown and under + its process's lock (`process::teardown_resources`), on the CPU the kill + found it on: a killed task is never migrated. The job places no thread of + another process, and the tree has no affinity, so where the server dies is + the scheduler's. The judge prints the disable's asker and requires nothing + of it: a judge that required a crossing no job arranges would be a flake. +- **The boot processor's answer from a lock's spin.** `tlb::poll` answers a + round for a boot processor that spins with interrupts closed on a lock the + asker may hold, where no kick reaches it. No row arranges that contention, + and the kernel's line does not say whether a write was answered from the + kick or from a spin. +- **The enable's arrangement has a window.** The job's claiming thread reads + its x2APIC id and then enters the kernel + (`tests/toyos-rust-tests/src/bin/acpi_release.rs`, + `claim_off_the_boot_processor`). The kernel moves no running thread, so the + asker is the CPU the thread read, unless a preemption between that read and + the kernel's lock in `smi_cmd::write` queues the thread behind another and + an idle boot processor takes it. With no affinity in the tree the window + cannot be closed. + +The disable, on the recorded `acpicase` boots. Ten before `289906d5b`: the +`legacy mode again` line was logged on cpu0 in nine and on cpu1 in one, an +early head of the stage that added the claim, whose line named no asker; the +one of the ten whose line names it, `448107b54`, reads `on cpu0, asked from +cpu0`. `289906d5b` reads `ACPI_DISABLE 0xf1 written to SMI_CMD 0xb2 on cpu0, +asked from cpu2`, and the same head with the read of the CPU removed from +`answer` reads `on cpu2, asked from cpu2`. So the wait under the dying +process's lock has run on the machine once, by the scheduler's doing and not +the job's: of the three boots whose line names the asker, one did not cross. +Whether any write on any boot was answered from a lock's spin is not known: +no line says which path answered. + +The window, on those two boots, the only ones of the job that places its +claim: neither hit it. Each enable reads `asked from cpu1` beside the job's `acpi_release: claimed from +the CPU of x2APIC id 2`. A boot that does hit it reds the row as + +`FAIL acpi_server_death: the job's claim was asked from the boot processor, so no CPU asked it for this write:` + +over an enable line reading `on cpu0, asked from cpu0`, while the job's own +line names a non-zero x2APIC id. That pair is this window and no defect of +the kernel's write: it is answered here, by closing the window or by the +row's judge giving up the refusal, and never by a second boot. + +What could arrange the disable's asker, unbuilt and unmeasured: a holder that +reads itself off the boot processor and ends holding the claim, so its own +teardown is the release; or a thread of the job's kept running on the boot +processor from before the server's spawn to after its kill, so that no +placement puts the server there, which rests on where a wake puts a parked +thread and that has not been read against it. Either inherits the window +above. + +Owned by `issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md`. + +**Exit**: a T14 row's judge refuses a boot whose `legacy mode again` line +reads `asked from cpu0`, on a job that arranges the asker and does not leave +it to the scheduler; a T14 row reads a kernel line saying a write to +`SMI_CMD` was answered from a lock's spin; and the thread whose asker a judge +refuses is held to its CPU from before it reads where it is until the kernel +has taken its request. diff --git a/issues/nothing-reads-whether-an-sci-source-is-enabled-when-acpi-disable-is-written.md b/issues/nothing-reads-whether-an-sci-source-is-enabled-when-acpi-disable-is-written.md new file mode 100644 index 00000000000..94889b47633 --- /dev/null +++ b/issues/nothing-reads-whether-an-sci-source-is-enabled-when-acpi-disable-is-written.md @@ -0,0 +1,29 @@ +--- +status: open +kind: finding +opened: 2026-10-07 +--- + +# Nothing reads whether an SCI source is enabled when ACPI_DISABLE is written + +ACPI 6.5 Table 5.9, on `ACPI_DISABLE`, has the OS hand ownership back "by +relinquishing use to the ACPI hardware registers, masking off all SCI +interrupts, clearing the SCI_EN bit and then writing ACPI_DISABLE to the +SMI_CMD port from the boot processor." + +`acpi_mode::release` (`kernel/src/arch/x86_64/acpi_mode.rs`) writes the +disable when the claim's last handle goes, which is after its holder stopped +running, and nothing in the kernel reads the PM1 enable register or the GPE0 +enable bytes before it: what a killed `/system/bin/acpiserver` left enabled is +enabled still when the firmware takes the registers back. It does not clear +`SCI_EN` itself, by §4.8.2.5 and Table 4.13 of the same document, and says so +at the site. No failure is recorded: on the T14 `SCI_EN` read clear after +every recorded disable. + +Owned by `issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md`. + +**Exit**: the release's `legacy mode again` line says what the PM1 enable +register and the GPE0 enable bytes read at the write on the T14's +`acpi_server_death` boot, and either every one reads zero there by the +release's own doing or this file is promoted with what the firmware did with +the ones that did not. diff --git a/issues/the-kernel-writes-smi-cmd-from-whichever-cpu-its-caller-runs-on.md b/issues/the-kernel-writes-smi-cmd-from-whichever-cpu-its-caller-runs-on.md deleted file mode 100644 index cafece06d30..00000000000 --- a/issues/the-kernel-writes-smi-cmd-from-whichever-cpu-its-caller-runs-on.md +++ /dev/null @@ -1,38 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-07 ---- - -# The kernel writes SMI_CMD from whichever CPU its caller runs on - -ACPI 6.5 Table 5.9, on `SMI_CMD`: "OSPM issues commands to the SMI_CMD port -synchronously from the boot processor." On `ACPI_DISABLE`: "An OS can hand -ownership back to SMI by relinquishing use to the ACPI hardware registers, -masking off all SCI interrupts, clearing the SCI_EN bit and then writing -ACPI_DISABLE to the SMI_CMD port from the boot processor." - -`kernel/src/arch/x86_64/acpi_mode.rs` writes `ACPI_ENABLE` on the CPU the -claimant mints on and `ACPI_DISABLE` on the CPU the claim's last handle goes -on. On the T14's `acpi_server_death` boots at `ff4945d6d` and `8d7004d3b` the -enable was written from cpu7 (`acpi: ACPI mode: ACPI_ENABLE 0xf0 written to -SMI_CMD 0xb2, SCI_EN set 16687ns after; cpu7's SMI count 4819 before the -write and 4820 after`) and the disable from cpu0; on every other recorded -boot the enable came from cpu0. Each took: `SCI_EN` read set, and clear after -the disable. No failure is recorded, and no firmware but the T14's has been -read. - -The release follows §4.8.2.5 of the same document, which has OSPM write -`ACPI_DISABLE` and poll `SCI_EN` until it reads reset, and Table 4.13, which -makes the bit the hardware's to set and reset and has OSPM preserve it. Table -5.9's sequence, in which the OS clears `SCI_EN` itself before the write, -contradicts both, and the kernel does not follow it. Whether the release -leaves any SCI source enabled when it writes the disable is not read here. - -Owned by `issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md`, -whose stage 1 landed these writes. - -**Exit**: every write to `SMI_CMD` is made on the boot processor by -construction, and the kernel's `ACPI mode:` and `legacy mode again` lines name -it on the T14's `acpi_server_death` boot; or the owner rules that the CPU does -not matter, and that ruling is recorded at the site. diff --git a/kernel/src/arch/x86_64/acpi_mode.rs b/kernel/src/arch/x86_64/acpi_mode.rs index 602f3aee847..fb9ac1bec61 100644 --- a/kernel/src/arch/x86_64/acpi_mode.rs +++ b/kernel/src/arch/x86_64/acpi_mode.rs @@ -2,17 +2,16 @@ //! //! **This kernel puts the machine in ACPI mode only for a holder of the //! `acpi` claim, and back in the mode its firmware handed over when the claim -//! goes.** The mint writes `ACPI_ENABLE` to `SMI_CMD` (ACPI 6.5 Table 5.9) -//! where `SCI_EN` reads clear, and waits for the firmware to set it; the +//! goes.** The mint writes `ACPI_ENABLE` to `SMI_CMD` (ACPI 6.5 Table 5.9), +//! through `smi_cmd::write` as every write to that port is and so on the boot +//! processor, where `SCI_EN` reads clear, and waits for the firmware to set it; the //! release writes `ACPI_DISABLE` where the mint wrote the enable and reads //! `SCI_EN` until it is clear, with the row still held, so a dead server //! leaves the buttons to the firmware again before anyone else can claim //! them, and so does a mint that wrote the enable and then failed. A firmware //! that does not clear it is said by name, and the next release writes the -//! disable again. **Neither is written once the stop has -//! begun**: the power-off waits out a write in flight and then owns the -//! hardware, and an SMI it did not make is one its S5 entry was never -//! measured against. +//! disable again. Neither is written once the stop has begun: `smi_cmd` +//! refuses it. //! //! **A machine stays in legacy mode where its firmware serves something no //! holder could**: an embedded controller the ECDT does not name, or a power @@ -38,13 +37,12 @@ use toyos_abi::acpi::{AcpiInfo, Block, FIXED_POWER_BUTTON}; use toyos_acpi::{Ec, FixedHardware, LegacyMode, PowerButton}; use toyos_userbound::Ports; -use super::cpu; use super::pio::{self, Declared, TakenBack}; use super::power::SCI_EN; +use super::{cpu, smi_cmd}; use crate::device::ClaimError; use crate::isa::{self, Function}; use crate::log; -use crate::sync::{Lock, LockGuard}; use crate::time::{Deadline, Duration}; /// `isa`'s row for the fixed hardware. @@ -66,8 +64,8 @@ const HANDBACK: Duration = Duration::from_millis(100); struct Hardware { fixed: FixedHardware, control: Declared, - /// `SMI_CMD`, declared, and what is written to it. - legacy: Option<(Declared, LegacyMode)>, + /// `SMI_CMD`, declared to `smi_cmd`, and what is written to it. + legacy: Option, /// Or why it is none a holder can be handed. ec: Result, } @@ -79,21 +77,6 @@ static HARDWARE: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); /// the release writes `ACPI_DISABLE`. static ENABLED: AtomicBool = AtomicBool::new(false); -/// Held across every write to `SMI_CMD`. -static SMI_CMD_WRITE: Lock<()> = Lock::new(()); - -/// The right to write `SMI_CMD`, held across the write; none once the stop has -/// begun. -fn smi_cmd_write() -> Option> { - let writing = SMI_CMD_WRITE.lock(); - (!crate::quiesce::begun()).then_some(writing) -} - -/// Wait out a write to `SMI_CMD` in flight: the stop has begun, so none follows. -pub fn settle(_taken: &TakenBack) { - drop(SMI_CMD_WRITE.lock()); -} - fn hardware() -> Option<&'static Hardware> { let at = HARDWARE.load(Ordering::Acquire); // SAFETY: `init` stored a leaked `Box` once, and nothing frees it. @@ -120,11 +103,10 @@ pub fn init(rsdp_addr: u64) { let Some(control) = super::power::pm1a_control() else { return log!("acpi: no ACPI row — no PM1a control block declared"); }; - let declared = |legacy: LegacyMode| pio::declare("SMI_CMD", Ports::one(legacy.smi_cmd)).map(|port| (port, legacy)); - let legacy = match fixed.legacy.map(declared).transpose() { - Ok(legacy) => legacy, - Err(why) => return log!("acpi: no ACPI row — SMI_CMD not declared: {why:?}"), - }; + if let Some(Err(why)) = fixed.legacy.map(|legacy| smi_cmd::declare(legacy.smi_cmd)) { + return log!("acpi: no ACPI row — SMI_CMD not declared: {why:?}"); + } + let legacy = fixed.legacy; let ec = embedded_controller(rsdp_addr, fixed.gpe0); let Some(sci) = super::ioapic::sci(fixed.sci_int) else { return log!("acpi: no ACPI row — no I/O APIC carries the SCI"); @@ -223,7 +205,7 @@ fn enter(hardware: &Hardware) -> Result<(), ClaimError> { log!("acpi: this machine stays in legacy mode — {why}"); Err(ClaimError::Unusable) }; - let Some((smi_cmd, legacy)) = hardware.legacy else { + let Some(legacy) = hardware.legacy else { return refuse("the FADT names no SMI_CMD, ACPI_ENABLE and ACPI_DISABLE to leave it and come back with"); }; let enable = legacy.acpi_enable.get(); @@ -236,19 +218,10 @@ fn enter(hardware: &Hardware) -> Result<(), ClaimError> { return refuse("its power button is a control method device, which only AML serves"); } - // The write raises a firmware interrupt where `APMC_EN` is set, counted on - // the CPU that makes it, so both reads are that CPU's. - let (me, before, after) = { - let Some(_writing) = smi_cmd_write() else { - return refuse("the machine is stopping"); - }; - let _closed = crate::arch::IrqGuard::close(); - let before = super::counters::read().smi; - // SAFETY: `SMI_CMD`, declared, and the value the FADT names for it. - unsafe { cpu::outb(smi_cmd.port(0), enable) }; - ENABLED.store(true, Ordering::Relaxed); - (super::percpu::cpu_id(), before, super::counters::read().smi) + let Some(write) = smi_cmd::write(enable) else { + return refuse("the machine is stopping"); }; + ENABLED.store(true, Ordering::Relaxed); let written = crate::clock::now(); let by = Deadline::at(written + HANDOVER); let parkable = crate::scheduler::Parkable::at_entry(); @@ -266,11 +239,9 @@ fn enter(hardware: &Hardware) -> Result<(), ClaimError> { } } log!( - "acpi: ACPI mode: ACPI_ENABLE {enable:#04x} written to SMI_CMD {:#x}, SCI_EN set {} after; cpu{me}'s SMI count {} before the write and {} after", - smi_cmd.ports().first(), + "acpi: ACPI mode: ACPI_ENABLE {enable:#04x} written to SMI_CMD {:#x} {write}; SCI_EN set {} after", + legacy.smi_cmd, crate::clock::now() - written, - before.map_or_else(|| "unread".into(), |n| format!("{n}")), - after.map_or_else(|| "unread".into(), |n| format!("{n}")), ); Ok(()) } @@ -287,16 +258,15 @@ pub fn release(row: usize) { /// `ACPI_DISABLE`, written where the mint wrote `ACPI_ENABLE`, and `SCI_EN` /// read until it is clear; nothing once the stop has begun. Spun, never -/// parked: the task a claim's last handle goes with may be dying. +/// parked: the task a claim's last handle goes with may be dying. `SCI_EN` is +/// the hardware's to reset (ACPI 6.5 §4.8.2.5, Table 4.13), so it is not +/// cleared here before the write as Table 5.9's `ACPI_DISABLE` has it. fn leave(hardware: &Hardware) { - let (smi_cmd, legacy) = hardware.legacy.expect("ACPI_ENABLE was written to it"); + let legacy = hardware.legacy.expect("ACPI_ENABLE was written to it"); let disable = legacy.acpi_disable.get(); - let Some(writing) = smi_cmd_write() else { + let Some(write) = smi_cmd::write(disable) else { return log!("acpi: ACPI_DISABLE not written: the machine is stopping, and its power-off owns ACPI mode"); }; - // SAFETY: `SMI_CMD`, declared, and the value the FADT names for it. - unsafe { cpu::outb(smi_cmd.port(0), disable) }; - drop(writing); let written = crate::clock::now(); let by = Deadline::at(written + HANDBACK); let control = loop { @@ -306,16 +276,19 @@ fn leave(hardware: &Hardware) { } if by.reached(crate::clock::now()) { return log!( - "acpi: still in ACPI mode: ACPI_DISABLE {disable:#04x} written to SMI_CMD, and PM1a_CNT reads {control:#06x} \ - {HANDBACK} after, SCI_EN still set: nothing serves this machine's buttons until a holder claims them" + "acpi: still in ACPI mode: ACPI_DISABLE {disable:#04x} written to SMI_CMD {:#x} {write}; PM1a_CNT reads \ + {control:#06x} {HANDBACK} after, SCI_EN still set: nothing serves this machine's buttons until a holder \ + claims them", + legacy.smi_cmd ); } core::hint::spin_loop(); }; ENABLED.store(false, Ordering::Relaxed); log!( - "acpi: legacy mode again: ACPI_DISABLE {disable:#04x} written to SMI_CMD, PM1a_CNT reads {control:#06x} {} after, \ - SCI_EN clear", + "acpi: legacy mode again: ACPI_DISABLE {disable:#04x} written to SMI_CMD {:#x} {write}; PM1a_CNT reads \ + {control:#06x} {} after, SCI_EN clear", + legacy.smi_cmd, crate::clock::now() - written, ); } diff --git a/kernel/src/arch/x86_64/idt/kick.rs b/kernel/src/arch/x86_64/idt/kick.rs index 7e957753a8e..0afa1e25882 100644 --- a/kernel/src/arch/x86_64/idt/kick.rs +++ b/kernel/src/arch/x86_64/idt/kick.rs @@ -3,6 +3,7 @@ use super::device_irq::device_irq_entry; extern "sysv64" fn kick_handler() { crate::arch::percpu::irq_took!(Kick); crate::counters::serve_here(); + crate::arch::smi_cmd::serve_here(); crate::preempt::set_need_resched(); crate::arch::apic::eoi(); } diff --git a/kernel/src/arch/x86_64/mod.rs b/kernel/src/arch/x86_64/mod.rs index 8ff1f396084..de6dfb94d6b 100644 --- a/kernel/src/arch/x86_64/mod.rs +++ b/kernel/src/arch/x86_64/mod.rs @@ -34,6 +34,7 @@ pub mod pio; pub mod pmu; pub mod power; pub mod rtc; +pub mod smi_cmd; pub mod smp; pub mod switch; pub mod syscall; diff --git a/kernel/src/arch/x86_64/power.rs b/kernel/src/arch/x86_64/power.rs index c3698b7161e..d1d86746d9d 100644 --- a/kernel/src/arch/x86_64/power.rs +++ b/kernel/src/arch/x86_64/power.rs @@ -170,7 +170,7 @@ pub fn off(stopping: crate::quiesce::Stopping) -> ! { let (Some(control), true) = (PM1A_CNT.get(), SOFT_OFF.load(Ordering::Acquire)) else { cpu::halt() }; let control = control.port(0); let taken = pio::take_back(&stopping); - super::acpi_mode::settle(&taken); + super::smi_cmd::settle(&taken); let held = cpu::inw(control); if held & SCI_EN != 0 { super::acpi_mode::quiet(&taken); diff --git a/kernel/src/arch/x86_64/smi_cmd.rs b/kernel/src/arch/x86_64/smi_cmd.rs new file mode 100644 index 00000000000..564cb12c989 --- /dev/null +++ b/kernel/src/arch/x86_64/smi_cmd.rs @@ -0,0 +1,164 @@ +//! `SMI_CMD`: the port a write to is a command to the firmware, and the one +//! function that writes it. +//! +//! **Every write is made on the boot processor, wherever its caller runs.** +//! ACPI 6.5 Table 5.9, of `SMI_CMD`: "OSPM issues commands to the SMI_CMD port +//! synchronously from the boot processor"; and of `ACPI_DISABLE`, "writing +//! ACPI_DISABLE to the SMI_CMD port from the boot processor". The boot +//! processor is cpu0: the CPU the firmware handed over on, which brought up +//! every other. +//! +//! A caller on another CPU asks a round of [`Shootdown`]'s protocol with the +//! boot processor as its one target, as a counters read asks one of every CPU: +//! it kicks the boot processor, which makes the write from its kick handler +//! whether it was idle or busy, or from a lock's spin where its interrupts are +//! closed ([`serve_here`]), and spins until that round is answered. [`write`] +//! therefore returns only once the `out` has retired, which is after the +//! firmware's handler has. A boot processor that answers neither way within +//! [`DEAF_CPU`] is a panic, as one that answers no TLB shootdown is. +//! +//! The port is this module's alone, so the `out` in [`answer`] is the only +//! one the kernel can make to it, and [`answer`] reads which CPU it is on +//! with interrupts closed beside that `out`: no caller's state decides it. +//! +//! **None is made once the stop has begun**: the power-off waits out a write +//! in flight ([`settle`]) and then owns the hardware, and an SMI it did not +//! make is one its S5 entry was never measured against. + +use core::fmt; +use alloc::string::String; +use core::sync::atomic::{AtomicU32, AtomicU64, AtomicU8, Ordering::Relaxed}; + +use toyos_userbound::{Ports, Undeclared}; + +use super::pio::{self, Slot, TakenBack}; +use super::{apic, cpu, percpu, IrqGuard}; +use crate::shootdown::Shootdown; +use crate::sync::Lock; +use crate::time::{Deadline, Duration, DEAF_CPU}; + +/// The boot processor. +const BOOT: u32 = 0; + +/// The port, where the FADT names one. +static PORT: Slot = Slot::empty(); + +/// Held from a write's ask to its answer, so one round is in flight at most +/// and each is answered by one write. +static WRITING: Lock<()> = Lock::new(()); +static ROUND: Shootdown = Shootdown::new(); +/// What the round in flight writes: stored before the round is issued, read by its answer. +static ASKED: AtomicU8 = AtomicU8::new(0); +/// What the last answer read, published by the round's own answer. +static ON: AtomicU32 = AtomicU32::new(0); +static HELD_NS: AtomicU64 = AtomicU64::new(0); +static SMIS: [AtomicU64; 2] = [const { AtomicU64::new(UNREAD) }; 2]; + +/// No SMI count: the register holds 32 bits. +const UNREAD: u64 = u64::MAX; + +/// One write, as the CPU that made it read it. +pub struct Written { + /// The CPU the `out` ran on, read there. + on: u32, + asked_from: u32, + /// From before the `out` to after it, which is the firmware's handler + /// where the write raises an SMI. + held: Duration, + /// That CPU's SMI count either side of the write, where it reads one. + smis: [u64; 2], +} + +impl fmt::Display for Written { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let Self { on, asked_from, held, smis } = self; + let [before, after] = smis.map(|n| if n == UNREAD { String::from("unread") } else { alloc::format!("{n}") }); + write!( + f, + "on cpu{on}, asked from cpu{asked_from}; the write held cpu{on} {}ns, its SMI count {before} before the write and {after} after", + held.nanos() + ) + } +} + +/// Declare the port the FADT names. Boot's. +pub fn declare(port: u16) -> Result<(), Undeclared> { + PORT.set(pio::declare("SMI_CMD", Ports::one(port))?); + Ok(()) +} + +/// Write `value` to `SMI_CMD` on the boot processor and return once it is +/// written; `None`, and nothing written, once the stop has begun. +pub fn write(value: u8) -> Option { + // Preemption is off under the lock, so this CPU is the caller's throughout. + let _writing = WRITING.lock(); + if crate::quiesce::begun() { + return None; + } + let asked_from = percpu::cpu_id(); + ASKED.store(value, Relaxed); + let generation = ROUND.issue(); + // The boot processor's own ask, unless a kick taken since the issue has answered it already. + answer(); + if !ROUND.served(BOOT as usize, generation) { + apic::kick_cpu(BOOT); + let by = Deadline::at(crate::clock::now() + Duration::from_nanos(DEAF_CPU.nanos())); + while !ROUND.served(BOOT as usize, generation) { + assert!( + !by.reached(crate::clock::now()), + "smi_cmd: the boot processor has not written {value:#04x} for cpu{asked_from} in {DEAF_CPU}: it is not taking interrupts" + ); + core::hint::spin_loop(); + // A caller with interrupts closed owes the boot processor's shootdown an answer while it waits. + super::tlb::poll(); + } + } + Some(Written { + on: ON.load(Relaxed), + asked_from, + held: Duration::from_nanos(HELD_NS.load(Relaxed)), + smis: [SMIS[0].load(Relaxed), SMIS[1].load(Relaxed)], + }) +} + +/// The boot processor's answer to a round it owes: its kick handler's, and +/// `tlb::poll`'s for a boot processor spinning on a lock with interrupts +/// closed, which takes no kick and may be waiting on the caller's lock. +/// Every CPU calls it; [`answer`] holds which one writes. +#[inline] +pub fn serve_here() { + // The hint before the mask: with no round in flight a kick or a spin pays two loads. + if ROUND.owes(BOOT as usize) { + answer(); + } +} + +/// The write the round in flight asks for, if none has answered it and this +/// is the boot processor; nothing on any other CPU. Interrupts are closed +/// from the read of the CPU to the `out`, so the two are one CPU's and no +/// kick's answer nests in this one and writes twice. Takes no lock and +/// allocates nothing. +fn answer() { + let _closed = IrqGuard::close(); + if percpu::cpu_id() != BOOT { + return; + } + ROUND.serve_if_owed(BOOT as usize, || { + let port = PORT.get().expect("smi_cmd: a write asked of a machine whose FADT names no SMI_CMD").port(0); + let value = ASKED.load(Relaxed); + let smi = || super::counters::read().smi.unwrap_or(UNREAD); + let before = smi(); + let from = crate::clock::now(); + // SAFETY: `SMI_CMD`, declared; the value is one the FADT names for it, by `write`'s caller. + unsafe { cpu::outb(port, value) }; + HELD_NS.store((crate::clock::now() - from).nanos(), Relaxed); + SMIS[0].store(before, Relaxed); + SMIS[1].store(smi(), Relaxed); + ON.store(percpu::cpu_id(), Relaxed); + }); +} + +/// Wait out a write to `SMI_CMD` in flight: the stop has begun, so none follows. +pub fn settle(_taken: &TakenBack) { + drop(WRITING.lock()); +} diff --git a/kernel/src/arch/x86_64/tlb.rs b/kernel/src/arch/x86_64/tlb.rs index a6565cd2f12..75eda87bae9 100644 --- a/kernel/src/arch/x86_64/tlb.rs +++ b/kernel/src/arch/x86_64/tlb.rs @@ -157,9 +157,11 @@ pub fn serve_ipi() { } /// Answers a pending shootdown without taking a lock or allocating, so it is -/// safe from inside `Lock::lock`'s spin. +/// safe from inside `Lock::lock`'s spin; and, on the boot processor, a write +/// to `SMI_CMD` asked of it, which a spinner with `IF` clear owes the same way. #[inline] pub fn poll() { + super::smi_cmd::serve_here(); if !smp::answering() { return; } diff --git a/tests/toyos-rust-tests/src/arch/cpu.rs b/tests/toyos-rust-tests/src/arch/cpu.rs new file mode 100644 index 00000000000..bffac734eae --- /dev/null +++ b/tests/toyos-rust-tests/src/arch/cpu.rs @@ -0,0 +1,18 @@ +//! Which CPU a thread is on, as that CPU answers. + +#[cfg(target_arch = "x86_64")] +pub use x86_64::*; + +#[cfg(target_arch = "x86_64")] +mod x86_64 { + use core::arch::x86_64::{__cpuid, __cpuid_count}; + + /// The x2APIC id of the CPU that ran this (CPUID leaf 0BH, `EDX`: "x2APIC + /// ID of the current logical processor", Intel SDM vol. 2A). The caller's + /// CPU only until the scheduler moves the thread. + pub fn x2apic_id() -> u32 { + let leaves = __cpuid(0).eax; + assert!(leaves >= 0xb, "this CPU's highest basic CPUID leaf is {leaves:#x}, below 0BH: it names no x2APIC id"); + __cpuid_count(0xb, 0).edx + } +} diff --git a/tests/toyos-rust-tests/src/bin/acpi_release.rs b/tests/toyos-rust-tests/src/bin/acpi_release.rs index 92fcc5b4896..91ffc261e07 100644 --- a/tests/toyos-rust-tests/src/bin/acpi_release.rs +++ b/tests/toyos-rust-tests/src/bin/acpi_release.rs @@ -1,5 +1,6 @@ //! The ACPI server's death, on a boot that does not start it: this job claims -//! the fixed hardware, which puts the machine in ACPI mode, hands the claim to +//! the fixed hardware from a thread off the boot processor, which puts the +//! machine in ACPI mode by a write another CPU asked for, hands the claim to //! `/system/bin/acpiserver`, and kills it once it has armed. The kernel's //! release of the claim writes `ACPI_DISABLE` and says what `SCI_EN` then //! reads, which the `acpi_server_death` metal row judges; this asserts only @@ -8,17 +9,47 @@ use std::io::{BufRead, BufReader}; use std::os::toyos::process::CommandExt; use std::process::{Command, Stdio}; +use std::thread; use toyos::endow::{Endowments, DEV_PREFIX, SYSCAP_LABEL}; use toyos::syscap::SysCap; use toyos::Device; use toyos_abi::syscall::DeviceType; +#[path = "../arch/cpu.rs"] +mod cpu; + const SERVER: &str = "/system/bin/acpiserver"; +/// Threads started in turn before one must have found itself off the boot +/// processor: a spawn goes to the least loaded CPU from a start that rotates. +const STARTS: usize = 64; + +/// The claim, asked by a thread that read itself off the boot processor as +/// its first act: x2APIC id 0, which the judge holds against the kernel's own +/// `asked from`. The kernel moves no running thread, so the asker is that CPU +/// unless a preemption between the read and the kernel's lock queued the +/// thread behind another and an idle boot processor took it. +fn claim_off_the_boot_processor(cap: &SysCap) -> Device { + for _ in 0..STARTS { + let claimed = thread::scope(|threads| { + let asker = threads.spawn(|| { + let on = cpu::x2apic_id(); + (on != 0).then(|| (on, cap.claim::(DeviceType::Acpi).expect("acpi_release: the fixed hardware's claim"))) + }); + asker.join().expect("acpi_release: the claiming thread") + }); + if let Some((on, claim)) = claimed { + println!("acpi_release: claimed from the CPU of x2APIC id {on}"); + return claim; + } + } + panic!("acpi_release: {STARTS} threads in a row started on the boot processor"); +} + fn main() { let cap: SysCap = Endowments::get().take(SYSCAP_LABEL).expect("test-runner endows a device-minting capability"); - let claim: Device = cap.claim(DeviceType::Acpi).expect("acpi_release: the fixed hardware's claim"); + let claim = claim_off_the_boot_processor(&cap); let mut server = Command::new(SERVER) .endow(&format!("{DEV_PREFIX}{}", DeviceType::Acpi.class_name()), claim.into_raw().0) .stdout(Stdio::piped()) diff --git a/tests/toyos.rs b/tests/toyos.rs index 9aa5ca5b4a1..996ed86de37 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -3643,7 +3643,7 @@ fn counters_on_metal(back: &metal::Readback) -> Result<(), String> { let delta = |a: &Read, b: &Read, cpu: usize, name: &str| b[&cpu][name] - a[&cpu][name]; // The boot's one write to `SMI_CMD`, and what the CPU that made it read after it. let enabled = kernel.must_say("acpi: ACPI mode: ACPI_ENABLE ")?; - let writer: usize = field_between(enabled, "; cpu", "'s SMI count ")?.parse().map_err(|_| format!("no CPU in {enabled:?}"))?; + let writer = smi_cmd_writer(enabled)?; let after = number_between(enabled, " before the write and ", " after")?; if idle0[&writer]["smi"] < after { return Err(format!("idle0 read cpu{writer}'s SMI count below what it read after the ACPI enable ({after}): it read before the boot's last write to SMI_CMD")); @@ -3722,6 +3722,9 @@ fn counters_on_metal(back: &metal::Readback) -> Result<(), String> { fn acpi_events_on_metal(back: &metal::Readback) -> Result<(), String> { back.job_passed("test_rs_acpi_hold")?; let (log, kernel) = (back.log(), back.kernel()); + let enabled = kernel.must_say("acpi: ACPI mode: ACPI_ENABLE 0xf0 written to SMI_CMD 0xb2 ")?; + smi_cmd_writer(enabled)?; + eprintln!(" [acpi] {}", enabled.trim()); kernel.must_say( "acpi: the ACPI row: PM1a events 0x1800+4, GPE0 0x1860+32, SCI gsi 9 level/high, the \ fixed-hardware power button, embedded controller at 0x66/0x62 on GPE 0x6e; the firmware \ @@ -3753,17 +3756,36 @@ fn acpi_events_on_metal(back: &metal::Readback) -> Result<(), String> { /// The server's death on the T14: the kernel put the machine in ACPI mode for /// the job's claim, and when the killed server's claim went it wrote /// `ACPI_DISABLE` (the FADT's 0xf1) and read `SCI_EN` clear: the firmware has -/// the buttons again. +/// the buttons again. Each write was made on the boot processor, and the +/// enable was asked from another CPU: the job claims from a thread it found +/// off the boot processor, so that write is the one that crossed. fn acpi_death_on_metal(back: &metal::Readback) -> Result<(), String> { back.job_passed("test_rs_acpi_release")?; let kernel = back.kernel(); - kernel.must_say("acpi: ACPI mode: ACPI_ENABLE 0xf0 written to SMI_CMD 0xb2, SCI_EN set ")?; + let enabled = kernel.must_say("acpi: ACPI mode: ACPI_ENABLE 0xf0 written to SMI_CMD 0xb2 ")?; // The kernel says this only of a `PM1a_CNT` it read with `SCI_EN` clear. - let left = kernel.must_say("acpi: legacy mode again: ACPI_DISABLE 0xf1 written to SMI_CMD, PM1a_CNT reads ")?; - eprintln!(" [acpi] {}", left.trim()); + let left = kernel.must_say("acpi: legacy mode again: ACPI_DISABLE 0xf1 written to SMI_CMD 0xb2 ")?; + for line in [enabled, left] { + smi_cmd_writer(line)?; + eprintln!(" [acpi] {}", line.trim()); + } + if number_between(enabled, ", asked from cpu", "; the write held cpu")? == 0 { + return Err(format!("the job's claim was asked from the boot processor, so no CPU asked it for this write: {enabled}")); + } Ok(()) } +/// The CPU a kernel line says wrote `SMI_CMD`, read on that CPU beside the +/// `out`: the boot processor, cpu0, or the line is refused (ACPI 6.5 Table +/// 5.9, "from the boot processor"). +fn smi_cmd_writer(line: &str) -> Result { + let writer = number_between(line, " written to SMI_CMD 0xb2 on cpu", ", asked from cpu")? as usize; + if writer != 0 { + return Err(format!("SMI_CMD was written on cpu{writer}, not on the boot processor: {line}")); + } + Ok(writer) +} + /// The same, parsed. fn number_between(log: &str, head: &str, tail: &str) -> Result { let word = field_between(log, head, tail)?;