Every write to SMI_CMD is made on the boot processor, by one function that reads its CPU beside the out

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.

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.

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<LegacyMode>,
     /// Or why it is none a holder can be handed.
     ec: Result<Ec, String>,
 }
@@ -79,21 +77,6 @@ static HARDWARE: AtomicPtr<Hardware> = 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<LockGuard<'static, ()>> {
-    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,
     );
 }

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();
 }

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;

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);