diff --git a/Cargo.lock b/Cargo.lock index d30c2f3930..aea4390d5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1269,6 +1269,7 @@ dependencies = [ "i2c-interface", "libredox", "log", + "pcid", "redox-scheme", "redox_syscall 0.9.0+rb0.3.1", "ron", diff --git a/drivers/i2c/designware/src/lib.rs b/drivers/i2c/designware/src/lib.rs index 5bc22db706..c90bcb775b 100644 --- a/drivers/i2c/designware/src/lib.rs +++ b/drivers/i2c/designware/src/lib.rs @@ -3,10 +3,11 @@ //! Ported from Linux 7.1 `drivers/i2c/busses/i2c-designware-master.c` and //! `i2c-designware-common.c` (register model in `i2c-designware-core.h`). //! Polling variant: transfers complete by polling IC_STATUS with bounded -//! timeouts instead of interrupt-driven completion, the same approach used -//! by the U-Boot DesignWare driver. Transfer semantics (RESTART/STOP -//! sequencing, TX_ABRT error decode, 7/10-bit addressing) follow the Linux -//! master driver. +//! deadlines instead of interrupt-driven completion, the same approach +//! used by the U-Boot DesignWare driver. Every transaction follows the +//! Linux `i2c_dw_xfer_init` ordering: wait for bus idle, disable the +//! adapter, program IC_CON/IC_TAR, re-enable, then transfer — the +//! DesignWare databook forbids IC_CON/IC_TAR writes while enabled. use std::fmt; use std::time::{Duration, Instant}; @@ -27,6 +28,7 @@ const DW_IC_ENABLE: usize = 0x6c; const DW_IC_STATUS: usize = 0x70; const DW_IC_SDA_HOLD: usize = 0x7c; const DW_IC_TX_ABRT_SOURCE: usize = 0x80; +const DW_IC_ENABLE_STATUS: usize = 0x9c; const DW_IC_COMP_VERSION: usize = 0xf8; // IC_CON bits (i2c-designware-core.h). @@ -48,8 +50,10 @@ const IC_STATUS_TFE: u32 = 1 << 2; const IC_STATUS_RFNE: u32 = 1 << 3; const IC_STATUS_MASTER_ACTIVITY: u32 = 1 << 5; -// IC_RAW_INTR_STAT bit for transfer abort. +// IC_RAW_INTR_STAT / IC_ENABLE bits. const IC_RAW_TX_ABRT: u32 = 1 << 6; +const IC_ENABLE_ENABLE: u32 = 1 << 0; +const IC_ENABLE_ABORT: u32 = 1 << 1; // IC_TX_ABRT_SOURCE codes (i2c-designware-core.h). const ABRT_7B_ADDR_NOACK: u32 = 1 << 0; @@ -75,6 +79,17 @@ const DW_IC_TAR_10BITADDR_MASTER: u32 = 1 << 12; const BYTE_WAIT_TIMEOUT: Duration = Duration::from_millis(50); /// Polling budget for whole-transfer completion (FIFO drain + bus idle). const COMPLETION_TIMEOUT: Duration = Duration::from_millis(250); +/// Enable/disable acknowledgement budget. Linux uses up to 100 rounds of a +/// 25 µs signaling-period wait (~2.5 ms at 400 kHz); 10 ms is safer here. +const ENABLE_STATUS_TIMEOUT: Duration = Duration::from_millis(10); +/// ABORT bit self-clearing budget (Linux: 10 × 1 ms). +const ABORT_TIMEOUT: Duration = Duration::from_millis(100); + +/// Hard limits for incoming requests — the bus is slow and small by design, +/// and unbounded requests would OOM or monopolize the daemon. +pub const MAX_SEGMENTS: usize = 16; +pub const MAX_SEGMENT_BYTES: usize = 4096; +pub const MAX_TOTAL_BYTES: usize = 16384; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum DwError { @@ -84,10 +99,13 @@ pub enum DwError { ArbitrationLost, /// Hardware abort that is not a simple NACK. Abort(u32), - /// Polling timeout waiting for FIFO space, data, or bus idle. + /// Polling deadline expired waiting for FIFO space, data, bus idle, + /// or an enable/abort acknowledgement. Timeout, /// Controller reported MASTER_DIS abort (master mode disabled). MasterDisabled, + /// Request violated engine limits or used an unsupported mode. + Invalid(&'static str), } impl fmt::Display for DwError { @@ -98,6 +116,7 @@ impl fmt::Display for DwError { DwError::Abort(src) => write!(f, "transfer abort (IC_TX_ABRT_SOURCE={src:#x})"), DwError::Timeout => write!(f, "transfer timeout"), DwError::MasterDisabled => write!(f, "master mode disabled"), + DwError::Invalid(reason) => write!(f, "invalid request: {reason}"), } } } @@ -111,8 +130,6 @@ pub struct SclTiming { pub hcnt: u32, pub lcnt: u32, pub sda_hold_tx: u32, - pub sda_fall_ns: u32, - pub scl_fall_ns: u32, } /// Bus timing inputs in the same shape Linux's intel-lpss-pci.c carries them @@ -137,43 +154,33 @@ pub const BXT_I2C_TIMING: BusTiming = BusTiming { }; impl BusTiming { - /// Fast-mode (400 kHz) SCL counts. Formulas from - /// `i2c-designware-common.c:i2c_dw_scl_hcnt/lcnt`: - /// hcnt = IC_CLK * (tHD;STA + tf) - 3 - /// lcnt = IC_CLK * (tLOW + tf) - 1 - /// with tHD;STA = 0.6 us and tLOW = 1.3 us (I2C fast-mode spec). - pub fn fast_mode(&self) -> SclTiming { - let clk_khz = self.ic_clk_hz / 1000; - let hcnt = (clk_khz as u64 * (600 + self.scl_fall_ns as u64) / 1_000_000) - .saturating_sub(3) as u32; - let lcnt = (clk_khz as u64 * (1300 + self.scl_fall_ns as u64) / 1_000_000) - .saturating_sub(1) as u32; + fn scl_counts(&self, t_symbol_ns: u64, t_low_ns: u64) -> SclTiming { + let ic_clk_khz = self.ic_clk_hz as u64 / 1000; + // i2c_dw_scl_hcnt: IC_CLK * (tHD;STA + sda_fall) rounded, minus 3. + // i2c_dw_scl_lcnt: IC_CLK * (tLOW + scl_fall) rounded, minus 1. + let hcnt = (ic_clk_khz * (t_symbol_ns + self.sda_fall_ns as u64) + 500_000) + / 1_000_000 + - 3; + let lcnt = (ic_clk_khz * (t_low_ns + self.scl_fall_ns as u64) + 500_000) + / 1_000_000 + - 1; // IC_SDA_HOLD TX value is in IC_CLK cycles (datasheet §5.11). - let sda_hold_tx = (clk_khz as u64 * self.sda_hold_ns as u64).div_ceil(1_000_000) as u32; + let sda_hold_tx = (ic_clk_khz * self.sda_hold_ns as u64).div_ceil(1_000_000) as u32; SclTiming { - hcnt, - lcnt, + hcnt: hcnt as u32, + lcnt: lcnt as u32, sda_hold_tx, - sda_fall_ns: self.sda_fall_ns, - scl_fall_ns: self.scl_fall_ns, } } - /// Standard-mode (100 kHz) SCL counts: tHD;STA = 4.0 us, tLOW = 4.7 us. + /// Fast-mode (400 kHz) counts: tHD;STA = 0.6 µs, tLOW = 1.3 µs. + pub fn fast_mode(&self) -> SclTiming { + self.scl_counts(600, 1300) + } + + /// Standard-mode (100 kHz) counts: tHD;STA = 4.0 µs, tLOW = 4.7 µs. pub fn standard_mode(&self) -> SclTiming { - let clk_khz = self.ic_clk_hz / 1000; - let hcnt = (clk_khz as u64 * (4000 + self.scl_fall_ns as u64) / 1_000_000) - .saturating_sub(3) as u32; - let lcnt = (clk_khz as u64 * (4700 + self.scl_fall_ns as u64) / 1_000_000) - .saturating_sub(1) as u32; - let sda_hold_tx = (clk_khz as u64 * self.sda_hold_ns as u64).div_ceil(1_000_000) as u32; - SclTiming { - hcnt, - lcnt, - sda_hold_tx, - sda_fall_ns: self.sda_fall_ns, - scl_fall_ns: self.scl_fall_ns, - } + self.scl_counts(4000, 4700) } } @@ -184,6 +191,62 @@ pub enum Segment { Read(usize), } +/// Validated, hardware-independent view of one transfer request. +pub struct ValidatedRequest { + pub address: u16, + pub ten_bit: bool, + pub segments: Vec, +} + +/// Validate request shape and engine limits before any MMIO happens. +pub fn validate_request( + address: u16, + ten_bit: bool, + segments: &[Segment], + supports_10bit: bool, +) -> Result { + if segments.is_empty() { + return Err(DwError::Invalid("no segments")); + } + if segments.len() > MAX_SEGMENTS { + return Err(DwError::Invalid("too many segments")); + } + if ten_bit { + if !supports_10bit { + return Err(DwError::Invalid("10-bit addressing not supported by adapter")); + } + if address > 0x3ff { + return Err(DwError::Invalid("10-bit address out of range")); + } + } else if address > 0x7f { + return Err(DwError::Invalid("7-bit address out of range")); + } + + let mut total = 0usize; + for segment in segments { + let len = match segment { + Segment::Write(data) => data.len(), + Segment::Read(len) => *len, + }; + if len == 0 { + return Err(DwError::Invalid("zero-length segment")); + } + if len > MAX_SEGMENT_BYTES { + return Err(DwError::Invalid("segment too large")); + } + total += len; + if total > MAX_TOTAL_BYTES { + return Err(DwError::Invalid("transfer too large")); + } + } + + Ok(ValidatedRequest { + address, + ten_bit, + segments: segments.to_vec(), + }) +} + /// DesignWare master engine over a mapped MMIO register block. pub struct DwI2c { base: *mut u8, @@ -201,6 +264,14 @@ impl DwI2c { Self { base } } + /// Base of the mapped register block. Callers use it to reach + /// vendor-specific registers outside the DesignWare window (e.g. the + /// Intel LPSS private block at +0x200) after validating the real + /// mapping length themselves. + pub fn mmio_base(&self) -> *mut u8 { + self.base + } + fn read32(&self, reg: usize) -> u32 { unsafe { (self.base.add(reg) as *const u32).read_volatile() } } @@ -209,12 +280,65 @@ impl DwI2c { unsafe { (self.base.add(reg) as *mut u32).write_volatile(value) } } - fn enabled(&self) -> bool { - self.read32(DW_IC_ENABLE) & 1 != 0 + fn sleep_poll() { + // Linux polls enable/abort status with 25 µs signaling-period waits; + // short sleeps keep the daemon from burning a timeslice per byte. + std::thread::sleep(Duration::from_micros(25)); } - fn set_enabled(&self, enable: bool) { - self.write32(DW_IC_ENABLE, enable as u32); + /// Write ENABLE=0 and wait for IC_ENABLE_STATUS to deassert + /// (`__i2c_dw_disable` minus the abort branch, which `recover()` owns). + fn disable(&self) -> Result<(), DwError> { + let start = Instant::now(); + loop { + self.write32(DW_IC_ENABLE, 0); + if self.read32(DW_IC_ENABLE_STATUS) & 1 == 0 { + return Ok(()); + } + if start.elapsed() > ENABLE_STATUS_TIMEOUT { + return Err(DwError::Timeout); + } + Self::sleep_poll(); + } + } + + fn enable(&self) -> Result<(), DwError> { + let start = Instant::now(); + self.write32(DW_IC_ENABLE, IC_ENABLE_ENABLE); + loop { + if self.read32(DW_IC_ENABLE_STATUS) & 1 != 0 { + return Ok(()); + } + if start.elapsed() > ENABLE_STATUS_TIMEOUT { + return Err(DwError::Timeout); + } + Self::sleep_poll(); + } + } + + /// Recover the controller after an abort or timeout: set the ABORT bit + /// and wait for it to self-clear (Linux `__i2c_dw_disable` abort branch), + /// then disable and flush stale state. Best-effort by design — the next + /// transfer re-initializes target state anyway. + fn recover(&self) { + let enabled = self.read32(DW_IC_ENABLE) & IC_ENABLE_ENABLE != 0; + if !enabled { + self.write32(DW_IC_ENABLE, IC_ENABLE_ENABLE); + std::thread::sleep(Duration::from_micros(25)); + } + self.write32(DW_IC_ENABLE, IC_ENABLE_ENABLE | IC_ENABLE_ABORT); + + let start = Instant::now(); + while self.read32(DW_IC_ENABLE) & IC_ENABLE_ABORT != 0 { + if start.elapsed() > ABORT_TIMEOUT { + break; + } + Self::sleep_poll(); + } + + let _ = self.disable(); + let _ = self.read32(DW_IC_CLR_INTR); + let _ = self.read32(DW_IC_CLR_TX_ABRT); } /// Initialize the controller for master-mode operation. @@ -227,7 +351,7 @@ impl DwI2c { let fast = timing.fast_mode(); let standard = timing.standard_mode(); - self.set_enabled(false); + let _ = self.disable(); let con = IC_CON_MASTER | IC_CON_SPEED_FAST @@ -251,28 +375,35 @@ impl DwI2c { self.write32(DW_IC_SDA_HOLD, hold); } - // Polling driver: no interrupt sources, clear any pending state. self.write32(DW_IC_INTR_MASK, 0); let _ = self.read32(DW_IC_CLR_INTR); let _ = self.read32(DW_IC_CLR_TX_ABRT); - self.set_enabled(true); + let _ = self.enable(); } - fn wait_for(&self, mut condition: impl FnMut(u32) -> bool, timeout: Duration) -> Result<(), DwError> { + /// Poll until `condition(IC_STATUS)` holds. An abort is checked FIRST on + /// every iteration — a NACKed write drains the FIFO and drops activity, + /// which would otherwise satisfy completion predicates and report + /// success for a failed transaction. + fn wait_for( + &self, + mut condition: impl FnMut(u32) -> bool, + timeout: Duration, + ) -> Result<(), DwError> { let start = Instant::now(); loop { + if self.read32(DW_IC_RAW_INTR_STAT) & IC_RAW_TX_ABRT != 0 { + return Err(self.decode_abort()); + } let status = self.read32(DW_IC_STATUS); if condition(status) { return Ok(()); } - if self.read32(DW_IC_RAW_INTR_STAT) & IC_RAW_TX_ABRT != 0 { - return Err(self.decode_abort()); - } if start.elapsed() > timeout { return Err(DwError::Timeout); } - std::thread::yield_now(); + Self::sleep_poll(); } } @@ -290,7 +421,17 @@ impl DwI2c { } } - fn set_target_address(&self, address: u16, ten_bit: bool) { + /// Linux `i2c_dw_xfer_init`: disable the adapter, program addressing + /// mode and target address, re-enable, then flush stale interrupt state. + fn xfer_init(&self, address: u16, ten_bit: bool) -> Result<(), DwError> { + // i2c_dw_wait_bus_not_busy before reprogramming. + self.wait_for( + |status| status & (IC_STATUS_ACTIVITY | IC_STATUS_MASTER_ACTIVITY) == 0, + COMPLETION_TIMEOUT, + )?; + + self.disable()?; + let mut con = self.read32(DW_IC_CON); if ten_bit { con |= IC_CON_10BITADDR_MASTER; @@ -304,44 +445,53 @@ impl DwI2c { tar |= DW_IC_TAR_10BITADDR_MASTER; } self.write32(DW_IC_TAR, tar); + + self.write32(DW_IC_INTR_MASK, 0); + self.enable()?; + let _ = self.read32(DW_IC_ENABLE_STATUS); + let _ = self.read32(DW_IC_CLR_INTR); + let _ = self.read32(DW_IC_CLR_TX_ABRT); + Ok(()) } - /// Execute a sequence of write/read segments against one slave address, - /// issuing a repeated START between segments and a STOP after the last - /// one (or leaving the bus held when `stop` is false). + /// Execute a validated sequence of write/read segments against one slave + /// address, issuing a repeated START between segments and a STOP after + /// the last byte. /// - /// Mirrors `i2c_dw_xfer` segment handling (i2c-designware-master.c): - /// writes feed IC_DATA_CMD with STOP on the final byte of the final - /// segment, reads issue IC_DATA_CMD_READ commands (RESTART on the first - /// read command after a direction change) and drain RX FIFO as data - /// arrives. - pub fn xfer( - &self, - address: u16, - ten_bit: bool, - segments: &[Segment], - stop: bool, - ) -> Result>, DwError> { - if segments.is_empty() { - return Ok(Vec::new()); + /// `stop == false` (bus held after the last segment, combined-format + /// continuation across calls) is rejected honestly: the completion wait + /// requires bus idle, so an unterminated transaction can only time out. + pub fn xfer(&self, request: &ValidatedRequest, stop: bool) -> Result>, DwError> { + if !stop { + return Err(DwError::Invalid("unterminated transfers (stop=false) are not supported")); } - if !self.enabled() { - return Err(DwError::MasterDisabled); - } - - self.set_target_address(address, ten_bit); - let _ = self.read32(DW_IC_CLR_TX_ABRT); let mut reads: Vec> = Vec::new(); - let last = segments.len() - 1; + let result = self.xfer_inner(request, &mut reads); + if result.is_err() { + self.recover(); + } + result.map(|_| reads) + } - for (index, segment) in segments.iter().enumerate() { + fn xfer_inner( + &self, + request: &ValidatedRequest, + reads: &mut Vec>, + ) -> Result<(), DwError> { + self.xfer_init(request.address, request.ten_bit)?; + + let last = request.segments.len() - 1; + for (index, segment) in request.segments.iter().enumerate() { let is_last_segment = index == last; match segment { Segment::Write(data) => { for (byte_index, byte) in data.iter().enumerate() { let mut cmd = u32::from(*byte); - if is_last_segment && byte_index == data.len() - 1 && stop { + if index > 0 && byte_index == 0 { + cmd |= IC_DATA_CMD_RESTART; + } + if is_last_segment && byte_index == data.len() - 1 { cmd |= IC_DATA_CMD_STOP; } self.wait_for( @@ -355,10 +505,10 @@ impl DwI2c { let mut buf = Vec::with_capacity(*len); for byte_index in 0..*len { let mut cmd = IC_DATA_CMD_READ; - if byte_index == 0 { + if index > 0 && byte_index == 0 { cmd |= IC_DATA_CMD_RESTART; } - if is_last_segment && byte_index == len - 1 && stop { + if is_last_segment && byte_index == len - 1 { cmd |= IC_DATA_CMD_STOP; } self.wait_for( @@ -378,17 +528,14 @@ impl DwI2c { } } - // Wait for the transfer to drain and the bus to go idle - // (i2c_dw_wait_bus_not_busy). + // i2c_dw_wait_bus_not_busy: FIFO drained and master logic idle. self.wait_for( |status| { status & IC_STATUS_TFE != 0 && status & (IC_STATUS_ACTIVITY | IC_STATUS_MASTER_ACTIVITY) == 0 }, COMPLETION_TIMEOUT, - )?; - - Ok(reads) + ) } } @@ -398,39 +545,63 @@ mod tests { #[test] fn bxt_fast_mode_matches_linux_formula() { - // 133 MHz, tHD;STA=0.6us, tLOW=1.3us, scl_fall=208ns: - // hcnt = 133000*808/1e6 - 3 = 104, lcnt = 133000*1508/1e6 - 1 = 199. + // i2c-designware-master.c i2c_dw_init_master: hcnt uses sda_fall, + // lcnt uses scl_fall, division rounds to nearest: + // hcnt = round(133000*(600+171)/1e6) - 3 = 103 - 3 = 100 + // lcnt = round(133000*(1300+208)/1e6) - 1 = 201 - 1 = 200 let timing = BXT_I2C_TIMING.fast_mode(); - assert_eq!(timing.hcnt, 104); - assert_eq!(timing.lcnt, 199); - // sda_hold_tx = ceil(133000 * 42 / 1e6) = ceil(5.586) = 6. + assert_eq!(timing.hcnt, 100); + assert_eq!(timing.lcnt, 200); assert_eq!(timing.sda_hold_tx, 6); } #[test] fn bxt_standard_mode_matches_linux_formula() { - // hcnt = 133000*4208/1e6 - 3 = 556, lcnt = 133000*4908/1e6 - 1 = 651. + // hcnt = round(133000*(4000+171)/1e6) - 3 = 555 - 3 = 552 + // lcnt = round(133000*(4700+208)/1e6) - 1 = 653 - 1 = 652 let timing = BXT_I2C_TIMING.standard_mode(); - assert_eq!(timing.hcnt, 556); - assert_eq!(timing.lcnt, 651); + assert_eq!(timing.hcnt, 552); + assert_eq!(timing.lcnt, 652); assert_eq!(timing.sda_hold_tx, 6); } -} + fn valid_segments() -> Vec { + vec![Segment::Write(vec![0x00, 0x01]), Segment::Read(4)] + } + + #[test] + fn validation_accepts_legitimate_request() { + let request = validate_request(0x2c, false, &valid_segments(), false).unwrap(); + assert_eq!(request.address, 0x2c); + assert!(!request.ten_bit); + } + + #[test] + fn validation_rejects_bad_requests() { + assert!(validate_request(0x2c, false, &[], false).is_err()); + assert!(validate_request(0x80, false, &valid_segments(), false).is_err()); + assert!(validate_request(0x2c, false, &valid_segments(), true).is_ok()); + assert!(validate_request(0x400, true, &valid_segments(), true).is_err()); + assert!(validate_request(0x2c, false, &[Segment::Read(0)], false).is_err()); + assert!(validate_request(0x2c, false, &[Segment::Read(MAX_SEGMENT_BYTES + 1)], false).is_err()); + let oversized = vec![Segment::Read(MAX_TOTAL_BYTES / 2 + 1); 3]; + assert!(validate_request(0x2c, false, &oversized, false).is_err()); + } +} pub mod endpoint { //! Shared `/scheme/` transfer endpoint for DesignWare-based //! controller daemons. Accepts bare `I2cTransferRequest` payloads and //! executes them on the matching controller's engine. use anyhow::{Context, Result}; - use i2c_interface::{I2cTransferRequest, I2cTransferResponse}; + use i2c_interface::{I2cTransferRequest, I2cTransferResponse, I2cTransferStatus}; use redox_scheme::scheme::{register_sync_scheme, SchemeSync}; use redox_scheme::{CallerCtx, OpenResult, Socket}; use scheme_utils::{Blocking, HandleMap}; use syscall::schemev2::NewFdFlags; use syscall::{Error as SysError, EBADF, EINVAL, ENOENT}; - use super::{DwI2c, Segment}; + use super::{validate_request, DwError, DwI2c, Segment}; /// A brought-up controller: initialized engine plus the names consumers /// use to address it (driver name and ACPI aliases). @@ -438,6 +609,7 @@ pub mod endpoint { pub name: String, pub aliases: Vec, pub engine: DwI2c, + pub supports_10bit_addr: bool, } enum Handle { @@ -450,29 +622,94 @@ pub mod endpoint { controllers: Vec, } + /// Why a request could not be routed to exactly one controller. + pub enum ResolveError { + NoMatch, + Ambiguous(Vec), + } + impl Endpoint { + fn failure(status: I2cTransferStatus, message: String) -> I2cTransferResponse { + I2cTransferResponse { + ok: false, + read_data: Vec::new(), + error: Some(message), + status, + } + } + + /// Resolve the requested adapter to exactly one controller. Exact + /// name/alias matches win globally; a short last-component form + /// (e.g. "I2C0") is accepted only when it identifies one controller + /// unambiguously. + fn resolve(&self, requested: &str) -> Result<&EndpointController, ResolveError> { + fn last_component(value: &str) -> &str { + value.rsplit('.').next().unwrap_or(value) + } + + let normalized = requested + .trim_start_matches("\\_SB_.") + .trim_start_matches('_'); + + if let Some(exact) = self.controllers.iter().find(|controller| { + controller.name == requested + || controller + .aliases + .iter() + .any(|alias| alias == requested || alias == &normalized) + }) { + return Ok(exact); + } + + let fuzzy: Vec<&EndpointController> = self + .controllers + .iter() + .filter(|controller| { + last_component(&controller.name) == last_component(normalized) + || controller + .aliases + .iter() + .any(|alias| last_component(alias) == last_component(normalized)) + }) + .collect(); + + match fuzzy.as_slice() { + [only] => Ok(*only), + [] => Err(ResolveError::NoMatch), + many => Err(ResolveError::Ambiguous( + many.iter().map(|controller| controller.name.clone()).collect(), + )), + } + } + fn execute(&self, request: &I2cTransferRequest) -> I2cTransferResponse { if request.segments.is_empty() { return I2cTransferResponse { ok: true, read_data: Vec::new(), error: None, + status: I2cTransferStatus::Ok, }; } - let Some(controller) = self - .controllers - .iter() - .find(|controller| adapter_matches(&controller.name, &controller.aliases, &request.adapter)) - else { - return I2cTransferResponse { - ok: false, - read_data: Vec::new(), - error: Some(format!( - "no controller matches adapter '{}'", - request.adapter - )), - }; + let controller = match self.resolve(&request.adapter) { + Ok(controller) => controller, + Err(ResolveError::NoMatch) => { + return Self::failure( + I2cTransferStatus::Error, + format!("no controller matches adapter '{}'", request.adapter), + ); + } + Err(ResolveError::Ambiguous(names)) => { + return Self::failure( + I2cTransferStatus::Error, + format!( + "adapter '{}' is ambiguous between controllers: {}", + request.adapter, + names.join(", ") + ), + ); + } }; let first = &request.segments[0]; @@ -482,11 +719,10 @@ pub mod endpoint { .iter() .any(|segment| segment.address != address || segment.ten_bit_address != ten_bit) { - return I2cTransferResponse { - ok: false, - read_data: Vec::new(), - error: Some("mixed-address segment sequences are not supported".to_string()), - }; + return Self::failure( + I2cTransferStatus::Error, + "mixed-address segment sequences are not supported".to_string(), + ); } let ops: Vec = request @@ -498,43 +734,32 @@ pub mod endpoint { }) .collect(); - match controller.engine.xfer(address, ten_bit, &ops, request.stop) { + let validated = match validate_request(address, ten_bit, &ops, controller.supports_10bit_addr) { + Ok(validated) => validated, + Err(err) => { + return Self::failure(I2cTransferStatus::Error, format!("{err}")); + } + }; + + match controller.engine.xfer(&validated, request.stop) { Ok(reads) => I2cTransferResponse { ok: true, read_data: reads, error: None, + status: I2cTransferStatus::Ok, }, - Err(err) => I2cTransferResponse { - ok: false, - read_data: Vec::new(), - error: Some(format!("{}: {err}", controller.name)), - }, + Err(err) => { + let status = match err { + DwError::Nack => I2cTransferStatus::Nack, + DwError::Timeout => I2cTransferStatus::Timeout, + _ => I2cTransferStatus::Error, + }; + Self::failure(status, format!("{}: {err}", controller.name)) + } } } } - /// Adapter name matching tolerant to ACPI path normalization forms: - /// exact match, then last-dot-component match - /// ("I2C0" ≡ "PC00.I2C0" ≡ "\_SB_.PC00.I2C0"). - fn adapter_matches(name: &str, aliases: &[String], requested: &str) -> bool { - fn last_component(value: &str) -> &str { - value.rsplit('.').next().unwrap_or(value) - } - - let normalized = requested - .trim_start_matches("\\_SB_.") - .trim_start_matches('_'); - if name == requested || aliases.iter().any(|alias| alias == requested || alias == &normalized) { - return true; - } - if last_component(name) == last_component(normalized) { - return true; - } - aliases - .iter() - .any(|alias| last_component(alias) == last_component(normalized)) - } - impl SchemeSync for Endpoint { fn scheme_root(&mut self) -> syscall::Result { Ok(self.handles.insert(Handle::SchemeRoot)) @@ -629,7 +854,7 @@ pub mod endpoint { pub fn serve( scheme_name: &str, controllers: Vec, - on_ready: impl FnOnce(), + on_ready: impl FnOnce() -> Result<()>, ) -> Result<()> { let socket = Socket::create().context("failed to create scheme socket")?; let mut endpoint = Endpoint { @@ -644,7 +869,9 @@ pub mod endpoint { endpoint.controllers.len() ); - on_ready(); + // Readiness is reported only if the daemon's own setup (namespace + // reduction included) fully succeeds. + on_ready()?; let blocking = Blocking::new(&socket, 16); blocking @@ -659,13 +886,51 @@ pub mod endpoint { mod tests { use super::*; + fn resolve_via<'a>( + controllers: &'a [(&'a str, Vec)], + requested: &str, + ) -> Result<&'a str, ()> { + fn last_component(value: &str) -> &str { + value.rsplit('.').next().unwrap_or(value) + } + let normalized = requested + .trim_start_matches("\\_SB_.") + .trim_start_matches('_'); + if let Some(exact) = controllers.iter().find(|(name, aliases)| { + *name == requested + || aliases + .iter() + .any(|alias| alias == requested || alias == &normalized) + }) { + return Ok(exact.0); + } + let fuzzy: Vec<&&str> = controllers + .iter() + .map(|(name, aliases)| (name, aliases)) + .filter(|(name, aliases)| { + last_component(name) == last_component(normalized) + || aliases + .iter() + .any(|alias| last_component(alias) == last_component(normalized)) + }) + .map(|(name, _)| name) + .collect(); + match fuzzy.as_slice() { + [only] => Ok(**only), + _ => Err(()), + } + } + #[test] - fn adapter_name_matching_forms() { - let aliases = vec!["PC00.I2C0".to_string()]; - assert!(adapter_matches("intel-lpss:00--15.0", &aliases, "PC00.I2C0")); - assert!(adapter_matches("intel-lpss:00--15.0", &aliases, "\\_SB_.PC00.I2C0")); - assert!(adapter_matches("intel-lpss:00--15.0", &aliases, "I2C0")); - assert!(!adapter_matches("intel-lpss:00--15.0", &aliases, "I2C3")); + fn resolve_prefers_exact_over_fuzzy() { + let controllers = vec![ + ("intel-lpss:00--15.0", vec!["PC00.I2C0".to_string()]), + ("intel-lpss:00--15.1", vec!["PC00.I2C1".to_string()]), + ]; + assert_eq!(resolve_via(&controllers, "PC00.I2C0"), Ok("intel-lpss:00--15.0")); + assert_eq!(resolve_via(&controllers, "\\_SB_.PC00.I2C1"), Ok("intel-lpss:00--15.1")); + assert_eq!(resolve_via(&controllers, "I2C0"), Ok("intel-lpss:00--15.0")); + assert!(resolve_via(&controllers, "I2C3").is_err()); } } } diff --git a/drivers/i2c/dw-acpi-i2cd/src/main.rs b/drivers/i2c/dw-acpi-i2cd/src/main.rs index 6c62673ad3..3b4e082218 100644 --- a/drivers/i2c/dw-acpi-i2cd/src/main.rs +++ b/drivers/i2c/dw-acpi-i2cd/src/main.rs @@ -103,9 +103,8 @@ fn daemon_main(daemon: daemon::Daemon) -> Result<()> { let _guards = guards; dw_i2c::endpoint::serve("dw-acpi-i2c", endpoint_controllers, || { daemon.ready(); - if let Err(err) = libredox::call::setrens(0, 0) { - log::error!("dw-acpi-i2cd: failed to enter null namespace: {err}"); - } + libredox::call::setrens(0, 0).context("failed to enter null namespace")?; + Ok(()) }) } @@ -322,6 +321,11 @@ fn bring_up_controller( name: format!("{prefix}:{device}"), aliases: vec![device], engine, + supports_10bit_addr: resources + .serial_bus + .as_ref() + .map(|bus| bus.access_mode_10bit) + .unwrap_or(false), }; Ok((endpoint, (mmio, registration))) } diff --git a/drivers/i2c/i2c-interface/src/lib.rs b/drivers/i2c/i2c-interface/src/lib.rs index 48721e3c2b..e6cbbad753 100644 --- a/drivers/i2c/i2c-interface/src/lib.rs +++ b/drivers/i2c/i2c-interface/src/lib.rs @@ -45,6 +45,10 @@ pub struct I2cTransferResponse { pub ok: bool, pub read_data: Vec>, pub error: Option, + /// Machine-readable outcome; `ok` is kept for wire compatibility and + /// always equals `status == I2cTransferStatus::Ok`. + #[serde(default)] + pub status: I2cTransferStatus, } #[derive(Clone, Debug, Default, Serialize, Deserialize)] @@ -74,8 +78,9 @@ pub struct I2cAdapterInfo { pub aliases: Vec, } -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub enum I2cTransferStatus { + #[default] Ok, Nack, Timeout, diff --git a/drivers/i2c/i2cd/src/main.rs b/drivers/i2c/i2cd/src/main.rs index 8bc2cb2d42..f0193d068f 100644 --- a/drivers/i2c/i2cd/src/main.rs +++ b/drivers/i2c/i2cd/src/main.rs @@ -4,7 +4,7 @@ use std::process; use anyhow::{Context, Result}; use i2c_interface::{ I2cAdapterInfo, I2cControlRequest, I2cControlResponse, I2cTransferRequest, - I2cTransferResponse, + I2cTransferResponse, I2cTransferStatus, }; use redox_scheme::scheme::SchemeSync; use redox_scheme::{CallerCtx, OpenResult, Socket}; @@ -25,6 +25,12 @@ struct AdapterEntry { info: I2cAdapterInfo, } +/// Why a request could not be routed to exactly one adapter. +enum ResolveError { + NoMatch, + Ambiguous(Vec), +} + struct I2cDaemon { handles: HandleMap, adapters: BTreeMap, @@ -84,11 +90,11 @@ impl I2cDaemon { Self::set_pending_response(handle, I2cControlResponse::AdapterList(adapters)) } - /// Resolve an adapter by request name. Consumers identify adapters by the - /// ACPI resource-source string from the device's `_CRS` (e.g. - /// "\_SB_.PC00.I2C0"), so matching is tolerant to normalization forms: - /// exact name/alias match first, then last-dot-component match. - fn resolve_adapter(&self, requested: &str) -> Option<&I2cAdapterInfo> { + /// Resolve an adapter by request name. Exact name/alias matches win + /// globally; a short last-component form (e.g. "I2C0") is accepted only + /// when it identifies exactly one adapter, so a partial name can never + /// silently route a transaction to the wrong bus. + fn resolve_adapter(&self, requested: &str) -> Result<&I2cAdapterInfo, ResolveError> { fn last_component(value: &str) -> &str { value.rsplit('.').next().unwrap_or(value) } @@ -97,7 +103,8 @@ impl I2cDaemon { .trim_start_matches("\\_SB_.") .trim_start_matches('_'); - self.adapters + if let Some(exact) = self + .adapters .values() .map(|entry| &entry.info) .find(|info| { @@ -106,12 +113,42 @@ impl I2cDaemon { .aliases .iter() .any(|alias| alias == requested || alias == &normalized) - || last_component(&info.name) == last_component(normalized) + }) + { + return Ok(exact); + } + + let fuzzy: Vec<&I2cAdapterInfo> = self + .adapters + .values() + .map(|entry| &entry.info) + .filter(|info| { + last_component(&info.name) == last_component(normalized) || info .aliases .iter() .any(|alias| last_component(alias) == last_component(normalized)) }) + .collect(); + + match fuzzy.as_slice() { + [only] => Ok(*only), + [] => Err(ResolveError::NoMatch), + many => Err(ResolveError::Ambiguous( + many.iter().map(|info| info.name.clone()).collect(), + )), + } + } + + /// Provider scheme names are used as `/scheme//transfer` — they + /// must be a single safe path component. + fn is_valid_provider_scheme(name: &str) -> bool { + !name.is_empty() + && name.len() <= 64 + && name + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || matches!(c, '.' | '_' | '-')) + && !name.starts_with('.') } /// Forward a transfer to the provider daemon that owns the adapter's @@ -119,41 +156,56 @@ impl I2cDaemon { /// back the response. The wire format is the bare `I2cTransferRequest` / /// `I2cTransferResponse` pair that in-tree consumers (i2c-hidd) speak. fn forward_transfer(request: &I2cTransferRequest, info: &I2cAdapterInfo) -> I2cTransferResponse { - let fail = |message: String| I2cTransferResponse { + let fail = |status: I2cTransferStatus, message: String| I2cTransferResponse { ok: false, read_data: Vec::new(), error: Some(message), + status, }; if info.provider_scheme.is_empty() { - return fail(format!( - "adapter '{}' has no transfer provider", - info.name - )); + return fail( + I2cTransferStatus::Error, + format!("adapter '{}' has no transfer provider", info.name), + ); } let path = format!("/scheme/{}/transfer", info.provider_scheme); let mut file = match std::fs::OpenOptions::new().read(true).write(true).open(&path) { Ok(file) => file, - Err(err) => return fail(format!("cannot open {path}: {err}")), + Err(err) => return fail(I2cTransferStatus::Error, format!("cannot open {path}: {err}")), }; let payload = match ron::ser::to_string(request) { Ok(payload) => payload, - Err(err) => return fail(format!("cannot encode transfer request: {err}")), + Err(err) => { + return fail( + I2cTransferStatus::Error, + format!("cannot encode transfer request: {err}"), + ) + } }; if let Err(err) = std::io::Write::write_all(&mut file, payload.as_bytes()) { - return fail(format!("cannot write transfer to {path}: {err}")); + return fail( + I2cTransferStatus::Timeout, + format!("cannot write transfer to {path}: {err}"), + ); } let mut text = String::new(); if let Err(err) = std::io::Read::read_to_string(&mut file, &mut text) { - return fail(format!("cannot read transfer response from {path}: {err}")); + return fail( + I2cTransferStatus::Timeout, + format!("cannot read transfer response from {path}: {err}"), + ); } match ron::from_str::(&text) { Ok(response) => response, - Err(err) => fail(format!("invalid transfer response from {path}: {err}")), + Err(err) => fail( + I2cTransferStatus::Error, + format!("invalid transfer response from {path}: {err}"), + ), } } @@ -291,7 +343,7 @@ impl SchemeSync for I2cDaemon { ron::from_str(text).map_err(|_| SysError::new(EINVAL))?; let response = match self.resolve_adapter(&transfer.adapter) { - Some(info) => { + Ok(info) => { let info = info.clone(); log::debug!( "i2cd: routing transfer for '{}' to provider '{}' ({})", @@ -301,13 +353,24 @@ impl SchemeSync for I2cDaemon { ); Self::forward_transfer(&transfer, &info) } - None => I2cTransferResponse { + Err(ResolveError::NoMatch) => I2cTransferResponse { ok: false, read_data: Vec::new(), error: Some(format!( "no registered adapter matches '{}'", transfer.adapter )), + status: I2cTransferStatus::Error, + }, + Err(ResolveError::Ambiguous(names)) => I2cTransferResponse { + ok: false, + read_data: Vec::new(), + error: Some(format!( + "adapter '{}' is ambiguous between adapters: {}", + transfer.adapter, + names.join(", ") + )), + status: I2cTransferStatus::Error, }, }; @@ -326,6 +389,17 @@ impl SchemeSync for I2cDaemon { match request { I2cControlRequest::RegisterAdapter { mut info } => { + if !info.provider_scheme.is_empty() && !Self::is_valid_provider_scheme(&info.provider_scheme) { + let handle = self.handles.get_mut(id)?; + Self::set_pending_response( + handle, + I2cControlResponse::Error(format!( + "invalid provider_scheme '{}': must be a single safe scheme-name component", + info.provider_scheme + )), + )?; + return Ok(buf.len()); + } let adapter_id = self.next_id; self.next_id = self.next_id.checked_add(1).ok_or(SysError::new(EINVAL))?; info.id = adapter_id; diff --git a/drivers/i2c/intel-lpss-i2cd/Cargo.toml b/drivers/i2c/intel-lpss-i2cd/Cargo.toml index 8c6f67f5c2..96077db37a 100644 --- a/drivers/i2c/intel-lpss-i2cd/Cargo.toml +++ b/drivers/i2c/intel-lpss-i2cd/Cargo.toml @@ -18,6 +18,7 @@ common = { path = "../../common" } daemon = { path = "../../../daemon" } dw-i2c = { path = "../designware" } i2c-interface = { path = "../i2c-interface" } +pcid = { path = "../../pcid" } scheme-utils = { path = "../../../scheme-utils" } [lints] diff --git a/drivers/i2c/intel-lpss-i2cd/src/main.rs b/drivers/i2c/intel-lpss-i2cd/src/main.rs index 5a3f6a492d..5805acc84f 100644 --- a/drivers/i2c/intel-lpss-i2cd/src/main.rs +++ b/drivers/i2c/intel-lpss-i2cd/src/main.rs @@ -26,6 +26,15 @@ use serde::Deserialize; use dw_i2c::endpoint::EndpointController; use dw_i2c::{DwI2c, BXT_I2C_TIMING}; +use pcid_interface::PciFunctionHandle; + +// Intel LPSS private registers at BAR0 + 0x200 (Linux drivers/mfd/intel-lpss.c +// intel_lpss_init_dev): reset cycle + 64-bit remap address programming. +const LPSS_PRIV_OFFSET: usize = 0x200; +const LPSS_PRIV_RESETS: usize = 0x04; +const LPSS_PRIV_RESETS_FUNC_IDMA: u32 = 0x3 | (1 << 2); +const LPSS_PRIV_REMAP_ADDR_LO: usize = 0x40; +const LPSS_PRIV_REMAP_ADDR_HI: usize = 0x44; const SUPPORTED_ACPI_IDS: &[&str] = &["INT33C2", "INT33C3", "INT3432", "INT3433", "INTC10EF"]; @@ -43,12 +52,22 @@ const PCI_VENDOR_INTEL: u16 = 0x8086; const PCI_CLASS_SERIAL_BUS: u8 = 0x0c; const PCI_SUBCLASS_I2C: u8 = 0x80; +/// Keeps the hardware claim and MMIO mapping alive. The PCI variant owns +/// the pcid claim (released on drop); the ACPI variant owns the raw mapping. +/// The fields are intentionally never read — ownership is the mechanism. +#[allow(dead_code)] +enum MmioGuard { + Pci(PciFunctionHandle), + Acpi(PhysBorrowed), +} + struct Controller { name: String, aliases: Vec, engine: DwI2c, + supports_10bit_addr: bool, registration: File, - mmio: PhysBorrowed, + guard: MmioGuard, } #[derive(Debug, Deserialize)] @@ -104,8 +123,8 @@ fn daemon_main(daemon: daemon::Daemon) -> Result<()> { let mut controllers = Vec::new(); - for discovered in discover_pci_controllers() { - match bring_up_controller(discovered) { + for candidate in discover_pci_controllers() { + match bring_up_pci_controller(candidate) { Ok(controller) => controllers.push(controller), Err(err) => log::warn!("intel-lpss-i2cd: PCI controller bring-up skipped: {err:#}"), } @@ -133,26 +152,28 @@ fn daemon_main(daemon: daemon::Daemon) -> Result<()> { name, aliases, engine, + supports_10bit_addr, registration, - mmio, + guard, } = controller; - guards.push((registration, mmio)); + guards.push((registration, guard)); EndpointController { name, aliases, engine, + supports_10bit_addr, } }) .collect(); // Register the scheme first, then notify init (xhcid ordering) so a // `type = { scheme = "i2c-lpss" }` service only reports ready once the - // scheme path actually exists. + // scheme path actually exists. Namespace reduction is fail-closed: if it + // fails the daemon exits instead of running unsandboxed. dw_i2c::endpoint::serve("i2c-lpss", endpoint_controllers, || { daemon.ready(); - if let Err(err) = libredox::call::setrens(0, 0) { - log::error!("intel-lpss-i2cd: failed to enter null namespace: {err}"); - } + libredox::call::setrens(0, 0).context("failed to enter null namespace")?; + Ok(()) }) } @@ -190,39 +211,122 @@ fn bring_up_controller(discovered: DiscoveredController) -> Result { aliases, ); - let info = I2cAdapterInfo { + let supports_10bit_addr = discovered + .resources + .serial_bus + .as_ref() + .map(|bus| bus.access_mode_10bit) + .unwrap_or(true); + + let registration = register_adapter(&I2cAdapterInfo { id: 0, name: name.clone(), max_transaction_size: 0, - supports_10bit_addr: discovered - .resources - .serial_bus - .as_ref() - .map(|bus| bus.access_mode_10bit) - .unwrap_or(true), + supports_10bit_addr, provider_scheme: "i2c-lpss".to_string(), aliases: aliases.clone(), - }; - let mut registration = register_adapter(&info) - .with_context(|| format!("failed to register {name} with i2cd"))?; - let response = read_registration_response(&mut registration) - .with_context(|| format!("failed to read i2cd registration response for {name}"))?; + }) + .and_then(|mut file| read_registration_response(&mut file).map(|response| (file, response))) + .with_context(|| format!("failed to register {name} with i2cd"))?; - match response { + match registration.1 { I2cControlResponse::AdapterRegistered { id } => { log::info!("intel-lpss-i2cd: adapter {name} registered with i2cd as {id}"); } - other => { - log::warn!("intel-lpss-i2cd: unexpected i2cd registration response for {name}: {other:?}"); - } + other => anyhow::bail!("unexpected i2cd registration response for {name}: {other:?}"), } Ok(Controller { name, aliases, engine, - registration, - mmio, + supports_10bit_addr, + registration: registration.0, + guard: MmioGuard::Acpi(mmio), + }) +} + +// --------------------------------------------------------------------------- +// PCI bring-up (Linux intel-lpss-pci.c model) +// --------------------------------------------------------------------------- + +struct PciCandidate { + path: std::path::PathBuf, + location: String, + device_id: u16, +} + +fn bring_up_pci_controller(candidate: PciCandidate) -> Result { + let mut handle = PciFunctionHandle::connect_by_path(&candidate.path) + .with_context(|| format!("failed to claim {}", candidate.location))?; + + handle.enable_device(); + + let config = handle.config(); + let (bar_phys, bar_len) = config.func.bars[0].expect_mem(); + let bar_phys = usize::try_from(bar_phys).context("BAR0 address does not fit in usize")?; + anyhow::ensure!( + bar_len >= LPSS_PRIV_OFFSET + LPSS_PRIV_REMAP_ADDR_HI + 4, + "{}: BAR0 too small for LPSS private registers ({bar_len:#x})", + candidate.location, + ); + + let engine = unsafe { + let mapped = handle.map_bar(0); + DwI2c::new(mapped.ptr.as_ptr()) + }; + + // intel_lpss_init_dev: reset cycle, then program the 64-bit remap address. + unsafe { + let priv_base = engine.mmio_base().add(LPSS_PRIV_OFFSET); + (priv_base.add(LPSS_PRIV_RESETS) as *mut u32).write_volatile(0); + (priv_base.add(LPSS_PRIV_RESETS) as *mut u32) + .write_volatile(LPSS_PRIV_RESETS_FUNC_IDMA); + (priv_base.add(LPSS_PRIV_REMAP_ADDR_LO) as *mut u32) + .write_volatile(bar_phys as u32); + (priv_base.add(LPSS_PRIV_REMAP_ADDR_HI) as *mut u32) + .write_volatile((bar_phys >> 32) as u32); + } + + engine.init(&BXT_I2C_TIMING); + + let name = format!("intel-lpss:{}", candidate.location); + let aliases = discover_acpi_aliases(bar_phys); + + log::info!( + "intel-lpss-i2cd: controller {} pci=8086:{:04x} bar={:#x}+{:#x} aliases={:?}", + name, + candidate.device_id, + bar_phys, + bar_len, + aliases, + ); + + let registration = register_adapter(&I2cAdapterInfo { + id: 0, + name: name.clone(), + max_transaction_size: 0, + supports_10bit_addr: true, + provider_scheme: "i2c-lpss".to_string(), + aliases: aliases.clone(), + }) + .and_then(|mut file| read_registration_response(&mut file).map(|response| (file, response))) + .with_context(|| format!("failed to register {name} with i2cd"))?; + + match registration.1 { + I2cControlResponse::AdapterRegistered { id } => { + log::info!("intel-lpss-i2cd: adapter {name} registered with i2cd as {id}"); + } + other => anyhow::bail!("unexpected i2cd registration response for {name}: {other:?}"), + } + + Ok(Controller { + name, + aliases, + engine, + supports_10bit_addr: true, + registration: registration.0, + guard: MmioGuard::Pci(handle), }) } @@ -270,7 +374,7 @@ fn read_registration_response(file: &mut File) -> Result { // PCI discovery (Linux intel-lpss-pci.c model) // --------------------------------------------------------------------------- -fn discover_pci_controllers() -> Vec { +fn discover_pci_controllers() -> Vec { let entries = match fs::read_dir("/scheme/pci") { Ok(entries) => entries, Err(err) => { @@ -300,53 +404,22 @@ fn discover_pci_controllers() -> Vec { continue; } - let Some(mmio_base) = pci_bar0_address(&config) else { - log::warn!("intel-lpss-i2cd: {device:04x} has no usable BAR0, skipped"); - continue; - }; - let location = entry .file_name() .to_str() .map(str::to_owned) .unwrap_or_else(|| format!("pci-{device:04x}")); - let irq = config[0x3c]; - let irq = (irq != 0 && irq != 0xff).then_some(u32::from(irq)); - out.push(DiscoveredController { - device: location, - hid: format!("pci:8086:{device:04x}"), - resources: ControllerResources { - mmio_base, - mmio_len: 0x1000, - irq, - serial_bus: None, - }, + out.push(PciCandidate { + path: entry.path(), + location, + device_id: device, }); } out } -/// Extract a usable memory BAR0 from raw PCI config space. Handles both -/// 32-bit and 64-bit memory BARs; returns the CPU-physical base address. -fn pci_bar0_address(config: &[u8]) -> Option { - let bar_lo = u32::from_le_bytes([config[0x10], config[0x11], config[0x12], config[0x13]]); - if bar_lo == 0 || bar_lo & 1 != 0 { - // Not present or an I/O-space BAR; LPSS I2C is always memory-mapped. - return None; - } - let is_64bit = (bar_lo >> 1) & 0x3 == 0x2; - let base_lo = u64::from(bar_lo & !0xf); - let base = if is_64bit { - let bar_hi = u32::from_le_bytes([config[0x14], config[0x15], config[0x16], config[0x17]]); - base_lo | (u64::from(bar_hi) << 32) - } else { - base_lo - }; - (base != 0).then_some(base as usize) -} - // --------------------------------------------------------------------------- // ACPI discovery (legacy LPSS HID model) // --------------------------------------------------------------------------- @@ -550,26 +623,13 @@ mod tests { use super::*; #[test] - fn bar0_32bit_memory() { - let mut config = [0u8; 0x40]; - config[0x10..0x14].copy_from_slice(&0x5019_2de0u32.to_le_bytes()); - assert_eq!(pci_bar0_address(&config), Some(0x5019_2de0)); + fn lpss_pci_id_table_covers_arl_h_and_mtl_p() { + // Host-critical controllers (LG Gram 16Z90TP: 8086:7778, 8086:7779). + for id in [0x7778u16, 0x7779, 0x777a, 0x777b, 0x7750, 0x7751] { + assert!(LPSS_I2C_PCI_IDS.contains(&id), "ARL-H id {id:#06x} missing"); + } + for id in [0x7e50u16, 0x7e51, 0x7e78, 0x7e79, 0x7e7a, 0x7e7b] { + assert!(LPSS_I2C_PCI_IDS.contains(&id), "MTL-P id {id:#06x} missing"); + } } - - #[test] - fn bar0_64bit_memory() { - let mut config = [0u8; 0x40]; - // 64-bit BAR: flags bits [2:1] == 0b10, base 0x5019_92de_0000. - config[0x10..0x14].copy_from_slice(&0x92de_0004u32.to_le_bytes()); - config[0x14..0x18].copy_from_slice(&0x5019u32.to_le_bytes()); - assert_eq!(pci_bar0_address(&config), Some(0x5019_92de_0000)); - } - - #[test] - fn bar0_io_space_rejected() { - let mut config = [0u8; 0x40]; - config[0x10..0x14].copy_from_slice(&0xefa1u32.to_le_bytes()); - assert_eq!(pci_bar0_address(&config), None); - } - }