From 5a936fc5527eb371e2d69baa639f281210b655b2 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Tue, 29 Sep 2026 03:37:05 +0000 Subject: [PATCH 1/4] acpi: wire and expose the SCI pin According to the ACPI spec., the SCI pin is driven by multiple inputs that are OR'ed to determine if the guest should receive an interrupt[^1]. Introduce `OrGate`, a new component that can front an `Arc` as output and and expose multiple inputs. The output pin state is driven by the OR value of the inputs. The new `sci_pin()` method of `Piix3Lpc` creates and returns a new input to the OR gate fronting the SCI pin. [^1]: https://uefi.org/specs/ACPI/6.6/04_ACPI_Hardware_Specification.html#example-event-structure-for-a-legacyacpi-compatible-event-model --- lib/propolis/src/hw/chipset/i440fx.rs | 18 +- lib/propolis/src/intr_pins.rs | 568 ++++++++++++++++++++++++++ 2 files changed, 580 insertions(+), 6 deletions(-) diff --git a/lib/propolis/src/hw/chipset/i440fx.rs b/lib/propolis/src/hw/chipset/i440fx.rs index beafb08a1..478047b6d 100644 --- a/lib/propolis/src/hw/chipset/i440fx.rs +++ b/lib/propolis/src/hw/chipset/i440fx.rs @@ -20,7 +20,7 @@ use crate::hw::pci::topology::{LogicalBusId, RoutedBusId}; use crate::hw::pci::{ self, Bdf, INTxPinID, LintrCfg, PcieCfgDecoder, PioCfgDecoder, }; -use crate::intr_pins::{IntrPin, LegacyPIC, LegacyPin, NoOpPin}; +use crate::intr_pins::{IntrPin, LegacyPIC, LegacyPin, NoOpPin, OrGate}; use crate::lifecycle; use crate::migrate::*; use crate::mmio::MmioFn; @@ -100,12 +100,14 @@ impl IntrPin for LNKPin { struct IrqConfig { pic: Arc, - lnk_pins: [Arc; 4], - #[allow(unused)] - // XXX: wire up SCI notifications - sci_pin: Arc, + // The SCI state can be driven by multiple inputs that OR'ed to determine + // if an interrupt should happen, so model it as an OrGate and hand out new + // inputs for components that connect to the SCI pin. + // + // Refer to Fig. 4.3 of the ACPI spec. v6.6 for more information. + sci_pin: Arc, } impl IrqConfig { fn create(hdl: Arc) -> Arc { @@ -120,7 +122,7 @@ impl IrqConfig { Arc::new(LNKPin::new()), Arc::new(LNKPin::new()), ], - sci_pin, + sci_pin: OrGate::new(sci_pin), }) } fn set_lnk_route(&self, idx: usize, irq: Option) { @@ -386,6 +388,10 @@ impl Piix3Lpc { }) } + pub fn sci_pin(&self) -> Arc { + self.irq_config.sci_pin.new_input() + } + pub fn attach(self: &Arc, pio: &PioBus) { let this = Arc::clone(self); let piofn = Arc::new(move |port: u16, rwo: RWOp| this.pio_rw(port, rwo)) diff --git a/lib/propolis/src/intr_pins.rs b/lib/propolis/src/intr_pins.rs index cb4a289f8..7b2300e57 100644 --- a/lib/propolis/src/intr_pins.rs +++ b/lib/propolis/src/intr_pins.rs @@ -241,6 +241,106 @@ struct FPInner { func: Box, } +/// Aggregates multiple input pins into a single `output` [IntrPin] using an OR +/// operation. Inputs are also [IntrPin] themselves. Use [OrGate::new_input] to +/// retrieve a new input. +/// +/// The gate is considered stateless for migration purposes. It's expected that +/// the components connected to the inputs manage and reconstruct their own +/// state after a migration. +/// +/// The current implementation uses a single [std::sync::Mutex], and is +/// overly-protective of concurrent operations. It would be possible to allow +/// inputs to change independently, but we would need to be careful to also +/// synchronize updates to the output pin. +/// +/// Internal state is stored in a `>` to make the code easier to read +/// and maintain, but it would be possible to use something like a +/// [std::sync::atomic::AtomicU64] bitmask instead. +pub struct OrGate { + inputs: Mutex>, + output: Arc, +} +impl OrGate { + pub fn new(pin: Arc) -> Arc { + Arc::new(Self { output: pin, inputs: Mutex::new(vec![]) }) + } + + // The gate is expected to be constructed once during VM initialization, so + // inputs are currently never removed. + pub fn new_input(self: &Arc) -> Arc { + let mut inputs = self.inputs.lock().unwrap(); + let pin = Arc::new(OrGatePin { idx: inputs.len(), gate: self.clone() }); + inputs.push(false); + pin + } + + fn set_input_state(&self, input: usize, is_asserted: bool) { + let mut inputs = self.inputs.lock().unwrap(); + inputs[input] = is_asserted; + + // Only modify the output if the gate state actually changed to avoid + // spurious edges. + let should_assert = OrGate::is_asserted(&inputs); + if should_assert != self.output.is_asserted() { + self.output.set_state(should_assert); + } + } + + fn pulse_input(&self) { + let inputs = self.inputs.lock().unwrap(); + if !OrGate::is_asserted(&inputs) { + self.output.pulse(); + } + } + + fn is_input_asserted(&self, input: usize) -> bool { + let inputs = self.inputs.lock().unwrap(); + inputs[input] + } + + fn import_input_state(&self, input: usize, is_asserted: bool) { + let mut inputs = self.inputs.lock().unwrap(); + inputs[input] = is_asserted; + + let should_assert = OrGate::is_asserted(&inputs); + if should_assert != self.output.is_asserted() { + self.output.import_state(should_assert); + } + } + + fn is_asserted(inputs: &[bool]) -> bool { + inputs.contains(&true) + } +} + +pub struct OrGatePin { + idx: usize, + gate: Arc, +} +impl IntrPin for OrGatePin { + fn assert(&self) { + self.gate.set_input_state(self.idx, true); + } + fn deassert(&self) { + self.gate.set_input_state(self.idx, false); + } + fn pulse(&self) { + self.gate.pulse_input(); + } + fn is_asserted(&self) -> bool { + self.gate.is_input_asserted(self.idx) + } + fn import_state(&self, is_asserted: bool) { + self.gate.import_input_state(self.idx, is_asserted) + } +} +impl Drop for OrGatePin { + fn drop(&mut self) { + self.deassert(); + } +} + pub struct NoOpPin {} impl IntrPin for NoOpPin { @@ -252,3 +352,471 @@ impl IntrPin for NoOpPin { } fn import_state(&self, _: bool) {} } + +#[cfg(test)] +pub mod test { + use super::*; + + use std::hint; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Barrier; + use std::thread; + + pub struct TestPin { + state: Mutex, + } + impl TestPin { + pub fn new() -> Self { + Self { + state: Mutex::new(TestPinState { + is_asserted: false, + rises: 0, + falls: 0, + pulses: 0, + }), + } + } + + // Number of times the pin went from deasserted to asserted. + pub fn rises(&self) -> usize { + self.state.lock().unwrap().rises + } + + // Number of times the pin went from asserted to deasserted. + pub fn falls(&self) -> usize { + self.state.lock().unwrap().falls + } + + // Number of times the pin was pulsed. A pulse also generates a rise + // and a fall. + pub fn pulses(&self) -> usize { + self.state.lock().unwrap().pulses + } + } + impl IntrPin for TestPin { + fn assert(&self) { + self.state.lock().unwrap().assert(); + } + fn deassert(&self) { + self.state.lock().unwrap().deassert(); + } + fn pulse(&self) { + self.state.lock().unwrap().pulse(); + } + fn is_asserted(&self) -> bool { + self.state.lock().unwrap().is_asserted + } + fn import_state(&self, is_asserted: bool) { + self.state.lock().unwrap().import_state(is_asserted); + } + } + + struct TestPinState { + is_asserted: bool, + rises: usize, + falls: usize, + pulses: usize, + } + impl TestPinState { + fn assert(&mut self) { + if !self.is_asserted { + self.is_asserted = true; + self.rises += 1; + } + } + fn deassert(&mut self) { + if self.is_asserted { + self.is_asserted = false; + self.falls += 1; + } + } + fn pulse(&mut self) { + if !self.is_asserted { + self.rises += 1; + self.falls += 1; + self.pulses += 1; + } + } + fn import_state(&mut self, is_asserted: bool) { + self.is_asserted = is_asserted; + } + } + + // Type used to control an OrGate pin during tests. + type PinSignal = Arc, usize) + Send + Sync>; + + // Test harness that provides an OrGate connected to a TestPin. Inputs + // are controlled using PinSignal closures and are synchronized with a + // shared clock. The PinSignal closure receives the current tick as + // input. + // + // Both the gate and the output pin can be accessed for asserting the + // overall state of the system. + struct OrGateDriver { + gate: Arc, + output: Arc, + start_asserted: bool, + + pins: Vec>, + signals: Vec, + } + impl OrGateDriver { + // Returns a new OrGateDriver with all pins deasserted. + fn new() -> Self { + Self::new_inner(false) + } + + // Returns a new OrGateDriver with all pins asserted. + fn new_asserted() -> Self { + Self::new_inner(true) + } + + fn new_inner(start_asserted: bool) -> Self { + let output = Arc::new(TestPin::new()); + Self { + output: output.clone(), + gate: OrGate::new(output.clone()), + start_asserted, + pins: vec![], + signals: vec![], + } + } + + fn add_signal(&mut self, f: PinSignal) { + let pin = self.gate.new_input(); + self.pins.push(pin); + self.signals.push(f); + } + + fn run(&self, ticks: usize) { + // The last thread to arrive at a Barrier wakes the other threads + // and proceeds without locking. This difference can make timing + // slightly different between inputs. + // + // The two Barriers and AtomicBool below are used to achieve a more + // consistent timing. Threads are controlled by a driver loop that + // runs in the main thread. + // + // Pin threads first block on the `start` `Barrier` and then + // spinlock on the `go` `AtomicBool` until the driver flips its + // value. This ensures all threads are awake and ready to poke at + // their pins. + // + // The driver then waits for all threads to complete before + // flipping `go` again for the next tick. + let start = Arc::new(Barrier::new(self.pins.len() + 1)); + let done = Arc::new(Barrier::new(self.pins.len() + 1)); + let go = Arc::new(AtomicBool::new(false)); + + let threads = self + .pins + .iter() + .enumerate() + .map(|(pin_idx, pin)| { + if self.start_asserted { + pin.import_state(true); + } + + let start = start.clone(); + let done = done.clone(); + let go = go.clone(); + let pin = pin.clone(); + let f = self.signals[pin_idx].clone(); + + thread::spawn(move || { + for i in 0..ticks { + // Live-wait for all other threads and the Go! + // signal from the driver. + start.wait(); + while !go.load(Ordering::Acquire) { + hint::spin_loop(); + } + + f(&pin, i); + + // Block until everyone is ready. + done.wait(); + } + }) + }) + .collect::>(); + + for _ in 0..ticks { + go.store(false, Ordering::Release); + start.wait(); + go.store(true, Ordering::Release); + done.wait(); + } + + threads.into_iter().for_each(|t| t.join().unwrap()); + } + } + + #[test] + fn test_or_gate() { + let output_pin = Arc::new(TestPin::new()); + let or_gate = OrGate::new(output_pin.clone()); + + let pin1 = or_gate.new_input(); + let pin2 = or_gate.new_input(); + let pin3 = or_gate.new_input(); + + // Assert pin1. + // Expect gate output to go high. + pin1.assert(); + assert!(pin1.is_asserted()); + assert!(output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 1); + assert_eq!(output_pin.falls(), 0); + + // Assert pin2. + // Expect gate output to remain high and not re-assert. + pin2.assert(); + assert!(pin2.is_asserted()); + assert!(output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 1); + assert_eq!(output_pin.falls(), 0); + + // Deassert pin1. + // Expect gate output to remain high and not re-assert. + pin1.deassert(); + assert!(!pin1.is_asserted()); + assert!(output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 1); + assert_eq!(output_pin.falls(), 0); + + // Deassert pin2. + // Expect gate output to go low. + pin2.deassert(); + assert!(!pin2.is_asserted()); + assert!(!output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 1); + assert_eq!(output_pin.falls(), 1); + + // Deassert already deasserted pin3 while gate output is low. + // Expect no-op. + pin3.deassert(); + assert!(!pin3.is_asserted()); + assert!(!output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 1); + assert_eq!(output_pin.falls(), 1); + } + + #[test] + fn test_or_gate_pulse() { + let output_pin = Arc::new(TestPin::new()); + let or_gate = OrGate::new(output_pin.clone()); + + let pin1 = or_gate.new_input(); + let pin2 = or_gate.new_input(); + + // Pulse while gate output is low. + // Expect output pulse. + pin1.pulse(); + assert!(!output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 1); + assert_eq!(output_pin.falls(), 1); + assert_eq!(output_pin.pulses(), 1); + + // Pulse while gate output is high. + // Expect no-op. + pin1.assert(); + pin2.pulse(); + assert!(output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 2); + assert_eq!(output_pin.falls(), 1); + assert_eq!(output_pin.pulses(), 1); + + // Pulse pin that is already high. + // Expect no-op. + assert!(pin1.is_asserted()); + pin1.pulse(); + assert!(output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 2); + assert_eq!(output_pin.falls(), 1); + assert_eq!(output_pin.pulses(), 1); + } + + #[test] + fn test_or_gate_drop_input() { + let output_pin = Arc::new(TestPin::new()); + let or_gate = OrGate::new(output_pin.clone()); + + let pin1 = or_gate.new_input(); + let pin2 = or_gate.new_input(); + let pin3 = or_gate.new_input(); + + // Assert two of the inputs. + pin1.assert(); + pin2.assert(); + assert!(output_pin.is_asserted()); + + // Drop one asserted input. + // Expect gate output to remain high. + drop(pin1); + assert!(output_pin.is_asserted()); + + // Drop the other asserted input. + // Expect gate output to go low. + drop(pin2); + assert!(!output_pin.is_asserted()); + + // Drop last input. + // Expect gate output to remain unchanged. + drop(pin3); + assert!(!output_pin.is_asserted()); + } + + #[test] + fn test_or_gate_import_state() { + let output_pin = Arc::new(TestPin::new()); + let or_gate = OrGate::new(output_pin.clone()); + + let pin1 = or_gate.new_input(); + let pin2 = or_gate.new_input(); + let pin3 = or_gate.new_input(); + + // Import pin states. + // Expect output state to change without an edge. + pin1.import_state(false); + assert!(!output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 0); + + pin2.import_state(true); + assert!(output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 0); + + pin3.import_state(false); + assert!(output_pin.is_asserted()); + assert_eq!(output_pin.rises(), 0); + } + + #[test] + fn test_or_gate_concurrent_assert() { + let mut driver = OrGateDriver::new(); + + // Assert all pins concurrently. + // Expect output to go high just once. + (0..10).for_each(|_| { + driver.add_signal(Arc::new(move |pin, _| { + pin.assert(); + })); + }); + driver.run(100); + + assert!(driver.output.is_asserted()); + assert_eq!(driver.output.rises(), 1); + assert_eq!(driver.output.falls(), 0); + } + + #[test] + fn test_or_gate_concurrent_deassert() { + let mut driver = OrGateDriver::new_asserted(); + + // Deassert all pins concurrently. + // Expect output to go low just once. + (0..10).for_each(|_| { + driver.add_signal(Arc::new(move |pin, _| { + pin.deassert(); + })); + }); + driver.run(100); + + assert!(!driver.output.is_asserted()); + assert_eq!(driver.output.rises(), 0); + assert_eq!(driver.output.falls(), 1); + } + + #[test] + fn test_or_gate_concurrent_pulse() { + let mut driver = OrGateDriver::new(); + + // Pulse all pins concurrently. + // Expect all pulses to be observed by the output. + (0..10).for_each(|_| { + driver.add_signal(Arc::new(move |pin, _| { + pin.pulse(); + })); + }); + driver.run(100); + + assert!(!driver.output.is_asserted()); + assert_eq!(driver.output.pulses(), 1000); + } + + // Assert and deassert two pins with square waves in lockstep. + // + // Expect output to match the input square wave. The thread that first + // transition its pin will drive the output while the other one will be a + // no-op. + // + // ┌─┐ ┌─┐ ┌─┐ ┌─┐ + // pin1 ─┘ └─┘ └─┘ └─┘ └─ + // ┌─┐ ┌─┐ ┌─┐ ┌─┐ + // pin2 ─┘ └─┘ └─┘ └─┘ └─ + // ┌─┐ ┌─┐ ┌─┐ ┌─┐ + // output ─┘ └─┘ └─┘ └─┘ └─ + #[test] + fn test_or_gate_concurrent_square_wave() { + let mut driver = OrGateDriver::new(); + driver.add_signal(Arc::new(move |pin, i| { + if i % 2 == 0 { + pin.assert(); + } else { + pin.deassert(); + } + })); + driver.add_signal(Arc::new(move |pin, i| { + if i % 2 == 0 { + pin.assert(); + } else { + pin.deassert(); + } + })); + driver.run(100); + + assert!(!driver.output.is_asserted()); + assert_eq!(driver.output.rises(), 50); + assert_eq!(driver.output.falls(), 50); + } + + // Assert and deassert a pin with a square wave while pulsing another pin. + // + // There are four possible outcomes on each tick: + // 1. pin1 asserts _after_ pin2 pulses => pulsed while low, output pulses. + // 2. pin1 deasserts _after_ pin2 pulses => pulsed while high, no-op. + // 3. pin1 asserts _before_ pin2 pulses => pulsed while high, no-op. + // 4. pin1 deasserts _before_ pin2 pulses => pulsed while low, output pulses. + // + // ┌─┐ ┌─┐ + // pin1 ─┘ └─┘ └── + // ▲ ▲ ▲ ▲ + // pin2 ─┴─┴─┴─┴── + // ▲┌┐ ┌─┐▲ + // output ─┴┘└─┘ └┴─ + // + // In theory, we should expect about half the pulses to cause the output to + // also pulse but, in practice, the threads will race to lock the gate, so + // the actual pulse count is non-deterministic and flaky to assert. + // + // Expect to at least not have any spurious edges or pulses. + #[test] + fn test_or_gate_concurrent_pulse_with_square_wave() { + let mut driver = OrGateDriver::new(); + driver.add_signal(Arc::new(move |pin, i| { + if i % 2 == 0 { + pin.assert(); + } else { + pin.deassert(); + } + })); + driver.add_signal(Arc::new(move |pin, _| { + pin.pulse(); + })); + driver.run(100); + + assert!(!driver.output.is_asserted()); + assert_eq!(driver.output.rises(), driver.output.falls()); + assert!(driver.output.pulses() <= 100); + } +} From bfd5354bfc111b403111cb6a7d22edeb88d9d28e Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Mon, 19 Oct 2026 22:33:30 +0000 Subject: [PATCH 2/4] acpi: move SCI gate to new Acpi struct When working on more ACPI features, I realized that it would make more sense for an Acpi struct to own the SCI OrGate. The Acpi struct will hold and manage ACPI registers, so it should the one controlling inputs to the SCI gate. --- bin/propolis-server/src/lib/initializer.rs | 7 +++-- bin/propolis-standalone/src/main.rs | 11 ++++---- lib/propolis/src/firmware/acpi/mod.rs | 22 +++++++++++++++ lib/propolis/src/hw/chipset/i440fx.rs | 31 +++++++--------------- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/bin/propolis-server/src/lib/initializer.rs b/bin/propolis-server/src/lib/initializer.rs index 0da580375..b85abe9d0 100644 --- a/bin/propolis-server/src/lib/initializer.rs +++ b/bin/propolis-server/src/lib/initializer.rs @@ -341,6 +341,7 @@ impl MachineInitializer<'_> { } }), )); + let sci_pin = Arc::new(i440fx::LNKPin::new()); let chipset_hb = i440fx::I440FxHostBridge::create( pci_topology, @@ -350,8 +351,10 @@ impl MachineInitializer<'_> { enable_pcie: i440fx.enable_pcie, }, ); - let chipset_lpc = - i440fx::Piix3Lpc::create(self.machine.hdl.clone()); + let chipset_lpc = i440fx::Piix3Lpc::create( + self.machine.hdl.clone(), + sci_pin.clone(), + ); let chipset_pm = i440fx::Piix3PM::create( self.machine.hdl.clone(), diff --git a/bin/propolis-standalone/src/main.rs b/bin/propolis-standalone/src/main.rs index 57c290331..d744eb742 100644 --- a/bin/propolis-standalone/src/main.rs +++ b/bin/propolis-standalone/src/main.rs @@ -26,7 +26,7 @@ use propolis::chardev::{BlockingSource, Sink, Source, UDSock}; use propolis::common::{DeviceMetadataMap, GB, MB}; use propolis::enlightenment::{bhyve, hyperv, Enlightenment}; use propolis::firmware::{acpi, smbios}; -use propolis::hw::chipset::{i440fx, Chipset}; +use propolis::hw::chipset::{i440fx, i440fx::LNKPin, Chipset}; use propolis::hw::ps2::ctrl::PS2Ctrl; use propolis::hw::qemu::fwcfg; use propolis::hw::uart::{LpcUart, LpcUartMetadata}; @@ -740,7 +740,7 @@ impl Instance { } } - fn generate_pins(&self) -> (Arc, Arc) { + fn generate_pins(&self) -> (Arc, Arc, Arc) { let power_eq = self.0.eq.clone(); let power_pin = propolis::intr_pins::FuncPin::new(Box::new(move |rising| { @@ -761,7 +761,7 @@ impl Instance { ); } })); - (Arc::new(power_pin), Arc::new(reset_pin)) + (Arc::new(power_pin), Arc::new(reset_pin), Arc::new(LNKPin::new())) } fn lock(&self) -> Option> { @@ -1214,7 +1214,7 @@ fn setup_instance( guard.inventory.register_instance(vcpu, &vcpu.id.to_string()) } - let (power_pin, reset_pin) = inst.generate_pins(); + let (power_pin, reset_pin, sci_pin) = inst.generate_pins(); let pci_topo = propolis::hw::pci::topology::Builder::new().finish(machine)?.topology; @@ -1226,7 +1226,8 @@ fn setup_instance( ..Default::default() }, ); - let chipset_lpc = i440fx::Piix3Lpc::create(machine.hdl.clone()); + let chipset_lpc = + i440fx::Piix3Lpc::create(machine.hdl.clone(), sci_pin.clone()); let chipset_pm = i440fx::Piix3PM::create( machine.hdl.clone(), chipset_hb.power_pin(), diff --git a/lib/propolis/src/firmware/acpi/mod.rs b/lib/propolis/src/firmware/acpi/mod.rs index 5130f059a..e0d702d1c 100644 --- a/lib/propolis/src/firmware/acpi/mod.rs +++ b/lib/propolis/src/firmware/acpi/mod.rs @@ -18,9 +18,12 @@ //! //! +use std::sync::Arc; + use serde::{Deserialize, Serialize}; use crate::hw::chipset::i440fx; +use crate::intr_pins::{IntrPin, OrGate}; pub mod aml; pub mod dsdt; @@ -89,3 +92,22 @@ const LOCAL_APIC_LEN: u32 = 0x10_0000; // information. const GPE0_BLK_ADDR: u16 = 0xafe0; const GPE0_BLK_LEN: u8 = 4; + +pub struct Acpi { + // The SCI state can be driven by multiple inputs that OR'ed to determine + // if an interrupt should happen, so model it as an OrGate and hand out new + // inputs for components that connect to the SCI pin. + // + // Refer to Fig. 4.3 of the ACPI spec. v6.6 for more information. + sci_gate: Arc, +} + +impl Acpi { + pub fn create(sci_pin: Arc) -> Arc { + Arc::new(Self { sci_gate: OrGate::new(sci_pin) }) + } + + pub fn sci_pin(&self) -> Arc { + self.sci_gate.new_input() + } +} diff --git a/lib/propolis/src/hw/chipset/i440fx.rs b/lib/propolis/src/hw/chipset/i440fx.rs index 478047b6d..e31ea6f60 100644 --- a/lib/propolis/src/hw/chipset/i440fx.rs +++ b/lib/propolis/src/hw/chipset/i440fx.rs @@ -20,7 +20,7 @@ use crate::hw::pci::topology::{LogicalBusId, RoutedBusId}; use crate::hw::pci::{ self, Bdf, INTxPinID, LintrCfg, PcieCfgDecoder, PioCfgDecoder, }; -use crate::intr_pins::{IntrPin, LegacyPIC, LegacyPin, NoOpPin, OrGate}; +use crate::intr_pins::{IntrPin, LegacyPIC, LegacyPin, NoOpPin}; use crate::lifecycle; use crate::migrate::*; use crate::mmio::MmioFn; @@ -37,7 +37,7 @@ pub const DEFAULT_HB_BDF: Bdf = Bdf::new_unchecked(0, 0, 0); pub const DEFAULT_LPC_BDF: Bdf = Bdf::new_unchecked(0, 1, 0); pub const DEFAULT_PM_BDF: Bdf = Bdf::new_unchecked(0, 1, 3); -struct LNKPin { +pub struct LNKPin { inner: Mutex, } struct LNKPinInner { @@ -45,7 +45,7 @@ struct LNKPinInner { pin: Option, } impl LNKPin { - fn new() -> Self { + pub fn new() -> Self { Self { inner: Mutex::new(LNKPinInner { asserted: false, pin: None }) } } fn reassign(&self, new_pin: Option) { @@ -101,18 +101,10 @@ impl IntrPin for LNKPin { struct IrqConfig { pic: Arc, lnk_pins: [Arc; 4], - - // The SCI state can be driven by multiple inputs that OR'ed to determine - // if an interrupt should happen, so model it as an OrGate and hand out new - // inputs for components that connect to the SCI pin. - // - // Refer to Fig. 4.3 of the ACPI spec. v6.6 for more information. - sci_pin: Arc, } impl IrqConfig { - fn create(hdl: Arc) -> Arc { + fn create(hdl: Arc, sci_pin: Arc) -> Arc { let pic = LegacyPIC::new(hdl); - let sci_pin = Arc::new(LNKPin::new()); sci_pin.reassign(pic.pin_handle(SCI_IRQ)); Arc::new(Self { pic, @@ -122,7 +114,6 @@ impl IrqConfig { Arc::new(LNKPin::new()), Arc::new(LNKPin::new()), ], - sci_pin: OrGate::new(sci_pin), }) } fn set_lnk_route(&self, idx: usize, irq: Option) { @@ -359,7 +350,7 @@ pub struct Piix3Lpc { post_code: AtomicU8, } impl Piix3Lpc { - pub fn create(hdl: Arc) -> Arc { + pub fn create(hdl: Arc, sci_pin: Arc) -> Arc { let pci_state = pci::Builder::new(pci::Ident { vendor_id: VENDOR_INTEL, device_id: PIIX3_ISA_DEV_ID, @@ -372,7 +363,7 @@ impl Piix3Lpc { .add_custom_cfg(PIR_OFFSET as u8, PIR_LEN as u8) .finish(); - let irq_config = IrqConfig::create(hdl.clone()); + let irq_config = IrqConfig::create(hdl.clone(), sci_pin); Arc::new(Self { pci_state, @@ -388,10 +379,6 @@ impl Piix3Lpc { }) } - pub fn sci_pin(&self) -> Arc { - self.irq_config.sci_pin.new_input() - } - pub fn attach(self: &Arc, pio: &PioBus) { let this = Arc::clone(self); let piofn = Arc::new(move |port: u16, rwo: RWOp| this.pio_rw(port, rwo)) @@ -1167,8 +1154,9 @@ mod test { fn lpc_pci_cfg_read() { let hdl = Arc::new(VmmHdl::new_test(0).unwrap()); let scaffold = Scaffold::new(); + let sci_pin = Arc::new(LNKPin::new()); - let lpc = Piix3Lpc::create(hdl); + let lpc = Piix3Lpc::create(hdl, sci_pin); let _bus = setup_attach(&scaffold, lpc.clone()); cfg_read(lpc.as_ref() as &dyn Endpoint); @@ -1178,8 +1166,9 @@ mod test { fn lpc_pci_cfg_write() { let hdl = Arc::new(VmmHdl::new_test(0).unwrap()); let scaffold = Scaffold::new(); + let sci_pin = Arc::new(LNKPin::new()); - let lpc = Piix3Lpc::create(hdl); + let lpc = Piix3Lpc::create(hdl, sci_pin); let _bus = setup_attach(&scaffold, lpc.clone()); cfg_write(lpc.as_ref() as &dyn Endpoint); From c331fb670fc5053e4c2f3e0c743314b9e77096b1 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Tue, 20 Oct 2026 14:44:35 +0000 Subject: [PATCH 3/4] acpi: store OrGate inputs in u64 --- lib/propolis/src/intr_pins.rs | 109 +++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 36 deletions(-) diff --git a/lib/propolis/src/intr_pins.rs b/lib/propolis/src/intr_pins.rs index 7b2300e57..efaa47e15 100644 --- a/lib/propolis/src/intr_pins.rs +++ b/lib/propolis/src/intr_pins.rs @@ -253,86 +253,123 @@ struct FPInner { /// overly-protective of concurrent operations. It would be possible to allow /// inputs to change independently, but we would need to be careful to also /// synchronize updates to the output pin. -/// -/// Internal state is stored in a `>` to make the code easier to read -/// and maintain, but it would be possible to use something like a -/// [std::sync::atomic::AtomicU64] bitmask instead. pub struct OrGate { - inputs: Mutex>, - output: Arc, + inner: Mutex, } impl OrGate { pub fn new(pin: Arc) -> Arc { - Arc::new(Self { output: pin, inputs: Mutex::new(vec![]) }) + Arc::new(Self { inner: Mutex::new(OrGateInner::new(pin)) }) } // The gate is expected to be constructed once during VM initialization, so // inputs are currently never removed. pub fn new_input(self: &Arc) -> Arc { - let mut inputs = self.inputs.lock().unwrap(); - let pin = Arc::new(OrGatePin { idx: inputs.len(), gate: self.clone() }); - inputs.push(false); - pin + let mut inner = self.inner.lock().unwrap(); + + let idx = inner.idx; + inner.idx += 1; + Arc::new(OrGatePin { mask: 1 << idx, gate: self.clone() }) } - fn set_input_state(&self, input: usize, is_asserted: bool) { - let mut inputs = self.inputs.lock().unwrap(); - inputs[input] = is_asserted; + fn set_input_state(&self, mask: u64, is_asserted: bool) { + let mut inner = self.inner.lock().unwrap(); + + if is_asserted { + inner.inputs.assert(mask) + } else { + inner.inputs.deassert(mask) + }; // Only modify the output if the gate state actually changed to avoid // spurious edges. - let should_assert = OrGate::is_asserted(&inputs); - if should_assert != self.output.is_asserted() { - self.output.set_state(should_assert); + let should_assert = inner.inputs.is_asserted(); + if should_assert != inner.output.is_asserted() { + inner.output.set_state(should_assert); } } fn pulse_input(&self) { - let inputs = self.inputs.lock().unwrap(); - if !OrGate::is_asserted(&inputs) { - self.output.pulse(); + let inner = self.inner.lock().unwrap(); + if !inner.inputs.is_asserted() { + inner.output.pulse(); } } - fn is_input_asserted(&self, input: usize) -> bool { - let inputs = self.inputs.lock().unwrap(); - inputs[input] + fn is_input_asserted(&self, mask: u64) -> bool { + let inner = self.inner.lock().unwrap(); + inner.inputs.is_input_asserted(mask) } - fn import_input_state(&self, input: usize, is_asserted: bool) { - let mut inputs = self.inputs.lock().unwrap(); - inputs[input] = is_asserted; + fn import_input_state(&self, mask: u64, is_asserted: bool) { + let mut inner = self.inner.lock().unwrap(); + + if is_asserted { + inner.inputs.assert(mask) + } else { + inner.inputs.deassert(mask) + }; - let should_assert = OrGate::is_asserted(&inputs); - if should_assert != self.output.is_asserted() { - self.output.import_state(should_assert); + let should_assert = inner.inputs.is_asserted(); + if should_assert != inner.output.is_asserted() { + inner.output.import_state(should_assert); } } +} - fn is_asserted(inputs: &[bool]) -> bool { - inputs.contains(&true) +struct OrGateInner { + idx: u8, + inputs: OrGateInputs, + output: Arc, +} +impl OrGateInner { + fn new(pin: Arc) -> Self { + Self { idx: 0, inputs: OrGateInputs::new(), output: pin } + } +} + +#[derive(Copy, Clone)] +struct OrGateInputs(u64); +impl OrGateInputs { + fn new() -> Self { + Self(0) + } + #[inline] + fn is_asserted(self) -> bool { + self.0 != 0 + } + #[inline] + fn is_input_asserted(self, mask: u64) -> bool { + self.0 & mask != 0 + } + #[inline] + fn assert(&mut self, mask: u64) { + self.0 |= mask; + } + #[inline] + fn deassert(&mut self, mask: u64) { + self.0 &= !mask; } } pub struct OrGatePin { - idx: usize, + mask: u64, gate: Arc, } impl IntrPin for OrGatePin { fn assert(&self) { - self.gate.set_input_state(self.idx, true); + self.gate.set_input_state(self.mask, true); } fn deassert(&self) { - self.gate.set_input_state(self.idx, false); + self.gate.set_input_state(self.mask, false); } fn pulse(&self) { self.gate.pulse_input(); } fn is_asserted(&self) -> bool { - self.gate.is_input_asserted(self.idx) + self.gate.is_input_asserted(self.mask) } fn import_state(&self, is_asserted: bool) { - self.gate.import_input_state(self.idx, is_asserted) + self.gate.import_input_state(self.mask, is_asserted) } } impl Drop for OrGatePin { From 7f7d095e23864b8ed152d3785f101c18785bd1dd Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Tue, 20 Oct 2026 14:56:56 +0000 Subject: [PATCH 4/4] acpi: guard max number of OrGate inputs --- lib/propolis/src/intr_pins.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/propolis/src/intr_pins.rs b/lib/propolis/src/intr_pins.rs index efaa47e15..06a34e65b 100644 --- a/lib/propolis/src/intr_pins.rs +++ b/lib/propolis/src/intr_pins.rs @@ -261,12 +261,12 @@ impl OrGate { Arc::new(Self { inner: Mutex::new(OrGateInner::new(pin)) }) } - // The gate is expected to be constructed once during VM initialization, so - // inputs are currently never removed. pub fn new_input(self: &Arc) -> Arc { let mut inner = self.inner.lock().unwrap(); let idx = inner.idx; + assert!(idx < 64, "max number of inputs reached"); + inner.idx += 1; Arc::new(OrGatePin { mask: 1 << idx, gate: self.clone() }) } @@ -856,4 +856,14 @@ pub mod test { assert_eq!(driver.output.rises(), driver.output.falls()); assert!(driver.output.pulses() <= 100); } + + #[test] + #[should_panic(expected = "max number of inputs reached")] + fn panic_or_gate_max_inputs() { + let output_pin = Arc::new(TestPin::new()); + let or_gate = OrGate::new(output_pin.clone()); + for _ in 0..100 { + _ = or_gate.new_input(); + } + } }