v5.2: G-A4 iwlwifi spawned-mode reads PCID_CLIENT_CHANNEL
Refactor daemon_target_from_env to prefer PCID_CLIENT_CHANNEL (the
channel contract used by driver-manager) over PCID_DEVICE_PATH
(legacy). Previously the --daemon branch silently ignored the
channel granted by driver-manager and looked for PCID_DEVICE_PATH,
which is unset in the spawned-daemon path. This caused --daemon
to work only by accident of the scan fallback (selecting the
first Intel Wi-Fi device).
Architecture:
- New DaemonSource enum (Channel, DevicePath) classifies the
selected source.
- New select_daemon_source(channel: Option<&str>,
device_path: Option<&str>) -> Option<DaemonSource> is a pure
function so the selection logic is testable on any platform.
- daemon_target_from_env() now reads PCID_CLIENT_CHANNEL first;
if set, calls bdf_from_channel() which uses
pcid_interface::PciFunctionHandle::connect_default() to consume
the granted channel and extract BDF from
handle.config().func.addr (PciAddress whose Display impl
produces SSSS:BB:DD.F, matching PciLocation exactly).
- PCID_DEVICE_PATH is preserved as the legacy fallback for
manual CLI mode only - it is NOT consulted when
PCID_CLIENT_CHANNEL is set (avoids silent fallback that hides
spawn-contract bugs).
- On malformed channel, bdf_from_channel() exits via
connect_default()'s built-in process::exit(1) - loud failure,
not silent fallback.
Dependencies:
- Added pcid_interface = { path = "../../../../sources/base/drivers/pcid",
package = "pcid" } to target-cfg(redox) deps. The pcid crate's
lib target is named pcid_interface; package renaming is required
to use it under that name in edition 2024.
- [patch.crates-io] for redox-driver-sys ensures transitive deps
resolve to our local fork.
Tests:
- 3 tests pass (all up from pre-fix).
- cli_flow::cli_daemon_target_exits_when_neither_env_set: end-to-end
test that --daemon with neither env var exits cleanly.
- cli_flow::cli_flow_reports_bounded_intel_progression: existing
full init flow test passes.
- Unit tests in main.rs for select_daemon_source cover all
env-var combinations (channel-only, device-path-only, both,
neither).
Per local/AGENTS.md:
- No new branches (work on 0.3.1)
- No stubs, no todo!/unimplemented!
- Cat 1 in-house recipe - source IS the durable location
Closes G-A4 from v4.8 audit. Operator confirmed earlier
instruction reversed: this work IS expected.
Driver-manager config at local/config/drivers.d/70-wifi.toml
spawns iwlwifi with --daemon and passes PCID_CLIENT_CHANNEL.
This commit makes iwlwifi actually consume that channel
end-to-end.
This commit is contained in:
@@ -15,6 +15,10 @@ linux-kpi = { path = "../../linux-kpi/source" }
|
||||
|
||||
[target.'cfg(target_os = "redox")'.dependencies]
|
||||
redox-driver-sys = { path = "../../redox-driver-sys/source", features = ["redox"] }
|
||||
pcid_interface = { path = "../../../../sources/base/drivers/pcid", package = "pcid" }
|
||||
|
||||
[patch.crates-io]
|
||||
redox-driver-sys = { path = "../../redox-driver-sys/source" }
|
||||
|
||||
[build-dependencies]
|
||||
cc = "1"
|
||||
|
||||
@@ -13,6 +13,9 @@ use redox_driver_sys::pci::{PciLocation, PCI_VENDOR_ID_INTEL};
|
||||
use std::ffi::CString;
|
||||
use thiserror::Error;
|
||||
|
||||
#[cfg(target_os = "redox")]
|
||||
use pcid_interface::PciFunctionHandle;
|
||||
|
||||
#[cfg(target_os = "redox")]
|
||||
use linux_kpi::firmware::{release_firmware, request_firmware, Firmware};
|
||||
|
||||
@@ -232,6 +235,16 @@ fn main() {
|
||||
std::thread::sleep(std::time::Duration::from_secs(3600));
|
||||
}
|
||||
}
|
||||
Some("--daemon-target") => match daemon_target_from_env() {
|
||||
Some(target) => println!("daemon_target={target}"),
|
||||
None => {
|
||||
eprintln!(
|
||||
"redbear-iwlwifi: no daemon target — set PCID_CLIENT_CHANNEL \
|
||||
(channel contract) or PCID_DEVICE_PATH (legacy CLI)"
|
||||
);
|
||||
std::process::exit(1);
|
||||
}
|
||||
},
|
||||
Some("--irq-test") => {
|
||||
let target = args.next();
|
||||
run_device_action(&firmware_root, target, irq_test_candidate, "irq-test")
|
||||
@@ -246,18 +259,101 @@ fn main() {
|
||||
}
|
||||
_ => {
|
||||
eprintln!(
|
||||
"redbear-iwlwifi: use --probe, --status <device>, --prepare <device>, --transport-probe <device>, --init-transport <device>, --activate-nic <device>, --scan <device>, --connect <device> <ssid> <security> [key], --disconnect <device>, --full-init <device>, --irq-test <device>, --dma-test <device>, or --retry <device>"
|
||||
"redbear-iwlwifi: use --probe, --status <device>, --prepare <device>, --transport-probe <device>, --init-transport <device>, --activate-nic <device>, --scan <device>, --connect <device> <ssid> <security> [key], --disconnect <device>, --full-init <device>, --daemon [device], --daemon-target, --irq-test <device>, --dma-test <device>, or --retry <device>"
|
||||
);
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Which environment variable the daemon should use to discover its PCI target.
|
||||
///
|
||||
/// `Channel` takes precedence — it is the standard pcid channel contract used by
|
||||
/// the driver-manager. `DevicePath` is the legacy pcid-spawner CLI fallback,
|
||||
/// preserved for manual invocation only.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum DaemonSource {
|
||||
/// `PCID_CLIENT_CHANNEL` — the channel contract (driver-manager spawned daemons).
|
||||
Channel,
|
||||
/// `PCID_DEVICE_PATH` — the legacy CLI contract (manual invocation only).
|
||||
DevicePath,
|
||||
}
|
||||
|
||||
/// Pure selection logic — decides which env var supplies the daemon target.
|
||||
///
|
||||
/// Channel always wins when set. This function is platform-independent and
|
||||
/// does not open any file descriptors, so it can be unit-tested on the host
|
||||
/// without a running pcid or driver-manager.
|
||||
pub(crate) fn select_daemon_source(
|
||||
channel: Option<&str>,
|
||||
device_path: Option<&str>,
|
||||
) -> Option<DaemonSource> {
|
||||
if channel.is_some() {
|
||||
Some(DaemonSource::Channel)
|
||||
} else if device_path.is_some() {
|
||||
Some(DaemonSource::DevicePath)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the daemon target BDF from the environment.
|
||||
///
|
||||
/// Priority:
|
||||
/// 1. `PCID_CLIENT_CHANNEL` (channel contract — used by driver-manager).
|
||||
/// On Redox this opens the pcid channel via `PciFunctionHandle::connect_default()`
|
||||
/// and extracts the BDF from the PCI address. `connect_default()` calls
|
||||
/// `process::exit(1)` on a malformed channel, which is the correct
|
||||
/// behaviour for a spawned daemon — loud failure, never silent fallback.
|
||||
/// 2. `PCID_DEVICE_PATH` (legacy CLI contract — manual invocation only).
|
||||
fn daemon_target_from_env() -> Option<String> {
|
||||
let path = env::var("PCID_DEVICE_PATH").ok()?;
|
||||
let name = path.rsplit('/').next()?;
|
||||
let location = redox_driver_sys::pci::parse_scheme_entry(name)?;
|
||||
Some(location.to_string())
|
||||
let channel = env::var("PCID_CLIENT_CHANNEL").ok();
|
||||
let device_path = env::var("PCID_DEVICE_PATH").ok();
|
||||
|
||||
match select_daemon_source(channel.as_deref(), device_path.as_deref()) {
|
||||
Some(DaemonSource::Channel) => {
|
||||
#[cfg(target_os = "redox")]
|
||||
{
|
||||
Some(bdf_from_channel())
|
||||
}
|
||||
#[cfg(not(target_os = "redox"))]
|
||||
{
|
||||
// Channel mode requires the pcid channel fd, which only exists
|
||||
// on Redox. If we reach here on the host it's a misconfiguration.
|
||||
let _ = channel;
|
||||
eprintln!(
|
||||
"redbear-iwlwifi: PCID_CLIENT_CHANNEL is set but channel mode \
|
||||
requires the Redox target — refusing to fall back to \
|
||||
PCID_DEVICE_PATH"
|
||||
);
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
Some(DaemonSource::DevicePath) => {
|
||||
let path = device_path.unwrap();
|
||||
let name = path.rsplit('/').next()?;
|
||||
let location = redox_driver_sys::pci::parse_scheme_entry(name)?;
|
||||
Some(location.to_string())
|
||||
}
|
||||
None => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract the BDF string from the pcid channel handle.
|
||||
///
|
||||
/// Opens the channel via `PciFunctionHandle::connect_default()` (which reads
|
||||
/// `PCID_CLIENT_CHANNEL`), then formats the PCI address as `SSSS:BB:DD.F`.
|
||||
///
|
||||
/// `connect_default()` panics with `process::exit(1)` on any failure (missing
|
||||
/// env var, invalid fd, channel protocol error). This is intentional — a
|
||||
/// spawned daemon must fail loudly, not silently fall back.
|
||||
#[cfg(target_os = "redox")]
|
||||
fn bdf_from_channel() -> String {
|
||||
let handle = PciFunctionHandle::connect_default();
|
||||
let addr = handle.config().func.addr;
|
||||
// PciAddress's Display impl produces exactly "{:04x}:{:02x}:{:02x}.{}",
|
||||
// matching PciLocation's Display — the format select_candidate expects.
|
||||
addr.to_string()
|
||||
}
|
||||
|
||||
fn run_connect_action(
|
||||
@@ -1392,8 +1488,11 @@ fn dma_test_candidate(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::sync::Mutex;
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
static DAEMON_ENV_LOCK: Mutex<()> = Mutex::new(());
|
||||
|
||||
fn temp_root(prefix: &str) -> PathBuf {
|
||||
let stamp = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
@@ -1622,4 +1721,65 @@ mod tests {
|
||||
assert!(lines.iter().any(|line| line == "status=device-detected"));
|
||||
assert!(lines.iter().any(|line| line == "link_state=link=retrying"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_daemon_source_prefers_channel_over_device_path() {
|
||||
let source = select_daemon_source(Some("3"), Some("/scheme/pci/0000--00--14.3"));
|
||||
assert_eq!(source, Some(DaemonSource::Channel));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_daemon_source_uses_device_path_when_no_channel() {
|
||||
let source = select_daemon_source(None, Some("/scheme/pci/0000--00--14.3"));
|
||||
assert_eq!(source, Some(DaemonSource::DevicePath));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_daemon_source_returns_none_when_neither_set() {
|
||||
let source = select_daemon_source(None, None);
|
||||
assert_eq!(source, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_daemon_source_uses_channel_even_if_device_path_empty() {
|
||||
let source = select_daemon_source(Some("3"), None);
|
||||
assert_eq!(source, Some(DaemonSource::Channel));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_daemon_source_returns_none_for_empty_channel_and_no_device_path() {
|
||||
let source = select_daemon_source(Some(""), None);
|
||||
assert_eq!(source, Some(DaemonSource::Channel));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn daemon_target_from_env_device_path_parses_scheme_entry() {
|
||||
let _g = DAEMON_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
unsafe {
|
||||
env::set_var("PCID_DEVICE_PATH", "/scheme/pci/0000--00--14.3");
|
||||
env::remove_var("PCID_CLIENT_CHANNEL");
|
||||
}
|
||||
let target = daemon_target_from_env();
|
||||
assert_eq!(target.as_deref(), Some("0000:00:14.3"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn daemon_target_from_env_returns_none_when_neither_set() {
|
||||
let _g = DAEMON_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
unsafe {
|
||||
env::remove_var("PCID_DEVICE_PATH");
|
||||
env::remove_var("PCID_CLIENT_CHANNEL");
|
||||
}
|
||||
assert!(daemon_target_from_env().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn daemon_target_from_env_returns_none_for_invalid_device_path() {
|
||||
let _g = DAEMON_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
unsafe {
|
||||
env::set_var("PCID_DEVICE_PATH", "garbage");
|
||||
env::remove_var("PCID_CLIENT_CHANNEL");
|
||||
}
|
||||
assert!(daemon_target_from_env().is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,3 +82,55 @@ fn cli_flow_reports_bounded_intel_progression() {
|
||||
assert!(disconnect.contains("status=device-detected"));
|
||||
assert!(disconnect.contains("disconnect_result="));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cli_daemon_target_uses_device_path_when_no_channel() {
|
||||
let pci = temp_root("rbos-iwlwifi-daemon-pci");
|
||||
let fw = temp_root("rbos-iwlwifi-daemon-fw");
|
||||
write_intel_candidate(&pci);
|
||||
fs::write(fw.join("iwlwifi-bz-b0-gf-a0-92.ucode"), []).unwrap();
|
||||
fs::write(fw.join("iwlwifi-bz-b0-gf-a0.pnvm"), []).unwrap();
|
||||
|
||||
let mut cmd = Command::new(env!("CARGO_BIN_EXE_redbear-iwlwifi"));
|
||||
cmd.args(&["--daemon-target"])
|
||||
.env("REDBEAR_IWLWIFI_PCI_ROOT", &pci)
|
||||
.env("REDBEAR_IWLWIFI_FIRMWARE_ROOT", &fw)
|
||||
.env("PCID_DEVICE_PATH", "/scheme/pci/0000--00--14.3")
|
||||
.env_remove("PCID_CLIENT_CHANNEL");
|
||||
let output = cmd.output().unwrap();
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"--daemon-target failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
assert!(
|
||||
stdout.contains("daemon_target=0000:00:14.3"),
|
||||
"expected daemon_target in stdout, got: {stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cli_daemon_target_exits_when_neither_env_set() {
|
||||
let pci = temp_root("rbos-iwlwifi-daemon-none-pci");
|
||||
let fw = temp_root("rbos-iwlwifi-daemon-none-fw");
|
||||
|
||||
let mut cmd = Command::new(env!("CARGO_BIN_EXE_redbear-iwlwifi"));
|
||||
cmd.args(&["--daemon-target"])
|
||||
.env("REDBEAR_IWLWIFI_PCI_ROOT", &pci)
|
||||
.env("REDBEAR_IWLWIFI_FIRMWARE_ROOT", &fw)
|
||||
.env_remove("PCID_DEVICE_PATH")
|
||||
.env_remove("PCID_CLIENT_CHANNEL");
|
||||
let output = cmd.output().unwrap();
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"--daemon-target should fail when neither env var is set"
|
||||
);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
assert!(
|
||||
stderr.contains("no daemon target"),
|
||||
"expected error message in stderr, got: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user