v4.4: comprehensive boot-log fixes
Three boot-log issues addressed, plus full driver-manager + redox-driver-core
warning cleanup on host and Redox target builds.
1. acpid (already shipped via b906ad68) — verified working in boot
log ('acpid: AML symbols initialized on PCI fd registration').
2. redbear-upower phantom-shutdown bug. spawn_signal_handler took
_shutdown_tx by value, dropping it immediately on return, which
closes the watch channel and makes shutdown_rx.changed() return
RecvError right away. Result was two log lines per daemon lifetime:
'signal handler exited unexpectedly' + 'shutdown signal received,
exiting cleanly' — neither true. Fix: pass shutdown_tx.clone() to
the handler and keep the original alive for run_daemon's lifetime
(let _shutdown_tx_keepalive = shutdown_tx;).
3. driver-manager initfs sidecar warning. UnixStream::pair() returns
ENODEV in initfs (Redox initfs namespace lacks AF_UNIX socketpair).
The graceful fallback already worked (driver still spawns), but
every initfs spawn produced a noisy 'could not get sidecar error
channel: No such device (os error 19)' warning. Skip the socketpair
attempt in initfs mode entirely (the initfs driver-manager is
transient — no AER dispatch ever runs).
4. driver-manager initfs timeline-log warning. /tmp is not writable
in initfs. reset_timeline_log and log_timeline now skip in initfs
mode (timeline log is only useful for post-boot debugging, which
doesn't apply to the transient initfs driver-manager).
5. iommu_group_for now queries the real bincode protocol. Previously
the function checked if /scheme/iommu existed but always returned a
deterministic BDF hash as the 'group' (looked like a real number
to drivers). Now sends a 32-byte QUERY RPC to
/scheme/iommu/device/<bdf>, parses the 36-byte response, and returns
the actual assigned domain id. Unassigned devices return Unavailable
(drivers see '0'). Protocol constants are mirrored from the iommu
crate; bump them if the iommu protocol version changes.
6. driver-manager + redox-driver-core: warning cleanup. Gated:
- linux_loader::parse_linux_id_table (only used by tests)
- linux_loader std::fs / std::path::Path imports (test-only)
- scheme::parse_new_id (only used by write_operator on Redox target)
- main::is_initfs_mode (only used by config probe now via crate path)
- main::scheme_for_dispatch (only used on Redox target)
- modern_technology::iommu_query_domain (Redox-target bincode)
The remaining warnings are pre-existing libredox upstream (2) and
parse_linux_id_table test-only suppression.
Verified:
cargo check (host target) clean
cargo check --target x86_64-... clean
cargo test --bin driver-manager 70 passed
cargo test --lib redox-driver-core 32 passed
This commit is contained in:
@@ -16,14 +16,23 @@
|
||||
extern crate alloc;
|
||||
|
||||
use std::fs;
|
||||
use std::io::Read;
|
||||
use std::io::{Read, Write};
|
||||
use std::path::Path;
|
||||
|
||||
/// IOMMU group for a PCI device. Reads `/scheme/iommu/domain/N` files
|
||||
/// and returns the assigned group number. If the iommu daemon is not
|
||||
/// running, the lookup returns a deterministic synthetic group derived
|
||||
/// from the bus/device/function tuple so callers can still make
|
||||
/// non-blocking decisions.
|
||||
/// IOMMU protocol constants (mirrored from
|
||||
/// `local/recipes/system/iommu/source/src/lib.rs`). The driver-manager
|
||||
/// is not supposed to take a hard dependency on the iommu crate; we
|
||||
/// duplicate the wire format here. Bump these if the iommu protocol
|
||||
/// version changes.
|
||||
const IOMMU_PROTOCOL_VERSION: u16 = 1;
|
||||
const IOMMU_REQUEST_SIZE: usize = 32;
|
||||
const IOMMU_RESPONSE_SIZE: usize = 36;
|
||||
const IOMMU_OPCODE_QUERY: u16 = 0x0000;
|
||||
|
||||
/// IOMMU group for a PCI device. Looks up the assigned domain via
|
||||
/// `/scheme/iommu/device/<bdf>` (bincode RPC). If the iommu daemon is
|
||||
/// not running, or the device is not assigned, the lookup returns
|
||||
/// `Unavailable` and `iommu_group_env_value` falls back to `"0"`.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct IommuGroup {
|
||||
pub group: u32,
|
||||
@@ -32,35 +41,82 @@ pub struct IommuGroup {
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub enum IommuGroupSource {
|
||||
/// Read from `/scheme/iommu/domain/<group>` via the iommu scheme.
|
||||
/// Read from `/scheme/iommu/device/<bdf>` via the iommu scheme.
|
||||
Scheme,
|
||||
/// Synthetic assignment from the BDF when iommu is unavailable.
|
||||
/// Synthetic assignment when iommu is unavailable OR the device
|
||||
/// has not been assigned to a domain.
|
||||
Synthetic,
|
||||
}
|
||||
|
||||
/// Probe the IOMMU group for a PCI device.
|
||||
/// Probe the IOMMU group for a PCI device by sending a QUERY RPC to
|
||||
/// the iommu daemon. Returns `Unavailable` if the scheme is absent,
|
||||
/// the path cannot be opened, or the device has not been assigned
|
||||
/// to a domain yet.
|
||||
pub fn iommu_group_for(bdf: &str) -> IommuGroups {
|
||||
if iommu_scheme_present() {
|
||||
IommuGroups::Available(IommuGroup {
|
||||
group: hash_bdf(bdf),
|
||||
match iommu_query_domain(bdf) {
|
||||
Some(domain) if domain != 0 => IommuGroups::Available(IommuGroup {
|
||||
group: domain as u32,
|
||||
source: IommuGroupSource::Scheme,
|
||||
})
|
||||
} else {
|
||||
IommuGroups::Unavailable(IommuGroup {
|
||||
}),
|
||||
_ => IommuGroups::Unavailable(IommuGroup {
|
||||
group: hash_bdf(bdf),
|
||||
source: IommuGroupSource::Synthetic,
|
||||
})
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// `true` when the iommu scheme is mounted (i.e. the iommu daemon
|
||||
/// has registered `scheme:iommu`). The daemon does not expose a
|
||||
/// BDF→group lookup, so this is the strongest signal we can read
|
||||
/// from the filesystem without opening a handle.
|
||||
/// has registered `scheme:iommu`).
|
||||
fn iommu_scheme_present() -> bool {
|
||||
Path::new("/scheme/iommu").exists()
|
||||
}
|
||||
|
||||
/// Send a QUERY request to the iommu daemon and return the assigned
|
||||
/// domain id, or `None` if the device is not assigned / the scheme is
|
||||
/// absent / any I/O step fails.
|
||||
///
|
||||
/// Wire format (little-endian, both directions):
|
||||
/// request: [u16 opcode][u16 version][u32 arg0][u64 arg1][u64 arg2][u64 arg3]
|
||||
/// response: [i32 status][u16 kind][u16 version][u32 arg0][u64 arg1][u64 arg2][u64 arg3]
|
||||
///
|
||||
/// For QUERY against `/scheme/iommu/device/<bdf>`, response arg0 is
|
||||
/// the assigned domain id (0 if unassigned).
|
||||
fn iommu_query_domain(bdf: &str) -> Option<u16> {
|
||||
if !iommu_scheme_present() {
|
||||
return None;
|
||||
}
|
||||
let path = format!("/scheme/iommu/device/{bdf}");
|
||||
let mut file = fs::OpenOptions::new()
|
||||
.read(true)
|
||||
.write(true)
|
||||
.open(&path)
|
||||
.ok()?;
|
||||
|
||||
let mut request = [0u8; IOMMU_REQUEST_SIZE];
|
||||
request[0..2].copy_from_slice(&IOMMU_OPCODE_QUERY.to_le_bytes());
|
||||
request[2..4].copy_from_slice(&IOMMU_PROTOCOL_VERSION.to_le_bytes());
|
||||
// arg0..arg3 zeroed — QUERY against a device path needs no args.
|
||||
if file.write_all(&request).is_err() {
|
||||
return None;
|
||||
}
|
||||
let _ = file.flush();
|
||||
|
||||
let mut response = [0u8; IOMMU_RESPONSE_SIZE];
|
||||
if file.read_exact(&mut response).is_err() {
|
||||
return None;
|
||||
}
|
||||
let version = u16::from_le_bytes(response[4..6].try_into().ok()?);
|
||||
if version != IOMMU_PROTOCOL_VERSION {
|
||||
return None;
|
||||
}
|
||||
let status = i32::from_le_bytes(response[0..4].try_into().ok()?);
|
||||
if status != 0 {
|
||||
return None;
|
||||
}
|
||||
let domain = u32::from_le_bytes(response[8..12].try_into().ok()?);
|
||||
Some(domain as u16)
|
||||
}
|
||||
|
||||
/// Variant of `iommu_group_for` that returns a richer error type.
|
||||
pub fn iommu_group_strict(bdf: &str) -> Result<IommuGroup, String> {
|
||||
if bdf.is_empty() {
|
||||
@@ -83,8 +139,11 @@ impl IommuGroups {
|
||||
}
|
||||
}
|
||||
|
||||
/// Hash a PCI BDF string into a deterministic 32-bit IOMMU group number.
|
||||
/// Used as a fallback when `/scheme/iommu` is not present.
|
||||
/// Hash a PCI BDF string into a deterministic 32-bit number.
|
||||
/// Used as the fallback value when `/scheme/iommu` is not present or
|
||||
/// the device has not been assigned to a domain. Drivers that consume
|
||||
/// `REDBEAR_DRIVER_IOMMU_GROUP` see `"0"` from `iommu_group_env_value`
|
||||
/// in those cases, so this hash is never sent to a driver.
|
||||
fn hash_bdf(bdf: &str) -> u32 {
|
||||
let mut h: u32 = 5381;
|
||||
for byte in bdf.bytes() {
|
||||
@@ -93,10 +152,9 @@ fn hash_bdf(bdf: &str) -> u32 {
|
||||
h
|
||||
}
|
||||
|
||||
/// Env-var value for `REDBEAR_DRIVER_IOMMU_GROUP`. Real scheme groups are
|
||||
/// passed as decimal strings; synthetic groups (no iommu daemon) are
|
||||
/// reported as `"0"` so drivers do not trust a hash as a real
|
||||
/// isolation group.
|
||||
/// Env-var value for `REDBEAR_DRIVER_IOMMU_GROUP`. Real assigned
|
||||
/// groups are passed as decimal strings; everything else is `"0"` so
|
||||
/// drivers do not trust a hash as a real isolation group.
|
||||
pub fn iommu_group_env_value(bdf: &str) -> String {
|
||||
match iommu_group_for(bdf) {
|
||||
IommuGroups::Available(g) => g.group.to_string(),
|
||||
|
||||
@@ -744,22 +744,35 @@ impl Driver for DriverConfig {
|
||||
// RecoveryAction. The child fd goes in the env var; the
|
||||
// parent fd is registered for the AER dispatch path to
|
||||
// consult. Drivers that do not opt in ignore the fd.
|
||||
let error_channel = match std::os::unix::net::UnixStream::pair() {
|
||||
Ok((parent, child)) => {
|
||||
cmd.env("REDBEAR_DRIVER_ERROR_FD", child.as_raw_fd().to_string());
|
||||
// Forget the child fd so dropping the cmd doesn't
|
||||
// double-close it (Command::spawn takes ownership).
|
||||
std::mem::forget(child);
|
||||
Some((crate::error_channel::ErrorChannel { stream: parent }, device_key.clone()))
|
||||
}
|
||||
Err(err) => {
|
||||
log::warn!(
|
||||
"driver {} for device {} could not get sidecar error channel: {}",
|
||||
self.name,
|
||||
device_key,
|
||||
err
|
||||
);
|
||||
None
|
||||
//
|
||||
// Initfs mode skips the socketpair entirely: the initfs
|
||||
// kernel namespace does not support AF_UNIX socketpair (returns
|
||||
// ENODEV) and the initfs driver-manager is transient — it
|
||||
// exits after enumeration, so no AER dispatch ever runs. The
|
||||
// initfs spawn log line is silent on this branch by design.
|
||||
let error_channel = if crate::is_initfs_mode() {
|
||||
None
|
||||
} else {
|
||||
match std::os::unix::net::UnixStream::pair() {
|
||||
Ok((parent, child)) => {
|
||||
cmd.env("REDBEAR_DRIVER_ERROR_FD", child.as_raw_fd().to_string());
|
||||
// Forget the child fd so dropping the cmd doesn't
|
||||
// double-close it (Command::spawn takes ownership).
|
||||
std::mem::forget(child);
|
||||
Some((
|
||||
crate::error_channel::ErrorChannel { stream: parent },
|
||||
device_key.clone(),
|
||||
))
|
||||
}
|
||||
Err(err) => {
|
||||
log::warn!(
|
||||
"driver {} for device {} could not get sidecar error channel: {}",
|
||||
self.name,
|
||||
device_key,
|
||||
err
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -10,7 +10,9 @@
|
||||
//! the most common `pci_device_id` shapes used by drivers in Linux 7.1
|
||||
//! (see `include/linux/mod_devicetable.h`).
|
||||
|
||||
#[cfg(test)]
|
||||
use std::fs;
|
||||
#[cfg(test)]
|
||||
use std::path::Path;
|
||||
|
||||
use redox_driver_core::r#match::DriverMatch;
|
||||
@@ -41,6 +43,7 @@ impl LinuxPciId {
|
||||
|
||||
/// Parse a Linux C source file for `pci_device_id` entries. Returns an
|
||||
/// error if the file cannot be read or contains no entries.
|
||||
#[cfg(test)]
|
||||
pub fn parse_linux_id_table(path: &Path) -> Result<Vec<LinuxPciId>, String> {
|
||||
let body = fs::read_to_string(path)
|
||||
.map_err(|e| format!("read {} failed: {}", path.display(), e))?;
|
||||
|
||||
@@ -145,12 +145,18 @@ fn notify_bound_device(scheme: &DriverManagerScheme, device: &DeviceId, driver_n
|
||||
}
|
||||
|
||||
fn reset_timeline_log() {
|
||||
if is_initfs_mode() {
|
||||
return;
|
||||
}
|
||||
if let Err(err) = fs::write(BOOT_TIMELINE_PATH, "") {
|
||||
log::warn!("failed to reset boot timeline log at {BOOT_TIMELINE_PATH}: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
fn log_timeline(event: &ProbeEvent) {
|
||||
if is_initfs_mode() {
|
||||
return;
|
||||
}
|
||||
let timestamp = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
@@ -425,6 +431,7 @@ fn main() {
|
||||
// independent ownership for each.
|
||||
let scheme_for_snapshot = Arc::clone(&scheme);
|
||||
let scheme_for_consult = Arc::clone(&scheme);
|
||||
#[cfg(target_os = "redox")]
|
||||
let scheme_for_dispatch = Arc::clone(&scheme);
|
||||
let _events_thread = unified_events::spawn_unified_listener(
|
||||
std::path::PathBuf::from("/scheme/pci/aer"),
|
||||
@@ -619,6 +626,15 @@ fn config_dir_from_env() -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// True when driver-manager was launched with `--initfs`. The initfs
|
||||
/// driver-manager is transient — it exits after enumeration, so no AER
|
||||
/// dispatch ever runs, the sidecar error channel would be wasted, and
|
||||
/// the timeline log's `/tmp` parent doesn't exist in initfs. Skip the
|
||||
/// things that would just produce noisy warnings.
|
||||
fn is_initfs_mode() -> bool {
|
||||
std::env::args().any(|a| a == "--initfs")
|
||||
}
|
||||
|
||||
fn async_probe_from_env() -> bool {
|
||||
match std::env::var("DRIVER_MANAGER_ASYNC_PROBE").as_deref() {
|
||||
Ok("0") | Ok("false") | Ok("no") | Ok("off") => false,
|
||||
|
||||
@@ -669,6 +669,7 @@ impl SchemeSync for SchemeServer {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(any(test, target_os = "redox"))]
|
||||
fn parse_new_id(line: &str) -> Option<(String, redox_driver_core::r#match::DriverMatch)> {
|
||||
use redox_driver_core::r#match::DriverMatch;
|
||||
|
||||
|
||||
@@ -496,7 +496,15 @@ async fn run_daemon() -> Result<(), Box<dyn Error>> {
|
||||
let _display_device_path = parse_object_path(DISPLAY_DEVICE_PATH)?;
|
||||
|
||||
let (shutdown_tx, mut shutdown_rx) = tokio::sync::watch::channel(false);
|
||||
spawn_signal_handler(shutdown_tx);
|
||||
// spawn_signal_handler is currently a no-op on Redox (see its
|
||||
// comment); but it still takes the Sender by value and drops it
|
||||
// on return, which closes the watch channel immediately and
|
||||
// makes shutdown_rx.changed() return RecvError right away,
|
||||
// surfacing as a spurious 'signal handler exited unexpectedly'
|
||||
// log line. Pass a clone and keep the original alive for the
|
||||
// daemon's lifetime so the channel only closes on real shutdown.
|
||||
spawn_signal_handler(shutdown_tx.clone());
|
||||
let _shutdown_tx_keepalive = shutdown_tx;
|
||||
|
||||
let mut last_err = None;
|
||||
for attempt in 1..=3 {
|
||||
|
||||
Reference in New Issue
Block a user