From e7fe3183b0c3aa4c41cc55ac548ace4924bbf29d Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 2 Mar 2025 12:36:35 +0100 Subject: [PATCH] Make all pcid_interface methods abort the process on errors Effectively the only way to recover from errors in the communication with pcid is by restarting the driver from scratch possibly after restarting pcid. As such moving the aborts from individual drivers to pcid_interface simplifies drivers while at the same time allowing nicer error messages. --- Cargo.lock | 1 - audio/ac97d/src/main.rs | 3 +- audio/ihdad/src/main.rs | 24 +--- common/Cargo.toml | 2 +- graphics/bgad/src/main.rs | 3 +- graphics/virtio-gpud/src/main.rs | 2 +- net/e1000d/src/main.rs | 8 +- net/ixgbed/src/main.rs | 5 +- net/rtl8139d/src/main.rs | 37 ++--- net/rtl8168d/src/main.rs | 37 ++--- net/virtio-netd/src/main.rs | 2 +- pcid-spawner/src/main.rs | 2 +- pcid/Cargo.toml | 1 - pcid/src/driver_interface/mod.rs | 225 +++++++++++++++++-------------- storage/ahcid/src/main.rs | 7 +- storage/ided/src/main.rs | 3 +- storage/nvmed/src/main.rs | 29 ++-- storage/virtio-blkd/src/main.rs | 2 +- vboxd/src/main.rs | 8 +- virtio-core/src/arch/x86.rs | 6 +- virtio-core/src/probe.rs | 4 +- virtio-core/src/transport.rs | 8 -- xhcid/src/main.rs | 34 ++--- 23 files changed, 191 insertions(+), 262 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 484b372504..e101de9edb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -971,7 +971,6 @@ dependencies = [ "redox-scheme 0.4.0", "redox_syscall", "serde", - "thiserror", ] [[package]] diff --git a/audio/ac97d/src/main.rs b/audio/ac97d/src/main.rs index a7a8c0327f..92bd69db41 100644 --- a/audio/ac97d/src/main.rs +++ b/audio/ac97d/src/main.rs @@ -18,8 +18,7 @@ use syscall::{Packet, SchemeBlockMut}; pub mod device; fn main() { - let pcid_handle = - PciFunctionHandle::connect_default().expect("ac97d: failed to setup channel to pcid"); + let pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let mut name = pci_config.func.name(); diff --git a/audio/ihdad/src/main.rs b/audio/ihdad/src/main.rs index 231b32e926..e9a91590ec 100755 --- a/audio/ihdad/src/main.rs +++ b/audio/ihdad/src/main.rs @@ -31,19 +31,14 @@ QEMU ICH9 8086:293E fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { let pci_config = pcid_handle.config(); - let all_pci_features = pcid_handle - .fetch_all_features() - .expect("ihdad: failed to fetch pci features"); + let all_pci_features = pcid_handle.fetch_all_features(); log::debug!("PCI FEATURES: {:?}", all_pci_features); let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); if has_msi && !has_msix { - let capability = match pcid_handle - .feature_info(PciFeature::Msi) - .expect("ihdad: failed to retrieve the MSI capability structure from pcid") - { + let capability = match pcid_handle.feature_info(PciFeature::Msi) { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), }; @@ -61,13 +56,9 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; - pcid_handle - .set_feature_info(SetFeatureInfo::Msi(set_feature_info)) - .expect("ihdad: failed to set feature info"); + pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)); - pcid_handle - .enable_feature(PciFeature::Msi) - .expect("ihdad: failed to enable MSI"); + pcid_handle.enable_feature(PciFeature::Msi); log::debug!("Enabled MSI"); interrupt_handle @@ -103,8 +94,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { log::LevelFilter::Info, ); - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("ihdad: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); @@ -113,9 +103,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { log::info!(" + IHDA {}", pci_config.func.display()); - let address = unsafe { pcid_handle.map_bar(0).expect("ihdad") } - .ptr - .as_ptr() as usize; + let address = unsafe { pcid_handle.map_bar(0) }.ptr.as_ptr() as usize; //TODO: MSI-X let mut irq_file = get_int_method(&mut pcid_handle); diff --git a/common/Cargo.toml b/common/Cargo.toml index 50437c7d4b..278e00c9c3 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -10,5 +10,5 @@ license = "MIT" [dependencies] libredox = "0.1.3" log = "0.4" -redox_syscall = "0.5" +redox_syscall = { version = "0.5", features = ["std"] } redox-log = "0.1.2" diff --git a/graphics/bgad/src/main.rs b/graphics/bgad/src/main.rs index a208db2b27..72824703ed 100644 --- a/graphics/bgad/src/main.rs +++ b/graphics/bgad/src/main.rs @@ -17,8 +17,7 @@ mod bga; mod scheme; fn main() { - let pcid_handle = - PciFunctionHandle::connect_default().expect("bgad: failed to setup channel to pcid"); + let pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let mut name = pci_config.func.name(); diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index 8170c6f33b..6b0b62f3fe 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -361,7 +361,7 @@ impl XferToHost2d { static DEVICE: spin::Once = spin::Once::new(); fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { - let mut pcid_handle = PciFunctionHandle::connect_default()?; + let mut pcid_handle = PciFunctionHandle::connect_default(); // Double check that we have the right device. // diff --git a/net/e1000d/src/main.rs b/net/e1000d/src/main.rs index 4d2299d89e..412986739a 100644 --- a/net/e1000d/src/main.rs +++ b/net/e1000d/src/main.rs @@ -8,8 +8,7 @@ use pcid_interface::PciFunctionHandle; pub mod device; fn main() { - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("e1000d: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let mut name = pci_config.func.name(); @@ -25,10 +24,7 @@ fn main() { redox_daemon::Daemon::new(move |daemon| { let mut irq_file = irq.irq_handle("e1000d"); - let address = unsafe { pcid_handle.map_bar(0) } - .expect("e1000d") - .ptr - .as_ptr() as usize; + let address = unsafe { pcid_handle.map_bar(0) }.ptr.as_ptr() as usize; let device = unsafe { device::Intel8254x::new(address).expect("e1000d: failed to allocate device") }; diff --git a/net/ixgbed/src/main.rs b/net/ixgbed/src/main.rs index a066c3db0a..c6ea98600f 100644 --- a/net/ixgbed/src/main.rs +++ b/net/ixgbed/src/main.rs @@ -10,8 +10,7 @@ pub mod device; mod ixgbe; fn main() { - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("ixgbed: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let mut name = pci_config.func.name(); @@ -27,7 +26,7 @@ fn main() { redox_daemon::Daemon::new(move |daemon| { let mut irq_file = irq.irq_handle("ixgbed"); - let mapped_bar = unsafe { pcid_handle.map_bar(0) }.expect("ixgbed"); + let mapped_bar = unsafe { pcid_handle.map_bar(0) }; let address = mapped_bar.ptr.as_ptr(); let size = mapped_bar.bar_size; diff --git a/net/rtl8139d/src/main.rs b/net/rtl8139d/src/main.rs index a8ff4dab49..c0272136c7 100644 --- a/net/rtl8139d/src/main.rs +++ b/net/rtl8139d/src/main.rs @@ -47,19 +47,14 @@ impl MappedMsixRegs { fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { let pci_config = pcid_handle.config(); - let all_pci_features = pcid_handle - .fetch_all_features() - .expect("rtl8139d: failed to fetch pci features"); + let all_pci_features = pcid_handle.fetch_all_features(); log::info!("PCI FEATURES: {:?}", all_pci_features); let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); if has_msi && !has_msix { - let capability = match pcid_handle - .feature_info(PciFeature::Msi) - .expect("rtl8139d: failed to retrieve the MSI capability structure from pcid") - { + let capability = match pcid_handle.feature_info(PciFeature::Msi) { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), }; @@ -77,27 +72,20 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; - pcid_handle - .set_feature_info(SetFeatureInfo::Msi(set_feature_info)) - .expect("rtl8139d: failed to set feature info"); + pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)); - pcid_handle - .enable_feature(PciFeature::Msi) - .expect("rtl8139d: failed to enable MSI"); + pcid_handle.enable_feature(PciFeature::Msi); log::info!("Enabled MSI"); interrupt_handle } else if has_msix { - let msix_info = match pcid_handle - .feature_info(PciFeature::MsiX) - .expect("rtl8139d: failed to retrieve the MSI-X capability structure from pcid") - { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX) { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; msix_info.validate(pci_config.func.bars); - let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar).expect("rtl8139d") } + let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar) } .ptr .as_ptr() as usize; @@ -127,9 +115,7 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { interrupt_handle }; - pcid_handle - .enable_feature(PciFeature::MsiX) - .expect("rtl8139d: failed to enable MSI-X"); + pcid_handle.enable_feature(PciFeature::MsiX); log::info!("Enabled MSI-X"); method @@ -161,11 +147,7 @@ fn map_bar(pcid_handle: &mut PciFunctionHandle) -> *mut u8 { for &barnum in &[2, 1] { match config.func.bars[usize::from(barnum)] { pcid_interface::PciBar::Memory32 { .. } | pcid_interface::PciBar::Memory64 { .. } => unsafe { - return pcid_handle - .map_bar(barnum) - .expect("rtl8139d: failed to map address") - .ptr - .as_ptr(); + return pcid_handle.map_bar(barnum).ptr.as_ptr(); }, other => log::warn!("BAR {} is {:?} instead of memory BAR", barnum, other), } @@ -182,8 +164,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { log::LevelFilter::Info, ); - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("rtl8139d: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); diff --git a/net/rtl8168d/src/main.rs b/net/rtl8168d/src/main.rs index 50fefba7aa..44150572f7 100644 --- a/net/rtl8168d/src/main.rs +++ b/net/rtl8168d/src/main.rs @@ -47,19 +47,14 @@ impl MappedMsixRegs { fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { let pci_config = pcid_handle.config(); - let all_pci_features = pcid_handle - .fetch_all_features() - .expect("rtl8168d: failed to fetch pci features"); + let all_pci_features = pcid_handle.fetch_all_features(); log::info!("PCI FEATURES: {:?}", all_pci_features); let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); if has_msi && !has_msix { - let capability = match pcid_handle - .feature_info(PciFeature::Msi) - .expect("rtl8168d: failed to retrieve the MSI capability structure from pcid") - { + let capability = match pcid_handle.feature_info(PciFeature::Msi) { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), }; @@ -77,27 +72,20 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; - pcid_handle - .set_feature_info(SetFeatureInfo::Msi(set_feature_info)) - .expect("rtl8168d: failed to set feature info"); + pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)); - pcid_handle - .enable_feature(PciFeature::Msi) - .expect("rtl8168d: failed to enable MSI"); + pcid_handle.enable_feature(PciFeature::Msi); log::info!("Enabled MSI"); interrupt_handle } else if has_msix { - let msix_info = match pcid_handle - .feature_info(PciFeature::MsiX) - .expect("rtl8168d: failed to retrieve the MSI-X capability structure from pcid") - { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX) { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; msix_info.validate(pci_config.func.bars); - let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar).expect("rtl8168d") } + let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar) } .ptr .as_ptr() as usize; @@ -127,9 +115,7 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> File { interrupt_handle }; - pcid_handle - .enable_feature(PciFeature::MsiX) - .expect("rtl8168d: failed to enable MSI-X"); + pcid_handle.enable_feature(PciFeature::MsiX); log::info!("Enabled MSI-X"); method @@ -161,11 +147,7 @@ fn map_bar(pcid_handle: &mut PciFunctionHandle) -> *mut u8 { for &barnum in &[2, 1] { match config.func.bars[usize::from(barnum)] { pcid_interface::PciBar::Memory32 { .. } | pcid_interface::PciBar::Memory64 { .. } => unsafe { - return pcid_handle - .map_bar(barnum) - .expect("rtl8168d: failed to map address") - .ptr - .as_ptr(); + return pcid_handle.map_bar(barnum).ptr.as_ptr(); }, other => log::warn!("BAR {} is {:?} instead of memory BAR", barnum, other), } @@ -182,8 +164,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { log::LevelFilter::Info, ); - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("rtl8168d: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); diff --git a/net/virtio-netd/src/main.rs b/net/virtio-netd/src/main.rs index c2c4a798dc..ac4c2fa92b 100644 --- a/net/virtio-netd/src/main.rs +++ b/net/virtio-netd/src/main.rs @@ -28,7 +28,7 @@ static_assertions::const_assert_eq!(core::mem::size_of::(), 12); const MAX_BUFFER_LEN: usize = 65535; fn deamon(daemon: redox_daemon::Daemon) -> Result<(), Box> { - let mut pcid_handle = PciFunctionHandle::connect_default()?; + let mut pcid_handle = PciFunctionHandle::connect_default(); // Double check that we have the right device. // diff --git a/pcid-spawner/src/main.rs b/pcid-spawner/src/main.rs index a8ac5d5e13..e99ce2e320 100644 --- a/pcid-spawner/src/main.rs +++ b/pcid-spawner/src/main.rs @@ -83,7 +83,7 @@ fn main() -> Result<()> { log::info!("pcid-spawner: spawn {:?}", command); - handle.enable_device()?; + handle.enable_device(); let channel_fd = handle.into_inner_fd(); command.env("PCID_CLIENT_CHANNEL", channel_fd.to_string()); diff --git a/pcid/Cargo.toml b/pcid/Cargo.toml index b2d6b51317..78f0192346 100644 --- a/pcid/Cargo.toml +++ b/pcid/Cargo.toml @@ -23,7 +23,6 @@ redox-daemon = "0.1" redox-scheme = "0.4" redox_syscall = "0.5.9" serde = { version = "1", features = ["derive"] } -thiserror = "1" common = { path = "../common" } libredox = "0.1.3" diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index 7365276dc9..d1fa8d733d 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -1,13 +1,12 @@ -use std::fmt; use std::fs::File; use std::io::prelude::*; use std::os::fd::{FromRawFd, IntoRawFd, RawFd}; use std::path::Path; use std::ptr::NonNull; use std::{env, io}; +use std::{fmt, process}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; -use thiserror::Error; pub use bar::PciBar; pub use cap::VendorSpecificCapability; @@ -186,25 +185,6 @@ pub enum PciFeatureInfo { MsiX(msi::MsixInfo), } -#[derive(Debug, Error)] -pub enum PcidClientHandleError { - #[error("i/o error: {0}")] - IoError(#[from] io::Error), - - #[error("JSON ser/de error: {0}")] - SerializationError(#[from] bincode::Error), - - #[error("environment variable error: {0}")] - EnvError(#[from] env::VarError), - - #[error("malformed fd: {0}")] - EnvValidityError(std::num::ParseIntError), - - #[error("invalid response: {0:?}")] - InvalidResponse(PcidClientResponse), -} -pub type Result = std::result::Result; - // TODO: Remove these "features" and just go strait to the actual thing. #[derive(Debug, Default, Serialize, Deserialize)] @@ -286,75 +266,91 @@ pub struct PciFunctionHandle { mapped_bars: [Option; 6], } -#[doc(hidden)] -pub fn send(w: &mut W, message: &T) -> Result<()> { +fn send(w: &mut File, message: &T) { let mut data = Vec::new(); - bincode::serialize_into(&mut data, message)?; - assert_eq!(w.write(&data)?, data.len()); - Ok(()) + bincode::serialize_into(&mut data, message).expect("couldn't serialize pcid message"); + match w.write(&data) { + Ok(len) => assert_eq!(len, data.len()), + Err(err) => { + log::error!("writing pcid request failed: {err}"); + process::exit(1); + } + } } -#[doc(hidden)] -pub fn recv(r: &mut R) -> Result { +fn recv(r: &mut File) -> T { let mut length_bytes = [0u8; 8]; - r.read_exact(&mut length_bytes)?; + if let Err(err) = r.read_exact(&mut length_bytes) { + log::error!("reading pcid response length failed: {err}"); + process::exit(1); + } let length = u64::from_le_bytes(length_bytes); if length > 0x100_000 { panic!("pcid_interface: buffer too large"); } let mut data = vec![0u8; length as usize]; - r.read_exact(&mut data)?; + if let Err(err) = r.read_exact(&mut data) { + log::error!("reading pcid response failed: {err}"); + process::exit(1); + } - Ok(bincode::deserialize_from(&data[..])?) + bincode::deserialize_from(&data[..]).expect("couldn't deserialize pcid message") } impl PciFunctionHandle { - pub fn connect_default() -> Result { - let channel_fd = env::var("PCID_CLIENT_CHANNEL")? - .parse::() - .map_err(PcidClientHandleError::EnvValidityError)?; + pub fn connect_default() -> Self { + let channel_fd = match env::var("PCID_CLIENT_CHANNEL") { + Ok(channel_fd) => channel_fd, + Err(err) => { + log::error!("PCID_CLIENT_CHANNEL invalid: {err}"); + process::exit(1); + } + }; + let channel_fd = match channel_fd.parse::() { + Ok(channel_fd) => channel_fd, + Err(err) => { + log::error!("PCID_CLIENT_CHANNEL invalid: {err}"); + process::exit(1); + } + }; Self::connect_common(channel_fd) } - pub fn connect_by_path(device_path: &Path) -> Result { + pub fn connect_by_path(device_path: &Path) -> io::Result { let channel_fd = syscall::open( device_path.join("channel").to_str().unwrap(), syscall::O_RDWR, ) - .map_err(|err| { - PcidClientHandleError::IoError(io::Error::other(format!( - "failed to open pcid channel: {}", - err - ))) - })?; - Self::connect_common(channel_fd as RawFd) + .map_err(|err| io::Error::other(format!("failed to open pcid channel: {}", err)))?; + Ok(Self::connect_common(channel_fd as RawFd)) } - fn connect_common( - channel_fd: i32, - ) -> std::result::Result { + fn connect_common(channel_fd: i32) -> PciFunctionHandle { let mut channel = unsafe { File::from_raw_fd(channel_fd) }; - send(&mut channel, &PcidClientRequest::RequestConfig)?; - let config = match recv(&mut channel)? { + send(&mut channel, &PcidClientRequest::RequestConfig); + let config = match recv(&mut channel) { PcidClientResponse::Config(a) => a, - other => return Err(PcidClientHandleError::InvalidResponse(other)), + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } }; - Ok(Self { + Self { channel, config, mapped_bars: [const { None }; 6], - }) + } } pub fn into_inner_fd(self) -> RawFd { self.channel.into_raw_fd() } - fn send(&mut self, req: &PcidClientRequest) -> Result<()> { + fn send(&mut self, req: &PcidClientRequest) { send(&mut self.channel, req) } - fn recv(&mut self) -> Result { + fn recv(&mut self) -> PcidClientResponse { recv(&mut self.channel) } @@ -362,73 +358,97 @@ impl PciFunctionHandle { self.config.clone() } - pub fn enable_device(&mut self) -> Result<()> { - self.send(&PcidClientRequest::EnableDevice)?; - match self.recv()? { - PcidClientResponse::EnabledDevice => Ok(()), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub fn enable_device(&mut self) { + self.send(&PcidClientRequest::EnableDevice); + match self.recv() { + PcidClientResponse::EnabledDevice => {} + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } - pub fn get_vendor_capabilities(&mut self) -> Result> { - self.send(&PcidClientRequest::RequestVendorCapabilities)?; - match self.recv()? { - PcidClientResponse::VendorCapabilities(a) => Ok(a), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub fn get_vendor_capabilities(&mut self) -> Vec { + self.send(&PcidClientRequest::RequestVendorCapabilities); + match self.recv() { + PcidClientResponse::VendorCapabilities(a) => a, + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } // FIXME turn into struct with bool fields - pub fn fetch_all_features(&mut self) -> Result> { - self.send(&PcidClientRequest::RequestFeatures)?; - match self.recv()? { - PcidClientResponse::AllFeatures(a) => Ok(a), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub fn fetch_all_features(&mut self) -> Vec { + self.send(&PcidClientRequest::RequestFeatures); + match self.recv() { + PcidClientResponse::AllFeatures(a) => a, + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } - pub fn enable_feature(&mut self, feature: PciFeature) -> Result<()> { - self.send(&PcidClientRequest::EnableFeature(feature))?; - match self.recv()? { - PcidClientResponse::FeatureEnabled(feat) if feat == feature => Ok(()), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub fn enable_feature(&mut self, feature: PciFeature) { + self.send(&PcidClientRequest::EnableFeature(feature)); + match self.recv() { + PcidClientResponse::FeatureEnabled(feat) if feat == feature => {} + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } - pub fn feature_info(&mut self, feature: PciFeature) -> Result { - self.send(&PcidClientRequest::FeatureInfo(feature))?; - match self.recv()? { - PcidClientResponse::FeatureInfo(feat, info) if feat == feature => Ok(info), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub fn feature_info(&mut self, feature: PciFeature) -> PciFeatureInfo { + self.send(&PcidClientRequest::FeatureInfo(feature)); + match self.recv() { + PcidClientResponse::FeatureInfo(feat, info) if feat == feature => info, + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } - pub fn set_feature_info(&mut self, info: SetFeatureInfo) -> Result<()> { - self.send(&PcidClientRequest::SetFeatureInfo(info))?; - match self.recv()? { - PcidClientResponse::SetFeatureInfo(_) => Ok(()), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub fn set_feature_info(&mut self, info: SetFeatureInfo) { + self.send(&PcidClientRequest::SetFeatureInfo(info)); + match self.recv() { + PcidClientResponse::SetFeatureInfo(_) => {} + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } - pub unsafe fn read_config(&mut self, offset: u16) -> Result { - self.send(&PcidClientRequest::ReadConfig(offset))?; - match self.recv()? { - PcidClientResponse::ReadConfig(value) => Ok(value), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub unsafe fn read_config(&mut self, offset: u16) -> u32 { + self.send(&PcidClientRequest::ReadConfig(offset)); + match self.recv() { + PcidClientResponse::ReadConfig(value) => value, + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } - pub unsafe fn write_config(&mut self, offset: u16, value: u32) -> Result<()> { - self.send(&PcidClientRequest::WriteConfig(offset, value))?; - match self.recv()? { - PcidClientResponse::WriteConfig => Ok(()), - other => Err(PcidClientHandleError::InvalidResponse(other)), + pub unsafe fn write_config(&mut self, offset: u16, value: u32) { + self.send(&PcidClientRequest::WriteConfig(offset, value)); + match self.recv() { + PcidClientResponse::WriteConfig => {} + other => { + log::error!("received wrong pcid response: {other:?}"); + process::exit(1); + } } } - pub unsafe fn map_bar(&mut self, bir: u8) -> Result<&MappedBar> { + pub unsafe fn map_bar(&mut self, bir: u8) -> &MappedBar { let mapped_bar = &mut self.mapped_bars[bir as usize]; if let Some(mapped_bar) = mapped_bar { - Ok(mapped_bar) + mapped_bar } else { let (bar, bar_size) = self.config.func.bars[bir as usize].expect_mem(); - let ptr = unsafe { + let ptr = match unsafe { common::physmap( bar, bar_size, @@ -436,13 +456,18 @@ impl PciFunctionHandle { // FIXME once the kernel supports this use write-through for prefetchable BAR common::MemoryType::Uncacheable, ) - } - .map_err(|err| io::Error::other(format!("failed to map BAR at {bar:016X}: {err}")))?; + } { + Ok(ptr) => ptr, + Err(err) => { + log::error!("failed to map BAR at {bar:016X}: {err}"); + process::exit(1); + } + }; - Ok(mapped_bar.insert(MappedBar { + mapped_bar.insert(MappedBar { ptr: NonNull::new(ptr.cast::()).expect("Mapping a BAR resulted in a nullptr"), bar_size, - })) + }) } } } diff --git a/storage/ahcid/src/main.rs b/storage/ahcid/src/main.rs index 89467e04ce..1c5239c1b5 100644 --- a/storage/ahcid/src/main.rs +++ b/storage/ahcid/src/main.rs @@ -25,8 +25,7 @@ fn main() { } fn daemon(daemon: redox_daemon::Daemon) -> ! { - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("ahcid: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let mut name = pci_config.func.name(); @@ -47,9 +46,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { info!(" + AHCI {}", pci_config.func.display()); - let address = unsafe { pcid_handle.map_bar(5).expect("ahcid") } - .ptr - .as_ptr() as usize; + let address = unsafe { pcid_handle.map_bar(5) }.ptr.as_ptr() as usize; { let scheme_name = format!("disk.{}", name); let socket = Socket::nonblock(&scheme_name).expect("ahcid: failed to create disk scheme"); diff --git a/storage/ided/src/main.rs b/storage/ided/src/main.rs index 068477cbd7..2bf9c7c3e9 100644 --- a/storage/ided/src/main.rs +++ b/storage/ided/src/main.rs @@ -28,8 +28,7 @@ fn main() { } fn daemon(daemon: redox_daemon::Daemon) -> ! { - let pcid_handle = - PciFunctionHandle::connect_default().expect("ided: failed to setup channel to pcid"); + let pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); diff --git a/storage/nvmed/src/main.rs b/storage/nvmed/src/main.rs index 420f1e17e7..93bd7e47c9 100644 --- a/storage/nvmed/src/main.rs +++ b/storage/nvmed/src/main.rs @@ -26,7 +26,7 @@ fn get_int_method( log::trace!("Begin get_int_method"); use pcid_interface::irq_helpers; - let features = pcid_handle.fetch_all_features().unwrap(); + let features = pcid_handle.fetch_all_features(); let has_msi = features.iter().any(|feature| feature.is_msi()); let has_msix = features.iter().any(|feature| feature.is_msix()); @@ -37,13 +37,13 @@ fn get_int_method( use self::nvme::MappedMsixRegs; use pcid_interface::msi::MsixTableEntry; - let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).unwrap() { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX) { PciFeatureInfo::MsiX(msix) => msix, _ => unreachable!(), }; msix_info.validate(function.bars); fn bar_base(pcid_handle: &mut PciFunctionHandle, bir: u8) -> Result> { - Ok(unsafe { pcid_handle.map_bar(bir) }.expect("nvmed").ptr) + Ok(unsafe { pcid_handle.map_bar(bir) }.ptr) } let table_bar_base: *mut u8 = bar_base(pcid_handle, msix_info.table_bar)?.as_ptr(); let table_base = unsafe { table_bar_base.offset(msix_info.table_offset as isize) }; @@ -59,7 +59,7 @@ fn get_int_method( table_entry.mask(); } - pcid_handle.enable_feature(PciFeature::MsiX).unwrap(); + pcid_handle.enable_feature(PciFeature::MsiX); let (msix_vector_number, irq_handle) = { let entry: &mut MsixTableEntry = &mut table_entries[0]; @@ -84,7 +84,7 @@ fn get_int_method( Ok((interrupt_method, interrupt_sources)) } else if has_msi { // Message signaled interrupts. - let msi_info = match pcid_handle.feature_info(PciFeature::Msi).unwrap() { + let msi_info = match pcid_handle.feature_info(PciFeature::Msi) { PciFeatureInfo::Msi(msi) => msi, _ => unreachable!(), }; @@ -97,13 +97,11 @@ fn get_int_method( let (msg_addr_and_data, irq_handle) = irq_helpers::allocate_single_interrupt_vector_for_msi(bsp_cpu_id); - pcid_handle - .set_feature_info(SetFeatureInfo::Msi(MsiSetFeatureInfo { - message_address_and_data: Some(msg_addr_and_data), - multi_message_enable: Some(0), // enable 2^0=1 vectors - mask_bits: None, - })) - .unwrap(); + pcid_handle.set_feature_info(SetFeatureInfo::Msi(MsiSetFeatureInfo { + message_address_and_data: Some(msg_addr_and_data), + multi_message_enable: Some(0), // enable 2^0=1 vectors + mask_bits: None, + })); (0, irq_handle) }; @@ -115,7 +113,7 @@ fn get_int_method( let interrupt_sources = InterruptSources::Msi(std::iter::once((msi_vector_number, irq_handle)).collect()); - pcid_handle.enable_feature(PciFeature::Msi).unwrap(); + pcid_handle.enable_feature(PciFeature::Msi); Ok((interrupt_method, interrupt_sources)) } else if let Some(irq) = function.legacy_interrupt_line { @@ -146,8 +144,7 @@ fn main() { redox_daemon::Daemon::new(daemon).expect("nvmed: failed to daemonize"); } fn daemon(daemon: redox_daemon::Daemon) -> ! { - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("nvmed: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let scheme_name = format!("disk.{}-nvme", pci_config.func.name()); @@ -162,7 +159,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { log::debug!("NVME PCI CONFIG: {:?}", pci_config); - let address = unsafe { pcid_handle.map_bar(0).expect("nvmed").ptr }; + let address = unsafe { pcid_handle.map_bar(0).ptr }; let socket = Socket::create(&scheme_name).expect("nvmed: failed to create disk scheme"); diff --git a/storage/virtio-blkd/src/main.rs b/storage/virtio-blkd/src/main.rs index 2db8ab4779..ac94312eef 100644 --- a/storage/virtio-blkd/src/main.rs +++ b/storage/virtio-blkd/src/main.rs @@ -107,7 +107,7 @@ pub struct BlockVirtRequest { const_assert_eq!(core::mem::size_of::(), 16); fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { - let mut pcid_handle = PciFunctionHandle::connect_default()?; + let mut pcid_handle = PciFunctionHandle::connect_default(); // Double check that we have the right device. // diff --git a/vboxd/src/main.rs b/vboxd/src/main.rs index 63935cf0a8..cac69b6d0d 100644 --- a/vboxd/src/main.rs +++ b/vboxd/src/main.rs @@ -191,8 +191,7 @@ impl VboxGuestInfo { } fn main() { - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("vboxd: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let mut name = pci_config.func.name(); @@ -237,10 +236,7 @@ fn main() { let mut irq_file = irq.irq_handle("vboxd"); let mut port = Pio::::new(bar0 as u16); - let address = unsafe { pcid_handle.map_bar(1) } - .expect("vboxd") - .ptr - .as_ptr(); + let address = unsafe { pcid_handle.map_bar(1) }.ptr.as_ptr(); { let vmmdev = unsafe { &mut *(address as *mut VboxVmmDev) }; diff --git a/virtio-core/src/arch/x86.rs b/virtio-core/src/arch/x86.rs index ebea0f9aa7..7275c63e41 100644 --- a/virtio-core/src/arch/x86.rs +++ b/virtio-core/src/arch/x86.rs @@ -12,13 +12,13 @@ pub fn enable_msix(pcid_handle: &mut PciFunctionHandle) -> Result { let pci_config = pcid_handle.config(); // Extended message signaled interrupts. - let msix_info = match pcid_handle.feature_info(PciFeature::MsiX)? { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX) { PciFeatureInfo::MsiX(capability) => capability, _ => unreachable!(), }; msix_info.validate(pci_config.func.bars); - let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar)? } + let bar_address = unsafe { pcid_handle.map_bar(msix_info.table_bar) } .ptr .as_ptr() as usize; let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; @@ -43,7 +43,7 @@ pub fn enable_msix(pcid_handle: &mut PciFunctionHandle) -> Result { interrupt_handle }; - pcid_handle.enable_feature(PciFeature::MsiX)?; + pcid_handle.enable_feature(PciFeature::MsiX); log::info!("virtio: using MSI-X (interrupt_handle={interrupt_handle:?})"); Ok(interrupt_handle) diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index aa67b2e71c..25cf1efb92 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -66,7 +66,7 @@ pub fn probe_device(pcid_handle: &mut PciFunctionHandle) -> Result Result for Error { - fn from(value: pcid_interface::PcidClientHandleError) -> Self { - Self::PcidClientHandle(value) - } -} - /// Returns the queue part sizes in bytes. /// /// ## Reference diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 7f9dc09b39..67bec0174c 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -62,19 +62,14 @@ fn get_int_method( ) -> (Option, InterruptMethod) { let pci_config = pcid_handle.config(); - let all_pci_features = pcid_handle - .fetch_all_features() - .expect("xhcid: failed to fetch pci features"); + let all_pci_features = pcid_handle.fetch_all_features(); log::debug!("XHCI PCI FEATURES: {:?}", all_pci_features); let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); if has_msi && !has_msix { - let mut capability = match pcid_handle - .feature_info(PciFeature::Msi) - .expect("xhcid: failed to retrieve the MSI capability structure from pcid") - { + let mut capability = match pcid_handle.feature_info(PciFeature::Msi) { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), }; @@ -92,21 +87,14 @@ fn get_int_method( message_address_and_data: Some(msg_addr_and_data), mask_bits: None, }; - pcid_handle - .set_feature_info(SetFeatureInfo::Msi(set_feature_info)) - .expect("xhcid: failed to set feature info"); + pcid_handle.set_feature_info(SetFeatureInfo::Msi(set_feature_info)); - pcid_handle - .enable_feature(PciFeature::Msi) - .expect("xhcid: failed to enable MSI"); + pcid_handle.enable_feature(PciFeature::Msi); log::debug!("Enabled MSI"); (Some(interrupt_handle), InterruptMethod::Msi) } else if has_msix { - let msix_info = match pcid_handle - .feature_info(PciFeature::MsiX) - .expect("xhcid: failed to retrieve the MSI-X capability structure from pcid") - { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX) { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; @@ -142,9 +130,7 @@ fn get_int_method( ) }; - pcid_handle - .enable_feature(PciFeature::MsiX) - .expect("xhcid: failed to enable MSI-X"); + pcid_handle.enable_feature(PciFeature::MsiX); log::debug!("Enabled MSI-X"); method @@ -181,8 +167,7 @@ fn main() { } fn daemon(daemon: redox_daemon::Daemon) -> ! { - let mut pcid_handle = - PciFunctionHandle::connect_default().expect("xhcid: failed to setup channel to pcid"); + let mut pcid_handle = PciFunctionHandle::connect_default(); let pci_config = pcid_handle.config(); let mut name = pci_config.func.name(); @@ -198,10 +183,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { log::debug!("XHCI PCI CONFIG: {:?}", pci_config); - let address = unsafe { pcid_handle.map_bar(0) } - .expect("xhcid") - .ptr - .as_ptr() as usize; + let address = unsafe { pcid_handle.map_bar(0) }.ptr.as_ptr() as usize; let (irq_file, interrupt_method) = (None, InterruptMethod::Polling); //get_int_method(&mut pcid_handle, address); //TODO: Fix interrupts.